Merge pull request #1 from Moumouls/pr-migrations-moumouls

fix: test & review

Author: sadortun

spec/DefinedSchemas.spec.js

@@ -14,12 +14,15 @@ describe('DefinedSchemas', () => {
   let config;
   beforeEach(async () => {
     jasmine.DEFAULT_TIMEOUT_INTERVAL = 100000000;
-
     config = Config.get('test');
-    await config.database.adapter.deleteAllClasses();
+    if (config) {
+      await config.database.adapter.deleteAllClasses();
+    }
   });
   afterAll(async () => {
-    await config.database.adapter.deleteAllClasses();
+    if (config) {
+      await config.database.adapter.deleteAllClasses();
+    }
   });
 
   describe('Fields', () => {
@@ -544,21 +547,17 @@ describe('DefinedSchemas', () => {
     expect(schema.className).toEqual('Test');
 
     const schemas = await Parse.Schema.all();
-    expect(schemas.length).toEqual(3);
+    // Role could be flaky since all system classes are not ensured
+    // at start up by the DefinedSchema system
+    expect(schemas.filter(({ className }) => className !== '_Role').length).toEqual(3);
 
-    try {
-      await new Parse.Schema('TheNewTest').save();
-      fail('TheNewTest.save() should have failed');
-    } catch (e) {
-      expect(e.message).toContain('Cannot perform this operation when schemas options is used.');
-    }
+    await expectAsync(new Parse.Schema('TheNewTest').save()).toBeRejectedWithError(
+      'Cannot perform this operation when schemas options is used.'
+    );
 
-    try {
-      await new Parse.Schema('_User').update();
-      fail('_User.update() should have failed');
-    } catch (e) {
-      expect(e.message).toContain('Cannot perform this operation when schemas options is used.');
-    }
+    await expectAsync(new Parse.Schema('_User').update()).toBeRejectedWithError(
+      'Cannot perform this operation when schemas options is used.'
+    );
   });
   it('should only enable delete class endpoint since', async () => {
     await reconfigureServer({
@@ -574,34 +573,26 @@ describe('DefinedSchemas', () => {
     expect(schemas.length).toEqual(3);
   });
   it('should run beforeMigration before execution of DefinedSchemas', async () => {
-    let before = false;
-    const server = await reconfigureServer({
+    const config = {
       schema: {
         definitions: [{ className: '_User' }, { className: 'Test' }],
-        beforeMigration: async () => {
-          expect(before).toEqual(false);
-          before = true;
-        },
+        beforeMigration: async () => {},
       },
-    });
-    before = true;
-    expect(before).toEqual(true);
-    expect(server).toBeDefined();
+    };
+    const spy = spyOn(config.schema, 'beforeMigration');
+    await reconfigureServer(config);
+    expect(spy).toHaveBeenCalledTimes(1);
   });
   it('should run afterMigration after execution of DefinedSchemas', async () => {
-    let before = false;
-    const server = await reconfigureServer({
+    const config = {
       schema: {
         definitions: [{ className: '_User' }, { className: 'Test' }],
-        afterMigration: async () => {
-          expect(before).toEqual(false);
-          before = true;
-        },
+        afterMigration: async () => {},
       },
-    });
-    before = true;
-    expect(before).toEqual(true);
-    expect(server).toBeDefined();
+    };
+    const spy = spyOn(config.schema, 'afterMigration');
+    await reconfigureServer(config);
+    expect(spy).toHaveBeenCalledTimes(1);
   });
 
   it('should use logger in case of error', async () => {

src/Adapters/Storage/Mongo/MongoSchemaCollection.js

@@ -1,6 +1,6 @@
 import MongoCollection from './MongoCollection';
 import Parse from 'parse/node';
-import _ from 'lodash'
+import _ from 'lodash';
 
 function mongoFieldToParseSchemaField(type) {
   if (type[0] === '*') {
@@ -277,9 +277,8 @@ class MongoSchemaCollection {
   }
 
   async updateFieldOptions(className: string, fieldName: string, fieldType: any) {
-    const fieldOptions = _.cloneDeep(fieldType);
-    delete fieldOptions.type
-    delete fieldOptions.targetClass
+    // eslint-disable-next-line no-unused-vars
+    const { type, targetClass, ...fieldOptions } = fieldType;
     await this.upsertSchema(
       className,
       { [fieldName]: { $exists: true } },

src/Config.js

@@ -13,7 +13,7 @@ import {
   SecurityOptions,
   SchemaOptions,
 } from './Options/Definitions';
-import { isBoolean, isString, isArray } from 'lodash';
+import { isBoolean, isString } from 'lodash';
 
 function removeTrailingSlash(str) {
   if (!str) {
@@ -133,12 +133,13 @@ export class Config {
   }
 
   static validateSchemaOptions(schema: SchemaOptions) {
+    if (!schema) return;
     if (Object.prototype.toString.call(schema) !== '[object Object]') {
       throw 'Parse Server option schema must be an object.';
     }
     if (schema.definitions === undefined) {
       schema.definitions = SchemaOptions.definitions.default;
-    } else if (!isArray(schema.definitions)) {
+    } else if (!Array.isArray(schema.definitions)) {
       throw 'Parse Server option schema.definitions must be an array.';
     }
     if (schema.strict === undefined) {
@@ -164,12 +165,12 @@ export class Config {
     if (schema.beforeMigration === undefined) {
       schema.beforeMigration = null;
     } else if (schema.beforeMigration !== null && typeof schema.beforeMigration !== 'function') {
-      throw 'Parse Server option schema.beforeMigration must be a boolean.';
+      throw 'Parse Server option schema.beforeMigration must be a function.';
     }
     if (schema.afterMigration === undefined) {
       schema.afterMigration = null;
     } else if (schema.afterMigration !== null && typeof schema.afterMigration !== 'function') {
-      throw 'Parse Server option schema.afterMigration must be a boolean.';
+      throw 'Parse Server option schema.afterMigration must be a function.';
     }
   }
 

src/Controllers/SchemaController.js

@@ -1116,11 +1116,10 @@ export default class SchemaController {
       // we can safely return
       if (isValidation || _.isEqual(expectedType, type)) {
         return undefined;
-      } else {
-        // Field options are may be changed
-        // ensure to have an update to date schema field
-        return this._dbAdapter.updateFieldOptions(className, fieldName, type);
       }
+      // Field options are may be changed
+      // ensure to have an update to date schema field
+      return this._dbAdapter.updateFieldOptions(className, fieldName, type);
     }
 
     return this._dbAdapter

src/Routers/SchemasRouter.js

@@ -36,7 +36,7 @@ function getOneSchema(req) {
 }
 
 const checkIfDefinedSchemasIsUsed = req => {
-  if (req.config && req.config.schema && req.config.schema.lockSchemas === true) {
+  if (req.config?.schema?.lockSchemas === true) {
     throw new Parse.Error(
       Parse.Error.OPERATION_FORBIDDEN,
       'Cannot perform this operation when schemas options is used.'
@@ -46,27 +46,27 @@ const checkIfDefinedSchemasIsUsed = req => {
 
 export const internalCreateSchema = async (className, body, config) => {
   const controller = await config.database.loadSchema({ clearCache: true });
+  const response = await controller.addClassIfNotExists(
+    className,
+    body.fields,
+    body.classLevelPermissions,
+    body.indexes
+  );
   return {
-    response: await controller.addClassIfNotExists(
-      className,
-      body.fields,
-      body.classLevelPermissions,
-      body.indexes
-    ),
+    response,
   };
 };
 
 export const internalUpdateSchema = async (className, body, config) => {
   const controller = await config.database.loadSchema({ clearCache: true });
-  return {
-    response: await controller.updateClass(
-      className,
-      body.fields || {},
-      body.classLevelPermissions,
-      body.indexes,
-      config.database
-    ),
-  };
+  const response = await controller.updateClass(
+    className,
+    body.fields || {},
+    body.classLevelPermissions,
+    body.indexes,
+    config.database
+  );
+  return { response };
 };
 
 async function createSchema(req) {

src/SchemaMigrations/DefinedSchemas.js

@@ -18,7 +18,6 @@ export class DefinedSchemas {
     this.localSchemas = [];
     this.config = Config.get(config.appId);
     this.schemaOptions = schemaOptions;
-
     if (schemaOptions && schemaOptions.definitions) {
       if (!Array.isArray(schemaOptions.definitions)) {
         throw `"schema.definitions" must be an array of schemas`;
@@ -66,13 +65,13 @@ export class DefinedSchemas {
   async execute() {
     try {
       logger.info('Running Migrations');
-      if (this.schemaOptions && this.schemaOptions.beforeMigration) {
+      if (this.schemaOptions?.beforeMigration) {
         await Promise.resolve(this.schemaOptions.beforeMigration());
       }
 
       await this.executeMigrations();
 
-      if (this.schemaOptions && this.schemaOptions.afterMigration) {
+      if (this.schemaOptions?.afterMigration) {
         await Promise.resolve(this.schemaOptions.afterMigration());
       }
 
@@ -151,8 +150,8 @@ export class DefinedSchemas {
   }
 
   // Required for testing purpose
-  async wait(time) {
-    await new Promise(resolve => setTimeout(resolve, time));
+  wait(time) {
+    return new Promise(resolve => setTimeout(resolve, time));
   }
 
   async enforceCLPForNonProvidedClass(): void {