devide methods for adding condition to modules

packages/remark-lsx/src/server/routes/list-pages/add-num-condition.ts

@@ -0,0 +1,37 @@
+import type { IPage } from '@growi/core';
+import { OptionParser } from '@growi/core/dist/plugin';
+import createError from 'http-errors';
+import type { Query, Document } from 'mongoose';
+
+
+/**
+ * add num condition that limit fetched pages
+ */
+export const addNumCondition = (query: Query<IPage[], Document>, optionsNum: true | string | number | null): Query<IPage[], Document> => {
+  // when option strings is 'num=', the option value is true
+  if (optionsNum == null || optionsNum === true) {
+    throw createError(400, 'The value of num option is invalid.');
+  }
+
+  if (typeof optionsNum === 'number') {
+    return query.limit(optionsNum);
+  }
+
+  const range = OptionParser.parseRange(optionsNum);
+
+  if (range == null) {
+    return query;
+  }
+
+  const start = range.start;
+  const end = range.end;
+
+  if (start < 1 || end < 1) {
+    throw createError(400, `specified num is [${start}:${end}] : start and end are must be larger than 1`);
+  }
+
+  const skip = start - 1;
+  const limit = end - skip;
+
+  return query.skip(skip).limit(limit);
+};

packages/remark-lsx/src/server/routes/list-pages/add-sort-condition.ts

@@ -0,0 +1,26 @@
+import type { IPage } from '@growi/core';
+import createError from 'http-errors';
+import type { Query, Document } from 'mongoose';
+
+/**
+ * add sort condition(sort key & sort order)
+ *
+ * If only the reverse option is specified, the sort key is 'path'.
+ * If only the sort key is specified, the sort order is the ascending order.
+ *
+ */
+export const addSortCondition = (query: Query<IPage[], Document>, optionsSortArg?: string, optionsReverse?: string): Query<IPage[], Document> => {
+  // init sort key
+  const optionsSort = optionsSortArg ?? 'path';
+
+  // the default sort order
+  const isReversed = optionsReverse === 'true';
+
+  if (optionsSort !== 'path' && optionsSort !== 'createdAt' && optionsSort !== 'updatedAt') {
+    throw createError(400, `The specified value '${optionsSort}' for the sort option is invalid. It must be 'path', 'createdAt' or 'updatedAt'.`);
+  }
+
+  const sortOption = {};
+  sortOption[optionsSort] = isReversed ? -1 : 1;
+  return query.sort(sortOption);
+};

packages/remark-lsx/src/server/routes/list-pages/generate-base-query.ts

@@ -3,13 +3,13 @@ import { model } from 'mongoose';
 import type { Document, Query } from 'mongoose';
 
 export type PageQueryBuilder = {
-  query: Query<IPage, Document>,
+  query: Query<IPage[], Document>,
   addConditionToListOnlyDescendants: (pagePath: string) => PageQueryBuilder,
   addConditionToFilteringByViewerForList: (builder: PageQueryBuilder, user: IUser) => PageQueryBuilder,
 };
 
 export const generateBaseQuery = (pagePath: string, user: IUser): PageQueryBuilder => {
-  const Page = model<IPage>('Page');
+  const Page = model<IPage[]>('Page');
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   const PageAny = Page as any;
 

packages/remark-lsx/src/server/routes/list-pages/index.spec.ts

@@ -1,20 +1,24 @@
-import type { IUser } from '@growi/core';
+import { IPage, IUser } from '@growi/core';
 import type { Request, Response } from 'express';
+import createError from 'http-errors';
+import type { Query, Document } from 'mongoose';
 import { mock } from 'vitest-mock-extended';
 
-import type { PageQueryBuilder } from './generate-base-query';
-
 import { listPages } from '.';
 
 
 // mocking modules
 const mocks = vi.hoisted(() => {
   return {
+    addNumConditionMock: vi.fn(),
+    addSortConditionMock: vi.fn(),
     generateBaseQueryMock: vi.fn(),
     getToppageViewersCountMock: vi.fn(),
   };
 });
 
+vi.mock('./add-num-condition', () => ({ addNumCondition: mocks.addNumConditionMock }));
+vi.mock('./add-sort-condition', () => ({ addSortCondition: mocks.addSortConditionMock }));
 vi.mock('./generate-base-query', () => ({ generateBaseQuery: mocks.generateBaseQueryMock }));
 vi.mock('./get-toppage-viewers-count', () => ({ getToppageViewersCount: mocks.getToppageViewersCountMock }));
 
@@ -32,19 +36,56 @@ describe('listPages', () => {
     await listPages(reqMock, resMock);
 
     // then
-    expect(resMock.status).toBeCalledWith(400);
+    expect(resMock.status).toHaveBeenCalledOnce();
     expect(resStatusMock.send).toHaveBeenCalledOnce();
+    expect(mocks.generateBaseQueryMock).not.toHaveBeenCalled();
   });
 
   describe('with num option', () => {
 
-    it('returns 500 HTTP response when an unexpected error occured', async() => {
+    mocks.generateBaseQueryMock.mockImplementation(() => vi.fn());
+    mocks.getToppageViewersCountMock.mockImplementation(() => 99);
+
+    it('returns 200 HTTP response', async() => {
       // setup
-      const builderMock = mock<PageQueryBuilder>();
-      mocks.generateBaseQueryMock.mockImplementation(() => builderMock);
+      const reqMock = mock<Request & { user: IUser }>();
+      reqMock.query = { pagePath: '/Sandbox' };
+
+      const pageMock = mock<IPage>();
+      const queryMock = mock<Query<IPage[], Document>>();
+      queryMock.exec.mockImplementation(async() => [pageMock]);
+      mocks.addSortConditionMock.mockImplementation(() => queryMock);
+
+      const resMock = mock<Response>();
+      const resStatusMock = mock<Response>();
+      resMock.status.calledWith(200).mockReturnValue(resStatusMock);
 
+      // when
+      await listPages(reqMock, resMock);
+
+      // then
+      expect(mocks.generateBaseQueryMock).toHaveBeenCalledOnce();
+      expect(mocks.getToppageViewersCountMock).toHaveBeenCalledOnce();
+      expect(mocks.addNumConditionMock).toHaveBeenCalledOnce();
+      expect(mocks.addSortConditionMock).toHaveBeenCalledOnce();
+      expect(resMock.status).toHaveBeenCalledOnce();
+      expect(resStatusMock.send).toHaveBeenCalledWith({
+        pages: [pageMock],
+        toppageViewersCount: 99,
+      });
+    });
+
+    it('returns 500 HTTP response when an unexpected error occured', async() => {
+      // setup
       const reqMock = mock<Request & { user: IUser }>();
       reqMock.query = { pagePath: '/Sandbox' };
+
+      // an Error instance will be thrown by addNumConditionMock
+      const expectedError = new Error('error for test');
+      mocks.addNumConditionMock.mockImplementation(() => {
+        throw expectedError;
+      });
+
       const resMock = mock<Response>();
       const resStatusMock = mock<Response>();
       resMock.status.calledWith(500).mockReturnValue(resStatusMock);
@@ -53,8 +94,39 @@ describe('listPages', () => {
       await listPages(reqMock, resMock);
 
       // then
-      expect(resMock.status).toBeCalledWith(500);
-      expect(resStatusMock.send).toHaveBeenCalledOnce();
+      expect(mocks.generateBaseQueryMock).toHaveBeenCalledOnce();
+      expect(mocks.getToppageViewersCountMock).toHaveBeenCalledOnce();
+      expect(mocks.addNumConditionMock).toHaveBeenCalledOnce(); // throw an error
+      expect(mocks.addSortConditionMock).not.toHaveBeenCalledOnce(); // does not called
+      expect(resMock.status).toHaveBeenCalledOnce();
+      expect(resStatusMock.send).toHaveBeenCalledWith(expectedError);
+    });
+
+    it('returns 400 HTTP response when the value is invalid', async() => {
+      // setup
+      const reqMock = mock<Request & { user: IUser }>();
+      reqMock.query = { pagePath: '/Sandbox' };
+
+      // an http-errors instance will be thrown by addNumConditionMock
+      const expectedError = createError(400, 'error for test');
+      mocks.addNumConditionMock.mockImplementation(() => {
+        throw expectedError;
+      });
+
+      const resMock = mock<Response>();
+      const resStatusMock = mock<Response>();
+      resMock.status.calledWith(400).mockReturnValue(resStatusMock);
+
+      // when
+      await listPages(reqMock, resMock);
+
+      // then
+      expect(mocks.generateBaseQueryMock).toHaveBeenCalledOnce();
+      expect(mocks.getToppageViewersCountMock).toHaveBeenCalledOnce();
+      expect(mocks.addNumConditionMock).toHaveBeenCalledOnce(); // throw an error
+      expect(mocks.addSortConditionMock).not.toHaveBeenCalledOnce(); // does not called
+      expect(resMock.status).toHaveBeenCalledOnce();
+      expect(resStatusMock.send).toHaveBeenCalledWith(expectedError);
     });
 
   });

packages/remark-lsx/src/server/routes/list-pages/index.ts

@@ -6,6 +6,8 @@ import escapeStringRegexp from 'escape-string-regexp';
 import type { Request, Response } from 'express';
 import createError, { isHttpError } from 'http-errors';
 
+import { addNumCondition } from './add-num-condition';
+import { addSortCondition } from './add-sort-condition';
 import { generateBaseQuery } from './generate-base-query';
 import { getToppageViewersCount } from './get-toppage-viewers-count';
 
@@ -47,38 +49,6 @@ function addDepthCondition(query, pagePath, optionsDepth) {
   });
 }
 
-/**
- * add num condition that limit fetched pages
- */
-function addNumCondition(query, pagePath, optionsNum) {
-  // when option strings is 'num=', the option value is true
-  if (optionsNum == null || optionsNum === true) {
-    throw createError(400, 'The value of num option is invalid.');
-  }
-
-  if (typeof optionsNum === 'number') {
-    return query.limit(optionsNum);
-  }
-
-  const range = OptionParser.parseRange(optionsNum);
-
-  if (range == null) {
-    return query;
-  }
-
-  const start = range.start;
-  const end = range.end;
-
-  if (start < 1 || end < 1) {
-    throw createError(400, `specified num is [${start}:${end}] : start and end are must be larger than 1`);
-  }
-
-  const skip = start - 1;
-  const limit = end - skip;
-
-  return query.skip(skip).limit(limit);
-}
-
 /**
  * add filter condition that filter fetched pages
  */
@@ -118,34 +88,21 @@ function addExceptCondition(query, pagePath, optionsFilter) {
   return this.addFilterCondition(query, pagePath, optionsFilter, true);
 }
 
-/**
- * add sort condition(sort key & sort order)
- *
- * If only the reverse option is specified, the sort key is 'path'.
- * If only the sort key is specified, the sort order is the ascending order.
- *
- */
-function addSortCondition(query, pagePath, optionsSortArg, optionsReverse) {
-  // init sort key
-  const optionsSort = optionsSortArg ?? 'path';
-
-  // the default sort order
-  const isReversed = optionsReverse === 'true';
-
-  if (optionsSort !== 'path' && optionsSort !== 'createdAt' && optionsSort !== 'updatedAt') {
-    throw createError(400, `The specified value '${optionsSort}' for the sort option is invalid. It must be 'path', 'createdAt' or 'updatedAt'.`);
-  }
 
-  const sortOption = {};
-  sortOption[optionsSort] = isReversed ? -1 : 1;
-  return query.sort(sortOption);
+export type ListPagesOptions = {
+  depth?: string,
+  num?: string,
+  filter?: string,
+  except?: string,
+  sort?: string,
+  reverse?: string,
 }
 
 export const listPages = async(req: Request & { user: IUser }, res: Response): Promise<Response> => {
   const user = req.user;
 
   let pagePath: string;
-  let options: string | undefined;
+  let options: ListPagesOptions | undefined;
 
   try {
     // TODO: use express-validator
@@ -176,21 +133,21 @@ export const listPages = async(req: Request & { user: IUser }, res: Response): P
   let query = builder.query;
   try {
     // depth
-    if (options.depth != null) {
-      query = addDepthCondition(query, pagePath, options.depth);
+    if (options?.depth != null) {
+      query = addDepthCondition(query, pagePath, options?.depth);
     }
     // filter
-    if (options.filter != null) {
-      query = addFilterCondition(query, pagePath, options.filter);
+    if (options?.filter != null) {
+      query = addFilterCondition(query, pagePath, options?.filter);
     }
-    if (options.except != null) {
-      query = addExceptCondition(query, pagePath, options.except);
+    if (options?.except != null) {
+      query = addExceptCondition(query, pagePath, options?.except);
     }
     // num
-    const optionsNum = options.num || DEFAULT_PAGES_NUM;
-    query = addNumCondition(query, pagePath, optionsNum);
+    const optionsNum = options?.num || DEFAULT_PAGES_NUM;
+    query = addNumCondition(query, optionsNum);
     // sort
-    query = addSortCondition(query, pagePath, options.sort, options.reverse);
+    query = addSortCondition(query, options?.sort, options?.reverse);
 
     const pages = await query.exec();
     return res.status(200).send({ pages, toppageViewersCount });