From 0b79b20afe58d2568d84fe7f331b372fc5e8415f Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 16 Jan 2025 17:28:25 -0500 Subject: [PATCH 01/18] Allow file adapter to override file name and location + give createFile access to config to allow getFileLocation calls internally File adapter may specify a different filename or location during creation which should be returned after content storage. Example, adapter timestamps upload filenames. getFileLocation may result in a slightly different url than the one used to create the file. In the case where the url returned is identical this is not a problem. Additionally file adapter could call getFile location internally if it wanted to (and should probably have access to config for that reason) --- src/Controllers/FilesController.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Controllers/FilesController.js b/src/Controllers/FilesController.js index a88c527b00..59a3448962 100644 --- a/src/Controllers/FilesController.js +++ b/src/Controllers/FilesController.js @@ -29,11 +29,11 @@ export class FilesController extends AdaptableController { filename = randomHexString(32) + '_' + filename; } - const location = await this.adapter.getFileLocation(config, filename); - await this.adapter.createFile(filename, data, contentType, options); + const defaultLocation = await this.adapter.getFileLocation(config, filename); + const createResult = await this.adapter.createFile(filename, data, contentType, options, config); return { - url: location, - name: filename, + url: createResult?.url || defaultLocation, + name: createResult?.name || filename, } } From 2036369d45ec45863bcecdf212b2f0cc38de97d2 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 16 Jan 2025 18:39:04 -0500 Subject: [PATCH 02/18] Only call getFileLocation if no url is provided but use updated filename in getFileLocation --- src/Controllers/FilesController.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Controllers/FilesController.js b/src/Controllers/FilesController.js index 59a3448962..733b2544cd 100644 --- a/src/Controllers/FilesController.js +++ b/src/Controllers/FilesController.js @@ -29,11 +29,14 @@ export class FilesController extends AdaptableController { filename = randomHexString(32) + '_' + filename; } - const defaultLocation = await this.adapter.getFileLocation(config, filename); const createResult = await this.adapter.createFile(filename, data, contentType, options, config); + filename = createResult?.name || filename; // if createFile returns a new filename, use it + + const url = createResult?.url || await this.adapter.getFileLocation(config, filename); // if createFile returns a new url, use it otherwise get the url from the adapter + return { - url: createResult?.url || defaultLocation, - name: createResult?.name || filename, + url: url, + name: filename, } } From 1df8f1a0c12f4a2ab9e95a8e6d76e3ef188e4a40 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 16 Jan 2025 19:03:50 -0500 Subject: [PATCH 03/18] Modify description and typescript --- src/Adapters/Files/FilesAdapter.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index f06c52df89..4c269620c8 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -30,13 +30,14 @@ export class FilesAdapter { * @param {string} contentType - the supposed contentType * @discussion the contentType can be undefined if the controller was not able to determine it * @param {object} options - (Optional) options to be passed to file adapter (S3 File Adapter Only) + * @param {Config} config -(Optional) server configuration * - tags: object containing key value pairs that will be stored with file * - metadata: object containing key value pairs that will be sotred with file (https://docs.aws.amazon.com/AmazonS3/latest/user-guide/add-object-metadata.html) * @discussion options are not supported by all file adapters. Check the your adapter's documentation for compatibility * - * @return {Promise} a promise that should fail if the storage didn't succeed + * @return {Promise|Promise<{url?: string, name?: string}>} Either a plain promise that should fail if storage didn't succeed, or a promise resolving to an object containing url and/or an updated filename */ - createFile(filename: string, data, contentType: string, options: Object): Promise {} + createFile(filename: string, data, contentType: string, options: Object, config: Config): Promise {} /** Responsible for deleting the specified file * From a9381bc8a09fcdc5cf1b9a3ff75044a4f6df5c4a Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 16 Jan 2025 19:06:12 -0500 Subject: [PATCH 04/18] Add optional location element too --- src/Adapters/Files/FilesAdapter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index 4c269620c8..a0285d95a8 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -35,7 +35,7 @@ export class FilesAdapter { * - metadata: object containing key value pairs that will be sotred with file (https://docs.aws.amazon.com/AmazonS3/latest/user-guide/add-object-metadata.html) * @discussion options are not supported by all file adapters. Check the your adapter's documentation for compatibility * - * @return {Promise|Promise<{url?: string, name?: string}>} Either a plain promise that should fail if storage didn't succeed, or a promise resolving to an object containing url and/or an updated filename + * @return {Promise|Promise<{url?: string, name?: string, location?: string}>} Either a plain promise that should fail if storage didn't succeed, or a promise resolving to an object containing url and/or an updated filename and/or location (if relevant) */ createFile(filename: string, data, contentType: string, options: Object, config: Config): Promise {} From 46b1e14ec20d5a21eb50d13a811f23080b030c17 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 16 Jan 2025 19:17:35 -0500 Subject: [PATCH 05/18] Update FilesAdapter.js --- src/Adapters/Files/FilesAdapter.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index a0285d95a8..6301f6acdf 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -30,10 +30,11 @@ export class FilesAdapter { * @param {string} contentType - the supposed contentType * @discussion the contentType can be undefined if the controller was not able to determine it * @param {object} options - (Optional) options to be passed to file adapter (S3 File Adapter Only) - * @param {Config} config -(Optional) server configuration * - tags: object containing key value pairs that will be stored with file - * - metadata: object containing key value pairs that will be sotred with file (https://docs.aws.amazon.com/AmazonS3/latest/user-guide/add-object-metadata.html) - * @discussion options are not supported by all file adapters. Check the your adapter's documentation for compatibility + * - metadata: object containing key value pairs that will be stored with file (https://docs.aws.amazon.com/AmazonS3/latest/user-guide/add-object-metadata.html) + * @discussion options are not supported by all file adapters. Check the your adapter's documentation for compatibility + * @param {Config} config -(Optional) server configuration + * @discussion config is not supported by all file adapters. Check the your adapter's documentation for compatibility * * @return {Promise|Promise<{url?: string, name?: string, location?: string}>} Either a plain promise that should fail if storage didn't succeed, or a promise resolving to an object containing url and/or an updated filename and/or location (if relevant) */ From 7c4e69fbdc75e96ed2a653ec561a16d722f31d89 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:26:50 -0500 Subject: [PATCH 06/18] Remove trialing space --- src/Adapters/Files/FilesAdapter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index 6301f6acdf..7520e24649 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -32,7 +32,7 @@ export class FilesAdapter { * @param {object} options - (Optional) options to be passed to file adapter (S3 File Adapter Only) * - tags: object containing key value pairs that will be stored with file * - metadata: object containing key value pairs that will be stored with file (https://docs.aws.amazon.com/AmazonS3/latest/user-guide/add-object-metadata.html) - * @discussion options are not supported by all file adapters. Check the your adapter's documentation for compatibility + * @discussion options are not supported by all file adapters. Check the your adapter's documentation for compatibility * @param {Config} config -(Optional) server configuration * @discussion config is not supported by all file adapters. Check the your adapter's documentation for compatibility * From c87b86d3c377d842f1f4aeebcba92ea2d03cc937 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:33:58 -0500 Subject: [PATCH 07/18] change to undefined as a fallback --- src/Adapters/Files/FilesAdapter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index 7520e24649..19846301fb 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -36,7 +36,7 @@ export class FilesAdapter { * @param {Config} config -(Optional) server configuration * @discussion config is not supported by all file adapters. Check the your adapter's documentation for compatibility * - * @return {Promise|Promise<{url?: string, name?: string, location?: string}>} Either a plain promise that should fail if storage didn't succeed, or a promise resolving to an object containing url and/or an updated filename and/or location (if relevant) + * @return {Promise<{url?: string, name?: string, location?: string}>|Promise} Either a plain promise that should fail if storage didn't succeed, or a promise resolving to an object containing url and/or an updated filename and/or location (if relevant) */ createFile(filename: string, data, contentType: string, options: Object, config: Config): Promise {} From ce2c8f93a292d43f88b0de9d2ed2746f2e33785b Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:48:38 -0500 Subject: [PATCH 08/18] Add tests for different return conditions --- spec/FilesController.spec.js | 107 +++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index 30acf7d13c..2792290c58 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -218,4 +218,111 @@ describe('FilesController', () => { expect(gridFSAdapter.validateFilename(fileName)).not.toBe(null); done(); }); + + it('should return valid filename or url from createFile response when provided', async () => { + const config = Config.get(Parse.applicationId); + + // Test case 1: adapter returns new filename and url + const adapterWithReturn = { + createFile: () => { + return Promise.resolve({ + name: 'newfilename.txt', + url: 'http://new.url/newfilename.txt' + }); + }, + getFileLocation: () => { + return Promise.resolve('http://default.url/file.txt'); + }, + validateFilename: () => null + }; + + const controllerWithReturn = new FilesController(adapterWithReturn); + const result1 = await controllerWithReturn.createFile( + config, + 'originalfile.txt', + 'data', + 'text/plain' + ); + + expect(result1.name).toBe('newfilename.txt'); + expect(result1.url).toBe('http://new.url/newfilename.txt'); + + // Test case 2: adapter returns nothing, falling back to default behavior + const adapterWithoutReturn = { + createFile: () => { + return Promise.resolve(); + }, + getFileLocation: (config, filename) => { + return Promise.resolve(`http://default.url/${filename}`); + }, + validateFilename: () => null + }; + + const controllerWithoutReturn = new FilesController(adapterWithoutReturn); + const result2 = await controllerWithoutReturn.createFile( + config, + 'originalfile.txt', + 'data', + 'text/plain', + {}, + { preserveFileName: true } // To make filename predictable + ); + + expect(result2.name).toBe('originalfile.txt'); + expect(result2.url).toBe('http://default.url/originalfile.txt'); + + // Test case 3: adapter returns partial info (only url) + // This is a valid scenario, as the adapter may return a modified filename + // but may result in a mismatch between the filename and the resource URL + const adapterWithOnlyURL = { + createFile: () => { + return Promise.resolve({ + url: 'http://new.url/partialfile.txt' + }); + }, + getFileLocation: () => { + return Promise.resolve('http://default.url/file.txt'); + }, + validateFilename: () => null + }; + + const controllerWithPartial = new FilesController(adapterWithOnlyURL); + const result3 = await controllerWithPartial.createFile( + config, + 'originalfile.txt', + 'data', + 'text/plain', + {}, + { preserveFileName: true } // To make filename predictable + ); + + expect(result3.name).toBe('originalfile.txt'); + expect(result3.url).toBe('http://new.url/partialfile.txt'); // Technically, the resource does not need to match the filename + + // Test case 4: adapter returns only filename + const adapterWithOnlyFilename = { + createFile: () => { + return Promise.resolve({ + name: 'newname.txt' + }); + }, + getFileLocation: (config, filename) => { + return Promise.resolve(`http://default.url/${filename}`); + }, + validateFilename: () => null + }; + + const controllerWithOnlyFilename = new FilesController(adapterWithOnlyFilename); + const result4 = await controllerWithOnlyFilename.createFile( + config, + 'originalfile.txt', + 'data', + 'text/plain', + {}, + { preserveFileName: true } + ); + + expect(result4.name).toBe('newname.txt'); + expect(result4.url).toBe('http://default.url/newname.txt'); + }); }); From 6298c0cbc01fa95eecb84797e6ac98f01c178d87 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:51:59 -0500 Subject: [PATCH 09/18] Update description of config slightly --- src/Adapters/Files/FilesAdapter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index 19846301fb..aaccb49903 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -33,8 +33,8 @@ export class FilesAdapter { * - tags: object containing key value pairs that will be stored with file * - metadata: object containing key value pairs that will be stored with file (https://docs.aws.amazon.com/AmazonS3/latest/user-guide/add-object-metadata.html) * @discussion options are not supported by all file adapters. Check the your adapter's documentation for compatibility - * @param {Config} config -(Optional) server configuration - * @discussion config is not supported by all file adapters. Check the your adapter's documentation for compatibility + * @param {Config} config - (Optional) server configuration + * @discussion config may be passed to adapter to allow for more complex configuration and internal call of getFileLocation (if needed). This argument is not supported by all file adapters. Check the your adapter's documentation for compatibility * * @return {Promise<{url?: string, name?: string, location?: string}>|Promise} Either a plain promise that should fail if storage didn't succeed, or a promise resolving to an object containing url and/or an updated filename and/or location (if relevant) */ From 657dba601bb289e2d370026c8c256be4b668a2b8 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Sat, 1 Feb 2025 14:58:30 -0500 Subject: [PATCH 10/18] Modify mock adapter initialization to use struct in beginning --- spec/FilesController.spec.js | 96 ++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 53 deletions(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index 2792290c58..71453523e3 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -221,101 +221,91 @@ describe('FilesController', () => { it('should return valid filename or url from createFile response when provided', async () => { const config = Config.get(Parse.applicationId); - + // Test case 1: adapter returns new filename and url - const adapterWithReturn = { - createFile: () => { - return Promise.resolve({ - name: 'newfilename.txt', - url: 'http://new.url/newfilename.txt' - }); - }, - getFileLocation: () => { - return Promise.resolve('http://default.url/file.txt'); - }, - validateFilename: () => null + const adapterWithReturn = { ...mockAdapter }; + adapterWithReturn.createFile = () => { + return Promise.resolve({ + name: 'newFilename.txt', + url: 'http://new.url/newFilename.txt' + }); + }; + adapterWithReturn.getFileLocation = () => { + return Promise.resolve('http://default.url/file.txt'); }; - const controllerWithReturn = new FilesController(adapterWithReturn); const result1 = await controllerWithReturn.createFile( config, - 'originalfile.txt', + 'originalFile.txt', 'data', 'text/plain' ); - - expect(result1.name).toBe('newfilename.txt'); - expect(result1.url).toBe('http://new.url/newfilename.txt'); + expect(result1.name).toBe('newFilename.txt'); + expect(result1.url).toBe('http://new.url/newFilename.txt'); // Test case 2: adapter returns nothing, falling back to default behavior - const adapterWithoutReturn = { - createFile: () => { - return Promise.resolve(); - }, - getFileLocation: (config, filename) => { - return Promise.resolve(`http://default.url/${filename}`); - }, - validateFilename: () => null + const adapterWithoutReturn = { ...mockAdapter }; + adapterWithoutReturn.createFile = () => { + return Promise.resolve(); }; - + adapterWithoutReturn.getFileLocation = (config, filename) => { + return Promise.resolve(`http://default.url/${filename}`); + }; + const controllerWithoutReturn = new FilesController(adapterWithoutReturn); const result2 = await controllerWithoutReturn.createFile( config, - 'originalfile.txt', + 'originalFile.txt', 'data', 'text/plain', {}, { preserveFileName: true } // To make filename predictable ); - expect(result2.name).toBe('originalfile.txt'); - expect(result2.url).toBe('http://default.url/originalfile.txt'); + expect(result2.name).toBe('originalFile.txt'); + expect(result2.url).toBe('http://default.url/originalFile.txt'); // Test case 3: adapter returns partial info (only url) // This is a valid scenario, as the adapter may return a modified filename // but may result in a mismatch between the filename and the resource URL - const adapterWithOnlyURL = { - createFile: () => { - return Promise.resolve({ - url: 'http://new.url/partialfile.txt' - }); - }, - getFileLocation: () => { - return Promise.resolve('http://default.url/file.txt'); - }, - validateFilename: () => null + const adapterWithOnlyURL = { ...mockAdapter }; + adapterWithOnlyURL.createFile = () => { + return Promise.resolve({ + url: 'http://new.url/partialFile.txt' + }); + }; + adapterWithOnlyURL.getFileLocation = () => { + return Promise.resolve('http://default.url/file.txt'); }; const controllerWithPartial = new FilesController(adapterWithOnlyURL); const result3 = await controllerWithPartial.createFile( config, - 'originalfile.txt', + 'originalFile.txt', 'data', 'text/plain', {}, { preserveFileName: true } // To make filename predictable ); - expect(result3.name).toBe('originalfile.txt'); - expect(result3.url).toBe('http://new.url/partialfile.txt'); // Technically, the resource does not need to match the filename + expect(result3.name).toBe('originalFile.txt'); + expect(result3.url).toBe('http://new.url/partialFile.txt'); // Technically, the resource does not need to match the filename // Test case 4: adapter returns only filename - const adapterWithOnlyFilename = { - createFile: () => { - return Promise.resolve({ - name: 'newname.txt' - }); - }, - getFileLocation: (config, filename) => { - return Promise.resolve(`http://default.url/${filename}`); - }, - validateFilename: () => null + const adapterWithOnlyFilename = { ...mockAdapter }; + adapterWithOnlyFilename.createFile = () => { + return Promise.resolve({ + name: 'newname.txt' + }); + }; + adapterWithOnlyFilename.getFileLocation = (config, filename) => { + return Promise.resolve(`http://default.url/${filename}`); }; const controllerWithOnlyFilename = new FilesController(adapterWithOnlyFilename); const result4 = await controllerWithOnlyFilename.createFile( config, - 'originalfile.txt', + 'originalFile.txt', 'data', 'text/plain', {}, From 39f48bd432b5c297860f0e977e1992a995d5d98b Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Sat, 1 Feb 2025 15:08:42 -0500 Subject: [PATCH 11/18] Update to include config in arguments for createFileSpy --- spec/CloudCode.spec.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 0b881bef47..588363c1d7 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -3695,11 +3695,13 @@ describe('saveFile hooks', () => { foo: 'bar', }, }; + const config = Config.get('test'); expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), newData, 'text/plain', - newOptions + newOptions, + config ); }); @@ -3727,11 +3729,13 @@ describe('saveFile hooks', () => { foo: 'bar', }, }; + const config = Config.get('test'); expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), newData, newContentType, - newOptions + newOptions, + config ); const expectedFileName = 'donald_duck.pdf'; expect(file._name.indexOf(expectedFileName)).toBe(file._name.length - expectedFileName.length); @@ -3757,11 +3761,13 @@ describe('saveFile hooks', () => { metadata: { foo: 'bar' }, tags: { bar: 'foo' }, }; + const config = Config.get('test'); expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), jasmine.any(Buffer), 'text/plain', - options + options, + config ); }); From 59ed527d797c40194a07b10afabf8fbb6ad98374 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 6 Feb 2025 20:46:14 -0500 Subject: [PATCH 12/18] Remove space + move preserve filename args to files controller initializer --- spec/FilesController.spec.js | 18 ++++++++---------- src/Controllers/FilesController.js | 2 +- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index 71453523e3..b0442e74b5 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -233,7 +233,8 @@ describe('FilesController', () => { adapterWithReturn.getFileLocation = () => { return Promise.resolve('http://default.url/file.txt'); }; - const controllerWithReturn = new FilesController(adapterWithReturn); + const controllerWithReturn = new FilesController(adapterWithReturn, null, { preserveFileName: true }); + // preserveFileName is true to make filename behaviors predictable const result1 = await controllerWithReturn.createFile( config, 'originalFile.txt', @@ -252,14 +253,13 @@ describe('FilesController', () => { return Promise.resolve(`http://default.url/${filename}`); }; - const controllerWithoutReturn = new FilesController(adapterWithoutReturn); + const controllerWithoutReturn = new FilesController(adapterWithoutReturn, null, { preserveFileName: true }); const result2 = await controllerWithoutReturn.createFile( config, 'originalFile.txt', 'data', 'text/plain', - {}, - { preserveFileName: true } // To make filename predictable + {} ); expect(result2.name).toBe('originalFile.txt'); @@ -278,14 +278,13 @@ describe('FilesController', () => { return Promise.resolve('http://default.url/file.txt'); }; - const controllerWithPartial = new FilesController(adapterWithOnlyURL); + const controllerWithPartial = new FilesController(adapterWithOnlyURL, null, { preserveFileName: true }); const result3 = await controllerWithPartial.createFile( config, 'originalFile.txt', 'data', 'text/plain', - {}, - { preserveFileName: true } // To make filename predictable + {} ); expect(result3.name).toBe('originalFile.txt'); @@ -302,14 +301,13 @@ describe('FilesController', () => { return Promise.resolve(`http://default.url/${filename}`); }; - const controllerWithOnlyFilename = new FilesController(adapterWithOnlyFilename); + const controllerWithOnlyFilename = new FilesController(adapterWithOnlyFilename, null, { preserveFileName: true }); const result4 = await controllerWithOnlyFilename.createFile( config, 'originalFile.txt', 'data', 'text/plain', - {}, - { preserveFileName: true } + {} ); expect(result4.name).toBe('newname.txt'); diff --git a/src/Controllers/FilesController.js b/src/Controllers/FilesController.js index 733b2544cd..32924d6b26 100644 --- a/src/Controllers/FilesController.js +++ b/src/Controllers/FilesController.js @@ -33,7 +33,7 @@ export class FilesController extends AdaptableController { filename = createResult?.name || filename; // if createFile returns a new filename, use it const url = createResult?.url || await this.adapter.getFileLocation(config, filename); // if createFile returns a new url, use it otherwise get the url from the adapter - + return { url: url, name: filename, From 3fee23a19a8ced10f51abceca3b6764f98de4790 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Wed, 13 Aug 2025 11:08:19 -0400 Subject: [PATCH 13/18] Split into separate test --- spec/FilesController.spec.js | 55 +++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index b0442e74b5..49c91d98d9 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -219,10 +219,8 @@ describe('FilesController', () => { done(); }); - it('should return valid filename or url from createFile response when provided', async () => { + it('should return filename and url when adapter returns both', async () => { const config = Config.get(Parse.applicationId); - - // Test case 1: adapter returns new filename and url const adapterWithReturn = { ...mockAdapter }; adapterWithReturn.createFile = () => { return Promise.resolve({ @@ -234,17 +232,20 @@ describe('FilesController', () => { return Promise.resolve('http://default.url/file.txt'); }; const controllerWithReturn = new FilesController(adapterWithReturn, null, { preserveFileName: true }); - // preserveFileName is true to make filename behaviors predictable - const result1 = await controllerWithReturn.createFile( + + const result = await controllerWithReturn.createFile( config, 'originalFile.txt', 'data', 'text/plain' ); - expect(result1.name).toBe('newFilename.txt'); - expect(result1.url).toBe('http://new.url/newFilename.txt'); - // Test case 2: adapter returns nothing, falling back to default behavior + expect(result.name).toBe('newFilename.txt'); + expect(result.url).toBe('http://new.url/newFilename.txt'); + }); + + it('should use original filename and generate url when adapter returns nothing', async () => { + const config = Config.get(Parse.applicationId); const adapterWithoutReturn = { ...mockAdapter }; adapterWithoutReturn.createFile = () => { return Promise.resolve(); @@ -254,20 +255,20 @@ describe('FilesController', () => { }; const controllerWithoutReturn = new FilesController(adapterWithoutReturn, null, { preserveFileName: true }); - const result2 = await controllerWithoutReturn.createFile( + const result = await controllerWithoutReturn.createFile( config, 'originalFile.txt', 'data', 'text/plain', {} ); - - expect(result2.name).toBe('originalFile.txt'); - expect(result2.url).toBe('http://default.url/originalFile.txt'); - // Test case 3: adapter returns partial info (only url) - // This is a valid scenario, as the adapter may return a modified filename - // but may result in a mismatch between the filename and the resource URL + expect(result.name).toBe('originalFile.txt'); + expect(result.url).toBe('http://default.url/originalFile.txt'); + }); + + it('should use original filename when adapter returns only url', async () => { + const config = Config.get(Parse.applicationId); const adapterWithOnlyURL = { ...mockAdapter }; adapterWithOnlyURL.createFile = () => { return Promise.resolve({ @@ -277,20 +278,22 @@ describe('FilesController', () => { adapterWithOnlyURL.getFileLocation = () => { return Promise.resolve('http://default.url/file.txt'); }; - + const controllerWithPartial = new FilesController(adapterWithOnlyURL, null, { preserveFileName: true }); - const result3 = await controllerWithPartial.createFile( + const result = await controllerWithPartial.createFile( config, 'originalFile.txt', 'data', 'text/plain', {} ); - - expect(result3.name).toBe('originalFile.txt'); - expect(result3.url).toBe('http://new.url/partialFile.txt'); // Technically, the resource does not need to match the filename - // Test case 4: adapter returns only filename + expect(result.name).toBe('originalFile.txt'); + expect(result.url).toBe('http://new.url/partialFile.txt'); + }); + + it('should use adapter filename and generate url when adapter returns only filename', async () => { + const config = Config.get(Parse.applicationId); const adapterWithOnlyFilename = { ...mockAdapter }; adapterWithOnlyFilename.createFile = () => { return Promise.resolve({ @@ -300,17 +303,17 @@ describe('FilesController', () => { adapterWithOnlyFilename.getFileLocation = (config, filename) => { return Promise.resolve(`http://default.url/${filename}`); }; - + const controllerWithOnlyFilename = new FilesController(adapterWithOnlyFilename, null, { preserveFileName: true }); - const result4 = await controllerWithOnlyFilename.createFile( + const result = await controllerWithOnlyFilename.createFile( config, 'originalFile.txt', 'data', 'text/plain', {} ); - - expect(result4.name).toBe('newname.txt'); - expect(result4.url).toBe('http://default.url/newname.txt'); + + expect(result.name).toBe('newname.txt'); + expect(result.url).toBe('http://default.url/newname.txt'); }); }); From 623d083c7da4e75b5472db7e2e54c28f710d6d2c Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Wed, 13 Aug 2025 12:11:31 -0400 Subject: [PATCH 14/18] Limit the file adapter config to just the minimum it needs to get location Strip down the config variables that get sent along with the file --- spec/CloudCode.spec.js | 21 ++++++++++++++++++--- src/Controllers/FilesController.js | 11 +++++++++-- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 588363c1d7..1ab40f881f 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -3696,12 +3696,17 @@ describe('saveFile hooks', () => { }, }; const config = Config.get('test'); + const expectedConfig = { + applicationId: config.applicationId, + mount: config.mount, + fileKey: config.fileKey + }; expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), newData, 'text/plain', newOptions, - config + expectedConfig ); }); @@ -3730,12 +3735,17 @@ describe('saveFile hooks', () => { }, }; const config = Config.get('test'); + const expectedConfig = { + applicationId: config.applicationId, + mount: config.mount, + fileKey: config.fileKey + }; expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), newData, newContentType, newOptions, - config + expectedConfig ); const expectedFileName = 'donald_duck.pdf'; expect(file._name.indexOf(expectedFileName)).toBe(file._name.length - expectedFileName.length); @@ -3762,12 +3772,17 @@ describe('saveFile hooks', () => { tags: { bar: 'foo' }, }; const config = Config.get('test'); + const expectedConfig = { + applicationId: config.applicationId, + mount: config.mount, + fileKey: config.fileKey + }; expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), jasmine.any(Buffer), 'text/plain', options, - config + expectedConfig ); }); diff --git a/src/Controllers/FilesController.js b/src/Controllers/FilesController.js index 32924d6b26..d2a44bdd92 100644 --- a/src/Controllers/FilesController.js +++ b/src/Controllers/FilesController.js @@ -29,10 +29,17 @@ export class FilesController extends AdaptableController { filename = randomHexString(32) + '_' + filename; } - const createResult = await this.adapter.createFile(filename, data, contentType, options, config); + // Create a clean config object with only the properties needed by file adapters + const adapterConfig = { + applicationId: config.applicationId, + mount: config.mount, + fileKey: config.fileKey + }; + + const createResult = await this.adapter.createFile(filename, data, contentType, options, adapterConfig); filename = createResult?.name || filename; // if createFile returns a new filename, use it - const url = createResult?.url || await this.adapter.getFileLocation(config, filename); // if createFile returns a new url, use it otherwise get the url from the adapter + const url = createResult?.url || await this.adapter.getFileLocation(adapterConfig, filename); // if createFile returns a new url, use it otherwise get the url from the adapter return { url: url, From a0a17821d7ad46ab9891f43ec3a6fe28a7e8a8bf Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Wed, 13 Aug 2025 12:24:04 -0400 Subject: [PATCH 15/18] Match the actual test keys --- spec/CloudCode.spec.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 1ab40f881f..4225ac054a 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -3695,11 +3695,11 @@ describe('saveFile hooks', () => { foo: 'bar', }, }; - const config = Config.get('test'); + // Get the actual config values that will be used const expectedConfig = { - applicationId: config.applicationId, - mount: config.mount, - fileKey: config.fileKey + applicationId: 'test', + mount: 'http://localhost:8378/1', + fileKey: 'test' }; expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), @@ -3734,11 +3734,11 @@ describe('saveFile hooks', () => { foo: 'bar', }, }; - const config = Config.get('test'); + // Get the actual config values that will be used const expectedConfig = { - applicationId: config.applicationId, - mount: config.mount, - fileKey: config.fileKey + applicationId: 'test', + mount: 'http://localhost:8378/1', + fileKey: 'test' }; expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), @@ -3771,11 +3771,11 @@ describe('saveFile hooks', () => { metadata: { foo: 'bar' }, tags: { bar: 'foo' }, }; - const config = Config.get('test'); + // Get the actual config values that will be used const expectedConfig = { - applicationId: config.applicationId, - mount: config.mount, - fileKey: config.fileKey + applicationId: 'test', + mount: 'http://localhost:8378/1', + fileKey: 'test' }; expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), From ff23067d50e6073208e9b8424ff59d96ea2d29f2 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 14 Aug 2025 13:32:39 -0400 Subject: [PATCH 16/18] Change domain to example --- spec/FilesController.spec.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index 49c91d98d9..250e367e1f 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -225,11 +225,11 @@ describe('FilesController', () => { adapterWithReturn.createFile = () => { return Promise.resolve({ name: 'newFilename.txt', - url: 'http://new.url/newFilename.txt' + url: 'http://example.com/newFilename.txt' }); }; adapterWithReturn.getFileLocation = () => { - return Promise.resolve('http://default.url/file.txt'); + return Promise.resolve('http://example.com/file.txt'); }; const controllerWithReturn = new FilesController(adapterWithReturn, null, { preserveFileName: true }); @@ -241,7 +241,7 @@ describe('FilesController', () => { ); expect(result.name).toBe('newFilename.txt'); - expect(result.url).toBe('http://new.url/newFilename.txt'); + expect(result.url).toBe('http://example.com/newFilename.txt'); }); it('should use original filename and generate url when adapter returns nothing', async () => { @@ -251,7 +251,7 @@ describe('FilesController', () => { return Promise.resolve(); }; adapterWithoutReturn.getFileLocation = (config, filename) => { - return Promise.resolve(`http://default.url/${filename}`); + return Promise.resolve(`http://example.com/${filename}`); }; const controllerWithoutReturn = new FilesController(adapterWithoutReturn, null, { preserveFileName: true }); @@ -264,7 +264,7 @@ describe('FilesController', () => { ); expect(result.name).toBe('originalFile.txt'); - expect(result.url).toBe('http://default.url/originalFile.txt'); + expect(result.url).toBe('http://example.com/originalFile.txt'); }); it('should use original filename when adapter returns only url', async () => { @@ -272,11 +272,11 @@ describe('FilesController', () => { const adapterWithOnlyURL = { ...mockAdapter }; adapterWithOnlyURL.createFile = () => { return Promise.resolve({ - url: 'http://new.url/partialFile.txt' + url: 'http://example.com/partialFile.txt' }); }; adapterWithOnlyURL.getFileLocation = () => { - return Promise.resolve('http://default.url/file.txt'); + return Promise.resolve('http://example.com/file.txt'); }; const controllerWithPartial = new FilesController(adapterWithOnlyURL, null, { preserveFileName: true }); @@ -289,7 +289,7 @@ describe('FilesController', () => { ); expect(result.name).toBe('originalFile.txt'); - expect(result.url).toBe('http://new.url/partialFile.txt'); + expect(result.url).toBe('http://example.com/partialFile.txt'); }); it('should use adapter filename and generate url when adapter returns only filename', async () => { @@ -301,7 +301,7 @@ describe('FilesController', () => { }); }; adapterWithOnlyFilename.getFileLocation = (config, filename) => { - return Promise.resolve(`http://default.url/${filename}`); + return Promise.resolve(`http://example.com/${filename}`); }; const controllerWithOnlyFilename = new FilesController(adapterWithOnlyFilename, null, { preserveFileName: true }); @@ -314,6 +314,6 @@ describe('FilesController', () => { ); expect(result.name).toBe('newname.txt'); - expect(result.url).toBe('http://default.url/newname.txt'); + expect(result.url).toBe('http://example.com/newname.txt'); }); }); From 6114fbf482980642012d568be8c956b7a0b5eb5e Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 14 Aug 2025 13:37:18 -0400 Subject: [PATCH 17/18] Use config and stripped down config --- spec/CloudCode.spec.js | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 4225ac054a..d310b6bf10 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -3696,11 +3696,14 @@ describe('saveFile hooks', () => { }, }; // Get the actual config values that will be used + const config = Config.get('test'); + const expectedConfig = { - applicationId: 'test', - mount: 'http://localhost:8378/1', - fileKey: 'test' + applicationId: config.applicationId, + mount: config.mount, + fileKey: config.fileKey }; + expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), newData, @@ -3734,12 +3737,14 @@ describe('saveFile hooks', () => { foo: 'bar', }, }; - // Get the actual config values that will be used + const config = Config.get('test'); + const expectedConfig = { - applicationId: 'test', - mount: 'http://localhost:8378/1', - fileKey: 'test' + applicationId: config.applicationId, + mount: config.mount, + fileKey: config.fileKey }; + expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), newData, @@ -3771,12 +3776,14 @@ describe('saveFile hooks', () => { metadata: { foo: 'bar' }, tags: { bar: 'foo' }, }; - // Get the actual config values that will be used + const config = Config.get('test'); + const expectedConfig = { - applicationId: 'test', - mount: 'http://localhost:8378/1', - fileKey: 'test' + applicationId: config.applicationId, + mount: config.mount, + fileKey: config.fileKey }; + expect(createFileSpy).toHaveBeenCalledWith( jasmine.any(String), jasmine.any(Buffer), From 028103afafd918b359f74432e358046c6bd4c1c3 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 14 Aug 2025 13:47:51 -0400 Subject: [PATCH 18/18] Rename to basicServerConfig --- src/Controllers/FilesController.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Controllers/FilesController.js b/src/Controllers/FilesController.js index d2a44bdd92..fcb545b5b8 100644 --- a/src/Controllers/FilesController.js +++ b/src/Controllers/FilesController.js @@ -30,16 +30,16 @@ export class FilesController extends AdaptableController { } // Create a clean config object with only the properties needed by file adapters - const adapterConfig = { + const basicServerConfig = { applicationId: config.applicationId, mount: config.mount, fileKey: config.fileKey }; - const createResult = await this.adapter.createFile(filename, data, contentType, options, adapterConfig); + const createResult = await this.adapter.createFile(filename, data, contentType, options, basicServerConfig); filename = createResult?.name || filename; // if createFile returns a new filename, use it - const url = createResult?.url || await this.adapter.getFileLocation(adapterConfig, filename); // if createFile returns a new url, use it otherwise get the url from the adapter + const url = createResult?.url || await this.adapter.getFileLocation(basicServerConfig, filename); // if createFile returns a new url, use it otherwise get the url from the adapter return { url: url,