diff --git a/cypress/e2e/SessionApi.spec.js b/cypress/e2e/SessionApi.spec.js index 3a3276fc483..863efede0c7 100644 --- a/cypress/e2e/SessionApi.spec.js +++ b/cypress/e2e/SessionApi.spec.js @@ -115,7 +115,7 @@ describe('The session Api', function() { const version = 0 cy.pushSteps({ connection, steps, version }) .its('version') - .should('eql', 1) + .should('be.at.least', 1) cy.syncSteps(connection) .its('steps[0].data') .should('eql', steps) @@ -170,7 +170,7 @@ describe('The session Api', function() { it('saves', function() { cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('eql', 1) + .should('be.at.least', 1) cy.syncSteps(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true }) cy.downloadFile(filePath) .its('data') @@ -181,7 +181,7 @@ describe('The session Api', function() { const documentState = 'Base64 encoded string' cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('eql', 1) + .should('be.at.least', 1) cy.syncSteps(connection, { version: 1, autosaveContent: '# Heading 1', @@ -244,7 +244,7 @@ describe('The session Api', function() { it('saves public', function() { cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('eql', 1) + .should('be.at.least', 1) cy.syncSteps(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true }) cy.login(user) cy.prepareSessionApi() @@ -257,7 +257,7 @@ describe('The session Api', function() { const documentState = 'Base64 encoded string' cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('eql', 1) + .should('be.at.least', 1) cy.syncSteps(connection, { version: 1, autosaveContent: '# Heading 1', @@ -328,7 +328,7 @@ describe('The session Api', function() { let joining cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('eql', 1) + .should('be.at.least', 1) cy.createTextSession(undefined, { filePath: '', shareToken }) .then(con => { joining = con @@ -345,7 +345,7 @@ describe('The session Api', function() { cy.log('Initial user pushes steps') cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('eql', 1) + .should('be.at.least', 1) cy.log('Other user creates session') cy.createTextSession(undefined, { filePath: '', shareToken }) .then(con => { diff --git a/lib/Db/Step.php b/lib/Db/Step.php index 92ae39d2d08..0898f515ee9 100644 --- a/lib/Db/Step.php +++ b/lib/Db/Step.php @@ -37,8 +37,17 @@ * @method setDocumentId(int $documentId): void */ class Step extends Entity implements JsonSerializable { + + /* + * Transition: We now use the auto-incrementing id as the version. + * To ensure that new steps always have a larger version than those that + * used the version field, use the largest possible value for BIGINT. + */ + public const VERSION_STORED_IN_ID = 18446744073709551615; + + public $id = null; protected string $data = ''; - protected int $version = 0; + protected int $version = self::VERSION_STORED_IN_ID; protected int $sessionId = 0; protected int $documentId = 0; @@ -54,10 +63,13 @@ public function jsonSerialize(): array { if (\json_last_error() !== JSON_ERROR_NONE) { throw new \InvalidArgumentException('Failed to parse step data'); } + $version = $this->version === self::VERSION_STORED_IN_ID + ? $this->id + : $this->getVersion(); return [ 'id' => $this->id, 'data' => $jsonData, - 'version' => $this->version, + 'version' => $version, 'sessionId' => $this->sessionId ]; } diff --git a/lib/Db/StepMapper.php b/lib/Db/StepMapper.php index 85838c36fc8..39cb1baea37 100644 --- a/lib/Db/StepMapper.php +++ b/lib/Db/StepMapper.php @@ -33,19 +33,16 @@ public function __construct(IDBConnection $db) { parent::__construct($db, 'text_steps', Step::class); } - public function find($documentId, $fromVersion, $lastAckedVersion = null) { + public function find($documentId, $fromVersion) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from($this->getTableName()) ->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId))) - ->andWhere($qb->expr()->gt('version', $qb->createNamedParameter($fromVersion))); - if ($lastAckedVersion) { - $qb->andWhere($qb->expr()->lte('version', $qb->createNamedParameter($lastAckedVersion))); - } + ->andWhere($qb->expr()->gt('version', $qb->createNamedParameter($fromVersion))) + ->andWhere($qb->expr()->gt('id', $qb->createNamedParameter($fromVersion))); $qb ->setMaxResults(1000) - ->orderBy('version') ->orderBy('id'); return $this->findEntities($qb); @@ -54,11 +51,11 @@ public function find($documentId, $fromVersion, $lastAckedVersion = null) { public function getLatestVersion($documentId): ?int { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $result = $qb->select('version') + $result = $qb->select('id') ->from($this->getTableName()) ->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId))) ->setMaxResults(1) - ->orderBy('version', 'DESC') + ->orderBy('id', 'DESC') ->execute(); $data = $result->fetch(); @@ -66,7 +63,7 @@ public function getLatestVersion($documentId): ?int { return null; } - return $data['version']; + return $data['id']; } public function deleteAll($documentId): void { @@ -76,11 +73,12 @@ public function deleteAll($documentId): void { ->executeStatement(); } + // not in use right now public function deleteBeforeVersion($documentId, $version): void { $qb = $this->db->getQueryBuilder(); $qb->delete($this->getTableName()) ->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId))) - ->andWhere($qb->expr()->lte('version', $qb->createNamedParameter($version))) + ->andWhere($qb->expr()->lte('id', $qb->createNamedParameter($version))) ->executeStatement(); } @@ -89,6 +87,7 @@ public function deleteAfterVersion($documentId, $version): int { return $qb->delete($this->getTableName()) ->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId))) ->andWhere($qb->expr()->gt('version', $qb->createNamedParameter($version))) + ->andWhere($qb->expr()->gt('id', $qb->createNamedParameter($version))) ->executeStatement(); } } diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index 1bf5f8b1fec..6a59640f7ab 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -274,15 +274,14 @@ private function insertSteps($documentId, $sessionId, $steps, $version): int { throw new InvalidArgumentException('Failed to encode steps'); } $stepsVersion = $this->stepMapper->getLatestVersion($document->getId()); - $newVersion = $stepsVersion + count($steps); - $this->logger->debug("Adding steps to $documentId: bumping version from $stepsVersion to $newVersion"); - $this->cache->set('document-version-' . $document->getId(), $newVersion); $step = new Step(); $step->setData($stepsJson); $step->setSessionId($sessionId); $step->setDocumentId($documentId); - $step->setVersion($newVersion); - $this->stepMapper->insert($step); + $step = $this->stepMapper->insert($step); + $newVersion = $step->getId(); + $this->logger->debug("Adding steps to " . $documentId . ": bumping version from $stepsVersion to $newVersion"); + $this->cache->set('document-version-' . $documentId, $newVersion); // TODO write steps to cache for quicker reading return $newVersion; } catch (DoesNotExistException $e) {