Skip to content

Commit

Permalink
feat: GEO-1007 Schedule to delete announcements (#820)
Browse files Browse the repository at this point in the history
  • Loading branch information
jer3k authored Oct 24, 2024
1 parent cb81bbd commit 161de73
Show file tree
Hide file tree
Showing 17 changed files with 480 additions and 18 deletions.
5 changes: 1 addition & 4 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
# Matched against repo root (asterisk)
* @sukanya-rath

# Matched against directories
# /.github/workflows/ @sukanya-rath
* @bcgov/FIN_IMB_AMD

# See https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners
17 changes: 17 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,23 @@
"order": 1
}
},
{
// Start clamav server which depends on clamav also running in podman.
// clamav requires quite a bit of memory and therefore isn't included with the rest by detault.
"name": "ClamAV Server",
"request": "launch",
"runtimeArgs": ["run-script", "dev"],
"runtimeExecutable": "npm",
"skipFiles": ["<node_internals>/**"],
"type": "node",
"cwd": "${workspaceFolder}/clamav-service",
"preLaunchTask": "Launch ClamAV",
"outputCapture": "std",
"presentation": {
"group": "Individual Servers",
"order": 1
}
},
{
"name": "Admin-Frontend Server",
"request": "launch",
Expand Down
11 changes: 10 additions & 1 deletion .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
"command": "podman-compose up -d database-migrations",
"problemMatcher": []
},
{
// This creates a new temporary container in podman to run clamav.
"label": "Launch ClamAV",
"type": "shell",
"command": "podman run --rm -d -p 3310:3310 clamav/clamav",
"problemMatcher": []
},
{
// After making changes to the database via Flyway, Run this task to
// bring all those changes into Prisma to be available in the code.
Expand All @@ -18,7 +25,9 @@
"options": {
"cwd": "${workspaceFolder}/backend"
},
"dependsOn": ["Launch and migrate database"],
"dependsOn": [
"Launch and migrate database"
],
"problemMatcher": []
},
{
Expand Down
1 change: 1 addition & 0 deletions backend/src/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ config.defaults({
schedulerTimeZone: process.env.REPORTS_SCHEDULER_CRON_TIMEZONE,
schedulerExpireAnnountmentsCronTime:
process.env.EXPIRE_ANNOUNCEMENTS_CRON_CRONTIME,
deleteAnnouncementsCronTime: process.env.DELETE_ANNOUNCEMENTS_CRON_CRONTIME,
enableEmailExpiringAnnouncements:
process.env.ENABLE_EMAIL_EXPIRING_ANNOUNCEMENTS?.toUpperCase() == 'TRUE',
deleteAnnouncementsDurationInDays: parseInt(
Expand Down
50 changes: 48 additions & 2 deletions backend/src/external/services/s3-api.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { faker } from '@faker-js/faker';
import { downloadFile } from './s3-api';
import { GetObjectCommand, ListObjectsCommand } from '@aws-sdk/client-s3';
import { downloadFile, deleteFiles } from './s3-api';
import {
DeleteObjectsCommand,
GetObjectCommand,
ListObjectsCommand,
} from '@aws-sdk/client-s3';
import { APP_ANNOUNCEMENTS_FOLDER } from '../../constants/admin';

const mockFindFirstOrThrow = jest.fn();
jest.mock('../../v1/prisma/prisma-client', () => ({
Expand Down Expand Up @@ -143,4 +148,45 @@ describe('S3Api', () => {
expect(res.status).toHaveBeenCalledWith(400);
});
});
describe('deleteFiles', () => {
it('should handle multiple ids, ids that dont exist, and ids that have already been deleted', async () => {
const ids = ['id1', 'id2', 'id3', 'id4'];
const fileId1a = { Key: `${APP_ANNOUNCEMENTS_FOLDER}/id1/file1a` };
const fileId1b = { Key: `${APP_ANNOUNCEMENTS_FOLDER}/id1/file1b` };
const fileId1c = { Key: `${APP_ANNOUNCEMENTS_FOLDER}/id1/file1c` };
const fileId2 = {
Key: `${APP_ANNOUNCEMENTS_FOLDER}/id2/file2`,
Code: 'NoSuchKey',
};
const fileId3 = {
Key: `${APP_ANNOUNCEMENTS_FOLDER}/id3/file3`,
Code: 'OtherError',
};

mockSend.mockImplementation((...args) => {
const [command] = args;
if (command instanceof ListObjectsCommand) {
// test: multiple files in id1. id4 doesn't exist
if (command.input.Prefix == `${APP_ANNOUNCEMENTS_FOLDER}/id1`)
return { Contents: [fileId1a, fileId1b, fileId1c] };
if (command.input.Prefix == `${APP_ANNOUNCEMENTS_FOLDER}/id2`)
return { Contents: [fileId2] };
if (command.input.Prefix == `${APP_ANNOUNCEMENTS_FOLDER}/id3`)
return { Contents: [fileId3] };
return {};
}
if (command instanceof DeleteObjectsCommand) {
return {
Deleted: [fileId1a, fileId1b, fileId1c],
Errors: [fileId2, fileId3],
};
}
});

const result = await deleteFiles(ids);

//id3 had a file that failed to delete, so it shouldn't say that id3 was deleted
expect(result).toEqual(new Set(['id1', 'id2', 'id4']));
});
});
});
90 changes: 83 additions & 7 deletions backend/src/external/services/s3-api.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import {
GetObjectCommand,
ListObjectsCommand,
DeleteObjectsCommand,
S3Client,
type ObjectIdentifier,
type _Object,
type _Error,
} from '@aws-sdk/client-s3';
import prisma from '../../v1/prisma/prisma-client';
import os from 'os';
Expand All @@ -13,15 +17,22 @@ import {
S3_OPTIONS,
} from '../../constants/admin';

/* */

const getFileList = async (s3Client: S3Client, key: string) => {
const response = await s3Client.send(
new ListObjectsCommand({
Bucket: S3_BUCKET,
Prefix: `${APP_ANNOUNCEMENTS_FOLDER}/${key}`,
}),
);
return response.Contents ?? [];
};

const getMostRecentFile = async (s3Client: S3Client, key: string) => {
try {
const response = await s3Client.send(
new ListObjectsCommand({
Bucket: S3_BUCKET,
Prefix: `${APP_ANNOUNCEMENTS_FOLDER}/${key}`,
}),
);
const sortedData: any = response.Contents.sort((a: any, b: any) => {
const fileList = await getFileList(s3Client, key);
const sortedData = fileList.sort((a, b) => {
const modifiedDateA: any = new Date(a.LastModified);
const modifiedDateB: any = new Date(b.LastModified);
return modifiedDateB - modifiedDateA;
Expand Down Expand Up @@ -102,3 +113,68 @@ export const downloadFile = async (res, fileId: string) => {
res.status(400).json({ message: 'Invalid request', error });
}
};

/**
* Delete multiple files from the object store that follow the 'keep-history-strategy'
* @param ids
* @returns A Set<string> of id's that are no longer in the object store
*/
export const deleteFiles = async (ids: string[]): Promise<Set<string>> => {
const s3Client = new S3Client(S3_OPTIONS);
try {
// Get all the files stored under each id
const filesPerId = await Promise.all(
ids.map((id) => getFileList(s3Client, id)), //TODO: try deleting the folders instead of each individual file. https://stackoverflow.com/a/73367823
);
const idsWithNoFiles = ids.filter(
(id, index) => filesPerId[index].length === 0,
);
const files = filesPerId.flat();

// Group into 1000 items because DeleteObjectsCommand can only do 1000 at a time
const groupedFiles: _Object[][] = [];
for (let i = 0; i < files.length; i += 1000)
groupedFiles.push(files.slice(i, i + 1000));

// delete all files in object store
const responsePerGroup = await Promise.all(
groupedFiles.map((group) =>
s3Client.send(
new DeleteObjectsCommand({
Bucket: S3_BUCKET,
Delete: { Objects: group as ObjectIdentifier[] },
}),
),
),
);

// report any errors
responsePerGroup.forEach((r) =>
r.Errors.forEach((e) => {
if (e.Code == 'NoSuchKey') idsWithNoFiles.push(getIdFromKey(e.Key));
logger.error(e.Message);
}),
);

// Return the id of all successful deleted
const successfulIds = responsePerGroup.flatMap((r) =>
r.Deleted.reduce((acc, x) => {
acc.push(getIdFromKey(x.Key));
return acc;
}, [] as string[]),
);

return new Set(idsWithNoFiles.concat(successfulIds)); //remove duplicates
} catch (error) {
logger.error(error);
return new Set();
}
};

/**
* Given a string in this format, return the 'id' portion of the string
* ${APP_ANNOUNCEMENTS_FOLDER}/${id}/${file}
*/
function getIdFromKey(key: string): string {
return key.replace(`${APP_ANNOUNCEMENTS_FOLDER}/`, '').split('/', 1)[0];
}
44 changes: 44 additions & 0 deletions backend/src/schedulers/delete-announcements-scheduler.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import waitFor from 'wait-for-expect';
import deleteAnnouncementsJob from './delete-announcements-scheduler';
import { announcementService } from '../v1/services/announcements-service';

jest.mock('../config', () => ({
config: {
get: (key: string) => {
const settings = {
'server:deleteAnnouncementsCronTime': '121212121',
};

return settings[key];
},
},
}));

jest.mock('./create-job', () => ({
createJob: jest.fn((cronTime, callback, mutex, { title, message }) => {
return {
start: jest.fn(async () => {
console.log(`Mock run`);
try {
await callback(); // Simulate the callback execution
} catch (e) {
console.error(`Mock error`);
} finally {
console.log(`Mock end run`);
}
}),
};
}),
}));
jest.mock('../v1/services/announcements-service');

describe('delete-announcements-scheduler', () => {
it('should run the function', async () => {
deleteAnnouncementsJob.start();
await waitFor(async () => {
expect(
announcementService.deleteAnnouncementsSchedule,
).toHaveBeenCalled();
});
});
});
25 changes: 25 additions & 0 deletions backend/src/schedulers/delete-announcements-scheduler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { config } from '../config';
import { logger as log } from '../logger';
import advisoryLock from 'advisory-lock';
import { createJob } from './create-job';
import { announcementService } from '../v1/services/announcements-service';

const SCHEDULER_NAME = 'DeleteAnnouncements';
const mutex = advisoryLock(config.get('server:databaseUrl'))(
`${SCHEDULER_NAME}-lock`,
);
const crontime = config.get('server:deleteAnnouncementsCronTime');

export default createJob(
crontime,
async () => {
log.info(`Starting scheduled job '${SCHEDULER_NAME}'.`);
await announcementService.deleteAnnouncementsSchedule();
log.info(`Completed scheduled job '${SCHEDULER_NAME}'.`);
},
mutex,
{
title: `Error in ${SCHEDULER_NAME}`,
message: `Error in scheduled job: ${SCHEDULER_NAME}`,
},
);
10 changes: 10 additions & 0 deletions backend/src/schedulers/run.all.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ jest.mock('./email-expiring-announcements-scheduler', () => ({
},
}));

const mockDeleteAnnouncementsScheduler = jest.fn();
jest.mock('./delete-announcements-scheduler', () => ({
__esModule: true,

default: {
start: () => mockDeleteAnnouncementsScheduler(),
},
}));

describe('run.all', () => {
it('should start all jobs', async () => {
run();
Expand All @@ -53,5 +62,6 @@ describe('run.all', () => {
expect(mockStartReportLock).toHaveBeenCalled();
expect(mockExpireAnnouncementsLock).toHaveBeenCalled();
expect(mockEmailExpiringAnnouncementsScheduler).toHaveBeenCalled();
expect(mockDeleteAnnouncementsScheduler).toHaveBeenCalled();
});
});
2 changes: 2 additions & 0 deletions backend/src/schedulers/run.all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import deleteUserErrorsJob from './delete-user-errors-scheduler';
import lockReportsJob from './lock-reports-scheduler';
import expireAnnouncementsJob from './expire-announcements-scheduler';
import emailExpiringAnnouncementsJob from './email-expiring-announcements-scheduler';
import deleteAnnouncementsJob from './delete-announcements-scheduler';

export const run = () => {
try {
Expand All @@ -12,6 +13,7 @@ export const run = () => {
lockReportsJob?.start();
expireAnnouncementsJob?.start();
emailExpiringAnnouncementsJob?.start();
deleteAnnouncementsJob?.start();
} catch (error) {
/* istanbul ignore next */
logger.error(error);
Expand Down
Loading

0 comments on commit 161de73

Please sign in to comment.