From 5383a170506f7188338218a07e3acb5f6149cf00 Mon Sep 17 00:00:00 2001 From: Sadiq Khoja Date: Wed, 11 Oct 2023 21:03:32 -0400 Subject: [PATCH 1/4] Updated PATCH entities/:uuid for resolve conflict functionality --- lib/model/query/entities.js | 12 +++- lib/resources/entities.js | 17 +++++- test/integration/api/entities.js | 102 +++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 2 deletions(-) diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index 37827431c..84b66479e 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -132,7 +132,16 @@ createVersion.audit = (updatedEntity, dataset, partial, subDef) => (log) => { }; createVersion.audit.withResult = true; +//////////////////////////////////////////////////////////////////////////////// +// RESOLVE CONFLICT +const resolveConflict = (entity, dataset) => ({ run }) => // eslint-disable-line no-unused-vars + run(sql`update entities set conflict=null where "id"=${entity.id}`); +resolveConflict.audit = (entity, dataset) => (log) => log('entity.update.resolve', dataset, { + entityId: entity.id, + entityDefId: entity.aux.currentVersion.id, + entity: { uuid: entity.uuid, dataset: dataset.name } +}); ///////////////////////////////////////////////////////////////////////// // Processing submission events to create and update entities @@ -449,5 +458,6 @@ module.exports = { createVersion, countByDatasetId, getById, getAll, getAllDefs, del, - createEntitiesFromPendingSubmissions + createEntitiesFromPendingSubmissions, + resolveConflict }; diff --git a/lib/resources/entities.js b/lib/resources/entities.js index 8696f6227..e20fdcb56 100644 --- a/lib/resources/entities.js +++ b/lib/resources/entities.js @@ -104,6 +104,13 @@ module.exports = (service, endpoint) => { const entity = await Entities.getById(dataset.id, params.uuid).then(getOrNotFound); + // User just wants to resolve the conflict, so body is empty + // Resolve the conflict and shortcircuit + if (isTrue(query.resolve) && (!body || Object.keys(body).length === 0)) { + await Entities.resolveConflict(entity, dataset); + return entity; + } + const clientEntityVersion = headers['if-match'] && headers['if-match'].replaceAll('"', ''); const serverEntityVersion = entity.aux.currentVersion.version; // aka baseVersion @@ -116,7 +123,15 @@ module.exports = (service, endpoint) => { const sourceId = await Entities.createSource(); - return Entities.createVersion(dataset, partial, null, serverEntityVersion + 1, sourceId, serverEntityVersion, userAgent); + const updatedEntity = await Entities.createVersion(dataset, partial, null, serverEntityVersion + 1, sourceId, serverEntityVersion, userAgent); + + // User wants to resolve conflict in addition to update the Entity + if (isTrue(query.resolve)) { + await Entities.resolveConflict(updatedEntity, dataset); + } + + return updatedEntity; + })); service.delete('/projects/:projectId/datasets/:name/entities/:uuid', endpoint(async ({ Datasets, Entities }, { auth, params, queryOptions }) => { diff --git a/test/integration/api/entities.js b/test/integration/api/entities.js index e0971f55f..a7dda74ff 100644 --- a/test/integration/api/entities.js +++ b/test/integration/api/entities.js @@ -2,6 +2,7 @@ const appRoot = require('app-root-path'); const { testService } = require('../setup'); const testData = require('../../data/xml'); const { sql } = require('slonik'); +const should = require('should'); const { exhaust } = require(appRoot + '/lib/worker/worker'); @@ -1340,6 +1341,107 @@ describe('Entities API', () => { audit.details.entity.uuid.should.eql('12345678-1234-4123-8234-123456789abc'); audit.details.entity.dataset.should.eql('people'); })); + + describe('resolve conflict', () => { + + const createConflict = async (service, container) => { + const asAlice = await service.login('alice'); + + await asAlice.post('/v1/projects/1/forms?publish=true') + .send(testData.forms.simpleEntity) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.post('/v1/projects/1/forms/simpleEntity/submissions') + .send(testData.instances.simpleEntity.one) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?force=true') + .send({ data: { age: '99' } }) + .expect(200); + + await asAlice.post('/v1/projects/1/forms?publish=true') + .send(testData.forms.updateEntity) + .set('Content-Type', 'application/xml') + .expect(200); + + // all properties changed + await asAlice.post('/v1/projects/1/forms/updateEntity/submissions') + .send(testData.instances.updateEntity.one) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + }; + + it('should resolve the conflict without updating data', testService(async (service, container) => { + await createConflict(service, container); + + const asAlice = await service.login('alice'); + + 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') + .set('X-Extended-Metadata', true) + .expect(200) + .then(({ body: person }) => { + should(person.conflict).be.null(); + }); + })); + + it('should resolve the conflict with updating data', testService(async (service, container) => { + await createConflict(service, container); + + const asAlice = await service.login('alice'); + + await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true') + .send({ data: { first_name: 'John', age: '10' } }) + .set('If-Match', '"3"') + .expect(200); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') + .set('X-Extended-Metadata', true) + .expect(200) + .then(({ body: person }) => { + should(person.conflict).be.null(); + + person.currentVersion.data.age.should.be.eql('10'); + person.currentVersion.data.first_name.should.be.eql('John'); + }); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc/audits') + .expect(200) + .then(({ body: audits }) => { + audits[0].action.should.be.eql('entity.update.resolve'); + }); + })); + + it('should resolve the conflict and forcefully update the entity', testService(async (service, container) => { + await createConflict(service, container); + + const asAlice = await service.login('alice'); + + await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true&force=true') + .send({ data: { first_name: 'John', age: '10' } }) + .expect(200); + + await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') + .set('X-Extended-Metadata', true) + .expect(200) + .then(({ body: person }) => { + should(person.conflict).be.null(); + + person.currentVersion.data.age.should.be.eql('10'); + person.currentVersion.data.first_name.should.be.eql('John'); + }); + })); + + }); + }); describe('DELETE /datasets/:name/entities/:uuid', () => { From 20049260db92454bd9acb5d2496c69c96c24c3e5 Mon Sep 17 00:00:00 2001 From: Sadiq Khoja Date: Thu, 12 Oct 2023 11:59:47 -0400 Subject: [PATCH 2/4] Update API doc --- docs/api.yaml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/api.yaml b/docs/api.yaml index e366d61ba..73a515fcb 100644 --- a/docs/api.yaml +++ b/docs/api.yaml @@ -10235,6 +10235,10 @@ paths: Use this API to update one or all properties of an Entity. It will throw `400 - Bad Request` if any of the updating properties doesn't exist in the dataset. To unset value of any property, you can set it to empty string (""). Setting it to `null` will throw an error. + + **Resolve the conflict** + + You can also use this endpoint to resolve the conflict by passing `resolve=true` query parameter. If you set `resolve` parameter then providing data in the body is optional, in that case only `conflict` status from the Entity will be cleared. operationId: Updating an Entity parameters: - name: projectId @@ -10258,6 +10262,20 @@ paths: schema: type: string example: 54a405a0-53ce-4748-9788-d23a30cc3afa + - name: force + in: query + description: Flag to forcefully update the Entity + required: false + schema: + type: boolean + example: true + - name: resolve + in: query + description: Flag to resolve the conflict + required: false + schema: + type: boolean + example: true requestBody: content: '*/*': From 54b73169286d584512bfffe0ead72906b6535c74 Mon Sep 17 00:00:00 2001 From: Sadiq Khoja Date: Fri, 13 Oct 2023 15:19:05 -0400 Subject: [PATCH 3/4] Add new verb in audit filter --- docs/api.yaml | 42 +++++++++++++++++++++++++++++++- lib/model/query/audits.js | 4 +-- test/integration/api/audits.js | 21 +++++++++++++++- test/integration/api/entities.js | 5 +++- 4 files changed, 67 insertions(+), 5 deletions(-) diff --git a/docs/api.yaml b/docs/api.yaml index 73a515fcb..da0984d37 100644 --- a/docs/api.yaml +++ b/docs/api.yaml @@ -36,7 +36,19 @@ info: Here major and breaking changes to the API are listed by version. - ## ODK Central v2023.4 + ### ODK Central v2023.5 + + **Added**: + + - Entities accessed via API now have conflict and version-specific conflictingProperties fields + + * Conflicts are either hard (baseVersion conflicts and multiple versions update the same property), soft (baseVersion conflicts but data updates are independent), or null + + **Changed**: + + - Conflicts can be resolved using the [PATCH entity](/central-api-entity-management/#updating-an-entity) endpoint with or without new data + + ### ODK Central v2023.4 **Added**: @@ -9957,6 +9969,7 @@ paths: updatedAt: '2018-03-21T12:45:02.312Z' deletedAt: '2018-03-21T12:45:02.312Z' creatorId: 1 + conflict: null creator: createdAt: '2018-04-18T23:19:14.802Z' displayName: My Display Name @@ -9970,6 +9983,7 @@ paths: createdAt: '2018-03-21T12:45:02.312Z' creatorId: 1 userAgent: Enketo/3.0.4 + conflictingProperties: null creator: createdAt: '2018-04-18T23:19:14.802Z' displayName: My Display Name @@ -10042,12 +10056,14 @@ paths: updatedAt: '2018-03-21T12:45:02.312Z' deletedAt: '2018-03-21T12:45:02.312Z' creatorId: 1 + conflict: null currentVersion: label: John (88) current: true createdAt: '2018-03-21T12:45:02.312Z' creatorId: 1 userAgent: Enketo/3.0.4 + conflictingProperties: null data: firstName: John age: '88' @@ -10101,12 +10117,14 @@ paths: updatedAt: '2018-03-21T12:45:02.312Z' deletedAt: '2018-03-21T12:45:02.312Z' creatorId: 1 + conflict: null currentVersion: label: John (88) current: true createdAt: '2018-03-21T12:45:02.312Z' creatorId: 1 userAgent: Enketo/3.0.4 + conflictingProperties: null data: firstName: John age: '88' @@ -10121,6 +10139,7 @@ paths: updatedAt: '2018-03-21T12:45:02.312Z' deletedAt: '2018-03-21T12:45:02.312Z' creatorId: 1 + conflict: null creator: createdAt: '2018-04-18T23:19:14.802Z' displayName: My Display Name @@ -10134,6 +10153,7 @@ paths: createdAt: '2018-03-21T12:45:02.312Z' creatorId: 1 userAgent: Enketo/3.0.4 + conflictingProperties: null data: firstName: John age: '88' @@ -10305,12 +10325,14 @@ paths: updatedAt: '2018-03-21T12:45:02.312Z' deletedAt: '2018-03-21T12:45:02.312Z' creatorId: 1 + conflict: null currentVersion: label: John (88) current: true createdAt: '2018-03-21T12:45:02.312Z' creatorId: 1 userAgent: Enketo/3.0.4 + conflictingProperties: null data: firstName: John age: '88' @@ -10390,6 +10412,7 @@ paths: createdAt: '2018-03-21T12:45:02.312Z' creatorId: 1 userAgent: Enketo/3.0.4 + conflictingProperties: null data: firstName: John age: '88' @@ -10405,6 +10428,7 @@ paths: createdAt: '2018-03-21T12:45:02.312Z' creatorId: 1 userAgent: Enketo/3.0.4 + conflictingProperties: null data: firstName: John age: '88' @@ -13193,6 +13217,17 @@ components: type: number description: The ID of the Actor (App User, User, or Public Link) that originally created the Entity. example: 1 + conflict: + type: string + description: |- + Type of the conflict. + + `hard`: baseVersion conflicts and multiple versions update the same property + + `soft`: baseVersion conflicts but data updates are independent + enum: + - soft + - hard EntityVersionFields: type: object properties: @@ -13212,6 +13247,11 @@ components: type: string description: The self-identified `userAgent` of the device that created the `Entity` version. example: Enketo/3.0.4 + conflictingProperties: + type: array + description: list of properties updated offline simultaneously. + items: + type: string EntitySummary: allOf: - $ref: '#/components/schemas/EntitySummaryFields' diff --git a/lib/model/query/audits.js b/lib/model/query/audits.js index 24ae80cb8..4c45eef82 100644 --- a/lib/model/query/audits.js +++ b/lib/model/query/audits.js @@ -36,7 +36,7 @@ const actionCondition = (action) => { // The backup action was logged by a backup script that has been removed. // Even though the script has been removed, the audit log entries it logged // have not, so we should continue to exclude those. - return sql`action not in ('entity.create', 'entity.create.error', 'entity.update.version', 'entity.delete', 'submission.create', 'submission.update', 'submission.update.version', 'submission.attachment.update', 'backup', 'analytics')`; + return sql`action not in ('entity.create', 'entity.create.error', 'entity.update.version', 'entity.update.resolve', 'entity.delete', 'submission.create', 'submission.update', 'submission.update.version', 'submission.attachment.update', 'backup', 'analytics')`; else if (action === 'user') return sql`action in ('user.create', 'user.update', 'user.delete', 'user.assignment.create', 'user.assignment.delete', 'user.session.create')`; else if (action === 'field_key') @@ -52,7 +52,7 @@ const actionCondition = (action) => { else if (action === 'dataset') return sql`action in ('dataset.create', 'dataset.update')`; else if (action === 'entity') - return sql`action in ('entity.create', 'entity.create.error', 'entity.update.version', 'entity.delete')`; + return sql`action in ('entity.create', 'entity.create.error', 'entity.update.version', 'entity.update.resolve', 'entity.delete')`; return sql`action=${action}`; }; diff --git a/test/integration/api/audits.js b/test/integration/api/audits.js index 5131cfe78..074ccf804 100644 --- a/test/integration/api/audits.js +++ b/test/integration/api/audits.js @@ -382,6 +382,23 @@ describe('/audits', () => { data: { age: '77', first_name: 'Alan' } }) .expect(200); + + await asAlice.post('/v1/projects/1/forms?publish=true') + .send(testData.forms.updateEntity) + .set('Content-Type', 'application/xml') + .expect(200); + + // all properties changed + await asAlice.post('/v1/projects/1/forms/updateEntity/submissions') + .send(testData.instances.updateEntity.one) + .set('Content-Type', 'application/xml') + .expect(200); + + await exhaust(container); + + await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true') + .expect(200); + await asAlice.delete('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc') .expect(200) .then(({ body }) => { @@ -391,9 +408,11 @@ describe('/audits', () => { await asAlice.get('/v1/audits?action=entity') .expect(200) .then(({ body }) => { - body.length.should.equal(4); + body.length.should.equal(6); body.map(a => a.action).should.eql([ 'entity.delete', + 'entity.update.resolve', + 'entity.update.version', 'entity.update.version', 'entity.create.error', 'entity.create' diff --git a/test/integration/api/entities.js b/test/integration/api/entities.js index a7dda74ff..cc31d5837 100644 --- a/test/integration/api/entities.js +++ b/test/integration/api/entities.js @@ -1398,7 +1398,9 @@ describe('Entities API', () => { const asAlice = await service.login('alice'); - await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true') + const asBob = await service.login('bob'); + + await asBob.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true') .send({ data: { first_name: 'John', age: '10' } }) .set('If-Match', '"3"') .expect(200); @@ -1417,6 +1419,7 @@ describe('Entities API', () => { .expect(200) .then(({ body: audits }) => { audits[0].action.should.be.eql('entity.update.resolve'); + audits[0].actor.displayName.should.eql('Bob'); }); })); From d069b22f18c767b8d3a8c77d7f6308d371a87393 Mon Sep 17 00:00:00 2001 From: Sadiq Khoja Date: Tue, 17 Oct 2023 10:58:54 -0400 Subject: [PATCH 4/4] return error if there's no conflict --- docs/api.yaml | 4 ++-- lib/resources/entities.js | 2 ++ lib/util/problem.js | 2 ++ test/integration/api/entities.js | 10 ++++++++++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/docs/api.yaml b/docs/api.yaml index da0984d37..e3a6c19a1 100644 --- a/docs/api.yaml +++ b/docs/api.yaml @@ -36,13 +36,13 @@ info: Here major and breaking changes to the API are listed by version. - ### ODK Central v2023.5 + ## ODK Central v2023.5 **Added**: - Entities accessed via API now have conflict and version-specific conflictingProperties fields - * Conflicts are either hard (baseVersion conflicts and multiple versions update the same property), soft (baseVersion conflicts but data updates are independent), or null + * Conflicts are either `hard` (baseVersion conflicts and multiple versions update the same property), `soft` (baseVersion conflicts but data updates are independent), or `null` **Changed**: diff --git a/lib/resources/entities.js b/lib/resources/entities.js index e20fdcb56..8aeaf2470 100644 --- a/lib/resources/entities.js +++ b/lib/resources/entities.js @@ -104,6 +104,8 @@ module.exports = (service, endpoint) => { const entity = await Entities.getById(dataset.id, params.uuid).then(getOrNotFound); + if (isTrue(query.resolve) && !entity.conflict) return reject(Problem.user.noConflictEntity()); + // User just wants to resolve the conflict, so body is empty // Resolve the conflict and shortcircuit if (isTrue(query.resolve) && (!body || Object.keys(body).length === 0)) { diff --git a/lib/util/problem.js b/lib/util/problem.js index 0f5a2b247..ee1025e35 100644 --- a/lib/util/problem.js +++ b/lib/util/problem.js @@ -122,6 +122,8 @@ const problems = { // { expected: "list of expected parameters", actual: "list of provided parameters" } unexpectedAttributes: problem(400.31, ({ expected, actual }) => `Expected parameters: (${expected.join(', ')}). Got (${actual.join(', ')}).`), + noConflictEntity: problem(400.32, () => 'The Entity doesn\'t have any conflict'), + // no detail information for security reasons. authenticationFailed: problem(401.2, () => 'Could not authenticate with the provided credentials.'), diff --git a/test/integration/api/entities.js b/test/integration/api/entities.js index cc31d5837..ecde98285 100644 --- a/test/integration/api/entities.js +++ b/test/integration/api/entities.js @@ -1443,6 +1443,16 @@ describe('Entities API', () => { }); })); + it('should throw error if there is no conflict', testEntities(async (service) => { + const asAlice = await service.login('alice'); + + await asAlice.patch('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc?resolve=true') + .expect(400) + .then(({ body }) => { + body.code.should.be.eql(400.32); + }); + })); + }); });