Merge pull request #10795 from growilabs/fix/page-info-for-sharelink

support: Extract `/page/info` endpoint handler into a dedicated module

apps/app/src/server/middlewares/admin-required.ts

@@ -1,5 +1,6 @@
-import type { IUserHasId } from '@growi/core';
+import type { IUser } from '@growi/core';
 import type { NextFunction, Request, Response } from 'express';
+import type { HydratedDocument } from 'mongoose';
 
 import loggerFactory from '~/utils/logger';
 
@@ -7,7 +8,7 @@ import type Crowi from '../crowi';
 
 const logger = loggerFactory('growi:middleware:admin-required');
 
-type RequestWithUser = Request & { user?: IUserHasId };
+type RequestWithUser = Request & { user?: HydratedDocument<IUser> };
 
 type FallbackFunction = (
   req: RequestWithUser,

apps/app/src/server/middlewares/login-required.ts

@@ -1,5 +1,6 @@
-import type { IUserHasId } from '@growi/core';
+import type { IUser } from '@growi/core';
 import type { NextFunction, Request, Response } from 'express';
+import type { HydratedDocument } from 'mongoose';
 
 import { createRedirectToForUnauthenticated } from '~/server/util/createRedirectToForUnauthenticated';
 import loggerFactory from '~/utils/logger';
@@ -10,7 +11,7 @@ import { UserStatus } from '../models/user/conts';
 const logger = loggerFactory('growi:middleware:login-required');
 
 type RequestWithUser = Request & {
-  user?: IUserHasId;
+  user?: HydratedDocument<IUser>;
   isSharedPage?: boolean;
   isBrandLogo?: boolean;
   session?: { redirectTo?: string };

apps/app/src/server/routes/apiv3/page/get-page-info.integ.ts

@@ -0,0 +1,285 @@
+import type { NextFunction, Request, Response } from 'express';
+import express from 'express';
+import mockRequire from 'mock-require';
+import { Types } from 'mongoose';
+import request from 'supertest';
+import { mockDeep } from 'vitest-mock-extended';
+
+import { getInstance } from '^/test/setup/crowi';
+
+import type Crowi from '~/server/crowi';
+import type { PageDocument } from '~/server/models/page';
+import type { ApiV3Response } from '~/server/routes/apiv3/interfaces/apiv3-response';
+import * as findPageModule from '~/server/service/page/find-page-and-meta-data-by-viewer';
+
+// Extend Request type for test
+interface TestRequest extends Request {
+  isSharedPage?: boolean;
+  crowi?: Crowi;
+}
+
+// Passthrough middleware for testing - skips authentication
+const passthroughMiddleware = (
+  _req: Request,
+  _res: Response,
+  next: NextFunction,
+) => next();
+
+// Mock certify-shared-page middleware - sets isSharedPage when shareLinkId is present
+const mockCertifySharedPage = (
+  req: TestRequest,
+  _res: Response,
+  next: NextFunction,
+) => {
+  const { shareLinkId, pageId } = req.query;
+  if (shareLinkId && pageId) {
+    // In real implementation, this checks if shareLink exists and is valid
+    req.isSharedPage = true;
+  }
+  next();
+};
+
+// Mock middlewares using vi.mock (hoisted to top)
+vi.mock('~/server/middlewares/access-token-parser', () => ({
+  accessTokenParser: () => passthroughMiddleware,
+}));
+
+vi.mock('~/server/middlewares/login-required', () => ({
+  default: () => (req: TestRequest, _res: Response, next: NextFunction) => {
+    // Allow access if isSharedPage is true (anonymous user accessing share link)
+    if (req.isSharedPage) {
+      return next();
+    }
+    // For non-shared pages, authentication would be required
+    return next();
+  },
+}));
+
+describe('GET /info', () => {
+  let app: express.Application;
+  let crowi: Crowi;
+
+  // Valid ObjectId strings for testing
+  const validPageId = '507f1f77bcf86cd799439011';
+  const validShareLinkId = '507f1f77bcf86cd799439012';
+
+  beforeAll(async () => {
+    crowi = await getInstance();
+  });
+
+  beforeEach(async () => {
+    // Mock certify-shared-page middleware
+    mockRequire(
+      '../../../middlewares/certify-shared-page',
+      () => mockCertifySharedPage,
+    );
+
+    // Mock findPageAndMetaDataByViewer with default successful response
+    const mockSpy = vi.spyOn(findPageModule, 'findPageAndMetaDataByViewer');
+
+    // Create type-safe mock PageDocument using vitest-mock-extended
+    // Note: mockDeep makes all properties optional, but _id must be required
+    const mockPageDoc = mockDeep<PageDocument>({
+      _id: new Types.ObjectId(validPageId),
+      path: '/test-page',
+      status: 'published',
+      isEmpty: false,
+      grant: 1,
+      descendantCount: 0,
+      commentCount: 0,
+    });
+
+    type PageInfoExt = Exclude<
+      Awaited<
+        ReturnType<typeof findPageModule.findPageAndMetaDataByViewer>
+      >['meta'],
+      { isNotFound: true }
+    >;
+
+    mockSpy.mockResolvedValue({
+      // mockDeep creates DeepMockProxy which conflicts with Required<{_id}>
+      // so we acknowledge this limitation for Mongoose documents
+      data: mockPageDoc as typeof mockPageDoc &
+        Required<{ _id: Types.ObjectId }>,
+      meta: {
+        isNotFound: false,
+        isV5Compatible: true,
+        isEmpty: false,
+        isMovable: false,
+        isDeletable: false,
+        isAbleToDeleteCompletely: false,
+        isRevertible: false,
+        bookmarkCount: 0,
+      } satisfies PageInfoExt,
+    });
+
+    // Setup express app with middleware
+    app = express();
+    app.use(express.json());
+
+    // Add apiv3 response helpers
+    app.use((_req, res: ApiV3Response, next) => {
+      res.apiv3 = (data: unknown) => res.json(data);
+      res.apiv3Err = (error: unknown, statusCode?: number) => {
+        // Validation errors come as arrays and should return 400
+        const status = statusCode ?? (Array.isArray(error) ? 400 : 500);
+        const errorMessage =
+          typeof error === 'object' &&
+          error !== null &&
+          'message' in error &&
+          typeof error.message === 'string'
+            ? error.message
+            : String(error);
+        return res.status(status).json({ error: errorMessage });
+      };
+      next();
+    });
+
+    // Inject crowi instance
+    app.use((req: TestRequest, _res, next) => {
+      req.crowi = crowi;
+      next();
+    });
+
+    // Mount the page router
+    const pageModule = await import('./index');
+    const factoryCandidate =
+      'default' in pageModule ? pageModule.default : pageModule;
+    if (typeof factoryCandidate !== 'function') {
+      throw new Error('Module does not export a router factory function');
+    }
+    const pageRouter = factoryCandidate(crowi);
+    app.use('/', pageRouter);
+  });
+
+  afterEach(() => {
+    // Clean up mocks
+    mockRequire.stopAll();
+    vi.clearAllMocks();
+    vi.restoreAllMocks();
+  });
+
+  describe('Normal page access', () => {
+    it('should return 200 with page meta when pageId is valid', async () => {
+      const response = await request(app)
+        .get('/info')
+        .query({ pageId: validPageId });
+
+      expect(response.status).toBe(200);
+      expect(response.body).toHaveProperty('isNotFound');
+      expect(response.body).toHaveProperty('isV5Compatible');
+      expect(response.body).toHaveProperty('isEmpty');
+      expect(response.body).toHaveProperty('bookmarkCount');
+      expect(response.body.isNotFound).toBe(false);
+    });
+
+    it('should return 403 when page is forbidden', async () => {
+      const mockSpy = vi.spyOn(findPageModule, 'findPageAndMetaDataByViewer');
+      mockSpy.mockResolvedValue({
+        data: null,
+        meta: {
+          isNotFound: true,
+          isForbidden: true,
+        },
+      } satisfies Awaited<
+        ReturnType<typeof findPageModule.findPageAndMetaDataByViewer>
+      >);
+
+      const response = await request(app)
+        .get('/info')
+        .query({ pageId: validPageId });
+
+      expect(response.status).toBe(403);
+      expect(response.body).toHaveProperty('error');
+    });
+
+    it('should return 200 when page is not found but not forbidden', async () => {
+      const mockSpy = vi.spyOn(findPageModule, 'findPageAndMetaDataByViewer');
+      mockSpy.mockResolvedValue({
+        data: null,
+        meta: {
+          isNotFound: true,
+          isForbidden: false,
+        },
+      } satisfies Awaited<
+        ReturnType<typeof findPageModule.findPageAndMetaDataByViewer>
+      >);
+
+      const response = await request(app)
+        .get('/info')
+        .query({ pageId: validPageId });
+
+      expect(response.status).toBe(200);
+      expect(response.body).toHaveProperty('isNotFound');
+      expect(response.body.isNotFound).toBe(true);
+      expect(response.body.isForbidden).toBe(false);
+    });
+  });
+
+  describe('Share link access', () => {
+    it('should return 200 when accessing with both pageId and shareLinkId', async () => {
+      const response = await request(app)
+        .get('/info')
+        .query({ pageId: validPageId, shareLinkId: validShareLinkId });
+
+      expect(response.status).toBe(200);
+      expect(response.body).toHaveProperty('isNotFound');
+      expect(response.body).toHaveProperty('bookmarkCount');
+      expect(response.body.isNotFound).toBe(false);
+    });
+
+    it('should accept shareLinkId as optional parameter', async () => {
+      const response = await request(app)
+        .get('/info')
+        .query({ pageId: validPageId, shareLinkId: validShareLinkId });
+
+      expect(response.status).not.toBe(400); // Should not be validation error
+    });
+  });
+
+  describe('Validation', () => {
+    it('should reject invalid pageId format', async () => {
+      const response = await request(app)
+        .get('/info')
+        .query({ pageId: 'invalid-id' });
+
+      expect(response.status).toBe(400);
+    });
+
+    it('should reject invalid shareLinkId format', async () => {
+      const response = await request(app)
+        .get('/info')
+        .query({ pageId: validPageId, shareLinkId: 'invalid-id' });
+
+      expect(response.status).toBe(400);
+    });
+
+    it('should require pageId parameter', async () => {
+      const response = await request(app).get('/info');
+
+      expect(response.status).toBe(400);
+    });
+
+    it('should work with only pageId (shareLinkId is optional)', async () => {
+      const response = await request(app)
+        .get('/info')
+        .query({ pageId: validPageId });
+
+      expect(response.status).toBe(200);
+    });
+  });
+
+  describe('Error handling', () => {
+    it('should return 500 when service throws an error', async () => {
+      vi.spyOn(findPageModule, 'findPageAndMetaDataByViewer').mockRejectedValue(
+        new Error('Service error'),
+      );
+
+      const response = await request(app)
+        .get('/info')
+        .query({ pageId: validPageId });
+
+      expect(response.status).toBe(500);
+    });
+  });
+});

apps/app/src/server/routes/apiv3/page/get-page-info.ts

@@ -0,0 +1,118 @@
+import type { IUser } from '@growi/core';
+import { isIPageNotFoundInfo, SCOPE } from '@growi/core';
+import { ErrorV3 } from '@growi/core/dist/models';
+import type { Request, RequestHandler } from 'express';
+import { query } from 'express-validator';
+import type { HydratedDocument } from 'mongoose';
+
+import type Crowi from '~/server/crowi';
+import { accessTokenParser } from '~/server/middlewares/access-token-parser';
+import loginRequiredFactory from '~/server/middlewares/login-required';
+import { findPageAndMetaDataByViewer } from '~/server/service/page/find-page-and-meta-data-by-viewer';
+import loggerFactory from '~/utils/logger';
+
+import { apiV3FormValidator } from '../../../middlewares/apiv3-form-validator';
+import type { ApiV3Response } from '../interfaces/apiv3-response';
+
+const logger = loggerFactory('growi:routes:apiv3:page:get-page-info');
+
+// Extend Request to include middleware-added properties
+interface RequestWithAuth extends Request {
+  user?: HydratedDocument<IUser>;
+  isSharedPage?: boolean;
+}
+
+/**
+ * @swagger
+ *
+ *    /page/info:
+ *      get:
+ *        tags: [Page]
+ *        summary: /page/info
+ *        description: Get summary informations for a page
+ *        parameters:
+ *          - name: pageId
+ *            in: query
+ *            required: true
+ *            description: page id
+ *            schema:
+ *              $ref: '#/components/schemas/ObjectId'
+ *          - name: shareLinkId
+ *            in: query
+ *            description: share link id for shared page access
+ *            schema:
+ *              $ref: '#/components/schemas/ObjectId'
+ *        responses:
+ *          200:
+ *            description: Successfully retrieved current page info.
+ *            content:
+ *              application/json:
+ *                schema:
+ *                  $ref: '#/components/schemas/PageInfoExt'
+ *          403:
+ *            description: Page is forbidden.
+ *          500:
+ *            description: Internal server error.
+ */
+export const getPageInfoHandlerFactory = (crowi: Crowi): RequestHandler[] => {
+  const loginRequired = loginRequiredFactory(crowi, true);
+  const certifySharedPage = require('../../../middlewares/certify-shared-page')(
+    crowi,
+  );
+  const { pageService, pageGrantService } = crowi;
+
+  // define validators for req.query
+  const validator = [
+    query('pageId').isMongoId().withMessage('pageId is required'),
+    query('shareLinkId').optional({ checkFalsy: true }).isMongoId(),
+  ];
+
+  return [
+    accessTokenParser([SCOPE.READ.FEATURES.PAGE]),
+    certifySharedPage,
+    loginRequired,
+    ...validator,
+    apiV3FormValidator,
+    async (req: RequestWithAuth, res: ApiV3Response) => {
+      const { user, isSharedPage } = req;
+      const { pageId } = req.query;
+
+      // pageId is validated by express-validator as MongoId, so it's a string
+      const pageIdString = typeof pageId === 'string' ? pageId : String(pageId);
+
+      try {
+        const { meta } = await findPageAndMetaDataByViewer(
+          pageService,
+          pageGrantService,
+          {
+            pageId: pageIdString,
+            path: null,
+            user,
+            isSharedPage,
+          },
+        );
+
+        if (isIPageNotFoundInfo(meta)) {
+          // Return error only when the page is forbidden
+          if (meta.isForbidden) {
+            return res.apiv3Err(
+              new ErrorV3(
+                'Page is forbidden',
+                'page-is-forbidden',
+                undefined,
+                meta,
+              ),
+              403,
+            );
+          }
+        }
+
+        // Empty pages (isEmpty: true) should return page info for UI operations
+        return res.apiv3(meta);
+      } catch (err) {
+        logger.error('get-page-info', err);
+        return res.apiv3Err(err, 500);
+      }
+    },
+  ];
+};

apps/app/src/server/routes/apiv3/page/index.ts

@@ -53,6 +53,7 @@ import loggerFactory from '~/utils/logger';
 import type { ApiV3Response } from '../interfaces/apiv3-response';
 import { checkPageExistenceHandlersFactory } from './check-page-existence';
 import { createPageHandlersFactory } from './create-page';
+import { getPageInfoHandlerFactory } from './get-page-info';
 import { getPagePathsWithDescendantCountFactory } from './get-page-paths-with-descendant-count';
 import { getYjsDataHandlerFactory } from './get-yjs-data';
 import { publishPageHandlersFactory } from './publish-page';
@@ -108,7 +109,6 @@ module.exports = (crowi: Crowi) => {
       query('includeEmpty').optional().isBoolean(),
     ],
     likes: [body('pageId').isString(), body('bool').isBoolean()],
-    info: [query('pageId').isMongoId().withMessage('pageId is required')],
     getGrantData: [
       query('pageId').isMongoId().withMessage('pageId is required'),
     ],
@@ -587,72 +587,7 @@ module.exports = (crowi: Crowi) => {
     },
   );
 
-  /**
-   * @swagger
-   *
-   *    /page/info:
-   *      get:
-   *        tags: [Page]
-   *        summary: /page/info
-   *        description: Get summary informations for a page
-   *        parameters:
-   *          - name: pageId
-   *            in: query
-   *            required: true
-   *            description: page id
-   *            schema:
-   *              $ref: '#/components/schemas/ObjectId'
-   *        responses:
-   *          200:
-   *            description: Successfully retrieved current page info.
-   *            content:
-   *              application/json:
-   *                schema:
-   *                  $ref: '#/components/schemas/PageInfoExt'
-   *          500:
-   *            description: Internal server error.
-   */
-  router.get(
-    '/info',
-    accessTokenParser([SCOPE.READ.FEATURES.PAGE]),
-    certifySharedPage,
-    loginRequired,
-    validator.info,
-    apiV3FormValidator,
-    async (req, res) => {
-      const { user, isSharedPage } = req;
-      const { pageId } = req.query;
-
-      try {
-        const { meta } = await findPageAndMetaDataByViewer(
-          pageService,
-          pageGrantService,
-          { pageId, path: null, user, isSharedPage },
-        );
-
-        if (isIPageNotFoundInfo(meta)) {
-          // Return error only when the page is forbidden
-          if (meta.isForbidden) {
-            return res.apiv3Err(
-              new ErrorV3(
-                'Page is forbidden',
-                'page-is-forbidden',
-                undefined,
-                meta,
-              ),
-              403,
-            );
-          }
-        }
-
-        // Empty pages (isEmpty: true) should return page info for UI operations
-        return res.apiv3(meta);
-      } catch (err) {
-        logger.error('get-page-info', err);
-        return res.apiv3Err(err, 500);
-      }
-    },
-  );
+  router.get('/info', getPageInfoHandlerFactory(crowi));
 
   /**
    * @swagger

apps/app/src/stores/page.tsx

@@ -90,6 +90,20 @@ export const mutateAllPageInfo = (): Promise<void[]> => {
   return mutate((key) => Array.isArray(key) && key[0] === '/page/info');
 };
 
+/**
+ * Build query params for /page/info endpoint.
+ * Only includes shareLinkId when it is a non-empty string.
+ */
+const buildPageInfoParams = (
+  pageId: string,
+  shareLinkId: string | null | undefined,
+): { pageId: string; shareLinkId?: string } => {
+  if (shareLinkId != null && shareLinkId.trim().length > 0) {
+    return { pageId, shareLinkId };
+  }
+  return { pageId };
+};
+
 export const useSWRxPageInfo = (
   pageId: string | null | undefined,
   shareLinkId?: string | null,
@@ -98,19 +112,20 @@ export const useSWRxPageInfo = (
   // Cache remains from guest mode when logging in via the Login lead, so add 'isGuestUser' key
   const isGuestUser = useIsGuestUser();
 
-  // assign null if shareLinkId is undefined in order to identify SWR key only by pageId
-  const fixedShareLinkId = shareLinkId ?? null;
-
   const key = useMemo(() => {
     return pageId != null
-      ? ['/page/info', pageId, fixedShareLinkId, isGuestUser]
+      ? ['/page/info', pageId, shareLinkId, isGuestUser]
       : null;
-  }, [fixedShareLinkId, isGuestUser, pageId]);
+  }, [shareLinkId, isGuestUser, pageId]);
 
   const swrResult = useSWRImmutable(
     key,
-    ([endpoint, pageId, shareLinkId]: [string, string, string | null]) =>
-      apiv3Get(endpoint, { pageId, shareLinkId }).then(
+    ([endpoint, pageId, shareLinkId]: [
+      string,
+      string,
+      string | null | undefined,
+    ]) =>
+      apiv3Get(endpoint, buildPageInfoParams(pageId, shareLinkId)).then(
         (response) => response.data,
       ),
     { fallbackData: initialData },
@@ -136,19 +151,20 @@ export const useSWRMUTxPageInfo = (
   // Cache remains from guest mode when logging in via the Login lead, so add 'isGuestUser' key
   const isGuestUser = useIsGuestUser();
 
-  // assign null if shareLinkId is undefined in order to identify SWR key only by pageId
-  const fixedShareLinkId = shareLinkId ?? null;
-
   const key = useMemo(() => {
     return pageId != null
-      ? ['/page/info', pageId, fixedShareLinkId, isGuestUser]
+      ? ['/page/info', pageId, shareLinkId, isGuestUser]
       : null;
-  }, [fixedShareLinkId, isGuestUser, pageId]);
+  }, [shareLinkId, isGuestUser, pageId]);
 
   return useSWRMutation(
     key,
-    ([endpoint, pageId, shareLinkId]: [string, string, string | null]) =>
-      apiv3Get(endpoint, { pageId, shareLinkId }).then(
+    ([endpoint, pageId, shareLinkId]: [
+      string,
+      string,
+      string | null | undefined,
+    ]) =>
+      apiv3Get(endpoint, buildPageInfoParams(pageId, shareLinkId)).then(
         (response) => response.data,
       ),
   );