Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Features/entities/relevant to conflict flag #1028

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 39 additions & 12 deletions lib/data/entity.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,39 +354,66 @@ 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: { sourceEvent, submissionCreate, submission },
sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved
sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved
sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved
lastGoodVersion: false
sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved
});

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;
sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved

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

// 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)) : [];
sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved
}

return result;
};

Expand Down
6 changes: 3 additions & 3 deletions lib/resources/entities.js
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
85 changes: 85 additions & 0 deletions test/integration/api/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,91 @@ describe('Entities API', () => {
versions[3].conflictingProperties.should.be.eql(['age', 'label']);
});
}));

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.some(v => v.version === 1).should.be.false();
sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved

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.some(v => v.version < 3).should.be.false();
sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved

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

describe('GET /datasets/:name/entities/:uuid/diffs', () => {
Expand Down
21 changes: 13 additions & 8 deletions test/unit/data/entity.js
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 @@ -682,12 +683,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: { name: 'Jane' }, conflictingProp: [], baseVersion: 1 }),
sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -696,12 +699,14 @@ 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: { age: '77' }, conflictingProperties: [], baseVersion: 1 }),
sadiqkhoja marked this conversation as resolved.
Show resolved Hide resolved
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']);
Expand Down