Add circular dependency detection to CI (#7316)

* add circular dependency detection to CI

* fixed Auth-RestWrite circular dependency

* updated package lock

* fixed Logger circular dependency

* fix lint

.github/workflows/ci.yml

@@ -52,6 +52,26 @@ jobs:
        - name: Install dependencies
          run: npm ci
        - run: npm run lint
+  check-circular:
+     name: Circular Dependencies
+     timeout-minutes: 5
+     runs-on: ubuntu-18.04
+     steps:
+       - uses: actions/checkout@v2
+       - name: Use Node.js ${{ matrix.NODE_VERSION }}
+         uses: actions/setup-node@v1
+         with:
+           node-version: ${{ matrix.node-version }}
+       - name: Cache Node.js modules
+         uses: actions/cache@v2
+         with:
+           path: ~/.npm
+           key: ${{ runner.os }}-node-${{ matrix.NODE_VERSION }}-${{ hashFiles('**/package-lock.json') }}
+           restore-keys: |
+             ${{ runner.os }}-node-${{ matrix.NODE_VERSION }}-
+       - name: Install dependencies
+         run: npm ci
+       - run: npm run madge:circular
   check-mongo:
     strategy:
       matrix:

package.json

@@ -91,6 +91,7 @@
     "jsdoc": "3.6.3",
     "jsdoc-babel": "0.5.0",
     "lint-staged": "10.2.3",
+    "madge": "4.0.2",
     "mock-mail-adapter": "file:spec/support/MockMailAdapter",
     "mongodb-runner": "4.8.1",
     "mongodb-version-list": "1.0.0",
@@ -122,7 +123,8 @@
     "start": "node ./bin/parse-server",
     "prettier": "prettier --write '{src,spec}/{**/*,*}.js'",
     "prepare": "npm run build",
-    "postinstall": "node -p 'require(\"./postinstall.js\")()'"
+    "postinstall": "node -p 'require(\"./postinstall.js\")()'",
+    "madge:circular": "node_modules/.bin/madge ./src --circular"
   },
   "engines": {
     "node": ">= 8"

src/Auth.js

@@ -1,4 +1,3 @@
-const cryptoUtils = require('./cryptoUtils');
 const RestQuery = require('./RestQuery');
 const Parse = require('parse/node');
 
@@ -299,39 +298,6 @@ Auth.prototype._getAllRolesNamesForRoleIds = function (roleIDs, names = [], quer
     });
 };
 
-const createSession = function (
-  config,
-  { userId, createdWith, installationId, additionalSessionData }
-) {
-  const token = 'r:' + cryptoUtils.newToken();
-  const expiresAt = config.generateSessionExpiresAt();
-  const sessionData = {
-    sessionToken: token,
-    user: {
-      __type: 'Pointer',
-      className: '_User',
-      objectId: userId,
-    },
-    createdWith,
-    restricted: false,
-    expiresAt: Parse._encode(expiresAt),
-  };
-
-  if (installationId) {
-    sessionData.installationId = installationId;
-  }
-
-  Object.assign(sessionData, additionalSessionData);
-  // We need to import RestWrite at this point for the cyclic dependency it has to it
-  const RestWrite = require('./RestWrite');
-
-  return {
-    sessionData,
-    createSession: () =>
-      new RestWrite(config, master(config), '_Session', null, sessionData).execute(),
-  };
-};
-
 module.exports = {
   Auth,
   master,
@@ -339,5 +305,4 @@ module.exports = {
   readOnly,
   getAuthForSessionToken,
   getAuthForLegacySessionToken,
-  createSession,
 };

src/Controllers/AdaptableController.js

@@ -10,7 +10,6 @@ based on the parameters passed
 
 // _adapter is private, use Symbol
 var _adapter = Symbol();
-import Config from '../Config';
 
 export class AdaptableController {
   constructor(adapter, appId, options) {
@@ -28,10 +27,6 @@ export class AdaptableController {
     return this[_adapter];
   }
 
-  get config() {
-    return Config.get(this.appId);
-  }
-
   expectedAdapterType() {
     throw new Error('Subclasses should implement expectedAdapterType()');
   }

src/Controllers/UserController.js

@@ -5,6 +5,7 @@ import MailAdapter from '../Adapters/Email/MailAdapter';
 import rest from '../rest';
 import Parse from 'parse/node';
 import AccountLockout from '../AccountLockout';
+import Config from '../Config';
 
 var RestQuery = require('../RestQuery');
 var Auth = require('../Auth');
@@ -14,6 +15,10 @@ export class UserController extends AdaptableController {
     super(adapter, appId, options);
   }
 
+  get config() {
+    return Config.get(this.appId);
+  }
+
   validateAdapter(adapter) {
     // Allow no adapter
     if (!adapter && !this.shouldVerifyEmails) {

src/RestWrite.js

@@ -857,7 +857,7 @@ RestWrite.prototype.createSessionToken = async function () {
     return;
   }
 
-  const { sessionData, createSession } = Auth.createSession(this.config, {
+  const { sessionData, createSession } = RestWrite.createSession(this.config, {
     userId: this.objectId(),
     createdWith: {
       action: this.storage['authProvider'] ? 'login' : 'signup',
@@ -873,6 +873,37 @@ RestWrite.prototype.createSessionToken = async function () {
   return createSession();
 };
 
+RestWrite.createSession = function (
+  config,
+  { userId, createdWith, installationId, additionalSessionData }
+) {
+  const token = 'r:' + cryptoUtils.newToken();
+  const expiresAt = config.generateSessionExpiresAt();
+  const sessionData = {
+    sessionToken: token,
+    user: {
+      __type: 'Pointer',
+      className: '_User',
+      objectId: userId,
+    },
+    createdWith,
+    restricted: false,
+    expiresAt: Parse._encode(expiresAt),
+  };
+
+  if (installationId) {
+    sessionData.installationId = installationId;
+  }
+
+  Object.assign(sessionData, additionalSessionData);
+
+  return {
+    sessionData,
+    createSession: () =>
+      new RestWrite(config, Auth.master(config), '_Session', null, sessionData).execute(),
+  };
+};
+
 // Delete email reset tokens if user is changing password or email.
 RestWrite.prototype.deleteEmailResetTokenIfNeeded = function () {
   if (this.className !== '_User' || this.query === null) {
@@ -978,7 +1009,7 @@ RestWrite.prototype.handleSession = function () {
       additionalSessionData[key] = this.data[key];
     }
 
-    const { sessionData, createSession } = Auth.createSession(this.config, {
+    const { sessionData, createSession } = RestWrite.createSession(this.config, {
       userId: this.auth.user.id,
       createdWith: {
         action: 'create',
@@ -1258,7 +1289,7 @@ RestWrite.prototype.handleInstallation = function () {
   return promise;
 };
 
-// If we short-circuted the object response - then we need to make sure we expand all the files,
+// If we short-circuited the object response - then we need to make sure we expand all the files,
 // since this might not have a query, meaning it won't return the full result back.
 // TODO: (nlutsenko) This should die when we move to per-class based controllers on _Session/_User
 RestWrite.prototype.expandFilesForExistingObjects = function () {

src/Routers/SessionsRouter.js

@@ -2,6 +2,7 @@ import ClassesRouter from './ClassesRouter';
 import Parse from 'parse/node';
 import rest from '../rest';
 import Auth from '../Auth';
+import RestWrite from '../RestWrite';
 
 export class SessionsRouter extends ClassesRouter {
   className() {
@@ -41,7 +42,7 @@ export class SessionsRouter extends ClassesRouter {
     if (!user) {
       throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'invalid session');
     }
-    const { sessionData, createSession } = Auth.createSession(config, {
+    const { sessionData, createSession } = RestWrite.createSession(config, {
       userId: user.id,
       createdWith: {
         action: 'upgrade',

src/Routers/UsersRouter.js

@@ -9,6 +9,7 @@ import Auth from '../Auth';
 import passwordCrypto from '../password';
 import { maybeRunTrigger, Types as TriggerTypes } from '../triggers';
 import { promiseEnsureIdempotency } from '../middlewares';
+import RestWrite from '../RestWrite';
 
 export class UsersRouter extends ClassesRouter {
   className() {
@@ -218,7 +219,7 @@ export class UsersRouter extends ClassesRouter {
       req.config
     );
 
-    const { sessionData, createSession } = Auth.createSession(req.config, {
+    const { sessionData, createSession } = RestWrite.createSession(req.config, {
       userId: user.objectId,
       createdWith: {
         action: 'login',