Merge pull request #4784 from weseek/imprv/commonize-fulltext-and-custom-search

imprv: Commonize fulltext and custom search

packages/app/src/components/SearchPage.jsx

@@ -173,9 +173,9 @@ class SearchPage extends React.Component {
     }
   }
 
-  selectPage= (pageId) => {
+  selectPage = (pageId) => {
     const index = this.state.searchedPages.findIndex((page) => {
-      return page._id === pageId;
+      return page.pageData._id === pageId;
     });
     this.setState({
       focusedPage: this.state.searchedPages[index],

packages/app/src/components/SearchPage/SearchResultContent.tsx

@@ -3,37 +3,44 @@ import React, { FC } from 'react';
 import RevisionLoader from '../Page/RevisionLoader';
 import AppContainer from '../../client/services/AppContainer';
 
+import { IPageSearchResultData } from '../../interfaces/search';
+
 
 type Props ={
   appContainer: AppContainer,
   searchingKeyword:string,
-  focusedPage : any,
+  focusedPage: IPageSearchResultData,
 }
 const SearchResultContent: FC<Props> = (props: Props) => {
   // Temporaly workaround for lint error
   // later needs to be fixed: RevisoinRender to typescriptcomponet
   const RevisionRenderTypeAny: any = RevisionLoader;
-  const renderPage = (page) => {
+  const renderPage = (page: IPageSearchResultData) => {
+    const { pageData } = page;
+    if (pageData == null) {
+      return null;
+    }
+
     const growiRenderer = props.appContainer.getRenderer('searchresult');
     let showTags = false;
-    if (page.tags != null && page.tags.length > 0) { showTags = true }
+    if (pageData.tags != null && pageData.tags.length > 0) { showTags = true }
     return (
-      <div key={page._id} className="search-result-page mb-5">
+      <div key={pageData._id} className="search-result-page mb-5">
         <h2>
-          <a href={page.path} className="text-break">
-            {page.path}
+          <a href={pageData.path} className="text-break">
+            {pageData.path}
           </a>
           {showTags && (
             <div className="mt-1 small">
-              <i className="tag-icon icon-tag"></i> {page.tags.join(', ')}
+              <i className="tag-icon icon-tag"></i> {pageData.tags.join(', ')}
             </div>
           )}
         </h2>
         <RevisionRenderTypeAny
           growiRenderer={growiRenderer}
-          pageId={page._id}
-          pagePath={page.path}
-          revisionId={page.revision}
+          pageId={pageData._id}
+          pagePath={pageData.path}
+          revisionId={pageData.revision}
           highlightKeywords={props.searchingKeyword}
         />
       </div>

packages/app/src/components/SearchPage/SearchResultListItem.tsx

@@ -126,7 +126,8 @@ const SearchResultListItem: FC<Props> = (props:Props) => {
               <Clamp
                 lines={2}
               >
-                {pageMeta.elasticSearchResult && <div className="mt-1" dangerouslySetInnerHTML={{ __html: pageMeta.elasticSearchResult.snippet }}></div>}
+                {pageMeta.elasticSearchResult != null
+                && <div className="mt-1" dangerouslySetInnerHTML={{ __html: pageMeta.elasticSearchResult.snippet }}></div>}
               </Clamp>
             </div>
           </div>

packages/app/src/server/interfaces/search.ts

@@ -29,11 +29,14 @@ export interface SearchDelegator<T = unknown> {
 }
 
 export type Result<T> = {
-  data: T
+  data: T[]
 }
 
 export type MetaData = {
-  meta?: { [key:string]: any }
+  meta: {
+    [key:string]: any,
+    total: number,
+  }
 }
 
 export type SearchableData = {

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

@@ -38,6 +38,7 @@ type TargetAndAncestorsResult = {
   rootPage: PageDocument
 }
 export interface PageModel extends Model<PageDocument> {
+  [x: string]: any; // for obsolete methods
   createEmptyPagesByPaths(paths: string[]): Promise<void>
   getParentIdAndFillAncestors(path: string): Promise<string | null>
   findByPathAndViewer(path: string | null, user, userGroups?, useFindOne?): Promise<PageDocument[]>

packages/app/src/server/routes/index.js

@@ -146,7 +146,7 @@ module.exports = function(crowi, app) {
   app.get('/download/:id([0-9a-z]{24})'    , loginRequired, attachment.api.download);
 
   app.get('/_search'                 , loginRequired , search.searchPage);
-  app.get('/_api/search'             , accessTokenParser , loginRequired , search.api.search);
+  app.get('/_api/search'             , accessTokenParser , loginRequired, search.api.search);
 
   app.get('/_api/check_username'           , user.api.checkUsername);
   app.get('/_api/me/user-group-relations'  , accessTokenParser , loginRequiredStrictly , me.api.userGroupRelations);

packages/app/src/server/routes/search.js

@@ -1,4 +1,6 @@
-const { serializeUserSecurely } = require('../models/serializers/user-serializer');
+const { default: loggerFactory } = require('~/utils/logger');
+
+const logger = loggerFactory('growi:routes:search');
 
 /**
  * @swagger
@@ -137,54 +139,19 @@ module.exports = function(crowi, app) {
 
     const searchOpts = { ...paginateOpts, type };
 
-    const result = {};
+    let searchResult;
+    let delegatorName;
     try {
-      const esResult = searchService.formatResult(
-        await searchService.searchKeyword(keyword, user, userGroups, searchOpts),
-      );
-
-      // create score map for sorting
-      // key: id , value: score
-      const scoreMap = {};
-      for (const esPage of esResult.data) {
-        scoreMap[esPage._id] = esPage._score;
-      }
-
-      const ids = esResult.data.map((page) => { return page._id });
-      const findResult = await Page.findListByPageIds(ids);
-
-      // add tags data to page
-      findResult.pages.map((pageData) => {
-        const data = esResult.data.find((data) => {
-          return pageData.id === data._id;
-        });
-        pageData._doc.tags = data._source.tag_names;
-        return pageData;
-      });
-
-      result.meta = esResult.meta;
-      result.totalCount = findResult.totalCount;
-      result.data = findResult.pages
-        .map((pageData) => {
-          if (pageData.lastUpdateUser != null && pageData.lastUpdateUser instanceof User) {
-            pageData.lastUpdateUser = serializeUserSecurely(pageData.lastUpdateUser);
-          }
-
-          const data = esResult.data.find((data) => {
-            return pageData.id === data._id;
-          });
-
-          const pageMeta = {
-            bookmarkCount: data._source.bookmark_count || 0,
-            elasticSearchResult: data.elasticSearchResult,
-          };
+      [searchResult, delegatorName] = await searchService.searchKeyword(keyword, user, userGroups, searchOpts);
+    }
+    catch (err) {
+      logger.error('Failed to search', err);
+      return res.json(ApiResponse.error(err));
+    }
 
-          return { pageData, pageMeta };
-        })
-        .sort((page1, page2) => {
-          // note: this do not consider NaN
-          return scoreMap[page2._id] - scoreMap[page1._id];
-        });
+    let result;
+    try {
+      result = await searchService.formatSearchResult(searchResult, delegatorName);
     }
     catch (err) {
       return res.json(ApiResponse.error(err));

packages/app/src/server/service/search-delegator/elasticsearch.ts

@@ -12,6 +12,7 @@ import { PageDocument, PageModel } from '../../models/page';
 import {
   MetaData, SearchDelegator, Result, SearchableData, QueryTerms,
 } from '../../interfaces/search';
+import { SearchDelegatorName } from '~/interfaces/named-query';
 
 const logger = loggerFactory('growi:service:search-delegator:elasticsearch');
 
@@ -23,6 +24,8 @@ type Data = any;
 
 class ElasticsearchDelegator implements SearchDelegator<Data> {
 
+  name!: SearchDelegatorName.DEFAULT
+
   configManager!: any
 
   socketIoService!: any
@@ -36,6 +39,7 @@ class ElasticsearchDelegator implements SearchDelegator<Data> {
   esUri: string
 
   constructor(configManager, socketIoService) {
+    this.name = SearchDelegatorName.DEFAULT;
     this.configManager = configManager;
     this.socketIoService = socketIoService;
 

packages/app/src/server/service/search-delegator/private-legacy-pages.ts

@@ -6,17 +6,18 @@ import { IPage } from '~/interfaces/page';
 import {
   MetaData, Result, SearchableData, SearchDelegator,
 } from '../../interfaces/search';
+import { serializeUserSecurely } from '../../models/serializers/user-serializer';
 
 
-type Data = {
-  pages: IPage[]
-}
-
-class PrivateLegacyPagesDelegator implements SearchDelegator<Data> {
+class PrivateLegacyPagesDelegator implements SearchDelegator<IPage> {
 
   name!: SearchDelegatorName.PRIVATE_LEGACY_PAGES
 
-  async search(data: SearchableData | null, user, userGroups, option): Promise<Result<Data> & MetaData> {
+  constructor() {
+    this.name = SearchDelegatorName.PRIVATE_LEGACY_PAGES;
+  }
+
+  async search(_data: SearchableData | null, user, userGroups, option): Promise<Result<IPage> & MetaData> {
     const { offset, limit } = option;
 
     if (offset == null || limit == null) {
@@ -32,18 +33,24 @@ class PrivateLegacyPagesDelegator implements SearchDelegator<Data> {
 
     const queryBuilder = new PageQueryBuilder(Page.find());
 
-    const pages: PageDocument[] = await queryBuilder
+    const _pages: PageDocument[] = await queryBuilder
       .addConditionAsNonRootPage()
       .addConditionAsNotMigrated()
       .addConditionToFilteringByViewer(user, userGroups)
       .addConditionToPagenate(offset, limit)
       .query
-      .lean()
+      .populate('lastUpdateUser')
       .exec();
 
+    const pages = _pages.map((page) => {
+      page.lastUpdateUser = serializeUserSecurely(page.lastUpdateUser);
+      return page;
+    });
+
     return {
-      data: {
-        pages,
+      data: pages,
+      meta: {
+        total: pages.length,
       },
     };
   }

packages/app/src/server/service/search.ts

@@ -11,6 +11,9 @@ import ElasticsearchDelegator from './search-delegator/elasticsearch';
 import PrivateLegacyPagesDelegator from './search-delegator/private-legacy-pages';
 
 import loggerFactory from '~/utils/logger';
+import { PageModel } from '../models/page';
+import { serializeUserSecurely } from '../models/serializers/user-serializer';
+import { IPage } from '~/interfaces/page';
 
 // eslint-disable-next-line no-unused-vars
 const logger = loggerFactory('growi:service:search');
@@ -31,6 +34,28 @@ const normalizeQueryString = (_queryString: string): string => {
   return queryString;
 };
 
+export type FormattedSearchResult = {
+  data: {
+    pageData: IPage
+    pageMeta: {
+      bookmarkCount?: number
+      elasticsearchResult?: {
+        snippet?: string
+        matchedPath?: string
+        highlightedPath?: string
+      }
+    }
+  }[]
+
+  totalCount: number
+
+  meta: {
+    total: number
+    took?: number
+    count?: number
+  }
+}
+
 class SearchService implements SearchQueryParser, SearchResolver {
 
   crowi!: any
@@ -228,7 +253,7 @@ class SearchService implements SearchQueryParser, SearchResolver {
     return [this.nqDelegators[SearchDelegatorName.DEFAULT], data];
   }
 
-  async searchKeyword(keyword: string, user, userGroups, searchOpts): Promise<Result<any> & MetaData> {
+  async searchKeyword(keyword: string, user, userGroups, searchOpts): Promise<[Result<any> & MetaData, string]> {
     let parsedQuery;
     // parse
     try {
@@ -250,7 +275,7 @@ class SearchService implements SearchQueryParser, SearchResolver {
       throw err;
     }
 
-    return delegator.search(data, user, userGroups, searchOpts);
+    return [await delegator.search(data, user, userGroups, searchOpts), delegator.name];
   }
 
   parseQueryString(queryString: string): QueryTerms {
@@ -331,22 +356,97 @@ class SearchService implements SearchQueryParser, SearchResolver {
     return terms;
   }
 
+  // TODO: optimize the way to check isFormattable e.g. check data schema of searchResult
+  // So far, it determines by delegatorName passed by searchService.searchKeyword
+  checkIsFormattable(searchResult, delegatorName): boolean {
+    return delegatorName === SearchDelegatorName.DEFAULT;
+  }
+
   /**
    * formatting result
    */
-  formatResult(esResult) {
-    esResult.data.forEach((data) => {
-      const highlightData = data._highlight;
-      const snippet = highlightData['body.en'] || highlightData['body.ja'] || '';
-      const pathMatch = highlightData['path.en'] || highlightData['path.ja'] || '';
-
-      data.elasticSearchResult = {
-        snippet: filterXss.process(snippet),
-        // todo: use filter xss.process() for matchedPath;
-        matchedPath: pathMatch,
+  async formatSearchResult(searchResult: Result<any> & MetaData, delegatorName): Promise<FormattedSearchResult> {
+    if (!this.checkIsFormattable(searchResult, delegatorName)) {
+      const data = searchResult.data.map((page) => {
+        return {
+          pageData: page,
+          pageMeta: {},
+        };
+      });
+
+      return {
+        data,
+        totalCount: data.length,
+        meta: searchResult.meta,
       };
+    }
+
+    /*
+     * Format ElasticSearch result
+     */
+
+    const Page = this.crowi.model('Page') as PageModel;
+    const User = this.crowi.model('User');
+    const result = {} as FormattedSearchResult;
+
+    // create score map for sorting
+    // key: id , value: score
+    const scoreMap = {};
+    for (const esPage of searchResult.data) {
+      scoreMap[esPage._id] = esPage._score;
+    }
+
+    const ids = searchResult.data.map((page) => { return page._id });
+    const findResult = await Page.findListByPageIds(ids);
+
+    // add tags data to page
+    findResult.pages.map((pageData) => {
+      const data = searchResult.data.find((data) => {
+        return pageData.id === data._id;
+      });
+      pageData._doc.tags = data._source.tag_names;
+      return pageData;
     });
-    return esResult;
+
+    result.meta = searchResult.meta;
+    result.totalCount = findResult.totalCount;
+    result.data = findResult.pages
+      .map((pageData) => {
+        if (pageData.lastUpdateUser != null && pageData.lastUpdateUser instanceof User) {
+          pageData.lastUpdateUser = serializeUserSecurely(pageData.lastUpdateUser);
+        }
+
+        const data = searchResult.data.find((data) => {
+          return pageData.id === data._id;
+        });
+
+        // increment elasticSearchResult
+        let elasticSearchResult;
+        const highlightData = data._highlight;
+        if (highlightData != null) {
+          const snippet = highlightData['body.en'] || highlightData['body.ja'] || '';
+          const pathMatch = highlightData['path.en'] || highlightData['path.ja'] || '';
+
+          elasticSearchResult = {
+            snippet: filterXss.process(snippet),
+            // todo: use filter xss.process() for matchedPath;
+            matchedPath: pathMatch,
+          };
+        }
+
+        const pageMeta = {
+          bookmarkCount: data._source.bookmark_count || 0,
+          elasticSearchResult,
+        };
+
+        return { pageData, pageMeta };
+      })
+      .sort((page1, page2) => {
+        // note: this do not consider NaN
+        return scoreMap[page2.pageData._id] - scoreMap[page1.pageData._id];
+      });
+
+    return result;
   }
 
 }

packages/app/src/test/integration/service/search/search-service.test.js

@@ -187,19 +187,14 @@ describe('SearchService test', () => {
       ]);
 
       const queryString = '[nq:named_query1]';
-      const parsedQuery = {
-        queryString,
-        delegatorName: PRIVATE_LEGACY_PAGES,
-      };
-
-      const [delegator, data] = await searchService.resolve(parsedQuery);
 
-      const result = await delegator.search(data, testUser1, null, { limit: 0, offset: 0 });
+      const [result, delegatorName] = await searchService.searchKeyword(queryString, testUser1, null, { offset: 0, limit: 100 });
 
-      const resultPaths = result.data.pages.map(page => page.path);
+      const resultPaths = result.data.map(page => page.path);
       const flag = resultPaths.includes('/user1') && resultPaths.includes('/user1_owner') && resultPaths.includes('/user2_public');
 
       expect(flag).toBe(true);
+      expect(delegatorName).toBe(PRIVATE_LEGACY_PAGES);
     });
   });
 

packages/ui/src/components/PagePath/PageListMeta.jsx

@@ -23,12 +23,12 @@ export class PageListMeta extends React.Component {
     }
 
     let commentCount;
-    if (page.commentCount > 0) {
+    if (page.commentCount != null && page.commentCount > 0) {
       commentCount = <span><i className="icon-bubble" />{page.commentCount}</span>;
     }
 
     let likerCount;
-    if (page.liker.length > 0) {
+    if (page.liker != null && page.liker.length > 0) {
       likerCount = <span><i className="icon-like" />{page.liker.length}</span>;
     }