Different checking of baseVersion in entity upsert case

lib/data/entity.js

@@ -55,9 +55,10 @@ const extractLabelFromSubmission = (entity, options = { create: true }) => {
 };
 
 // Input: object representing entity, parsed from submission XML
-const extractBaseVersionFromSubmission = (entity) => {
-  const { baseVersion, update } = entity.system;
-  if ((update === '1' || update === 'true')) {
+const extractBaseVersionFromSubmission = (entity, options = { update: true }) => {
+  const { update } = options;
+  const { baseVersion } = entity.system;
+  if (update) {
     if (!baseVersion)
       throw Problem.user.missingParameter({ field: 'baseVersion' });
 

lib/model/frames/entity.js

@@ -39,7 +39,7 @@ class Entity extends Frame.define(
     // validation for each field happens within each function
     const uuid = normalizeUuid(entityData.system.id);
     const label = extractLabelFromSubmission(entityData, options);
-    const baseVersion = extractBaseVersionFromSubmission(entityData);
+    const baseVersion = extractBaseVersionFromSubmission(entityData, options);
     const branchId = extractBranchIdFromSubmission(entityData);
     const trunkVersion = extractTrunkVersionFromSubmission(entityData);
     const dataReceived = { ...data, ...(label && { label }) };

lib/model/query/entities.js

@@ -486,7 +486,11 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Audits, Dataset
       maybeEntity = await Entities._updateEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, forceOutOfOrderProcessing);
     } catch (err) {
       const attemptCreate = (entityData.system.create === '1' || entityData.system.create === 'true') || forceOutOfOrderProcessing;
-      if ((err.problemCode === 404.8) && attemptCreate) {
+      // The two cases we attempt to create after a failed update:
+      // 1. entity not found
+      // 2. baseVersion is empty and failed to parse in the update case
+      // This second one is a special case related to issue c#727
+      if ((err.problemCode === 404.8 || err.problemCode === 400.2) && attemptCreate) {
         maybeEntity = await Entities._createEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent, forceOutOfOrderProcessing);
       } else {
         throw (err);

test/integration/api/entities.js

@@ -3238,6 +3238,53 @@ describe('Entities API', () => {
             logs[1].action.should.be.eql('submission.create');
           });
       }));
+
+      it('should create entity when both create and update are true but baseVersion is empty', testEntityUpdates(async (service, container) => {
+        const asAlice = await service.login('alice');
+
+        await asAlice.post('/v1/projects/1/forms/updateEntity/submissions')
+          .send(testData.instances.updateEntity.one
+            .replace('id="12345678-1234-4123-8234-123456789abc"', 'id="12345678-1234-4123-8234-123456789ddd"')
+            .replace('update="1"', 'create="1" update="1"')
+            .replace('baseVersion="1"', 'baseVersion=""'))
+          .expect(200);
+
+        await exhaust(container);
+        await asAlice.get('/v1/projects/1/datasets/people/entities')
+          .expect(200)
+          .then(({ body: people }) => {
+            // existing entity + new entity
+            people.length.should.be.eql(2);
+          });
+      }));
+
+      it('should log error when both create and update are true, baseVersion is empty but entity uuid exists', testEntityUpdates(async (service, container) => {
+        const asAlice = await service.login('alice');
+
+        // This submission uses an existing entity uuid
+        // has create and update both set to true
+        // and is missing a baseVersion.
+        // This will attempt to update but fail because it finds no
+        // baseVersion to parse in the update case
+        // and will then attempt to create but fail because
+        // the entity uuid does actually exist.
+        await asAlice.post('/v1/projects/1/forms/updateEntity/submissions')
+          .send(testData.instances.updateEntity.one
+            .replace('update="1"', 'create="1" update="1"')
+            .replace('baseVersion="1"', 'baseVersion=""'))
+          .expect(200);
+
+        await exhaust(container);
+
+        await asAlice.get('/v1/projects/1/forms/updateEntity/submissions/one/audits')
+          .expect(200)
+          .then(({ body: logs }) => {
+            logs[0].action.should.be.eql('entity.error');
+            logs[0].details.problem.problemCode.should.be.eql(409.3);
+            logs[0].details.errorMessage.should.be.eql('A resource already exists with uuid value(s) of 12345678-1234-4123-8234-123456789abc.');
+            logs[1].action.should.be.eql('submission.create');
+          });
+      }));
     });
 
     // 10 examples described in central issue #517

test/unit/data/entity.js

@@ -131,32 +131,26 @@ describe('extracting and validating entities', () => {
 
     describe('extractBaseVersionFromSubmission', () => {
       it('should extract integer base version when update is true', () => {
-        const entity = { system: { update: '1', baseVersion: '99' } };
-        extractBaseVersionFromSubmission(entity).should.equal(99);
+        const entity = { system: { baseVersion: '99' } };
+        extractBaseVersionFromSubmission(entity, { update: true }).should.equal(99);
       });
 
       it('not return base version if create is true because it is not relevant', () => {
-        const entity = { system: { create: '1', baseVersion: '99' } };
-        should.not.exist(extractBaseVersionFromSubmission(entity));
-      });
-
-      it('not return base version if neither create nor update are provided', () => {
         const entity = { system: { baseVersion: '99' } };
-        should.not.exist(extractBaseVersionFromSubmission(entity));
+        should.not.exist(extractBaseVersionFromSubmission(entity, { create: true }));
       });
 
-      it('should complain if baseVersion is missing when update is true (update = 1)', () => {
-        const entity = { system: { update: '1' } };
-        assert.throws(() => { extractBaseVersionFromSubmission(entity); }, (err) => {
-          err.problemCode.should.equal(400.2);
-          err.message.should.equal('Required parameter baseVersion missing.');
-          return true;
-        });
+      it('not complain if baseVersion is missing when update is false and create is true', () => {
+        const entity = { system: { update: '1', create: '1' } };
+        // the create/update values do not get pulled from the entity system data here
+        // but rather from the branch in the code that decides whether a create
+        // or update is currently being attempted.
+        should.not.exist(extractBaseVersionFromSubmission(entity, { create: true }));
       });
 
-      it('should complain if baseVersion is missing when update is true (update = true)', () => {
-        const entity = { system: { update: 'true' } };
-        assert.throws(() => { extractBaseVersionFromSubmission(entity); }, (err) => {
+      it('should complain if baseVersion is missing when update is true', () => {
+        const entity = { system: { update: '1' } };
+        assert.throws(() => { extractBaseVersionFromSubmission(entity, { update: true }); }, (err) => {
           err.problemCode.should.equal(400.2);
           err.message.should.equal('Required parameter baseVersion missing.');
           return true;