Skip to content

Commit

Permalink
Features/entities/relevant to conflict flag (#1028)
Browse files Browse the repository at this point in the history 
* Added relevantToConflict flag in entity diff API

* incorporated PR feedback

* Use array indexing instead of Maps
+ change the definition of lastGoodKnow version

* added more tests
  • Loading branch information
@sadiqkhoja
sadiqkhoja authored Oct 31, 2023
1 parent 9d1bbd7 commit 5b386ee
Show file tree
Hide file tree
Showing 4 changed files with 198 additions and 25 deletions.
55 changes: 43 additions & 12 deletions lib/data/entity.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,39 +354,70 @@ const ConflictType = {
SOFT: 'soft', // discrepancy in version number but no overlaping properties
HARD: 'hard' // discrepancy in version number and one/more properties updated simultaneously
};
const getWithConflictDetails = (defs) => {
const defMap = new Map(defs.map(d => [d.version, d]));

const getWithConflictDetails = (defs, audits, relevantToConflict) => {
const result = [];

const lastResolveEvent = audits.findLast(a => a.action === 'entity.update.resolve');

const auditMap = new Map(audits
.filter(a => a.action === 'entity.create' || a.action === 'entity.update.version')
.map(a => [a.details.entityDefId, a.details]));

let lastGoodVersion = 0; // Entity Version is never 0

const relevantBaseVersions = new Set();

for (const def of defs) {
const v = mergeLeft(def,
const { sourceEvent, submissionCreate, submission } = auditMap.get(def.id);

const v = mergeLeft(def.forApi(),
{
conflict: null,
resolved: false,
baseDiff: [],
serverDiff: [],
conflictingProp: [],
source: {} // TODO
source: { event: sourceEvent, submissionCreate, submission },
lastGoodVersion: false
});

if (v.version > 1) { // v.root is false here - can use either
const conflict = v.version !== (v.baseVersion + 1);

v.baseDiff = getDiffProp(v.dataReceived, defMap.get(v.baseVersion).data);
if ('label' in v.dataReceived && v.dataReceived.label !== defMap.get(v.baseVersion).label) v.baseDiff.push('label');
v.baseDiff = getDiffProp(v.dataReceived, defs[v.baseVersion - 1].data);
if ('label' in v.dataReceived && v.dataReceived.label !== defs[v.baseVersion - 1].label) v.baseDiff.push('label');

v.serverDiff = getDiffProp(v.dataReceived, defMap.get(v.version - 1).data);
if ('label' in v.dataReceived && v.dataReceived.label !== defMap.get(v.version - 1).label) v.serverDiff.push('label');
v.serverDiff = getDiffProp(v.dataReceived, defs[v.version - 2].data);
if ('label' in v.dataReceived && v.dataReceived.label !== defs[v.version - 2].label) v.serverDiff.push('label');

if (conflict) {
v.conflict = v.conflictingProp && v.conflictingProp.length > 0 ? ConflictType.HARD : ConflictType.SOFT;
v.conflict = v.conflictingProperties && v.conflictingProperties.length > 0 ? ConflictType.HARD : ConflictType.SOFT;

v.resolved = lastResolveEvent && lastResolveEvent.details.entityDefId >= def.id;

if (!v.resolved && lastGoodVersion === 0) {
lastGoodVersion = v.version - 1;
result[v.version - 2].lastGoodVersion = true;
}
}

v.resolve = false; // TOOD
// We have some unresolved conflicts
if (lastGoodVersion > 0) {
relevantBaseVersions.add(v.baseVersion);
}
}

result.push(v);
}

// We return all the versions after last good version and the base versions of those.
if (relevantToConflict) {
return lastGoodVersion > 0 ? result.filter(v => v.version >= lastGoodVersion || relevantBaseVersions.has(v.version)) : [];
}

if (lastGoodVersion === 0) {
result[result.length - 1].lastGoodVersion = true;
}

return result;
};

Expand Down
6 changes: 3 additions & 3 deletions lib/resources/entities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ module.exports = (service, endpoint) => {
return withEtag(entity.aux.currentVersion.version, () => entity);
}));

service.get('/projects/:projectId/datasets/:name/entities/:uuid/versions', endpoint(async ({ Datasets, Entities, Audits }, { params, auth, queryOptions }) => {
service.get('/projects/:projectId/datasets/:name/entities/:uuid/versions', endpoint(async ({ Datasets, Entities, Audits }, { params, auth, queryOptions, query }) => {

const dataset = await Datasets.get(params.projectId, params.name, true).then(getOrNotFound);

Expand All @@ -47,9 +47,9 @@ module.exports = (service, endpoint) => {
// it means there's no entity with the provided UUID
if (defs.length === 0) return reject(Problem.user.notFound());

const audits = Audits.getByEntityId(defs[0].id, queryOptions);
const audits = await Audits.getByEntityId(defs[0].entityId, queryOptions);

return getWithConflictDetails(defs.map(d => d.forApi()), audits);
return getWithConflictDetails(defs, audits, isTrue(query.relevantToConflict));
}));

service.get('/projects/:projectId/datasets/:name/entities/:uuid/diffs', endpoint(async ({ Datasets, Entities }, { params, auth, queryOptions }) => {
Expand Down
125 changes: 123 additions & 2 deletions test/integration/api/entities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ describe('Entities API', () => {
});

versions[1].data.should.be.eql({ age: '12', first_name: 'John' });
versions[1].lastGoodVersion.should.be.true();
});
}));

Expand Down Expand Up @@ -549,10 +550,130 @@ describe('Entities API', () => {
v.should.have.property('data');
});

versions[2].conflictingProperties.should.be.eql([]);
versions[3].conflictingProperties.should.be.eql(['age', 'label']);
const thirdVersion = versions[2];
thirdVersion.conflict.should.be.eql('soft');
thirdVersion.conflictingProperties.should.be.eql([]);
thirdVersion.source.event.action.should.be.eql('submission.create');
thirdVersion.source.submission.instanceId.should.be.eql('two');

const fourthVersion = versions[3];
fourthVersion.conflict.should.be.eql('hard');
fourthVersion.conflictingProperties.should.be.eql(['age', 'label']);
fourthVersion.source.event.action.should.be.eql('submission.create');
fourthVersion.source.submission.instanceId.should.be.eql('one');

});
}));

describe('relevantToConflict', () => {

const createConflictOnV2 = async (user, container) => {
await user.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc')
.send({ data: { age: '12' } })
.set('If-Match', '"1"')
.expect(200);

await user.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc')
.send({ data: { age: '18' } })
.set('If-Match', '"2"')
.expect(200);

await user.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.updateEntity)
.set('Content-Type', 'application/xml')
.expect(200);

// Hard conflict - all properties are changed
await user.post('/v1/projects/1/forms/updateEntity/submissions')
.send(testData.instances.updateEntity.one.replace('baseVersion="1"', 'baseVersion="2"'))
.set('Content-Type', 'application/xml')
.expect(200);

await exhaust(container);
};

it('should return only relevent versions needed for conflict resolution', testEntities(async (service, container) => {
const asAlice = await service.login('alice');

await createConflictOnV2(asAlice, container);

await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc/versions?relevantToConflict=true')
.expect(200)
.then(({ body: versions }) => {
// Doesn't return first version
versions.map(v => v.version).should.eql([2, 3, 4]);

versions[1].lastGoodVersion.should.be.true();
versions[2].conflictingProperties.should.be.eql(['age']);
});
}));

it('should return empty array when all conflicts are resolved', testEntities(async (service, container) => {
const asAlice = await service.login('alice');

await createConflictOnV2(asAlice, container);

await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true')
.expect(200);

await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc/versions?relevantToConflict=true')
.expect(200)
.then(({ body: versions }) => {
versions.length.should.be.eql(0);
});
}));

it('should return only relevent versions after conflict resolution', testEntities(async (service, container) => {
const asAlice = await service.login('alice');

await createConflictOnV2(asAlice, container);

await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true')
.expect(200);

await asAlice.post('/v1/projects/1/forms/updateEntity/submissions')
.send(testData.instances.updateEntity.two
.replace('baseVersion="1"', 'baseVersion="3"'))
.set('Content-Type', 'application/xml')
.expect(200);

await exhaust(container);

await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc/versions?relevantToConflict=true')
.expect(200)
.then(({ body: versions }) => {
// Doesn't return old versions
versions.map(v => v.version).should.eql([3, 4, 5]);

versions[1].lastGoodVersion.should.be.true();
versions[2].conflictingProperties.should.be.eql(['label']);
});
}));

it('should correctly set `resolved` flag for the versions', testEntities(async (service, container) => {
const asAlice = await service.login('alice');

await createConflictOnV2(asAlice, container);

await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true')
.expect(200);

await asAlice.post('/v1/projects/1/forms/updateEntity/submissions')
.send(testData.instances.updateEntity.two
.replace('baseVersion="1"', 'baseVersion="2"'))
.set('Content-Type', 'application/xml')
.expect(200);

await exhaust(container);

await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc/versions')
.expect(200)
.then(({ body: versions }) => {
// resolved flag is true only for the old conflict
versions.map(v => v.resolved).should.eql([false, false, false, true, false]);
});
}));
});
});

describe('GET /datasets/:name/entities/:uuid/diffs', () => {
Expand Down
37 changes: 29 additions & 8 deletions test/unit/data/entity.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const should = require('should');
const appRoot = require('app-root-path');
const assert = require('assert');
const { ConflictType } = require('../../../lib/data/entity');
const { Entity } = require('../../../lib/model/frames');
const { parseSubmissionXml, extractEntity, validateEntity, extractSelectedProperties, selectFields, diffEntityData, getDiffProp, getWithConflictDetails } = require(appRoot + '/lib/data/entity');
const { fieldsFor } = require(appRoot + '/test/util/schema');
const testData = require(appRoot + '/test/data/xml');
Expand Down Expand Up @@ -687,12 +688,14 @@ describe('extracting and validating entities', () => {

it('should fill in correct information for SOFT conflict', () => {
const defs = [
{ version: 1, label: 'John', data: { name: 'John', age: '88' }, dataReceived: { name: 'John', age: '88' }, conflictingProp: null, baseVersion: null },
{ version: 2, label: 'Jane', data: { name: 'Jane', age: '88' }, dataReceived: { name: 'Jane' }, conflictingProp: [], baseVersion: 1 },
{ version: 3, label: 'Jane', data: { name: 'Jane', age: '99' }, dataReceived: { age: '99' }, conflictingProp: [], baseVersion: 1 }
new Entity.Def({ id: 0, version: 1, label: 'John', data: { name: 'John', age: '88' }, dataReceived: { name: 'John', age: '88' }, conflictingProp: null, baseVersion: null }),
new Entity.Def({ id: 0, version: 2, label: 'Jane', data: { name: 'Jane', age: '88' }, dataReceived: { label: 'Jane', name: 'Jane' }, conflictingProp: [], baseVersion: 1 }),
new Entity.Def({ id: 0, version: 3, label: 'Jane', data: { name: 'Jane', age: '99' }, dataReceived: { age: '99' }, conflictingProp: [], baseVersion: 1 })
];

const result = getWithConflictDetails(defs);
const audits = [{ action: 'entity.create', details: { entityDefId: 0 } }];

const result = getWithConflictDetails(defs, audits, false);

result[2].conflict.should.be.eql(ConflictType.SOFT);
result[2].baseDiff.should.be.eql(['age']);
Expand All @@ -701,16 +704,34 @@ describe('extracting and validating entities', () => {

it('should fill in correct information for HARD conflict', () => {
const defs = [
{ version: 1, label: 'John', data: { name: 'John', age: '88' }, dataReceived: { name: 'John', age: '88' }, conflictingProp: null, baseVersion: null },
{ version: 2, label: 'Jane', data: { name: 'Jane', age: '77' }, dataReceived: { age: '77' }, conflictingProp: [], baseVersion: 1 },
{ version: 3, label: 'Jane', data: { name: 'Jane', age: '99' }, dataReceived: { age: '99' }, conflictingProp: ['age'], baseVersion: 1 }
new Entity.Def({ id: 0, version: 1, label: 'John', data: { name: 'John', age: '88' }, dataReceived: { name: 'John', age: '88' }, conflictingProperties: null, baseVersion: null }),
new Entity.Def({ id: 0, version: 2, label: 'Jane', data: { name: 'Jane', age: '77' }, dataReceived: { label: 'Jane', name: 'Jane', age: '77' }, conflictingProperties: [], baseVersion: 1 }),
new Entity.Def({ id: 0, version: 3, label: 'Jane', data: { name: 'Jane', age: '99' }, dataReceived: { age: '99' }, conflictingProperties: ['age'], baseVersion: 1 })
];

const result = getWithConflictDetails(defs);
const audits = [{ action: 'entity.create', details: { entityDefId: 0 } }];

const result = getWithConflictDetails(defs, audits, false);

result[2].conflict.should.be.eql(ConflictType.HARD);
result[2].baseDiff.should.be.eql(['age']);
result[2].serverDiff.should.be.eql(['age']);
});

it('should return only relevant versions', () => {
const defs = [
new Entity.Def({ id: 0, version: 1, label: 'John', data: { name: 'John', age: '88' }, dataReceived: { name: 'John', age: '88' }, conflictingProp: null, baseVersion: null }),
new Entity.Def({ id: 0, version: 2, label: 'Robert', data: { name: 'Robert', age: '20' }, dataReceived: { label: 'Robert', name: 'Robert', age: '20' }, conflictingProp: null, baseVersion: 1 }),
new Entity.Def({ id: 0, version: 3, label: 'Jane', data: { name: 'Jane', age: '20' }, dataReceived: { label: 'Jane', name: 'Jane' }, conflictingProp: [], baseVersion: 2 }),
new Entity.Def({ id: 0, version: 4, label: 'Jane', data: { name: 'Jane', age: '99' }, dataReceived: { age: '99' }, conflictingProp: [], baseVersion: 2 }),
new Entity.Def({ id: 0, version: 5, label: 'Jane', data: { name: 'Jane', age: '10' }, dataReceived: { age: '10' }, conflictingProp: [], baseVersion: 3 }),
];

const audits = [{ action: 'entity.create', details: { entityDefId: 0 } }];

const result = getWithConflictDetails(defs, audits, true);

result.map(v => v.version).should.eql([2, 3, 4, 5]);
});
});
});

0 comments on commit 5b386ee

Please sign in to comment.