From 5b321c1eb51b9d52cd7c731dc237c6b15b6c3e88 Mon Sep 17 00:00:00 2001 From: Florian Necas Date: Thu, 5 Oct 2023 14:14:29 +0200 Subject: [PATCH] feat(es): use simpler logic for generating a query string from filters feat(es): move simple filters to the filter array to save performance see https://www.elastic.co/guide/en/elasticsearch/reference/7.17/query-dsl-bool-query.html#query-dsl-bool-query "The clause (query) must appear in matching documents. However unlike must the score of the query will be ignored. Filter clauses are executed in filter context, meaning that scoring is ignored and clauses are considered for caching." --- .../elasticsearch.service.spec.ts | 195 +++++++++++++----- .../elasticsearch/elasticsearch.service.ts | 66 ++++-- ...rganizations-from-metadata.service.spec.ts | 4 +- 3 files changed, 198 insertions(+), 67 deletions(-) diff --git a/libs/api/repository/src/lib/gn4/elasticsearch/elasticsearch.service.spec.ts b/libs/api/repository/src/lib/gn4/elasticsearch/elasticsearch.service.spec.ts index 4dbe744ef7..1851409542 100644 --- a/libs/api/repository/src/lib/gn4/elasticsearch/elasticsearch.service.spec.ts +++ b/libs/api/repository/src/lib/gn4/elasticsearch/elasticsearch.service.spec.ts @@ -1,6 +1,6 @@ import { ElasticsearchService } from './elasticsearch.service' import { ES_FIXTURE_AGGS_RESPONSE } from '@geonetwork-ui/common/fixtures' -import { EsSearchParams } from '../types/elasticsearch.model' +import { EsSearchParams } from '@geonetwork-ui/api/metadata-converter' describe('ElasticsearchService', () => { let service: ElasticsearchService @@ -96,26 +96,31 @@ describe('ElasticsearchService', () => { }) }) describe('#buildPayloadQuery', () => { - it('add any and other fields query_strings', () => { + it('should not add fields query_strings if fieldsSearchFilters Object is empty', () => { const query = service['buildPayloadQuery']( { - Org: { - world: true, - }, any: 'hello', }, - {} + {}, + ['record-1', 'record-2', 'record-3'] ) + expect(query).toEqual({ bool: { - filter: [], - should: [], - must: [ + filter: [ { terms: { isTemplate: ['n'], }, }, + { + ids: { + values: ['record-1', 'record-2', 'record-3'], + }, + }, + ], + should: [], + must: [ { query_string: { default_operator: 'AND', @@ -130,11 +135,6 @@ describe('ElasticsearchService', () => { query: 'hello', }, }, - { - query_string: { - query: '(Org:"world")', - }, - }, ], must_not: { terms: { @@ -157,14 +157,25 @@ describe('ElasticsearchService', () => { ) expect(query).toEqual({ bool: { - filter: [], - should: [], - must: [ + filter: [ { terms: { isTemplate: ['n'], }, }, + { + query_string: { + query: 'Org:("world")', + }, + }, + { + ids: { + values: ['record-1', 'record-2', 'record-3'], + }, + }, + ], + should: [], + must: [ { query_string: { default_operator: 'AND', @@ -179,16 +190,6 @@ describe('ElasticsearchService', () => { query: 'hello', }, }, - { - query_string: { - query: '(Org:"world")', - }, - }, - { - ids: { - values: ['record-1', 'record-2', 'record-3'], - }, - }, ], must_not: { terms: { @@ -203,6 +204,10 @@ describe('ElasticsearchService', () => { { Org: { world: true, + world2: true, + }, + name: { + john: true, }, any: 'hello', }, @@ -211,14 +216,25 @@ describe('ElasticsearchService', () => { ) expect(query).toEqual({ bool: { - filter: [], - should: [], - must: [ + filter: [ { terms: { isTemplate: ['n'], }, }, + { + query_string: { + query: 'Org:("world" OR "world2") AND name:("john")', + }, + }, + { + ids: { + values: [], + }, + }, + ], + should: [], + must: [ { query_string: { default_operator: 'AND', @@ -233,9 +249,38 @@ describe('ElasticsearchService', () => { query: 'hello', }, }, + ], + must_not: { + terms: { + resourceType: ['service', 'map', 'map/static', 'mapDigital'], + }, + }, + }, + }) + }) + it('handle negative and empty filters', () => { + const query = service['buildPayloadQuery']( + { + Org: { + world: false, + }, + name: {}, + message: '', + }, + {}, + [] + ) + expect(query).toEqual({ + bool: { + filter: [ + { + terms: { + isTemplate: ['n'], + }, + }, { query_string: { - query: '(Org:"world")', + query: 'Org:(-"world")', }, }, { @@ -244,6 +289,61 @@ describe('ElasticsearchService', () => { }, }, ], + should: [], + must: [], + must_not: { + terms: { + resourceType: ['service', 'map', 'map/static', 'mapDigital'], + }, + }, + }, + }) + }) + it('handle filters expressed as queries', () => { + const query = service['buildPayloadQuery']( + { + Org: 'world AND world2', + any: 'hello', + }, + {}, + [] + ) + expect(query).toEqual({ + bool: { + filter: [ + { + terms: { + isTemplate: ['n'], + }, + }, + { + query_string: { + query: 'Org:(world AND world2)', + }, + }, + { + ids: { + values: [], + }, + }, + ], + should: [], + must: [ + { + query_string: { + default_operator: 'AND', + fields: [ + 'resourceTitleObject.langfre^5', + 'tag.langfre^4', + 'resourceAbstractObject.langfre^3', + 'lineageObject.langfre^2', + 'any.langfre', + 'uuid', + ], + query: 'hello', + }, + }, + ], must_not: { terms: { resourceType: ['service', 'map', 'map/static', 'mapDigital'], @@ -264,7 +364,7 @@ describe('ElasticsearchService', () => { ) }) it('escapes special char', () => { - expect(query.bool.must[1].query_string.query).toEqual( + expect(query.bool.must[0].query_string.query).toEqual( `scot \\(\\)\\{\\?\\[ \\/ test` ) }) @@ -287,9 +387,7 @@ describe('ElasticsearchService', () => { it('adds boosting of 7 for intersecting with it and boosting of 10 on geoms within', () => { const query = service['buildPayloadQuery']( { - Org: { - world: true, - }, + Org: 'world', any: 'hello', }, {}, @@ -298,13 +396,19 @@ describe('ElasticsearchService', () => { ) expect(query).toEqual({ bool: { - filter: [], - must: [ + filter: [ { terms: { isTemplate: ['n'], }, }, + { + query_string: { + query: 'Org:(world)', + }, + }, + ], + must: [ { query_string: { default_operator: 'AND', @@ -319,11 +423,6 @@ describe('ElasticsearchService', () => { query: 'hello', }, }, - { - query_string: { - query: '(Org:"world")', - }, - }, ], must_not: { terms: { @@ -611,6 +710,7 @@ describe('ElasticsearchService', () => { filters: { filter1: { field1: '100' }, filter2: { field2: { value1: true, value3: true } }, + filter3: 'my own query', }, }, myHistogram: { @@ -623,14 +723,13 @@ describe('ElasticsearchService', () => { myFilters: { filters: { filter1: { - match: { - field1: '100', - }, + query_string: { query: 'field1:(100)' }, }, filter2: { - match: { - field2: { value1: true, value3: true }, - }, + query_string: { query: 'field2:("value1" OR "value3")' }, + }, + filter3: { + query_string: { query: 'my own query' }, }, }, }, diff --git a/libs/api/repository/src/lib/gn4/elasticsearch/elasticsearch.service.ts b/libs/api/repository/src/lib/gn4/elasticsearch/elasticsearch.service.ts index 8a9b92af1d..4da929819e 100644 --- a/libs/api/repository/src/lib/gn4/elasticsearch/elasticsearch.service.ts +++ b/libs/api/repository/src/lib/gn4/elasticsearch/elasticsearch.service.ts @@ -5,6 +5,8 @@ import { Aggregation, AggregationParams, AggregationsParams, + FieldFilter, + FieldFilters, FilterAggregationParams, SortByField, } from '@geonetwork-ui/common/domain/search' @@ -177,17 +179,36 @@ export class ElasticsearchService { }) } + private filtersToQueryString(filters: FieldFilters): string { + const makeQuery = (filter: FieldFilter): string => { + if (typeof filter === 'string') { + return filter + } + return Object.keys(filter) + .map((key) => { + if (filter[key] === true) { + return `"${key}"` + } + return `-"${key}"` + }) + .join(' OR ') + } + return Object.keys(filters) + .filter( + (fieldname) => + filters[fieldname] && JSON.stringify(filters[fieldname]) !== '{}' + ) + .map((fieldname) => `${fieldname}:(${makeQuery(filters[fieldname])})`) + .join(' AND ') + } + private buildPayloadQuery( { any, ...fieldSearchFilters }: SearchFilters, configFilters: SearchFilters, uuids?: string[], geometry?: Geometry ) { - const queryFilters = this.stateFiltersToQueryString(fieldSearchFilters) - const must = [this.queryFilterOnValues('isTemplate', 'n')] as Record< - string, - unknown - >[] + const must = [] as Record[] const must_not = { ...this.queryFilterOnValues('resourceType', [ 'service', @@ -197,6 +218,10 @@ export class ElasticsearchService { ]), } const should = [] as Record[] + const filter = [this.queryFilterOnValues('isTemplate', 'n')] as Record< + string, + unknown + >[] if (any) { must.push({ @@ -210,15 +235,16 @@ export class ElasticsearchService { }, }) } + const queryFilters = this.filtersToQueryString(fieldSearchFilters) if (queryFilters) { - must.push({ + filter.push({ query_string: { query: queryFilters, }, }) } if (uuids) { - must.push({ + filter.push({ ids: { values: uuids, }, @@ -252,7 +278,7 @@ export class ElasticsearchService { must, must_not, should, - filter: [], + filter, }, } } @@ -348,6 +374,7 @@ export class ElasticsearchService { * } * } */ + // FIXME: this is not used anymore stateFiltersToQueryString(facetsState) { const query = [] for (const indexKey in facetsState) { @@ -365,6 +392,7 @@ export class ElasticsearchService { return this.combineQueryGroups(query) } + // FIXME: this is not used anymore private parseStateNode(nodeName, node, indexKey) { let queryString = '' if (node && typeof node === 'object') { @@ -416,20 +444,24 @@ export class ElasticsearchService { } buildAggregationsPayload(aggregations: AggregationsParams): any { - const mapFilterAggregation = (filterAgg: FilterAggregationParams) => ({ - match: filterAgg, - }) const mapToESAggregation = (aggregation: AggregationParams) => { switch (aggregation.type) { case 'filters': return { - filters: Object.keys(aggregation.filters).reduce( - (prev, curr) => ({ + filters: Object.keys(aggregation.filters).reduce((prev, curr) => { + const filter = aggregation.filters[curr] + return { ...prev, - [curr]: mapFilterAggregation(aggregation.filters[curr]), - }), - {} - ), + [curr]: { + query_string: { + query: + typeof filter === 'string' + ? filter + : this.filtersToQueryString(filter), + }, + }, + } + }, {}), } case 'terms': return { diff --git a/libs/api/repository/src/lib/gn4/organizations/organizations-from-metadata.service.spec.ts b/libs/api/repository/src/lib/gn4/organizations/organizations-from-metadata.service.spec.ts index 09fd07fcb9..879763bec1 100644 --- a/libs/api/repository/src/lib/gn4/organizations/organizations-from-metadata.service.spec.ts +++ b/libs/api/repository/src/lib/gn4/organizations/organizations-from-metadata.service.spec.ts @@ -270,7 +270,7 @@ describe.each(['4.2.2-00', '4.2.3-xx', '4.2.5-xx'])( size: 0, query: { bool: { - must: [{ terms: { isTemplate: ['n'] } }], + must: [], must_not: { terms: { resourceType: [ @@ -282,7 +282,7 @@ describe.each(['4.2.2-00', '4.2.3-xx', '4.2.5-xx'])( }, }, should: [], - filter: [], + filter: [{ terms: { isTemplate: ['n'] } }], }, }, _source: [],