Skip to content

Commit

Permalink
Entity submission create-as-update by logging forceProcessed in source
Browse files Browse the repository at this point in the history
  • Loading branch information
ktuite committed Oct 28, 2024
1 parent 4e34147 commit 087e350
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 25 deletions.
1 change: 1 addition & 0 deletions lib/model/frames/entity.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
) {
Expand Down
20 changes: 20 additions & 0 deletions lib/model/migrations/20241028-01-add-force-entity-def-source.js
Original file line number Diff line number Diff line change
@@ -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 };
60 changes: 48 additions & 12 deletions lib/model/query/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down Expand Up @@ -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 },
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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) {
Expand Down
24 changes: 11 additions & 13 deletions test/integration/api/offline-entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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' });
});
}));

Expand Down

0 comments on commit 087e350

Please sign in to comment.