Skip to content

Commit

Permalink
TechDebt: AWS V3 Update (#1304)
Browse files Browse the repository at this point in the history
* AWS V3 Update

* Remove unused attachment resubmit code

* WIP: Fix unit tests

* Fix tests

---------

Co-authored-by: Macgregor Aubertin-Young <[email protected]>
  • Loading branch information
NickPhura and mauberti-bc authored Jun 28, 2024
1 parent ce03ca4 commit 9113f6a
Show file tree
Hide file tree
Showing 23 changed files with 2,351 additions and 1,236 deletions.
2,809 changes: 2,089 additions & 720 deletions api/package-lock.json

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@
"npm": ">= 10.0.0"
},
"dependencies": {
"@aws-sdk/client-s3": "^3.583.0",
"@aws-sdk/s3-request-presigner": "^3.583.0",
"@turf/bbox": "^6.5.0",
"@turf/circle": "^6.5.0",
"@turf/helpers": "^6.5.0",
"@turf/meta": "^6.5.0",
"adm-zip": "^0.5.5",
"ajv": "^8.12.0",
"aws-sdk": "^2.1565.0",
"axios": "^1.6.7",
"clamscan": "^2.2.1",
"dayjs": "^1.11.10",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { S3 } from 'aws-sdk';
import { DeleteObjectCommandOutput } from '@aws-sdk/client-s3';
import chai, { expect } from 'chai';
import { describe } from 'mocha';
import sinon from 'sinon';
Expand Down Expand Up @@ -66,7 +66,7 @@ describe('deleteSurvey', () => {

const fileUtilsStub = sinon
.stub(file_utils, 'deleteFileFromS3')
.resolves(false as unknown as S3.DeleteObjectOutput);
.resolves(false as unknown as DeleteObjectCommandOutput);

let actualResult: any = null;
const sampleRes = {
Expand Down Expand Up @@ -107,7 +107,9 @@ describe('deleteSurvey', () => {

const deleteSurveyStub = sinon.stub(SurveyService.prototype, 'deleteSurvey').resolves();

const fileUtilsStub = sinon.stub(file_utils, 'deleteFileFromS3').resolves(true as unknown as S3.DeleteObjectOutput);
const fileUtilsStub = sinon
.stub(file_utils, 'deleteFileFromS3')
.resolves(true as unknown as DeleteObjectCommandOutput);

let actualResult: any = null;
const sampleRes = {
Expand Down
23 changes: 13 additions & 10 deletions api/src/paths/resources/list.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe('listResources', () => {

it('returns an empty array if no resources are found', async () => {
const listFilesStub = sinon.stub(fileUtils, 'listFilesFromS3').resolves({
$metadata: {},
Contents: []
});

Expand All @@ -35,7 +36,7 @@ describe('listResources', () => {
});

it('returns an array of resources', async () => {
const mockMetadata = {
const mockMetadata: Record<string, Record<string, string>> = {
['key1']: {
'template-name': 'name1',
'template-type': 'type1',
Expand All @@ -55,11 +56,14 @@ describe('listResources', () => {

sinon.stub(fileUtils, 'getObjectMeta').callsFake((key: string) => {
return Promise.resolve({
$metadata: {},
Conents: [],
Metadata: mockMetadata[key]
});
});

const listFilesStub = sinon.stub(fileUtils, 'listFilesFromS3').resolves({
$metadata: {},
Contents: [
{
Key: 'key1',
Expand Down Expand Up @@ -89,7 +93,7 @@ describe('listResources', () => {
files: [
{
fileName: 'key1',
url: 's3.host.example.com/test-bucket/key1',
url: 'https://s3.host.example.com/test-bucket/key1',
lastModified: new Date('2023-01-01').toISOString(),
fileSize: 5,
metadata: {
Expand All @@ -100,7 +104,7 @@ describe('listResources', () => {
},
{
fileName: 'key2',
url: 's3.host.example.com/test-bucket/key2',
url: 'https://s3.host.example.com/test-bucket/key2',
lastModified: new Date('2023-01-02').toISOString(),
fileSize: 10,
metadata: {
Expand All @@ -111,7 +115,7 @@ describe('listResources', () => {
},
{
fileName: 'key3',
url: 's3.host.example.com/test-bucket/key3',
url: 'https://s3.host.example.com/test-bucket/key3',
lastModified: new Date('2023-01-03').toISOString(),
fileSize: 15,
metadata: {
Expand All @@ -126,14 +130,13 @@ describe('listResources', () => {
});

it('should filter out directories from the s3 list respones', async () => {
sinon.stub(fileUtils, 'getObjectMeta').resolves({});
sinon.stub(fileUtils, 'getObjectMeta').resolves({
$metadata: {}
});

const listFilesStub = sinon.stub(fileUtils, 'listFilesFromS3').resolves({
Contents: [
{
Key: 'templates/Current/'
}
]
$metadata: {},
Contents: [{ Key: 'templates/Current/' }]
});

const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();
Expand Down
5 changes: 2 additions & 3 deletions api/src/paths/resources/list.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Object as S3Object } from 'aws-sdk/clients/s3';
import { RequestHandler } from 'express';
import { Operation } from 'express-openapi';
import { getObjectMeta, getS3HostUrl, listFilesFromS3 } from '../../utils/file-utils';
Expand Down Expand Up @@ -94,8 +93,8 @@ export function listResources(): RequestHandler {
* which fetch the metadata for each object in the list.
*/
const filePromises = (response?.Contents || [])
.filter((file: S3Object) => !file.Key?.endsWith('/'))
.map(async (file: S3Object) => {
.filter((file) => !file.Key?.endsWith('/'))
.map(async (file) => {
let metadata = {};
let fileName = '';

Expand Down
85 changes: 41 additions & 44 deletions api/src/services/attachment-service.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import AWS from 'aws-sdk';
import { DeleteObjectCommandOutput } from '@aws-sdk/client-s3';
import chai, { expect } from 'chai';
import { describe } from 'mocha';
import { QueryResult } from 'pg';
Expand All @@ -17,6 +17,7 @@ import {
} from '../repositories/attachment-repository';
import { SurveyAttachmentPublish, SurveyReportPublish } from '../repositories/history-publish-repository';
import { getMockDBConnection } from '../__mocks__/db';
import * as fileUtils from './../utils/file-utils';
import { AttachmentService } from './attachment-service';
import { HistoryPublishService } from './history-publish-service';

Expand Down Expand Up @@ -289,7 +290,7 @@ describe('AttachmentService', () => {
const getProjectReportStub = sinon
.stub(AttachmentService.prototype, 'getProjectReportAttachmentById')
.resolves({
key: 'key',
key: 's3_key',
uuid: 'uuid',
project_report_attachment_id: 1
} as unknown as IProjectReportAttachment);
Expand All @@ -306,21 +307,20 @@ describe('AttachmentService', () => {
.stub(AttachmentService.prototype, '_deleteProjectAttachmentRecord')
.resolves();

const mockS3Client = new AWS.S3();
sinon.stub(AWS, 'S3').returns(mockS3Client);
const deleteS3 = sinon.stub(mockS3Client, 'deleteObject').returns({
promise: () =>
Promise.resolve({
DeleteMarker: true
})
} as AWS.Request<AWS.S3.DeleteObjectOutput, AWS.AWSError>);
const mockDeleteResponse: DeleteObjectCommandOutput = {
$metadata: {},
DeleteMarker: true,
RequestCharged: 'requester',
VersionId: '123456'
};
const deleteFileFromS3Stub = sinon.stub(fileUtils, 'deleteFileFromS3').resolves(mockDeleteResponse);

await service.deleteProjectAttachment(1, 1, ATTACHMENT_TYPE.REPORT);

expect(getProjectReportStub).to.be.called;
expect(deleteProjectReportAuthorsStub).to.be.called;
expect(deleteProjectReportAttachmentStub).to.be.called;
expect(deleteS3).to.be.called;
expect(deleteFileFromS3Stub).to.be.calledOnceWith('s3_key');

expect(deleteProjectAttachmentStub).to.not.be.called;
expect(getProjectAttachmentStub).to.not.be.called;
Expand All @@ -335,7 +335,7 @@ describe('AttachmentService', () => {
const getProjectReportStub = sinon
.stub(AttachmentService.prototype, 'getProjectReportAttachmentById')
.resolves({
key: 'key',
key: 's3_key',
uuid: 'uuid',
project_report_attachment_id: 1
} as unknown as IProjectReportAttachment);
Expand All @@ -348,28 +348,27 @@ describe('AttachmentService', () => {
const getProjectAttachmentStub = sinon
.stub(AttachmentService.prototype, 'getProjectAttachmentById')
.resolves({
key: 'key',
key: 's3_key',
uuid: 'uuid',
project_attachment_id: 1
} as unknown as IProjectAttachment);
const deleteProjectAttachmentStub = sinon
.stub(AttachmentService.prototype, '_deleteProjectAttachmentRecord')
.resolves();

const mockS3Client = new AWS.S3();
sinon.stub(AWS, 'S3').returns(mockS3Client);
const deleteS3 = sinon.stub(mockS3Client, 'deleteObject').returns({
promise: () =>
Promise.resolve({
DeleteMarker: true
})
} as AWS.Request<AWS.S3.DeleteObjectOutput, AWS.AWSError>);
const mockDeleteResponse: DeleteObjectCommandOutput = {
$metadata: {},
DeleteMarker: true,
RequestCharged: 'requester',
VersionId: '123456'
};
const deleteFileFromS3Stub = sinon.stub(fileUtils, 'deleteFileFromS3').resolves(mockDeleteResponse);

await service.deleteProjectAttachment(1, 1, ATTACHMENT_TYPE.OTHER);

expect(getProjectAttachmentStub).to.be.called;
expect(deleteProjectAttachmentStub).to.be.called;
expect(deleteS3).to.be.called;
expect(deleteFileFromS3Stub).to.be.calledOnceWith('s3_key');

expect(getProjectReportStub).to.not.be.called;
expect(deleteProjectReportAuthorsStub).to.not.be.called;
Expand Down Expand Up @@ -739,7 +738,7 @@ describe('AttachmentService', () => {
survey_report_attachment_id: 1,
survey_attachment_id: 1,
uuid: 'uuid',
key: 's3/key'
key: 's3_key'
} as unknown as ISurveyAttachment);
const deleteSurveyReportPublishStub = sinon
.stub(HistoryPublishService.prototype, 'deleteSurveyReportAttachmentPublishRecord')
Expand All @@ -764,24 +763,23 @@ describe('AttachmentService', () => {
survey_report_attachment_id: 1,
survey_attachment_id: 1,
uuid: 'uuid',
key: 's3/key'
key: 's3_key'
} as unknown as ISurveyReportAttachment);

const mockS3Client = new AWS.S3();
sinon.stub(AWS, 'S3').returns(mockS3Client);
const deleteS3 = sinon.stub(mockS3Client, 'deleteObject').returns({
promise: () =>
Promise.resolve({
DeleteMarker: true
})
} as AWS.Request<AWS.S3.DeleteObjectOutput, AWS.AWSError>);
const mockDeleteResponse: DeleteObjectCommandOutput = {
$metadata: {},
DeleteMarker: true,
RequestCharged: 'requester',
VersionId: '123456'
};
const deleteFileFromS3Stub = sinon.stub(fileUtils, 'deleteFileFromS3').resolves(mockDeleteResponse);

await service.deleteSurveyAttachment(1, 1, ATTACHMENT_TYPE.OTHER);

expect(getSurveyAttachmentStub).to.be.called;
expect(attachmentPublishDeleteStub).to.be.called;
expect(deleteSurveyAttachmentStub).to.be.called;
expect(deleteS3).to.be.called;
expect(deleteFileFromS3Stub).to.be.calledOnceWith('s3_key');

expect(getSurveyReportStub).to.be.not.called;
expect(deleteSurveyReportPublishStub).to.be.not.called;
Expand All @@ -801,7 +799,7 @@ describe('AttachmentService', () => {
survey_report_attachment_id: 1,
survey_attachment_id: 1,
uuid: 'uuid',
key: 's3/key'
key: 's3_key'
} as unknown as ISurveyAttachment);
const attachmentPublishStatusStub = sinon
.stub(HistoryPublishService.prototype, 'getSurveyAttachmentPublishRecord')
Expand All @@ -827,25 +825,24 @@ describe('AttachmentService', () => {
survey_report_attachment_id: 1,
survey_attachment_id: 1,
uuid: 'uuid',
key: 's3/key'
key: 's3_key'
} as unknown as ISurveyReportAttachment);

const mockS3Client = new AWS.S3();
sinon.stub(AWS, 'S3').returns(mockS3Client);
const deleteS3 = sinon.stub(mockS3Client, 'deleteObject').returns({
promise: () =>
Promise.resolve({
DeleteMarker: true
})
} as AWS.Request<AWS.S3.DeleteObjectOutput, AWS.AWSError>);
const mockDeleteResponse: DeleteObjectCommandOutput = {
$metadata: {},
DeleteMarker: true,
RequestCharged: 'requester',
VersionId: '123456'
};
const deleteFileFromS3Stub = sinon.stub(fileUtils, 'deleteFileFromS3').resolves(mockDeleteResponse);

await service.deleteSurveyAttachment(1, 1, ATTACHMENT_TYPE.REPORT);

expect(getSurveyReportStub).to.be.called;
expect(deleteSurveyReportPublishStub).to.be.called;
expect(deleteSurveyReportAuthorsStub).to.be.called;
expect(deleteSurveyReportStub).to.be.called;
expect(deleteS3).to.be.called;
expect(deleteFileFromS3Stub).to.be.calledOnceWith('s3_key');

expect(getSurveyAttachmentStub).to.be.not.called;
expect(attachmentPublishStatusStub).to.be.not.called;
Expand Down
2 changes: 1 addition & 1 deletion api/src/services/observation-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ export class ObservationService extends DBService {
const s3Object = await getFileFromS3(observationSubmissionRecord.key);

// Get the csv file from the S3 object
const mediaFile = parseS3File(s3Object);
const mediaFile = await parseS3File(s3Object);

// Validate the CSV file mime type
if (mediaFile.mimetype !== 'text/csv') {
Expand Down
10 changes: 8 additions & 2 deletions api/src/services/platform-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,10 @@ export class PlatformService extends DBService {
const artifact = {
submission_uuid: submissionUUID,
artifact_upload_key: artifactUploadKey,
data: s3File.Body as Buffer,
// TODO: Cast to unknown required due to issue in aws-sdk v3 typings
// See https://stackoverflow.com/questions/76142043/getting-a-readable-from-getobject-in-aws-s3-sdk-v3
// See https://github.com/aws/aws-sdk-js-v3/issues/4720
data: s3File.Body as unknown as Buffer,
fileName: attachment.file_name,
mimeType: s3File.ContentType || mime.getType(attachment.file_name) || 'application/octet-stream'
};
Expand Down Expand Up @@ -321,7 +324,10 @@ export class PlatformService extends DBService {
const artifact = {
submission_uuid: submissionUUID,
artifact_upload_key: artifactUploadKey,
data: s3File.Body as Buffer,
// TODO: Cast to unknown required due to issue in aws-sdk v3 typings
// See https://stackoverflow.com/questions/76142043/getting-a-readable-from-getobject-in-aws-s3-sdk-v3
// See https://github.com/aws/aws-sdk-js-v3/issues/4720
data: s3File.Body as unknown as Buffer,
fileName: attachment.file_name,
mimeType: s3File.ContentType || mime.getType(attachment.file_name) || 'application/octet-stream'
};
Expand Down
2 changes: 1 addition & 1 deletion api/src/services/telemetry-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class TelemetryService extends DBService {
const s3Object = await getFileFromS3(submission.key);

// step 3 parse the file
const mediaFile = parseS3File(s3Object);
const mediaFile = await parseS3File(s3Object);

// step 4 validate csv
if (mediaFile.mimetype !== 'text/csv') {
Expand Down
Loading

0 comments on commit 9113f6a

Please sign in to comment.