From 29d3838c6df22cae23468b4f1aad0c4e507e0625 Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Mon, 9 Nov 2020 13:54:11 +0100 Subject: [PATCH] SavedObjects search_dsl: add match_phrase_prefix clauses when using prefix search (#82693) (#82933) * add match_phrase_prefix clauses when using prefix search * add FTR tests --- .../lib/search_dsl/query_params.test.ts | 473 ++++++++++++------ .../service/lib/search_dsl/query_params.ts | 189 +++++-- .../service/lib/search_dsl/search_dsl.test.ts | 1 - .../service/lib/search_dsl/search_dsl.ts | 1 - .../apis/saved_objects/find.js | 64 +++ .../saved_objects/find_edgecases/data.json | 93 ++++ .../find_edgecases/mappings.json | 267 ++++++++++ 7 files changed, 883 insertions(+), 205 deletions(-) create mode 100644 test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/data.json create mode 100644 test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/mappings.json diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts index 333f5caf72525..a8c5df8d64630 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts @@ -21,28 +21,64 @@ import { esKuery } from '../../../es_query'; type KueryNode = any; -import { typeRegistryMock } from '../../../saved_objects_type_registry.mock'; +import { SavedObjectTypeRegistry } from '../../../saved_objects_type_registry'; import { ALL_NAMESPACES_STRING } from '../utils'; import { getQueryParams, getClauseForReference } from './query_params'; -const registry = typeRegistryMock.create(); +const registerTypes = (registry: SavedObjectTypeRegistry) => { + registry.registerType({ + name: 'pending', + hidden: false, + namespaceType: 'single', + mappings: { + properties: { title: { type: 'text' } }, + }, + management: { + defaultSearchField: 'title', + }, + }); -const MAPPINGS = { - properties: { - pending: { properties: { title: { type: 'text' } } }, - saved: { + registry.registerType({ + name: 'saved', + hidden: false, + namespaceType: 'single', + mappings: { properties: { title: { type: 'text', fields: { raw: { type: 'keyword' } } }, obj: { properties: { key1: { type: 'text' } } }, }, }, - // mock registry returns isMultiNamespace=true for 'shared' type - shared: { properties: { name: { type: 'keyword' } } }, - // mock registry returns isNamespaceAgnostic=true for 'global' type - global: { properties: { name: { type: 'keyword' } } }, - }, + management: { + defaultSearchField: 'title', + }, + }); + + registry.registerType({ + name: 'shared', + hidden: false, + namespaceType: 'multiple', + mappings: { + properties: { name: { type: 'keyword' } }, + }, + management: { + defaultSearchField: 'name', + }, + }); + + registry.registerType({ + name: 'global', + hidden: false, + namespaceType: 'agnostic', + mappings: { + properties: { name: { type: 'keyword' } }, + }, + management: { + defaultSearchField: 'name', + }, + }); }; -const ALL_TYPES = Object.keys(MAPPINGS.properties); + +const ALL_TYPES = ['pending', 'saved', 'shared', 'global']; // get all possible subsets (combination) of all types const ALL_TYPE_SUBSETS = ALL_TYPES.reduce( (subsets, value) => subsets.concat(subsets.map((set) => [...set, value])), @@ -51,48 +87,53 @@ const ALL_TYPE_SUBSETS = ALL_TYPES.reduce( .filter((x) => x.length) // exclude empty set .map((x) => (x.length === 1 ? x[0] : x)); // if a subset is a single string, destructure it -const createTypeClause = (type: string, namespaces?: string[]) => { - if (registry.isMultiNamespace(type)) { - const array = [...(namespaces ?? ['default']), ALL_NAMESPACES_STRING]; - return { - bool: { - must: expect.arrayContaining([{ terms: { namespaces: array } }]), - must_not: [{ exists: { field: 'namespace' } }], - }, - }; - } else if (registry.isSingleNamespace(type)) { - const nonDefaultNamespaces = namespaces?.filter((n) => n !== 'default') ?? []; - const should: any = []; - if (nonDefaultNamespaces.length > 0) { - should.push({ terms: { namespace: nonDefaultNamespaces } }); - } - if (namespaces?.includes('default')) { - should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } }); - } - return { - bool: { - must: [{ term: { type } }], - should: expect.arrayContaining(should), - minimum_should_match: 1, - must_not: [{ exists: { field: 'namespaces' } }], - }, - }; - } - // isNamespaceAgnostic - return { - bool: expect.objectContaining({ - must_not: [{ exists: { field: 'namespace' } }, { exists: { field: 'namespaces' } }], - }), - }; -}; - /** * Note: these tests cases are defined in the order they appear in the source code, for readability's sake */ describe('#getQueryParams', () => { - const mappings = MAPPINGS; + let registry: SavedObjectTypeRegistry; type Result = ReturnType; + beforeEach(() => { + registry = new SavedObjectTypeRegistry(); + registerTypes(registry); + }); + + const createTypeClause = (type: string, namespaces?: string[]) => { + if (registry.isMultiNamespace(type)) { + const array = [...(namespaces ?? ['default']), ALL_NAMESPACES_STRING]; + return { + bool: { + must: expect.arrayContaining([{ terms: { namespaces: array } }]), + must_not: [{ exists: { field: 'namespace' } }], + }, + }; + } else if (registry.isSingleNamespace(type)) { + const nonDefaultNamespaces = namespaces?.filter((n) => n !== 'default') ?? []; + const should: any = []; + if (nonDefaultNamespaces.length > 0) { + should.push({ terms: { namespace: nonDefaultNamespaces } }); + } + if (namespaces?.includes('default')) { + should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } }); + } + return { + bool: { + must: [{ term: { type } }], + should: expect.arrayContaining(should), + minimum_should_match: 1, + must_not: [{ exists: { field: 'namespaces' } }], + }, + }; + } + // isNamespaceAgnostic + return { + bool: expect.objectContaining({ + must_not: [{ exists: { field: 'namespace' } }, { exists: { field: 'namespaces' } }], + }), + }; + }; + describe('kueryNode filter clause', () => { const expectResult = (result: Result, expected: any) => { expect(result.query.bool.filter).toEqual(expect.arrayContaining([expected])); @@ -100,13 +141,13 @@ describe('#getQueryParams', () => { describe('`kueryNode` parameter', () => { it('does not include the clause when `kueryNode` is not specified', () => { - const result = getQueryParams({ mappings, registry, kueryNode: undefined }); + const result = getQueryParams({ registry, kueryNode: undefined }); expect(result.query.bool.filter).toHaveLength(1); }); it('includes the specified Kuery clause', () => { const test = (kueryNode: KueryNode) => { - const result = getQueryParams({ mappings, registry, kueryNode }); + const result = getQueryParams({ registry, kueryNode }); const expected = esKuery.toElasticsearchQuery(kueryNode); expect(result.query.bool.filter).toHaveLength(2); expectResult(result, expected); @@ -165,7 +206,6 @@ describe('#getQueryParams', () => { it('does not include the clause when `hasReference` is not specified', () => { const result = getQueryParams({ - mappings, registry, hasReference: undefined, }); @@ -176,7 +216,6 @@ describe('#getQueryParams', () => { it('creates a should clause for specified reference when operator is `OR`', () => { const hasReference = { id: 'foo', type: 'bar' }; const result = getQueryParams({ - mappings, registry, hasReference, hasReferenceOperator: 'OR', @@ -192,7 +231,6 @@ describe('#getQueryParams', () => { it('creates a must clause for specified reference when operator is `AND`', () => { const hasReference = { id: 'foo', type: 'bar' }; const result = getQueryParams({ - mappings, registry, hasReference, hasReferenceOperator: 'AND', @@ -210,7 +248,6 @@ describe('#getQueryParams', () => { { id: 'hello', type: 'dolly' }, ]; const result = getQueryParams({ - mappings, registry, hasReference, hasReferenceOperator: 'OR', @@ -229,7 +266,6 @@ describe('#getQueryParams', () => { { id: 'hello', type: 'dolly' }, ]; const result = getQueryParams({ - mappings, registry, hasReference, hasReferenceOperator: 'AND', @@ -244,7 +280,6 @@ describe('#getQueryParams', () => { it('defaults to `OR` when operator is not specified', () => { const hasReference = { id: 'foo', type: 'bar' }; const result = getQueryParams({ - mappings, registry, hasReference, }); @@ -278,14 +313,13 @@ describe('#getQueryParams', () => { }; it('searches for all known types when `type` is not specified', () => { - const result = getQueryParams({ mappings, registry, type: undefined }); + const result = getQueryParams({ registry, type: undefined }); expectResult(result, ...ALL_TYPES); }); it('searches for specified type/s', () => { const test = (typeOrTypes: string | string[]) => { const result = getQueryParams({ - mappings, registry, type: typeOrTypes, }); @@ -309,18 +343,17 @@ describe('#getQueryParams', () => { const test = (namespaces?: string[]) => { for (const typeOrTypes of ALL_TYPE_SUBSETS) { - const result = getQueryParams({ mappings, registry, type: typeOrTypes, namespaces }); + const result = getQueryParams({ registry, type: typeOrTypes, namespaces }); const types = Array.isArray(typeOrTypes) ? typeOrTypes : [typeOrTypes]; expectResult(result, ...types.map((x) => createTypeClause(x, namespaces))); } // also test with no specified type/s - const result = getQueryParams({ mappings, registry, type: undefined, namespaces }); + const result = getQueryParams({ registry, type: undefined, namespaces }); expectResult(result, ...ALL_TYPES.map((x) => createTypeClause(x, namespaces))); }; it('normalizes and deduplicates provided namespaces', () => { const result = getQueryParams({ - mappings, registry, search: '*', namespaces: ['foo', '*', 'foo', 'bar', 'default'], @@ -360,7 +393,6 @@ describe('#getQueryParams', () => { it('supersedes `type` and `namespaces` parameters', () => { const result = getQueryParams({ - mappings, registry, type: ['pending', 'saved', 'shared', 'global'], namespaces: ['foo', 'bar', 'default'], @@ -381,148 +413,266 @@ describe('#getQueryParams', () => { }); }); - describe('search clause (query.bool.must.simple_query_string)', () => { - const search = 'foo*'; + describe('search clause (query.bool)', () => { + describe('when using simple search (query.bool.must.simple_query_string)', () => { + const search = 'foo'; - const expectResult = (result: Result, sqsClause: any) => { - expect(result.query.bool.must).toEqual([{ simple_query_string: sqsClause }]); - }; + const expectResult = (result: Result, sqsClause: any) => { + expect(result.query.bool.must).toEqual([{ simple_query_string: sqsClause }]); + }; - describe('`search` parameter', () => { - it('does not include clause when `search` is not specified', () => { - const result = getQueryParams({ - mappings, - registry, - search: undefined, + describe('`search` parameter', () => { + it('does not include clause when `search` is not specified', () => { + const result = getQueryParams({ + registry, + search: undefined, + }); + expect(result.query.bool.must).toBeUndefined(); }); - expect(result.query.bool.must).toBeUndefined(); - }); - it('creates a clause with query for specified search', () => { - const result = getQueryParams({ - mappings, - registry, - search, + it('creates a clause with query for specified search', () => { + const result = getQueryParams({ + registry, + search, + }); + expectResult(result, expect.objectContaining({ query: search })); }); - expectResult(result, expect.objectContaining({ query: search })); }); - }); - describe('`searchFields` and `rootSearchFields` parameters', () => { - const getExpectedFields = (searchFields: string[], typeOrTypes: string | string[]) => { - const types = Array.isArray(typeOrTypes) ? typeOrTypes : [typeOrTypes]; - return searchFields.map((x) => types.map((y) => `${y}.${x}`)).flat(); - }; + describe('`searchFields` and `rootSearchFields` parameters', () => { + const getExpectedFields = (searchFields: string[], typeOrTypes: string | string[]) => { + const types = Array.isArray(typeOrTypes) ? typeOrTypes : [typeOrTypes]; + return searchFields.map((x) => types.map((y) => `${y}.${x}`)).flat(); + }; - const test = ({ - searchFields, - rootSearchFields, - }: { - searchFields?: string[]; - rootSearchFields?: string[]; - }) => { - for (const typeOrTypes of ALL_TYPE_SUBSETS) { + const test = ({ + searchFields, + rootSearchFields, + }: { + searchFields?: string[]; + rootSearchFields?: string[]; + }) => { + for (const typeOrTypes of ALL_TYPE_SUBSETS) { + const result = getQueryParams({ + registry, + type: typeOrTypes, + search, + searchFields, + rootSearchFields, + }); + let fields = rootSearchFields || []; + if (searchFields) { + fields = fields.concat(getExpectedFields(searchFields, typeOrTypes)); + } + expectResult(result, expect.objectContaining({ fields })); + } + // also test with no specified type/s const result = getQueryParams({ - mappings, registry, - type: typeOrTypes, + type: undefined, search, searchFields, rootSearchFields, }); let fields = rootSearchFields || []; if (searchFields) { - fields = fields.concat(getExpectedFields(searchFields, typeOrTypes)); + fields = fields.concat(getExpectedFields(searchFields, ALL_TYPES)); } expectResult(result, expect.objectContaining({ fields })); - } - // also test with no specified type/s - const result = getQueryParams({ - mappings, - registry, - type: undefined, - search, - searchFields, - rootSearchFields, + }; + + it('throws an error if a raw search field contains a "." character', () => { + expect(() => + getQueryParams({ + registry, + type: undefined, + search, + searchFields: undefined, + rootSearchFields: ['foo', 'bar.baz'], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"rootSearchFields entry \\"bar.baz\\" is invalid: cannot contain \\".\\" character"` + ); }); - let fields = rootSearchFields || []; - if (searchFields) { - fields = fields.concat(getExpectedFields(searchFields, ALL_TYPES)); - } - expectResult(result, expect.objectContaining({ fields })); - }; - it('throws an error if a raw search field contains a "." character', () => { - expect(() => - getQueryParams({ - mappings, + it('includes lenient flag and all fields when `searchFields` and `rootSearchFields` are not specified', () => { + const result = getQueryParams({ registry, - type: undefined, search, searchFields: undefined, - rootSearchFields: ['foo', 'bar.baz'], - }) - ).toThrowErrorMatchingInlineSnapshot( - `"rootSearchFields entry \\"bar.baz\\" is invalid: cannot contain \\".\\" character"` - ); + rootSearchFields: undefined, + }); + expectResult(result, expect.objectContaining({ lenient: true, fields: ['*'] })); + }); + + it('includes specified search fields for appropriate type/s', () => { + test({ searchFields: ['title'] }); + }); + + it('supports boosting', () => { + test({ searchFields: ['title^3'] }); + }); + + it('supports multiple search fields', () => { + test({ searchFields: ['title, title.raw'] }); + }); + + it('includes specified raw search fields', () => { + test({ rootSearchFields: ['_id'] }); + }); + + it('supports multiple raw search fields', () => { + test({ rootSearchFields: ['_id', 'originId'] }); + }); + + it('supports search fields and raw search fields', () => { + test({ searchFields: ['title'], rootSearchFields: ['_id'] }); + }); }); - it('includes lenient flag and all fields when `searchFields` and `rootSearchFields` are not specified', () => { - const result = getQueryParams({ - mappings, + describe('`defaultSearchOperator` parameter', () => { + it('does not include default_operator when `defaultSearchOperator` is not specified', () => { + const result = getQueryParams({ + registry, + search, + defaultSearchOperator: undefined, + }); + expectResult( + result, + expect.not.objectContaining({ default_operator: expect.anything() }) + ); + }); + + it('includes specified default operator', () => { + const defaultSearchOperator = 'AND'; + const result = getQueryParams({ + registry, + search, + defaultSearchOperator, + }); + expectResult( + result, + expect.objectContaining({ default_operator: defaultSearchOperator }) + ); + }); + }); + }); + + describe('when using prefix search (query.bool.should)', () => { + const searchQuery = 'foo*'; + + const getQueryParamForSearch = ({ + search, + searchFields, + type, + }: { + search?: string; + searchFields?: string[]; + type?: string[]; + }) => + getQueryParams({ registry, search, - searchFields: undefined, - rootSearchFields: undefined, + searchFields, + type, }); - expectResult(result, expect.objectContaining({ lenient: true, fields: ['*'] })); - }); - it('includes specified search fields for appropriate type/s', () => { - test({ searchFields: ['title'] }); - }); + it('uses a `should` clause instead of `must`', () => { + const result = getQueryParamForSearch({ search: searchQuery, searchFields: ['title'] }); - it('supports boosting', () => { - test({ searchFields: ['title^3'] }); + expect(result.query.bool.must).toBeUndefined(); + expect(result.query.bool.should).toEqual(expect.any(Array)); + expect(result.query.bool.should.length).toBeGreaterThanOrEqual(1); + expect(result.query.bool.minimum_should_match).toBe(1); }); - - it('supports multiple search fields', () => { - test({ searchFields: ['title, title.raw'] }); + it('includes the `simple_query_string` in the `should` clauses', () => { + const result = getQueryParamForSearch({ search: searchQuery, searchFields: ['title'] }); + expect(result.query.bool.should[0]).toEqual({ + simple_query_string: expect.objectContaining({ + query: searchQuery, + }), + }); }); - it('includes specified raw search fields', () => { - test({ rootSearchFields: ['_id'] }); + it('adds a should clause for each `searchFields` / `type` tuple', () => { + const result = getQueryParamForSearch({ + search: searchQuery, + searchFields: ['title', 'desc'], + type: ['saved', 'pending'], + }); + const shouldClauses = result.query.bool.should; + + expect(shouldClauses.length).toBe(5); + + const mppClauses = shouldClauses.slice(1); + + expect( + mppClauses.map((clause: any) => Object.keys(clause.match_phrase_prefix)[0]) + ).toEqual(['saved.title', 'pending.title', 'saved.desc', 'pending.desc']); }); - it('supports multiple raw search fields', () => { - test({ rootSearchFields: ['_id', 'originId'] }); + it('uses all registered types when `type` is not provided', () => { + const result = getQueryParamForSearch({ + search: searchQuery, + searchFields: ['title'], + type: undefined, + }); + const shouldClauses = result.query.bool.should; + + expect(shouldClauses.length).toBe(5); + + const mppClauses = shouldClauses.slice(1); + + expect( + mppClauses.map((clause: any) => Object.keys(clause.match_phrase_prefix)[0]) + ).toEqual(['pending.title', 'saved.title', 'shared.title', 'global.title']); }); - it('supports search fields and raw search fields', () => { - test({ searchFields: ['title'], rootSearchFields: ['_id'] }); + it('removes the prefix search wildcard from the query', () => { + const result = getQueryParamForSearch({ + search: searchQuery, + searchFields: ['title'], + type: ['saved'], + }); + const shouldClauses = result.query.bool.should; + const mppClauses = shouldClauses.slice(1); + + expect(mppClauses[0].match_phrase_prefix['saved.title'].query).toEqual('foo'); }); - }); - describe('`defaultSearchOperator` parameter', () => { - it('does not include default_operator when `defaultSearchOperator` is not specified', () => { - const result = getQueryParams({ - mappings, - registry, - search, - defaultSearchOperator: undefined, + it("defaults to the type's default search field when `searchFields` is not specified", () => { + const result = getQueryParamForSearch({ + search: searchQuery, + searchFields: undefined, + type: ['saved', 'global'], }); - expectResult(result, expect.not.objectContaining({ default_operator: expect.anything() })); + const shouldClauses = result.query.bool.should; + + expect(shouldClauses.length).toBe(3); + + const mppClauses = shouldClauses.slice(1); + + expect( + mppClauses.map((clause: any) => Object.keys(clause.match_phrase_prefix)[0]) + ).toEqual(['saved.title', 'global.name']); }); - it('includes specified default operator', () => { - const defaultSearchOperator = 'AND'; - const result = getQueryParams({ - mappings, - registry, - search, - defaultSearchOperator, + it('supports boosting', () => { + const result = getQueryParamForSearch({ + search: searchQuery, + searchFields: ['title^3', 'description'], + type: ['saved'], }); - expectResult(result, expect.objectContaining({ default_operator: defaultSearchOperator })); + const shouldClauses = result.query.bool.should; + + expect(shouldClauses.length).toBe(3); + + const mppClauses = shouldClauses.slice(1); + + expect(mppClauses.map((clause: any) => clause.match_phrase_prefix)).toEqual([ + { 'saved.title': { query: 'foo', boost: 3 } }, + { 'saved.description': { query: 'foo', boost: 1 } }, + ]); }); }); }); @@ -532,7 +682,6 @@ describe('#getQueryParams', () => { it(`throws for ${type} when namespaces is an empty array`, () => { expect(() => getQueryParams({ - mappings, registry, namespaces: [], }) diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index 8d4fe13b9bede..f73777c4f454f 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -20,7 +20,6 @@ import { esKuery } from '../../../es_query'; type KueryNode = any; -import { getRootPropertiesObjects, IndexMapping } from '../../../mappings'; import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry'; import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils'; @@ -28,22 +27,17 @@ import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils'; * Gets the types based on the type. Uses mappings to support * null type (all types), a single type string or an array */ -function getTypes(mappings: IndexMapping, type?: string | string[]) { +function getTypes(registry: ISavedObjectTypeRegistry, type?: string | string[]) { if (!type) { - return Object.keys(getRootPropertiesObjects(mappings)); + return registry.getAllTypes().map((registeredType) => registeredType.name); } - - if (Array.isArray(type)) { - return type; - } - - return [type]; + return Array.isArray(type) ? type : [type]; } /** * Get the field params based on the types, searchFields, and rootSearchFields */ -function getFieldsForTypes( +function getSimpleQueryStringTypeFields( types: string[], searchFields: string[] = [], rootSearchFields: string[] = [] @@ -130,7 +124,6 @@ export interface HasReferenceQueryParams { export type SearchOperator = 'AND' | 'OR'; interface QueryParams { - mappings: IndexMapping; registry: ISavedObjectTypeRegistry; namespaces?: string[]; type?: string | string[]; @@ -188,11 +181,26 @@ export function getClauseForReference(reference: HasReferenceQueryParams) { }; } +// A de-duplicated set of namespaces makes for a more efficient query. +// +// Additionally, we treat the `*` namespace as the `default` namespace. +// In the Default Distribution, the `*` is automatically expanded to include all available namespaces. +// However, the OSS distribution (and certain configurations of the Default Distribution) can allow the `*` +// to pass through to the SO Repository, and eventually to this module. When this happens, we translate to `default`, +// since that is consistent with how a single-namespace search behaves in the OSS distribution. Leaving the wildcard in place +// would result in no results being returned, as the wildcard is treated as a literal, and not _actually_ as a wildcard. +// We had a good discussion around the tradeoffs here: https://github.com/elastic/kibana/pull/67644#discussion_r441055716 +const normalizeNamespaces = (namespacesToNormalize?: string[]) => + namespacesToNormalize + ? Array.from( + new Set(namespacesToNormalize.map((x) => (x === '*' ? DEFAULT_NAMESPACE_STRING : x))) + ) + : undefined; + /** * Get the "query" related keys for the search body */ export function getQueryParams({ - mappings, registry, namespaces, type, @@ -206,7 +214,7 @@ export function getQueryParams({ kueryNode, }: QueryParams) { const types = getTypes( - mappings, + registry, typeToNamespacesMap ? Array.from(typeToNamespacesMap.keys()) : type ); @@ -214,28 +222,10 @@ export function getQueryParams({ hasReference = [hasReference]; } - // A de-duplicated set of namespaces makes for a more effecient query. - // - // Additonally, we treat the `*` namespace as the `default` namespace. - // In the Default Distribution, the `*` is automatically expanded to include all available namespaces. - // However, the OSS distribution (and certain configurations of the Default Distribution) can allow the `*` - // to pass through to the SO Repository, and eventually to this module. When this happens, we translate to `default`, - // since that is consistent with how a single-namespace search behaves in the OSS distribution. Leaving the wildcard in place - // would result in no results being returned, as the wildcard is treated as a literal, and not _actually_ as a wildcard. - // We had a good discussion around the tradeoffs here: https://github.com/elastic/kibana/pull/67644#discussion_r441055716 - const normalizeNamespaces = (namespacesToNormalize?: string[]) => - namespacesToNormalize - ? Array.from( - new Set(namespacesToNormalize.map((x) => (x === '*' ? DEFAULT_NAMESPACE_STRING : x))) - ) - : undefined; - const bool: any = { filter: [ ...(kueryNode != null ? [esKuery.toElasticsearchQuery(kueryNode)] : []), - ...(hasReference && hasReference.length - ? [getReferencesFilter(hasReference, hasReferenceOperator)] - : []), + ...(hasReference?.length ? [getReferencesFilter(hasReference, hasReferenceOperator)] : []), { bool: { should: types.map((shouldType) => { @@ -251,16 +241,133 @@ export function getQueryParams({ }; if (search) { - bool.must = [ - { - simple_query_string: { - query: search, - ...getFieldsForTypes(types, searchFields, rootSearchFields), - ...(defaultSearchOperator ? { default_operator: defaultSearchOperator } : {}), - }, - }, - ]; + const useMatchPhrasePrefix = shouldUseMatchPhrasePrefix(search); + const simpleQueryStringClause = getSimpleQueryStringClause({ + search, + types, + searchFields, + rootSearchFields, + defaultSearchOperator, + }); + + if (useMatchPhrasePrefix) { + bool.should = [ + simpleQueryStringClause, + ...getMatchPhrasePrefixClauses({ search, searchFields, types, registry }), + ]; + bool.minimum_should_match = 1; + } else { + bool.must = [simpleQueryStringClause]; + } } return { query: { bool } }; } + +// we only want to add match_phrase_prefix clauses +// if the search is a prefix search +const shouldUseMatchPhrasePrefix = (search: string): boolean => { + return search.trim().endsWith('*'); +}; + +const getMatchPhrasePrefixClauses = ({ + search, + searchFields, + registry, + types, +}: { + search: string; + searchFields?: string[]; + types: string[]; + registry: ISavedObjectTypeRegistry; +}) => { + // need to remove the prefix search operator + const query = search.replace(/[*]$/, ''); + const mppFields = getMatchPhrasePrefixFields({ searchFields, types, registry }); + return mppFields.map(({ field, boost }) => { + return { + match_phrase_prefix: { + [field]: { + query, + boost, + }, + }, + }; + }); +}; + +interface FieldWithBoost { + field: string; + boost?: number; +} + +const getMatchPhrasePrefixFields = ({ + searchFields = [], + types, + registry, +}: { + searchFields?: string[]; + types: string[]; + registry: ISavedObjectTypeRegistry; +}): FieldWithBoost[] => { + const output: FieldWithBoost[] = []; + + searchFields = searchFields.filter((field) => field !== '*'); + let fields: string[]; + if (searchFields.length === 0) { + fields = types.reduce((typeFields, type) => { + const defaultSearchField = registry.getType(type)?.management?.defaultSearchField; + if (defaultSearchField) { + return [...typeFields, `${type}.${defaultSearchField}`]; + } + return typeFields; + }, [] as string[]); + } else { + fields = []; + for (const field of searchFields) { + fields = fields.concat(types.map((type) => `${type}.${field}`)); + } + } + + fields.forEach((rawField) => { + const [field, rawBoost] = rawField.split('^'); + let boost: number = 1; + if (rawBoost) { + try { + boost = parseInt(rawBoost, 10); + } catch (e) { + boost = 1; + } + } + if (isNaN(boost)) { + boost = 1; + } + output.push({ + field, + boost, + }); + }); + return output; +}; + +const getSimpleQueryStringClause = ({ + search, + types, + searchFields, + rootSearchFields, + defaultSearchOperator, +}: { + search: string; + types: string[]; + searchFields?: string[]; + rootSearchFields?: string[]; + defaultSearchOperator?: SearchOperator; +}) => { + return { + simple_query_string: { + query: search, + ...getSimpleQueryStringTypeFields(types, searchFields, rootSearchFields), + ...(defaultSearchOperator ? { default_operator: defaultSearchOperator } : {}), + }, + }; +}; diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts index a9f26f71a3f2b..3522ab9ef1736 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts @@ -76,7 +76,6 @@ describe('getSearchDsl', () => { getSearchDsl(mappings, registry, opts); expect(getQueryParams).toHaveBeenCalledTimes(1); expect(getQueryParams).toHaveBeenCalledWith({ - mappings, registry, namespaces: opts.namespaces, type: opts.type, diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts index d5da82e5617be..bddecc4d7f649 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts @@ -71,7 +71,6 @@ export function getSearchDsl( return { ...getQueryParams({ - mappings, registry, namespaces, type, diff --git a/test/api_integration/apis/saved_objects/find.js b/test/api_integration/apis/saved_objects/find.js index c2e36b4a669ff..e5da46644672b 100644 --- a/test/api_integration/apis/saved_objects/find.js +++ b/test/api_integration/apis/saved_objects/find.js @@ -334,6 +334,70 @@ export default function ({ getService }) { }); }); + describe('searching for special characters', () => { + before(() => esArchiver.load('saved_objects/find_edgecases')); + after(() => esArchiver.unload('saved_objects/find_edgecases')); + + it('can search for objects with dashes', async () => + await supertest + .get('/api/saved_objects/_find') + .query({ + type: 'visualization', + search_fields: 'title', + search: 'my-vis*', + }) + .expect(200) + .then((resp) => { + const savedObjects = resp.body.saved_objects; + expect(savedObjects.map((so) => so.attributes.title)).to.eql(['my-visualization']); + })); + + it('can search with the prefix search character just after a special one', async () => + await supertest + .get('/api/saved_objects/_find') + .query({ + type: 'visualization', + search_fields: 'title', + search: 'my-*', + }) + .expect(200) + .then((resp) => { + const savedObjects = resp.body.saved_objects; + expect(savedObjects.map((so) => so.attributes.title)).to.eql(['my-visualization']); + })); + + it('can search for objects with asterisk', async () => + await supertest + .get('/api/saved_objects/_find') + .query({ + type: 'visualization', + search_fields: 'title', + search: 'some*vi*', + }) + .expect(200) + .then((resp) => { + const savedObjects = resp.body.saved_objects; + expect(savedObjects.map((so) => so.attributes.title)).to.eql(['some*visualization']); + })); + + it('can still search tokens by prefix', async () => + await supertest + .get('/api/saved_objects/_find') + .query({ + type: 'visualization', + search_fields: 'title', + search: 'visuali*', + }) + .expect(200) + .then((resp) => { + const savedObjects = resp.body.saved_objects; + expect(savedObjects.map((so) => so.attributes.title)).to.eql([ + 'my-visualization', + 'some*visualization', + ]); + })); + }); + describe('without kibana index', () => { before( async () => diff --git a/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/data.json b/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/data.json new file mode 100644 index 0000000000000..0c8b35fd3f499 --- /dev/null +++ b/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/data.json @@ -0,0 +1,93 @@ +{ + "type": "doc", + "value": { + "index": ".kibana", + "id": "visualization:title-with-dash", + "source": { + "type": "visualization", + "updated_at": "2017-09-21T18:51:23.794Z", + "visualization": { + "title": "my-visualization", + "visState": "{}", + "uiStateJSON": "{\"spy\":{\"mode\":{\"name\":null,\"fill\":false}}}", + "description": "", + "version": 1, + "kibanaSavedObjectMeta": { + "searchSourceJSON": "{\"filter\":[],\"query\":{\"query\":\"\",\"language\":\"lucene\"}}" + } + }, + "references": [] + } + } +} + +{ + "type": "doc", + "value": { + "index": ".kibana", + "id": "visualization:title-with-asterisk", + "source": { + "type": "visualization", + "updated_at": "2017-09-21T18:51:23.794Z", + "visualization": { + "title": "some*visualization", + "visState": "{}", + "uiStateJSON": "{\"spy\":{\"mode\":{\"name\":null,\"fill\":false}}}", + "description": "", + "version": 1, + "kibanaSavedObjectMeta": { + "searchSourceJSON": "{\"filter\":[],\"query\":{\"query\":\"\",\"language\":\"lucene\"}}" + } + }, + "references": [] + } + } +} + + +{ + "type": "doc", + "value": { + "index": ".kibana", + "id": "visualization:noise-1", + "source": { + "type": "visualization", + "updated_at": "2017-09-21T18:51:23.794Z", + "visualization": { + "title": "Just some noise in the dataset", + "visState": "{}", + "uiStateJSON": "{\"spy\":{\"mode\":{\"name\":null,\"fill\":false}}}", + "description": "", + "version": 1, + "kibanaSavedObjectMeta": { + "searchSourceJSON": "{\"filter\":[],\"query\":{\"query\":\"\",\"language\":\"lucene\"}}" + } + }, + "references": [] + } + } +} + +{ + "type": "doc", + "value": { + "index": ".kibana", + "id": "visualization:noise-2", + "source": { + "type": "visualization", + "updated_at": "2017-09-21T18:51:23.794Z", + "visualization": { + "title": "Just some noise in the dataset", + "visState": "{}", + "uiStateJSON": "{\"spy\":{\"mode\":{\"name\":null,\"fill\":false}}}", + "description": "", + "version": 1, + "kibanaSavedObjectMeta": { + "searchSourceJSON": "{\"filter\":[],\"query\":{\"query\":\"\",\"language\":\"lucene\"}}" + } + }, + "references": [] + } + } +} + diff --git a/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/mappings.json b/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/mappings.json new file mode 100644 index 0000000000000..e601c43431437 --- /dev/null +++ b/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/mappings.json @@ -0,0 +1,267 @@ +{ + "type": "index", + "value": { + "index": ".kibana", + "settings": { + "index": { + "number_of_shards": "1", + "number_of_replicas": "1" + } + }, + "mappings": { + "dynamic": "strict", + "properties": { + "config": { + "dynamic": "true", + "properties": { + "buildNum": { + "type": "keyword" + }, + "defaultIndex": { + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } + } + } + }, + "dashboard": { + "properties": { + "description": { + "type": "text" + }, + "hits": { + "type": "integer" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "optionsJSON": { + "type": "text" + }, + "panelsJSON": { + "type": "text" + }, + "refreshInterval": { + "properties": { + "display": { + "type": "keyword" + }, + "pause": { + "type": "boolean" + }, + "section": { + "type": "integer" + }, + "value": { + "type": "integer" + } + } + }, + "timeFrom": { + "type": "keyword" + }, + "timeRestore": { + "type": "boolean" + }, + "timeTo": { + "type": "keyword" + }, + "title": { + "type": "text" + }, + "uiStateJSON": { + "type": "text" + }, + "version": { + "type": "integer" + } + } + }, + "index-pattern": { + "properties": { + "fieldFormatMap": { + "type": "text" + }, + "fields": { + "type": "text" + }, + "intervalName": { + "type": "keyword" + }, + "notExpandable": { + "type": "boolean" + }, + "sourceFilters": { + "type": "text" + }, + "timeFieldName": { + "type": "keyword" + }, + "title": { + "type": "text" + } + } + }, + "search": { + "properties": { + "columns": { + "type": "keyword" + }, + "description": { + "type": "text" + }, + "hits": { + "type": "integer" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "sort": { + "type": "keyword" + }, + "title": { + "type": "text" + }, + "version": { + "type": "integer" + } + } + }, + "server": { + "properties": { + "uuid": { + "type": "keyword" + } + } + }, + "timelion-sheet": { + "properties": { + "description": { + "type": "text" + }, + "hits": { + "type": "integer" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "timelion_chart_height": { + "type": "integer" + }, + "timelion_columns": { + "type": "integer" + }, + "timelion_interval": { + "type": "keyword" + }, + "timelion_other_interval": { + "type": "keyword" + }, + "timelion_rows": { + "type": "integer" + }, + "timelion_sheet": { + "type": "text" + }, + "title": { + "type": "text" + }, + "version": { + "type": "integer" + } + } + }, + "namespace": { + "type": "keyword" + }, + "references": { + "properties": { + "id": { + "type": "keyword" + }, + "name": { + "type": "keyword" + }, + "type": { + "type": "keyword" + } + }, + "type": "nested" + }, + "type": { + "type": "keyword" + }, + "updated_at": { + "type": "date" + }, + "url": { + "properties": { + "accessCount": { + "type": "long" + }, + "accessDate": { + "type": "date" + }, + "createDate": { + "type": "date" + }, + "url": { + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 2048 + } + } + } + } + }, + "visualization": { + "properties": { + "description": { + "type": "text" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "savedSearchId": { + "type": "keyword" + }, + "title": { + "type": "text" + }, + "uiStateJSON": { + "type": "text" + }, + "version": { + "type": "integer" + }, + "visState": { + "type": "text" + } + } + } + } + } + } +}