Skip to content

Commit

Permalink
Improving out of order processing for complex branches
Browse files Browse the repository at this point in the history
  • Loading branch information
ktuite committed Jun 12, 2024
1 parent e82b5de commit 97ab093
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 3 deletions.
17 changes: 14 additions & 3 deletions lib/model/query/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,19 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss
const clientEntity = await Entity.fromParseEntityData(entityData, { update: true }); // validation happens here

// Get version of entity on the server
const serverEntity = (await Entities.getById(dataset.id, clientEntity.uuid, QueryOptions.forUpdate))
.orThrow(Problem.user.entityNotFound({ entityUuid: clientEntity.uuid, datasetName: dataset.name }));
// TODO: comments about what is happening here, too
let serverEntity = await Entities.getById(dataset.id, clientEntity.uuid, QueryOptions.forUpdate);
if (!serverEntity.isDefined()) {
if (clientEntity.def.branchId == null) {
throw Problem.user.entityNotFound({ entityUuid: clientEntity.uuid, datasetName: dataset.name });
} else {
await _holdSubmission(run, submissionDef.submissionId, submissionDef.id, clientEntity.def.branchId, clientEntity.def.baseVersion);
return null;
}
} else {
serverEntity = serverEntity.get();
}


let { conflict } = serverEntity;
let conflictingProperties; // Maybe we don't need to persist this??? just compute at the read time
Expand Down Expand Up @@ -391,7 +402,7 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Audits, Dataset
// Check for held submissions that follow this one in the same branch
if (entityData.system.branchId != null) {
// baseVersion could be '', meaning its a create
const currentBaseVersion = entityData.system.baseVersion === '' ? 1 : parseInt(entityData.system.baseVersion, 10);
const currentBaseVersion = entityData.system.baseVersion === '' ? 0 : parseInt(entityData.system.baseVersion, 10);
const nextSub = await _checkHeldSubmission(maybeOne, entityData.system.branchId, currentBaseVersion + 1);
if (nextSub.isDefined()) {
const { submissionId: nextSubmissionId, submissionDefId: nextSubmissionDefId } = nextSub.get();
Expand Down
73 changes: 73 additions & 0 deletions test/integration/api/offline-entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,40 @@ describe('Offline Entities', () => {
entity.aux.currentVersion.branchBaseVersion.should.equal(2);
entity.aux.currentVersion.data.should.eql({ age: '22', status: 'departed', first_name: 'Johnny' });
}));

it('should handle an offline branch that starts with a create', testOfflineEntities(async (service, container) => {
const asAlice = await service.login('alice');
const branchId = uuid();
const dataset = await container.Datasets.get(1, 'people', true).then(getOrNotFound);

// 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}"`)
)
.set('Content-Type', 'application/xml')
.expect(200);

// Second submission updates the entity
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('<status>new</status>', '<status>checked in</status>')
)
.set('Content-Type', 'application/xml')
.expect(200);

await exhaust(container);

const entity = await container.Entities.getById(dataset.id, '12345678-1234-4123-8234-123456789ddd').then(getOrNotFound);
entity.aux.currentVersion.branchId.should.equal(branchId);
entity.aux.currentVersion.version.should.equal(2);
entity.aux.currentVersion.baseVersion.should.equal(1);
entity.aux.currentVersion.data.should.eql({ age: '20', status: 'checked in', first_name: 'Megan' });
}));
});

describe('out of order runs', () => {
Expand Down Expand Up @@ -337,5 +371,44 @@ describe('Offline Entities', () => {
entity.aux.currentVersion.baseVersion.should.equal(3);
entity.aux.currentVersion.data.should.eql({ age: '22', status: 'departed', first_name: 'Johnny' });
}));

it('should handle offline update that comes before a create', testOfflineEntities(async (service, container) => {
const asAlice = await service.login('alice');
const branchId = uuid();
const dataset = await container.Datasets.get(1, 'people', true).then(getOrNotFound);

// Second submission updates the entity but entity hasn't been created yet
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('<status>new</status>', '<status>checked in</status>')
)
.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);

// First submission creating the entity comes in later
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);

const entity = await container.Entities.getById(dataset.id, '12345678-1234-4123-8234-123456789ddd').then(getOrNotFound);
entity.aux.currentVersion.branchId.should.equal(branchId);
entity.aux.currentVersion.version.should.equal(2);
entity.aux.currentVersion.baseVersion.should.equal(1);
entity.aux.currentVersion.data.should.eql({ age: '20', status: 'checked in', first_name: 'Megan' });
}));
});
});

0 comments on commit 97ab093

Please sign in to comment.