From 9584bafab51fed19a3d5459b9cbfc362906d1908 Mon Sep 17 00:00:00 2001 From: Vahid Sane Date: Sat, 10 Aug 2024 23:45:14 +0330 Subject: [PATCH 1/7] support async implementation of getFileLocation inside adapters --- spec/FilesController.spec.js | 6 +++--- src/Adapters/Files/FilesAdapter.js | 2 +- src/Controllers/FilesController.js | 24 +++++++++++------------ src/RestQuery.js | 31 ++++++++++++++---------------- src/RestWrite.js | 8 ++++---- src/Routers/FilesRouter.js | 2 +- src/Routers/UsersRouter.js | 4 ++-- 7 files changed, 37 insertions(+), 40 deletions(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index a16451f3ef..0962b69719 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -21,11 +21,11 @@ const mockAdapter = { // Small additional tests to improve overall coverage describe('FilesController', () => { - it('should properly expand objects', done => { + it('should properly expand objects', async done => { const config = Config.get(Parse.applicationId); const gridFSAdapter = new GridFSBucketAdapter('mongodb://localhost:27017/parse'); const filesController = new FilesController(gridFSAdapter); - const result = filesController.expandFilesInObject(config, function () {}); + const result = await filesController.expandFilesInObject(config, function () { }); expect(result).toBeUndefined(); @@ -37,7 +37,7 @@ describe('FilesController', () => { const anObject = { aFile: fullFile, }; - filesController.expandFilesInObject(config, anObject); + await filesController.expandFilesInObject(config, anObject); expect(anObject.aFile.url).toEqual('http://an.url'); done(); diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index afd06942e9..47fa9dcdd0 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -59,7 +59,7 @@ 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 {} 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..f5c1d679bc 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); @@ -1308,7 +1308,7 @@ RestWrite.prototype.handleInstallation = function () { throw new Parse.Error( 132, 'Must specify installationId when deviceToken ' + - 'matches multiple Installation objects' + 'matches multiple Installation objects' ); } else { // Multiple device token matches and we specified an installation ID, @@ -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..493772f9c3 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( @@ -609,7 +609,7 @@ export class UsersRouter extends ClassesRouter { const userString = req.auth && req.auth.user ? req.auth.user.id : undefined; logger.error( `Failed running auth step challenge for ${provider} for user ${userString} with Error: ` + - JSON.stringify(e), + JSON.stringify(e), { authenticationStep: 'challenge', error: e, From 34a517e331839f61269453b650c2ff32e8fa9691 Mon Sep 17 00:00:00 2001 From: Vahid Sane Date: Sun, 11 Aug 2024 01:39:56 +0330 Subject: [PATCH 2/7] fix type of getFileLocation function --- 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 47fa9dcdd0..7378492539 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -61,7 +61,7 @@ export class FilesAdapter { * * @return {string | Promise} Absolute URL */ - getFileLocation(config: Config, filename: string): string {} + getFileLocation(config: Config, filename: string | Promise): string {} /** Validate a filename for this adapter type * From 42baed0daacc55a1c84ac11a62aba8341d461c20 Mon Sep 17 00:00:00 2001 From: Vahid Sane Date: Sun, 11 Aug 2024 01:55:07 +0330 Subject: [PATCH 3/7] hotfix getFileLocation output type --- 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 7378492539..f06c52df89 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -61,7 +61,7 @@ export class FilesAdapter { * * @return {string | Promise} Absolute URL */ - getFileLocation(config: Config, filename: string | Promise): string {} + getFileLocation(config: Config, filename: string): string | Promise {} /** Validate a filename for this adapter type * From 40bcaa39ff9b25dbdbf1e34cad7746a2a0191929 Mon Sep 17 00:00:00 2001 From: Vahid Sane Date: Mon, 12 Aug 2024 00:52:40 +0330 Subject: [PATCH 4/7] fix FilesController tests and migrate to async version --- spec/FilesController.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index 0962b69719..2d191cf163 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -101,7 +101,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 done => { const config = Config.get(Parse.applicationId); const gridFSAdapter = new GridFSBucketAdapter('mongodb://localhost:27017/parse'); spyOn(gridFSAdapter, 'createFile'); @@ -112,7 +112,7 @@ 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( @@ -122,7 +122,7 @@ describe('FilesController', () => { 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 done => { const config = Config.get(Parse.applicationId); const gridFSAdapter = new GridFSBucketAdapter('mongodb://localhost:27017/parse'); spyOn(gridFSAdapter, 'createFile'); @@ -132,7 +132,7 @@ 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); From 98ff4cb2bbbf27c45decb208e2f175029ea2b4c5 Mon Sep 17 00:00:00 2001 From: Vahid Sane Date: Mon, 12 Aug 2024 01:05:09 +0330 Subject: [PATCH 5/7] add sync and async getFileLocation in adapter test --- spec/FilesController.spec.js | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index 2d191cf163..5ca054be25 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -21,9 +21,38 @@ const mockAdapter = { // Small additional tests to improve overall coverage describe('FilesController', () => { - it('should properly expand objects', async done => { + it('should properly expand objects with sync getFileLocation', async done => { 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 = 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'); + + done(); + }); + + it('should properly expand objects with async getFileLocation', async done => { + 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 () { }); From 63eb1716de505712adc4dd652260f32beefc3bf1 Mon Sep 17 00:00:00 2001 From: Vahid Sane Date: Mon, 12 Aug 2024 14:10:54 +0330 Subject: [PATCH 6/7] add more coverage --- spec/FilesController.spec.js | 53 +++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index 5ca054be25..a432ace98f 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -11,8 +11,8 @@ const mockAdapter = { createFile: () => { return Promise.reject(new Error('it failed with xyz')); }, - deleteFile: () => {}, - getFileData: () => {}, + deleteFile: () => { }, + getFileData: () => { }, getFileLocation: () => 'xyz', validateFilename: () => { return null; @@ -21,7 +21,7 @@ const mockAdapter = { // Small additional tests to improve overall coverage describe('FilesController', () => { - it('should properly expand objects with sync getFileLocation', async 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) => { @@ -42,11 +42,9 @@ describe('FilesController', () => { }; await filesController.expandFilesInObject(config, anObject); expect(anObject.aFile.url).toEqual('http://an.url'); - - done(); }); - it('should properly expand objects with async getFileLocation', async 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) => { @@ -68,10 +66,43 @@ describe('FilesController', () => { }; await filesController.expandFilesInObject(config, anObject); expect(anObject.aFile.url).toEqual('http://an.url'); + }); - done(); + 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', @@ -130,7 +161,7 @@ describe('FilesController', () => { done(); }); - it('should add a unique hash to the file name when the preserveFileName option is false', async 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'); @@ -147,11 +178,9 @@ describe('FilesController', () => { 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', async 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'); @@ -165,8 +194,6 @@ describe('FilesController', () => { expect(gridFSAdapter.createFile).toHaveBeenCalledTimes(1); expect(gridFSAdapter.createFile.calls.mostRecent().args[0]).toEqual(fileName); - - done(); }); it('should handle adapter without getMetadata', async () => { From d44f93f9f3d446feb2c55a1ef87e814a29748323 Mon Sep 17 00:00:00 2001 From: Vahid Sane Date: Sat, 17 Aug 2024 01:03:44 +0330 Subject: [PATCH 7/7] revert unwanted changes --- spec/FilesController.spec.js | 4 ++-- src/RestWrite.js | 2 +- src/Routers/UsersRouter.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index a432ace98f..30acf7d13c 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -11,8 +11,8 @@ const mockAdapter = { createFile: () => { return Promise.reject(new Error('it failed with xyz')); }, - deleteFile: () => { }, - getFileData: () => { }, + deleteFile: () => {}, + getFileData: () => {}, getFileLocation: () => 'xyz', validateFilename: () => { return null; diff --git a/src/RestWrite.js b/src/RestWrite.js index f5c1d679bc..4e243e4418 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1308,7 +1308,7 @@ RestWrite.prototype.handleInstallation = function () { throw new Parse.Error( 132, 'Must specify installationId when deviceToken ' + - 'matches multiple Installation objects' + 'matches multiple Installation objects' ); } else { // Multiple device token matches and we specified an installation ID, diff --git a/src/Routers/UsersRouter.js b/src/Routers/UsersRouter.js index 493772f9c3..a8fa208a8b 100644 --- a/src/Routers/UsersRouter.js +++ b/src/Routers/UsersRouter.js @@ -609,7 +609,7 @@ export class UsersRouter extends ClassesRouter { const userString = req.auth && req.auth.user ? req.auth.user.id : undefined; logger.error( `Failed running auth step challenge for ${provider} for user ${userString} with Error: ` + - JSON.stringify(e), + JSON.stringify(e), { authenticationStep: 'challenge', error: e,