Merge branch 'reactify-admin/external-account' into external-account-reflect-api

.vscode/launch.json

@@ -4,6 +4,12 @@
     // 詳細情報は次を確認してください: https://go.microsoft.com/fwlink/?linkid=830387
     "version": "0.2.0",
     "configurations": [
+      {
+        "type": "node",
+        "request": "attach",
+        "name": "Debug: Attach Debugger to Server",
+        "port": 9229
+      },
       {
         "type": "node",
         "request": "launch",
@@ -11,10 +17,9 @@
         "runtimeExecutable": "npm",
         "runtimeArgs": [
           "run",
-          "server:debug"
+          "server:nolazy"
         ],
         "port": 9229,
-        "timeout": 30000,
         "restart": true,
         "console": "integratedTerminal",
         "internalConsoleOptions": "neverOpen"

CHANGES.md

@@ -2,7 +2,8 @@
 
 ## 3.5.19-RC
 
-* 
+* Improvement: Drop unnecessary MongoDB collection indexes
+* Improvement: Organize MongoDB collection indexes uniqueness
 
 ## 3.5.18
 

config/jest.config.js

@@ -1,6 +1,15 @@
 // For a detailed explanation regarding each configuration property, visit:
 // https://jestjs.io/docs/en/configuration.html
 
+const MODULE_NAME_MAPPING = {
+  '@root/(.+)': '<rootDir>/$1',
+  '@commons/(.+)': '<rootDir>/src/lib/$1',
+  '@server/(.+)': '<rootDir>/src/server/$1',
+  '@alias/logger': '<rootDir>/src/lib/service/logger',
+  // -- doesn't work with unknown error -- 2019.06.19 Yuki Takei
+  // debug: '<rootDir>/src/lib/service/logger/alias-for-debug',
+};
+
 module.exports = {
   // Indicates whether each individual test should be reported during the run
   verbose: true,
@@ -19,14 +28,7 @@ module.exports = {
       // Automatically clear mock calls and instances between every test
       clearMocks: true,
       // A map from regular expressions to module names that allow to stub out resources with a single module
-      moduleNameMapper: {
-        '@root/(.+)': '<rootDir>/$1',
-        '@commons/(.+)': '<rootDir>/src/lib/$1',
-        '@server/(.+)': '<rootDir>/src/server/$1',
-        '@alias/logger': '<rootDir>/src/lib/service/logger',
-        // -- doesn't work with unknown error -- 2019.06.19 Yuki Takei
-        // debug: '<rootDir>/src/lib/service/logger/alias-for-debug',
-      },
+      moduleNameMapper: MODULE_NAME_MAPPING,
     },
     // {
     //   displayName: 'client',

package.json

@@ -53,8 +53,8 @@
     "preserver:prod": "npm run migrate",
     "prestart": "npm run build:prod",
     "resource": "node bin/download-cdn-resources.js",
-    "server:debug": "env-cmd -f config/env.dev.js node-dev --inspect src/server/app.js",
-    "server:dev": "env-cmd -f config/env.dev.js node-dev --respawn src/server/app.js",
+    "server:nolazy": "env-cmd -f config/env.dev.js node-dev --nolazy --inspect src/server/app.js",
+    "server:dev": "env-cmd -f config/env.dev.js node-dev --inspect src/server/app.js",
     "server:prod:ci": "npm run server:prod -- --ci",
     "server:prod": "env-cmd -f config/env.prod.js node src/server/app.js",
     "server": "npm run server:dev",

src/migrations/20191102223900-drop-configs-indices.js

@@ -0,0 +1,28 @@
+require('module-alias/register');
+const logger = require('@alias/logger')('growi:migrate:drop-configs-indices');
+
+const mongoose = require('mongoose');
+const config = require('@root/config/migrate');
+
+async function dropIndexIfExists(collection, indexName) {
+  if (await collection.indexExists(indexName)) {
+    await collection.dropIndex(indexName);
+  }
+}
+
+module.exports = {
+  async up(db) {
+    logger.info('Apply migration');
+    mongoose.connect(config.mongoUri, config.mongodb.options);
+
+    const collection = db.collection('configs');
+    await dropIndexIfExists(collection, 'ns_1');
+    await dropIndexIfExists(collection, 'key_1');
+
+    logger.info('Migration has successfully applied');
+  },
+
+  down(db) {
+    // do not rollback
+  },
+};

src/migrations/20191102223901-drop-pages-indices.js

@@ -0,0 +1,29 @@
+require('module-alias/register');
+const logger = require('@alias/logger')('growi:migrate:drop-pages-indices');
+
+const mongoose = require('mongoose');
+const config = require('@root/config/migrate');
+
+async function dropIndexIfExists(collection, indexName) {
+  if (await collection.indexExists(indexName)) {
+    await collection.dropIndex(indexName);
+  }
+}
+
+module.exports = {
+  async up(db) {
+    logger.info('Apply migration');
+    mongoose.connect(config.mongoUri, config.mongodb.options);
+
+    const collection = db.collection('pages');
+    await dropIndexIfExists(collection, 'lastUpdateUser_1');
+    await dropIndexIfExists(collection, 'liker_1');
+    await dropIndexIfExists(collection, 'seenUsers_1');
+
+    logger.info('Migration has successfully applied');
+  },
+
+  down(db) {
+    // do not rollback
+  },
+};

src/server/middlewares/ApiV3FormValidator.js

@@ -1,11 +1,11 @@
 const logger = require('@alias/logger')('growi:middlewares:ApiV3FormValidator');
 const { validationResult } = require('express-validator/check');
 
+const ErrorV3 = require('../models/vo/error-apiv3');
+
 class ApiV3FormValidator {
 
   constructor(crowi) {
-    const { ErrorV3 } = crowi.models;
-
     return (req, res, next) => {
       logger.debug('req.query', req.query);
       logger.debug('req.params', req.params);

src/server/models/attachment.js

@@ -6,6 +6,7 @@ const logger = require('@alias/logger')('growi:models:attachment');
 const path = require('path');
 
 const mongoose = require('mongoose');
+const uniqueValidator = require('mongoose-unique-validator');
 
 const ObjectId = mongoose.Schema.Types.ObjectId;
 
@@ -21,12 +22,13 @@ module.exports = function(crowi) {
     page: { type: ObjectId, ref: 'Page', index: true },
     creator: { type: ObjectId, ref: 'User', index: true },
     filePath: { type: String }, // DEPRECATED: remains for backward compatibility for v3.3.x or below
-    fileName: { type: String, required: true },
+    fileName: { type: String, required: true, unique: true },
     originalName: { type: String },
     fileFormat: { type: String, required: true },
     fileSize: { type: Number, default: 0 },
     createdAt: { type: Date, default: Date.now },
   });
+  attachmentSchema.plugin(uniqueValidator);
 
   attachmentSchema.virtual('filePathProxied').get(function() {
     return `/attachment/${this._id}`;

src/server/models/bookmark.js

@@ -1,10 +1,12 @@
-// disable no-return-await for model functions
 /* eslint-disable no-return-await */
 
+const debug = require('debug')('growi:models:bookmark');
+const mongoose = require('mongoose');
+const uniqueValidator = require('mongoose-unique-validator');
+
+const ObjectId = mongoose.Schema.Types.ObjectId;
+
 module.exports = function(crowi) {
-  const debug = require('debug')('growi:models:bookmark');
-  const mongoose = require('mongoose');
-  const ObjectId = mongoose.Schema.Types.ObjectId;
   const bookmarkEvent = crowi.event('bookmark');
 
   let bookmarkSchema = null;
@@ -16,6 +18,7 @@ module.exports = function(crowi) {
     createdAt: { type: Date, default: Date.now },
   });
   bookmarkSchema.index({ page: 1, user: 1 }, { unique: true });
+  bookmarkSchema.plugin(uniqueValidator);
 
   bookmarkSchema.statics.countByPageId = async function(pageId) {
     return await this.count({ page: pageId });

src/server/models/config.js

@@ -1,21 +1,22 @@
-// disable no-return-await for model functions
-/* eslint-disable no-return-await */
-
-/* eslint-disable no-use-before-define */
+const mongoose = require('mongoose');
+const uniqueValidator = require('mongoose-unique-validator');
 
 module.exports = function(crowi) {
-  const mongoose = require('mongoose');
 
   const configSchema = new mongoose.Schema({
-    ns: { type: String, required: true, index: true },
-    key: { type: String, required: true, index: true },
+    ns: { type: String, required: true },
+    key: { type: String, required: true },
     value: { type: String, required: true },
   });
+  // define unique compound index
+  configSchema.index({ ns: 1, key: 1 }, { unique: true });
+  configSchema.plugin(uniqueValidator);
 
   /**
    * default values when GROWI is cleanly installed
    */
   function getConfigsForInstalling() {
+    // eslint-disable-next-line no-use-before-define
     const config = getDefaultCrowiConfigs();
 
     // overwrite

src/server/models/index.js

@@ -15,7 +15,4 @@ module.exports = {
   GlobalNotificationSetting: require('./GlobalNotificationSetting'),
   GlobalNotificationMailSetting: require('./GlobalNotificationSetting/GlobalNotificationMailSetting'),
   GlobalNotificationSlackSetting: require('./GlobalNotificationSetting/GlobalNotificationSlackSetting'),
-
-  // non-persistent models
-  ErrorV3: require('./ErrorV3'),
 };

src/server/models/page-tag-relation.js

@@ -5,6 +5,7 @@ const flatMap = require('array.prototype.flatmap');
 
 const mongoose = require('mongoose');
 const mongoosePaginate = require('mongoose-paginate-v2');
+const uniqueValidator = require('mongoose-unique-validator');
 
 const ObjectId = mongoose.Schema.Types.ObjectId;
 
@@ -17,6 +18,7 @@ const schema = new mongoose.Schema({
     type: ObjectId,
     ref: 'Page',
     required: true,
+    index: true,
   },
   relatedTag: {
     type: ObjectId,
@@ -24,7 +26,10 @@ const schema = new mongoose.Schema({
     required: true,
   },
 });
+// define unique compound index
+schema.index({ page: 1, user: 1 }, { unique: true });
 schema.plugin(mongoosePaginate);
+schema.plugin(uniqueValidator);
 
 /**
  * PageTagRelation Class

src/server/models/page.js

@@ -39,9 +39,9 @@ const pageSchema = new mongoose.Schema({
   grantedUsers: [{ type: ObjectId, ref: 'User' }],
   grantedGroup: { type: ObjectId, ref: 'UserGroup', index: true },
   creator: { type: ObjectId, ref: 'User', index: true },
-  lastUpdateUser: { type: ObjectId, ref: 'User', index: true },
-  liker: [{ type: ObjectId, ref: 'User', index: true }],
-  seenUsers: [{ type: ObjectId, ref: 'User', index: true }],
+  lastUpdateUser: { type: ObjectId, ref: 'User' },
+  liker: [{ type: ObjectId, ref: 'User' }],
+  seenUsers: [{ type: ObjectId, ref: 'User' }],
   commentCount: { type: Number, default: 0 },
   extended: {
     type: String,

src/server/models/tag.js

@@ -3,6 +3,7 @@
 
 const mongoose = require('mongoose');
 const mongoosePaginate = require('mongoose-paginate-v2');
+const uniqueValidator = require('mongoose-unique-validator');
 
 /*
  * define schema
@@ -15,6 +16,7 @@ const schema = new mongoose.Schema({
   },
 });
 schema.plugin(mongoosePaginate);
+schema.plugin(uniqueValidator);
 
 /**
  * Tag Class

src/server/models/user-group-relation.js

@@ -1,6 +1,7 @@
 const debug = require('debug')('growi:models:userGroupRelation');
 const mongoose = require('mongoose');
 const mongoosePaginate = require('mongoose-paginate-v2');
+const uniqueValidator = require('mongoose-unique-validator');
 
 const ObjectId = mongoose.Schema.Types.ObjectId;
 
@@ -14,6 +15,8 @@ const schema = new mongoose.Schema({
   createdAt: { type: Date, default: Date.now, required: true },
 });
 schema.plugin(mongoosePaginate);
+schema.plugin(uniqueValidator);
+
 
 /**
  * UserGroupRelation Class

src/server/models/user.js

@@ -44,14 +44,14 @@ module.exports = function(crowi) {
     name: { type: String },
     username: { type: String, required: true, unique: true },
     email: { type: String, unique: true, sparse: true },
-    // === The official settings
+    // === Crowi settings
     // username: { type: String, index: true },
     // email: { type: String, required: true, index: true },
     // === crowi-plus (>= 2.1.0, <2.3.0) settings
     // email: { type: String, required: true, unique: true },
-    introduction: { type: String },
+    introduction: String,
     password: String,
-    apiToken: String,
+    apiToken: { type: String, index: true },
     lang: {
       type: String,
       // eslint-disable-next-line no-eval

src/server/models/ErrorV3.js → src/server/models/vo/error-apiv3.js

@@ -9,6 +9,4 @@ class ErrorV3 extends Error {
 
 }
 
-module.exports = function(crowi) {
-  return ErrorV3;
-};
+module.exports = ErrorV3;

src/server/routes/apiv3/markdown-setting.js

@@ -1,6 +1,6 @@
-/* eslint-disable no-unused-vars */
 const loggerFactory = require('@alias/logger');
 
+// eslint-disable-next-line no-unused-vars
 const logger = loggerFactory('growi:routes:apiv3:user-group');
 
 const express = require('express');
@@ -14,13 +14,12 @@ const router = express.Router();
  */
 
 module.exports = (crowi) => {
-  const loginRequiredStrictly = require('../../middleware/login-required')(crowi);
-  const adminRequired = require('../../middleware/admin-required')(crowi);
+  // const loginRequiredStrictly = require('../../middleware/login-required')(crowi);
+  // const adminRequired = require('../../middleware/admin-required')(crowi);
 
-  const {
-    ErrorV3,
-    Config,
-  } = crowi.models;
+  // const {
+  //   Config,
+  // } = crowi.models;
 
   return router;
 };

src/server/routes/apiv3/response.js

@@ -1,7 +1,8 @@
 const toArrayIfNot = require('../../../lib/util/toArrayIfNot');
 
+const ErrorV3 = require('../../models/vo/error-apiv3');
+
 const addCustomFunctionToResponse = (express, crowi) => {
-  const { ErrorV3 } = crowi.models;
 
   express.response.apiv3 = function(obj = {}) { // not arrow function
     // obj must be object

src/server/routes/apiv3/user-group-relation.js

@@ -4,6 +4,8 @@ const logger = loggerFactory('growi:routes:apiv3:user-group-relation'); // eslin
 
 const express = require('express');
 
+const ErrorV3 = require('../../models/vo/error-apiv3');
+
 const router = express.Router();
 
 /**
@@ -16,7 +18,7 @@ module.exports = (crowi) => {
   const loginRequiredStrictly = require('../../middleware/login-required')(crowi);
   const adminRequired = require('../../middleware/admin-required')(crowi);
 
-  const { ErrorV3, UserGroup, UserGroupRelation } = crowi.models;
+  const { UserGroup, UserGroupRelation } = crowi.models;
 
   /**
    * @swagger

src/server/routes/apiv3/user-group.js

@@ -9,12 +9,17 @@ const router = express.Router();
 const { body, param, query } = require('express-validator/check');
 const { sanitizeQuery } = require('express-validator/filter');
 
-const validator = {};
+const mongoose = require('mongoose');
 
-const { ObjectId } = require('mongoose').Types;
+const ErrorV3 = require('../../models/vo/error-apiv3');
 
 const { toPagingLimit, toPagingOffset } = require('../../util/express-validator/sanitizer');
 
+const validator = {};
+
+const { ObjectId } = mongoose.Types;
+
+
 /**
  * @swagger
  *  tags:
@@ -27,7 +32,6 @@ module.exports = (crowi) => {
   const csrf = require('../../middleware/csrf')(crowi);
 
   const {
-    ErrorV3,
     UserGroup,
     UserGroupRelation,
     User,

src/server/routes/apiv3/users.js

@@ -9,6 +9,8 @@ const router = express.Router();
 const { body } = require('express-validator/check');
 const { isEmail } = require('validator');
 
+const ErrorV3 = require('../../models/vo/error-apiv3');
+
 const PAGE_ITEMS = 50;
 
 const validator = {};
@@ -25,7 +27,6 @@ module.exports = (crowi) => {
   const csrf = require('../../middleware/csrf')(crowi);
 
   const {
-    ErrorV3,
     User,
     Page,
     ExternalAccount,

src/server/routes/comment.js

@@ -121,10 +121,13 @@ module.exports = function(crowi, app) {
     }
 
     // update page
-    const page = await Page.findOneAndUpdate({ _id: pageId }, {
-      lastUpdateUser: req.user,
-      updatedAt: new Date(),
-    });
+    const page = await Page.findOneAndUpdate(
+      { _id: pageId },
+      {
+        lastUpdateUser: req.user,
+        updatedAt: new Date(),
+      },
+    );
 
     res.json(ApiResponse.success({ comment: createdComment }));
 

src/test/crowi/crowi.test.js

@@ -1,26 +1,23 @@
-const helpers = require('@commons/util/helpers');
+const { getInstance } = require('../setup-crowi');
 
 describe('Test for Crowi application context', () => {
 
-  const Crowi = require('@server/crowi');
-
   describe('construction', () => {
-    test('initialize crowi context', () => {
-      const crowi = new Crowi(helpers.root());
-      expect(crowi).toBeInstanceOf(Crowi);
+    test('initialize crowi context', async() => {
+      const crowi = await getInstance();
       expect(crowi.version).toBe(require('@root/package.json').version);
       expect(typeof crowi.env).toBe('object');
     });
 
-    test('config getter, setter', () => {
-      const crowi = new Crowi(helpers.root());
+    test('config getter, setter', async() => {
+      const crowi = await getInstance();
       expect(crowi.getConfig()).toEqual({});
       crowi.setConfig({ test: 1 });
       expect(crowi.getConfig()).toEqual({ test: 1 });
     });
 
-    test('model getter, setter', () => {
-      const crowi = new Crowi(helpers.root());
+    test('model getter, setter', async() => {
+      const crowi = await getInstance();
       // set
       crowi.model('hoge', { fuga: 1 });
       expect(crowi.model('hoge')).toEqual({ fuga: 1 });

src/test/global-setup.js

@@ -1,3 +1,5 @@
+require('module-alias/register');
+
 // check env
 if (process.env.NODE_ENV !== 'test') {
   throw new Error('\'process.env.NODE_ENV\' must be \'test\'');
@@ -7,8 +9,18 @@ const mongoose = require('mongoose');
 
 const { getMongoUri } = require('../lib/util/mongoose-utils');
 
+const { getInstance } = require('./setup-crowi');
+
 module.exports = async() => {
   await mongoose.connect(getMongoUri(), { useNewUrlParser: true });
+
+  // drop database
   await mongoose.connection.dropDatabase();
+
+  // init DB
+  const crowi = await getInstance();
+  const appService = crowi.appService;
+  await appService.initDB();
+
   await mongoose.disconnect();
 };

src/test/setup-crowi.js

@@ -8,10 +8,6 @@ async function createInstance() {
   const crowi = new Crowi(helpers.root());
   await crowi.initForTest();
 
-  // init DB
-  const appService = crowi.appService;
-  await appService.initDB();
-
   return crowi;
 }
 

src/test/util/slack.test.js

@@ -1,10 +1,15 @@
-const helpers = require('@commons/util/helpers');
-
-const Crowi = require('@server/crowi');
+const { getInstance } = require('../setup-crowi');
 
 describe('Slack Util', () => {
-  const crowi = new Crowi(helpers.root());
-  const slack = require(`${crowi.libDir}/util/slack`)(crowi);
+
+  let crowi;
+  let slack;
+
+  beforeEach(async(done) => {
+    crowi = await getInstance();
+    slack = require(`${crowi.libDir}/util/slack`)(crowi);
+    done();
+  });
 
   test('post comment method exists', () => {
     expect(slack.postComment).toBeInstanceOf(Function);