Merge pull request #1335 from weseek/imprv/organize-mongodb-indices

Imprv/organize mongodb indices

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',

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/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/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/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);