diff --git a/cypress/e2e/api/SessionApi.spec.js b/cypress/e2e/api/SessionApi.spec.js index 36d8f94e1e5..82b7879f398 100644 --- a/cypress/e2e/api/SessionApi.spec.js +++ b/cypress/e2e/api/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) @@ -173,7 +173,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.save(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true }) cy.downloadFile(filePath) .its('data') @@ -184,7 +184,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.save(connection, { version: 1, autosaveContent: '# Heading 1', @@ -247,7 +247,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.save(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true }) cy.login(user) cy.prepareSessionApi() @@ -260,7 +260,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.save(connection, { version: 1, autosaveContent: '# Heading 1', @@ -331,7 +331,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 @@ -348,7 +348,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 b2c1ff1aa63..876c4f7dda0 100644 --- a/lib/Db/Step.php +++ b/lib/Db/Step.php @@ -37,9 +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; @@ -55,10 +63,13 @@ public function jsonSerialize(): array { if (\json_last_error() !== JSON_ERROR_NONE) { throw new \InvalidArgumentException('Failed to parse step data'); } + $version = $this->getVersion() == self::VERSION_STORED_IN_ID + ? $this->getId() + : $this->getVersion(); return [ 'id' => $this->getId(), 'data' => $jsonData, - 'version' => $this->getVersion(), + 'version' => $version, 'sessionId' => $this->getSessionId() ]; } diff --git a/lib/Db/StepMapper.php b/lib/Db/StepMapper.php index 11b7d59f894..8279f7d044b 100644 --- a/lib/Db/StepMapper.php +++ b/lib/Db/StepMapper.php @@ -36,19 +36,16 @@ public function __construct(IDBConnection $db) { /** * @return Step[] */ - public function find(int $documentId, int $fromVersion, int $lastAckedVersion = null): array { + public function find(int $documentId, int $fromVersion): array { /* @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); @@ -57,11 +54,11 @@ public function find(int $documentId, int $fromVersion, int $lastAckedVersion = public function getLatestVersion(int $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') ->executeQuery(); $data = $result->fetch(); @@ -69,7 +66,7 @@ public function getLatestVersion(int $documentId): ?int { return null; } - return $data['version']; + return $data['id']; } public function deleteAll(int $documentId): void { @@ -79,11 +76,12 @@ public function deleteAll(int $documentId): void { ->executeStatement(); } + // not in use right now public function deleteBeforeVersion(int $documentId, int $version): int { $qb = $this->db->getQueryBuilder(); return $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(); } @@ -92,6 +90,7 @@ public function deleteAfterVersion(int $documentId, int $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 29a86171f39..b73bdb8a2b0 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -263,15 +263,14 @@ private function insertSteps(Document $document, Session $session, array $steps, try { $stepsJson = json_encode($steps, JSON_THROW_ON_ERROR); $stepsVersion = $this->stepMapper->getLatestVersion($document->getId()); - $newVersion = $stepsVersion + count($steps); - $this->logger->debug("Adding steps to " . $document->getId() . ": bumping version from $stepsVersion to $newVersion"); - $this->cache->set('document-version-' . $document->getId(), $newVersion); $step = new Step(); $step->setData($stepsJson); $step->setSessionId($session->getId()); $step->setDocumentId($document->getId()); - $step->setVersion($newVersion); - $this->stepMapper->insert($step); + $step = $this->stepMapper->insert($step); + $newVersion = $step->getId(); + $this->logger->debug("Adding steps to " . $document->getId() . ": bumping version from $stepsVersion to $newVersion"); + $this->cache->set('document-version-' . $document->getId(), $newVersion); // TODO write steps to cache for quicker reading return $newVersion; } catch (\Throwable $e) {