diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 0b881bef47..d310b6bf10 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -3695,11 +3695,21 @@ describe('saveFile hooks', () => { foo: 'bar', }, }; + // Get the actual config values that will be used + 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 + newOptions, + expectedConfig ); }); @@ -3727,11 +3737,20 @@ describe('saveFile hooks', () => { foo: 'bar', }, }; + 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 + newOptions, + expectedConfig ); const expectedFileName = 'donald_duck.pdf'; expect(file._name.indexOf(expectedFileName)).toBe(file._name.length - expectedFileName.length); @@ -3757,11 +3776,20 @@ describe('saveFile hooks', () => { metadata: { foo: 'bar' }, 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 + options, + expectedConfig ); }); diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index 30acf7d13c..250e367e1f 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -218,4 +218,102 @@ describe('FilesController', () => { expect(gridFSAdapter.validateFilename(fileName)).not.toBe(null); done(); }); + + it('should return filename and url when adapter returns both', async () => { + const config = Config.get(Parse.applicationId); + const adapterWithReturn = { ...mockAdapter }; + adapterWithReturn.createFile = () => { + return Promise.resolve({ + name: 'newFilename.txt', + url: 'http://example.com/newFilename.txt' + }); + }; + adapterWithReturn.getFileLocation = () => { + return Promise.resolve('http://example.com/file.txt'); + }; + const controllerWithReturn = new FilesController(adapterWithReturn, null, { preserveFileName: true }); + + const result = await controllerWithReturn.createFile( + config, + 'originalFile.txt', + 'data', + 'text/plain' + ); + + expect(result.name).toBe('newFilename.txt'); + expect(result.url).toBe('http://example.com/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(); + }; + adapterWithoutReturn.getFileLocation = (config, filename) => { + return Promise.resolve(`http://example.com/${filename}`); + }; + + const controllerWithoutReturn = new FilesController(adapterWithoutReturn, null, { preserveFileName: true }); + const result = await controllerWithoutReturn.createFile( + config, + 'originalFile.txt', + 'data', + 'text/plain', + {} + ); + + expect(result.name).toBe('originalFile.txt'); + expect(result.url).toBe('http://example.com/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({ + url: 'http://example.com/partialFile.txt' + }); + }; + adapterWithOnlyURL.getFileLocation = () => { + return Promise.resolve('http://example.com/file.txt'); + }; + + const controllerWithPartial = new FilesController(adapterWithOnlyURL, null, { preserveFileName: true }); + const result = await controllerWithPartial.createFile( + config, + 'originalFile.txt', + 'data', + 'text/plain', + {} + ); + + expect(result.name).toBe('originalFile.txt'); + expect(result.url).toBe('http://example.com/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({ + name: 'newname.txt' + }); + }; + adapterWithOnlyFilename.getFileLocation = (config, filename) => { + return Promise.resolve(`http://example.com/${filename}`); + }; + + const controllerWithOnlyFilename = new FilesController(adapterWithOnlyFilename, null, { preserveFileName: true }); + const result = await controllerWithOnlyFilename.createFile( + config, + 'originalFile.txt', + 'data', + 'text/plain', + {} + ); + + expect(result.name).toBe('newname.txt'); + expect(result.url).toBe('http://example.com/newname.txt'); + }); }); diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index f06c52df89..aaccb49903 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -31,12 +31,14 @@ export class FilesAdapter { * @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) * - 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) + * - 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 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} a promise that should fail if the storage didn't succeed + * @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): Promise {} + createFile(filename: string, data, contentType: string, options: Object, config: Config): Promise {} /** Responsible for deleting the specified file * diff --git a/src/Controllers/FilesController.js b/src/Controllers/FilesController.js index a88c527b00..fcb545b5b8 100644 --- a/src/Controllers/FilesController.js +++ b/src/Controllers/FilesController.js @@ -29,10 +29,20 @@ 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); + // Create a clean config object with only the properties needed by file adapters + const basicServerConfig = { + applicationId: config.applicationId, + mount: config.mount, + fileKey: config.fileKey + }; + + 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(basicServerConfig, filename); // if createFile returns a new url, use it otherwise get the url from the adapter + return { - url: location, + url: url, name: filename, } }