diff options
| author | YS <47836716+yszkst@users.noreply.github.com> | 2023-03-23 13:48:14 +0900 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2023-03-23 13:48:14 +0900 |
| commit | 658901a47f3171941a66fa3e0a4906a9d30acbb5 (patch) | |
| tree | 6d3b2e7795994cceae88fb19e3a89f1210ba9d04 /packages/backend | |
| parent | fix(backend): 絵文字を編集すると保存できないことがある問... (diff) | |
| download | sharkey-658901a47f3171941a66fa3e0a4906a9d30acbb5.tar.gz sharkey-658901a47f3171941a66fa3e0a4906a9d30acbb5.tar.bz2 sharkey-658901a47f3171941a66fa3e0a4906a9d30acbb5.zip | |
bump aws-sdk to v3 for s3 (#10363)
* indent
* aws-sdk v3移行
Diffstat (limited to 'packages/backend')
| -rw-r--r-- | packages/backend/package.json | 5 | ||||
| -rw-r--r-- | packages/backend/src/core/DriveService.ts | 53 | ||||
| -rw-r--r-- | packages/backend/src/core/S3Service.ts | 61 | ||||
| -rw-r--r-- | packages/backend/test/unit/DriveService.ts | 59 | ||||
| -rw-r--r-- | packages/backend/test/unit/S3Service.ts | 77 |
5 files changed, 180 insertions, 75 deletions
diff --git a/packages/backend/package.json b/packages/backend/package.json index 5a3dcfb5e7..3f640c4a63 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -37,6 +37,9 @@ "@tensorflow/tfjs-node": "4.2.0" }, "dependencies": { + "@aws-sdk/client-s3": "^3.294.0", + "@aws-sdk/lib-storage": "^3.294.0", + "@aws-sdk/node-http-handler": "^3.292.0", "@bull-board/api": "5.0.0", "@bull-board/fastify": "5.0.0", "@bull-board/ui": "5.0.0", @@ -59,7 +62,6 @@ "ajv": "8.12.0", "archiver": "5.3.1", "autwh": "0.1.0", - "aws-sdk": "2.1318.0", "bcryptjs": "2.4.3", "blurhash": "2.0.5", "bull": "4.10.4", @@ -190,6 +192,7 @@ "@types/ws": "8.5.4", "@typescript-eslint/eslint-plugin": "5.54.1", "@typescript-eslint/parser": "5.54.1", + "aws-sdk-client-mock": "^2.1.1", "cross-env": "7.0.3", "eslint": "8.35.0", "eslint-plugin-import": "2.27.5", diff --git a/packages/backend/src/core/DriveService.ts b/packages/backend/src/core/DriveService.ts index f1e93d6dd9..97d03d5f52 100644 --- a/packages/backend/src/core/DriveService.ts +++ b/packages/backend/src/core/DriveService.ts @@ -4,6 +4,7 @@ import { v4 as uuid } from 'uuid'; import sharp from 'sharp'; import { sharpBmp } from 'sharp-read-bmp'; import { IsNull } from 'typeorm'; +import { DeleteObjectCommandInput, PutObjectCommandInput, NoSuchKey } from '@aws-sdk/client-s3'; import { DI } from '@/di-symbols.js'; import type { DriveFilesRepository, UsersRepository, DriveFoldersRepository, UserProfilesRepository } from '@/models/index.js'; import type { Config } from '@/config.js'; @@ -36,7 +37,6 @@ import { bindThis } from '@/decorators.js'; import { RoleService } from '@/core/RoleService.js'; import { correctFilename } from '@/misc/correct-filename.js'; import { isMimeImage } from '@/misc/is-mime-image.js'; -import type S3 from 'aws-sdk/clients/s3.js'; type AddFileArgs = { /** User who wish to add file */ @@ -81,6 +81,7 @@ type UploadFromUrlArgs = { export class DriveService { private registerLogger: Logger; private downloaderLogger: Logger; + private deleteLogger: Logger; constructor( @Inject(DI.config) @@ -118,6 +119,7 @@ export class DriveService { const logger = new Logger('drive', 'blue'); this.registerLogger = logger.createSubLogger('register', 'yellow'); this.downloaderLogger = logger.createSubLogger('downloader'); + this.deleteLogger = logger.createSubLogger('delete'); } /*** @@ -368,7 +370,7 @@ export class DriveService { Body: stream, ContentType: type, CacheControl: 'max-age=31536000, immutable', - } as S3.PutObjectRequest; + } as PutObjectCommandInput; if (filename) params.ContentDisposition = contentDisposition( 'inline', @@ -378,21 +380,16 @@ export class DriveService { ); if (meta.objectStorageSetPublicRead) params.ACL = 'public-read'; - const s3 = this.s3Service.getS3(meta); - - const upload = s3.upload(params, { - partSize: s3.endpoint.hostname === 'storage.googleapis.com' ? 500 * 1024 * 1024 : 8 * 1024 * 1024, - }); - - await upload.promise() + await this.s3Service.upload(meta, params) .then( result => { - if (result) { + if ('Bucket' in result) { // CompleteMultipartUploadCommandOutput this.registerLogger.debug(`Uploaded: ${result.Bucket}/${result.Key} => ${result.Location}`); - } else { - this.registerLogger.error(`Upload Result Empty: key = ${key}, filename = ${filename}`); + } else { // AbortMultipartUploadCommandOutput + this.registerLogger.error(`Upload Result Aborted: key = ${key}, filename = ${filename}`); } - }, + }) + .catch( err => { this.registerLogger.error(`Upload Failed: key = ${key}, filename = ${filename}`, err); }, @@ -528,10 +525,10 @@ export class DriveService { }; const properties: { - width?: number; - height?: number; - orientation?: number; - } = {}; + width?: number; + height?: number; + orientation?: number; + } = {}; if (info.width) { properties['width'] = info.width; @@ -720,22 +717,22 @@ export class DriveService { @bindThis public async deleteObjectStorageFile(key: string) { const meta = await this.metaService.fetch(); - - const s3 = this.s3Service.getS3(meta); - try { - await s3.deleteObject({ - Bucket: meta.objectStorageBucket!, + const param = { + Bucket: meta.objectStorageBucket, Key: key, - }).promise(); + } as DeleteObjectCommandInput; + + await this.s3Service.delete(meta, param); } catch (err: any) { - if (err.code === 'NoSuchKey') { - console.warn(`The object storage had no such key to delete: ${key}. Skipping this.`, err); + if (err.name === 'NoSuchKey') { + this.deleteLogger.warn(`The object storage had no such key to delete: ${key}. Skipping this.`, err as Error); return; + } else { + throw new Error(`Failed to delete the file from the object storage with the given key: ${key}`, { + cause: err, + }); } - throw new Error(`Failed to delete the file from the object storage with the given key: ${key}`, { - cause: err, - }); } } diff --git a/packages/backend/src/core/S3Service.ts b/packages/backend/src/core/S3Service.ts index cc8f950813..629278d915 100644 --- a/packages/backend/src/core/S3Service.ts +++ b/packages/backend/src/core/S3Service.ts @@ -1,11 +1,16 @@ import { URL } from 'node:url'; +import * as http from 'node:http'; +import * as https from 'node:https'; import { Inject, Injectable } from '@nestjs/common'; -import S3 from 'aws-sdk/clients/s3.js'; +import { DeleteObjectCommand, S3Client } from '@aws-sdk/client-s3'; +import { Upload } from '@aws-sdk/lib-storage'; +import { NodeHttpHandler, NodeHttpHandlerOptions } from '@aws-sdk/node-http-handler'; import { DI } from '@/di-symbols.js'; import type { Config } from '@/config.js'; import type { Meta } from '@/models/entities/Meta.js'; import { HttpRequestService } from '@/core/HttpRequestService.js'; import { bindThis } from '@/decorators.js'; +import type { DeleteObjectCommandInput, PutObjectCommandInput } from '@aws-sdk/client-s3'; @Injectable() export class S3Service { @@ -18,25 +23,47 @@ export class S3Service { } @bindThis - public getS3(meta: Meta) { + public getS3Client(meta: Meta): S3Client { const u = meta.objectStorageEndpoint - ? `${meta.objectStorageUseSSL ? 'https://' : 'http://'}${meta.objectStorageEndpoint}` - : `${meta.objectStorageUseSSL ? 'https://' : 'http://'}example.net`; + ? `${meta.objectStorageUseSSL ? 'https' : 'http'}://${meta.objectStorageEndpoint}` + : `${meta.objectStorageUseSSL ? 'https' : 'http'}://example.net`; // dummy url to select http(s) agent - return new S3({ - endpoint: meta.objectStorageEndpoint && meta.objectStorageEndpoint.length > 0 - ? meta.objectStorageEndpoint - : undefined, - accessKeyId: meta.objectStorageAccessKey!, - secretAccessKey: meta.objectStorageSecretKey!, + const agent = this.httpRequestService.getAgentByUrl(new URL(u), !meta.objectStorageUseProxy); + const handlerOption: NodeHttpHandlerOptions = {}; + if (meta.objectStorageUseSSL) { + handlerOption.httpsAgent = agent as https.Agent; + } else { + handlerOption.httpAgent = agent as http.Agent; + } + + return new S3Client({ + endpoint: meta.objectStorageEndpoint ? u : undefined, + credentials: (meta.objectStorageAccessKey !== null && meta.objectStorageSecretKey !== null) ? { + accessKeyId: meta.objectStorageAccessKey, + secretAccessKey: meta.objectStorageSecretKey, + } : undefined, region: meta.objectStorageRegion ?? undefined, - sslEnabled: meta.objectStorageUseSSL, - s3ForcePathStyle: !meta.objectStorageEndpoint // AWS with endPoint omitted - ? false - : meta.objectStorageS3ForcePathStyle, - httpOptions: { - agent: this.httpRequestService.getAgentByUrl(new URL(u), !meta.objectStorageUseProxy), - }, + tls: meta.objectStorageUseSSL, + forcePathStyle: meta.objectStorageEndpoint ? meta.objectStorageS3ForcePathStyle : false, // AWS with endPoint omitted + requestHandler: new NodeHttpHandler(handlerOption), }); } + + @bindThis + public async upload(meta: Meta, input: PutObjectCommandInput) { + const client = this.getS3Client(meta); + return new Upload({ + client, + params: input, + partSize: (client.config.endpoint && (await client.config.endpoint()).hostname === 'storage.googleapis.com') + ? 500 * 1024 * 1024 + : 8 * 1024 * 1024, + }).done(); + } + + @bindThis + public delete(meta: Meta, input: DeleteObjectCommandInput) { + const client = this.getS3Client(meta); + return client.send(new DeleteObjectCommand(input)); + } } diff --git a/packages/backend/test/unit/DriveService.ts b/packages/backend/test/unit/DriveService.ts index 0549800a68..4065665579 100644 --- a/packages/backend/test/unit/DriveService.ts +++ b/packages/backend/test/unit/DriveService.ts @@ -1,55 +1,56 @@ process.env.NODE_ENV = 'test'; -import { jest } from '@jest/globals'; import { Test } from '@nestjs/testing'; +import { DeleteObjectCommandOutput, DeleteObjectCommand, NoSuchKey, InvalidObjectState, S3Client } from '@aws-sdk/client-s3'; +import { mockClient } from 'aws-sdk-client-mock'; import { GlobalModule } from '@/GlobalModule.js'; import { DriveService } from '@/core/DriveService.js'; import { CoreModule } from '@/core/CoreModule.js'; -import { S3Service } from '@/core/S3Service'; -import type { Meta } from '@/models'; -import type { DeleteObjectOutput } from 'aws-sdk/clients/s3'; -import type { AWSError } from 'aws-sdk/lib/error'; -import type { PromiseResult, Request } from 'aws-sdk/lib/request'; import type { TestingModule } from '@nestjs/testing'; describe('DriveService', () => { let app: TestingModule; let driveService: DriveService; + const s3Mock = mockClient(S3Client); - beforeEach(async () => { + beforeAll(async () => { app = await Test.createTestingModule({ imports: [GlobalModule, CoreModule], - providers: [DriveService, S3Service], + providers: [DriveService], }).compile(); app.enableShutdownHooks(); driveService = app.get<DriveService>(DriveService); + }); - const s3Service = app.get<S3Service>(S3Service); - const s3 = s3Service.getS3({} as Meta); + beforeEach(async () => { + s3Mock.reset(); + }); - // new S3() surprisingly does not return an instance of class S3. - // Let's use getPrototypeOf here to get a real prototype, since spying on S3.prototype doesn't work. - // TODO: Use `aws-sdk-client-mock` package when upgrading to AWS SDK v3. - jest.spyOn(Object.getPrototypeOf(s3), 'deleteObject').mockImplementation(() => { - // Roughly mock AWS request object - return { - async promise(): Promise<PromiseResult<DeleteObjectOutput, AWSError>> { - const err = new Error('mock') as AWSError; - err.code = 'NoSuchKey'; - throw err; - }, - } as Request<DeleteObjectOutput, AWSError>; - }); + afterAll(async () => { + await app.close(); }); describe('Object storage', () => { + test('delete a file', async () => { + s3Mock.on(DeleteObjectCommand) + .resolves({} as DeleteObjectCommandOutput); + + await driveService.deleteObjectStorageFile('peace of the world'); + }); + + test('delete a file then unexpected error', async () => { + s3Mock.on(DeleteObjectCommand) + .rejects(new InvalidObjectState({ $metadata: {}, message: '' })); + + await expect(driveService.deleteObjectStorageFile('unexpected')).rejects.toThrowError(Error); + }); + test('delete a file with no valid key', async () => { - try { - await driveService.deleteObjectStorageFile('lol no way'); - } catch (err: any) { - console.log(err.cause); - throw err; - } + // Some S3 implementations returns 404 Not Found on deleting with a non-existent key + s3Mock.on(DeleteObjectCommand) + .rejects(new NoSuchKey({ $metadata: {}, message: 'allowed error.' })); + + await driveService.deleteObjectStorageFile('lol no way'); }); }); }); diff --git a/packages/backend/test/unit/S3Service.ts b/packages/backend/test/unit/S3Service.ts new file mode 100644 index 0000000000..1dfa22afd2 --- /dev/null +++ b/packages/backend/test/unit/S3Service.ts @@ -0,0 +1,77 @@ +process.env.NODE_ENV = 'test'; + +import { Test } from '@nestjs/testing'; +import { UploadPartCommand, CompleteMultipartUploadCommand, CreateMultipartUploadCommand, S3Client, PutObjectCommand } from '@aws-sdk/client-s3'; +import { mockClient } from 'aws-sdk-client-mock'; +import { GlobalModule } from '@/GlobalModule.js'; +import { CoreModule } from '@/core/CoreModule.js'; +import { S3Service } from '@/core/S3Service'; +import { Meta } from '@/models'; +import type { TestingModule } from '@nestjs/testing'; + +describe('S3Service', () => { + let app: TestingModule; + let s3Service: S3Service; + const s3Mock = mockClient(S3Client); + + beforeAll(async () => { + app = await Test.createTestingModule({ + imports: [GlobalModule, CoreModule], + providers: [S3Service], + }).compile(); + app.enableShutdownHooks(); + s3Service = app.get<S3Service>(S3Service); + }); + + beforeEach(async () => { + s3Mock.reset(); + }); + + afterAll(async () => { + await app.close(); + }); + + describe('upload', () => { + test('upload a file', async () => { + s3Mock.on(PutObjectCommand).resolves({}); + + await s3Service.upload({ objectStorageRegion: 'us-east-1' } as Meta, { + Bucket: 'fake', + Key: 'fake', + Body: 'x', + }); + }); + + test('upload a large file', async () => { + s3Mock.on(CreateMultipartUploadCommand).resolves({ UploadId: '1' }); + s3Mock.on(UploadPartCommand).resolves({ ETag: '1' }); + s3Mock.on(CompleteMultipartUploadCommand).resolves({ Bucket: 'fake', Key: 'fake' }); + + await s3Service.upload({} as Meta, { + Bucket: 'fake', + Key: 'fake', + Body: 'x'.repeat(8 * 1024 * 1024 + 1), // デフォルトpartSizeにしている 8 * 1024 * 1024 を越えるサイズ + }); + }); + + test('upload a file error', async () => { + s3Mock.on(PutObjectCommand).rejects({ name: 'Fake Error' }); + + await expect(s3Service.upload({ objectStorageRegion: 'us-east-1' } as Meta, { + Bucket: 'fake', + Key: 'fake', + Body: 'x', + })).rejects.toThrowError(Error); + }); + + test('upload a large file error', async () => { + s3Mock.on(UploadPartCommand).rejects(); + + await expect(s3Service.upload({} as Meta, { + Bucket: 'fake', + Key: 'fake', + Body: 'x'.repeat(8 * 1024 * 1024 + 1), // デフォルトpartSizeにしている 8 * 1024 * 1024 を越えるサイズ + })).rejects.toThrowError(Error); + }); + }); +}); |