From 8b507476ee4f3749fbcc5da1896c6a4aca50aba5 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Mon, 16 Sep 2024 13:11:41 -0700 Subject: [PATCH 1/4] Mark soft conflict if not contiguous with trunk --- lib/data/entity.js | 85 ++++++++++++++++++++++- test/integration/api/offline-entities.js | 86 ++++++++++++++++++++++++ 2 files changed, 170 insertions(+), 1 deletion(-) diff --git a/lib/data/entity.js b/lib/data/entity.js index 4acdf0525..b8fc72740 100644 --- a/lib/data/entity.js +++ b/lib/data/entity.js @@ -397,6 +397,64 @@ const diffEntityData = (defData) => { return diffs; }; +// Copied from frontend though it may not all be necessary +// Offline branch +class Branch { + // firstUpdate is the first offline update (not create) to be processed from + // the branch. entityRoot is the first version of the entity. + constructor(firstUpdate, entityRoot) { + if (firstUpdate.trunkVersion != null) { + // The first version from the branch to be processed (not necessarily the + // first in the original branch order) + this.first = firstUpdate; + + // How many versions that have been processed are from the branch? + this.length = 1; + + // Was this.first processed in branch order, or was it processed before an + // earlier change in the branch? + const { trunkVersion } = firstUpdate; + this.firstInOrder = firstUpdate.branchBaseVersion === trunkVersion; + + /* this.lastContiguousWithTrunk is the version number of the last version + from the branch that is contiguous with the trunk version. In other words, + it is the version number of the last version where there has been no + update from outside the branch between the version and the trunk version. + this.lastContiguousWithTrunk is not related to branch order: as long as + there hasn't been an update from outside the branch, the branch is + contiguous, regardless of the order of the updates within it. */ + this.lastContiguousWithTrunk = firstUpdate.version === trunkVersion + 1 + ? firstUpdate.version + : 0; + } else { + // If the entity was both created and updated offline before being sent to + // the server, then we treat the creation as part of the same branch as + // the update(s). The creation doesn't have a branch ID, but we treat it + // as part of the branch anyway. + this.first = entityRoot; + // If the submission for the entity creation was received late and + // processed out of order, then firstUpdate.version === 1. In that case, + // we can't reliably determine which entity version corresponds to the + // entity creation, so we don't treat the creation as part of the branch. + this.length = firstUpdate.version === 1 ? 1 : 2; + this.firstInOrder = this.length === 2; + this.lastContiguousWithTrunk = firstUpdate.version === 2 ? 2 : 1; + } + + this.id = firstUpdate.branchId; + // The last version from the branch to be processed + this.last = firstUpdate; + } + + add(version) { + this.length += 1; + this.last = version; + if (version.baseVersion === this.lastContiguousWithTrunk && + version.version === version.baseVersion + 1) + this.lastContiguousWithTrunk = version.version; + } +} + // Returns an array of properties which are different between // `dataReceived` and `otherVersionData` const getDiffProp = (dataReceived, otherVersionData) => @@ -419,6 +477,26 @@ const getWithConflictDetails = (defs, audits, relevantToConflict) => { const relevantBaseVersions = new Set(); + // build up branches + const branches = new Map(); + for (const version of defs) { + const { branchId } = version; + if (branchId != null && version.branchBaseVersion != null) { + const existingBranch = branches.get(branchId); + if (existingBranch == null) { + const newBranch = new Branch(version, defs[0]); + branches.set(branchId, newBranch); + version.branch = newBranch; + // If the entity was created offline, then add the branch to the + // entity creation. + newBranch.first.branch = newBranch; + } else { + existingBranch.add(version); + version.branch = existingBranch; + } + } + } + for (const def of defs) { const v = mergeLeft(def.forApi(), @@ -438,7 +516,12 @@ const getWithConflictDetails = (defs, audits, relevantToConflict) => { v.source = event.source; if (v.version > 1) { // v.root is false here - can use either - const conflict = v.version !== (v.baseVersion + 1); + let notContiguousWithTrunk = false; + if (v.branchId != null) { + notContiguousWithTrunk = branches.get(v.branchId).lastContiguousWithTrunk < v.baseVersion; + } + + const conflict = v.version !== (v.baseVersion + 1) || notContiguousWithTrunk; v.baseDiff = getDiffProp(v.dataReceived, { ...defs[v.baseVersion - 1].data, label: defs[v.baseVersion - 1].label }); diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index f13ec188c..5166947ae 100644 --- a/test/integration/api/offline-entities.js +++ b/test/integration/api/offline-entities.js @@ -1343,6 +1343,92 @@ describe('Offline Entities', () => { }); }); + describe('conflict cases', () => { + it('should mark an update that is not contiguous with its trunk version as a soft conflict', 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); + + // 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']); + }); + })); + + it('should mark an update that is not contiguous with its trunk version as a soft conflict', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + + // Send second update first + 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', 'checked in') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + await container.Entities.processBacklog(true); + + // Send first update now (it will be applied right away) + 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); + + // Send fourth update + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('branchId=""', `branchId="${branchId}"`) + .replace('one', 'one-update4') + .replace('baseVersion="1"', 'baseVersion="4"') + .replace('arrived', 'departed') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + await container.Entities.processBacklog(true); + + 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, 'hard', 'soft']); + }); + })); + }); + describe('locking an entity while processing a related submission', function() { this.timeout(8000); From 21a580d4aab993ac55cac1059650186e53e85840 Mon Sep 17 00:00:00 2001 From: Matthew White Date: Thu, 19 Sep 2024 09:16:18 -0400 Subject: [PATCH 2/4] Remove unneeded code and make other small changes --- lib/data/entity.js | 86 ++++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 53 deletions(-) diff --git a/lib/data/entity.js b/lib/data/entity.js index b8fc72740..160c64e83 100644 --- a/lib/data/entity.js +++ b/lib/data/entity.js @@ -397,25 +397,14 @@ const diffEntityData = (defData) => { return diffs; }; -// Copied from frontend though it may not all be necessary +// Copied from frontend // Offline branch class Branch { // firstUpdate is the first offline update (not create) to be processed from - // the branch. entityRoot is the first version of the entity. - constructor(firstUpdate, entityRoot) { + // the branch. + constructor(firstUpdate) { if (firstUpdate.trunkVersion != null) { - // The first version from the branch to be processed (not necessarily the - // first in the original branch order) - this.first = firstUpdate; - - // How many versions that have been processed are from the branch? - this.length = 1; - - // Was this.first processed in branch order, or was it processed before an - // earlier change in the branch? const { trunkVersion } = firstUpdate; - this.firstInOrder = firstUpdate.branchBaseVersion === trunkVersion; - /* this.lastContiguousWithTrunk is the version number of the last version from the branch that is contiguous with the trunk version. In other words, it is the version number of the last version where there has been no @@ -427,28 +416,30 @@ class Branch { ? firstUpdate.version : 0; } else { - // If the entity was both created and updated offline before being sent to - // the server, then we treat the creation as part of the same branch as - // the update(s). The creation doesn't have a branch ID, but we treat it - // as part of the branch anyway. - this.first = entityRoot; - // If the submission for the entity creation was received late and - // processed out of order, then firstUpdate.version === 1. In that case, - // we can't reliably determine which entity version corresponds to the - // entity creation, so we don't treat the creation as part of the branch. - this.length = firstUpdate.version === 1 ? 1 : 2; - this.firstInOrder = this.length === 2; + /* + If the entity was both created and updated offline before being sent to + the server, then there technically isn't a trunk version. On the flip + side, there also isn't a contiguity problem -- except in one special case. + If the submission for the entity creation is sent and processed, but the + submission for the update is not sent at the same time for some reason, + then it's possible for another update to be made between the two. Once the + original update is sent and processed, it will no longer be contiguous + with the entity creation. + + Another special case is if the submission for the entity creation was sent + late and processed out of order. In that case, firstUpdate.version === 1. + There's again no contiguity problem (just an order problem), so + lastContiguousWithTrunk should equal 1. + + The normal case is if firstUpdate.version === 2. + */ this.lastContiguousWithTrunk = firstUpdate.version === 2 ? 2 : 1; } this.id = firstUpdate.branchId; - // The last version from the branch to be processed - this.last = firstUpdate; } add(version) { - this.length += 1; - this.last = version; if (version.baseVersion === this.lastContiguousWithTrunk && version.version === version.baseVersion + 1) this.lastContiguousWithTrunk = version.version; @@ -477,27 +468,18 @@ const getWithConflictDetails = (defs, audits, relevantToConflict) => { const relevantBaseVersions = new Set(); - // build up branches const branches = new Map(); - for (const version of defs) { - const { branchId } = version; - if (branchId != null && version.branchBaseVersion != null) { - const existingBranch = branches.get(branchId); - if (existingBranch == null) { - const newBranch = new Branch(version, defs[0]); - branches.set(branchId, newBranch); - version.branch = newBranch; - // If the entity was created offline, then add the branch to the - // entity creation. - newBranch.first.branch = newBranch; - } else { - existingBranch.add(version); - version.branch = existingBranch; - } - } - } for (const def of defs) { + // build up branches + const { branchId } = def; + if (branchId != null) { + const existingBranch = branches.get(branchId); + if (existingBranch == null) + branches.set(branchId, new Branch(def)); + else + existingBranch.add(def); + } const v = mergeLeft(def.forApi(), { @@ -516,12 +498,10 @@ const getWithConflictDetails = (defs, audits, relevantToConflict) => { v.source = event.source; if (v.version > 1) { // v.root is false here - can use either - let notContiguousWithTrunk = false; - if (v.branchId != null) { - notContiguousWithTrunk = branches.get(v.branchId).lastContiguousWithTrunk < v.baseVersion; - } - - const conflict = v.version !== (v.baseVersion + 1) || notContiguousWithTrunk; + const baseNotContiguousWithTrunk = v.branchId != null && + branches.get(v.branchId).lastContiguousWithTrunk < v.baseVersion; + const conflict = v.version !== (v.baseVersion + 1) || + baseNotContiguousWithTrunk; v.baseDiff = getDiffProp(v.dataReceived, { ...defs[v.baseVersion - 1].data, label: defs[v.baseVersion - 1].label }); From f4d54fc0b6970fe0bc6ab28de8468e7cc31123b1 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Mon, 21 Oct 2024 15:20:57 -0700 Subject: [PATCH 3/4] 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() { From 9dd9db032e61109dc0091668abfb55e4f9d43ea6 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Mon, 28 Oct 2024 13:07:22 -0700 Subject: [PATCH 4/4] add more comments and explanation to tests --- lib/model/query/entities.js | 3 +- test/integration/api/offline-entities.js | 54 ++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index c7a34e6a5..edf7a3ffe 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -304,14 +304,13 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss serverDiffData.label = serverEntity.aux.currentVersion.label; conflictingProperties = Object.keys(clientEntity.def.dataReceived).filter(key => key in serverDiffData && clientEntity.def.dataReceived[key] !== serverDiffData[key]); - 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) { + if (interrupted && conflict !== ConflictType.HARD) { conflict = ConflictType.SOFT; } } diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index c2bd88e7e..1a0124175 100644 --- a/test/integration/api/offline-entities.js +++ b/test/integration/api/offline-entities.js @@ -1353,7 +1353,9 @@ describe('Offline Entities', () => { .send({ data: { age: '24' } }) .expect(200); - // Send update (change status from null to arrived) + // Send update (change status from null to arrived, no other properties included/changed) + // Introduces a soft conflict because baseVersion is 1 + // But no hard conflict await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') .send(testData.instances.offlineEntity.one .replace('branchId=""', `branchId="${branchId}"`) @@ -1361,7 +1363,10 @@ describe('Offline Entities', () => { .set('Content-Type', 'application/xml') .expect(200); - // Send second update (change age from 22 to 26) + // Send second update (change age from 22 to 26, instead of changing status) + // Doesn't conflict with previous version (from the same offline branch) + // But it should be marked as a soft conflict because the branch was + // interrupted by the first API update. await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') .send(testData.instances.offlineEntity.one .replace('branchId=""', `branchId="${branchId}"`) @@ -1374,16 +1379,26 @@ describe('Offline Entities', () => { await exhaust(container); + // Final version should have soft conflict 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']); }); + + // Overall entity should have soft conflict + // (A test below shows this is set explicitily and not just carried over + // from the previous conflict state.) + 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 (due to force processing) as a soft conflict', testOfflineEntities(async (service, container) => { const asAlice = await service.login('alice'); const branchId = uuid(); + // Scenario described in issue: c#698 // Send second update first await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') .send(testData.instances.offlineEntity.one @@ -1399,6 +1414,9 @@ describe('Offline Entities', () => { await container.Entities.processBacklog(true); // Send first update now (it will be applied right away) + // Introduces a hard conflict because it will find baseVersion v1 + // and the force-processed update above also branched of v1 + // and both update 'status'. await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') .send(testData.instances.offlineEntity.one .replace('branchId=""', `branchId="${branchId}"`) @@ -1408,7 +1426,13 @@ describe('Offline Entities', () => { await exhaust(container); - // Send fourth update + // Entity conflict should be hard at this point + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') + .then(({ body: entity }) => { + should(entity.conflict).equal('hard'); + }); + + // Send fourth update (skipping a 3rd update so this must be force-applied) await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') .send(testData.instances.offlineEntity.one .replace('branchId=""', `branchId="${branchId}"`) @@ -1422,10 +1446,19 @@ describe('Offline Entities', () => { await exhaust(container); await container.Entities.processBacklog(true); + // All updates are from the same branch, but 1 expected version is missing + // and updates came in out of order. Unclear if final 'soft' conflict is what we + // want or if it should possibly be null. 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, 'hard', 'soft']); }); + + // Hard conflict is carried forward + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') + .then(({ body: entity }) => { + should(entity.conflict).equal('hard'); + }); })); 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) => { @@ -1437,7 +1470,9 @@ describe('Offline Entities', () => { .send({ data: { age: '24' } }) .expect(200); - // Send update (change status from null to arrived) + // Send update (change status from null to arrived, don't change age) + // Has soft conflict with parallel update but doesn't have hard conflict + // because different properties were changed. await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') .send(testData.instances.offlineEntity.one .replace('branchId=""', `branchId="${branchId}"`) @@ -1446,10 +1481,19 @@ describe('Offline Entities', () => { .expect(200); await exhaust(container); + // Entity has soft conflict at this point + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') + .then(({ body: entity }) => { + should(entity.conflict).equal('soft'); + }); + 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) + // Doesn't conflict with previous version (from the same offline branch) + // But it should be marked as a soft conflict because the branch was + // interrupted by the first API update. await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') .send(testData.instances.offlineEntity.one .replace('branchId=""', `branchId="${branchId}"`) @@ -1461,11 +1505,13 @@ describe('Offline Entities', () => { .expect(200); await exhaust(container); + // Final version conflict is soft because of interrupted branch 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']); }); + // Entity version is soft await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') .then(({ body: entity }) => { should(entity.conflict).equal('soft');