diff --git a/lib/model/frames/entity.js b/lib/model/frames/entity.js index 42359f47d..a36be0038 100644 --- a/lib/model/frames/entity.js +++ b/lib/model/frames/entity.js @@ -124,6 +124,7 @@ Entity.Def.Source = class extends Frame.define( table('entity_def_sources', 'source'), 'details', readable, 'submissionDefId', 'auditId', + 'forceProcessed', embedded('submissionDef'), embedded('audit') ) { diff --git a/lib/model/migrations/20241028-01-add-force-entity-def-source.js b/lib/model/migrations/20241028-01-add-force-entity-def-source.js new file mode 100644 index 000000000..1c54134c5 --- /dev/null +++ b/lib/model/migrations/20241028-01-add-force-entity-def-source.js @@ -0,0 +1,20 @@ +// Copyright 2024 ODK Central Developers +// See the NOTICE file at the top-level directory of this distribution and at +// https://github.com/getodk/central-backend/blob/master/NOTICE. +// This file is part of ODK Central. It is subject to the license terms in +// the LICENSE file found in the top-level directory of this distribution and at +// https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central, +// including this file, may be copied, modified, propagated, or distributed +// except according to the terms contained in the LICENSE file. + +const up = (db) => db.raw(` + ALTER TABLE entity_def_sources + ADD COLUMN "forceProcessed" BOOLEAN +`); + +const down = (db) => db.raw(` + ALTER TABLE entity_def_sources + DROP COLUMN "forceProcessed" +`); + +module.exports = { up, down }; diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index edf7a3ffe..8f3516564 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -24,10 +24,10 @@ const { getOrReject, runSequentially } = require('../../util/promise'); ///////////////////////////////////////////////////////////////////////////////// // ENTITY DEF SOURCES -const createSource = (details = null, subDefId = null, eventId = null) => ({ one }) => { +const createSource = (details = null, subDefId = null, eventId = null, forceProcessed = false) => ({ one }) => { const type = (subDefId) ? 'submission' : 'api'; - return one(sql`insert into entity_def_sources ("type", "submissionDefId", "auditId", "details") - values (${type}, ${subDefId}, ${eventId}, ${JSON.stringify(details)}) + return one(sql`insert into entity_def_sources ("type", "submissionDefId", "auditId", "details", "forceProcessed") + values (${type}, ${subDefId}, ${eventId}, ${JSON.stringify(details)}, ${forceProcessed}) returning id`) .then((row) => row.id); }; @@ -230,8 +230,14 @@ const _createEntity = (dataset, entityData, submissionId, submissionDef, submiss ? _partial.auxWith('def', { branchBaseVersion: _partial.def.baseVersion }) : _partial; + // Because of entity processing flow, we need to check for a conflict explicitly without + // distrupting the database transaction. + const maybeEntity = await Entities.getById(dataset.id, partial.uuid); + if (maybeEntity.isDefined()) + throw Problem.user.uniquenessViolation({ fields: ['uuid'], values: partial.uuid }); + const sourceDetails = { submission: { instanceId: submissionDef.instanceId }, parentEventId: parentEvent ? parentEvent.id : undefined }; - const sourceId = await Entities.createSource(sourceDetails, submissionDefId, event.id); + const sourceId = await Entities.createSource(sourceDetails, submissionDefId, event.id, forceOutOfOrderProcessing); const entity = await Entities.createNew(dataset, partial, submissionDef, sourceId); await Audits.log({ id: event.actorId }, 'entity.create', { acteeId: dataset.acteeId }, @@ -245,14 +251,17 @@ const _createEntity = (dataset, entityData, submissionId, submissionDef, submiss return entity; }; -const _updateEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, forceOutOfOrderProcessing = false) => async ({ Audits, Entities }) => { +// Extra flags +// - forceOutOfOrderProcessing: entity was in backlog and was force-processed, which affects base version calculation +// - createSubAsUpdate: submission was meant to *create* entity but is being parsed and applied as an update +const _updateEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, forceOutOfOrderProcessing = false, createSubAsUpdate = false) => async ({ Audits, Entities }) => { if (!(event.action === 'submission.create' || event.action === 'submission.update.version' || event.action === 'submission.reprocess')) return null; // Get client version of entity - const clientEntity = await Entity.fromParseEntityData(entityData, { update: true }); // validation happens here + const clientEntity = await Entity.fromParseEntityData(entityData, createSubAsUpdate ? { create: true } : { update: true }); // validation happens here // Figure out the intended baseVersion // If this is an offline update with a branchId, the baseVersion value is local to that offline context. @@ -261,7 +270,7 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss // Try computing base version. // But if there is a 404.8 not found error, double-check if the entity never existed or was deleted. try { - baseEntityDef = await Entities._computeBaseVersion(event.id, dataset, clientEntity, submissionDef, forceOutOfOrderProcessing); + baseEntityDef = await Entities._computeBaseVersion(event.id, dataset, clientEntity, submissionDef, forceOutOfOrderProcessing, createSubAsUpdate); } catch (err) { if (err.problemCode === 404.8) { // Look up deleted entity by passing deleted as option argData @@ -307,6 +316,8 @@ 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 if (createSubAsUpdate) { + conflict = 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); @@ -323,7 +334,7 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss const sourceDetails = { submission: { instanceId: submissionDef.instanceId } }; - const sourceId = await Entities.createSource(sourceDetails, submissionDefId, event.id); + const sourceId = await Entities.createSource(sourceDetails, submissionDefId, event.id, forceOutOfOrderProcessing); const partial = new Entity.Partial(serverEntity.with({ conflict }), { def: new Entity.Def({ data: mergedData, @@ -356,8 +367,16 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss // Used by _updateVerison to figure out the intended base version in Central // based on the branchId, trunkVersion, and baseVersion in the submission -const _computeBaseVersion = (eventId, dataset, clientEntity, submissionDef, forceOutOfOrderProcessing = false) => async ({ Entities }) => { - if (!clientEntity.def.branchId) { +const _computeBaseVersion = (eventId, dataset, clientEntity, submissionDef, forceOutOfOrderProcessing = false, createSubAsUpdate = false) => async ({ Entities }) => { + if (clientEntity.def.baseVersion == null && createSubAsUpdate) { + // if no baseVersion is specified but we are updating and trying to find the base version + // we are probably in the special case of force-apply create-as-update. get the latest version. + + const latestEntity = await Entities.getById(dataset.id, clientEntity.uuid) + .then(getOrReject(Problem.user.entityNotFound({ entityUuid: clientEntity.uuid, datasetName: dataset.name }))); + return latestEntity.aux.currentVersion; + + } else if (!clientEntity.def.branchId) { // no offline branching to deal with, use baseVersion as is const condition = { version: clientEntity.def.baseVersion }; @@ -501,8 +520,25 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Audits, Dataset throw (err); } } - else if (entityData.system.create === '1' || entityData.system.create === 'true') - maybeEntity = await Entities._createEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent, forceOutOfOrderProcessing); + else if (entityData.system.create === '1' || entityData.system.create === 'true') { + try { + maybeEntity = await Entities._createEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent, forceOutOfOrderProcessing); + } catch (err) { + // There was a problem creating the entity + // If it is a uuid collision, check if the entity was created via an update + // in which case its ok to apply this create as an update + if (err.problemCode === 409.3) { + const rootDef = await Entities.getDef(dataset.id, entityData.system.id, new QueryOptions({ root: true })).then(o => o.orNull()); + if (rootDef && rootDef.aux.source.forceProcessed) { + maybeEntity = await Entities._updateEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, forceOutOfOrderProcessing, true); + } else { + throw (err); + } + } else { + throw (err); + } + } + } // Check for held submissions that follow this one in the same branch if (maybeEntity != null) { diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index 1a0124175..b5eaea4d4 100644 --- a/test/integration/api/offline-entities.js +++ b/test/integration/api/offline-entities.js @@ -1091,7 +1091,7 @@ describe('Offline Entities', () => { backlogCount.should.equal(0); })); - it.skip('should apply an entity update as a create, and then properly handle the delayed create', testOfflineEntities(async (service, container) => { + it('should apply an entity update as a create, and then properly handle the delayed create', testOfflineEntities(async (service, container) => { const asAlice = await service.login('alice'); const branchId = uuid(); @@ -1126,35 +1126,33 @@ describe('Offline Entities', () => { body.currentVersion.data.should.eql({ status: 'checked in' }); body.currentVersion.label.should.eql('auto generated'); body.currentVersion.branchId.should.equal(branchId); + body.currentVersion.branchBaseVersion.should.equal(1); should.not.exist(body.currentVersion.baseVersion); - should.not.exist(body.currentVersion.branchBaseVersion); // No base version because this is a create, though maybe this should be here. should.not.exist(body.currentVersion.trunkVersion); }); backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); backlogCount.should.equal(0); + await asAlice.get(`/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd`) + .expect(200) + .then(({ body }) => { + body.currentVersion.data.should.eql({ status: 'checked in' }); + }); + // First submission creates the entity, but this will be processed as an update await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') - .send(testData.instances.offlineEntity.two - .replace('branchId=""', `branchId="${branchId}"`) - ) + .send(testData.instances.offlineEntity.two) .set('Content-Type', 'application/xml') .expect(200); await exhaust(container); - // In the default behavior, attempting create on an entity that already exists causes a conflict error. - await asAlice.get('/v1/projects/1/forms/offlineEntity/submissions/two/audits') - .expect(200) - .then(({ body }) => { - body[0].details.errorMessage.should.eql('A resource already exists with uuid value(s) of 12345678-1234-4123-8234-123456789ddd.'); - }); - await asAlice.get(`/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd`) .expect(200) .then(({ body }) => { - body.currentVersion.version.should.equal(1); + body.currentVersion.version.should.equal(2); + body.currentVersion.data.should.eql({ age: '20', status: 'new', first_name: 'Megan' }); }); }));