refactor models/page.js

src/server/models/page.js

@@ -96,14 +96,6 @@ const extractToAncestorsPaths = (pagePath) => {
   return ancestorsPaths;
 };
 
-const addSlashOfEnd = (path) => {
-  let returnPath = path;
-  if (!path.match(/\/$/)) {
-    returnPath += '/';
-  }
-  return returnPath;
-};
-
 /**
  * populate page (Query or Document) to show revision
  * @param {any} page Query or Document
@@ -148,15 +140,50 @@ class PageQueryBuilder {
   }
 
   /**
-   * generate the query to find the page that is match with `path` and its descendants
+   * generate the query to find the pages '{path}/*' and '{path}' self.
+   * If top page, return without doing anything.
    */
   addConditionToListWithDescendants(path, option) {
-    // ignore other pages than descendants
-    // eslint-disable-next-line no-param-reassign
-    path = addSlashOfEnd(path);
+    // No request is set for the top page
+    if (isTopPage(path)) {
+      return this;
+    }
+
+    const pathNormalized = pathUtils.normalizePath(path);
+    const pathWithTrailingSlash = pathUtils.addTrailingSlash(path);
+
+    const startsPattern = escapeStringRegexp(pathWithTrailingSlash);
+
+    this.query = this.query
+      .and({
+        $or: [
+          { path: pathNormalized },
+          { path: new RegExp(`^${startsPattern}`) },
+        ],
+      });
+
+    return this;
+  }
+
+  /**
+   * generate the query to find the pages '{path}/*' (exclude '{path}' self).
+   * If top page, return without doing anything.
+   */
+  addConditionToListOnlyDescendants(path, option) {
+    // No request is set for the top page
+    if (isTopPage(path)) {
+      return this;
+    }
+
+    const pathWithTrailingSlash = pathUtils.addTrailingSlash(path);
+
+    const startsPattern = escapeStringRegexp(pathWithTrailingSlash);
+
+    this.query = this.query
+      .and({ path: new RegExp(`^${startsPattern}`) });
 
-    this.addConditionToListByStartWith(path, option);
     return this;
+
   }
 
   /**
@@ -173,36 +200,11 @@ class PageQueryBuilder {
     if (isTopPage(path)) {
       return this;
     }
-    const pathCondition = [];
-
-    /*
-     * 1. add condition for finding the page completely match with `path` w/o last slash
-     */
-    let pathSlashOmitted = path;
-    if (path.match(/\/$/)) {
-      pathSlashOmitted = path.substr(0, path.length - 1);
-      pathCondition.push({ path: pathSlashOmitted });
-    }
-    /*
-     * 2. add decendants
-     */
-    const pattern = escapeStringRegexp(path); // escape
 
-    let queryReg;
-    try {
-      queryReg = new RegExp(`^${pattern}`);
-    }
-    // if regular expression is invalid
-    catch (e) {
-      // force to escape
-      queryReg = new RegExp(`^${escapeStringRegexp(pattern)}`);
-    }
-    pathCondition.push({ path: queryReg });
+    const startsPattern = escapeStringRegexp(path);
 
     this.query = this.query
-      .and({
-        $or: pathCondition,
-      });
+      .and({ path: new RegExp(`^${startsPattern}`) });
 
     return this;
   }
@@ -1386,13 +1388,6 @@ module.exports = function(crowi) {
 
   };
 
-  /**
-   * return path that added slash to the end for specified path
-   */
-  pageSchema.statics.addSlashOfEnd = function(path) {
-    return addSlashOfEnd(path);
-  };
-
   pageSchema.statics.GRANT_PUBLIC = GRANT_PUBLIC;
   pageSchema.statics.GRANT_RESTRICTED = GRANT_RESTRICTED;
   pageSchema.statics.GRANT_SPECIFIED = GRANT_SPECIFIED;

src/test/models/page.test.js

@@ -11,6 +11,7 @@ describe('Page', () => {
   // eslint-disable-next-line no-unused-vars
   let crowi;
   let Page;
+  let PageQueryBuilder;
   let User;
   let UserGroup;
   let UserGroupRelation;
@@ -22,6 +23,8 @@ describe('Page', () => {
     UserGroup = mongoose.model('UserGroup');
     UserGroupRelation = mongoose.model('UserGroupRelation');
     Page = mongoose.model('Page');
+    PageQueryBuilder = Page.PageQueryBuilder;
+
 
     await User.insertMany([
       { name: 'Anon 0', username: 'anonymous0', email: 'anonymous0@example.com' },
@@ -297,42 +300,78 @@ describe('Page', () => {
     });
   });
 
-  describe('findListWithDescendants', () => {
-    test('should return only /page/', async() => {
-      const result = await Page.findListWithDescendants('/page/', testUser0, { isRegExpEscapedFromPath: true });
+  describe('PageQueryBuilder.addConditionToListWithDescendants', () => {
+    test('can retrieve descendants of /page', async() => {
+      const builder = new PageQueryBuilder(Page.find());
+      builder.addConditionToListWithDescendants('/page');
+
+      const result = await builder.query.exec();
 
       // assert totalCount
-      expect(result.totalCount).toEqual(1);
+      expect(result.length).toEqual(1);
       // assert paths
-      const pagePaths = result.pages.map((page) => { return page.path });
+      const pagePaths = result.map((page) => { return page.path });
       expect(pagePaths).toContainEqual('/page/for/extended');
     });
 
-    test('should return only /page1/', async() => {
-      const result = await Page.findListWithDescendants('/page1/', testUser0, { isRegExpEscapedFromPath: true });
+    test('can retrieve descendants of /page1', async() => {
+      const builder = new PageQueryBuilder(Page.find());
+      builder.addConditionToListWithDescendants('/page1/');
+
+      const result = await builder.query.exec();
 
       // assert totalCount
-      expect(result.totalCount).toEqual(2);
+      expect(result.length).toEqual(2);
       // assert paths
-      const pagePaths = result.pages.map((page) => { return page.path });
+      const pagePaths = result.map((page) => { return page.path });
       expect(pagePaths).toContainEqual('/page1');
       expect(pagePaths).toContainEqual('/page1/child1');
     });
   });
 
-  describe('findListByStartWith', () => {
-    test('should return pages which starts with /page', async() => {
-      const result = await Page.findListByStartWith('/page', testUser0, {});
+  describe('PageQueryBuilder.addConditionToListOnlyDescendants', () => {
+    test('can retrieve only descendants of /page', async() => {
+      const builder = new PageQueryBuilder(Page.find());
+      builder.addConditionToListOnlyDescendants('/page');
+
+      const result = await builder.query.exec();
+
+      // assert totalCount
+      expect(result.length).toEqual(1);
+      // assert paths
+      const pagePaths = result.map((page) => { return page.path });
+      expect(pagePaths).toContainEqual('/page/for/extended');
+    });
+
+    test('can retrieve only descendants of /page1', async() => {
+      const builder = new PageQueryBuilder(Page.find());
+      builder.addConditionToListOnlyDescendants('/page1');
+
+      const result = await builder.query.exec();
+
+      // assert totalCount
+      expect(result.length).toEqual(1);
+      // assert paths
+      const pagePaths = result.map((page) => { return page.path });
+      expect(pagePaths).toContainEqual('/page1/child1');
+    });
+  });
+
+  describe('PageQueryBuilder.addConditionToListByStartWith', () => {
+    test('can retrieve pages which starts with /page', async() => {
+      const builder = new PageQueryBuilder(Page.find());
+      builder.addConditionToListByStartWith('/page');
+
+      const result = await builder.query.exec();
 
       // assert totalCount
-      expect(result.totalCount).toEqual(4);
+      expect(result.length).toEqual(4);
       // assert paths
-      const pagePaths = result.pages.map((page) => { return page.path });
+      const pagePaths = result.map((page) => { return page.path });
       expect(pagePaths).toContainEqual('/page/for/extended');
       expect(pagePaths).toContainEqual('/page1');
       expect(pagePaths).toContainEqual('/page1/child1');
       expect(pagePaths).toContainEqual('/page2');
     });
-
   });
 });