From 1a2da4055abe831b3017172fb75e16d7a8093873 Mon Sep 17 00:00:00 2001 From: Vahid Sane Date: Tue, 27 Aug 2024 18:39:19 +0330 Subject: [PATCH] feat: Add support for asynchronous invocation of `FilesAdapter.getFileLocation` (#9271) --- spec/FilesController.spec.js | 80 +++++++++++++++++++++++++----- src/Adapters/Files/FilesAdapter.js | 4 +- src/Controllers/FilesController.js | 24 ++++----- src/RestQuery.js | 31 ++++++------ src/RestWrite.js | 6 +-- src/Routers/FilesRouter.js | 2 +- src/Routers/UsersRouter.js | 2 +- 7 files changed, 101 insertions(+), 48 deletions(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index a16451f3ef..30acf7d13c 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -21,11 +21,14 @@ const mockAdapter = { // Small additional tests to improve overall coverage describe('FilesController', () => { - it('should properly expand objects', done => { + it('should properly expand objects with sync getFileLocation', async () => { const config = Config.get(Parse.applicationId); const gridFSAdapter = new GridFSBucketAdapter('mongodb://localhost:27017/parse'); + gridFSAdapter.getFileLocation = (config, filename) => { + return config.mount + '/files/' + config.applicationId + '/' + encodeURIComponent(filename); + } const filesController = new FilesController(gridFSAdapter); - const result = filesController.expandFilesInObject(config, function () {}); + const result = await filesController.expandFilesInObject(config, function () { }); expect(result).toBeUndefined(); @@ -37,12 +40,69 @@ describe('FilesController', () => { const anObject = { aFile: fullFile, }; - filesController.expandFilesInObject(config, anObject); + await filesController.expandFilesInObject(config, anObject); expect(anObject.aFile.url).toEqual('http://an.url'); + }); - done(); + it('should properly expand objects with async getFileLocation', async () => { + const config = Config.get(Parse.applicationId); + const gridFSAdapter = new GridFSBucketAdapter('mongodb://localhost:27017/parse'); + gridFSAdapter.getFileLocation = async (config, filename) => { + await Promise.resolve(); + return config.mount + '/files/' + config.applicationId + '/' + encodeURIComponent(filename); + } + const filesController = new FilesController(gridFSAdapter); + const result = await filesController.expandFilesInObject(config, function () { }); + + expect(result).toBeUndefined(); + + const fullFile = { + type: '__type', + url: 'http://an.url', + }; + + const anObject = { + aFile: fullFile, + }; + await filesController.expandFilesInObject(config, anObject); + expect(anObject.aFile.url).toEqual('http://an.url'); + }); + + it('should call getFileLocation when config.fileKey is undefined', async () => { + const config = {}; + const gridFSAdapter = new GridFSBucketAdapter('mongodb://localhost:27017/parse'); + + const fullFile = { + name: 'mock-name', + __type: 'File', + }; + gridFSAdapter.getFileLocation = jasmine.createSpy('getFileLocation').and.returnValue(Promise.resolve('mock-url')); + const filesController = new FilesController(gridFSAdapter); + + const anObject = { aFile: fullFile }; + await filesController.expandFilesInObject(config, anObject); + expect(gridFSAdapter.getFileLocation).toHaveBeenCalledWith(config, fullFile.name); + expect(anObject.aFile.url).toEqual('mock-url'); }); + it('should call getFileLocation when config.fileKey is defined', async () => { + const config = { fileKey: 'mock-key' }; + const gridFSAdapter = new GridFSBucketAdapter('mongodb://localhost:27017/parse'); + + const fullFile = { + name: 'mock-name', + __type: 'File', + }; + gridFSAdapter.getFileLocation = jasmine.createSpy('getFileLocation').and.returnValue(Promise.resolve('mock-url')); + const filesController = new FilesController(gridFSAdapter); + + const anObject = { aFile: fullFile }; + await filesController.expandFilesInObject(config, anObject); + expect(gridFSAdapter.getFileLocation).toHaveBeenCalledWith(config, fullFile.name); + expect(anObject.aFile.url).toEqual('mock-url'); + }); + + it_only_db('mongo')('should pass databaseOptions to GridFSBucketAdapter', async () => { await reconfigureServer({ databaseURI: 'mongodb://localhost:27017/parse', @@ -101,7 +161,7 @@ describe('FilesController', () => { done(); }); - it('should add a unique hash to the file name when the preserveFileName option is false', done => { + it('should add a unique hash to the file name when the preserveFileName option is false', async () => { const config = Config.get(Parse.applicationId); const gridFSAdapter = new GridFSBucketAdapter('mongodb://localhost:27017/parse'); spyOn(gridFSAdapter, 'createFile'); @@ -112,17 +172,15 @@ describe('FilesController', () => { preserveFileName: false, }); - filesController.createFile(config, fileName); + await filesController.createFile(config, fileName); expect(gridFSAdapter.createFile).toHaveBeenCalledTimes(1); expect(gridFSAdapter.createFile.calls.mostRecent().args[0]).toMatch( `^.{32}_${regexEscapedFileName}$` ); - - done(); }); - it('should not add a unique hash to the file name when the preserveFileName option is true', done => { + it('should not add a unique hash to the file name when the preserveFileName option is true', async () => { const config = Config.get(Parse.applicationId); const gridFSAdapter = new GridFSBucketAdapter('mongodb://localhost:27017/parse'); spyOn(gridFSAdapter, 'createFile'); @@ -132,12 +190,10 @@ describe('FilesController', () => { preserveFileName: true, }); - filesController.createFile(config, fileName); + await filesController.createFile(config, fileName); expect(gridFSAdapter.createFile).toHaveBeenCalledTimes(1); expect(gridFSAdapter.createFile.calls.mostRecent().args[0]).toEqual(fileName); - - done(); }); it('should handle adapter without getMetadata', async () => { diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index afd06942e9..f06c52df89 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -59,9 +59,9 @@ export class FilesAdapter { * @param {Config} config - server configuration * @param {string} filename * - * @return {string} Absolute URL + * @return {string | Promise} Absolute URL */ - getFileLocation(config: Config, filename: string): string {} + getFileLocation(config: Config, filename: string): string | Promise {} /** Validate a filename for this adapter type * diff --git a/src/Controllers/FilesController.js b/src/Controllers/FilesController.js index aaff8511fe..b6900af02c 100644 --- a/src/Controllers/FilesController.js +++ b/src/Controllers/FilesController.js @@ -15,7 +15,7 @@ export class FilesController extends AdaptableController { return this.adapter.getFileData(filename); } - createFile(config, filename, data, contentType, options) { + async createFile(config, filename, data, contentType, options) { const extname = path.extname(filename); const hasExtension = extname.length > 0; @@ -30,13 +30,12 @@ export class FilesController extends AdaptableController { filename = randomHexString(32) + '_' + filename; } - const location = this.adapter.getFileLocation(config, filename); - return this.adapter.createFile(filename, data, contentType, options).then(() => { - return Promise.resolve({ - url: location, - name: filename, - }); - }); + const location = await this.adapter.getFileLocation(config, filename); + await this.adapter.createFile(filename, data, contentType, options); + return { + url: location, + name: filename, + } } deleteFile(config, filename) { @@ -55,9 +54,10 @@ export class FilesController extends AdaptableController { * with the current mount point and app id. * Object may be a single object or list of REST-format objects. */ - expandFilesInObject(config, object) { + async expandFilesInObject(config, object) { if (object instanceof Array) { - object.map(obj => this.expandFilesInObject(config, obj)); + const promises = object.map(obj => this.expandFilesInObject(config, obj)); + await Promise.all(promises); return; } if (typeof object !== 'object') { @@ -74,7 +74,7 @@ export class FilesController extends AdaptableController { // all filenames starting with a "-" seperated UUID should be from files.parse.com // all other filenames have been migrated or created from Parse Server if (config.fileKey === undefined) { - fileObject['url'] = this.adapter.getFileLocation(config, filename); + fileObject['url'] = await this.adapter.getFileLocation(config, filename); } else { if (filename.indexOf('tfss-') === 0) { fileObject['url'] = @@ -83,7 +83,7 @@ export class FilesController extends AdaptableController { fileObject['url'] = 'http://files.parse.com/' + config.fileKey + '/' + encodeURIComponent(filename); } else { - fileObject['url'] = this.adapter.getFileLocation(config, filename); + fileObject['url'] = await this.adapter.getFileLocation(config, filename); } } } diff --git a/src/RestQuery.js b/src/RestQuery.js index 57fc435526..621700984b 100644 --- a/src/RestQuery.js +++ b/src/RestQuery.js @@ -735,7 +735,7 @@ _UnsafeRestQuery.prototype.replaceEquality = function () { // Returns a promise for whether it was successful. // Populates this.response with an object that only has 'results'. -_UnsafeRestQuery.prototype.runFind = function (options = {}) { +_UnsafeRestQuery.prototype.runFind = async function (options = {}) { if (this.findOptions.limit === 0) { this.response = { results: [] }; return Promise.resolve(); @@ -749,24 +749,21 @@ _UnsafeRestQuery.prototype.runFind = function (options = {}) { if (options.op) { findOptions.op = options.op; } - return this.config.database - .find(this.className, this.restWhere, findOptions, this.auth) - .then(results => { - if (this.className === '_User' && !findOptions.explain) { - for (var result of results) { - this.cleanResultAuthData(result); - } - } + const results = await this.config.database.find(this.className, this.restWhere, findOptions, this.auth); + if (this.className === '_User' && !findOptions.explain) { + for (var result of results) { + this.cleanResultAuthData(result); + } + } - this.config.filesController.expandFilesInObject(this.config, results); + await this.config.filesController.expandFilesInObject(this.config, results); - if (this.redirectClassName) { - for (var r of results) { - r.className = this.redirectClassName; - } - } - this.response = { results: results }; - }); + if (this.redirectClassName) { + for (var r of results) { + r.className = this.redirectClassName; + } + } + this.response = { results: results }; }; // Returns a promise for whether it was successful. diff --git a/src/RestWrite.js b/src/RestWrite.js index c2d4e1580c..4e243e4418 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -322,7 +322,7 @@ RestWrite.prototype.runBeforeLoginTrigger = async function (userData) { const extraData = { className: this.className }; // Expand file objects - this.config.filesController.expandFilesInObject(this.config, userData); + await this.config.filesController.expandFilesInObject(this.config, userData); const user = triggers.inflate(extraData, userData); @@ -1412,10 +1412,10 @@ RestWrite.prototype.handleInstallation = function () { // If we short-circuited the object response - then we need to make sure we expand all the files, // since this might not have a query, meaning it won't return the full result back. // TODO: (nlutsenko) This should die when we move to per-class based controllers on _Session/_User -RestWrite.prototype.expandFilesForExistingObjects = function () { +RestWrite.prototype.expandFilesForExistingObjects = async function () { // Check whether we have a short-circuited response - only then run expansion. if (this.response && this.response.response) { - this.config.filesController.expandFilesInObject(this.config, this.response.response); + await this.config.filesController.expandFilesInObject(this.config, this.response.response); } }; diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 332cd75748..a13b95e5e4 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -263,7 +263,7 @@ export class FilesRouter { const { filename } = req.params; // run beforeDeleteFile trigger const file = new Parse.File(filename); - file._url = filesController.adapter.getFileLocation(req.config, filename); + file._url = await filesController.adapter.getFileLocation(req.config, filename); const fileObject = { file, fileSize: null }; await triggers.maybeRunFileTrigger( triggers.Types.beforeDelete, diff --git a/src/Routers/UsersRouter.js b/src/Routers/UsersRouter.js index 8af8dc5434..a8fa208a8b 100644 --- a/src/Routers/UsersRouter.js +++ b/src/Routers/UsersRouter.js @@ -264,7 +264,7 @@ export class UsersRouter extends ClassesRouter { // Remove hidden properties. UsersRouter.removeHiddenProperties(user); - req.config.filesController.expandFilesInObject(req.config, user); + await req.config.filesController.expandFilesInObject(req.config, user); // Before login trigger; throws if failure await maybeRunTrigger(