Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for asynchronous invocation of FilesAdapter.getFileLocation #9271

Merged
merged 10 commits into from
Aug 27, 2024
80 changes: 68 additions & 12 deletions spec/FilesController.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () { });
mtrezza marked this conversation as resolved.
Show resolved Hide resolved

expect(result).toBeUndefined();

Expand All @@ -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',
Expand Down Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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 () => {
Expand Down
4 changes: 2 additions & 2 deletions src/Adapters/Files/FilesAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ export class FilesAdapter {
* @param {Config} config - server configuration
* @param {string} filename
*
* @return {string} Absolute URL
* @return {string | Promise<string>} Absolute URL
*/
getFileLocation(config: Config, filename: string): string {}
getFileLocation(config: Config, filename: string): string | Promise<string> {}

/** Validate a filename for this adapter type
*
Expand Down
24 changes: 12 additions & 12 deletions src/Controllers/FilesController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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') {
Expand All @@ -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'] =
Expand All @@ -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);
}
}
}
Expand Down
31 changes: 14 additions & 17 deletions src/RestQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions src/RestWrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/Routers/FilesRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/Routers/UsersRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading