Skip to content

Commit

Permalink
Remove expectation of branchId from entity create
Browse files Browse the repository at this point in the history
  • Loading branch information
@ktuite
ktuite committed Aug 20, 2024
1 parent 900d604 commit 1c6e3f7
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 54 deletions.
17 changes: 17 additions & 0 deletions lib/model/migrations/20240820-01-backlog-add-entityid.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// 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 = async (db) => {
await db.raw(`ALTER TABLE entity_submission_backlog
ADD COLUMN "entityUuid" UUID NOT NULL`);
};

const down = (db) => db.raw('ALTER TABLE DROP COLUMN "entityUuid"');

module.exports = { up, down };
27 changes: 16 additions & 11 deletions lib/model/query/entities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ select ins.*, def.id as "entityDefId" from ins, def;`)
new Entity(entityData, {
currentVersion: new Entity.Def({
id: entityDefId,
entityId: entityData.id,
branchId: partial.def.branchId
entityId: entityData.id
})
}));
};
Expand Down Expand Up @@ -201,9 +200,14 @@ const _checkHeldSubmission = (maybeOne, submissionId) => maybeOne(sql`
SELECT * FROM entity_submission_backlog
WHERE "submissionId"=${submissionId}`);

const _getNextHeldSubmissionInBranch = (maybeOne, branchId, branchBaseVersion) => maybeOne(sql`
SELECT * FROM entity_submission_backlog
WHERE "branchId"=${branchId} AND "branchBaseVersion" = ${branchBaseVersion}`);
const _getNextHeldSubmissionInBranch = (maybeOne, entityUuid, branchId, branchBaseVersion) => (
(branchId == null)
? maybeOne(sql`
SELECT * FROM entity_submission_backlog
WHERE "entityUuid" = ${entityUuid} AND "branchBaseVersion" = 1`)
: maybeOne(sql`
SELECT * FROM entity_submission_backlog
WHERE "branchId"=${branchId} AND "branchBaseVersion" = ${branchBaseVersion}`));

const DAY_RANGE = config.has('default.taskSchedule.forceProcess')
? config.get('default.taskSchedule.forceProcess')
Expand All @@ -222,9 +226,9 @@ const _deleteHeldSubmissionByEventId = (eventId) => ({ run }) => run(sql`
WHERE "auditId"=${eventId}`);

// Used by _computeBaseVersion below to hold submissions that are not yet ready to be processed
const _holdSubmission = (eventId, submissionId, submissionDefId, branchId, branchBaseVersion) => async ({ run }) => run(sql`
INSERT INTO entity_submission_backlog ("auditId", "submissionId", "submissionDefId", "branchId", "branchBaseVersion", "loggedAt")
VALUES (${eventId}, ${submissionId}, ${submissionDefId}, ${branchId}, ${branchBaseVersion}, CLOCK_TIMESTAMP())
const _holdSubmission = (eventId, submissionId, submissionDefId, entityUuid, branchId, branchBaseVersion) => async ({ run }) => run(sql`
INSERT INTO entity_submission_backlog ("auditId", "submissionId", "submissionDefId", "entityUuid", "branchId", "branchBaseVersion", "loggedAt")
VALUES (${eventId}, ${submissionId}, ${submissionDefId}, ${entityUuid}, ${branchId}, ${branchBaseVersion}, CLOCK_TIMESTAMP())
`);

// Used by _updateVerison below to figure out the intended base version in Central
Expand Down Expand Up @@ -270,7 +274,7 @@ const _computeBaseVersion = (eventId, dataset, clientEntity, submissionDef, forc
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.def.branchId, clientEntity.def.baseVersion);
await Entities._holdSubmission(eventId, submissionDef.submissionId, submissionDef.id, clientEntity.uuid, clientEntity.def.branchId, clientEntity.def.baseVersion);
return null;
}
}
Expand Down Expand Up @@ -473,11 +477,12 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Audits, Dataset
maybeEntity = await Entities._createEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent);

// Check for held submissions that follow this one in the same branch
if (maybeEntity != null && maybeEntity.aux.currentVersion.branchId != null) {
if (maybeEntity != null) {
const { uuid: entityUuid } = maybeEntity;
const { branchId, branchBaseVersion } = maybeEntity.aux.currentVersion;
// branchBaseVersion could be undefined if handling an offline create
const currentBranchBaseVersion = branchBaseVersion ?? 0;
const nextSub = await _getNextHeldSubmissionInBranch(maybeOne, branchId, currentBranchBaseVersion + 1);
const nextSub = await _getNextHeldSubmissionInBranch(maybeOne, entityUuid, branchId, currentBranchBaseVersion + 1);
if (nextSub.isDefined() && !forceOutOfOrderProcessing) {
const { submissionId: nextSubmissionId, submissionDefId: nextSubmissionDefId, auditId } = nextSub.get();
await Entities._deleteHeldSubmissionByEventId(auditId);
Expand Down
58 changes: 15 additions & 43 deletions test/integration/api/offline-entities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,12 @@ const testOfflineEntities = (test) => testService(async (service, container) =>

describe('Offline Entities', () => {
describe('parsing branchId and trunkVersion from submission xml', () => {
it('should parse and save branch info from sub creating an entity', testOfflineEntities(async (service, container) => {
it('should parse and save info from submission creating an entity', testOfflineEntities(async (service, container) => {
const asAlice = await service.login('alice');
const branchId = uuid();

// create=1 is specified, but baseVersion, trunkVersion, and branchId are all empty for entity creation
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);

Expand All @@ -54,7 +52,8 @@ describe('Offline Entities', () => {
should.not.exist(body.currentVersion.trunkVersion);
should.not.exist(body.currentVersion.baseVersion);
should.not.exist(body.currentVersion.branchBaseVersion);
body.currentVersion.branchId.should.equal(branchId);
// There is also no branchId because Collect does not send one for entity creation
should.not.exist(body.currentVersion.branchId);
});
}));

Expand Down Expand Up @@ -141,31 +140,6 @@ describe('Offline Entities', () => {
body[0].details.errorMessage.should.eql('Required parameter branchId missing.');
});
}));

it('should ignore empty string trunkVersion and branchId values in create scenario', testOfflineEntities(async (service, container) => {
const asAlice = await service.login('alice');

await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions')
.send(testData.instances.offlineEntity.two
.replace('trunkVersion="1"', `trunkVersion=""`)
)
.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(1);
should.not.exist(body.currentVersion.baseVersion);
body.currentVersion.data.should.eql({ age: '20', status: 'new', first_name: 'Megan' });

should.not.exist(body.currentVersion.trunkVersion);
should.not.exist(body.currentVersion.branchBaseVersion);
should.not.exist(body.currentVersion.branchId);
});
}));
});

describe('offline branches submitted in order', () => {
Expand Down Expand Up @@ -303,9 +277,7 @@ describe('Offline Entities', () => {

// First submission creates the entity, offline version is now 1
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);

Expand Down Expand Up @@ -366,6 +338,8 @@ describe('Offline Entities', () => {
.then(({ body }) => {
should.not.exist(body[0].details.problem);
});

// This submission is now held in the backlog, but that functionality is checked in later tests
}));

it('should not apply out of order update from a run after starting a run', testOfflineEntities(async (service, container) => {
Expand Down Expand Up @@ -504,6 +478,7 @@ describe('Offline Entities', () => {
const branchId = uuid();

// Send the second submission that updates an entity (before the entity has been created)
// This submission has a branchId
await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions')
.send(testData.instances.offlineEntity.two
.replace('create="1"', 'update="1"')
Expand All @@ -521,10 +496,10 @@ describe('Offline Entities', () => {
backlogCount.should.equal(1);

// Send the second submission to create the entity
// This submission does not have an explicit branchId, but it should trigger
// the processing of the next submission in the branch.
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);

Expand Down Expand Up @@ -581,10 +556,9 @@ describe('Offline Entities', () => {
backlogCount.should.equal(2);

// Third (but logically first) submission to create entity
// which does not have an explicit branchId.
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);

Expand Down Expand Up @@ -741,9 +715,7 @@ describe('Offline Entities', () => {
// First submission creates the entity, offline version is now 1
// But this submission requires approval so it wont get processed at first
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);

Expand Down

0 comments on commit 1c6e3f7

Please sign in to comment.