-
Notifications
You must be signed in to change notification settings - Fork 76
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
#710: Restore Entities, Purge Entities, ability to see deleted Entities #1349
base: master
Are you sure you want to change the base?
Changes from 1 commit
7b38616
90f985c
0713461
0f5d716
813fc25
5be8b33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// Copyright 2024 ODK Central Developers | ||
// See the NOTICE file at the top-level directory of this distribution and at | ||
// https://github.com/getodk/central-backend/blob/master/NOTICE. | ||
// This file is part of ODK Central. It is subject to the license terms in | ||
// the LICENSE file found in the top-level directory of this distribution and at | ||
// https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central, | ||
// including this file, may be copied, modified, propagated, or distributed | ||
// except according to the terms contained in the LICENSE file. | ||
|
||
const up = (db) => db.raw(` | ||
ALTER TABLE public.entity_defs DROP CONSTRAINT entity_defs_entityid_foreign; | ||
ALTER TABLE public.entity_defs ADD CONSTRAINT entity_defs_entityid_foreign FOREIGN KEY ("entityId") REFERENCES public.entities(id) ON DELETE CASCADE; | ||
`); | ||
|
||
const down = ((db) => db.raw(` | ||
ALTER TABLE public.entity_defs DROP CONSTRAINT entity_defs_entityid_foreign; | ||
ALTER TABLE public.entity_defs ADD CONSTRAINT entity_defs_entityid_foreign FOREIGN KEY ("entityId") REFERENCES public.entities(id); | ||
`)); | ||
|
||
module.exports = { up, down }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
// Copyright 2024 ODK Central Developers | ||
// See the NOTICE file at the top-level directory of this distribution and at | ||
// https://github.com/getodk/central-backend/blob/master/NOTICE. | ||
// This file is part of ODK Central. It is subject to the license terms in | ||
// the LICENSE file found in the top-level directory of this distribution and at | ||
// https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central, | ||
// including this file, may be copied, modified, propagated, or distributed | ||
// except according to the terms contained in the LICENSE file. | ||
|
||
const up = (db) => db.raw(` | ||
CREATE INDEX audits_details_entity_uuid ON public.audits USING hash ((details->'entity'->>'uuid')) | ||
WHERE ACTION IN ('entity.create', 'entity.update', 'entity.update.version', 'entity.update.resolve', 'entity.delete', 'entity.restore'); | ||
|
||
CREATE INDEX audits_details_entityUuids ON audits USING gin ((details -> 'entityUuids') jsonb_path_ops) | ||
WHERE ACTION = 'entities.purge'; | ||
`); | ||
|
||
const down = ((db) => db.raw(` | ||
DROP INDEX audits_details_entity_uuid; | ||
DROP INDEX audits_details_entityUuids; | ||
`)); | ||
|
||
module.exports = { up, down }; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// Copyright 2024 ODK Central Developers | ||
// See the NOTICE file at the top-level directory of this distribution and at | ||
// https://github.com/getodk/central-backend/blob/master/NOTICE. | ||
// This file is part of ODK Central. It is subject to the license terms in | ||
// the LICENSE file found in the top-level directory of this distribution and at | ||
// https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central, | ||
// including this file, may be copied, modified, propagated, or distributed | ||
// except according to the terms contained in the LICENSE file. | ||
|
||
const up = (db) => db.raw(` | ||
UPDATE audits SET details = ('{ "entity":{ "uuid":"' || (details->>'uuid') || '"}}')::JSONB | ||
WHERE action = 'entity.delete' AND details \\? 'uuid'; | ||
`); | ||
|
||
const down = () => {}; | ||
|
||
module.exports = { up, down }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,18 @@ const { isTrue } = require('../../util/http'); | |
const Problem = require('../../util/problem'); | ||
const { getOrReject, runSequentially } = require('../../util/promise'); | ||
|
||
///////////////////////////////////////////////////////////////////////////////// | ||
// Check if provided UUIDs (array) were used by purged entities. | ||
|
||
const _getPurgedUuids = (all, uuids) => all(sql` | ||
SELECT jsonb_array_elements_text(details -> 'entityUuids') AS uuid FROM audits a WHERE action = 'entities.purge' | ||
INTERSECT | ||
SELECT jsonb_array_elements_text(${JSON.stringify(uuids)})`) | ||
.then(all.map(r => r.uuid)); | ||
|
||
const _isPurgedUuid = (maybeOneFirst, uuid) => maybeOneFirst(sql` | ||
SELECT TRUE FROM audits WHERE action = 'entities.purge' AND details -> 'entityUuids' @> ${JSON.stringify([uuid])} | ||
`); | ||
|
||
///////////////////////////////////////////////////////////////////////////////// | ||
// ENTITY DEF SOURCES | ||
|
@@ -65,10 +77,16 @@ 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) => ({ one, context }) => { | ||
const createNew = (dataset, partial, subDef, sourceId, userAgentIn) => async ({ one, context, maybeOneFirst }) => { | ||
let creatorId; | ||
let userAgent; | ||
|
||
// Validate UUID against purged entities | ||
(await _isPurgedUuid(maybeOneFirst, partial.uuid)) | ||
.ifDefined(() => { | ||
throw Problem.user.uniquenessViolation({ fields: 'uuid', values: partial.uuid }); | ||
}); | ||
|
||
// Set creatorId and userAgent from submission def if it exists | ||
if (subDef != null) { | ||
({ submitterId: creatorId, userAgent } = subDef); | ||
|
@@ -111,6 +129,12 @@ const createMany = (dataset, rawEntities, sourceId, userAgentIn) => async ({ all | |
const creatorId = context.auth.actor.map((actor) => actor.id).orNull(); | ||
const userAgent = blankStringToNull(userAgentIn); | ||
|
||
// Validate UUID with the purged entities | ||
const purgedUuids = await _getPurgedUuids(all, rawEntities.map(e => e.uuid)); | ||
if (purgedUuids.length > 0) { | ||
throw Problem.user.uniquenessViolation({ fields: 'uuid', values: purgedUuids.join(', ') }); | ||
} | ||
|
||
// Augment parsed entity data with dataset and creator IDs | ||
const entitiesForInsert = rawEntities.map(e => new Entity({ datasetId: dataset.id, creatorId, ...e })); | ||
const entities = await all(sql`${insertMany(entitiesForInsert)} RETURNING id`); | ||
|
@@ -849,15 +873,94 @@ CROSS JOIN | |
const del = (entity) => ({ run }) => | ||
run(markDeleted(entity)); | ||
|
||
del.audit = (entity, dataset) => (log) => log('entity.delete', entity.with({ acteeId: dataset.acteeId }), { uuid: entity.uuid }); | ||
del.audit = (entity, dataset) => (log) => | ||
log( | ||
'entity.delete', | ||
entity.with({ acteeId: dataset.acteeId }), | ||
{ entity: { uuid: entity.uuid } } | ||
); | ||
Comment on lines
+877
to
+881
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made it consistent with other entity logs where uuid is under the entity object |
||
|
||
//////////////////////////////////////////////////////////////////////////////// | ||
// RESTORE ENTITY | ||
|
||
const restore = (entity) => ({ run }) => | ||
run(markUndeleted(entity)); | ||
|
||
restore.audit = (entity, dataset) => (log) => log('entity.restore', entity.with({ acteeId: dataset.acteeId }), { uuid: entity.uuid }); | ||
restore.audit = (entity, dataset) => (log) => | ||
log( | ||
'entity.restore', | ||
entity.with({ acteeId: dataset.acteeId }), | ||
{ entity: { uuid: entity.uuid } } | ||
); | ||
|
||
//////////////////////////////////////////////////////////////////////////////// | ||
// PURGE DELETED ENTITIES | ||
|
||
const PURGE_DAY_RANGE = config.has('default.taskSchedule.purge') | ||
? config.get('default.taskSchedule.purge') | ||
: 30; // Default is 30 days | ||
Comment on lines
+899
to
+901
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should move this to central place, this is present in three file at the moment. |
||
|
||
const _trashedFilter = (force, projectId, datasetName, entityUuid) => { | ||
const idFilters = [sql`TRUE`]; | ||
if (projectId) idFilters.push(sql`datasets."projectId" = ${projectId}`); | ||
if (datasetName) idFilters.push(sql`datasets."name" = ${datasetName}`); | ||
if (entityUuid) idFilters.push(sql`entities."uuid" = ${entityUuid}`); | ||
|
||
const idFilter = sql.join(idFilters, sql` AND `); | ||
|
||
return (force | ||
? sql`entities."deletedAt" IS NOT NULL AND ${idFilter}` | ||
: sql`entities."deletedAt" < CURRENT_DATE - CAST(${PURGE_DAY_RANGE} AS INT) AND ${idFilter}`); | ||
}; | ||
|
||
const purge = (force = false, projectId = null, datasetName = null, entityUuid = null) => ({ oneFirst }) => { | ||
if (entityUuid && (!projectId || !datasetName)) { | ||
throw Problem.internal.unknown({ error: 'Must specify projectId and datasetName to purge a specify entity.' }); | ||
} | ||
if (datasetName && !projectId) { | ||
throw Problem.internal.unknown({ error: 'Must specify projectId to purge all entities of a dataset/entity-list.' }); | ||
} | ||
|
||
// Reminder: 'notes' in audit table is set by 'x-action-notes' header of all | ||
// APIs with side-effects. Although we don't provide any way to set it from | ||
// the frontend (as of 2024.3), it is a good idea to clear it for purged | ||
// entities. | ||
return oneFirst(sql` | ||
WITH redacted_audits AS ( | ||
UPDATE audits SET notes = NULL | ||
FROM entities | ||
JOIN datasets ON entities."datasetId" = datasets.id | ||
WHERE (audits.details->'entity'->>'uuid') = entities.uuid | ||
AND ${_trashedFilter(force, projectId, datasetName, entityUuid)} | ||
), purge_audit AS ( | ||
INSERT INTO audits ("action", "loggedAt", "processed", "details") | ||
sadiqkhoja marked this conversation as resolved.
Show resolved
Hide resolved
|
||
SELECT 'entities.purge', clock_timestamp(), clock_timestamp(), jsonb_build_object('entitiesDeleted', COUNT(*), 'entityUuids', ARRAY_AGG(uuid)) | ||
FROM entities | ||
JOIN datasets ON entities."datasetId" = datasets.id | ||
WHERE ${_trashedFilter(force, projectId, datasetName, entityUuid)} | ||
HAVING count(*) > 0 | ||
), delete_entity_sources AS ( | ||
DELETE FROM entity_def_sources | ||
USING entity_defs, entities, datasets | ||
WHERE entity_def_sources.id = entity_defs."sourceId" | ||
AND entity_defs."entityId" = entities.id | ||
AND entities."datasetId" = datasets.id | ||
AND ${_trashedFilter(force, projectId, datasetName, entityUuid)} | ||
), delete_submission_backlog AS ( | ||
DELETE FROM entity_submission_backlog | ||
USING entities, datasets | ||
WHERE entity_submission_backlog."entityUuid"::varchar = entities.uuid | ||
AND entities."datasetId" = datasets.id | ||
AND ${_trashedFilter(force, projectId, datasetName, entityUuid)} | ||
), deleted_entities AS ( | ||
DELETE FROM entities | ||
USING datasets | ||
WHERE entities."datasetId" = datasets.id | ||
AND ${_trashedFilter(force, projectId, datasetName, entityUuid)} | ||
RETURNING 1 | ||
) | ||
SELECT COUNT(*) FROM deleted_entities`); | ||
}; | ||
|
||
module.exports = { | ||
createNew, _processSubmissionEvent, | ||
|
@@ -875,5 +978,5 @@ module.exports = { | |
countByDatasetId, getById, getDef, | ||
getAll, getAllDefs, del, | ||
createEntitiesFromPendingSubmissions, | ||
resolveConflict, restore | ||
resolveConflict, restore, purge | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can pass a single uuid in
_getPurgedUuuids
but my gut feeling is that_isPurgedUuid
would be faster. 🤷♂️There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
verified that
_isPurgedUuid
is using the index and is much quicker.