Entity submission create-as-update by logging forceProcessed in source

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')
 ) {

lib/model/migrations/20241030-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 };

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);
 };
@@ -211,7 +211,7 @@ const _lockEntity = (exec, uuid) => {
 ////////////////////////////////////////////////////////////////////////////////
 // WRAPPER FUNCTIONS FOR CREATING AND UPDATING ENTITIES
 
-const _createEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent, forceOutOfOrderProcessing = false) => async ({ Audits, Entities }) => {
+const _createEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent, forceOutOfOrderProcessing) => async ({ Audits, Entities }) => {
   // If dataset requires approval on submission to create an entity and this event is not
   // an approval event, then don't create an entity
   if ((dataset.approvalRequired && event.details.reviewState !== 'approved') ||
@@ -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
+  // disrupting 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 (and should be parsed as such) but is applied as an update
+const _updateEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, forceOutOfOrderProcessing, 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,14 @@ 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, createSubAsUpdate) => async ({ Entities }) => {
+  if (createSubAsUpdate) {
+    // We are 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 +518,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 && err.problemDetails?.fields[0] === 'uuid') {
+        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) {

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 as an update', testOfflineEntities(async (service, container) => {
       const asAlice = await service.login('alice');
       const branchId = uuid();
 
@@ -1126,35 +1126,82 @@ 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);
+          should(body.conflict).equal(null); // conflict should be null after update-as-create
         });
 
       backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`);
       backlogCount.should.equal(0);
 
       // 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)
+        .set('Content-Type', 'application/xml')
+        .expect(200);
+
+      await exhaust(container);
+
+      await asAlice.get(`/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd`)
+        .expect(200)
+        .then(({ body }) => {
+          body.currentVersion.version.should.equal(2);
+          body.currentVersion.data.should.eql({ age: '20', status: 'new', first_name: 'Megan' });
+          body.conflict.should.equal('soft'); // this should be marked as a soft conflict
+          body.currentVersion.baseVersion.should.equal(1); // baseVersion is set, but normally the baseVersion of an entity-create is null
+          // the rest of these are null like a normal entity-create
+          should.not.exist(body.currentVersion.branchBaseVersion);
+          should.not.exist(body.currentVersion.trunkVersion);
+          should.not.exist(body.currentVersion.branchId);
+        });
+    }));
+
+    it('should verify that the create-as-update submission was parsed as a create even when applied as an update', testOfflineEntities(async (service, container) => {
+      const asAlice = await service.login('alice');
+      const branchId = uuid();
+
+      // Send first submission, which is an update that will be applied as a create
+      // Removing extra fields of the submission to demonstrate a simpler update with missing fields
       await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions')
         .send(testData.instances.offlineEntity.two
+          .replace('create="1"', 'update="1"')
           .replace('branchId=""', `branchId="${branchId}"`)
+          .replace('two', 'two-update')
+          .replace('baseVersion=""', 'baseVersion="1"')
+          .replace('<status>new</status>', '<status>checked in</status>')
+          .replace('<label>Megan (20)</label>', '')
+          .replace('<age>20</age>', '')
+          .replace('<name>Megan</name>', '')
         )
         .set('Content-Type', 'application/xml')
         .expect(200);
 
       await exhaust(container);
+      await container.Entities.processBacklog(true);
 
-      // 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')
+      // First submission creates the entity, but it should trigger an entity-processing error
+      // because there is no label
+      await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions')
+        .send(testData.instances.offlineEntity.two
+          .replace('<label>Megan (20)</label>', '')
+        )
+        .set('Content-Type', 'application/xml')
+        .expect(200);
+
+      await exhaust(container);
+
+      await asAlice.get(`/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd`)
         .expect(200)
         .then(({ body }) => {
-          body[0].details.errorMessage.should.eql('A resource already exists with uuid value(s) of 12345678-1234-4123-8234-123456789ddd.');
+          body.currentVersion.version.should.equal(1);
         });
 
-      await asAlice.get(`/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd`)
+      await asAlice.get('/v1/projects/1/forms/offlineEntity/submissions/two/audits')
         .expect(200)
         .then(({ body }) => {
-          body.currentVersion.version.should.equal(1);
+          body[0].action.should.equal('entity.error');
+          body[0].details.errorMessage.should.eql('Required parameter label missing.');
         });
     }));