Merge pull request #6376 from weseek/imprv/pageredirect-operation

imprv: Re-implement retrievePageRedirectEndpoints with $graphLookup

packages/app/src/pages/[[...path]].page.tsx

@@ -368,7 +368,7 @@ async function injectPageData(context: GetServerSidePropsContext, props: Props):
 
   if (!isPermalink) {
     // check redirects
-    const chains = await PageRedirect.retrievePageRedirectChains(currentPathname);
+    const chains = await PageRedirect.retrievePageRedirectEndpoints(currentPathname);
     if (chains != null) {
       // overwrite currentPathname
       currentPathname = chains.end.toPath;

packages/app/src/server/models/page-redirect.ts

@@ -4,25 +4,37 @@ import {
   Schema, Model, Document,
 } from 'mongoose';
 
+import loggerFactory from '~/utils/logger';
+
 import { getOrCreateModel } from '../util/mongoose-utils';
 
-export interface IPageRedirectChains {
-  start: IPageRedirect,
-  end: IPageRedirect,
-}
 
-export interface IPageRedirect {
+const logger = loggerFactory('growi:models:page-redirects');
+
+
+export type IPageRedirect = {
   fromPath: string,
   toPath: string,
 }
 
+export type IPageRedirectEndpoints = {
+  start: IPageRedirect,
+  end: IPageRedirect,
+}
+
 export interface PageRedirectDocument extends IPageRedirect, Document {}
 
 export interface PageRedirectModel extends Model<PageRedirectDocument> {
-  retrievePageRedirectChains(fromPath: string, storedChains?: IPageRedirectChains): Promise<IPageRedirectChains>
-  removePageRedirectByToPath(toPath: string): Promise<void>
+  retrievePageRedirectEndpoints(fromPath: string): Promise<IPageRedirectEndpoints>
+  removePageRedirectsByToPath(toPath: string): Promise<void>
 }
 
+const CHAINS_FIELD_NAME = 'chains';
+const DEPTH_FIELD_NAME = 'depth';
+type IPageRedirectWithChains = PageRedirectDocument & {
+  [CHAINS_FIELD_NAME]: (PageRedirectDocument & { [DEPTH_FIELD_NAME]: number })[]
+};
+
 const schema = new Schema<PageRedirectDocument, PageRedirectModel>({
   fromPath: {
     type: String, required: true, unique: true, index: true,
@@ -30,18 +42,40 @@ const schema = new Schema<PageRedirectDocument, PageRedirectModel>({
   toPath: { type: String, required: true },
 });
 
-schema.statics.retrievePageRedirectChains = async function(fromPath: string, storedChains?: IPageRedirectChains): Promise<IPageRedirectChains|null> {
-  const chainedRedirect = await this.findOne({ fromPath });
+schema.statics.retrievePageRedirectEndpoints = async function(fromPath: string): Promise<IPageRedirectEndpoints|null> {
+  const aggResult: IPageRedirectWithChains[] = await this.aggregate([
+    { $match: { fromPath } },
+    {
+      $graphLookup: {
+        from: 'pageredirects',
+        startWith: '$toPath',
+        connectFromField: 'toPath',
+        connectToField: 'fromPath',
+        as: CHAINS_FIELD_NAME,
+        depthField: DEPTH_FIELD_NAME,
+      },
+    },
+  ]);
 
-  if (chainedRedirect == null) {
-    return storedChains ?? null;
+  if (aggResult.length === 0) {
+    return null;
   }
 
-  const chains = storedChains ?? { start: chainedRedirect, end: chainedRedirect };
-  chains.end = chainedRedirect;
+  if (aggResult.length > 1) {
+    logger.warn(`Although two or more PageRedirect documents starts from '${fromPath}' exists, The first one is used.`);
+  }
+
+  const redirectWithChains = aggResult[0];
+
+  // sort chains in desc
+  const sortedChains = redirectWithChains[CHAINS_FIELD_NAME].sort((a, b) => b[DEPTH_FIELD_NAME] - a[DEPTH_FIELD_NAME]);
+
+  const start = { fromPath: redirectWithChains.fromPath, toPath: redirectWithChains.toPath };
+  const end = sortedChains.length === 0
+    ? start
+    : sortedChains[0];
 
-  // find the end recursively
-  return this.retrievePageRedirectChains(chainedRedirect.toPath, chains);
+  return { start, end };
 };
 
 schema.statics.removePageRedirectByToPath = async function(toPath: string): Promise<void> {

packages/app/test/integration/models/page-redirect.test.js

@@ -14,6 +14,11 @@ describe('PageRedirect', () => {
     PageRedirect = mongoose.model('PageRedirect');
   });
 
+  beforeEach(async() => {
+    // clear collection
+    await PageRedirect.deleteMany({});
+  });
+
   describe('.removePageRedirectByToPath', () => {
     test('works fine', async() => {
       // setup:
@@ -44,20 +49,41 @@ describe('PageRedirect', () => {
     });
   });
 
-  describe('.retrievePageRedirectChains', () => {
+  describe('.retrievePageRedirectEndpoints', () => {
     test('shoud return null when data is not found', async() => {
       // setup:
       expect(await PageRedirect.findOne({ fromPath: '/path1' })).toBeNull();
 
       // when:
       // retrieve
-      const chains = await PageRedirect.retrievePageRedirectChains('/path1');
+      const endpoints = await PageRedirect.retrievePageRedirectEndpoints('/path1');
+
+      // then:
+      expect(endpoints).toBeNull();
+    });
+
+    test('shoud return IPageRedirectEnds (start and end is the same)', async() => {
+      // setup:
+      await PageRedirect.insertMany([
+        { fromPath: '/path1', toPath: '/path2' },
+      ]);
+      expect(await PageRedirect.findOne({ fromPath: '/path1' })).not.toBeNull();
+
+      // when:
+      // retrieve
+      const endpoints = await PageRedirect.retrievePageRedirectEndpoints('/path1');
 
       // then:
-      expect(chains).toBeNull();
+      expect(endpoints).not.toBeNull();
+      expect(endpoints.start).not.toBeNull();
+      expect(endpoints.start.fromPath).toEqual('/path1');
+      expect(endpoints.start.toPath).toEqual('/path2');
+      expect(endpoints.end).not.toBeNull();
+      expect(endpoints.end.fromPath).toEqual('/path1');
+      expect(endpoints.end.toPath).toEqual('/path2');
     });
 
-    test('shoud return IPageRedirectChains', async() => {
+    test('shoud return IPageRedirectEnds', async() => {
       // setup:
       await PageRedirect.insertMany([
         { fromPath: '/path1', toPath: '/path2' },
@@ -70,16 +96,16 @@ describe('PageRedirect', () => {
 
       // when:
       // retrieve
-      const chains = await PageRedirect.retrievePageRedirectChains('/path1');
+      const endpoints = await PageRedirect.retrievePageRedirectEndpoints('/path1');
 
       // then:
-      expect(chains).not.toBeNull();
-      expect(chains.start).not.toBeNull();
-      expect(chains.start.fromPath).toEqual('/path1');
-      expect(chains.start.toPath).toEqual('/path2');
-      expect(chains.end).not.toBeNull();
-      expect(chains.end.fromPath).toEqual('/path3');
-      expect(chains.end.toPath).toEqual('/path4');
+      expect(endpoints).not.toBeNull();
+      expect(endpoints.start).not.toBeNull();
+      expect(endpoints.start.fromPath).toEqual('/path1');
+      expect(endpoints.start.toPath).toEqual('/path2');
+      expect(endpoints.end).not.toBeNull();
+      expect(endpoints.end.fromPath).toEqual('/path3');
+      expect(endpoints.end.toPath).toEqual('/path4');
     });
   });