From 9c297b7bb88ffb5d2b6d86d6086b73d4159ea0e4 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Tue, 20 Aug 2024 12:53:18 -0700 Subject: [PATCH 1/9] Force-process entity submissions held in backlog --- lib/bin/process-held-submissions.js | 23 ++ lib/model/query/entities.js | 73 +++-- lib/task/process-held-submissions.js | 16 ++ test/integration/api/offline-entities.js | 346 +++++++++++++++++++++++ 4 files changed, 438 insertions(+), 20 deletions(-) create mode 100644 lib/bin/process-held-submissions.js create mode 100644 lib/task/process-held-submissions.js diff --git a/lib/bin/process-held-submissions.js b/lib/bin/process-held-submissions.js new file mode 100644 index 000000000..3f5b4336b --- /dev/null +++ b/lib/bin/process-held-submissions.js @@ -0,0 +1,23 @@ +// 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. +// +// This script re-processes submissions containing offline entity actions that +// were previously held in a backlog due to submissions coming in out of order. + +const { run } = require('../task/task'); +const { processHeldSubmissions } = require('../task/process-held-submissions'); + +const { program } = require('commander'); +program.option('-f, --force', 'Force all submissions in the backlog to be processed immediately.'); +program.parse(); + +const options = program.opts(); + +run(processHeldSubmissions(options.force) + .then((count) => `Submissions processed: ${count}`)); diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index 8b2b248fb..1c71c7204 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -7,8 +7,9 @@ // including this file, may be copied, modified, propagated, or distributed // except according to the terms contained in the LICENSE file. +const config = require('config'); const { sql } = require('slonik'); -const { Actor, Entity, Submission, Form } = require('../frames'); +const { Actor, Audit, Entity, Submission, Form } = require('../frames'); const { equals, extender, unjoiner, page, markDeleted, insertMany } = require('../../util/db'); const { map, mergeRight, pickAll } = require('ramda'); const { blankStringToNull, construct } = require('../../util/util'); @@ -17,7 +18,7 @@ const { odataFilter, odataOrderBy } = require('../../data/odata-filter'); const { odataToColumnMap, parseSubmissionXml, getDiffProp, ConflictType } = require('../../data/entity'); const { isTrue } = require('../../util/http'); const Problem = require('../../util/problem'); -const { runSequentially } = require('../../util/promise'); +const { getOrReject, runSequentially } = require('../../util/promise'); ///////////////////////////////////////////////////////////////////////////////// @@ -177,14 +178,18 @@ createVersion.audit.withResult = true; //////////////////////////////////////////////////////////////////////////////// // WRAPPER FUNCTIONS FOR CREATING AND UPDATING ENTITIES -const _createEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent) => async ({ Audits, Entities }) => { +const _createEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent, forceOutOfOrderProcessing = false) => 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') || (!dataset.approvalRequired && event.action === 'submission.update')) // don't process submission if approval is not required and submission metadata is updated return null; - // TODO: auto-generate a label if forced and if the submission doesn't provide one + // Auto-generate a label if forced and if the submission doesn't provide one + if (forceOutOfOrderProcessing && entityData.system.label == null) { + // eslint-disable-next-line no-param-reassign + entityData.system.label = 'auto generated'; + } const partial = await Entity.fromParseEntityData(entityData, { create: true }); const sourceDetails = { submission: { instanceId: submissionDef.instanceId }, parentEventId: parentEvent ? parentEvent.id : undefined }; @@ -202,7 +207,7 @@ const _createEntity = (dataset, entityData, submissionId, submissionDef, submiss return entity; }; -const _updateEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event) => async ({ Audits, Entities }) => { +const _updateEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, forceOutOfOrderProcessing = false) => async ({ Audits, Entities }) => { if (!(event.action === 'submission.create' || event.action === 'submission.update.version' || event.action === 'submission.reprocess')) @@ -213,7 +218,7 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss // Figure out the intended baseVersion // If this is an offline update with a branchId, the baseVersion value is local to that offline context. - const baseEntityDef = await Entities._computeBaseVersion(event.id, dataset, clientEntity, submissionDef); + const baseEntityDef = await Entities._computeBaseVersion(event.id, dataset, clientEntity, submissionDef, forceOutOfOrderProcessing); // If baseEntityVersion is null, we held a submission and will stop processing now. if (baseEntityDef == null) @@ -294,7 +299,7 @@ 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) => async ({ Entities }) => { +const _computeBaseVersion = (eventId, dataset, clientEntity, submissionDef, forceOutOfOrderProcessing = false) => async ({ Entities }) => { if (!clientEntity.def.branchId) { // no offline branching to deal with, use baseVersion as is @@ -327,10 +332,17 @@ const _computeBaseVersion = (eventId, dataset, clientEntity, submissionDef) => a const baseEntityVersion = await Entities.getDef(dataset.id, clientEntity.uuid, new QueryOptions({ condition })); if (!baseEntityVersion.isDefined()) { - // TODO: add case for force-processing - // If there is no base version and we are not forcing the processing, hold submission in the backlog. - await Entities._holdSubmission(eventId, submissionDef.submissionId, submissionDef.id, clientEntity.uuid, clientEntity.def.branchId, clientEntity.def.baseVersion); - return null; + if (forceOutOfOrderProcessing) { + // If the base version doesn't exist but we forcing the update anyway, use the latest version on the server as the base. + // If that can't be found, throw an error for _processSubmissionEvent to catch so it can try create instead of update. + 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 there is no base version and we are not forcing the processing, hold submission in the backlog. + await Entities._holdSubmission(eventId, submissionDef.submissionId, submissionDef.id, clientEntity.uuid, clientEntity.def.branchId, clientEntity.def.baseVersion); + return null; + } } // Return the base entity version @@ -351,7 +363,7 @@ const _getFormDefActions = (oneFirst, datasetId, formDefId) => oneFirst(sql` // so any errors can be rolled back and logged as an entity processing error. const _processSubmissionEvent = (event, parentEvent) => async ({ Audits, Datasets, Entities, Submissions, Forms, oneFirst }) => { const { submissionId, submissionDefId } = event.details; - // TODO: check parentEvent details to determine if this is a forced reprocessing or not + const forceOutOfOrderProcessing = parentEvent?.details?.force === true; const form = await Forms.getByActeeId(event.acteeId); // If form is deleted/purged then submission won't be there either. @@ -369,7 +381,6 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Audits, Dataset // don't try to process it now, it will be dequeued and reprocessed elsewhere. if (existingHeldSubmission.isDefined()) return null; - // TODO: check how force-reprocessing interacts with this logic above const submission = await Submissions.getSubAndDefById(submissionDefId); @@ -409,22 +420,21 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Audits, Dataset throw Problem.user.entityActionNotPermitted({ action, permitted: permittedActions }); } - // TODO: work out how force-reprocessing interacts with this logic (updateEntity and createEntity should know about it) let maybeEntity = null; // Try update before create (if both are specified) if (entityData.system.update === '1' || entityData.system.update === 'true') try { - maybeEntity = await Entities._updateEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event); + maybeEntity = await Entities._updateEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, forceOutOfOrderProcessing); } catch (err) { - const attemptCreate = (entityData.system.create === '1' || entityData.system.create === 'true'); + const attemptCreate = (entityData.system.create === '1' || entityData.system.create === 'true') || forceOutOfOrderProcessing; if ((err.problemCode === 404.8) && attemptCreate) { - maybeEntity = await Entities._createEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent); + maybeEntity = await Entities._createEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent, forceOutOfOrderProcessing); } else { throw (err); } } else if (entityData.system.create === '1' || entityData.system.create === 'true') - maybeEntity = await Entities._createEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent); + maybeEntity = await Entities._createEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent, forceOutOfOrderProcessing); // Check for held submissions that follow this one in the same branch if (maybeEntity != null) { @@ -434,8 +444,7 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Audits, Dataset const currentBranchBaseVersion = branchBaseVersion ?? 0; const nextSub = await Entities._getNextHeldSubmissionInBranch(entityUuid, branchId, currentBranchBaseVersion + 1); - // TODO: don't handle the next submission if the current one was processed forcefully - if (nextSub.isDefined()) { + if (nextSub.isDefined() && !forceOutOfOrderProcessing) { const { submissionId: nextSubmissionId, submissionDefId: nextSubmissionDefId, auditId } = nextSub.get(); await Entities._deleteHeldSubmissionByEventId(auditId); await Audits.log({ id: event.actorId }, 'submission.reprocess', { acteeId: event.acteeId }, @@ -496,6 +505,29 @@ const _deleteHeldSubmissionByEventId = (eventId) => ({ run }) => run(sql` WHERE "auditId"=${eventId}`); +//////////////////////////////////////////////////////////////////////////////// +// FORCE PROCESSING SUBMISSIONS FROM BACKLOG + +const DAY_RANGE = config.has('default.taskSchedule.forceProcess') + ? config.get('default.taskSchedule.forceProcess') + : 7; // Default is 7 days + +const _getHeldSubmissionsAsEvents = (force) => ({ all }) => all(sql` + SELECT audits.* FROM entity_submission_backlog + JOIN audits on entity_submission_backlog."auditId" = audits.id + ${force ? sql`` : sql`WHERE entity_submission_backlog."loggedAt" < current_date - cast(${DAY_RANGE} as int)`} + ORDER BY "branchId", "branchBaseVersion"`) + .then(map(construct(Audit))); + +const processHeldSubmissions = (force = false) => async (container) => { + const events = await container.Entities._getHeldSubmissionsAsEvents(force); + return runSequentially(events.map(event => + async () => { + await container.Entities._deleteHeldSubmissionByEventId(event.id); + return processSubmissionEvent(event, { details: { force: true } })(container); + })); +}; + //////////////////////////////////////////////////////////////////////////////// // PROCESSING PENDING SUBMISSIONS FROM TOGGLING DATASET APPROVALREQUIRED FLAG @@ -680,6 +712,7 @@ module.exports = { _computeBaseVersion, _holdSubmission, _checkHeldSubmission, _getNextHeldSubmissionInBranch, _deleteHeldSubmissionByEventId, + _getHeldSubmissionsAsEvents, processHeldSubmissions, processSubmissionEvent, streamForExport, getDefBySubmissionId, createVersion, diff --git a/lib/task/process-held-submissions.js b/lib/task/process-held-submissions.js new file mode 100644 index 000000000..214b3098d --- /dev/null +++ b/lib/task/process-held-submissions.js @@ -0,0 +1,16 @@ +// Copyright 2019 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. +// +// This task deletes expired sessions from the table so it does not become +// overladen and bogged down over time. + +const { task } = require('./task'); +const processHeldSubmissions = task.withContainer(({ Entities }) => Entities.processHeldSubmissions); +module.exports = { processHeldSubmissions }; + diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index 4a4527798..a11b1ccda 100644 --- a/test/integration/api/offline-entities.js +++ b/test/integration/api/offline-entities.js @@ -780,4 +780,350 @@ describe('Offline Entities', () => { count.should.equal(0); })); }); + + describe('force-processing held submissions', () => { + it('should apply an entity update when the previous update is missing', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + + // Trunk version is 1, but base version is 2 + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('branchId=""', `branchId="${branchId}"`) + .replace('baseVersion="1"', 'baseVersion="2"') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(1); + + await container.Entities.processHeldSubmissions(true); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') + .expect(200) + .then(({ body }) => { + body.currentVersion.version.should.equal(2); + body.currentVersion.baseVersion.should.equal(1); + body.currentVersion.data.should.eql({ age: '22', status: 'arrived', first_name: 'Johnny' }); + + body.currentVersion.branchId.should.equal(branchId); + body.currentVersion.trunkVersion.should.equal(1); + body.currentVersion.branchBaseVersion.should.equal(2); + }); + + backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(0); + })); + + it('should apply two updates when first upate is missing', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + + // Trunk version is 1, but base version is 2 + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('branchId=""', `branchId="${branchId}"`) + .replace('baseVersion="1"', 'baseVersion="2"') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + 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="3"') + .replace('arrived', 'departed') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(2); + + await container.Entities.processHeldSubmissions(true); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') + .expect(200) + .then(({ body }) => { + body.currentVersion.version.should.equal(3); + body.currentVersion.baseVersion.should.equal(2); + body.currentVersion.data.should.eql({ age: '22', status: 'departed', first_name: 'Johnny' }); + + body.currentVersion.branchId.should.equal(branchId); + body.currentVersion.trunkVersion.should.equal(1); + body.currentVersion.branchBaseVersion.should.equal(3); + }); + + backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(0); + })); + + it('should apply an entity update as a create', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + const newUuid = uuid(); + + // Base version is 1 but it doesnt exist + // trunk version doesnt make sense to exist here either + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('id="12345678-1234-4123-8234-123456789abc"', `id="${newUuid}"`) + .replace('branchId=""', `branchId="${branchId}"`) + .replace('trunkVersion="1"', 'trunkVersion=""') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(1); + + await container.Entities.processHeldSubmissions(true); + + await asAlice.get(`/v1/projects/1/datasets/people/entities/${newUuid}`) + .expect(200) + .then(({ body }) => { + body.currentVersion.version.should.equal(1); + body.currentVersion.data.should.eql({ status: 'arrived' }); + body.currentVersion.label.should.eql('auto generated'); + body.currentVersion.branchId.should.equal(branchId); + + + // This is the first version of the entity so there should be no base or trunk versions + should.not.exist(body.currentVersion.trunkVersion); + should.not.exist(body.currentVersion.baseVersion); + should.not.exist(body.currentVersion.branchBaseVersion); + }); + + backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(0); + })); + + it('should apply an entity update as a create followed by another update', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + const newUuid = uuid(); + + // Base version is 1 but it doesnt exist + // trunk version doesnt make sense to exist here either + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('id="12345678-1234-4123-8234-123456789abc"', `id="${newUuid}"`) + .replace('branchId=""', `branchId="${branchId}"`) + .replace('trunkVersion="1"', 'trunkVersion=""') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + // base version is 2 now + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('one', 'one-update') + .replace('id="12345678-1234-4123-8234-123456789abc"', `id="${newUuid}"`) + .replace('branchId=""', `branchId="${branchId}"`) + .replace('baseVersion="1"', 'baseVersion="2"') + .replace('trunkVersion="1"', 'trunkVersion=""') + .replace('arrived', 'Danachecked in') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(2); + + await container.Entities.processHeldSubmissions(true); + + await asAlice.get(`/v1/projects/1/datasets/people/entities/${newUuid}`) + .expect(200) + .then(({ body }) => { + body.currentVersion.version.should.equal(2); + body.currentVersion.data.should.eql({ status: 'checked in', first_name: 'Dana' }); + body.currentVersion.label.should.eql('auto generated'); + body.currentVersion.branchId.should.equal(branchId); + body.currentVersion.baseVersion.should.equal(1); + body.currentVersion.branchBaseVersion.should.equal(2); + should.not.exist(body.currentVersion.trunkVersion); + }); + + backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(0); + + // send in another update much later in the same branch + // base version is 10 now (many missing intermediate updates) + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('one', 'one-update10') + .replace('id="12345678-1234-4123-8234-123456789abc"', `id="${newUuid}"`) + .replace('branchId=""', `branchId="${branchId}"`) + .replace('baseVersion="1"', 'baseVersion="10"') + .replace('trunkVersion="1"', 'trunkVersion=""') + .replace('arrived', 'Danaregistered') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(1); + + await container.Entities.processHeldSubmissions(true); + + await asAlice.get(`/v1/projects/1/datasets/people/entities/${newUuid}`) + .expect(200) + .then(({ body }) => { + body.currentVersion.version.should.equal(3); + body.currentVersion.data.should.eql({ status: 'registered', first_name: 'Dana' }); + body.currentVersion.label.should.eql('auto generated'); + body.currentVersion.branchId.should.equal(branchId); + body.currentVersion.baseVersion.should.equal(2); + body.currentVersion.branchBaseVersion.should.equal(10); + should.not.exist(body.currentVersion.trunkVersion); + }); + + backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + 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) => { + 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('new', 'checked in') + .replace('', '') + .replace('20', '') + .replace('Megan', '') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(1); + + // Force the update submission to be processed as a create + await container.Entities.processHeldSubmissions(true); + + 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.data.should.eql({ status: 'checked in' }); + body.currentVersion.label.should.eql('auto generated'); + body.currentVersion.branchId.should.equal(branchId); + 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); + + // 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}"`) + ) + .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); + }); + })); + + describe('only force-process submissions held in backlog for a certain amount of time', () => { + it('should process a submission from over 7 days ago', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + + // Neither update below will be applied at first because the first + // update in the branch is missing. + + // Send the first submission, which will be held in the backlog + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('branchId=""', `branchId="${branchId}"`) + .replace('baseVersion="1"', 'baseVersion="2"') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(1); + + // Update the timestamp on this backlog + await container.run(sql`UPDATE entity_submission_backlog SET "loggedAt" = "loggedAt" - interval '8 days'`); + + // Send the next submission, which will also be held in the backlog. + // This submission immediately follows the previous one, but force-processing + // the first submission does not cause this one to be processed. + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('branchId=""', `branchId="${branchId}"`) + .replace('one', 'one-update') + .replace('baseVersion="1"', 'baseVersion="3"') + .replace('arrived', 'departed') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + // Both submissions should be in the backlog now + backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(2); + + // Process submissions that have been in the backlog for a long time + await container.Entities.processHeldSubmissions(); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') + .expect(200) + .then(({ body }) => { + body.currentVersion.version.should.equal(2); + body.currentVersion.baseVersion.should.equal(1); + body.currentVersion.data.should.eql({ age: '22', status: 'arrived', first_name: 'Johnny' }); + + body.currentVersion.branchId.should.equal(branchId); + body.currentVersion.trunkVersion.should.equal(1); + body.currentVersion.branchBaseVersion.should.equal(2); + }); + + // One submission should still be in the backlog + backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(1); + })); + }); + }); }); From 571c2f191e3776fd1d4e2872e3257557bfb60089 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Wed, 21 Aug 2024 13:50:50 -0700 Subject: [PATCH 2/9] Make task actually run --- lib/model/query/entities.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index 1c71c7204..b085cf347 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -172,6 +172,7 @@ createVersion.audit = (updatedEntity, dataset, partial, subDef) => (log) => { entityDefId: updatedEntity.aux.currentVersion.id, entity: { uuid: updatedEntity.uuid, dataset: dataset.name } }); + return Promise.resolve(); }; createVersion.audit.withResult = true; From b64f78363cd1ab91680bf55afb58cf5f7ccb5134 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Thu, 22 Aug 2024 16:45:00 -0700 Subject: [PATCH 3/9] Update formatting of force processing backlog --- lib/model/query/entities.js | 3 ++- lib/task/process-held-submissions.js | 2 +- test/integration/api/offline-entities.js | 4 +++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index b085cf347..1b55e4928 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -526,7 +526,8 @@ const processHeldSubmissions = (force = false) => async (container) => { async () => { await container.Entities._deleteHeldSubmissionByEventId(event.id); return processSubmissionEvent(event, { details: { force: true } })(container); - })); + })) + .then(() => events.length); }; //////////////////////////////////////////////////////////////////////////////// diff --git a/lib/task/process-held-submissions.js b/lib/task/process-held-submissions.js index 214b3098d..152075cbf 100644 --- a/lib/task/process-held-submissions.js +++ b/lib/task/process-held-submissions.js @@ -1,4 +1,4 @@ -// Copyright 2019 ODK Central Developers +// 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 diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index a11b1ccda..f554b93a8 100644 --- a/test/integration/api/offline-entities.js +++ b/test/integration/api/offline-entities.js @@ -1106,7 +1106,9 @@ describe('Offline Entities', () => { backlogCount.should.equal(2); // Process submissions that have been in the backlog for a long time - await container.Entities.processHeldSubmissions(); + // (only 1 of 2 should be processed) + const count = await container.Entities.processHeldSubmissions(); + count.should.equal(1); await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') .expect(200) From 002c82ae657b8b04dd6f091c172c75b67b10eb70 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Thu, 22 Aug 2024 16:49:09 -0700 Subject: [PATCH 4/9] Rename task to reference backlog --- ...cess-held-submissions.js => process-backlog.js} | 4 ++-- lib/model/query/entities.js | 4 ++-- ...cess-held-submissions.js => process-backlog.js} | 4 ++-- test/integration/api/offline-entities.js | 14 +++++++------- 4 files changed, 13 insertions(+), 13 deletions(-) rename lib/bin/{process-held-submissions.js => process-backlog.js} (89%) rename lib/task/{process-held-submissions.js => process-backlog.js} (82%) diff --git a/lib/bin/process-held-submissions.js b/lib/bin/process-backlog.js similarity index 89% rename from lib/bin/process-held-submissions.js rename to lib/bin/process-backlog.js index 3f5b4336b..be84f9f91 100644 --- a/lib/bin/process-held-submissions.js +++ b/lib/bin/process-backlog.js @@ -11,7 +11,7 @@ // were previously held in a backlog due to submissions coming in out of order. const { run } = require('../task/task'); -const { processHeldSubmissions } = require('../task/process-held-submissions'); +const { processBacklog } = require('../task/process-backlog'); const { program } = require('commander'); program.option('-f, --force', 'Force all submissions in the backlog to be processed immediately.'); @@ -19,5 +19,5 @@ program.parse(); const options = program.opts(); -run(processHeldSubmissions(options.force) +run(processBacklog(options.force) .then((count) => `Submissions processed: ${count}`)); diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index 1b55e4928..8a4667231 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -520,7 +520,7 @@ const _getHeldSubmissionsAsEvents = (force) => ({ all }) => all(sql` ORDER BY "branchId", "branchBaseVersion"`) .then(map(construct(Audit))); -const processHeldSubmissions = (force = false) => async (container) => { +const processBacklog = (force = false) => async (container) => { const events = await container.Entities._getHeldSubmissionsAsEvents(force); return runSequentially(events.map(event => async () => { @@ -714,7 +714,7 @@ module.exports = { _computeBaseVersion, _holdSubmission, _checkHeldSubmission, _getNextHeldSubmissionInBranch, _deleteHeldSubmissionByEventId, - _getHeldSubmissionsAsEvents, processHeldSubmissions, + _getHeldSubmissionsAsEvents, processBacklog, processSubmissionEvent, streamForExport, getDefBySubmissionId, createVersion, diff --git a/lib/task/process-held-submissions.js b/lib/task/process-backlog.js similarity index 82% rename from lib/task/process-held-submissions.js rename to lib/task/process-backlog.js index 152075cbf..f73701970 100644 --- a/lib/task/process-held-submissions.js +++ b/lib/task/process-backlog.js @@ -11,6 +11,6 @@ // overladen and bogged down over time. const { task } = require('./task'); -const processHeldSubmissions = task.withContainer(({ Entities }) => Entities.processHeldSubmissions); -module.exports = { processHeldSubmissions }; +const processBacklog = task.withContainer(({ Entities }) => Entities.processBacklog); +module.exports = { processBacklog }; diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index f554b93a8..56c853add 100644 --- a/test/integration/api/offline-entities.js +++ b/test/integration/api/offline-entities.js @@ -800,7 +800,7 @@ describe('Offline Entities', () => { let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); backlogCount.should.equal(1); - await container.Entities.processHeldSubmissions(true); + await container.Entities.processBacklog(true); await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') .expect(200) @@ -846,7 +846,7 @@ describe('Offline Entities', () => { let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); backlogCount.should.equal(2); - await container.Entities.processHeldSubmissions(true); + await container.Entities.processBacklog(true); await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') .expect(200) @@ -885,7 +885,7 @@ describe('Offline Entities', () => { let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); backlogCount.should.equal(1); - await container.Entities.processHeldSubmissions(true); + await container.Entities.processBacklog(true); await asAlice.get(`/v1/projects/1/datasets/people/entities/${newUuid}`) .expect(200) @@ -940,7 +940,7 @@ describe('Offline Entities', () => { let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); backlogCount.should.equal(2); - await container.Entities.processHeldSubmissions(true); + await container.Entities.processBacklog(true); await asAlice.get(`/v1/projects/1/datasets/people/entities/${newUuid}`) .expect(200) @@ -976,7 +976,7 @@ describe('Offline Entities', () => { backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); backlogCount.should.equal(1); - await container.Entities.processHeldSubmissions(true); + await container.Entities.processBacklog(true); await asAlice.get(`/v1/projects/1/datasets/people/entities/${newUuid}`) .expect(200) @@ -1020,7 +1020,7 @@ describe('Offline Entities', () => { backlogCount.should.equal(1); // Force the update submission to be processed as a create - await container.Entities.processHeldSubmissions(true); + await container.Entities.processBacklog(true); await asAlice.get(`/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd`) .expect(200) @@ -1107,7 +1107,7 @@ describe('Offline Entities', () => { // Process submissions that have been in the backlog for a long time // (only 1 of 2 should be processed) - const count = await container.Entities.processHeldSubmissions(); + const count = await container.Entities.processBacklog(); count.should.equal(1); await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') From 23715cf2c99a2b7988bff646daa0aae51cdbbc7f Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Thu, 22 Aug 2024 17:07:56 -0700 Subject: [PATCH 5/9] Adding tests for force processing backlog for deleted entity or submission --- test/integration/api/offline-entities.js | 70 ++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index 56c853add..17650a7f8 100644 --- a/test/integration/api/offline-entities.js +++ b/test/integration/api/offline-entities.js @@ -1127,5 +1127,75 @@ describe('Offline Entities', () => { backlogCount.should.equal(1); })); }); + + describe('force-processing deleted submissions and entities', () => { + it('should not process a submission in a soft-deleted form', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + + // Send the first submission, which will be held in the backlog because the base version is high + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('branchId=""', `branchId="${branchId}"`) + .replace('baseVersion="1"', 'baseVersion="2"') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + const backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(1); + + // Soft-delete the form to delete the submission + await asAlice.delete('/v1/projects/1/forms/offlineEntity'); + + // Process the backlog (count will be 1 but update should not be applied to entity) + const processedCount = await container.Entities.processBacklog(true); + processedCount.should.equal(1); + + // Check that the entity is still at version 1 + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') + .expect(200) + .then(({ body }) => { + body.currentVersion.version.should.equal(1); + }); + })); + + it('should not process a submission that has been soft-deleted', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + + // Send the first submission, which will be held in the backlog because the base version is high + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('branchId=""', `branchId="${branchId}"`) + .replace('baseVersion="1"', 'baseVersion="2"') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + const backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(1); + + // Soft-delete the submission via the database + await container.run(sql`UPDATE submissions SET "deletedAt" = NOW() WHERE "instanceId" = 'one'`); + + // Process the backlog (count will be 1 but update should not be applied to entity) + const processedCount = await container.Entities.processBacklog(true); + processedCount.should.equal(1); + + // Check that the entity is still at version 1 + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') + .expect(200) + .then(({ body }) => { + body.currentVersion.version.should.equal(1); + }); + })); + + // TODO: check deleted entity + }); }); }); From 9724f467a96e4ea95eb00ac019a51c75b9c8ecc2 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Fri, 23 Aug 2024 11:06:54 -0700 Subject: [PATCH 6/9] Dont force process a sub for a deleted entity --- lib/model/query/entities.js | 16 +++++++++- lib/util/problem.js | 3 ++ test/integration/api/offline-entities.js | 37 +++++++++++++++++++++++- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index 8a4667231..015a276e5 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -219,7 +219,21 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss // Figure out the intended baseVersion // If this is an offline update with a branchId, the baseVersion value is local to that offline context. - const baseEntityDef = await Entities._computeBaseVersion(event.id, dataset, clientEntity, submissionDef, forceOutOfOrderProcessing); + let baseEntityDef; + + // 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); + } catch (err) { + if (err.problemCode === 404.8) { + // Look up deleted entity by passing deleted as option argData + const deletedEntity = await Entities.getById(dataset.id, clientEntity.uuid, new QueryOptions({ argData: { deleted: 'true' } })); + if (deletedEntity.isDefined()) + throw Problem.user.entityDeleted({ entityUuid: clientEntity.uuid }); + } + throw err; + } // If baseEntityVersion is null, we held a submission and will stop processing now. if (baseEntityDef == null) diff --git a/lib/util/problem.js b/lib/util/problem.js index 76fde7b55..c5c99c61c 100644 --- a/lib/util/problem.js +++ b/lib/util/problem.js @@ -166,6 +166,9 @@ const problems = { // entity base version specified in submission does not exist entityVersionNotFound: problem(404.9, ({ baseVersion, entityUuid, datasetName }) => `Base version (${baseVersion}) does not exist for entity UUID (${entityUuid}) in dataset (${datasetName}).`), + // entity has been deleted + entityDeleted: problem(404.11, ({ entityUuid }) => `The entity with UUID (${entityUuid}) has been deleted.`), + // { allowed: [ acceptable formats ], got } unacceptableFormat: problem(406.1, ({ allowed }) => `Requested format not acceptable; this resource allows: ${allowed.join()}.`), diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index 17650a7f8..0cb2baf5f 100644 --- a/test/integration/api/offline-entities.js +++ b/test/integration/api/offline-entities.js @@ -1195,7 +1195,42 @@ describe('Offline Entities', () => { }); })); - // TODO: check deleted entity + it('should not process a submission for an entity that has been soft-deleted', testOfflineEntities(async (service, container) => { + const asAlice = await service.login('alice'); + const branchId = uuid(); + + // Send the first submission, which will be held in the backlog because the base version is high + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.one + .replace('branchId=""', `branchId="${branchId}"`) + .replace('baseVersion="1"', 'baseVersion="2"') + ) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(1); + + // Soft-delete the entity + await asAlice.delete('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc'); + + // Process the backlog (count will be 1 but update should not be applied to entity) + const processedCount = await container.Entities.processBacklog(true); + processedCount.should.equal(1); + + // Check that the backlog count is now 0 + backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); + backlogCount.should.equal(0); + + // Check for an entity error on the submission + await asAlice.get('/v1/projects/1/forms/offlineEntity/submissions/one/audits') + .expect(200) + .then(({ body }) => { + body[0].details.errorMessage.should.eql('The entity with UUID (12345678-1234-4123-8234-123456789abc) has been deleted.'); + }); + })); }); }); }); From d1ac1355735ba380fc1457bc0f5ea027e6d08c6d Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Fri, 23 Aug 2024 16:21:26 -0700 Subject: [PATCH 7/9] Add transaction to backlog processor task --- lib/model/query/entities.js | 20 +++++++++++++------- lib/task/process-backlog.js | 3 --- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index 015a276e5..610e72ae1 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -534,14 +534,19 @@ const _getHeldSubmissionsAsEvents = (force) => ({ all }) => all(sql` ORDER BY "branchId", "branchBaseVersion"`) .then(map(construct(Audit))); +const _processSingleBacklogEvent = (event) => (container) => + container.db.transaction(async (trxn) => { + const { Entities } = container.with({ db: trxn }); + await Entities._deleteHeldSubmissionByEventId(event.id); + await Entities.processSubmissionEvent(event, { details: { force: true } }); + return true; + }); + const processBacklog = (force = false) => async (container) => { const events = await container.Entities._getHeldSubmissionsAsEvents(force); - return runSequentially(events.map(event => - async () => { - await container.Entities._deleteHeldSubmissionByEventId(event.id); - return processSubmissionEvent(event, { details: { force: true } })(container); - })) - .then(() => events.length); + await runSequentially(events.map(event => + () => container.Entities._processSingleBacklogEvent(event))); + return events.length; }; //////////////////////////////////////////////////////////////////////////////// @@ -728,7 +733,8 @@ module.exports = { _computeBaseVersion, _holdSubmission, _checkHeldSubmission, _getNextHeldSubmissionInBranch, _deleteHeldSubmissionByEventId, - _getHeldSubmissionsAsEvents, processBacklog, + _getHeldSubmissionsAsEvents, + processBacklog, _processSingleBacklogEvent, processSubmissionEvent, streamForExport, getDefBySubmissionId, createVersion, diff --git a/lib/task/process-backlog.js b/lib/task/process-backlog.js index f73701970..1a8ba5bd7 100644 --- a/lib/task/process-backlog.js +++ b/lib/task/process-backlog.js @@ -6,9 +6,6 @@ // 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. -// -// This task deletes expired sessions from the table so it does not become -// overladen and bogged down over time. const { task } = require('./task'); const processBacklog = task.withContainer(({ Entities }) => Entities.processBacklog); From f0bfecd88f8ce2f8f8e50e1b4a6d66749ad4034b Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Fri, 23 Aug 2024 16:35:44 -0700 Subject: [PATCH 8/9] pass baseVersion through in force create --- lib/model/query/entities.js | 7 ++++++- test/integration/api/offline-entities.js | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index 610e72ae1..8fd0cf558 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -191,7 +191,12 @@ const _createEntity = (dataset, entityData, submissionId, submissionDef, submiss // eslint-disable-next-line no-param-reassign entityData.system.label = 'auto generated'; } - const partial = await Entity.fromParseEntityData(entityData, { create: true }); + + // Add the branchBaseVersion to the partial if we are forcing the create and it has one + const _partial = await Entity.fromParseEntityData(entityData, { create: true }); + const partial = (forceOutOfOrderProcessing) + ? _partial.auxWith('def', { branchBaseVersion: _partial.def.baseVersion }) + : _partial; const sourceDetails = { submission: { instanceId: submissionDef.instanceId }, parentEventId: parentEvent ? parentEvent.id : undefined }; const sourceId = await Entities.createSource(sourceDetails, submissionDefId, event.id); diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index 0cb2baf5f..faedb6e1a 100644 --- a/test/integration/api/offline-entities.js +++ b/test/integration/api/offline-entities.js @@ -876,6 +876,7 @@ describe('Offline Entities', () => { .replace('id="12345678-1234-4123-8234-123456789abc"', `id="${newUuid}"`) .replace('branchId=""', `branchId="${branchId}"`) .replace('trunkVersion="1"', 'trunkVersion=""') + .replace('baseVersion="1"', 'baseVersion="2"') ) .set('Content-Type', 'application/xml') .expect(200); @@ -894,12 +895,11 @@ describe('Offline Entities', () => { body.currentVersion.data.should.eql({ status: 'arrived' }); body.currentVersion.label.should.eql('auto generated'); body.currentVersion.branchId.should.equal(branchId); - + body.currentVersion.branchBaseVersion.should.equal(2); // This is the first version of the entity so there should be no base or trunk versions should.not.exist(body.currentVersion.trunkVersion); should.not.exist(body.currentVersion.baseVersion); - should.not.exist(body.currentVersion.branchBaseVersion); }); backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); From 55f02d400601c81625fb0826248f74a44cde19b0 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Fri, 23 Aug 2024 16:42:35 -0700 Subject: [PATCH 9/9] more empty label cases for update as create --- lib/model/query/entities.js | 2 +- test/integration/api/offline-entities.js | 27 +++++++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index 8fd0cf558..8b7de267f 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -187,7 +187,7 @@ const _createEntity = (dataset, entityData, submissionId, submissionDef, submiss return null; // Auto-generate a label if forced and if the submission doesn't provide one - if (forceOutOfOrderProcessing && entityData.system.label == null) { + if (forceOutOfOrderProcessing && (entityData.system.label == null || entityData.system.label.trim() === '')) { // eslint-disable-next-line no-param-reassign entityData.system.label = 'auto generated'; } diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index faedb6e1a..8c00002e1 100644 --- a/test/integration/api/offline-entities.js +++ b/test/integration/api/offline-entities.js @@ -881,10 +881,20 @@ describe('Offline Entities', () => { .set('Content-Type', 'application/xml') .expect(200); + await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions') + .send(testData.instances.offlineEntity.two + .replace('branchId=""', `branchId="${uuid()}"`) + .replace('create="1"', 'update="1"') + .replace('', '') + .replace('baseVersion=""', 'baseVersion="2"') + ) + .set('Content-Type', 'application/xml') + .expect(200); + await exhaust(container); let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); - backlogCount.should.equal(1); + backlogCount.should.equal(2); await container.Entities.processBacklog(true); @@ -902,6 +912,21 @@ describe('Offline Entities', () => { should.not.exist(body.currentVersion.baseVersion); }); + + 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.data.should.eql({ status: 'new', first_name: 'Megan', age: '20' }); + body.currentVersion.label.should.eql('auto generated'); + body.currentVersion.branchId.should.be.a.uuid(); + body.currentVersion.branchBaseVersion.should.equal(2); + + // This is the first version of the entity so there should be no base or trunk versions + should.not.exist(body.currentVersion.trunkVersion); + should.not.exist(body.currentVersion.baseVersion); + }); + backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`); backlogCount.should.equal(0); }));