From d5471a66a0f202746852acef9d77791e4c8571ac Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 16 Jan 2025 18:29:39 -0500 Subject: [PATCH 01/11] Revise createFile logic to preserve key & location - Add error handling for key generation - Standardize return object with location, url, and filename - Add support for optional config parameter - Return s3 response explicitly as a separate variable --- index.js | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/index.js b/index.js index d0589ee..e6ac4d0 100644 --- a/index.js +++ b/index.js @@ -139,16 +139,23 @@ class S3Adapter { // For a given config object, filename, and data, store a file in S3 // Returns a promise containing the S3 object creation response - async createFile(filename, data, contentType, options = {}) { + async createFile(filename, data, contentType, options = {}, config= {}) { + + let key_without_prefix = filename; + if (this._generateKey instanceof Function) { + try { + key_without_prefix = this._generateKey(filename); + }catch(e){ + throw new Error(e); // throw error if generateKey function fails + } + } + const params = { Bucket: this._bucket, - Key: this._bucketPrefix + filename, + Key: this._bucketPrefix + key_without_prefix, Body: data, }; - - if (this._generateKey instanceof Function) { - params.Key = this._bucketPrefix + this._generateKey(filename); - } + if (this._fileAcl) { if (this._fileAcl === 'none') { delete params.ACL; @@ -180,7 +187,14 @@ class S3Adapter { const endpoint = this._endpoint || `https://${this._bucket}.s3.${this._region}.amazonaws.com`; const location = `${endpoint}/${params.Key}`; - return Object.assign(response || {}, { Location: location }); + const url = await this.getFileLocation(config, key_without_prefix); + + return { + location: location, // actual upload location, used for tests + url: url, // optionally signed url (can be returned to client) + filename: key_without_prefix, // filename in storage + s3_response: response // raw s3 response + }; } async deleteFile(filename) { From a9edee2ca2c25ffc36c61d6f07435abf9514abb4 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 16 Jan 2025 18:42:17 -0500 Subject: [PATCH 02/11] Change to name to be consistent with parse server createFile --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index e6ac4d0..57c8085 100644 --- a/index.js +++ b/index.js @@ -192,7 +192,7 @@ class S3Adapter { return { location: location, // actual upload location, used for tests url: url, // optionally signed url (can be returned to client) - filename: key_without_prefix, // filename in storage + name: key_without_prefix, // filename in storage, consistent with other adapters s3_response: response // raw s3 response }; } From 2592fa82a79b0cfb921bc853362534002cce0205 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 16 Jan 2025 18:55:36 -0500 Subject: [PATCH 03/11] Update test.spec.js --- spec/test.spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/test.spec.js b/spec/test.spec.js index 28193d0..a75ff73 100644 --- a/spec/test.spec.js +++ b/spec/test.spec.js @@ -696,7 +696,7 @@ describe('S3Adapter tests', () => { const s3 = getMockS3Adapter(options); const fileName = 'randomFileName.txt'; const response = s3.createFile(fileName, 'hello world', 'text/utf8').then(value => { - const url = new URL(value.Location); + const url = new URL(value.location); expect(url.pathname.indexOf(fileName) > 13).toBe(true); }); promises.push(response); @@ -707,7 +707,7 @@ describe('S3Adapter tests', () => { const s3 = getMockS3Adapter(options); const fileName = 'foo/randomFileName.txt'; const response = s3.createFile(fileName, 'hello world', 'text/utf8').then(value => { - const url = new URL(value.Location); + const url = new URL(value.location); expect(url.pathname.substring(1)).toEqual(options.bucketPrefix + fileName); }); promises.push(response); @@ -717,7 +717,7 @@ describe('S3Adapter tests', () => { const s3 = getMockS3Adapter(options); const fileName = 'foo/randomFileName.txt'; const response = s3.createFile(fileName, 'hello world', 'text/utf8').then(value => { - const url = new URL(value.Location); + const url = new URL(value.location); expect(url.pathname.indexOf('foo/')).toEqual(6); expect(url.pathname.indexOf('random') > 13).toBe(true); }); From 5abd18a5002d21fc4fdfa4252fb3cc1fe27c420b Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 16 Jan 2025 19:22:52 -0500 Subject: [PATCH 04/11] Update index.js --- index.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 57c8085..e3bb0e7 100644 --- a/index.js +++ b/index.js @@ -187,13 +187,16 @@ class S3Adapter { const endpoint = this._endpoint || `https://${this._bucket}.s3.${this._region}.amazonaws.com`; const location = `${endpoint}/${params.Key}`; - const url = await this.getFileLocation(config, key_without_prefix); + let url; + if (Object.keys(config).length != 0) { // if config is passed, we can generate a presigned url here + url = await this.getFileLocation(config, key_without_prefix); + } return { location: location, // actual upload location, used for tests - url: url, // optionally signed url (can be returned to client) name: key_without_prefix, // filename in storage, consistent with other adapters - s3_response: response // raw s3 response + s3_response: response, // raw s3 response + ...url? {url: url} : {} // url (optionally presigned) or non-direct access url }; } From 537a5ce9d24e91d191cf9b37c721568410af0362 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Fri, 17 Jan 2025 12:57:59 -0500 Subject: [PATCH 05/11] Correction to make codecov happy --- lib/optionsFromArguments.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/optionsFromArguments.js b/lib/optionsFromArguments.js index f56c9bc..b05c1fc 100644 --- a/lib/optionsFromArguments.js +++ b/lib/optionsFromArguments.js @@ -42,12 +42,12 @@ const optionsFromArguments = function optionsFromArguments(args) { } else if (args.length === 2) { options.bucket = stringOrOptions; if (typeof args[1] !== 'object') { - throw new Error("Failed to configure S3Adapter. Arguments don't make sense"); + throw new Error('Failed to configure S3Adapter. Arguments don\'t make sense'); } otherOptions = args[1]; } else if (args.length > 2) { if (typeof args[1] !== 'string' || typeof args[2] !== 'string') { - throw new Error("Failed to configure S3Adapter. Arguments don't make sense"); + throw new Error('Failed to configure S3Adapter. Arguments don\'t make sense'); } options.accessKey = args[0]; options.secretKey = args[1]; @@ -81,7 +81,7 @@ const optionsFromArguments = function optionsFromArguments(args) { options.bucket = s3overrides.params.Bucket; } } else if (args.length > 2) { - throw new Error("Failed to configure S3Adapter. Arguments don't make sense"); + throw new Error('Failed to configure S3Adapter. Arguments don\'t make sense'); } options = fromOptionsDictionaryOrDefault(options, 's3overrides', s3overrides); From 61ae6b027ca680dc5163caafa0743d7d6c9a577d Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:03:46 -0500 Subject: [PATCH 06/11] Remove and add some spaces to make lint happy --- index.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index e3bb0e7..81ed05e 100644 --- a/index.js +++ b/index.js @@ -139,8 +139,8 @@ class S3Adapter { // For a given config object, filename, and data, store a file in S3 // Returns a promise containing the S3 object creation response - async createFile(filename, data, contentType, options = {}, config= {}) { - + async createFile(filename, data, contentType, options = {}, config = {}) { + let key_without_prefix = filename; if (this._generateKey instanceof Function) { try { @@ -149,13 +149,13 @@ class S3Adapter { throw new Error(e); // throw error if generateKey function fails } } - + const params = { Bucket: this._bucket, Key: this._bucketPrefix + key_without_prefix, Body: data, }; - + if (this._fileAcl) { if (this._fileAcl === 'none') { delete params.ACL; @@ -195,8 +195,8 @@ class S3Adapter { return { location: location, // actual upload location, used for tests name: key_without_prefix, // filename in storage, consistent with other adapters - s3_response: response, // raw s3 response - ...url? {url: url} : {} // url (optionally presigned) or non-direct access url + s3_response: response, // raw s3 response + ...url ? { url: url } : {} // url (optionally presigned) or non-direct access url }; } From 87573b9d659f8a02443176ff30d062775b76578d Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:23:25 -0500 Subject: [PATCH 07/11] Allow generate key to access certain createfile traits --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 81ed05e..ef1201b 100644 --- a/index.js +++ b/index.js @@ -144,7 +144,7 @@ class S3Adapter { let key_without_prefix = filename; if (this._generateKey instanceof Function) { try { - key_without_prefix = this._generateKey(filename); + key_without_prefix = this._generateKey(filename, contentType, options); }catch(e){ throw new Error(e); // throw error if generateKey function fails } From 429c9f133fd6933e529c8ee0c4e827b323c351cf Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Sat, 1 Feb 2025 14:12:27 -0500 Subject: [PATCH 08/11] Add tests for url when config is provided and error on generatekey --- spec/test.spec.js | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/spec/test.spec.js b/spec/test.spec.js index a75ff73..3b72ec3 100644 --- a/spec/test.spec.js +++ b/spec/test.spec.js @@ -834,6 +834,46 @@ describe('S3Adapter tests', () => { expect(commandArg).toBeInstanceOf(PutObjectCommand); expect(commandArg.input.ACL).toBeUndefined(); }); + + it('should return url when config is provided', async () => { + const options = { + bucket: 'bucket-1', + presignedUrl: true + }; + const s3 = new S3Adapter(options); + s3._s3Client = s3ClientMock; + + // Mock getFileLocation to return a presigned URL + spyOn(s3, 'getFileLocation').and.returnValue(Promise.resolve('https://presigned-url.com/file.txt')); + + const result = await s3.createFile( + 'file.txt', + 'hello world', + 'text/utf8', + {}, + { mount: 'http://example.com', applicationId: 'test123' } + ); + + expect(result.url).toBe('https://presigned-url.com/file.txt'); + expect(result.location).toBeDefined(); + expect(result.name).toBe('file.txt'); + expect(result.s3_response).toBeDefined(); + }); + + it('should handle generateKey function errors', async () => { + const options = { + bucket: 'bucket-1', + generateKey: () => { + throw new Error('Generate key failed'); + } + }; + const s3 = new S3Adapter(options); + s3._s3Client = s3ClientMock; + + await expectAsync( + s3.createFile('file.txt', 'hello world', 'text/utf8', {}) + ).toBeRejectedWithError('Generate key failed'); + }); }); describe('handleFileStream', () => { From 816c7805cdf6145cfaecd23dc3468750b50d579a Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Sat, 1 Feb 2025 14:21:52 -0500 Subject: [PATCH 09/11] Change spaces and adjust tests --- index.js | 6 +++--- spec/test.spec.js | 25 +++++++++++++------------ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/index.js b/index.js index ef1201b..2201987 100644 --- a/index.js +++ b/index.js @@ -140,7 +140,7 @@ class S3Adapter { // For a given config object, filename, and data, store a file in S3 // Returns a promise containing the S3 object creation response async createFile(filename, data, contentType, options = {}, config = {}) { - + let key_without_prefix = filename; if (this._generateKey instanceof Function) { try { @@ -149,13 +149,13 @@ class S3Adapter { throw new Error(e); // throw error if generateKey function fails } } - + const params = { Bucket: this._bucket, Key: this._bucketPrefix + key_without_prefix, Body: data, }; - + if (this._fileAcl) { if (this._fileAcl === 'none') { delete params.ACL; diff --git a/spec/test.spec.js b/spec/test.spec.js index 3b72ec3..8690a17 100644 --- a/spec/test.spec.js +++ b/spec/test.spec.js @@ -842,34 +842,35 @@ describe('S3Adapter tests', () => { }; const s3 = new S3Adapter(options); s3._s3Client = s3ClientMock; - // Mock getFileLocation to return a presigned URL spyOn(s3, 'getFileLocation').and.returnValue(Promise.resolve('https://presigned-url.com/file.txt')); - + const result = await s3.createFile( - 'file.txt', - 'hello world', - 'text/utf8', + 'file.txt', + 'hello world', + 'text/utf8', {}, { mount: 'http://example.com', applicationId: 'test123' } ); - - expect(result.url).toBe('https://presigned-url.com/file.txt'); - expect(result.location).toBeDefined(); - expect(result.name).toBe('file.txt'); - expect(result.s3_response).toBeDefined(); + + expect(result).toEqual({ + location: jasmine.any(String), + name: 'file.txt', + s3_response: jasmine.any(Object), + url: 'https://presigned-url.com/file.txt' + }); }); it('should handle generateKey function errors', async () => { const options = { bucket: 'bucket-1', generateKey: () => { - throw new Error('Generate key failed'); + throw 'Generate key failed'; } }; const s3 = new S3Adapter(options); s3._s3Client = s3ClientMock; - + await expectAsync( s3.createFile('file.txt', 'hello world', 'text/utf8', {}) ).toBeRejectedWithError('Generate key failed'); From 8c153667059262b89afd1d0f41a5916a8a11beaa Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Sat, 1 Feb 2025 14:31:00 -0500 Subject: [PATCH 10/11] Also mock s3 response for good measure --- spec/test.spec.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/test.spec.js b/spec/test.spec.js index 8690a17..e89c3e2 100644 --- a/spec/test.spec.js +++ b/spec/test.spec.js @@ -841,7 +841,15 @@ describe('S3Adapter tests', () => { presignedUrl: true }; const s3 = new S3Adapter(options); + + const mockS3Response = { + ETag: '"mock-etag"', + VersionId: 'mock-version', + Location: 'mock-location' + }; + s3ClientMock.send.and.returnValue(Promise.resolve(mockS3Response)); s3._s3Client = s3ClientMock; + // Mock getFileLocation to return a presigned URL spyOn(s3, 'getFileLocation').and.returnValue(Promise.resolve('https://presigned-url.com/file.txt')); From bbfcbf52907b88988ea0b97d06963bbda0487f3c Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 6 Feb 2025 20:24:36 -0500 Subject: [PATCH 11/11] Remove another two spaces + change liboptions error back to quotes --- index.js | 2 +- lib/optionsFromArguments.js | 6 +++--- spec/test.spec.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 2201987..024cc69 100644 --- a/index.js +++ b/index.js @@ -155,7 +155,7 @@ class S3Adapter { Key: this._bucketPrefix + key_without_prefix, Body: data, }; - + if (this._fileAcl) { if (this._fileAcl === 'none') { delete params.ACL; diff --git a/lib/optionsFromArguments.js b/lib/optionsFromArguments.js index b05c1fc..f56c9bc 100644 --- a/lib/optionsFromArguments.js +++ b/lib/optionsFromArguments.js @@ -42,12 +42,12 @@ const optionsFromArguments = function optionsFromArguments(args) { } else if (args.length === 2) { options.bucket = stringOrOptions; if (typeof args[1] !== 'object') { - throw new Error('Failed to configure S3Adapter. Arguments don\'t make sense'); + throw new Error("Failed to configure S3Adapter. Arguments don't make sense"); } otherOptions = args[1]; } else if (args.length > 2) { if (typeof args[1] !== 'string' || typeof args[2] !== 'string') { - throw new Error('Failed to configure S3Adapter. Arguments don\'t make sense'); + throw new Error("Failed to configure S3Adapter. Arguments don't make sense"); } options.accessKey = args[0]; options.secretKey = args[1]; @@ -81,7 +81,7 @@ const optionsFromArguments = function optionsFromArguments(args) { options.bucket = s3overrides.params.Bucket; } } else if (args.length > 2) { - throw new Error('Failed to configure S3Adapter. Arguments don\'t make sense'); + throw new Error("Failed to configure S3Adapter. Arguments don't make sense"); } options = fromOptionsDictionaryOrDefault(options, 's3overrides', s3overrides); diff --git a/spec/test.spec.js b/spec/test.spec.js index e89c3e2..1a4c66d 100644 --- a/spec/test.spec.js +++ b/spec/test.spec.js @@ -849,7 +849,7 @@ describe('S3Adapter tests', () => { }; s3ClientMock.send.and.returnValue(Promise.resolve(mockS3Response)); s3._s3Client = s3ClientMock; - + // Mock getFileLocation to return a presigned URL spyOn(s3, 'getFileLocation').and.returnValue(Promise.resolve('https://presigned-url.com/file.txt'));