Просмотр исходного кода

Implemented filter for named query

Taichi Masuyama 4 лет назад
Родитель
Сommit
4d2008fecf

+ 2 - 2
packages/app/src/server/interfaces/search.ts

@@ -31,9 +31,9 @@ export interface SearchDelegator<T = unknown, KEY extends AllTermsKey = AllTerms
   validateTerms(terms: QueryTerms): UnavailableTermsKey<KEY>[],
 }
 
-export type SearchableData = {
+export type SearchableData<T = Partial<QueryTerms>> = {
   queryString: string
-  terms: QueryTerms
+  terms: T
 }
 
 // Terms Key types

+ 49 - 3
packages/app/src/server/models/page.ts

@@ -16,7 +16,7 @@ import { getPageSchema, extractToAncestorsPaths, populateDataToShowRevision } fr
 import { ObjectIdLike } from '~/server/interfaces/mongoose-utils';
 import { PageRedirectModel } from './page-redirect';
 
-const { addTrailingSlash } = pathUtils;
+const { addTrailingSlash, normalizePath } = pathUtils;
 const { isTopPage, collectAncestorPaths } = pagePathUtils;
 
 const logger = loggerFactory('growi:models:page');
@@ -124,7 +124,7 @@ const generateChildrenRegExp = (path: string): RegExp => {
   return new RegExp(`^${path}(\\/[^/]+)\\/?$`);
 };
 
-class PageQueryBuilder {
+export class PageQueryBuilder {
 
   query: any;
 
@@ -245,7 +245,9 @@ class PageQueryBuilder {
    * *option*
    *   Left for backward compatibility
    */
-  addConditionToListByStartWith(path, option?) {
+  addConditionToListByStartWith(str: string): PageQueryBuilder {
+    const path = normalizePath(str);
+
     // No request is set for the top page
     if (isTopPage(path)) {
       return this;
@@ -259,6 +261,50 @@ class PageQueryBuilder {
     return this;
   }
 
+  addConditionToListByNotStartWith(str: string): PageQueryBuilder {
+    const path = normalizePath(str);
+
+    // No request is set for the top page
+    if (isTopPage(path)) {
+      return this;
+    }
+
+    const startsPattern = escapeStringRegexp(str);
+
+    this.query = this.query
+      .and({ path: new RegExp(`^(?!${startsPattern}).*$`) });
+
+    return this;
+  }
+
+  addConditionToListByMatch(str: string): PageQueryBuilder {
+    // No request is set for "/"
+    if (str === '/') {
+      return this;
+    }
+
+    const match = escapeStringRegexp(str);
+
+    this.query = this.query
+      .and({ path: new RegExp(`^(?=.*${match}).*$`) });
+
+    return this;
+  }
+
+  addConditionToListByNotMatch(str: string): PageQueryBuilder {
+    // No request is set for "/"
+    if (str === '/') {
+      return this;
+    }
+
+    const match = escapeStringRegexp(str);
+
+    this.query = this.query
+      .and({ path: new RegExp(`^(?!.*${match}).*$`) });
+
+    return this;
+  }
+
   async addConditionForParentNormalization(user) {
     // determine UserGroup condition
     let userGroups;

+ 28 - 0
packages/app/src/server/models/vo/error-search.ts

@@ -0,0 +1,28 @@
+import ExtensibleCustomError from 'extensible-custom-error';
+
+import { AllTermsKey } from '~/server/interfaces/search';
+
+export class SearchError extends ExtensibleCustomError {
+
+  readonly id = 'SearchError'
+
+  unavailableTermsKeys!: AllTermsKey[]
+
+  constructor(message = '', unavailableTermsKeys: AllTermsKey[]) {
+    super(message);
+    this.unavailableTermsKeys = unavailableTermsKeys;
+  }
+
+}
+
+export const isSearchError = (err: any): err is SearchError => {
+  if (err == null || typeof err !== 'object') {
+    return false;
+  }
+
+  if (err instanceof SearchError) {
+    return true;
+  }
+
+  return err?.id === 'SearchError';
+};

+ 10 - 3
packages/app/src/server/routes/search.js → packages/app/src/server/routes/search.ts

@@ -1,4 +1,5 @@
-const { default: loggerFactory } = require('~/utils/logger');
+import loggerFactory from '~/utils/logger';
+import { isSearchError, SearchError } from '../models/vo/error-search';
 
 const logger = loggerFactory('growi:routes:search');
 
@@ -35,8 +36,8 @@ module.exports = function(crowi, app) {
   const ApiResponse = require('../util/apiResponse');
   const ApiPaginate = require('../util/apiPaginate');
 
-  const actions = {};
-  const api = {};
+  const actions: any = {};
+  const api: any = {};
 
   actions.searchPage = function(req, res) {
     const keyword = req.query.q || null;
@@ -152,6 +153,12 @@ module.exports = function(crowi, app) {
     }
     catch (err) {
       logger.error('Failed to search', err);
+
+      if (isSearchError(err)) {
+        const { unavailableTermsKeys } = err;
+        return res.json(ApiResponse.error(err, 400, { unavailableTermsKeys }));
+      }
+
       return res.json(ApiResponse.error(err));
     }
 

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

@@ -38,6 +38,8 @@ const ES_SORT_ORDER = {
   [ASC]: 'asc',
 };
 
+const AVAILABLE_KEYS = ['match', 'not_match', 'phrase', 'not_phrase', 'prefix', 'not_prefix', 'tag', 'not_tag'];
+
 type Data = any;
 
 class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQueryTerms> {
@@ -737,7 +739,7 @@ class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQuer
     return query;
   }
 
-  appendCriteriaForQueryString(query, parsedKeywords: QueryTerms) {
+  appendCriteriaForQueryString(query, parsedKeywords: ESQueryTerms): void {
     query = this.initializeBoolQuery(query); // eslint-disable-line no-param-reassign
 
     if (parsedKeywords.match.length > 0) {
@@ -962,7 +964,7 @@ class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQuer
     };
   }
 
-  async search(data: SearchableData, user, userGroups, option): Promise<ISearchResult<unknown>> {
+  async search(data: SearchableData<ESQueryTerms>, user, userGroups, option): Promise<ISearchResult<unknown>> {
     const { queryString, terms } = data;
 
     if (terms == null) {
@@ -989,11 +991,15 @@ class ElasticsearchDelegator implements SearchDelegator<Data, ESTermsKey, ESQuer
   }
 
   isTermsNormalized(terms: Partial<QueryTerms>): terms is ESQueryTerms {
-    return true;
+    const keys = Object.keys(terms);
+
+    return keys.every(k => AVAILABLE_KEYS.includes(k));
   }
 
   validateTerms(terms: QueryTerms): UnavailableTermsKey<ESTermsKey>[] {
-    return [];
+    const keys = Object.keys(terms);
+
+    return keys.filter((k): k is UnavailableTermsKey<ESTermsKey> => !AVAILABLE_KEYS.includes(k));
   }
 
   async syncPageUpdated(page, user) {

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

@@ -1,6 +1,6 @@
 import mongoose from 'mongoose';
 
-import { PageModel, PageDocument } from '~/server/models/page';
+import { PageModel, PageDocument, PageQueryBuilder } from '~/server/models/page';
 import { SearchDelegatorName } from '~/interfaces/named-query';
 import { IPage } from '~/interfaces/page';
 import {
@@ -10,6 +10,7 @@ import {
 import { serializeUserSecurely } from '../../models/serializers/user-serializer';
 import { ISearchResult } from '~/interfaces/search';
 
+const AVAILABLE_KEYS = ['match', 'not_match', 'prefix', 'not_prefix'];
 
 class PrivateLegacyPagesDelegator implements SearchDelegator<IPage, MongoTermsKey, MongoQueryTerms> {
 
@@ -19,7 +20,8 @@ class PrivateLegacyPagesDelegator implements SearchDelegator<IPage, MongoTermsKe
     this.name = SearchDelegatorName.PRIVATE_LEGACY_PAGES;
   }
 
-  async search(data: SearchableData | null, user, userGroups, option): Promise<ISearchResult<IPage>> {
+  async search(data: SearchableData<MongoQueryTerms>, user, userGroups, option): Promise<ISearchResult<IPage>> {
+    const { terms } = data;
     const { offset, limit } = option;
 
     if (offset == null || limit == null) {
@@ -60,12 +62,37 @@ class PrivateLegacyPagesDelegator implements SearchDelegator<IPage, MongoTermsKe
     };
   }
 
+  addConditionByTerms(builder: PageQueryBuilder, terms: MongoQueryTerms): PageQueryBuilder {
+    const {
+      match, not_match: notMatch, prefix, not_prefix: notPrefix,
+    } = terms;
+
+    if (match.length > 0) {
+      match.forEach(m => builder.addConditionToListByMatch(m));
+    }
+    if (notMatch.length > 0) {
+      notMatch.forEach(nm => builder.addConditionToListByNotMatch(nm));
+    }
+    if (prefix.length > 0) {
+      prefix.forEach(p => builder.addConditionToListByStartWith(p));
+    }
+    if (notPrefix.length > 0) {
+      notPrefix.forEach(np => builder.addConditionToListByNotStartWith(np));
+    }
+
+    return builder;
+  }
+
   isTermsNormalized(terms: Partial<QueryTerms>): terms is MongoQueryTerms {
-    return true;
+    const keys = Object.keys(terms);
+
+    return keys.every(k => AVAILABLE_KEYS.includes(k));
   }
 
   validateTerms(terms: QueryTerms): UnavailableTermsKey<MongoTermsKey>[] {
-    return [];
+    const keys = Object.keys(terms);
+
+    return keys.filter((k): k is UnavailableTermsKey<MongoTermsKey> => !AVAILABLE_KEYS.includes(k));
   }
 
 }

+ 36 - 14
packages/app/src/server/service/search.ts

@@ -17,6 +17,7 @@ import { PageModel } from '../models/page';
 import { serializeUserSecurely } from '../models/serializers/user-serializer';
 
 import { ObjectIdLike } from '../interfaces/mongoose-utils';
+import { SearchError } from '../models/vo/error-search';
 
 // eslint-disable-next-line no-unused-vars
 const logger = loggerFactory('growi:service:search');
@@ -39,6 +40,10 @@ const normalizeQueryString = (_queryString: string): string => {
   return queryString;
 };
 
+const normalizeNQName = (nqName: string): string => {
+  return nqName.trim();
+};
+
 const findPageListByIds = async(pageIds: ObjectIdLike[], crowi: any) => {
 
   const Page = crowi.model('Page') as unknown as PageModel;
@@ -127,7 +132,7 @@ class SearchService implements SearchQueryParser, SearchResolver {
   generateNQDelegators(defaultDelegator: ElasticsearchDelegator): {[key in SearchDelegatorName]: SearchDelegator} {
     return {
       [SearchDelegatorName.DEFAULT]: defaultDelegator,
-      [SearchDelegatorName.PRIVATE_LEGACY_PAGES]: new PrivateLegacyPagesDelegator() as SearchDelegator,
+      [SearchDelegatorName.PRIVATE_LEGACY_PAGES]: new PrivateLegacyPagesDelegator() as unknown as SearchDelegator,
     };
   }
 
@@ -218,36 +223,32 @@ class SearchService implements SearchQueryParser, SearchResolver {
     return this.fullTextSearchDelegator.rebuildIndex();
   }
 
-  // TODO: https://redmine.weseek.co.jp/issues/92049 No need to parseNQString anymore
   async parseSearchQuery(queryString: string, nqName: string | null): Promise<ParsedQuery> {
     // eslint-disable-next-line no-param-reassign
     queryString = normalizeQueryString(queryString);
 
-    const regexp = new RegExp(/^\[nq:.+\]$/g); // https://regex101.com/r/FzDUvT/1
-    const replaceRegexp = new RegExp(/\[nq:|\]/g);
+    const terms = this.parseQueryString(queryString);
 
-    // when Normal Query
-    if (!regexp.test(queryString)) {
-      return { queryString, terms: this.parseQueryString(queryString) };
+    if (nqName == null) {
+      return { queryString, terms };
     }
 
-    // when Named Query
-    const name = queryString.replace(replaceRegexp, '');
-    const nq = await NamedQuery.findOne({ name });
+    const nq = await NamedQuery.findOne({ name: normalizeNQName(nqName) });
 
     // will delegate to full-text search
     if (nq == null) {
-      return { queryString, terms: this.parseQueryString(queryString) };
+      logger.debug(`Delegated to full-text search since a named query document did not found. (nqName="${nqName}")`);
+      return { queryString, terms };
     }
 
     const { aliasOf, delegatorName } = nq;
 
-    let parsedQuery;
+    let parsedQuery: ParsedQuery;
     if (aliasOf != null) {
       parsedQuery = { queryString: normalizeQueryString(aliasOf), terms: this.parseQueryString(aliasOf) };
     }
-    if (delegatorName != null) {
-      parsedQuery = { queryString, delegatorName };
+    else {
+      parsedQuery = { queryString, terms, delegatorName };
     }
 
     return parsedQuery;
@@ -264,6 +265,24 @@ class SearchService implements SearchQueryParser, SearchResolver {
     return [nqDeledator, data];
   }
 
+  /**
+   * Throws SearchError if data is corrupted.
+   * @param {SearchableData} data
+   * @param {SearchDelegator} delegator
+   * @throws {SearchError} SearchError
+   */
+  private validateSearchableData(delegator: SearchDelegator, data: SearchableData): void {
+    const { terms } = data;
+
+    if (delegator.isTermsNormalized(terms)) {
+      return;
+    }
+
+    const unavailableTermsKeys = delegator.validateTerms(terms);
+
+    throw new SearchError('The query string includes unavailable terms.', unavailableTermsKeys);
+  }
+
   async searchKeyword(keyword: string, nqName: string | null, user, userGroups, searchOpts): Promise<[ISearchResult<unknown>, string | null]> {
     let parsedQuery: ParsedQuery;
     // parse
@@ -286,6 +305,9 @@ class SearchService implements SearchQueryParser, SearchResolver {
       throw err;
     }
 
+    // throws
+    this.validateSearchableData(delegator, data);
+
     return [await delegator.search(data, user, userGroups, searchOpts), delegator.name ?? null];
   }