From 7ce52097df962b8f894723a3951564d3186d82dc Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Tue, 7 Jan 2020 14:48:20 -0500 Subject: [PATCH 01/36] added hint to aggregate --- src/Adapters/Storage/Mongo/MongoCollection.js | 4 ++-- src/Adapters/Storage/Mongo/MongoStorageAdapter.js | 4 +++- src/Controllers/DatabaseController.js | 13 ++++++++----- src/RestQuery.js | 5 +++++ src/Routers/AggregateRouter.js | 1 + src/Routers/ClassesRouter.js | 1 + 6 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/Adapters/Storage/Mongo/MongoCollection.js b/src/Adapters/Storage/Mongo/MongoCollection.js index 91c28b407d..1e5466f2e9 100644 --- a/src/Adapters/Storage/Mongo/MongoCollection.js +++ b/src/Adapters/Storage/Mongo/MongoCollection.js @@ -105,9 +105,9 @@ export default class MongoCollection { return this._mongoCollection.distinct(field, query); } - aggregate(pipeline, { maxTimeMS, readPreference } = {}) { + aggregate(pipeline, { maxTimeMS, readPreference, hint } = {}) { return this._mongoCollection - .aggregate(pipeline, { maxTimeMS, readPreference }) + .aggregate(pipeline, { maxTimeMS, readPreference, hint }) .toArray(); } diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 09d6cd319e..43a3a3c338 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -760,7 +760,8 @@ export class MongoStorageAdapter implements StorageAdapter { className: string, schema: any, pipeline: any, - readPreference: ?string + readPreference: ?string, + hint ) { let isPointerField = false; pipeline = pipeline.map(stage => { @@ -791,6 +792,7 @@ export class MongoStorageAdapter implements StorageAdapter { collection.aggregate(pipeline, { readPreference, maxTimeMS: this._maxTimeMS, + hint, }) ) .then(results => { diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 0a643e64ee..9b4e6bad16 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -1289,13 +1289,13 @@ class DatabaseController { distinct, pipeline, readPreference, + hint, }: any = {}, auth: any = {}, validSchemaController: SchemaController.SchemaController ): Promise { const isMaster = acl === undefined; const aclGroup = acl || []; - op = op || (typeof query.objectId == 'string' && Object.keys(query).length === 1 @@ -1406,7 +1406,8 @@ class DatabaseController { className, schema, query, - readPreference + readPreference, + hint ); } } else if (distinct) { @@ -1417,7 +1418,8 @@ class DatabaseController { className, schema, query, - distinct + distinct, + hint ); } } else if (pipeline) { @@ -1428,12 +1430,13 @@ class DatabaseController { className, schema, pipeline, - readPreference + readPreference, + hint ); } } else { return this.adapter - .find(className, schema, query, queryOptions) + .find(className, schema, query, queryOptions, hint) .then(objects => objects.map(object => { object = untransformObjectACL(object); diff --git a/src/RestQuery.js b/src/RestQuery.js index f2225ab3ee..34599e9230 100644 --- a/src/RestQuery.js +++ b/src/RestQuery.js @@ -32,6 +32,8 @@ function RestQuery( this.className = className; this.restWhere = restWhere; this.restOptions = restOptions; + this.hint = this.restOptions.hint; + delete this.restOptions.hint; this.clientSDK = clientSDK; this.runAfterFind = runAfterFind; this.response = null; @@ -661,6 +663,9 @@ RestQuery.prototype.runFind = function(options = {}) { if (options.op) { findOptions.op = options.op; } + if (this.hint) { + findOptions.hint = this.hint; + } return this.config.database .find(this.className, this.restWhere, findOptions, this.auth) .then(results => { diff --git a/src/Routers/AggregateRouter.js b/src/Routers/AggregateRouter.js index 591ebd3469..5a9d07265b 100644 --- a/src/Routers/AggregateRouter.js +++ b/src/Routers/AggregateRouter.js @@ -50,6 +50,7 @@ export class AggregateRouter extends ClassesRouter { if (typeof body.where === 'string') { body.where = JSON.parse(body.where); } + options.hint = body.hint; return rest .find( req.config, diff --git a/src/Routers/ClassesRouter.js b/src/Routers/ClassesRouter.js index 056a8df207..61040cbfdb 100644 --- a/src/Routers/ClassesRouter.js +++ b/src/Routers/ClassesRouter.js @@ -173,6 +173,7 @@ export class ClassesRouter extends PromiseRouter { 'readPreference', 'includeReadPreference', 'subqueryReadPreference', + 'hint', ]; for (const key of Object.keys(body)) { From a8d404ed366fcab4223276eea2ea573b6ccb603a Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Tue, 7 Jan 2020 17:35:09 -0500 Subject: [PATCH 02/36] added support for hint in query --- src/Adapters/Storage/Mongo/MongoCollection.js | 81 +++++++++++++++---- .../Storage/Mongo/MongoStorageAdapter.js | 16 ++-- src/Routers/ClassesRouter.js | 6 ++ src/triggers.js | 4 + 4 files changed, 86 insertions(+), 21 deletions(-) diff --git a/src/Adapters/Storage/Mongo/MongoCollection.js b/src/Adapters/Storage/Mongo/MongoCollection.js index 1e5466f2e9..075c1b1b03 100644 --- a/src/Adapters/Storage/Mongo/MongoCollection.js +++ b/src/Adapters/Storage/Mongo/MongoCollection.js @@ -13,7 +13,10 @@ export default class MongoCollection { // none, then build the geoindex. // This could be improved a lot but it's not clear if that's a good // idea. Or even if this behavior is a good idea. - find(query, { skip, limit, sort, keys, maxTimeMS, readPreference } = {}) { + find( + query, + { skip, limit, sort, keys, maxTimeMS, readPreference, hint } = {} + ) { // Support for Full Text Search - $text if (keys && keys.$score) { delete keys.$score; @@ -26,6 +29,7 @@ export default class MongoCollection { keys, maxTimeMS, readPreference, + hint, }).catch(error => { // Check for "no geoindex" error if ( @@ -54,19 +58,35 @@ export default class MongoCollection { keys, maxTimeMS, readPreference, + hint, }) ) ); }); } - _rawFind(query, { skip, limit, sort, keys, maxTimeMS, readPreference } = {}) { - let findOperation = this._mongoCollection.find(query, { - skip, - limit, - sort, - readPreference, - }); + _rawFind( + query, + { skip, limit, sort, keys, maxTimeMS, readPreference, hint } = {} + ) { + let findOperation; + if (hint) { + findOperation = this._mongoCollection + .find(query, { + skip, + limit, + sort, + readPreference, + }) + .hint(hint); + } else { + findOperation = this._mongoCollection.find(query, { + skip, + limit, + sort, + readPreference, + }); + } if (keys) { findOperation = findOperation.project(keys); @@ -79,15 +99,37 @@ export default class MongoCollection { return findOperation.toArray(); } - count(query, { skip, limit, sort, maxTimeMS, readPreference } = {}) { + count(query, { skip, limit, sort, maxTimeMS, readPreference, hint } = {}) { // If query is empty, then use estimatedDocumentCount instead. // This is due to countDocuments performing a scan, // which greatly increases execution time when being run on large collections. // See https://github.com/Automattic/mongoose/issues/6713 for more info regarding this problem. if (typeof query !== 'object' || !Object.keys(query).length) { - return this._mongoCollection.estimatedDocumentCount({ - maxTimeMS, - }); + if (hint) { + return this._mongoCollection + .estimatedDocumentCount({ + maxTimeMS, + }) + .hint(hint); + } else { + return this._mongoCollection.estimatedDocumentCount({ + maxTimeMS, + }); + } + } + + if (hint) { + const countOperation = this._mongoCollection + .countDocuments(query, { + skip, + limit, + sort, + maxTimeMS, + readPreference, + }) + .hint(hint); + + return countOperation; } const countOperation = this._mongoCollection.countDocuments(query, { @@ -101,14 +143,21 @@ export default class MongoCollection { return countOperation; } - distinct(field, query) { + distinct(field, query, hint) { + if (hint) { + return this._mongoCollection.distinct(field, query).hint; + } + return this._mongoCollection.distinct(field, query); } aggregate(pipeline, { maxTimeMS, readPreference, hint } = {}) { - return this._mongoCollection - .aggregate(pipeline, { maxTimeMS, readPreference, hint }) - .toArray(); + if (hint) { + return this._mongoCollection + .aggregate(pipeline, { maxTimeMS, readPreference }) + .hint(hint) + .toArray(); + } } insertOne(object, session) { diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 43a3a3c338..21fb0322f7 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -620,7 +620,8 @@ export class MongoStorageAdapter implements StorageAdapter { className: string, schema: SchemaType, query: QueryType, - { skip, limit, sort, keys, readPreference }: QueryOptions + { skip, limit, sort, keys, readPreference }: QueryOptions, + hint: ?mixed ): Promise { schema = convertParseSchemaToMongoSchema(schema); const mongoWhere = transformWhere(className, query, schema); @@ -652,6 +653,7 @@ export class MongoStorageAdapter implements StorageAdapter { keys: mongoKeys, maxTimeMS: this._maxTimeMS, readPreference, + hint, }) ) .then(objects => @@ -712,7 +714,8 @@ export class MongoStorageAdapter implements StorageAdapter { className: string, schema: SchemaType, query: QueryType, - readPreference: ?string + readPreference: ?string, + hint: ?mixed ) { schema = convertParseSchemaToMongoSchema(schema); readPreference = this._parseReadPreference(readPreference); @@ -721,6 +724,7 @@ export class MongoStorageAdapter implements StorageAdapter { collection.count(transformWhere(className, query, schema, true), { maxTimeMS: this._maxTimeMS, readPreference, + hint, }) ) .catch(err => this.handleError(err)); @@ -730,7 +734,8 @@ export class MongoStorageAdapter implements StorageAdapter { className: string, schema: SchemaType, query: QueryType, - fieldName: string + fieldName: string, + hint: ?mixed ) { schema = convertParseSchemaToMongoSchema(schema); const isPointerField = @@ -741,7 +746,8 @@ export class MongoStorageAdapter implements StorageAdapter { .then(collection => collection.distinct( transformField, - transformWhere(className, query, schema) + transformWhere(className, query, schema), + hint ) ) .then(objects => { @@ -761,7 +767,7 @@ export class MongoStorageAdapter implements StorageAdapter { schema: any, pipeline: any, readPreference: ?string, - hint + hint: ?mixed ) { let isPointerField = false; pipeline = pipeline.map(stage => { diff --git a/src/Routers/ClassesRouter.js b/src/Routers/ClassesRouter.js index 61040cbfdb..6400b46c54 100644 --- a/src/Routers/ClassesRouter.js +++ b/src/Routers/ClassesRouter.js @@ -220,6 +220,12 @@ export class ClassesRouter extends PromiseRouter { if (typeof body.subqueryReadPreference === 'string') { options.subqueryReadPreference = body.subqueryReadPreference; } + if ( + body.hint && + (typeof body.hint === 'string' || typeof body.hint === 'object') + ) { + options.hint = body.hint; + } return options; } diff --git a/src/triggers.js b/src/triggers.js index b340a4619c..60468e3aff 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -507,6 +507,10 @@ export function maybeRunQueryTrigger( restOptions = restOptions || {}; restOptions.order = jsonQuery.order; } + if (jsonQuery.hint) { + restOptions = restOptions || {}; + restOptions.hint = jsonQuery.hint; + } if (requestObject.readPreference) { restOptions = restOptions || {}; restOptions.readPreference = requestObject.readPreference; From 00a6b275c617d588b623cdf965eb0ee59cb7d55e Mon Sep 17 00:00:00 2001 From: Steve Date: Tue, 7 Jan 2020 21:40:30 -0500 Subject: [PATCH 03/36] added else clause to aggregate --- src/Adapters/Storage/Mongo/MongoCollection.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Adapters/Storage/Mongo/MongoCollection.js b/src/Adapters/Storage/Mongo/MongoCollection.js index 075c1b1b03..4f538447ed 100644 --- a/src/Adapters/Storage/Mongo/MongoCollection.js +++ b/src/Adapters/Storage/Mongo/MongoCollection.js @@ -158,6 +158,9 @@ export default class MongoCollection { .hint(hint) .toArray(); } + return this._mongoCollection + .aggregate(pipeline, { maxTimeMS, readPreference }) + .toArray(); } insertOne(object, session) { From d41d0359839a606691c7b3901afe1290daf769f9 Mon Sep 17 00:00:00 2001 From: Steve Date: Wed, 8 Jan 2020 10:01:39 -0500 Subject: [PATCH 04/36] fixed tests --- spec/ParseQuery.hint.spec.js | 111 +++++++++++++++++++++++++ src/Adapters/Storage/StorageAdapter.js | 13 ++- src/Controllers/DatabaseController.js | 3 +- 3 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 spec/ParseQuery.hint.spec.js diff --git a/spec/ParseQuery.hint.spec.js b/spec/ParseQuery.hint.spec.js new file mode 100644 index 0000000000..218680fe95 --- /dev/null +++ b/spec/ParseQuery.hint.spec.js @@ -0,0 +1,111 @@ +'use strict'; + +const MongoStorageAdapter = require('../lib/Adapters/Storage/Mongo/MongoStorageAdapter') + .default; +const mongoURI = + 'mongodb://localhost:27017/parseServerMongoAdapterTestDatabase'; +const PostgresStorageAdapter = require('../lib/Adapters/Storage/Postgres/PostgresStorageAdapter') + .default; +const postgresURI = + 'postgres://localhost:5432/parse_server_postgres_adapter_test_database'; +const request = require('../lib/request'); +let databaseAdapter; + +const hintHelper = () => { + if (process.env.PARSE_SERVER_TEST_DB === 'postgres') { + if (!databaseAdapter) { + databaseAdapter = new PostgresStorageAdapter({ uri: postgresURI }); + } + } else { + databaseAdapter = new MongoStorageAdapter({ uri: mongoURI }); + } + const subjects = [ + 'coffee', + 'Coffee Shopping', + 'Baking a cake', + 'baking', + 'Café Con Leche', + 'Сырники', + 'coffee and cream', + 'Cafe con Leche', + ]; + const requests = []; + for (const i in subjects) { + const request = { + method: 'POST', + body: { + subject: subjects[i], + comment: subjects[i], + }, + path: '/1/classes/TestObject', + }; + requests.push(request); + } + return reconfigureServer({ + appId: 'test', + restAPIKey: 'test', + publicServerURL: 'http://localhost:8378/1', + databaseAdapter, + }).then(() => { + return request({ + method: 'POST', + url: 'http://localhost:8378/1/batch', + body: { + requests, + }, + headers: { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'test', + 'Content-Type': 'application/json', + }, + }); + }); +}; + +describe('Parse.Query hint testing', () => { + it('should execute query with hint as a string', done => { + hintHelper() + .then(() => { + return request({ + method: 'POST', + url: 'http://localhost:8378/1/classes/TestObject', + body: { where: {}, _method: 'GET', hint: '_id_' }, + headers: { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'test', + 'Content-Type': 'application/json', + }, + }); + }) + .then( + resp => { + expect(resp.data.results.length).toBe(3); + done(); + }, + e => done.fail(e) + ); + }); + + it('should execute query with hint as object', done => { + hintHelper() + .then(() => { + return request({ + method: 'POST', + url: 'http://localhost:8378/1/classes/TestObject', + body: { where: {}, _method: 'GET', hint: { _id_: 1 } }, + headers: { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'test', + 'Content-Type': 'application/json', + }, + }); + }) + .then( + resp => { + expect(resp.data.results.length).toBe(3); + done(); + }, + e => done.fail(e) + ); + }); +}); diff --git a/src/Adapters/Storage/StorageAdapter.js b/src/Adapters/Storage/StorageAdapter.js index 6de3ea3cbd..d748a61dee 100644 --- a/src/Adapters/Storage/StorageAdapter.js +++ b/src/Adapters/Storage/StorageAdapter.js @@ -14,6 +14,7 @@ export type QueryOptions = { distinct?: boolean, pipeline?: any, readPreference?: ?string, + hint?: ?mixed, }; export type UpdateQueryOptions = { @@ -80,7 +81,8 @@ export interface StorageAdapter { className: string, schema: SchemaType, query: QueryType, - options: QueryOptions + options: QueryOptions, + hint?: mixed ): Promise<[any]>; ensureUniqueness( className: string, @@ -92,19 +94,22 @@ export interface StorageAdapter { schema: SchemaType, query: QueryType, readPreference?: string, - estimate?: boolean + estimate?: boolean, + hint?: mixed ): Promise; distinct( className: string, schema: SchemaType, query: QueryType, - fieldName: string + fieldName: string, + hint: ?mixed ): Promise; aggregate( className: string, schema: any, pipeline: any, - readPreference: ?string + readPreference: ?string, + hint: ?mixed ): Promise; performInitialization(options: ?any): Promise; diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 9b4e6bad16..052f964747 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -1407,7 +1407,8 @@ class DatabaseController { schema, query, readPreference, - hint + undefined, + hint, ); } } else if (distinct) { From 288bb2a203c3d820aa3bed8c1be03297ac91f466 Mon Sep 17 00:00:00 2001 From: Steve Date: Wed, 8 Jan 2020 13:09:42 -0500 Subject: [PATCH 05/36] updated tests --- spec/ParseQuery.hint.spec.js | 53 ++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/spec/ParseQuery.hint.spec.js b/spec/ParseQuery.hint.spec.js index 218680fe95..efe72b6e02 100644 --- a/spec/ParseQuery.hint.spec.js +++ b/spec/ParseQuery.hint.spec.js @@ -46,20 +46,43 @@ const hintHelper = () => { restAPIKey: 'test', publicServerURL: 'http://localhost:8378/1', databaseAdapter, - }).then(() => { - return request({ - method: 'POST', - url: 'http://localhost:8378/1/batch', - body: { - requests, - }, - headers: { - 'X-Parse-Application-Id': 'test', - 'X-Parse-REST-API-Key': 'test', - 'Content-Type': 'application/json', - }, + }) + .then(() => { + return request({ + method: 'POST', + url: 'http://localhost:8378/1/schemas/TestObject', + body: { + className: 'TestObject', + fields: { + subject: { + type: 'String', + }, + comment: { + type: 'String', + }, + }, + indexes: { + indexName: { + subject: 1, + }, + }, + }, + }); + }) + .then(() => { + return request({ + method: 'POST', + url: 'http://localhost:8378/1/batch', + body: { + requests, + }, + headers: { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'test', + 'Content-Type': 'application/json', + }, + }); }); - }); }; describe('Parse.Query hint testing', () => { @@ -69,7 +92,7 @@ describe('Parse.Query hint testing', () => { return request({ method: 'POST', url: 'http://localhost:8378/1/classes/TestObject', - body: { where: {}, _method: 'GET', hint: '_id_' }, + body: { where: {}, _method: 'GET', hint: 'subject' }, headers: { 'X-Parse-Application-Id': 'test', 'X-Parse-REST-API-Key': 'test', @@ -92,7 +115,7 @@ describe('Parse.Query hint testing', () => { return request({ method: 'POST', url: 'http://localhost:8378/1/classes/TestObject', - body: { where: {}, _method: 'GET', hint: { _id_: 1 } }, + body: { where: {}, _method: 'GET', hint: { subject: 1 } }, headers: { 'X-Parse-Application-Id': 'test', 'X-Parse-REST-API-Key': 'test', From 0863e847332c00c15035b6729462e9230e980079 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 9 Jan 2020 23:57:51 -0600 Subject: [PATCH 06/36] Add tests and clean up --- spec/ParseQuery.hint.spec.js | 219 ++++++++---------- src/Adapters/Storage/Mongo/MongoCollection.js | 78 ++----- .../Storage/Mongo/MongoStorageAdapter.js | 6 +- src/Adapters/Storage/StorageAdapter.js | 3 +- src/Controllers/DatabaseController.js | 5 +- 5 files changed, 118 insertions(+), 193 deletions(-) diff --git a/spec/ParseQuery.hint.spec.js b/spec/ParseQuery.hint.spec.js index efe72b6e02..1cc90b974d 100644 --- a/spec/ParseQuery.hint.spec.js +++ b/spec/ParseQuery.hint.spec.js @@ -1,134 +1,103 @@ 'use strict'; -const MongoStorageAdapter = require('../lib/Adapters/Storage/Mongo/MongoStorageAdapter') - .default; -const mongoURI = - 'mongodb://localhost:27017/parseServerMongoAdapterTestDatabase'; -const PostgresStorageAdapter = require('../lib/Adapters/Storage/Postgres/PostgresStorageAdapter') - .default; -const postgresURI = - 'postgres://localhost:5432/parse_server_postgres_adapter_test_database'; -const request = require('../lib/request'); -let databaseAdapter; +const Config = require('../lib/Config'); +const TestUtils = require('../lib/TestUtils'); -const hintHelper = () => { - if (process.env.PARSE_SERVER_TEST_DB === 'postgres') { - if (!databaseAdapter) { - databaseAdapter = new PostgresStorageAdapter({ uri: postgresURI }); - } - } else { - databaseAdapter = new MongoStorageAdapter({ uri: mongoURI }); - } - const subjects = [ - 'coffee', - 'Coffee Shopping', - 'Baking a cake', - 'baking', - 'Café Con Leche', - 'Сырники', - 'coffee and cream', - 'Cafe con Leche', - ]; - const requests = []; - for (const i in subjects) { - const request = { - method: 'POST', - body: { - subject: subjects[i], - comment: subjects[i], - }, - path: '/1/classes/TestObject', - }; - requests.push(request); - } - return reconfigureServer({ - appId: 'test', - restAPIKey: 'test', - publicServerURL: 'http://localhost:8378/1', - databaseAdapter, - }) - .then(() => { - return request({ - method: 'POST', - url: 'http://localhost:8378/1/schemas/TestObject', - body: { - className: 'TestObject', - fields: { - subject: { - type: 'String', - }, - comment: { - type: 'String', - }, - }, - indexes: { - indexName: { - subject: 1, - }, - }, - }, - }); - }) - .then(() => { - return request({ - method: 'POST', - url: 'http://localhost:8378/1/batch', - body: { - requests, - }, - headers: { - 'X-Parse-Application-Id': 'test', - 'X-Parse-REST-API-Key': 'test', - 'Content-Type': 'application/json', - }, - }); +let config; + +describe_only_db('mongo')('Parse.Query hint', () => { + beforeEach(() => { + config = Config.get('test'); + }); + + afterEach(async () => { + await config.database.schemaCache.clear(); + await TestUtils.destroyAllDataPermanently(false); + }); + + it('query find with hint string', async () => { + const object = new TestObject(); + await object.save(); + + const collection = await config.database.adapter._adaptiveCollection( + 'TestObject' + ); + let explain = await collection._rawFind( + { _id: object.id }, + { explain: true } + ); + expect(explain.queryPlanner.winningPlan.stage).toBe('IDHACK'); + explain = await collection._rawFind( + { _id: object.id }, + { hint: '_id_', explain: true } + ); + expect(explain.queryPlanner.winningPlan.stage).toBe('FETCH'); + expect(explain.queryPlanner.winningPlan.inputStage.indexName).toBe('_id_'); + }); + + it('query find with hint object', async () => { + const object = new TestObject(); + await object.save(); + + const collection = await config.database.adapter._adaptiveCollection( + 'TestObject' + ); + let explain = await collection._rawFind( + { _id: object.id }, + { explain: true } + ); + expect(explain.queryPlanner.winningPlan.stage).toBe('IDHACK'); + explain = await collection._rawFind( + { _id: object.id }, + { hint: { _id: 1 }, explain: true } + ); + expect(explain.queryPlanner.winningPlan.stage).toBe('FETCH'); + expect(explain.queryPlanner.winningPlan.inputStage.keyPattern).toEqual({ + _id: 1, }); -}; + }); + + it('query aggregate with hint string', async () => { + const object = new TestObject({ foo: 'bar' }); + await object.save(); -describe('Parse.Query hint testing', () => { - it('should execute query with hint as a string', done => { - hintHelper() - .then(() => { - return request({ - method: 'POST', - url: 'http://localhost:8378/1/classes/TestObject', - body: { where: {}, _method: 'GET', hint: 'subject' }, - headers: { - 'X-Parse-Application-Id': 'test', - 'X-Parse-REST-API-Key': 'test', - 'Content-Type': 'application/json', - }, - }); - }) - .then( - resp => { - expect(resp.data.results.length).toBe(3); - done(); - }, - e => done.fail(e) - ); + const collection = await config.database.adapter._adaptiveCollection( + 'TestObject' + ); + let result = await collection.aggregate([{ $group: { _id: '$foo' } }], { + explain: true, + }); + let { queryPlanner } = result[0].stages[0].$cursor; + expect(queryPlanner.winningPlan.stage).toBe('COLLSCAN'); + + result = await collection.aggregate([{ $group: { _id: '$foo' } }], { + hint: '_id_', + explain: true, + }); + queryPlanner = result[0].stages[0].$cursor.queryPlanner; + expect(queryPlanner.winningPlan.stage).toBe('FETCH'); + expect(queryPlanner.winningPlan.inputStage.indexName).toBe('_id_'); }); - it('should execute query with hint as object', done => { - hintHelper() - .then(() => { - return request({ - method: 'POST', - url: 'http://localhost:8378/1/classes/TestObject', - body: { where: {}, _method: 'GET', hint: { subject: 1 } }, - headers: { - 'X-Parse-Application-Id': 'test', - 'X-Parse-REST-API-Key': 'test', - 'Content-Type': 'application/json', - }, - }); - }) - .then( - resp => { - expect(resp.data.results.length).toBe(3); - done(); - }, - e => done.fail(e) - ); + it('query aggregate with hint object', async () => { + const object = new TestObject({ foo: 'bar' }); + await object.save(); + + const collection = await config.database.adapter._adaptiveCollection( + 'TestObject' + ); + let result = await collection.aggregate([{ $group: { _id: '$foo' } }], { + explain: true, + }); + let { queryPlanner } = result[0].stages[0].$cursor; + expect(queryPlanner.winningPlan.stage).toBe('COLLSCAN'); + + result = await collection.aggregate([{ $group: { _id: '$foo' } }], { + hint: { _id: 1 }, + explain: true, + }); + queryPlanner = result[0].stages[0].$cursor.queryPlanner; + expect(queryPlanner.winningPlan.stage).toBe('FETCH'); + expect(queryPlanner.winningPlan.inputStage.keyPattern).toEqual({ _id: 1 }); }); }); diff --git a/src/Adapters/Storage/Mongo/MongoCollection.js b/src/Adapters/Storage/Mongo/MongoCollection.js index 4f538447ed..7cabc522cd 100644 --- a/src/Adapters/Storage/Mongo/MongoCollection.js +++ b/src/Adapters/Storage/Mongo/MongoCollection.js @@ -15,7 +15,7 @@ export default class MongoCollection { // idea. Or even if this behavior is a good idea. find( query, - { skip, limit, sort, keys, maxTimeMS, readPreference, hint } = {} + { skip, limit, sort, keys, maxTimeMS, readPreference, hint, explain } = {} ) { // Support for Full Text Search - $text if (keys && keys.$score) { @@ -30,6 +30,7 @@ export default class MongoCollection { maxTimeMS, readPreference, hint, + explain, }).catch(error => { // Check for "no geoindex" error if ( @@ -59,6 +60,7 @@ export default class MongoCollection { maxTimeMS, readPreference, hint, + explain, }) ) ); @@ -67,26 +69,15 @@ export default class MongoCollection { _rawFind( query, - { skip, limit, sort, keys, maxTimeMS, readPreference, hint } = {} + { skip, limit, sort, keys, maxTimeMS, readPreference, hint, explain } = {} ) { - let findOperation; - if (hint) { - findOperation = this._mongoCollection - .find(query, { - skip, - limit, - sort, - readPreference, - }) - .hint(hint); - } else { - findOperation = this._mongoCollection.find(query, { - skip, - limit, - sort, - readPreference, - }); - } + let findOperation = this._mongoCollection.find(query, { + skip, + limit, + sort, + readPreference, + hint, + }); if (keys) { findOperation = findOperation.project(keys); @@ -96,7 +87,7 @@ export default class MongoCollection { findOperation = findOperation.maxTimeMS(maxTimeMS); } - return findOperation.toArray(); + return explain ? findOperation.explain(explain) : findOperation.toArray(); } count(query, { skip, limit, sort, maxTimeMS, readPreference, hint } = {}) { @@ -105,31 +96,9 @@ export default class MongoCollection { // which greatly increases execution time when being run on large collections. // See https://github.com/Automattic/mongoose/issues/6713 for more info regarding this problem. if (typeof query !== 'object' || !Object.keys(query).length) { - if (hint) { - return this._mongoCollection - .estimatedDocumentCount({ - maxTimeMS, - }) - .hint(hint); - } else { - return this._mongoCollection.estimatedDocumentCount({ - maxTimeMS, - }); - } - } - - if (hint) { - const countOperation = this._mongoCollection - .countDocuments(query, { - skip, - limit, - sort, - maxTimeMS, - readPreference, - }) - .hint(hint); - - return countOperation; + return this._mongoCollection.estimatedDocumentCount({ + maxTimeMS, + }); } const countOperation = this._mongoCollection.countDocuments(query, { @@ -138,28 +107,19 @@ export default class MongoCollection { sort, maxTimeMS, readPreference, + hint, }); return countOperation; } - distinct(field, query, hint) { - if (hint) { - return this._mongoCollection.distinct(field, query).hint; - } - + distinct(field, query) { return this._mongoCollection.distinct(field, query); } - aggregate(pipeline, { maxTimeMS, readPreference, hint } = {}) { - if (hint) { - return this._mongoCollection - .aggregate(pipeline, { maxTimeMS, readPreference }) - .hint(hint) - .toArray(); - } + aggregate(pipeline, { maxTimeMS, readPreference, hint, explain } = {}) { return this._mongoCollection - .aggregate(pipeline, { maxTimeMS, readPreference }) + .aggregate(pipeline, { maxTimeMS, readPreference, hint, explain }) .toArray(); } diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 21fb0322f7..6bc58336c7 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -734,8 +734,7 @@ export class MongoStorageAdapter implements StorageAdapter { className: string, schema: SchemaType, query: QueryType, - fieldName: string, - hint: ?mixed + fieldName: string ) { schema = convertParseSchemaToMongoSchema(schema); const isPointerField = @@ -746,8 +745,7 @@ export class MongoStorageAdapter implements StorageAdapter { .then(collection => collection.distinct( transformField, - transformWhere(className, query, schema), - hint + transformWhere(className, query, schema) ) ) .then(objects => { diff --git a/src/Adapters/Storage/StorageAdapter.js b/src/Adapters/Storage/StorageAdapter.js index d748a61dee..a693cd9d1b 100644 --- a/src/Adapters/Storage/StorageAdapter.js +++ b/src/Adapters/Storage/StorageAdapter.js @@ -101,8 +101,7 @@ export interface StorageAdapter { className: string, schema: SchemaType, query: QueryType, - fieldName: string, - hint: ?mixed + fieldName: string ): Promise; aggregate( className: string, diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 052f964747..1e3eb2451c 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -1408,7 +1408,7 @@ class DatabaseController { query, readPreference, undefined, - hint, + hint ); } } else if (distinct) { @@ -1419,8 +1419,7 @@ class DatabaseController { className, schema, query, - distinct, - hint + distinct ); } } else if (pipeline) { From 7d0c4e2186b0dffb49e747159e22edab0fbb1dc2 Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Mon, 13 Jan 2020 17:52:19 -0500 Subject: [PATCH 07/36] added beforeSaveFile and afterSaveFile triggers --- src/Routers/FilesRouter.js | 46 ++++++++++++++++++++--- src/cloud-code/Parse.Cloud.js | 70 +++++++++++++++++++++++++++++++++++ src/triggers.js | 57 ++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 5 deletions(-) diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 56fb5a3cc8..3155ba9fcc 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -5,6 +5,7 @@ import Parse from 'parse/node'; import Config from '../Config'; import mime from 'mime'; import logger from '../logger'; +const triggers = require('../triggers'); export class FilesRouter { expressRouter({ maxUploadSize = '20Mb' } = {}) { @@ -73,6 +74,7 @@ export class FilesRouter { const filesController = config.filesController; const filename = req.params.filename; const contentType = req.get('Content-type'); + const contentLength = req.get('Content-Length'); if (!req.body || !req.body.length) { next( @@ -87,12 +89,46 @@ export class FilesRouter { return; } - filesController - .createFile(config, filename, req.body, contentType) + const fileObject = { + filename, + contentType, + contentLength, + data: req.body, + }; + triggers + .maybeRunFileTrigger( + triggers.Types.beforeSaveFile, + fileObject, + config, + req.auth + ) .then(result => { - res.status(201); - res.set('Location', result.url); - res.json(result); + fileObject.filename = result.filename || fileObject.filename; + fileObject.contentType = result.contentType || fileObject.contentType; + fileObject.contentLength = + result.contentLength || fileObject.contentLength; + fileObject.data = result.data || fileObject.data; + return filesController.createFile( + config, + fileObject.filename, + fileObject.data, + fileObject.contentType + ); + }) + .then(result => { + fileObject.url = result.url; + return triggers + .maybeRunFileTrigger( + triggers.Types.afterSaveFile, + fileObject, + config, + req.auth + ) + .then(() => { + res.status(201); + res.set('Location', result.url); + res.json(result); + }); }) .catch(e => { logger.error('Error creating a file: ', e); diff --git a/src/cloud-code/Parse.Cloud.js b/src/cloud-code/Parse.Cloud.js index 5ad6769d3f..617cab51be 100644 --- a/src/cloud-code/Parse.Cloud.js +++ b/src/cloud-code/Parse.Cloud.js @@ -325,6 +325,52 @@ ParseCloud.afterFind = function(parseClass, handler) { ); }; +/** + * Registers an before save file function. + * + * **Available in Cloud Code only.** + * + * ``` + * Parse.Cloud.beforeSaveFile(async (request) => { + * // code here + * }) + *``` + * + * @method beforeSaveFile + * @name Parse.Cloud.beforeSaveFile + * @param {Function} func The function to run before saving a file. This function can be async and should take just one parameter, {@link Parse.Cloud.BeforeSaveFileRequest}. + */ +ParseCloud.beforeSaveFile = function(handler) { + triggers.addFileTrigger( + triggers.Types.beforeSaveFile, + handler, + Parse.applicationId + ); +}; + +/** + * Registers an after save file function. + * + * **Available in Cloud Code only.** + * + * ``` + * Parse.Cloud.afterSaveFile(async (request) => { + * // code here + * }) + *``` + * + * @method afterSaveFile + * @name Parse.Cloud.afterSaveFile + * @param {Function} func The function to run after saving a file. This function can be async and should take just one parameter, {@link Parse.Cloud.AfterSaveFileRequest}. + */ +ParseCloud.afterSaveFile = function(handler) { + triggers.addFileTrigger( + triggers.Types.afterSaveFile, + handler, + Parse.applicationId + ); +}; + ParseCloud.onLiveQueryEvent = function(handler) { triggers.addLiveQueryEventHandler(handler, Parse.applicationId); }; @@ -357,6 +403,30 @@ module.exports = ParseCloud; * @property {Parse.Object} original If set, the object, as currently stored. */ +/** + * @interface Parse.Cloud.BeforeSaveFileRequest + * @property {String} installationId If set, the installationId triggering the request. + * @property {Boolean} master If true, means the master key was used. + * @property {Parse.User} user If set, the user that made the request. + * @property {Object} fileObject The object triggering the hook. + * @property {String} ip The IP address of the client making the request. + * @property {Object} headers The original HTTP headers for the request. + * @property {String} triggerName The name of the trigger (`beforeSaveFile`, `afterSaveFile`) + * @property {Object} log The current logger inside Parse Server. + */ + +/** + * @interface Parse.Cloud.AfterSaveFileRequest + * @property {String} installationId If set, the installationId triggering the request. + * @property {Boolean} master If true, means the master key was used. + * @property {Parse.User} user If set, the user that made the request. + * @property {Object} fileObject The object triggering the hook. + * @property {String} ip The IP address of the client making the request. + * @property {Object} headers The original HTTP headers for the request. + * @property {String} triggerName The name of the trigger (`beforeSaveFile`, `afterSaveFile`) + * @property {Object} log The current logger inside Parse Server. + */ + /** * @interface Parse.Cloud.BeforeFindRequest * @property {String} installationId If set, the installationId triggering the request. diff --git a/src/triggers.js b/src/triggers.js index 60468e3aff..841aee76e0 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -11,8 +11,12 @@ export const Types = { afterDelete: 'afterDelete', beforeFind: 'beforeFind', afterFind: 'afterFind', + beforeSaveFile: 'beforeSaveFile', + afterSaveFile: 'afterSaveFile', }; +const FileClassName = '@File'; + const baseStore = function() { const Validators = {}; const Functions = {}; @@ -118,6 +122,10 @@ export function addTrigger(type, className, handler, applicationId) { add(Category.Triggers, `${type}.${className}`, handler, applicationId); } +export function addFileTrigger(type, handler, applicationId) { + add(Category.Triggers, `${type}.${FileClassName}`, handler, applicationId); +} + export function addLiveQueryEventHandler(handler, applicationId) { applicationId = applicationId || Parse.applicationId; _triggerStore[applicationId] = _triggerStore[applicationId] || baseStore(); @@ -143,6 +151,10 @@ export function getTrigger(className, triggerType, applicationId) { return get(Category.Triggers, `${triggerType}.${className}`, applicationId); } +export function getFileTrigger(type, applicationId) { + return getTrigger(FileClassName, type, applicationId); +} + export function triggerExists( className: string, type: string, @@ -663,3 +675,48 @@ export function runLiveQueryEventHandlers( } _triggerStore[applicationId].LiveQuery.forEach(handler => handler(data)); } + +export function getRequestFileObject(triggerType, auth, fileObject, config) { + const request = { + triggerName: triggerType, + fileObject, + master: false, + log: config.loggerController, + headers: config.headers, + ip: config.ip, + }; + + if (!auth) { + return request; + } + if (auth.isMaster) { + request['master'] = true; + } + if (auth.user) { + request['user'] = auth.user; + } + if (auth.installationId) { + request['installationId'] = auth.installationId; + } + return request; +} + +export function maybeRunFileTrigger(triggerType, fileObject, config, auth) { + const fileTrigger = getFileTrigger(triggerType, config.applicationId); + if (typeof fileTrigger === 'function') { + return Promise.resolve() + .then(() => { + const request = getRequestFileObject( + triggerType, + auth, + fileObject, + config + ); + return fileTrigger(request); + }) + .then(result => { + return result || fileObject; + }); + } + return Promise.resolve(fileObject); +} From eecc3ffb2e1c29336f99e0886196913526c91eb0 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Mon, 13 Jan 2020 18:16:22 -0600 Subject: [PATCH 08/36] Add support for explain --- spec/ParseQuery.hint.spec.js | 67 +++++++++++++++++++ .../Storage/Mongo/MongoStorageAdapter.js | 19 ++++-- src/Adapters/Storage/StorageAdapter.js | 7 +- src/Controllers/DatabaseController.js | 23 ++++++- src/RestQuery.js | 7 +- src/Routers/AggregateRouter.js | 12 +++- src/Routers/ClassesRouter.js | 4 ++ src/triggers.js | 4 ++ 8 files changed, 122 insertions(+), 21 deletions(-) diff --git a/spec/ParseQuery.hint.spec.js b/spec/ParseQuery.hint.spec.js index 1cc90b974d..43f1c4e9a7 100644 --- a/spec/ParseQuery.hint.spec.js +++ b/spec/ParseQuery.hint.spec.js @@ -2,9 +2,22 @@ const Config = require('../lib/Config'); const TestUtils = require('../lib/TestUtils'); +const request = require('../lib/request'); let config; +const masterKeyHeaders = { + 'X-Parse-Application-Id': 'test', + 'X-Parse-Rest-API-Key': 'rest', + 'X-Parse-Master-Key': 'test', + 'Content-Type': 'application/json', +}; + +const masterKeyOptions = { + headers: masterKeyHeaders, + json: true, +}; + describe_only_db('mongo')('Parse.Query hint', () => { beforeEach(() => { config = Config.get('test'); @@ -100,4 +113,58 @@ describe_only_db('mongo')('Parse.Query hint', () => { expect(queryPlanner.winningPlan.stage).toBe('FETCH'); expect(queryPlanner.winningPlan.inputStage.keyPattern).toEqual({ _id: 1 }); }); + + it('query find with hint (rest)', async () => { + const object = new TestObject(); + await object.save(); + let options = Object.assign({}, masterKeyOptions, { + url: Parse.serverURL + '/classes/TestObject', + qs: { + explain: true, + }, + }); + let response = await request(options); + let explain = response.data.results; + expect(explain.queryPlanner.winningPlan.inputStage.stage).toBe('COLLSCAN'); + + options = Object.assign({}, masterKeyOptions, { + url: Parse.serverURL + '/classes/TestObject', + qs: { + explain: true, + hint: '_id_', + }, + }); + response = await request(options); + explain = response.data.results; + expect( + explain.queryPlanner.winningPlan.inputStage.inputStage.indexName + ).toBe('_id_'); + }); + + it('query aggregate with hint (rest)', async () => { + const object = new TestObject({ foo: 'bar' }); + await object.save(); + let options = Object.assign({}, masterKeyOptions, { + url: Parse.serverURL + '/aggregate/TestObject', + qs: { + explain: true, + group: JSON.stringify({ objectId: '$foo' }), + }, + }); + let response = await request(options); + let { queryPlanner } = response.data.results[0].stages[0].$cursor; + expect(queryPlanner.winningPlan.stage).toBe('COLLSCAN'); + + options = Object.assign({}, masterKeyOptions, { + url: Parse.serverURL + '/aggregate/TestObject', + qs: { + explain: true, + hint: '_id_', + group: JSON.stringify({ objectId: '$foo' }), + }, + }); + response = await request(options); + queryPlanner = response.data.results[0].stages[0].$cursor.queryPlanner; + expect(queryPlanner.winningPlan.inputStage.keyPattern).toEqual({ _id: 1 }); + }); }); diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 6bc58336c7..e60551bdb0 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -620,8 +620,7 @@ export class MongoStorageAdapter implements StorageAdapter { className: string, schema: SchemaType, query: QueryType, - { skip, limit, sort, keys, readPreference }: QueryOptions, - hint: ?mixed + { skip, limit, sort, keys, readPreference, hint, explain }: QueryOptions ): Promise { schema = convertParseSchemaToMongoSchema(schema); const mongoWhere = transformWhere(className, query, schema); @@ -654,13 +653,17 @@ export class MongoStorageAdapter implements StorageAdapter { maxTimeMS: this._maxTimeMS, readPreference, hint, + explain, }) ) - .then(objects => - objects.map(object => + .then(objects => { + if (explain) { + return objects; + } + return objects.map(object => mongoObjectToParseObject(className, object, schema) - ) - ) + ); + }) .catch(err => this.handleError(err)); } @@ -765,7 +768,8 @@ export class MongoStorageAdapter implements StorageAdapter { schema: any, pipeline: any, readPreference: ?string, - hint: ?mixed + hint: ?mixed, + explain?: boolean ) { let isPointerField = false; pipeline = pipeline.map(stage => { @@ -797,6 +801,7 @@ export class MongoStorageAdapter implements StorageAdapter { readPreference, maxTimeMS: this._maxTimeMS, hint, + explain, }) ) .then(results => { diff --git a/src/Adapters/Storage/StorageAdapter.js b/src/Adapters/Storage/StorageAdapter.js index a693cd9d1b..ce134493bf 100644 --- a/src/Adapters/Storage/StorageAdapter.js +++ b/src/Adapters/Storage/StorageAdapter.js @@ -15,6 +15,7 @@ export type QueryOptions = { pipeline?: any, readPreference?: ?string, hint?: ?mixed, + explain?: Boolean, }; export type UpdateQueryOptions = { @@ -81,8 +82,7 @@ export interface StorageAdapter { className: string, schema: SchemaType, query: QueryType, - options: QueryOptions, - hint?: mixed + options: QueryOptions ): Promise<[any]>; ensureUniqueness( className: string, @@ -108,7 +108,8 @@ export interface StorageAdapter { schema: any, pipeline: any, readPreference: ?string, - hint: ?mixed + hint: ?mixed, + explain?: boolean ): Promise; performInitialization(options: ?any): Promise; diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 1e3eb2451c..81e32aff62 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -1290,6 +1290,7 @@ class DatabaseController { pipeline, readPreference, hint, + explain, }: any = {}, auth: any = {}, validSchemaController: SchemaController.SchemaController @@ -1333,7 +1334,15 @@ class DatabaseController { sort.updatedAt = sort._updated_at; delete sort._updated_at; } - const queryOptions = { skip, limit, sort, keys, readPreference }; + const queryOptions = { + skip, + limit, + sort, + keys, + readPreference, + hint, + explain, + }; Object.keys(sort).forEach(fieldName => { if (fieldName.match(/^authData\.([a-zA-Z0-9_]+)\.id$/)) { throw new Parse.Error( @@ -1431,12 +1440,20 @@ class DatabaseController { schema, pipeline, readPreference, - hint + hint, + explain ); } + } else if (explain) { + return this.adapter.find( + className, + schema, + query, + queryOptions + ); } else { return this.adapter - .find(className, schema, query, queryOptions, hint) + .find(className, schema, query, queryOptions) .then(objects => objects.map(object => { object = untransformObjectACL(object); diff --git a/src/RestQuery.js b/src/RestQuery.js index 34599e9230..468446561c 100644 --- a/src/RestQuery.js +++ b/src/RestQuery.js @@ -32,8 +32,6 @@ function RestQuery( this.className = className; this.restWhere = restWhere; this.restOptions = restOptions; - this.hint = this.restOptions.hint; - delete this.restOptions.hint; this.clientSDK = clientSDK; this.runAfterFind = runAfterFind; this.response = null; @@ -120,6 +118,8 @@ function RestQuery( case 'includeAll': this.includeAll = true; break; + case 'explain': + case 'hint': case 'distinct': case 'pipeline': case 'skip': @@ -663,9 +663,6 @@ RestQuery.prototype.runFind = function(options = {}) { if (options.op) { findOptions.op = options.op; } - if (this.hint) { - findOptions.hint = this.hint; - } return this.config.database .find(this.className, this.restWhere, findOptions, this.auth) .then(results => { diff --git a/src/Routers/AggregateRouter.js b/src/Routers/AggregateRouter.js index 5a9d07265b..94544282e2 100644 --- a/src/Routers/AggregateRouter.js +++ b/src/Routers/AggregateRouter.js @@ -4,7 +4,7 @@ import * as middleware from '../middlewares'; import Parse from 'parse/node'; import UsersRouter from './UsersRouter'; -const BASE_KEYS = ['where', 'distinct', 'pipeline']; +const BASE_KEYS = ['where', 'distinct', 'pipeline', 'hint', 'explain']; const PIPELINE_KEYS = [ 'addFields', @@ -46,11 +46,18 @@ export class AggregateRouter extends ClassesRouter { if (body.distinct) { options.distinct = String(body.distinct); } + if (body.hint) { + options.hint = body.hint; + delete body.hint; + } + if (body.explain) { + options.explain = body.explain; + delete body.explain; + } options.pipeline = AggregateRouter.getPipeline(body); if (typeof body.where === 'string') { body.where = JSON.parse(body.where); } - options.hint = body.hint; return rest .find( req.config, @@ -97,7 +104,6 @@ export class AggregateRouter extends ClassesRouter { */ static getPipeline(body) { let pipeline = body.pipeline || body; - if (!Array.isArray(pipeline)) { pipeline = Object.keys(pipeline).map(key => { return { [key]: pipeline[key] }; diff --git a/src/Routers/ClassesRouter.js b/src/Routers/ClassesRouter.js index 6400b46c54..0cbc8d215d 100644 --- a/src/Routers/ClassesRouter.js +++ b/src/Routers/ClassesRouter.js @@ -174,6 +174,7 @@ export class ClassesRouter extends PromiseRouter { 'includeReadPreference', 'subqueryReadPreference', 'hint', + 'explain', ]; for (const key of Object.keys(body)) { @@ -226,6 +227,9 @@ export class ClassesRouter extends PromiseRouter { ) { options.hint = body.hint; } + if (body.explain) { + options.explain = body.explain; + } return options; } diff --git a/src/triggers.js b/src/triggers.js index 60468e3aff..05ee7c278f 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -499,6 +499,10 @@ export function maybeRunQueryTrigger( restOptions = restOptions || {}; restOptions.excludeKeys = jsonQuery.excludeKeys; } + if (jsonQuery.explain) { + restOptions = restOptions || {}; + restOptions.explain = jsonQuery.explain; + } if (jsonQuery.keys) { restOptions = restOptions || {}; restOptions.keys = jsonQuery.keys; From c22ef49ca37440e84f7062725d1d8d623363ad04 Mon Sep 17 00:00:00 2001 From: Steve Date: Tue, 14 Jan 2020 21:12:29 -0500 Subject: [PATCH 09/36] added some validation --- src/Routers/FilesRouter.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 3155ba9fcc..3c6c44a848 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -103,11 +103,13 @@ export class FilesRouter { req.auth ) .then(result => { - fileObject.filename = result.filename || fileObject.filename; - fileObject.contentType = result.contentType || fileObject.contentType; - fileObject.contentLength = - result.contentLength || fileObject.contentLength; - fileObject.data = result.data || fileObject.data; + if (result && typeof result === 'object') { + fileObject.filename = result.filename || fileObject.filename; + fileObject.contentType = result.contentType || fileObject.contentType; + fileObject.contentLength = + result.contentLength || fileObject.contentLength; + fileObject.data = result.data || fileObject.data; + } return filesController.createFile( config, fileObject.filename, @@ -117,6 +119,7 @@ export class FilesRouter { }) .then(result => { fileObject.url = result.url; + fileObject.filename = result.name; return triggers .maybeRunFileTrigger( triggers.Types.afterSaveFile, From 8d1576bf1cddba9ad513cc3e570e05d639a38c5e Mon Sep 17 00:00:00 2001 From: Steve Date: Tue, 14 Jan 2020 22:28:49 -0500 Subject: [PATCH 10/36] added support for metadata and tags --- src/Adapters/Files/FilesAdapter.js | 12 ++++++++++-- src/Controllers/FilesController.js | 14 ++++++++------ src/Routers/FilesRouter.js | 12 ++++++++++-- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index 8c92454c2e..b7032b3eeb 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -30,11 +30,19 @@ export class FilesAdapter { * @param {string} filename - the filename to save * @param {*} data - the buffer of data from the file * @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) + * - 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 the contentType can be undefined if the controller was not able to determine it (https://docs.aws.amazon.com/AmazonS3/latest/user-guide/add-object-metadata.html) * * @return {Promise} a promise that should fail if the storage didn't succeed */ - createFile(filename: string, data, contentType: string): Promise {} + createFile( + filename: string, + data, + contentType: string, + options: object + ): Promise {} /** Responsible for deleting the specified file * diff --git a/src/Controllers/FilesController.js b/src/Controllers/FilesController.js index 461fa229a9..844330daa7 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) { + createFile(config, filename, data, contentType, options) { const extname = path.extname(filename); const hasExtension = extname.length > 0; @@ -31,12 +31,14 @@ export class FilesController extends AdaptableController { } const location = this.adapter.getFileLocation(config, filename); - return this.adapter.createFile(filename, data, contentType).then(() => { - return Promise.resolve({ - url: location, - name: filename, + return this.adapter + .createFile(filename, data, contentType, options) + .then(() => { + return Promise.resolve({ + url: location, + name: filename, + }); }); - }); } deleteFile(config, filename) { diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 3c6c44a848..2f61aa44a5 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -72,7 +72,7 @@ export class FilesRouter { createHandler(req, res, next) { const config = req.config; const filesController = config.filesController; - const filename = req.params.filename; + const { filename, options = {} } = req.params; const contentType = req.get('Content-type'); const contentLength = req.get('Content-Length'); @@ -94,6 +94,8 @@ export class FilesRouter { contentType, contentLength, data: req.body, + tags: options.tags, + metadata: options.metadata, }; triggers .maybeRunFileTrigger( @@ -109,12 +111,18 @@ export class FilesRouter { fileObject.contentLength = result.contentLength || fileObject.contentLength; fileObject.data = result.data || fileObject.data; + fileObject.tags = result.tags || fileObject.tags; + fileObject.metadata = result.metadata || fileObject.metadata; } return filesController.createFile( config, fileObject.filename, fileObject.data, - fileObject.contentType + fileObject.contentType, + { + tags: fileObject.tags, + metadata: fileObject.metadata, + } ); }) .then(result => { From 1c389ce68208088c2326691cdd603c51846ef8e2 Mon Sep 17 00:00:00 2001 From: Steve Date: Wed, 15 Jan 2020 22:21:49 -0500 Subject: [PATCH 11/36] tests? --- 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 b7032b3eeb..63ec01f455 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -41,7 +41,7 @@ export class FilesAdapter { filename: string, data, contentType: string, - options: object + options: Object ): Promise {} /** Responsible for deleting the specified file From 70a4c810776ad00711e2bb8978e4f26ad64fc651 Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Thu, 16 Jan 2020 14:21:36 -0500 Subject: [PATCH 12/36] trying tests --- spec/CloudCode.spec.js | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index eb80c986f2..171649d1ff 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -5,6 +5,19 @@ const request = require('../lib/request'); const InMemoryCacheAdapter = require('../lib/Adapters/Cache/InMemoryCacheAdapter') .InMemoryCacheAdapter; +const mockAdapter = { + createFile: () => ({ + name: 'some-file-name.txt', + url: 'http://www.somewhere.com/some-file-name.txt', + }), + deleteFile: () => {}, + getFileData: () => {}, + getFileLocation: () => 'xyz', + validateFilename: () => { + return null; + }, +}; + describe('Cloud Code', () => { it('can load absolute cloud code file', done => { reconfigureServer({ @@ -2573,4 +2586,24 @@ describe('beforeLogin hook', () => { expect(beforeFinds).toEqual(1); expect(afterFinds).toEqual(1); }); + + it('beforeSaveFile should not change file if nothing is returned', async () => { + await reconfigureServer({ filesAdapter: mockAdapter }) + Parse.Cloud.beforeSaveFile((req) => { + expect(req.triggerName).toEqual('beforeSaveFile'); + expect(req.fileObject).toEqual({ + filename: 'popeye.txt', + data: [1, 2, 3], + contentType: 'text/plain', + contentLength: 3 + }); + return; + }); + spyOn(Parse.Cloud, 'beforeSaveFile'); + const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + await file.save({ useMasterKey: true }); + expect(Parse.Cloud.beforeSaveFile).toHaveBeenCalled(); + }); }); + + From 370d091545ea38f8c5efd276f599d5b76c5c431d Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Thu, 16 Jan 2020 18:12:48 -0500 Subject: [PATCH 13/36] added tests --- spec/CloudCode.spec.js | 138 ++++++++++++++++++++++++++++++++++--- src/Routers/FilesRouter.js | 10 +-- src/triggers.js | 25 +++---- 3 files changed, 142 insertions(+), 31 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 171649d1ff..c4d90e9e0a 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -6,13 +6,13 @@ const InMemoryCacheAdapter = require('../lib/Adapters/Cache/InMemoryCacheAdapter .InMemoryCacheAdapter; const mockAdapter = { - createFile: () => ({ - name: 'some-file-name.txt', - url: 'http://www.somewhere.com/some-file-name.txt', + createFile: async (filename) => ({ + name: filename, + location: `http://www.somewhere.com/${filename}`, }), deleteFile: () => {}, getFileData: () => {}, - getFileLocation: () => 'xyz', + getFileLocation: (config, filename) => `http://www.somewhere.com/${filename}`, validateFilename: () => { return null; }, @@ -2588,21 +2588,137 @@ describe('beforeLogin hook', () => { }); it('beforeSaveFile should not change file if nothing is returned', async () => { - await reconfigureServer({ filesAdapter: mockAdapter }) - Parse.Cloud.beforeSaveFile((req) => { + await reconfigureServer({ filesAdapter: mockAdapter }); + Parse.Cloud.beforeSaveFile(() => { + return; + }); + const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + const result = await file.save({ useMasterKey: true }); + expect(result).toBe(result); + }); + + it('beforeSaveFile should throw error', async () => { + await reconfigureServer({ filesAdapter: mockAdapter }); + Parse.Cloud.beforeSaveFile(() => { + throw new Parse.Error(400, 'some-error-message'); + }); + const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + try { + await file.save({ useMasterKey: true }); + } catch (error) { + expect(error.message).toBe('Could not store file: popeye.txt.'); + } + }); + + it('beforeSaveFile should change values of uploaded file by editing fileObject directly', async () => { + await reconfigureServer({ filesAdapter: mockAdapter }); + const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); + Parse.Cloud.beforeSaveFile(async (req) => { expect(req.triggerName).toEqual('beforeSaveFile'); expect(req.fileObject).toEqual({ filename: 'popeye.txt', - data: [1, 2, 3], contentType: 'text/plain', - contentLength: 3 + contentLength: 174, + data: new Buffer([1, 2, 3]), + tags: undefined, + metadata: undefined, }); - return; + expect(req.master).toBe(true); + req.fileObject.filename = 'donald_duck.pdf'; + req.fileObject.data = new Buffer([4, 5, 6]); + req.fileObject.tags = { tagA: 'some-tag' } + req.fileObject.metadata = { foo: 'bar' }; + }); + const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + const result = await file.save({ useMasterKey: true }); + expect(result).toBe(file); + const newData = new Buffer([4, 5, 6]); + const newContentType = 'application/pdf'; + const newOptions = { + tags: { + tagA: 'some-tag', + }, + metadata: { + foo: 'bar', + }, + }; + expect(createFileSpy).toHaveBeenCalledWith(jasmine.any(String), newData, newContentType, newOptions); + const expectedFileName = 'donald_duck.pdf'; + expect(file._name.indexOf(expectedFileName)).toBe(file._name.length - expectedFileName.length); + }); + + it('beforeSaveFile should change values by returning new fileObject', async () => { await reconfigureServer({ filesAdapter: mockAdapter }); + const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); + Parse.Cloud.beforeSaveFile(async (req) => { + expect(req.triggerName).toEqual('beforeSaveFile'); + expect(req.fileObject).toEqual({ + filename: 'popeye.txt', + contentType: 'text/plain', + contentLength: 174, + data: new Buffer([1, 2, 3]), + tags: undefined, + metadata: undefined, + }); + return { + filename: 'donald_duck.pdf', + data: new Buffer([4, 5, 6]), + tags: { tagA: 'some-tag' }, + metadata: { foo: 'bar' }, + }; + }); + const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + const result = await file.save({ useMasterKey: true }); + expect(result).toBe(file); + const newData = new Buffer([4, 5, 6]); + const newContentType = 'application/pdf'; + const newOptions = { + tags: { + tagA: 'some-tag', + }, + metadata: { + foo: 'bar', + }, + }; + expect(createFileSpy).toHaveBeenCalledWith(jasmine.any(String), newData, newContentType, newOptions); + const expectedFileName = 'donald_duck.pdf'; + expect(file._name.indexOf(expectedFileName)).toBe(file._name.length - expectedFileName.length); + }); + + it('afterSaveFile should throw error', async () => { + await reconfigureServer({ filesAdapter: mockAdapter }); + Parse.Cloud.beforeSaveFile(async (req) => { + req.fileObject.filename = 'donald_duck.pdf'; + }); + Parse.Cloud.afterSaveFile(async () => { + throw new Parse.Error(400, 'some-error-message'); + }); + const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + try { + await file.save({ useMasterKey: true }); + } catch (error) { + expect(error.message.indexOf('Could not store file: ')).toBe(0); + const newFileName = 'donald_duck.pdf.'; + expect(error.message.indexOf(newFileName)).toBe(error.message.length - newFileName.length); + } + }); + + it('afterSaveFile should call with fileObject', async (done) => { + await reconfigureServer({ filesAdapter: mockAdapter }); + Parse.Cloud.beforeSaveFile(async (req) => { + req.fileObject.filename = 'donald_duck.pdf'; + req.fileObject.tags = { tagA: 'some-tag' }; + req.fileObject.metadata = { foo: 'bar' }; + }); + Parse.Cloud.afterSaveFile(async (req) => { + expect(req.master).toBe(true); + expect(req.fileObject.filename.indexOf('donald_duck.pdf')).toBe(req.fileObject.filename.length - 15); + expect(req.fileObject.contentType).toBe('application/pdf'); + expect(req.fileObject.tags).toEqual({ tagA: 'some-tag' }); + expect(req.fileObject.metadata).toEqual({ foo: 'bar' }); + done(); }); - spyOn(Parse.Cloud, 'beforeSaveFile'); const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); await file.save({ useMasterKey: true }); - expect(Parse.Cloud.beforeSaveFile).toHaveBeenCalled(); }); }); diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 2f61aa44a5..8f0101c36a 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -92,7 +92,7 @@ export class FilesRouter { const fileObject = { filename, contentType, - contentLength, + contentLength: parseInt(contentLength), data: req.body, tags: options.tags, metadata: options.metadata, @@ -107,13 +107,12 @@ export class FilesRouter { .then(result => { if (result && typeof result === 'object') { fileObject.filename = result.filename || fileObject.filename; - fileObject.contentType = result.contentType || fileObject.contentType; - fileObject.contentLength = - result.contentLength || fileObject.contentLength; + fileObject.contentLength = result.contentLength || fileObject.contentLength; fileObject.data = result.data || fileObject.data; fileObject.tags = result.tags || fileObject.tags; fileObject.metadata = result.metadata || fileObject.metadata; } + fileObject.contentType = mime.getType(fileObject.filename); return filesController.createFile( config, fileObject.filename, @@ -142,11 +141,12 @@ export class FilesRouter { }); }) .catch(e => { + // TODO: Should the error message from a throw in beforeSaveFile hook be used here (instead of `Could not store file: ${filename}`)? logger.error('Error creating a file: ', e); next( new Parse.Error( Parse.Error.FILE_SAVE_ERROR, - `Could not store file: ${filename}.` + `Could not store file: ${fileObject.filename}.` ) ); }); diff --git a/src/triggers.js b/src/triggers.js index 9ce4805a91..67d0cb6093 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -705,22 +705,17 @@ export function getRequestFileObject(triggerType, auth, fileObject, config) { return request; } -export function maybeRunFileTrigger(triggerType, fileObject, config, auth) { +export async function maybeRunFileTrigger(triggerType, fileObject, config, auth) { const fileTrigger = getFileTrigger(triggerType, config.applicationId); if (typeof fileTrigger === 'function') { - return Promise.resolve() - .then(() => { - const request = getRequestFileObject( - triggerType, - auth, - fileObject, - config - ); - return fileTrigger(request); - }) - .then(result => { - return result || fileObject; - }); + const request = getRequestFileObject( + triggerType, + auth, + fileObject, + config + ); + const result = fileTrigger(request); + return result || fileObject; } - return Promise.resolve(fileObject); + return fileObject; } From c4f6bd403a88caea8b80630c80393842d7bf704a Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Thu, 16 Jan 2020 18:39:36 -0500 Subject: [PATCH 14/36] fixed failing tests --- src/Routers/FilesRouter.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 8f0101c36a..47ffd7c2bd 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -112,7 +112,10 @@ export class FilesRouter { fileObject.tags = result.tags || fileObject.tags; fileObject.metadata = result.metadata || fileObject.metadata; } - fileObject.contentType = mime.getType(fileObject.filename); + if (fileObject.filename !== filename) { + // if the file name has been changed, update the contentType + fileObject.contentType = mime.getType(fileObject.filename); + } return filesController.createFile( config, fileObject.filename, From 663e3557b9297a500a176386085f7bfd16ec1d38 Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Thu, 16 Jan 2020 19:23:40 -0500 Subject: [PATCH 15/36] added some docs for fileObject --- src/cloud-code/Parse.Cloud.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/cloud-code/Parse.Cloud.js b/src/cloud-code/Parse.Cloud.js index 617cab51be..b50251c4d0 100644 --- a/src/cloud-code/Parse.Cloud.js +++ b/src/cloud-code/Parse.Cloud.js @@ -408,13 +408,25 @@ module.exports = ParseCloud; * @property {String} installationId If set, the installationId triggering the request. * @property {Boolean} master If true, means the master key was used. * @property {Parse.User} user If set, the user that made the request. - * @property {Object} fileObject The object triggering the hook. + * @property {Object} fileObject The object triggering the hook {@link Parse.Cloud.BeforeSaveFileRequest.fileObject}. * @property {String} ip The IP address of the client making the request. * @property {Object} headers The original HTTP headers for the request. * @property {String} triggerName The name of the trigger (`beforeSaveFile`, `afterSaveFile`) * @property {Object} log The current logger inside Parse Server. */ +/** + * @interface Parse.Cloud.BeforeSaveFileRequest.fileObject + * @property {String} filename The file name for the file to be saved + * @property {String} contentType The mime type + * @property {String} contentLength The byte size of the file being saved + * @property {Array} data The data that that should be saved + * 1. an Array of byte value Numbers, or + * 2. an Object like { base64: "..." } with a base64-encoded String. + * @property {Object} tags Key value pairs that should be stored as tags with file (S3 Only) + * @property {Object} metadata Key value pairs that should be stored as metadata with file (S3 Only) + */ + /** * @interface Parse.Cloud.AfterSaveFileRequest * @property {String} installationId If set, the installationId triggering the request. From 5b1c0f493f613d8649d78bebde6cb7709f26c984 Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Fri, 17 Jan 2020 13:39:42 -0500 Subject: [PATCH 16/36] updated hooks to use Parse.File --- spec/CloudCode.spec.js | 63 ++++----------- src/Routers/FilesRouter.js | 145 +++++++++++++++++++++------------- src/cloud-code/Parse.Cloud.js | 15 +--- src/triggers.js | 2 +- 4 files changed, 108 insertions(+), 117 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index c4d90e9e0a..2dc79e6e26 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -2615,25 +2615,14 @@ describe('beforeLogin hook', () => { const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); Parse.Cloud.beforeSaveFile(async (req) => { expect(req.triggerName).toEqual('beforeSaveFile'); - expect(req.fileObject).toEqual({ - filename: 'popeye.txt', - contentType: 'text/plain', - contentLength: 174, - data: new Buffer([1, 2, 3]), - tags: undefined, - metadata: undefined, - }); expect(req.master).toBe(true); - req.fileObject.filename = 'donald_duck.pdf'; - req.fileObject.data = new Buffer([4, 5, 6]); - req.fileObject.tags = { tagA: 'some-tag' } - req.fileObject.metadata = { foo: 'bar' }; + req.file.addMetadata('foo', 'bar'); + req.file.addTag('tagA', 'some-tag'); }); const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); const result = await file.save({ useMasterKey: true }); expect(result).toBe(file); - const newData = new Buffer([4, 5, 6]); - const newContentType = 'application/pdf'; + const newData = new Buffer([1, 2, 3]); const newOptions = { tags: { tagA: 'some-tag', @@ -2642,33 +2631,21 @@ describe('beforeLogin hook', () => { foo: 'bar', }, }; - expect(createFileSpy).toHaveBeenCalledWith(jasmine.any(String), newData, newContentType, newOptions); - const expectedFileName = 'donald_duck.pdf'; - expect(file._name.indexOf(expectedFileName)).toBe(file._name.length - expectedFileName.length); + expect(createFileSpy).toHaveBeenCalledWith(jasmine.any(String), newData, 'text/plain', newOptions); }); it('beforeSaveFile should change values by returning new fileObject', async () => { await reconfigureServer({ filesAdapter: mockAdapter }); const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); Parse.Cloud.beforeSaveFile(async (req) => { expect(req.triggerName).toEqual('beforeSaveFile'); - expect(req.fileObject).toEqual({ - filename: 'popeye.txt', - contentType: 'text/plain', - contentLength: 174, - data: new Buffer([1, 2, 3]), - tags: undefined, - metadata: undefined, - }); - return { - filename: 'donald_duck.pdf', - data: new Buffer([4, 5, 6]), - tags: { tagA: 'some-tag' }, - metadata: { foo: 'bar' }, - }; + const newFile = new Parse.File('donald_duck.pdf', [4, 5, 6], 'application/pdf'); + newFile.setMetadata({ foo: 'bar' }); + newFile.setTags({ tagA: 'some-tag' }); + return newFile; }); const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); const result = await file.save({ useMasterKey: true }); - expect(result).toBe(file); + expect(result).toBeInstanceOf(Parse.File); const newData = new Buffer([4, 5, 6]); const newContentType = 'application/pdf'; const newOptions = { @@ -2686,40 +2663,32 @@ describe('beforeLogin hook', () => { it('afterSaveFile should throw error', async () => { await reconfigureServer({ filesAdapter: mockAdapter }); - Parse.Cloud.beforeSaveFile(async (req) => { - req.fileObject.filename = 'donald_duck.pdf'; - }); Parse.Cloud.afterSaveFile(async () => { throw new Parse.Error(400, 'some-error-message'); }); - const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + const filename = 'donald_duck.pdf'; + const file = new Parse.File(filename, [1, 2, 3], 'text/plain'); try { await file.save({ useMasterKey: true }); } catch (error) { expect(error.message.indexOf('Could not store file: ')).toBe(0); - const newFileName = 'donald_duck.pdf.'; - expect(error.message.indexOf(newFileName)).toBe(error.message.length - newFileName.length); + expect(error.message.indexOf(filename)).toBe(error.message.length - filename.length - 1); } }); it('afterSaveFile should call with fileObject', async (done) => { await reconfigureServer({ filesAdapter: mockAdapter }); Parse.Cloud.beforeSaveFile(async (req) => { - req.fileObject.filename = 'donald_duck.pdf'; - req.fileObject.tags = { tagA: 'some-tag' }; - req.fileObject.metadata = { foo: 'bar' }; + req.file.setTags({ tagA: 'some-tag' }); + req.file.setMetadata({ foo: 'bar' }); }); Parse.Cloud.afterSaveFile(async (req) => { expect(req.master).toBe(true); - expect(req.fileObject.filename.indexOf('donald_duck.pdf')).toBe(req.fileObject.filename.length - 15); - expect(req.fileObject.contentType).toBe('application/pdf'); - expect(req.fileObject.tags).toEqual({ tagA: 'some-tag' }); - expect(req.fileObject.metadata).toEqual({ foo: 'bar' }); + expect(req.file._tags).toEqual({ tagA: 'some-tag' }); + expect(req.file._metadata).toEqual({ foo: 'bar' }); done(); }); const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); await file.save({ useMasterKey: true }); }); }); - - diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 47ffd7c2bd..cf759632fe 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -1,4 +1,4 @@ -import express from 'express'; +import express, { response } from 'express'; import BodyParser from 'body-parser'; import * as Middlewares from '../middlewares'; import Parse from 'parse/node'; @@ -6,6 +6,30 @@ import Config from '../Config'; import mime from 'mime'; import logger from '../logger'; const triggers = require('../triggers'); +const http = require('http'); + +const downloadFileFromURI = (uri) => { + return new Promise((res, rej) => { + http.get(uri, (response) => { + response.setDefaultEncoding('base64'); + let body = `data:${response.headers['content-type']};base64,`; + response.on('data', data => body += data); + response.on('end', () => res(body)); + }).on('error', (e) => { + rej(`Error downloading file from ${uri}: ${e.message}`); + }); + }); +}; + +const addFileDataIfNeeded = async (file) => { + if (file._source.format === 'uri') { + const base64 = await downloadFileFromURI(file._source.uri); + file._previousSave = file; + file._data = base64; + file._requestTask = null; + } + return file; +}; export class FilesRouter { expressRouter({ maxUploadSize = '20Mb' } = {}) { @@ -69,7 +93,7 @@ export class FilesRouter { } } - createHandler(req, res, next) { + async createHandler(req, res, next) { const config = req.config; const filesController = config.filesController; const { filename, options = {} } = req.params; @@ -89,70 +113,79 @@ export class FilesRouter { return; } - const fileObject = { - filename, - contentType, - contentLength: parseInt(contentLength), - data: req.body, - tags: options.tags, - metadata: options.metadata, - }; - triggers - .maybeRunFileTrigger( + const base64 = req.body.toString('base64'); + const file = new Parse.File(filename, { base64 }, contentType); + file.setTags(options.tags); + file.setMetadata(options.metadata); + const fileObject = { contentLength: parseInt(contentLength), file }; + try { + // run beforeSaveFile trigger + const triggerResult = await triggers.maybeRunFileTrigger( triggers.Types.beforeSaveFile, fileObject, config, req.auth ) - .then(result => { - if (result && typeof result === 'object') { - fileObject.filename = result.filename || fileObject.filename; - fileObject.contentLength = result.contentLength || fileObject.contentLength; - fileObject.data = result.data || fileObject.data; - fileObject.tags = result.tags || fileObject.tags; - fileObject.metadata = result.metadata || fileObject.metadata; - } - if (fileObject.filename !== filename) { - // if the file name has been changed, update the contentType - fileObject.contentType = mime.getType(fileObject.filename); + let saveResult; + // if a new ParseFile is returned check if it's an already saved file + if (triggerResult instanceof Parse.File) { + fileObject.file = triggerResult; + if (triggerResult.url()) { + // set contentLength to null because we wont know how big it is here + fileObject.contentLength = null; + saveResult = { + url: triggerResult.url(), + name: triggerResult._name, + }; } - return filesController.createFile( + } + // if the file returned by the trigger has already been saved skip saving anything + if (!saveResult) { + // if the ParseFile returned is type uri, download the file before saving it + await addFileDataIfNeeded(fileObject.file); + // save file + const createFileResult = await filesController.createFile( config, - fileObject.filename, - fileObject.data, - fileObject.contentType, + fileObject.file._name, + Buffer.from(fileObject.file._data, 'base64'), + fileObject.file._source.type, { - tags: fileObject.tags, - metadata: fileObject.metadata, + tags: fileObject.file._tags, + metadata: fileObject.file._metadata, } ); - }) - .then(result => { - fileObject.url = result.url; - fileObject.filename = result.name; - return triggers - .maybeRunFileTrigger( - triggers.Types.afterSaveFile, - fileObject, - config, - req.auth - ) - .then(() => { - res.status(201); - res.set('Location', result.url); - res.json(result); - }); - }) - .catch(e => { - // TODO: Should the error message from a throw in beforeSaveFile hook be used here (instead of `Could not store file: ${filename}`)? - logger.error('Error creating a file: ', e); - next( - new Parse.Error( - Parse.Error.FILE_SAVE_ERROR, - `Could not store file: ${fileObject.filename}.` - ) - ); - }); + // update file with new data + fileObject.file._name = createFileResult.name; + fileObject.file._url = createFileResult.url; + fileObject.file._requestTask = null; + fileObject.file._previousSave = Promise.resolve(fileObject.file); + saveResult = { + url: createFileResult.url, + name: createFileResult.name, + }; + } + // run afterSaveFile trigger + await triggers.maybeRunFileTrigger( + triggers.Types.afterSaveFile, + fileObject, + config, + req.auth + ); + res.status(201); + res.set('Location', saveResult.url); + res.json(saveResult); + + } catch (e) { + console.log(e); + // TODO: Should the error message from a throw in beforeSaveFile hook be used here (instead of `Could not store file: ${filename}`)? + logger.error('Error creating a file: ', e); + next( + new Parse.Error( + Parse.Error.FILE_SAVE_ERROR, + `Could not store file: ${fileObject.file._name}.` + ) + ); + } } deleteHandler(req, res, next) { diff --git a/src/cloud-code/Parse.Cloud.js b/src/cloud-code/Parse.Cloud.js index b50251c4d0..ace38eff0a 100644 --- a/src/cloud-code/Parse.Cloud.js +++ b/src/cloud-code/Parse.Cloud.js @@ -408,25 +408,14 @@ module.exports = ParseCloud; * @property {String} installationId If set, the installationId triggering the request. * @property {Boolean} master If true, means the master key was used. * @property {Parse.User} user If set, the user that made the request. - * @property {Object} fileObject The object triggering the hook {@link Parse.Cloud.BeforeSaveFileRequest.fileObject}. + * @property {Parse.File} file The file that triggered the hook + * @property {Integer} contentLength The value from Content-Length header * @property {String} ip The IP address of the client making the request. * @property {Object} headers The original HTTP headers for the request. * @property {String} triggerName The name of the trigger (`beforeSaveFile`, `afterSaveFile`) * @property {Object} log The current logger inside Parse Server. */ -/** - * @interface Parse.Cloud.BeforeSaveFileRequest.fileObject - * @property {String} filename The file name for the file to be saved - * @property {String} contentType The mime type - * @property {String} contentLength The byte size of the file being saved - * @property {Array} data The data that that should be saved - * 1. an Array of byte value Numbers, or - * 2. an Object like { base64: "..." } with a base64-encoded String. - * @property {Object} tags Key value pairs that should be stored as tags with file (S3 Only) - * @property {Object} metadata Key value pairs that should be stored as metadata with file (S3 Only) - */ - /** * @interface Parse.Cloud.AfterSaveFileRequest * @property {String} installationId If set, the installationId triggering the request. diff --git a/src/triggers.js b/src/triggers.js index 67d0cb6093..5446b7a2cc 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -682,8 +682,8 @@ export function runLiveQueryEventHandlers( export function getRequestFileObject(triggerType, auth, fileObject, config) { const request = { + ...fileObject, triggerName: triggerType, - fileObject, master: false, log: config.loggerController, headers: config.headers, From 14b41b38423c533a447ae2cc7b225436739cff92 Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Fri, 17 Jan 2020 17:25:40 -0500 Subject: [PATCH 17/36] added test for already saved file being returned in hook --- spec/CloudCode.spec.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 2dc79e6e26..83055113d5 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -2597,6 +2597,22 @@ describe('beforeLogin hook', () => { expect(result).toBe(result); }); + it('beforeSaveFile should return file that is already saved and not save anything to files adapter', async () => { + await reconfigureServer({ filesAdapter: mockAdapter }); + const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); + Parse.Cloud.beforeSaveFile(() => { + const newFile = new Parse.File('some-file.txt'); + newFile._url = 'http://www.somewhere.com/parse/files/some-app-id/some-file.txt'; + return newFile; + }); + const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + const result = await file.save({ useMasterKey: true }); + expect(result).toBe(result); + expect(result._name).toBe('some-file.txt'); + expect(result._url).toBe('http://www.somewhere.com/parse/files/some-app-id/some-file.txt'); + expect(createFileSpy).not.toHaveBeenCalled(); + }); + it('beforeSaveFile should throw error', async () => { await reconfigureServer({ filesAdapter: mockAdapter }); Parse.Cloud.beforeSaveFile(() => { @@ -2634,7 +2650,8 @@ describe('beforeLogin hook', () => { expect(createFileSpy).toHaveBeenCalledWith(jasmine.any(String), newData, 'text/plain', newOptions); }); - it('beforeSaveFile should change values by returning new fileObject', async () => { await reconfigureServer({ filesAdapter: mockAdapter }); + it('beforeSaveFile should change values by returning new fileObject', async () => { + await reconfigureServer({ filesAdapter: mockAdapter }); const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); Parse.Cloud.beforeSaveFile(async (req) => { expect(req.triggerName).toEqual('beforeSaveFile'); From a0087a084f8cdf9a4c37173058afec44329c8a26 Mon Sep 17 00:00:00 2001 From: Steve Date: Sun, 19 Jan 2020 12:19:58 -0500 Subject: [PATCH 18/36] added beforeDeleteFile and afterDeleteFile hooks --- spec/CloudCode.spec.js | 42 +++++++++++++++++++++++ src/Routers/FilesRouter.js | 54 +++++++++++++++++++---------- src/cloud-code/Parse.Cloud.js | 64 +++++++++++++++++++++++++++-------- src/triggers.js | 2 ++ 4 files changed, 129 insertions(+), 33 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 83055113d5..824dc21a6f 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -2708,4 +2708,46 @@ describe('beforeLogin hook', () => { const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); await file.save({ useMasterKey: true }); }); + + it('beforeDeleteFile should call with fileObject', async () => { + await reconfigureServer({ filesAdapter: mockAdapter }); + Parse.Cloud.beforeDeleteFile((req) => { + expect(req.file).toBeInstanceOf(Parse.File); + expect(req.file._name).toEqual('popeye.txt'); + expect(req.file._url).toEqual('http://www.somewhere.com/popeye.txt'); + }); + const file = new Parse.File('popeye.txt'); + await file.destroy({ useMasterKey: true }); + }); + + it('beforeDeleteFile should throw error', async (done) => { + await reconfigureServer({ filesAdapter: mockAdapter }); + Parse.Cloud.beforeDeleteFile(() => { + throw new Error('some error message'); + }); + const file = new Parse.File('popeye.txt'); + try { + await file.destroy({ useMasterKey: true }); + } catch (error) { + expect(error.message).toBe('Could not delete file.'); + done(); + } + }) + + it('afterDeleteFile should cal with fileObject', async (done) => { + await reconfigureServer({ filesAdapter: mockAdapter }); + Parse.Cloud.beforeDeleteFile((req) => { + expect(req.file).toBeInstanceOf(Parse.File); + expect(req.file._name).toEqual('popeye.txt'); + expect(req.file._url).toEqual('http://www.somewhere.com/popeye.txt'); + }); + Parse.Cloud.afterDeleteFile((req) => { + expect(req.file).toBeInstanceOf(Parse.File); + expect(req.file._name).toEqual('popeye.txt'); + expect(req.file._url).toEqual('http://www.somewhere.com/popeye.txt'); + done(); + }); + const file = new Parse.File('popeye.txt'); + await file.destroy({ useMasterKey: true }); + }); }); diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index cf759632fe..5ace4b03e2 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -176,7 +176,6 @@ export class FilesRouter { res.json(saveResult); } catch (e) { - console.log(e); // TODO: Should the error message from a throw in beforeSaveFile hook be used here (instead of `Could not store file: ${filename}`)? logger.error('Error creating a file: ', e); next( @@ -188,23 +187,42 @@ export class FilesRouter { } } - deleteHandler(req, res, next) { - const filesController = req.config.filesController; - filesController - .deleteFile(req.config, req.params.filename) - .then(() => { - res.status(200); - // TODO: return useful JSON here? - res.end(); - }) - .catch(() => { - next( - new Parse.Error( - Parse.Error.FILE_DELETE_ERROR, - 'Could not delete file.' - ) - ); - }); + async deleteHandler(req, res, next) { + try { + const { filesController } = req.config; + const { filename } = req.params; + // run beforeDeleteFile trigger + const file = new Parse.File(filename); + file._url = filesController.adapter.getFileLocation(req.config, filename); + const fileObject = { file } + await triggers.maybeRunFileTrigger( + triggers.Types.beforeDeleteFile, + fileObject, + req.config, + req.auth + ); + // delete file + await filesController.deleteFile(req.config, filename); + // run afterDeleteFile trigger + await triggers.maybeRunFileTrigger( + triggers.Types.afterDeleteFile, + fileObject, + req.config, + req.auth + ); + res.status(200); + // TODO: return useful JSON here? + res.end(); + } catch (e) { + // TODO: Should the error message from a throw in beforeDeleteFile hook be used here (instead of 'Could not delete file')? + logger.error('Error deleting a file: ', e); + next( + new Parse.Error( + Parse.Error.FILE_DELETE_ERROR, + 'Could not delete file.' + ) + ); + } } } diff --git a/src/cloud-code/Parse.Cloud.js b/src/cloud-code/Parse.Cloud.js index ace38eff0a..41a826d108 100644 --- a/src/cloud-code/Parse.Cloud.js +++ b/src/cloud-code/Parse.Cloud.js @@ -338,7 +338,7 @@ ParseCloud.afterFind = function(parseClass, handler) { * * @method beforeSaveFile * @name Parse.Cloud.beforeSaveFile - * @param {Function} func The function to run before saving a file. This function can be async and should take just one parameter, {@link Parse.Cloud.BeforeSaveFileRequest}. + * @param {Function} func The function to run before saving a file. This function can be async and should take just one parameter, {@link Parse.Cloud.FileTriggerRequest}. */ ParseCloud.beforeSaveFile = function(handler) { triggers.addFileTrigger( @@ -361,7 +361,7 @@ ParseCloud.beforeSaveFile = function(handler) { * * @method afterSaveFile * @name Parse.Cloud.afterSaveFile - * @param {Function} func The function to run after saving a file. This function can be async and should take just one parameter, {@link Parse.Cloud.AfterSaveFileRequest}. + * @param {Function} func The function to run after saving a file. This function can be async and should take just one parameter, {@link Parse.Cloud.FileTriggerRequest}. */ ParseCloud.afterSaveFile = function(handler) { triggers.addFileTrigger( @@ -371,6 +371,52 @@ ParseCloud.afterSaveFile = function(handler) { ); }; +/** + * Registers an before delete file function. + * + * **Available in Cloud Code only.** + * + * ``` + * Parse.Cloud.beforeDeleteFile(async (request) => { + * // code here + * }) + *``` + * + * @method beforeDeleteFile + * @name Parse.Cloud.beforeDeleteFile + * @param {Function} func The function to run before deleting a file. This function can be async and should take just one parameter, {@link Parse.Cloud.FileTriggerRequest}. + */ +ParseCloud.beforeDeleteFile = function(handler) { + triggers.addFileTrigger( + triggers.Types.beforeDeleteFile, + handler, + Parse.applicationId, + ); +}; + +/** + * Registers an after delete file function. + * + * **Available in Cloud Code only.** + * + * ``` + * Parse.Cloud.afterDeleteFile(async (request) => { + * // code here + * }) + *``` + * + * @method afterDeleteFile + * @name Parse.Cloud.afterDeleteFile + * @param {Function} func The function to after before deleting a file. This function can be async and should take just one parameter, {@link Parse.Cloud.FileTriggerRequest}. + */ +ParseCloud.afterDeleteFile = function(handler) { + triggers.addFileTrigger( + triggers.Types.afterDeleteFile, + handler, + Parse.applicationId, + ); +}; + ParseCloud.onLiveQueryEvent = function(handler) { triggers.addLiveQueryEventHandler(handler, Parse.applicationId); }; @@ -404,7 +450,7 @@ module.exports = ParseCloud; */ /** - * @interface Parse.Cloud.BeforeSaveFileRequest + * @interface Parse.Cloud.FileTriggerRequest * @property {String} installationId If set, the installationId triggering the request. * @property {Boolean} master If true, means the master key was used. * @property {Parse.User} user If set, the user that made the request. @@ -416,18 +462,6 @@ module.exports = ParseCloud; * @property {Object} log The current logger inside Parse Server. */ -/** - * @interface Parse.Cloud.AfterSaveFileRequest - * @property {String} installationId If set, the installationId triggering the request. - * @property {Boolean} master If true, means the master key was used. - * @property {Parse.User} user If set, the user that made the request. - * @property {Object} fileObject The object triggering the hook. - * @property {String} ip The IP address of the client making the request. - * @property {Object} headers The original HTTP headers for the request. - * @property {String} triggerName The name of the trigger (`beforeSaveFile`, `afterSaveFile`) - * @property {Object} log The current logger inside Parse Server. - */ - /** * @interface Parse.Cloud.BeforeFindRequest * @property {String} installationId If set, the installationId triggering the request. diff --git a/src/triggers.js b/src/triggers.js index 5446b7a2cc..99c2c2ede3 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -13,6 +13,8 @@ export const Types = { afterFind: 'afterFind', beforeSaveFile: 'beforeSaveFile', afterSaveFile: 'afterSaveFile', + beforeDeleteFile: 'beforeDeleteFile', + afterDeleteFile: 'afterDeleteFile', }; const FileClassName = '@File'; From d89b358e8b2956c25fc855e16d95c1edbdaff769 Mon Sep 17 00:00:00 2001 From: Steve Date: Sun, 19 Jan 2020 17:34:12 -0500 Subject: [PATCH 19/36] removed contentLength because it's already in the header --- src/Routers/FilesRouter.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 5ace4b03e2..01c63f5708 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -98,7 +98,6 @@ export class FilesRouter { const filesController = config.filesController; const { filename, options = {} } = req.params; const contentType = req.get('Content-type'); - const contentLength = req.get('Content-Length'); if (!req.body || !req.body.length) { next( @@ -117,7 +116,7 @@ export class FilesRouter { const file = new Parse.File(filename, { base64 }, contentType); file.setTags(options.tags); file.setMetadata(options.metadata); - const fileObject = { contentLength: parseInt(contentLength), file }; + const fileObject = { file }; try { // run beforeSaveFile trigger const triggerResult = await triggers.maybeRunFileTrigger( From 2982df9d953405e22136c496618a38aac8a3f8ff Mon Sep 17 00:00:00 2001 From: Steve Date: Sun, 19 Jan 2020 18:16:44 -0500 Subject: [PATCH 20/36] added fileSize param to FileTriggerRequest --- spec/CloudCode.spec.js | 39 +++++++++++++++++++++++++++++++++++ src/Routers/FilesRouter.js | 14 ++++++++----- src/cloud-code/Parse.Cloud.js | 3 ++- 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 824dc21a6f..0c8a71b3e9 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -2655,6 +2655,7 @@ describe('beforeLogin hook', () => { const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); Parse.Cloud.beforeSaveFile(async (req) => { expect(req.triggerName).toEqual('beforeSaveFile'); + expect(req.fileSize).toBe(3); const newFile = new Parse.File('donald_duck.pdf', [4, 5, 6], 'application/pdf'); newFile.setMetadata({ foo: 'bar' }); newFile.setTags({ tagA: 'some-tag' }); @@ -2678,6 +2679,26 @@ describe('beforeLogin hook', () => { expect(file._name.indexOf(expectedFileName)).toBe(file._name.length - expectedFileName.length); }); + it('afterSaveFile should set fileSize to null if beforeSave returns an already saved file', async () => { + await reconfigureServer({ filesAdapter: mockAdapter }); + const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); + Parse.Cloud.beforeSaveFile((req) => { + expect(req.fileSize).toBe(3); + const newFile = new Parse.File('some-file.txt'); + newFile._url = 'http://www.somewhere.com/parse/files/some-app-id/some-file.txt'; + return newFile; + }); + Parse.Cloud.afterSaveFile((req) => { + expect(req.fileSize).toBe(null); + }); + const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + const result = await file.save({ useMasterKey: true }); + expect(result).toBe(result); + expect(result._name).toBe('some-file.txt'); + expect(result._url).toBe('http://www.somewhere.com/parse/files/some-app-id/some-file.txt'); + expect(createFileSpy).not.toHaveBeenCalled(); + }); + it('afterSaveFile should throw error', async () => { await reconfigureServer({ filesAdapter: mockAdapter }); Parse.Cloud.afterSaveFile(async () => { @@ -2709,12 +2730,30 @@ describe('beforeLogin hook', () => { await file.save({ useMasterKey: true }); }); + it('afterSaveFile should change fileSize when file data changes', async (done) => { + await reconfigureServer({ filesAdapter: mockAdapter }); + Parse.Cloud.beforeSaveFile(async (req) => { + expect(req.fileSize).toBe(3); + expect(req.master).toBe(true); + const newFile = new Parse.File('donald_duck.pdf', [4, 5, 6, 7, 8, 9], 'application/pdf'); + return newFile; + }); + Parse.Cloud.afterSaveFile(async (req) => { + expect(req.fileSize).toBe(6); + expect(req.master).toBe(true); + done(); + }); + const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + await file.save({ useMasterKey: true }); + }); + it('beforeDeleteFile should call with fileObject', async () => { await reconfigureServer({ filesAdapter: mockAdapter }); Parse.Cloud.beforeDeleteFile((req) => { expect(req.file).toBeInstanceOf(Parse.File); expect(req.file._name).toEqual('popeye.txt'); expect(req.file._url).toEqual('http://www.somewhere.com/popeye.txt'); + expect(req.fileSize).toBe(null); }); const file = new Parse.File('popeye.txt'); await file.destroy({ useMasterKey: true }); diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 01c63f5708..321c366350 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -116,7 +116,8 @@ export class FilesRouter { const file = new Parse.File(filename, { base64 }, contentType); file.setTags(options.tags); file.setMetadata(options.metadata); - const fileObject = { file }; + const fileSize = Buffer.byteLength(req.body); + const fileObject = { file, fileSize }; try { // run beforeSaveFile trigger const triggerResult = await triggers.maybeRunFileTrigger( @@ -130,8 +131,8 @@ export class FilesRouter { if (triggerResult instanceof Parse.File) { fileObject.file = triggerResult; if (triggerResult.url()) { - // set contentLength to null because we wont know how big it is here - fileObject.contentLength = null; + // set fileSize to null because we wont know how big it is here + fileObject.fileSize = null; saveResult = { url: triggerResult.url(), name: triggerResult._name, @@ -142,11 +143,14 @@ export class FilesRouter { if (!saveResult) { // if the ParseFile returned is type uri, download the file before saving it await addFileDataIfNeeded(fileObject.file); + // update fileSize + const bufferData = Buffer.from(fileObject.file._data, 'base64'); + fileObject.fileSize = Buffer.byteLength(bufferData); // save file const createFileResult = await filesController.createFile( config, fileObject.file._name, - Buffer.from(fileObject.file._data, 'base64'), + bufferData, fileObject.file._source.type, { tags: fileObject.file._tags, @@ -193,7 +197,7 @@ export class FilesRouter { // run beforeDeleteFile trigger const file = new Parse.File(filename); file._url = filesController.adapter.getFileLocation(req.config, filename); - const fileObject = { file } + const fileObject = { file, fileSize: null } await triggers.maybeRunFileTrigger( triggers.Types.beforeDeleteFile, fileObject, diff --git a/src/cloud-code/Parse.Cloud.js b/src/cloud-code/Parse.Cloud.js index 41a826d108..fbac7ab1c0 100644 --- a/src/cloud-code/Parse.Cloud.js +++ b/src/cloud-code/Parse.Cloud.js @@ -454,7 +454,8 @@ module.exports = ParseCloud; * @property {String} installationId If set, the installationId triggering the request. * @property {Boolean} master If true, means the master key was used. * @property {Parse.User} user If set, the user that made the request. - * @property {Parse.File} file The file that triggered the hook + * @property {Parse.File} file The file that triggered the hook. + * @property {Integer} fileSize The size of the file in bytes. * @property {Integer} contentLength The value from Content-Length header * @property {String} ip The IP address of the client making the request. * @property {Object} headers The original HTTP headers for the request. From ac5ba86c51486e33fb2f5e4d1e7d945458cd1918 Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Wed, 22 Jan 2020 17:05:12 -0500 Subject: [PATCH 21/36] added support for client side metadata and tags --- spec/CloudCode.spec.js | 23 +++++++++++++++++++++++ src/Routers/FilesRouter.js | 7 ++++--- src/middlewares.js | 1 + 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 0c8a71b3e9..385dd531bb 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -2679,6 +2679,29 @@ describe('beforeLogin hook', () => { expect(file._name.indexOf(expectedFileName)).toBe(file._name.length - expectedFileName.length); }); + fit('beforeSaveFile should contain metadata and tags saved from client', async () => { + await reconfigureServer({ filesAdapter: mockAdapter }); + const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); + Parse.Cloud.beforeSaveFile(async (req) => { + expect(req.triggerName).toEqual('beforeSaveFile'); + expect(req.fileSize).toBe(3); + expect(req.file).toBeInstanceOf(Parse.File); + expect(req.file.name()).toBe('popeye.txt'); + expect(req.file.metadata()).toEqual({ foo: 'bar' }); + expect(req.file.tags()).toEqual({ bar: 'foo' }); + }); + const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + file.setMetadata({ foo: 'bar' }); + file.setTags({ bar: 'foo' }); + const result = await file.save({ useMasterKey: true }); + expect(result).toBeInstanceOf(Parse.File); + const options = { + metadata: { foo: 'bar' }, + tags: { bar: 'foo' }, + }; + expect(createFileSpy).toHaveBeenCalledWith(jasmine.any(String), jasmine.any(Buffer), 'text/plain', options); + }); + it('afterSaveFile should set fileSize to null if beforeSave returns an already saved file', async () => { await reconfigureServer({ filesAdapter: mockAdapter }); const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 321c366350..82a0e5026d 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -96,7 +96,7 @@ export class FilesRouter { async createHandler(req, res, next) { const config = req.config; const filesController = config.filesController; - const { filename, options = {} } = req.params; + const { filename } = req.params; const contentType = req.get('Content-type'); if (!req.body || !req.body.length) { @@ -114,8 +114,9 @@ export class FilesRouter { const base64 = req.body.toString('base64'); const file = new Parse.File(filename, { base64 }, contentType); - file.setTags(options.tags); - file.setMetadata(options.metadata); + const { metadata = {}, tags = {} } = req.fileData || {}; + file.setTags(tags); + file.setMetadata(metadata); const fileSize = Buffer.byteLength(req.body); const fileObject = { file, fileSize }; try { diff --git a/src/middlewares.js b/src/middlewares.js index 75923e713f..4b1db27c1c 100644 --- a/src/middlewares.js +++ b/src/middlewares.js @@ -110,6 +110,7 @@ export function handleParseHeaders(req, res, next) { } if (fileViaJSON) { + req.fileData = req.body.fileData; // We need to repopulate req.body with a buffer var base64 = req.body.base64; req.body = Buffer.from(base64, 'base64'); From 2cfe0863ce3ea1141c289ca6ceddedb8ebb11752 Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Wed, 22 Jan 2020 17:07:13 -0500 Subject: [PATCH 22/36] removed fit test --- spec/CloudCode.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 385dd531bb..0d8a5f62aa 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -2679,7 +2679,7 @@ describe('beforeLogin hook', () => { expect(file._name.indexOf(expectedFileName)).toBe(file._name.length - expectedFileName.length); }); - fit('beforeSaveFile should contain metadata and tags saved from client', async () => { + it('beforeSaveFile should contain metadata and tags saved from client', async () => { await reconfigureServer({ filesAdapter: mockAdapter }); const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); Parse.Cloud.beforeSaveFile(async (req) => { From 36222dc4fc146603b190777121737d1ac51ddfa3 Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Wed, 22 Jan 2020 17:10:54 -0500 Subject: [PATCH 23/36] removed unused import --- src/Routers/FilesRouter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 82a0e5026d..177fb8a828 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -1,4 +1,4 @@ -import express, { response } from 'express'; +import express from 'express'; import BodyParser from 'body-parser'; import * as Middlewares from '../middlewares'; import Parse from 'parse/node'; From 0c4e191de6f226335cf110285ef7c5b307f1259c Mon Sep 17 00:00:00 2001 From: Steve Date: Sat, 1 Feb 2020 13:42:22 -0500 Subject: [PATCH 24/36] added loging to file triggers --- src/triggers.js | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/triggers.js b/src/triggers.js index 99c2c2ede3..be6b17c966 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -710,14 +710,32 @@ export function getRequestFileObject(triggerType, auth, fileObject, config) { export async function maybeRunFileTrigger(triggerType, fileObject, config, auth) { const fileTrigger = getFileTrigger(triggerType, config.applicationId); if (typeof fileTrigger === 'function') { - const request = getRequestFileObject( - triggerType, - auth, - fileObject, - config - ); - const result = fileTrigger(request); - return result || fileObject; + try { + const request = getRequestFileObject( + triggerType, + auth, + fileObject, + config + ); + const result = await fileTrigger(request); + logTriggerSuccessBeforeHook( + triggerType, + 'Parse.File', + { ...fileObject.file.toJSON(), fileSize: fileObject.fileSize }, + result, + auth, + ) + return result || fileObject; + } catch (error) { + logTriggerErrorBeforeHook( + triggerType, + 'Parse.File', + { ...fileObject.file.toJSON(), fileSize: fileObject.fileSize }, + auth, + error, + ); + throw error; + } } return fileObject; } From 51db0a674e84312119f6e155dca81f60c49b0459 Mon Sep 17 00:00:00 2001 From: Steve Date: Sat, 1 Feb 2020 13:43:01 -0500 Subject: [PATCH 25/36] updated error message --- src/Routers/FilesRouter.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 177fb8a828..4ff77f8143 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -180,12 +180,11 @@ export class FilesRouter { res.json(saveResult); } catch (e) { - // TODO: Should the error message from a throw in beforeSaveFile hook be used here (instead of `Could not store file: ${filename}`)? logger.error('Error creating a file: ', e); next( new Parse.Error( Parse.Error.FILE_SAVE_ERROR, - `Could not store file: ${fileObject.file._name}.` + e.message ? e.message : `Could not store file: ${fileObject.file._name}.` ) ); } From 711c9fdf82c859d07ac0e03de2c8d2cf95cc21df Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Wed, 4 Mar 2020 13:09:37 -0500 Subject: [PATCH 26/36] updated error message --- src/Routers/FilesRouter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 177fb8a828..b6482b5ef1 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -185,7 +185,7 @@ export class FilesRouter { next( new Parse.Error( Parse.Error.FILE_SAVE_ERROR, - `Could not store file: ${fileObject.file._name}.` + e.message ? e.message : `Could not store file: ${fileObject.file._name}.` ) ); } From 299135d1f151ce6bdab4d930f9779217cc6049bd Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Wed, 4 Mar 2020 13:20:25 -0500 Subject: [PATCH 27/36] fixed tests --- spec/CloudCode.spec.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 0183c28eea..62ef89eb6b 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -2644,7 +2644,7 @@ describe('beforeLogin hook', () => { try { await file.save({ useMasterKey: true }); } catch (error) { - expect(error.message).toBe('Could not store file: popeye.txt.'); + expect(error.message).toBe('some-error-message'); } }); @@ -2754,8 +2754,7 @@ describe('beforeLogin hook', () => { try { await file.save({ useMasterKey: true }); } catch (error) { - expect(error.message.indexOf('Could not store file: ')).toBe(0); - expect(error.message.indexOf(filename)).toBe(error.message.length - filename.length - 1); + expect(error.message).toBe('some-error-message'); } }); From 476052d18815b195c16422079a65bd603d42fe5f Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Sun, 22 Mar 2020 13:31:22 -0400 Subject: [PATCH 28/36] fixed typos --- spec/CloudCode.spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 62ef89eb6b..12198bdb13 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -2616,7 +2616,7 @@ describe('beforeLogin hook', () => { }); const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); const result = await file.save({ useMasterKey: true }); - expect(result).toBe(result); + expect(result).toBe(file); }); it('beforeSaveFile should return file that is already saved and not save anything to files adapter', async () => { @@ -2629,7 +2629,7 @@ describe('beforeLogin hook', () => { }); const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); const result = await file.save({ useMasterKey: true }); - expect(result).toBe(result); + expect(result).toBe(file); expect(result._name).toBe('some-file.txt'); expect(result._url).toBe('http://www.somewhere.com/parse/files/some-app-id/some-file.txt'); expect(createFileSpy).not.toHaveBeenCalled(); @@ -2817,7 +2817,7 @@ describe('beforeLogin hook', () => { } }) - it('afterDeleteFile should cal with fileObject', async (done) => { + it('afterDeleteFile should call with fileObject', async (done) => { await reconfigureServer({ filesAdapter: mockAdapter }); Parse.Cloud.beforeDeleteFile((req) => { expect(req.file).toBeInstanceOf(Parse.File); From 70f9b30c4a24cd4df2130a7e38f536a34cc573bb Mon Sep 17 00:00:00 2001 From: stevestencil Date: Tue, 31 Mar 2020 01:27:01 -0400 Subject: [PATCH 29/36] Update package.json --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 57d530cac8..d6fa80146d 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "mime": "2.4.4", "mongodb": "3.5.4", "node-rsa": "1.0.7", - "parse": "2.11.0", + "parse": "2.12.0", "pg-promise": "10.4.4", "pluralize": "^8.0.0", "redis": "3.0.0", From f43c26c900f3bddb84db9b183e320fa72f1094a1 Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Tue, 31 Mar 2020 13:31:13 -0400 Subject: [PATCH 30/36] fixed failing test --- spec/FilesController.spec.js | 5 ++++- src/Routers/FilesRouter.js | 16 +++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index aef6448182..6592942459 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -70,7 +70,10 @@ describe('FilesController', () => { expect(log1.level).toBe('error'); const log2 = logs.find( - x => x.message === 'Could not store file: yolo.txt.' + // we can no longer expect 'Could not store file: yolo.txt' because we now + // want to return the actual error message that can possibly come from + // the beforeSaveFile hook + x => x.message === 'it failed with xyz' ); expect(log2.level).toBe('error'); expect(log2.code).toBe(130); diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 4ff77f8143..037d3372c9 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -31,6 +31,15 @@ const addFileDataIfNeeded = async (file) => { return file; }; +const errorMessageFromError = (e) => { + if (typeof e === 'string') { + return e; + } else if (e && e.message) { + return e.message; + } + return undefined; +} + export class FilesRouter { expressRouter({ maxUploadSize = '20Mb' } = {}) { var router = express.Router(); @@ -181,10 +190,11 @@ export class FilesRouter { } catch (e) { logger.error('Error creating a file: ', e); + const errorMessage = errorMessageFromError(e) || `Could not store file: ${fileObject.file._name}.`; next( new Parse.Error( Parse.Error.FILE_SAVE_ERROR, - e.message ? e.message : `Could not store file: ${fileObject.file._name}.` + errorMessage ) ); } @@ -217,12 +227,12 @@ export class FilesRouter { // TODO: return useful JSON here? res.end(); } catch (e) { - // TODO: Should the error message from a throw in beforeDeleteFile hook be used here (instead of 'Could not delete file')? logger.error('Error deleting a file: ', e); + const errorMessage = errorMessageFromError(e) || `Could not store file.`; next( new Parse.Error( Parse.Error.FILE_DELETE_ERROR, - 'Could not delete file.' + errorMessage ) ); } From ffad97c18e11f8c08a8ca4aa8d7f54445567001e Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Tue, 31 Mar 2020 14:29:39 -0400 Subject: [PATCH 31/36] fixed error message --- src/Routers/FilesRouter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 037d3372c9..ab46fca86c 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -228,7 +228,7 @@ export class FilesRouter { res.end(); } catch (e) { logger.error('Error deleting a file: ', e); - const errorMessage = errorMessageFromError(e) || `Could not store file.`; + const errorMessage = errorMessageFromError(e) || `Could not delete file.`; next( new Parse.Error( Parse.Error.FILE_DELETE_ERROR, From f34a8dbaea727ee8695472f548d097e7c0be57e4 Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Tue, 31 Mar 2020 14:45:54 -0400 Subject: [PATCH 32/36] fixed failing tests (hopefully) --- spec/CloudCode.spec.js | 2 +- spec/ParseFile.spec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 12198bdb13..53dd8fc46e 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -2812,7 +2812,7 @@ describe('beforeLogin hook', () => { try { await file.destroy({ useMasterKey: true }); } catch (error) { - expect(error.message).toBe('Could not delete file.'); + expect(error.message).toBe('some error message'); done(); } }) diff --git a/spec/ParseFile.spec.js b/spec/ParseFile.spec.js index 075adcbd86..fe057f3195 100644 --- a/spec/ParseFile.spec.js +++ b/spec/ParseFile.spec.js @@ -626,7 +626,7 @@ describe('Parse.File testing', () => { }).then(fail, response => { expect(response.status).toBe(400); const body = response.text; - expect(body).toEqual('{"code":153,"error":"Could not delete file."}'); + expect(body).toEqual('{"code":153,"error":"FileNotFound"}'); done(); }); }); From deea148239a900e19f046ee1374fa2eed2226e08 Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Tue, 31 Mar 2020 15:04:14 -0400 Subject: [PATCH 33/36] TESTS!!! --- spec/ParseFile.spec.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/ParseFile.spec.js b/spec/ParseFile.spec.js index fe057f3195..bb1c5c512f 100644 --- a/spec/ParseFile.spec.js +++ b/spec/ParseFile.spec.js @@ -626,7 +626,11 @@ describe('Parse.File testing', () => { }).then(fail, response => { expect(response.status).toBe(400); const body = response.text; - expect(body).toEqual('{"code":153,"error":"FileNotFound"}'); + expect(typeof body).toBe('string'); + const { code, error } = JSON.parse(body); + expect(code).toBe(153); + expect(typeof error).toBe('string'); + expect(error.length).toBeGreaterThan(0); done(); }); }); From b7edf4b3175a4f5c5b35ddd4e426e6f0dab42662 Mon Sep 17 00:00:00 2001 From: stevestencil Date: Tue, 31 Mar 2020 15:20:53 -0400 Subject: [PATCH 34/36] Update FilesAdapter.js fixed comment --- 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 63ec01f455..26532840b9 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -30,10 +30,10 @@ export class FilesAdapter { * @param {string} filename - the filename to save * @param {*} data - the buffer of data from the file * @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) * - 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 the contentType can be undefined if the controller was not able to determine it (https://docs.aws.amazon.com/AmazonS3/latest/user-guide/add-object-metadata.html) * * @return {Promise} a promise that should fail if the storage didn't succeed */ From bed35c9c9ef475e1e7675922e8bec4c1481ca06d Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Wed, 1 Apr 2020 09:59:02 -0400 Subject: [PATCH 35/36] added test for changing file name --- spec/CloudCode.spec.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 53dd8fc46e..bbd51d2a99 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -2724,6 +2724,21 @@ describe('beforeLogin hook', () => { expect(createFileSpy).toHaveBeenCalledWith(jasmine.any(String), jasmine.any(Buffer), 'text/plain', options); }); + it('beforeSaveFile should return same file data with new file name', async () => { + await reconfigureServer({ filesAdapter: mockAdapter }); + const config = Config.get('test'); + config.filesController.options.preserveFileName = true; + Parse.Cloud.beforeSaveFile(async ({ file }) => { + expect(file.name()).toBe('popeye.txt'); + const fileData = await file.getData(); + const newFile = new Parse.File('2020-04-01.txt', { base64: fileData }); + return newFile; + }); + const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain'); + const result = await file.save({ useMasterKey: true }); + expect(result.name()).toBe('2020-04-01.txt'); + }); + it('afterSaveFile should set fileSize to null if beforeSave returns an already saved file', async () => { await reconfigureServer({ filesAdapter: mockAdapter }); const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough(); From 859a6039b9b453a6c3dbcd44760496b99351aa2c Mon Sep 17 00:00:00 2001 From: Steve Stencil Date: Wed, 1 Apr 2020 14:10:17 -0400 Subject: [PATCH 36/36] updated comments --- spec/FilesController.spec.js | 3 --- src/Adapters/Files/FilesAdapter.js | 1 + src/cloud-code/Parse.Cloud.js | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index 6592942459..0ccc4ad074 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -70,9 +70,6 @@ describe('FilesController', () => { expect(log1.level).toBe('error'); const log2 = logs.find( - // we can no longer expect 'Could not store file: yolo.txt' because we now - // want to return the actual error message that can possibly come from - // the beforeSaveFile hook x => x.message === 'it failed with xyz' ); expect(log2.level).toBe('error'); diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index 63ec01f455..09b1adbcf7 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -33,6 +33,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 sotred with file (https://docs.aws.amazon.com/AmazonS3/latest/user-guide/add-object-metadata.html) + * Note: options are not supported by all file adapters. Check the your adapter's documentation for compatibility * @discussion the contentType can be undefined if the controller was not able to determine it (https://docs.aws.amazon.com/AmazonS3/latest/user-guide/add-object-metadata.html) * * @return {Promise} a promise that should fail if the storage didn't succeed diff --git a/src/cloud-code/Parse.Cloud.js b/src/cloud-code/Parse.Cloud.js index 350c658b04..c4bd380b1e 100644 --- a/src/cloud-code/Parse.Cloud.js +++ b/src/cloud-code/Parse.Cloud.js @@ -362,7 +362,7 @@ ParseCloud.afterFind = function(parseClass, handler) { }; /** - * Registers an before save file function. + * Registers a before save file function. * * **Available in Cloud Code only.** * @@ -408,7 +408,7 @@ ParseCloud.afterSaveFile = function(handler) { }; /** - * Registers an before delete file function. + * Registers a before delete file function. * * **Available in Cloud Code only.** *