diff --git a/lib/data/entity.js b/lib/data/entity.js
index 27c71ec37..050103439 100644
--- a/lib/data/entity.js
+++ b/lib/data/entity.js
@@ -69,14 +69,12 @@ const extractBaseVersionFromSubmission = (entity) => {
const extractBranchIdFromSubmission = (entity) => {
const { branchId } = entity.system;
- if (branchId === '')
+ if (branchId === '' || branchId == null)
return null;
- if (branchId) {
- const matches = _uuidPattern.exec(branchId);
- if (matches == null) throw Problem.user.invalidDataTypeOfParameter({ field: 'branchId', expected: 'valid version 4 UUID' });
- return matches[2].toLowerCase();
- }
+ const matches = _uuidPattern.exec(branchId);
+ if (matches == null) throw Problem.user.invalidDataTypeOfParameter({ field: 'branchId', expected: 'valid version 4 UUID' });
+ return matches[2].toLowerCase();
};
const extractTrunkVersionFromSubmission = (entity) => {
diff --git a/lib/model/migrations/20240607-02-add-submission-backlog.js b/lib/model/migrations/20240607-02-add-submission-backlog.js
index 27fc98301..0cda52ed5 100644
--- a/lib/model/migrations/20240607-02-add-submission-backlog.js
+++ b/lib/model/migrations/20240607-02-add-submission-backlog.js
@@ -9,11 +9,11 @@
const up = async (db) => {
await db.raw(`CREATE TABLE entity_submission_backlog (
- "submissionId" INT4,
- "submissionDefId" INT4,
- "branchId" UUID,
- "branchBaseVersion" INT4,
- "loggedAt" TIMESTAMPTZ(3),
+ "submissionId" INT4 NOT NULL,
+ "submissionDefId" INT4 NOT NULL,
+ "branchId" UUID NOT NULL,
+ "branchBaseVersion" INT4 NOT NULL,
+ "loggedAt" TIMESTAMPTZ(3) NOT NULL,
CONSTRAINT fk_submission_defs
FOREIGN KEY("submissionDefId")
REFERENCES submission_defs(id)
diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js
index 272cbee6a..fad3e6762 100644
--- a/lib/model/query/entities.js
+++ b/lib/model/query/entities.js
@@ -276,6 +276,11 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss
serverEntity = serverEntity.get();
}
+ // If the trunk version exists but is higher than current server version,
+ // that is a weird case that should not be processed OR held, and should log an error.
+ if (clientEntity.def.trunkVersion && clientEntity.def.trunkVersion > serverEntity.aux.currentVersion.version) {
+ throw Problem.user.entityVersionNotFound({ baseVersion: `trunkVersion=${clientEntity.def.trunkVersion}`, entityUuid: clientEntity.uuid, datasetName: dataset.name });
+ }
let { conflict } = serverEntity;
let conflictingProperties; // Maybe we don't need to persist this??? just compute at the read time
@@ -287,7 +292,7 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss
// If baseVersion is null, we held a submission and will stop processing now.
if (baseVersion == null)
- return;
+ return null;
if (baseVersion !== serverEntity.aux.currentVersion.version) {
@@ -416,14 +421,11 @@ 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
- // TODO: consider changing checkHeldSub query to check for existing version of entity
- // so that return value of create/update above doesn't matter as much.
- // Also consider actually checking entity uuid, in query.
if (maybeEntity != null && maybeEntity.aux.currentVersion.branchId != null) {
- const { branchId } = maybeEntity.aux.currentVersion;
- // baseVersion could be '', meaning its an offline create
- const currentBaseVersion = entityData.system.baseVersion === '' ? 0 : parseInt(entityData.system.baseVersion, 10);
- const nextSub = await _checkHeldSubmission(maybeOne, branchId, currentBaseVersion + 1);
+ const { branchId, branchBaseVersion } = maybeEntity.aux.currentVersion;
+ // branchBaseVersion could be undefined if handling an offline create
+ const currentBranchBaseVersion = branchBaseVersion ?? 0;
+ const nextSub = await _checkHeldSubmission(maybeOne, branchId, currentBranchBaseVersion + 1);
if (nextSub.isDefined()) {
const { submissionId: nextSubmissionId, submissionDefId: nextSubmissionDefId } = nextSub.get();
await Audits.log({ id: event.actorId }, 'submission.reprocess', { acteeId: event.acteeId },
diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js
index fc3d7d0ab..337a1cb2c 100644
--- a/test/integration/api/offline-entities.js
+++ b/test/integration/api/offline-entities.js
@@ -53,6 +53,7 @@ describe('Offline Entities', () => {
// 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);
body.currentVersion.branchId.should.equal(branchId);
});
}));
@@ -354,6 +355,7 @@ describe('Offline Entities', () => {
await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc')
.expect(200)
.then(({ body }) => {
+ body.currentVersion.version.should.equal(1);
// no status property in data because out of order update did not get applied
body.currentVersion.data.should.eql({ age: '22', first_name: 'Johnny' });
});
@@ -404,23 +406,32 @@ describe('Offline Entities', () => {
});
}));
- it('should not apply later trunkVersion when run has not even started', testOfflineEntities(async (service, container) => {
+ it('should not apply later trunkVersion (past existing server version)', testOfflineEntities(async (service, container) => {
const asAlice = await service.login('alice');
const branchId = uuid();
- // much later run index
+ // trunkVersion past existing server version
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="2"')
- .replace('arrived', 'departed')
+ .replace('trunkVersion="1"', 'trunkVersion="2"')
+ .replace('arrived', 'weird case')
)
.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(0);
+
+ await asAlice.get('/v1/projects/1/forms/offlineEntity/submissions/one/audits')
+ .expect(200)
+ .then(({ body }) => {
+ body[0].details.errorMessage.should.eql('Base version (trunkVersion=2) does not exist for entity UUID (12345678-1234-4123-8234-123456789abc) in dataset (people).');
+ });
+
+
await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc')
.expect(200)
.then(({ body }) => {
@@ -475,6 +486,12 @@ describe('Offline Entities', () => {
await exhaust(container);
+ await asAlice.get('/v1/projects/1/forms/offlineEntity/submissions/one-update2/audits')
+ .expect(200)
+ .then(({ body }) => {
+ body[1].action.should.equal('submission.reprocess');
+ });
+
await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc')
.expect(200)
.then(({ body }) => {
@@ -552,7 +569,7 @@ describe('Offline Entities', () => {
let backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`);
backlogCount.should.equal(1);
- // Second submission contains update after create (middle of branhc)
+ // Second submission contains update after create (middle of branch)
await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions')
.send(testData.instances.offlineEntity.two
.replace('create="1"', 'update="1"')