Skip to content
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

TechDebt: AWS V3 Update #1304

Merged
merged 10 commits into from
Jun 28, 2024
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
Loading