Skip to content

Commit

Permalink
return entityUniquenessViolationWithDeleted if entity is soft deleted…
Browse files Browse the repository at this point in the history 
… or purged
  • Loading branch information
@sadiqkhoja
sadiqkhoja committed Jan 28, 2025
1 parent cdcc822 commit 1e35617
Show file tree
Hide file tree
Showing 5 changed files with 320 additions and 18 deletions.
27 changes: 16 additions & 11 deletions lib/model/query/entities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,19 @@ const { PURGE_DAY_RANGE } = require('../../util/constants');
/////////////////////////////////////////////////////////////////////////////////
// Check if provided UUIDs (array) were used by purged entities.

const _getPurgedUuids = (all, uuids) => all(sql`
const _getPurgedOrDeletedUuids = (all, uuids) => all(sql`
SELECT jsonb_array_elements_text(details -> 'entityUuids') AS uuid FROM audits a WHERE action = 'entity.purge'
INTERSECT
SELECT jsonb_array_elements_text(${JSON.stringify(uuids)})`)
SELECT jsonb_array_elements_text(${JSON.stringify(uuids)})
UNION
SELECT uuid FROM entities WHERE "deletedAt" IS NOT NULL AND uuid = ANY (ARRAY[${sql.array(uuids, 'text')}])`)
.then(all.map(r => r.uuid));

const _isPurgedUuid = (maybeOneFirst, uuid) => maybeOneFirst(sql`
SELECT TRUE FROM audits WHERE action = 'entity.purge' AND details -> 'entityUuids' @> ${JSON.stringify([uuid])}
const _isPurgedOrDeletedUuid = (oneFirst, uuid) => oneFirst(sql`
SELECT
EXISTS ( SELECT 1 FROM audits WHERE action = 'entity.purge' AND details -> 'entityUuids' @> ${JSON.stringify([uuid])} )
OR
EXISTS ( SELECT 1 FROM entities WHERE uuid = ${uuid} AND "deletedAt" IS NOT NULL )
`);
/////////////////////////////////////////////////////////////////////////////////
// ENTITY DEF SOURCES
Expand Down Expand Up @@ -77,15 +82,14 @@ const nextval = sql`nextval(pg_get_serial_sequence('entities', 'id'))`;
// standard authenticated API request. The worker has better access to the event
// actor/initiator and actee/target so it will do the logging itself (including
// error logging).
const createNew = (dataset, partial, subDef, sourceId, userAgentIn) => async ({ one, context, maybeOneFirst }) => {
const createNew = (dataset, partial, subDef, sourceId, userAgentIn) => async ({ one, context, oneFirst }) => {
let creatorId;
let userAgent;

// Validate UUID against purged entities
(await _isPurgedUuid(maybeOneFirst, partial.uuid))
.ifDefined(() => {
throw Problem.user.uniquenessViolation({ fields: 'uuid', values: partial.uuid });
});
if (await _isPurgedOrDeletedUuid(oneFirst, partial.uuid)) {
throw Problem.user.entityUniquenessViolationWithDeleted({ entityUuids: [partial.uuid] });
}

// Set creatorId and userAgent from submission def if it exists
if (subDef != null) {
Expand Down Expand Up @@ -130,9 +134,9 @@ const createMany = (dataset, rawEntities, sourceId, userAgentIn) => async ({ all
const userAgent = blankStringToNull(userAgentIn);

// Validate UUID with the purged entities
const purgedUuids = await _getPurgedUuids(all, rawEntities.map(e => e.uuid));
const purgedUuids = await _getPurgedOrDeletedUuids(all, rawEntities.map(e => e.uuid));
if (purgedUuids.length > 0) {
throw Problem.user.uniquenessViolation({ fields: 'uuid', values: purgedUuids.join(', ') });
throw Problem.user.entityUniquenessViolationWithDeleted({ entityUuids: purgedUuids });
}

// Augment parsed entity data with dataset and creator IDs
Expand Down Expand Up @@ -942,6 +946,7 @@ const purge = (force = false, projectId = null, datasetName = null, entityUuid =
WHERE entity_def_sources.id = entity_defs."sourceId"
AND entity_defs."entityId" = entities.id
AND entities."datasetId" = datasets.id
AND (entity_def_sources.type = 'submission' OR (entity_def_sources.type = 'api' AND (entity_def_sources.details IS NULL OR entity_def_sources.details = 'null'))) -- don't detail bulk source
AND ${_trashedFilter(force, projectId, datasetName, entityUuid)}
), delete_submission_backlog AS (
DELETE FROM entity_submission_backlog
Expand Down
4 changes: 3 additions & 1 deletion lib/util/problem.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,9 @@ const problems = {

propertyNameConflict: problem(409.17, () => 'This form attempts to create new Entity properties that match with existing ones except for capitalization.'),

duplicateSubmission: problem(409.18, () => `This submission has been deleted. You may not resubmit it.`)
duplicateSubmission: problem(409.18, () => `This submission has been deleted. You may not resubmit it.`),

entityUniquenessViolationWithDeleted: problem(409.19, ({ entityUuids }) => `The following UUID(s) cannot be used because they are associated with deleted Entities: (${entityUuids.join(', ')}).`),
},
internal: {
// no detail information, as this is only called when we don't know what happened.
Expand Down
93 changes: 92 additions & 1 deletion test/integration/api/entities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1455,6 +1455,28 @@ describe('Entities API', () => {
.expect(409);
}));

it('should reject if entity with the same uuid is soft deleted', testEntities(async (service) => {
// Use testEntities here vs. testDataset to prepopulate with 2 entities
const asAlice = await service.login('alice');

await asAlice.delete('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc')
.expect(200);

await asAlice.post('/v1/projects/1/datasets/people/entities')
.send({
uuid: '12345678-1234-4123-8234-123456789abc',
label: 'Johnny Doe',
data: {
first_name: 'Johnny',
age: '22'
}
})
.expect(409)
.then(({ body }) => {
body.message.should.be.eql('The following UUID(s) cannot be used because they are associated with deleted Entities: (12345678-1234-4123-8234-123456789abc).');
});
}));

it('should reject if data properties do not match dataset exactly', testDataset(async (service) => {
const asAlice = await service.login('alice');

Expand Down Expand Up @@ -2322,7 +2344,6 @@ describe('Entities API', () => {
});
}));


it('should generate uuids for entities when no uuid is provided', testDataset(async (service) => {
const asAlice = await service.login('alice');

Expand Down Expand Up @@ -2579,6 +2600,76 @@ describe('Entities API', () => {
body[0].action.should.not.equal('entity.bulk.create');
});
}));

it('should not create any entities if there is a UUID collision with soft deleted Entity', testServiceFullTrx(async (service) => {
const asAlice = await service.login('alice');

await asAlice.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.simpleEntity)
.expect(200);

await asAlice.post('/v1/projects/1/datasets/people/entities')
.send({
uuid: '12345678-1234-4123-8234-123456789abc', // collision
label: 'John Doe',
data: {
first_name: 'John',
age: '22'
}
});

await asAlice.delete('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc')
.expect(200);

await asAlice.get('/v1/projects/1/datasets/people/entities')
.then(({ body }) => {
body.length.should.equal(0);
});

await asAlice.post('/v1/projects/1/datasets/people/entities')
.set('User-Agent', 'central/tests')
.send({
source: {
name: 'people.csv',
size: 100,
},
entities: [
{
uuid: '12345678-1234-4123-8234-123456789abc', // collision
label: 'John Doe',
data: {
first_name: 'John',
age: '22'
}
},
{
uuid: '12345678-1234-4123-8234-111111111bbb',
label: 'Alice',
data: {
first_name: 'Alice',
age: '44'
}
},
]
})
.expect(409)
.then(({ body }) => {
body.code.should.equal(409.19);
body.message.should.equal('The following UUID(s) cannot be used because they are associated with deleted Entities: (12345678-1234-4123-8234-123456789abc).');
});

// Entity list is still same length as before
await asAlice.get('/v1/projects/1/datasets/people/entities')
.then(({ body }) => {
body.length.should.equal(0); // same as before
});

// Most recent event is not a bulk create event
await asAlice.get('/v1/audits')
.then(({ body }) => {
body[0].action.should.not.equal('entity.bulk.create');
});
}));
});

describe('entity source when created through bulk append', () => {
Expand Down
64 changes: 59 additions & 5 deletions test/integration/other/entities-purging.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,23 @@ const createEntities = async (user, count, projectId, datasetName) => {
return uuids;
};

const createBulkEntities = async (user, count, projectId, datasetName) => {
const entities = [];
const uuids = [];
for (let i = 0; i < count; i += 1) {
const _uuid = uuid();
entities.push({ uuid: _uuid, label: 'label' });
uuids.push(_uuid);
}
await user.post(`/v1/projects/${projectId}/datasets/${datasetName}/entities`)
.send({
entities,
source: { name: 'bulk.csv', size: count }
})
.expect(200);
return uuids;
};

const createEntitiesViaSubmissions = async (user, container, count) => {
const uuids = [];
for (let i = 0; i < count; i += 1) {
Expand Down Expand Up @@ -286,7 +303,22 @@ describe('query module entities purge', () => {
should(auditNotes).be.null();
}));

it('should purge entity sources', testService(async (service, container) => {
it('should purge API entity sources', testService(async (service, container) => {
const { Entities, oneFirst } = container;
const asAlice = await service.login('alice');

await createDeletedEntities(asAlice, 2);

let sourcesCount = await oneFirst(sql`select count(1) from entity_def_sources`);
sourcesCount.should.be.equal(2);

await Entities.purge(true);

sourcesCount = await oneFirst(sql`select count(1) from entity_def_sources`);
sourcesCount.should.be.equal(0);
}));

it('should purge submission entity sources', testService(async (service, container) => {
const { Entities, oneFirst } = container;
const asAlice = await service.login('alice');

Expand All @@ -308,6 +340,28 @@ describe('query module entities purge', () => {
sourcesCount.should.be.equal(0);
}));

it('should not purge bulk entity sources', testService(async (service, container) => {
const { Entities, oneFirst } = container;
const asAlice = await service.login('alice');

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

const uuids = await createBulkEntities(asAlice, 2, 1, 'people');

let sourcesCount = await oneFirst(sql`select count(1) from entity_def_sources`);
sourcesCount.should.be.equal(1);

await deleteEntities(asAlice, uuids.slice(0, 1), 1, 'people');

await Entities.purge(true);

sourcesCount = await oneFirst(sql`select count(1) from entity_def_sources`);
sourcesCount.should.be.equal(1);
}));

it('should purge submission backlog for entities', testService(async (service, container) => {
const { Entities, oneFirst } = container;
const asAlice = await service.login('alice');
Expand Down Expand Up @@ -374,7 +428,7 @@ describe('query module entities purge', () => {
})
.expect(409)
.then(({ body }) => {
body.details.values.should.eql(uuids[0]);
body.message.includes(uuids[0]).should.be.true();
});
}));

Expand Down Expand Up @@ -405,7 +459,7 @@ describe('query module entities purge', () => {
})
.expect(409)
.then(({ body }) => {
uuids.forEach(i => body.details.values.includes(i).should.be.true);
uuids.forEach(i => body.message.includes(i).should.be.true());
});
}));
});
Expand All @@ -425,7 +479,7 @@ describe('query module entities purge', () => {

await asAlice.get('/v1/audits')
.then(({ body }) => {
const purgeLogs = body.filter((a) => a.action === 'entities.purge');
const purgeLogs = body.filter((a) => a.action === 'entity.purge');
purgeLogs.length.should.equal(2);

const [ peoplePurgeAudit ] = purgeLogs.filter(a => a.acteeId === peopleActeeId);
Expand All @@ -445,7 +499,7 @@ describe('query module entities purge', () => {

await asAlice.get('/v1/audits')
.then(({ body }) => {
body.filter((a) => a.action === 'entities.purge').length.should.equal(0);
body.filter((a) => a.action === 'entity.purge').length.should.equal(0);
});
}));
});
Expand Down
Loading

0 comments on commit 1e35617

Please sign in to comment.