From e86d3b5af32669092b515ac03e35c6ece0c27747 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Mon, 21 Oct 2024 15:20:57 -0700 Subject: [PATCH] query to check for non-contiguous interrupted branches --- lib/model/query/entities.js | 31 ++++- test/integration/api/offline-entities.js | 146 ++++++++++++++++++++++- 2 files changed, 175 insertions(+), 2 deletions(-) diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index 59c1a37bb..c7a34e6a5 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -308,6 +308,12 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss if (conflict !== ConflictType.HARD) { // We don't want to downgrade conflict here conflict = conflictingProperties.length > 0 ? ConflictType.HARD : ConflictType.SOFT; } + } else { + // This may still be a soft conflict if the new version is not contiguous with this branch's trunk version + const interrupted = await Entities._interruptedBranch(serverEntity.id, clientEntity); + if (interrupted) { + conflict = ConflictType.SOFT; + } } // merge data @@ -541,6 +547,29 @@ const processSubmissionEvent = (event, parentEvent) => (container) => //////////////////////////////////////////////////////////////////////////////// // Submission processing helper functions +// Used by _updateEntity to determine if a new offline update is contiguous with its trunk version +// by searching for an interrupting version with a different or null branchId that has a higher +// version than the trunk version of the given branch. +const _interruptedBranch = (entityId, clientEntity) => async ({ maybeOne }) => { + // If there is no branchId, the branch cannot be interrupted + if (clientEntity.def.branchId == null) + return false; + + // look for a version of a different branch that has a version + // higher than the trunkVersion, which indicates an interrupting version. + // if trunkVersion is null (becuase it is part of a branch beginning with + // an offline create), look for a version higher than 1 because version + // 1 is implicitly the create action of that offline branch. + const interruptingVersion = await maybeOne(sql` + SELECT version + FROM entity_defs + WHERE "branchId" IS DISTINCT FROM ${clientEntity.def.branchId} + AND version > ${clientEntity.def.trunkVersion || 1} + AND "entityId" = ${entityId} + LIMIT 1`); + return interruptingVersion.isDefined(); +}; + // Used by _computeBaseVersion to hold submissions that are not yet ready to be processed const _holdSubmission = (eventId, submissionId, submissionDefId, entityUuid, branchId, branchBaseVersion) => async ({ run }) => run(sql` INSERT INTO entity_submission_backlog ("auditId", "submissionId", "submissionDefId", "entityUuid", "branchId", "branchBaseVersion", "loggedAt") @@ -780,7 +809,7 @@ module.exports = { createSource, createMany, _createEntity, _updateEntity, - _computeBaseVersion, + _computeBaseVersion, _interruptedBranch, _holdSubmission, _checkHeldSubmission, _getNextHeldSubmissionInBranch, _deleteHeldSubmissionByEventId, _getHeldSubmissionsAsEvents, diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index 5166947ae..c2bd88e7e 100644 --- a/test/integration/api/offline-entities.js +++ b/test/integration/api/offline-entities.js @@ -1380,7 +1380,7 @@ describe('Offline Entities', () => { }); })); - it('should mark an update that is not contiguous with its trunk version as a soft conflict', testOfflineEntities(async (service, container) => { + it('should mark an update that is not contiguous (due to force processing) as a soft conflict', testOfflineEntities(async (service, container) => { const asAlice = await service.login('alice'); const branchId = uuid(); @@ -1427,6 +1427,150 @@ describe('Offline Entities', () => { versions.map(v => v.conflict).should.eql([null, null, 'hard', 'soft']); }); })); + + it('should mark an update that is not contiguous with its trunk version as a soft conflict on entity despite earlier conflict resolution', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + + // Update existing entity on server (change age from 22 to 24) + await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?baseVersion=1') + .send({ data: { age: '24' } }) + .expect(200); + + // Send update (change status from null to arrived) + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('branchId=""', `branchId="${branchId}"`) + ) + .set('Content-Type', 'application/xml') + .expect(200); + await exhaust(container); + + await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true&baseVersion=3') + .expect(200); + + // Send second update (change age from 22 to 26) + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('branchId=""', `branchId="${branchId}"`) + .replace('one', 'one-update2') + .replace('baseVersion="1"', 'baseVersion="2"') + .replace('arrived', '26') + ) + .set('Content-Type', 'application/xml') + .expect(200); + await exhaust(container); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc/versions') + .then(({ body: versions }) => { + versions.map(v => v.conflict).should.eql([null, null, 'soft', 'soft']); + }); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') + .then(({ body: entity }) => { + should(entity.conflict).equal('soft'); + }); + })); + + it('should mark an update that is not contiguous (from an offline create branch) as a soft conflict on entity despite earlier conflict resolution', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + + // Send initial submission to create entity + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.two) + .set('Content-Type', 'application/xml') + .expect(200); + await exhaust(container); + + // Update existing entity on server before getting the rest of the branch + await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd?baseVersion=1') + .send({ data: { age: '24' } }) + .expect(200); + + // Send update (change status from new to arrived) + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.two + .replace('two', 'two-update1') + .replace('branchId=""', `branchId="${branchId}"`) + .replace('create="1"', 'update="1"') + .replace('baseVersion=""', 'baseVersion="1"') + .replace('new', 'arrived') + ) + .set('Content-Type', 'application/xml') + .expect(200); + await exhaust(container); + + // Conflict is hard here + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd') + .then(({ body: entity }) => { + should(entity.conflict).equal('hard'); + }); + + // resolve the conflict + await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd?resolve=true&baseVersion=3') + .expect(200); + + // Send second update in offline create-update-update chain (change age from 22 to 26) + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.two + .replace('two', 'two-update2') + .replace('branchId=""', `branchId="${branchId}"`) + .replace('create="1"', 'update="1"') + .replace('baseVersion=""', 'baseVersion="2"') + .replace('new', 'arrived') + .replace('20', '27') + ) + .set('Content-Type', 'application/xml') + .expect(200); + await exhaust(container); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd/versions') + .then(({ body: versions }) => { + versions.map(v => v.conflict).should.eql([null, null, 'hard', 'soft']); + }); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd') + .then(({ body: entity }) => { + should(entity.conflict).equal('soft'); + }); + })); + + it('should check that interrupting version logic is doesnt flag non-conflicts as conflicts', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + + // Send initial submission to create entity + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.two) + .set('Content-Type', 'application/xml') + .expect(200); + await exhaust(container); + + // Send second update in offline create-update-update chain (change age from 22 to 26) + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.two + .replace('two', 'two-update') + .replace('branchId=""', `branchId="${branchId}"`) + .replace('create="1"', 'update="1"') + .replace('baseVersion=""', 'baseVersion="1"') + .replace('new', 'arrived') + .replace('20', '27') + ) + .set('Content-Type', 'application/xml') + .expect(200); + await exhaust(container); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd/versions') + .then(({ body: versions }) => { + versions.map(v => v.conflict).should.eql([null, null]); + }); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd') + .then(({ body: entity }) => { + should(entity.conflict).equal(null); + }); + })); }); describe('locking an entity while processing a related submission', function() {