Merge pull request #10159 from weseek/imprv/168543-make-search-query-type-safe

imprv: Make search query type safe

apps/app/src/server/service/search-delegator/elasticsearch-client-delegator/es7-client-delegator.ts

@@ -7,6 +7,8 @@ import {
   type estypes,
 } from '@elastic/elasticsearch7';
 
+import type { ES7SearchQuery } from './interfaces';
+
 export class ES7ClientDelegator {
 
   private client: Client;
@@ -35,28 +37,24 @@ export class ES7ClientDelegator {
     create: (params: RequestParams.IndicesCreate): Promise<ApiResponse<estypes.IndicesCreateResponse>> => this.client.indices.create(params),
     delete: (params: RequestParams.IndicesDelete): Promise<ApiResponse<estypes.IndicesDeleteResponse>> => this.client.indices.delete(params),
     exists: async(params: RequestParams.IndicesExists): Promise<estypes.IndicesExistsResponse> => {
-      const res = (await this.client.indices.exists(params)).body;
-      return res;
+      return (await this.client.indices.exists(params)).body;
     },
     existsAlias: async(params: RequestParams.IndicesExistsAlias): Promise<estypes.IndicesExistsAliasResponse> => {
-      const res = (await this.client.indices.existsAlias(params)).body;
-      return res;
+      return (await this.client.indices.existsAlias(params)).body;
     },
     putAlias: (params: RequestParams.IndicesPutAlias): Promise<ApiResponse<estypes.IndicesUpdateAliasesResponse>> => this.client.indices.putAlias(params),
     getAlias: async(params: RequestParams.IndicesGetAlias): Promise<estypes.IndicesGetAliasResponse> => {
-      const res = (await this.client.indices.getAlias(params)).body as estypes.IndicesGetAliasResponse;
-      return res;
+      return (await this.client.indices.getAlias<estypes.IndicesGetAliasResponse>(params)).body;
     },
     updateAliases: (params: RequestParams.IndicesUpdateAliases['body']): Promise<ApiResponse<estypes.IndicesUpdateAliasesResponse>> => {
       return this.client.indices.updateAliases({ body: params });
     },
-    validateQuery: async(params:RequestParams.IndicesValidateQuery): Promise<estypes.IndicesValidateQueryResponse> => {
-      const res = (await this.client.indices.validateQuery(params)).body as estypes.IndicesValidateQueryResponse;
-      return res;
+    validateQuery: async(params: RequestParams.IndicesValidateQuery<{ query?: estypes.QueryDslQueryContainer }>)
+      : Promise<estypes.IndicesValidateQueryResponse> => {
+      return (await this.client.indices.validateQuery<estypes.IndicesValidateQueryResponse>(params)).body;
     },
     stats: async(params: RequestParams.IndicesStats): Promise<estypes.IndicesStatsResponse> => {
-      const res = (await this.client.indices.stats(params)).body as estypes.IndicesStatsResponse;
-      return res;
+      return (await this.client.indices.stats<estypes.IndicesStatsResponse>(params)).body;
     },
   };
 
@@ -72,9 +70,8 @@ export class ES7ClientDelegator {
     return this.client.reindex({ wait_for_completion: false, body: { source: { index: indexName }, dest: { index: tmpIndexName } } });
   }
 
-  async search(params: RequestParams.Search): Promise<estypes.SearchResponse> {
-    const res = (await this.client.search(params)).body as estypes.SearchResponse;
-    return res;
+  async search(params: ES7SearchQuery): Promise<estypes.SearchResponse> {
+    return (await this.client.search<estypes.SearchResponse>(params)).body;
   }
 
 }

apps/app/src/server/service/search-delegator/elasticsearch-client-delegator/interfaces.ts

@@ -1,3 +1,7 @@
+import type { estypes as ES7types, RequestParams } from '@elastic/elasticsearch7';
+import type { estypes as ES8types } from '@elastic/elasticsearch8';
+import type { estypes as ES9types } from '@elastic/elasticsearch9';
+
 import type { ES7ClientDelegator } from './es7-client-delegator';
 import type { ES8ClientDelegator } from './es8-client-delegator';
 import type { ES9ClientDelegator } from './es9-client-delegator';
@@ -18,3 +22,38 @@ export const isES8ClientDelegator = (delegator: ElasticsearchClientDelegator): d
 export const isES9ClientDelegator = (delegator: ElasticsearchClientDelegator): delegator is ES9ClientDelegator => {
   return delegator.delegatorVersion === 9;
 };
+
+
+// Official library-derived interface
+// TODO: https://redmine.weseek.co.jp/issues/168446
+export type ES7SearchQuery = RequestParams.Search<{
+  query: ES7types.QueryDslQueryContainer
+  sort?: ES7types.Sort
+  highlight?: ES7types.SearchHighlight
+}>
+
+export interface ES8SearchQuery {
+  index: ES8types.IndexName
+  _source: ES8types.Fields
+  from?: number;
+  size?: number;
+  body: {
+    query: ES8types.QueryDslQueryContainer;
+    sort?: ES8types.Sort
+    highlight?: ES8types.SearchHighlight;
+  };
+}
+
+export interface ES9SearchQuery {
+  index: ES9types.IndexName
+  _source: ES9types.Fields
+  from?: number;
+  size?: number;
+  body: {
+    query: ES9types.QueryDslQueryContainer;
+    sort?: ES9types.Sort
+    highlight?: ES9types.SearchHighlight;
+  };
+}
+
+export type SearchQuery = ES7SearchQuery | ES8SearchQuery | ES9SearchQuery;

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

@@ -25,7 +25,15 @@ import type { UpdateOrInsertPagesOpts } from '../interfaces/search';
 import { aggregatePipelineToIndex } from './aggregate-to-index';
 import type { AggregatedPage, BulkWriteBody, BulkWriteCommand } from './bulk-write';
 import {
-  getClient, type ElasticsearchClientDelegator, isES9ClientDelegator, isES7ClientDelegator, isES8ClientDelegator,
+  getClient,
+  isES7ClientDelegator,
+  isES8ClientDelegator,
+  isES9ClientDelegator,
+  type SearchQuery,
+  type ES7SearchQuery,
+  type ES8SearchQuery,
+  type ES9SearchQuery,
+  type ElasticsearchClientDelegator,
 } from './elasticsearch-client-delegator';
 
 const logger = loggerFactory('growi:service:search-delegator:elasticsearch');
@@ -44,7 +52,7 @@ const ES_SORT_AXIS = {
 const ES_SORT_ORDER = {
   [DESC]: 'desc',
   [ASC]: 'asc',
-};
+} as const;
 
 const AVAILABLE_KEYS = ['match', 'not_match', 'phrase', 'not_phrase', 'prefix', 'not_prefix', 'tag', 'not_tag'];
 
@@ -551,26 +559,74 @@ class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQuer
    *   data: [ pages ...],
    * }
    */
-  async searchKeyword(query): Promise<ISearchResult<ISearchResultData>> {
+  async searchKeyword(query: SearchQuery): Promise<ISearchResult<ISearchResultData>> {
 
     // for debug
     if (process.env.NODE_ENV === 'development') {
       logger.debug('query: ', JSON.stringify(query, null, 2));
 
-      const validateQueryResponse = await this.client.indices.validateQuery({
-        index: query.index,
-        type: query.type,
-        explain: true,
-        body: {
-          query: query.body.query,
-        },
-      });
+
+      const validateQueryResponse = await (async() => {
+        if (isES7ClientDelegator(this.client)) {
+          const es7SearchQuery = query as ES7SearchQuery;
+          return this.client.indices.validateQuery({
+            explain: true,
+            index: es7SearchQuery.index,
+            body: {
+              query: es7SearchQuery.body?.query,
+            },
+          });
+        }
+
+        if (isES8ClientDelegator(this.client)) {
+          const es8SearchQuery = query as ES8SearchQuery;
+          return this.client.indices.validateQuery({
+            explain: true,
+            index: es8SearchQuery.index,
+            query: es8SearchQuery.body.query,
+          });
+        }
+
+        if (isES9ClientDelegator(this.client)) {
+          const es9SearchQuery = query as ES9SearchQuery;
+          return this.client.indices.validateQuery({
+            explain: true,
+            index: es9SearchQuery.index,
+            query: es9SearchQuery.body.query,
+          });
+        }
+
+        throw new Error('Unsupported Elasticsearch version');
+      })();
+
 
       // for debug
       logger.debug('ES result: ', validateQueryResponse);
     }
 
-    const searchResponse = await this.client.search(query);
+    const searchResponse = await (async() => {
+      if (isES7ClientDelegator(this.client)) {
+        return this.client.search(query as ES7SearchQuery);
+      }
+
+      if (isES8ClientDelegator(this.client)) {
+        return this.client.search(query as ES8SearchQuery);
+      }
+
+      if (isES9ClientDelegator(this.client)) {
+        const { body, ...rest } = query as ES9SearchQuery;
+        return this.client.search({
+          ...rest,
+          // Elimination of the body property since ES9
+          // https://raw.githubusercontent.com/elastic/elasticsearch-js/2f6200eb397df0e54d23848d769a93614ee1fb45/docs/release-notes/breaking-changes.md
+          query: body.query,
+          sort: body.sort,
+          highlight: body.highlight,
+        });
+      }
+
+      throw new Error('Unsupported Elasticsearch version');
+    })();
 
     const _total = searchResponse?.hits?.total;
     let total = 0;
@@ -597,44 +653,49 @@ class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQuer
 
   /**
    * create search query for Elasticsearch
-   *
-   * @param {object | undefined} option optional paramas
    * @returns {object} query object
    */
-  createSearchQuery(option?) {
-    let fields = ['path', 'bookmark_count', 'comment_count', 'seenUsers_count', 'updated_at', 'tag_names', 'comments'];
-    if (option) {
-      fields = option.fields || fields;
-    }
+  createSearchQuery(): SearchQuery {
+    const fields = ['path', 'bookmark_count', 'comment_count', 'seenUsers_count', 'updated_at', 'tag_names', 'comments'];
 
     // sort by score
-    const query = {
+    const query: SearchQuery = {
       index: this.aliasName,
       _source: fields,
       body: {
-        query: {}, // query
+        query: {
+          bool: {},
+        },
       },
     };
 
     return query;
   }
 
-  appendResultSize(query, from?, size?): void {
+  appendResultSize(query: SearchQuery, from?: number, size?: number): void {
     query.from = from || DEFAULT_OFFSET;
     query.size = size || DEFAULT_LIMIT;
   }
 
-  appendSortOrder(query, sortAxis: SORT_AXIS, sortOrder: SORT_ORDER): void {
+  appendSortOrder(query: SearchQuery, sortAxis: SORT_AXIS, sortOrder: SORT_ORDER): void {
+    if (query.body == null) {
+      throw new Error('query.body is not initialized');
+    }
+
     // default sort order is score descending
     const sort = ES_SORT_AXIS[sortAxis] || ES_SORT_AXIS[RELATION_SCORE];
     const order = ES_SORT_ORDER[sortOrder] || ES_SORT_ORDER[DESC];
-    query.body.sort = { [sort]: { order } };
+
+    query.body.sort = {
+      [sort]: { order },
+    };
+
   }
 
-  initializeBoolQuery(query) {
+  initializeBoolQuery(query: SearchQuery): SearchQuery {
     // query is created by createSearchQuery()
-    if (!query.body.query.bool) {
-      query.body.query.bool = {};
+    if (query?.body?.query?.bool == null) {
+      throw new Error('query.body.query.bool is not initialized');
     }
 
     const isInitialized = (query) => { return !!query && Array.isArray(query) };
@@ -651,14 +712,30 @@ class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQuer
     return query;
   }
 
-  appendCriteriaForQueryString(query, parsedKeywords: ESQueryTerms): void {
+  appendCriteriaForQueryString(query: SearchQuery, parsedKeywords: ESQueryTerms): void {
     query = this.initializeBoolQuery(query); // eslint-disable-line no-param-reassign
 
+    if (query.body?.query?.bool == null) {
+      throw new Error('query.body.query.bool is not initialized');
+    }
+
+    if (query.body?.query?.bool.must == null || !Array.isArray(query.body?.query?.bool.must)) {
+      throw new Error('query.body.query.bool.must is not initialized');
+    }
+
+    if (query.body?.query?.bool.must_not == null || !Array.isArray(query.body?.query?.bool.must_not)) {
+      throw new Error('query.body.query.bool.must_not is not initialized');
+    }
+
+    if (query.body?.query?.bool.filter == null || !Array.isArray(query.body?.query?.bool.filter)) {
+      throw new Error('query.body.query.bool.filter is not initialized');
+    }
+
     if (parsedKeywords.match.length > 0) {
       const q = {
         multi_match: {
           query: parsedKeywords.match.join(' '),
-          type: 'most_fields',
+          type: 'most_fields' as const,
           fields: ['path.ja^2', 'path.en^2', 'body.ja', 'body.en', 'comments.ja', 'comments.en'],
         },
       };
@@ -670,18 +747,18 @@ class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQuer
         multi_match: {
           query: parsedKeywords.not_match.join(' '),
           fields: ['path.ja', 'path.en', 'body.ja', 'body.en', 'comments.ja', 'comments.en'],
-          operator: 'or',
+          operator: 'or' as const,
         },
       };
       query.body.query.bool.must_not.push(q);
     }
 
     if (parsedKeywords.phrase.length > 0) {
-      parsedKeywords.phrase.forEach((phrase) => {
+      for (const phrase of parsedKeywords.phrase) {
         const phraseQuery = {
           multi_match: {
-            query: phrase, // each phrase is quoteted words like "This is GROWI"
-            type: 'phrase',
+            query: phrase, // query is created by createSearchQuery()
+            type: 'phrase' as const,
             fields: [
               // Not use "*.ja" fields here, because we want to analyze (parse) search words
               'path.raw^2',
@@ -691,15 +768,15 @@ class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQuer
           },
         };
         query.body.query.bool.must.push(phraseQuery);
-      });
+      }
     }
 
     if (parsedKeywords.not_phrase.length > 0) {
-      parsedKeywords.not_phrase.forEach((phrase) => {
+      for (const phrase of parsedKeywords.not_phrase) {
         const notPhraseQuery = {
           multi_match: {
             query: phrase, // each phrase is quoteted words
-            type: 'phrase',
+            type: 'phrase' as const,
             fields: [
               // Not use "*.ja" fields here, because we want to analyze (parse) search words
               'path.raw^2',
@@ -708,7 +785,7 @@ class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQuer
           },
         };
         query.body.query.bool.must_not.push(notPhraseQuery);
-      });
+      }
     }
 
     if (parsedKeywords.prefix.length > 0) {
@@ -740,12 +817,16 @@ class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQuer
     }
   }
 
-  async filterPagesByViewer(query, user, userGroups): Promise<void> {
+  async filterPagesByViewer(query: SearchQuery, user, userGroups): Promise<void> {
     const showPagesRestrictedByOwner = !configManager.getConfig('security:list-policy:hideRestrictedByOwner');
     const showPagesRestrictedByGroup = !configManager.getConfig('security:list-policy:hideRestrictedByGroup');
 
     query = this.initializeBoolQuery(query); // eslint-disable-line no-param-reassign
 
+    if (query.body?.query?.bool?.filter == null || !Array.isArray(query.body?.query?.bool?.filter)) {
+      throw new Error('query.body.query.bool is not initialized');
+    }
+
     const Page = mongoose.model('Page') as unknown as PageModel;
     const {
       GRANT_PUBLIC, GRANT_SPECIFIED, GRANT_OWNER, GRANT_USER_GROUP,
@@ -828,7 +909,11 @@ class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQuer
     };
   }
 
-  appendHighlight(query): void {
+  appendHighlight(query: SearchQuery): void {
+    if (query.body == null) {
+      throw new Error('query.body is not initialized');
+    }
+
     query.body.highlight = {
       fragmenter: 'simple',
       pre_tags: ["<em class='highlighted-keyword'>"],