From 547c40607462092f03073877375560d14798e64a Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Tue, 10 Sep 2024 14:58:50 -0700 Subject: [PATCH] refactoring purge task and cli --- lib/bin/{purge-forms.js => purge.js} | 24 +++- lib/model/query/forms.js | 6 +- lib/model/query/submissions.js | 25 ++-- lib/task/purge.js | 13 +- test/integration/other/blobs.js | 1 + test/integration/other/form-purging.js | 115 +++++++++++++++--- test/integration/other/submission-purging.js | 8 +- test/integration/task/purge.js | 120 +++++-------------- 8 files changed, 177 insertions(+), 135 deletions(-) rename lib/bin/{purge-forms.js => purge.js} (51%) diff --git a/lib/bin/purge-forms.js b/lib/bin/purge.js similarity index 51% rename from lib/bin/purge-forms.js rename to lib/bin/purge.js index cc5561d3e..a9e3bd739 100644 --- a/lib/bin/purge-forms.js +++ b/lib/bin/purge.js @@ -7,20 +7,32 @@ // including this file, may be copied, modified, propagated, or distributed // except according to the terms contained in the LICENSE file. // -// This script checks for (soft-)deleted forms and purges any that were deleted -// over 30 days ago. +// This script checks for (soft-)deleted forms and submissions and purges +// any that were deleted over 30 days ago. +// +// It also accepts command line arguments that can force the purging of +// forms and submissions that were deleted less than 30 days ago. +// +// It can also be used to purge a specific form or submission +// (that has already been marked deleted). const { run } = require('../task/task'); -const { purgeForms } = require('../task/purge'); +const { purgeForms, purgeSubmissions, purgeUnattachedBlobs } = require('../task/purge'); const { program } = require('commander'); program.option('-f, --force', 'Force any soft-deleted form to be purged right away.'); program.option('-i, --formId ', 'Purge a specific form based on its id.', parseInt); program.option('-p, --projectId ', 'Restrict purging to a specific project.', parseInt); -program.option('-x, --xmlFormId ', 'Restrict purging to specific form based on xmlFormId (must be used with project id).'); +program.option('-x, --xmlFormId ', 'Restrict purging to specific form based on xmlFormId (must be used with projectId).'); +program.option('-s, --instanceId ', 'Restrict purging to a specific submission based on instanceId (use with projectId and xmlFormId).'); + program.parse(); const options = program.opts(); -run(purgeForms(options.force, options.formId, options.projectId, options.xmlFormId) - .then((count) => `Forms purged: ${count}`)); +const purgeTasks = options.instanceId + ? purgeSubmissions(options.force, options.projectId, options.xmlFormId, options.submissionId) + : purgeForms(options.force, options.formId, options.projectId, options.xmlFormId); + +run(purgeTasks + .then((message) => purgeUnattachedBlobs().then(() => message))); diff --git a/lib/model/query/forms.js b/lib/model/query/forms.js index 4fa7bc769..c609e4b7f 100644 --- a/lib/model/query/forms.js +++ b/lib/model/query/forms.js @@ -406,7 +406,7 @@ const _trashedFilter = (force, id, projectId, xmlFormId) => { // 3. Update actees table for the specific form to leave some useful information behind // 4. Delete the forms and their resources from the database // 5. Purge unattached blobs -const purge = (force = false, id = null, projectId = null, xmlFormId = null) => ({ oneFirst, Blobs }) => { +const purge = (force = false, id = null, projectId = null, xmlFormId = null) => ({ oneFirst }) => { if (xmlFormId != null && projectId == null) throw Problem.internal.unknown({ error: 'Must also specify projectId when using xmlFormId' }); return oneFirst(sql` @@ -446,9 +446,7 @@ with redacted_audits as ( where ${_trashedFilter(force, id, projectId, xmlFormId)} returning 1 ) -select count(*) from deleted_forms`) - .then((count) => Blobs.purgeUnattached() - .then(() => Promise.resolve(count))); +select count(*) from deleted_forms`); }; //////////////////////////////////////////////////////////////////////////////// diff --git a/lib/model/query/submissions.js b/lib/model/query/submissions.js index 70d9c3a43..fc366a58d 100644 --- a/lib/model/query/submissions.js +++ b/lib/model/query/submissions.js @@ -400,23 +400,24 @@ const DAY_RANGE = config.has('default.taskSchedule.purge') ? config.get('default.taskSchedule.purge') : 30; // Default is 30 days -const _trashedFilter = (force, id) => { - const idFilter = (id - ? sql`and submissions.id = ${id}` +const _trashedFilter = (force, projectId, xmlFormId, instanceId) => { + const idFilter = ((instanceId != null) && (projectId != null) && (xmlFormId != null) + ? sql`and submissions."instanceId" = ${instanceId} + and forms."projectId" = ${projectId} + and forms."xmlFormId" = ${xmlFormId}` : sql``); return (force ? sql`submissions."deletedAt" is not null ${idFilter}` : sql`submissions."deletedAt" < current_date - cast(${DAY_RANGE} as int) ${idFilter}`); }; -// eslint-disable-next-line no-unused-vars -const purge = (force = false, id = null) => ({ oneFirst, Blobs }) => +const purge = (force = false, projectId = null, xmlFormId = null, instanceId = null) => ({ oneFirst }) => oneFirst(sql` with redacted_audits as ( update audits set notes = '' from submissions where (audits.details->>'submissionId')::int = submissions.id - and ${_trashedFilter(force, id)} + and ${_trashedFilter(force, projectId, xmlFormId, instanceId)} ), deleted_client_audits as ( delete from client_audits using submission_attachments, submission_defs, submissions @@ -424,18 +425,18 @@ with redacted_audits as ( and submission_attachments."submissionDefId" = submission_defs.id and submission_attachments."isClientAudit" = true and submission_defs."submissionId" = submissions.id - and ${_trashedFilter(force, id)} + and ${_trashedFilter(force, projectId, xmlFormId, instanceId)} ), purge_audits as ( insert into audits ("action", "loggedAt", "processed") values ('submission.purge', clock_timestamp(), clock_timestamp()) ), deleted_submissions as ( delete from submissions - where ${_trashedFilter(force, id)} - returning * + using forms + where submissions."formId" = forms.id + and ${_trashedFilter(force, projectId, xmlFormId, instanceId)} + returning submissions.* ) -select count(*) from deleted_submissions`) - .then((count) => Blobs.purgeUnattached() - .then(() => Promise.resolve(count))); +select count(*) from deleted_submissions`); module.exports = { createNew, createVersion, diff --git a/lib/task/purge.js b/lib/task/purge.js index 577ad9ff1..5def9fe5a 100644 --- a/lib/task/purge.js +++ b/lib/task/purge.js @@ -10,9 +10,14 @@ const { task } = require('./task'); const purgeForms = task.withContainer(({ Forms }) => (force = false, formId = null, projectId = null, xmlFormId = null) => - Forms.purge(force, formId, projectId, xmlFormId)); + Forms.purge(force, formId, projectId, xmlFormId) + .then((count) => `Forms purged: ${count}`)); -const purgeSubmissions = task.withContainer(({ Submissions }) => (force = false, submissionId = null) => - Submissions.purge(force, submissionId)); +const purgeSubmissions = task.withContainer(({ Submissions }) => async (force = false, projectId = null, xmlFormId = null, submissionId = null) => { + const count = await Submissions.purge(force, projectId, xmlFormId, submissionId); + return `Submissions purged: ${count}`; +}); -module.exports = { purgeForms, purgeSubmissions }; +const purgeUnattachedBlobs = task.withContainer(({ Blobs }) => () => Blobs.purgeUnattached()); + +module.exports = { purgeForms, purgeSubmissions, purgeUnattachedBlobs }; diff --git a/test/integration/other/blobs.js b/test/integration/other/blobs.js index 26ffb61d8..e17e1695f 100644 --- a/test/integration/other/blobs.js +++ b/test/integration/other/blobs.js @@ -43,6 +43,7 @@ describe('blob query module', () => { .expect(201)) .then(() => asAlice.delete('/v1/projects/1/forms/binaryType')) .then(() => container.Forms.purge(true)) + .then(() => container.Blobs.purgeUnattached()) .then(() => container.oneFirst(sql`select count(*) from blobs`)) .then((count) => count.should.equal(1))))); // }); diff --git a/test/integration/other/form-purging.js b/test/integration/other/form-purging.js index 2904a6605..cbfb63e4f 100644 --- a/test/integration/other/form-purging.js +++ b/test/integration/other/form-purging.js @@ -1,6 +1,7 @@ const { createReadStream, readFileSync } = require('fs'); const appPath = require('app-root-path'); const { sql } = require('slonik'); +const assert = require('assert'); const { testService } = require('../setup'); const testData = require('../../data/xml'); const { exhaust } = require(appPath + '/lib/worker/worker'); @@ -59,25 +60,6 @@ describe('query module form purge', () => { counts.should.eql([ 0, 0 ]); }))))); - it('should purge a deleted form by ID', testService((service, container) => - service.login('alice', (asAlice) => - asAlice.delete('/v1/projects/1/forms/simple') - .expect(200) - .then(() => asAlice.post('/v1/projects/1/forms') - .send(testData.forms.withAttachments) - .set('Content-Type', 'application/xml') - .expect(200)) - .then(() => container.Forms.getByProjectAndXmlFormId(1, 'withAttachments').then((o) => o.get())) - .then((ghostForm) => asAlice.delete('/v1/projects/1/withAttachments') - .then(() => container.Forms.purge(true, 1)) // force delete a single form - .then(() => Promise.all([ - container.oneFirst(sql`select count(*) from forms where id = ${ghostForm.id}`), - container.oneFirst(sql`select count(*) from forms where id = 1`), // deleted form id - ]) - .then((counts) => { - counts.should.eql([ 1, 0 ]); - })))))); - it('should log the purge action in the audit log', testService((service, container) => service.login('alice', (asAlice) => container.Forms.getByProjectAndXmlFormId(1, 'simple').then((o) => o.get()) // get the form before we delete it @@ -142,6 +124,7 @@ describe('query module form purge', () => { .then((ghostForm) => asAlice.delete('/v1/projects/1/forms/withAttachments') .expect(200) .then(() => container.Forms.purge(true)) + .then(() => container.Blobs.purgeUnattached()) .then(() => Promise.all([ container.oneFirst(sql`select count(*) from forms where id = ${ghostForm.id}`), container.oneFirst(sql`select count(*) from form_defs where "formId" = ${ghostForm.id}`), @@ -184,6 +167,97 @@ describe('query module form purge', () => { .then(() => container.oneFirst(sql`select count(*) from form_field_values`)) .then((count) => count.should.eql(0))))); + describe('purging specific forms via specific arguments', () => { + it('should purge a deleted form by ID', testService((service, container) => + service.login('alice', (asAlice) => + asAlice.delete('/v1/projects/1/forms/simple') + .expect(200) + .then(() => asAlice.post('/v1/projects/1/forms') + .send(testData.forms.withAttachments) + .set('Content-Type', 'application/xml') + .expect(200)) + .then(() => container.Forms.getByProjectAndXmlFormId(1, 'withAttachments').then((o) => o.get())) + .then((ghostForm) => asAlice.delete('/v1/projects/1/withAttachments') + .then(() => container.Forms.purge(true, 1)) // force delete a single form + .then(() => Promise.all([ + container.oneFirst(sql`select count(*) from forms where id = ${ghostForm.id}`), + container.oneFirst(sql`select count(*) from forms where id = 1`), // deleted form id + ]) + .then((counts) => { + counts.should.eql([ 1, 0 ]); + })))))); + + it('should purge all versions of deleted form in project', testService(async (service, container) => { + const asAlice = await service.login('alice'); + + await asAlice.delete('/v1/projects/1/forms/simple') + .expect(200); + + // new version (will be v2) + await asAlice.post('/v1/projects/1/forms?ignoreWarnings=true') + .send(testData.forms.simple) + .set('Content-Type', 'application/xml') + .expect(200); + + // publish new version v2 + await asAlice.post('/v1/projects/1/forms/simple/draft/publish?ignoreWarnings=true&version=v2') + .expect(200); + + // delete new version v2 + await asAlice.delete('/v1/projects/1/forms/simple') + .expect(200); + + // new version (will be v3) + await asAlice.post('/v1/projects/1/forms?ignoreWarnings=true') + .send(testData.forms.simple) + .set('Content-Type', 'application/xml') + .expect(200); + + // publish new version v3 but don't delete + await asAlice.post('/v1/projects/1/forms/simple/draft/publish?ignoreWarnings=true&version=v3') + .expect(200); + + const count = await container.Forms.purge(true, null, 1, 'simple'); + count.should.equal(2); + })); + + it('should purge named form only from specified project', testService(async (service, container) => { + const asAlice = await service.login('alice'); + + // delete simple form in project 1 (but don't purge it) + await asAlice.delete('/v1/projects/1/forms/simple') + .expect(200); + + const newProjectId = await asAlice.post('/v1/projects') + .send({ name: 'Project Two' }) + .then(({ body }) => body.id); + + await asAlice.post(`/v1/projects/${newProjectId}/forms?publish=true`) + .send(testData.forms.simple) + .set('Content-Type', 'application/xml') + .expect(200); + + await asAlice.delete(`/v1/projects/${newProjectId}/forms/simple`) + .expect(200); + + const count = await container.Forms.purge(true, null, newProjectId, 'simple'); + count.should.equal(1); + })); + + it('should throw an error when xmlFormId specified without project ID', testService(async (service, container) => { + const asAlice = await service.login('alice'); + + await asAlice.delete('/v1/projects/1/forms/simple') + .expect(200); + + await assert.throws(() => { container.Forms.purge(true, null, null, 'simple'); }, (err) => { + err.problemCode.should.equal(500.1); + err.problemDetails.error.should.equal('Must also specify projectId when using xmlFormId'); + return true; + }); + })); + }); + describe('purging form submissions', () => { const withSimpleIds = (deprecatedId, instanceId) => testData.instances.simple.one .replace('one${deprecatedId} { })) .then(() => asAlice.delete('/v1/projects/1/forms/binaryType')) .then(() => container.Forms.purge(true)) + .then(() => container.Blobs.purgeUnattached()) .then(() => container.oneFirst(sql`select count(*) from submission_attachments`) .then((count) => count.should.equal(0))) .then(() => container.oneFirst(sql`select count(*) from blobs`) @@ -280,6 +355,7 @@ describe('query module form purge', () => { .then(() => exhaust(container)) .then(() => asAlice.delete('/v1/projects/1/forms/audits')) .then(() => container.Forms.purge(true)) + .then(() => container.Blobs.purgeUnattached()) .then(() => Promise.all([ container.oneFirst(sql`select count(*) from client_audits`), container.oneFirst(sql`select count(*) from blobs`) @@ -294,6 +370,7 @@ describe('query module form purge', () => { .then(() => asAlice.delete('/v1/projects/1/forms/simple2') // Delete form .expect(200)) .then(() => container.Forms.purge(true)) + .then(() => container.Blobs.purgeUnattached()) .then(() => container.oneFirst(sql`select count(*) from blobs`)) .then((count) => count.should.equal(0))))); }); diff --git a/test/integration/other/submission-purging.js b/test/integration/other/submission-purging.js index d7abdc1d6..afede378c 100644 --- a/test/integration/other/submission-purging.js +++ b/test/integration/other/submission-purging.js @@ -104,7 +104,7 @@ describe('query module submission purge', () => { attachments.should.equal(0); })); - it('should purge blobs associated with attachments when purging submission', testService(async (service, { Submissions, oneFirst }) => { + it('should purge blobs associated with attachments when purging submission', testService(async (service, { Blobs, Submissions, oneFirst }) => { const asAlice = await service.login('alice'); await asAlice.post('/v1/projects/1/forms?publish=true') @@ -134,6 +134,9 @@ describe('query module submission purge', () => { await asAlice.delete('/v1/projects/1/forms/binaryType/submissions/both'); await Submissions.purge(true); + // Purge unattached blobs + await Blobs.purgeUnattached(); + // One blob still remains from first submission which was not deleted blobCount = await oneFirst(sql`select count(*) from blobs`); blobCount.should.equal(1); @@ -372,6 +375,9 @@ describe('query module submission purge', () => { // Purge the submission await container.Submissions.purge(true); + // Purge unattached blobs + await container.Blobs.purgeUnattached(); + // Check that some of the client audit events are deleted from the database const numClientAudits = await container.oneFirst(sql`select count(*) from client_audits`); numClientAudits.should.equal(3); // from the non-deleted submission diff --git a/test/integration/task/purge.js b/test/integration/task/purge.js index 66e387d4e..cf1783eee 100644 --- a/test/integration/task/purge.js +++ b/test/integration/task/purge.js @@ -1,10 +1,9 @@ const appRoot = require('app-root-path'); const assert = require('assert'); -const { testTask, testService } = require('../setup'); +const { testTask } = require('../setup'); const { purgeForms } = require(appRoot + '/lib/task/purge'); -const testData = require('../../data/xml'); -// The basics of this task are tested here, including returning the count +// The basics of this task are tested here, including returning the message // of purged forms, but the full functionality is more thoroughly tested in // test/integration/other/form-purging.js @@ -13,42 +12,42 @@ describe('task: purge deleted forms', () => { Forms.getByProjectAndXmlFormId(1, 'simple') .then((form) => Forms.del(form.get()) .then(() => purgeForms()) - .then((count) => { - count.should.equal(0); + .then((message) => { + message.should.equal('Forms purged: 0'); })))); it('should purge recently deleted form if forced', testTask(({ Forms }) => Forms.getByProjectAndXmlFormId(1, 'simple') .then((form) => Forms.del(form.get()) .then(() => purgeForms(true)) - .then((count) => { - count.should.equal(1); + .then((message) => { + message.should.equal('Forms purged: 1'); })))); - it('should return count for multiple forms purged', testTask(({ Forms }) => + it('should return message for multiple forms purged', testTask(({ Forms }) => Forms.getByProjectAndXmlFormId(1, 'simple') .then((form) => Forms.del(form.get())) .then(() => Forms.getByProjectAndXmlFormId(1, 'withrepeat') .then((form) => Forms.del(form.get()))) .then(() => purgeForms(true) - .then((count) => { - count.should.equal(2); + .then((message) => { + message.should.equal('Forms purged: 2'); })))); it('should not purge specific recently deleted form', testTask(({ Forms }) => Forms.getByProjectAndXmlFormId(1, 'simple') .then((form) => Forms.del(form.get()) .then(() => purgeForms(false, 1)) - .then((count) => { - count.should.equal(0); + .then((message) => { + message.should.equal('Forms purged: 0'); })))); it('should purge specific recently deleted form if forced', testTask(({ Forms }) => Forms.getByProjectAndXmlFormId(1, 'simple') .then((form) => Forms.del(form.get()) .then(() => purgeForms(true, 1)) - .then((count) => { - count.should.equal(1); + .then((message) => { + message.should.equal('Forms purged: 1'); })))); it('should force purge only specific form', testTask(({ Forms }) => @@ -57,8 +56,8 @@ describe('task: purge deleted forms', () => { .then(() => Forms.getByProjectAndXmlFormId(1, 'withrepeat') .then((form) => Forms.del(form.get()) .then(() => purgeForms(true, 1)) - .then((count) => { - count.should.equal(1); + .then((message) => { + message.should.equal('Forms purged: 1'); }))))); describe('with projectId', () => { @@ -66,16 +65,16 @@ describe('task: purge deleted forms', () => { Forms.getByProjectAndXmlFormId(1, 'simple') .then((form) => Forms.del(form.get()) .then(() => purgeForms(null, null, 1)) - .then((count) => { - count.should.equal(0); + .then((message) => { + message.should.equal('Forms purged: 0'); })))); it('should not purge recently deleted forms even if projectId AND formId is matched', testTask(({ Forms }) => Forms.getByProjectAndXmlFormId(1, 'simple') .then((form) => Forms.del(form.get()) .then(() => purgeForms(null, 1, 1)) - .then((count) => { - count.should.equal(0); + .then((message) => { + message.should.equal('Forms purged: 0'); })))); it('should purge specific form', testTask(({ Forms }) => @@ -84,8 +83,8 @@ describe('task: purge deleted forms', () => { .then(() => Forms.getByProjectAndXmlFormId(1, 'withrepeat') .then((form) => Forms.del(form.get()) .then(() => purgeForms(true, 1, 1)) - .then((count) => { - count.should.equal(1); + .then((message) => { + message.should.equal('Forms purged: 1'); }))))); it('should not purge specific form if tied to a different project', testTask(({ Forms }) => @@ -94,8 +93,8 @@ describe('task: purge deleted forms', () => { .then(() => Forms.getByProjectAndXmlFormId(1, 'withrepeat') .then((form) => Forms.del(form.get()) .then(() => purgeForms(true, 1, 2)) - .then((count) => { - count.should.equal(0); + .then((message) => { + message.should.equal('Forms purged: 0'); }))))); it('should purge all forms if no form ID supplied', testTask(({ Forms }) => @@ -104,8 +103,8 @@ describe('task: purge deleted forms', () => { .then(() => Forms.getByProjectAndXmlFormId(1, 'withrepeat') .then((form) => Forms.del(form.get()) .then(() => purgeForms(true, null, 1)) - .then((count) => { - count.should.equal(2); + .then((message) => { + message.should.equal('Forms purged: 2'); }))))); it('should not purge multiple forms if tied to a different project', testTask(({ Forms }) => @@ -114,8 +113,8 @@ describe('task: purge deleted forms', () => { .then(() => Forms.getByProjectAndXmlFormId(1, 'withrepeat') .then((form) => Forms.del(form.get()) .then(() => purgeForms(true, null, 2)) - .then((count) => { - count.should.equal(0); + .then((message) => { + message.should.equal('Forms purged: 0'); }))))); }); @@ -134,73 +133,16 @@ describe('task: purge deleted forms', () => { Forms.getByProjectAndXmlFormId(1, 'simple') .then((form) => Forms.del(form.get()) .then(() => purgeForms(true, null, 1, 'simple')) - .then((count) => { - count.should.equal(1); + .then((message) => { + message.should.equal('Forms purged: 1'); })))); it('should not purge form by project and xmlFormId if form deleted recently and not forced', testTask(({ Forms }) => Forms.getByProjectAndXmlFormId(1, 'simple') .then((form) => Forms.del(form.get()) .then(() => purgeForms(false, null, 1, 'simple')) - .then((count) => { - count.should.equal(0); + .then((message) => { + message.should.equal('Forms purged: 0'); })))); - - it('should purge all versions of deleted form in project', testService(async (service, container) => { - const asAlice = await service.login('alice'); - - await asAlice.delete('/v1/projects/1/forms/simple') - .expect(200); - - // new version (will be v2) - await asAlice.post('/v1/projects/1/forms?ignoreWarnings=true') - .send(testData.forms.simple) - .set('Content-Type', 'application/xml') - .expect(200); - - // publish new version v2 - await asAlice.post('/v1/projects/1/forms/simple/draft/publish?ignoreWarnings=true&version=v2') - .expect(200); - - // delete new version v2 - await asAlice.delete('/v1/projects/1/forms/simple') - .expect(200); - - // new version (will be v3) - await asAlice.post('/v1/projects/1/forms?ignoreWarnings=true') - .send(testData.forms.simple) - .set('Content-Type', 'application/xml') - .expect(200); - - // publish new version v3 but don't delete - await asAlice.post('/v1/projects/1/forms/simple/draft/publish?ignoreWarnings=true&version=v3') - .expect(200); - - const count = await container.Forms.purge(true, null, 1, 'simple'); - count.should.equal(2); - })); - - it('should purge named form only from specified project', testService(async (service, container) => { - const asAlice = await service.login('alice'); - - // delete simple form in project 1 (but don't purge it) - await asAlice.delete('/v1/projects/1/forms/simple') - .expect(200); - - const newProjectId = await asAlice.post('/v1/projects') - .send({ name: 'Project Two' }) - .then(({ body }) => body.id); - - await asAlice.post(`/v1/projects/${newProjectId}/forms?publish=true`) - .send(testData.forms.simple) - .set('Content-Type', 'application/xml') - .expect(200); - - await asAlice.delete(`/v1/projects/${newProjectId}/forms/simple`) - .expect(200); - - const count = await container.Forms.purge(true, null, newProjectId, 'simple'); - count.should.equal(1); - })); }); });