From 217806b7012d208fc80d8ae5017b6417a579e69f Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Fri, 6 Nov 2020 11:33:12 -0500 Subject: [PATCH] Allow the repository to search across all spaces --- .../lib/search_dsl/query_params.test.ts | 22 ++++++--- .../service/lib/search_dsl/query_params.ts | 49 +++++++++---------- .../apis/saved_objects/find.js | 25 +++++++++- 3 files changed, 59 insertions(+), 37 deletions(-) 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..fba5f57c82d61 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 @@ -22,7 +22,7 @@ import { esKuery } from '../../../es_query'; type KueryNode = any; import { typeRegistryMock } from '../../../saved_objects_type_registry.mock'; -import { ALL_NAMESPACES_STRING } from '../utils'; +import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils'; import { getQueryParams, getClauseForReference } from './query_params'; const registry = typeRegistryMock.create(); @@ -53,20 +53,26 @@ const ALL_TYPE_SUBSETS = ALL_TYPES.reduce( const createTypeClause = (type: string, namespaces?: string[]) => { if (registry.isMultiNamespace(type)) { - const array = [...(namespaces ?? ['default']), ALL_NAMESPACES_STRING]; + const array = [...(namespaces ?? [DEFAULT_NAMESPACE_STRING]), ALL_NAMESPACES_STRING]; + + const namespacesClause = { terms: { namespaces: array } }; return { bool: { - must: expect.arrayContaining([{ terms: { namespaces: array } }]), + must: namespaces?.includes(ALL_NAMESPACES_STRING) + ? expect.not.arrayContaining([namespacesClause]) + : expect.arrayContaining([namespacesClause]), must_not: [{ exists: { field: 'namespace' } }], }, }; } else if (registry.isSingleNamespace(type)) { - const nonDefaultNamespaces = namespaces?.filter((n) => n !== 'default') ?? []; + const nonDefaultNamespaces = namespaces?.filter((n) => n !== DEFAULT_NAMESPACE_STRING) ?? []; + const searchingAcrossAllNamespaces = namespaces?.includes(ALL_NAMESPACES_STRING) ?? false; + const should: any = []; - if (nonDefaultNamespaces.length > 0) { + if (nonDefaultNamespaces.length > 0 && !searchingAcrossAllNamespaces) { should.push({ terms: { namespace: nonDefaultNamespaces } }); } - if (namespaces?.includes('default')) { + if (namespaces?.includes(DEFAULT_NAMESPACE_STRING)) { should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } }); } return { @@ -318,7 +324,7 @@ describe('#getQueryParams', () => { expectResult(result, ...ALL_TYPES.map((x) => createTypeClause(x, namespaces))); }; - it('normalizes and deduplicates provided namespaces', () => { + it('deduplicates provided namespaces', () => { const result = getQueryParams({ mappings, registry, @@ -328,7 +334,7 @@ describe('#getQueryParams', () => { expectResult( result, - ...ALL_TYPES.map((x) => createTypeClause(x, ['foo', 'default', 'bar'])) + ...ALL_TYPES.map((x) => createTypeClause(x, ['foo', '*', 'bar', 'default'])) ); }); 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..19e43e9844567 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 @@ -81,34 +81,41 @@ function getClauseForType( if (namespaces.length === 0) { throw new Error('cannot specify empty namespaces array'); } + const searchAcrossAllNamespaces = namespaces.includes(ALL_NAMESPACES_STRING); + if (registry.isMultiNamespace(type)) { + const namespacesFilterClause = searchAcrossAllNamespaces + ? {} + : { terms: { namespaces: [...namespaces, ALL_NAMESPACES_STRING] } }; + return { bool: { - must: [ - { term: { type } }, - { terms: { namespaces: [...namespaces, ALL_NAMESPACES_STRING] } }, - ], + must: [{ term: { type } }, namespacesFilterClause], must_not: [{ exists: { field: 'namespace' } }], }, }; } else if (registry.isSingleNamespace(type)) { const should: Array> = []; const eligibleNamespaces = namespaces.filter((x) => x !== DEFAULT_NAMESPACE_STRING); - if (eligibleNamespaces.length > 0) { + if (eligibleNamespaces.length > 0 && !searchAcrossAllNamespaces) { should.push({ terms: { namespace: eligibleNamespaces } }); } if (namespaces.includes(DEFAULT_NAMESPACE_STRING)) { should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } }); } - if (should.length === 0) { - // This is indicitive of a bug, and not user error. - throw new Error('unhandled search condition: expected at least 1 `should` clause.'); - } + + const shouldClauseProps = + should.length > 0 + ? { + should, + minimum_should_match: 1, + } + : {}; + return { bool: { must: [{ term: { type } }], - should, - minimum_should_match: 1, + ...shouldClauseProps, must_not: [{ exists: { field: 'namespaces' } }], }, }; @@ -215,20 +222,8 @@ export function getQueryParams({ } // 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 uniqNamespaces = (namespacesToNormalize?: string[]) => + namespacesToNormalize ? Array.from(new Set(namespacesToNormalize)) : undefined; const bool: any = { filter: [ @@ -239,10 +234,10 @@ export function getQueryParams({ { bool: { should: types.map((shouldType) => { - const normalizedNamespaces = normalizeNamespaces( + const deduplicatedNamespaces = uniqNamespaces( typeToNamespacesMap ? typeToNamespacesMap.get(shouldType) : namespaces ); - return getClauseForType(registry, normalizedNamespaces, shouldType); + return getClauseForType(registry, deduplicatedNamespaces, shouldType); }), minimum_should_match: 1, }, diff --git a/test/api_integration/apis/saved_objects/find.js b/test/api_integration/apis/saved_objects/find.js index c2e36b4a669ff..57403cb9fd449 100644 --- a/test/api_integration/apis/saved_objects/find.js +++ b/test/api_integration/apis/saved_objects/find.js @@ -160,7 +160,7 @@ export default function ({ getService }) { }); describe('wildcard namespace', () => { - it('should return 200 with individual responses from the default namespace', async () => + it('should return 200 with individual responses from the all namespaces', async () => await supertest .get('/api/saved_objects/_find?type=visualization&fields=title&namespaces=*') .expect(200) @@ -168,7 +168,7 @@ export default function ({ getService }) { expect(resp.body).to.eql({ page: 1, per_page: 20, - total: 1, + total: 2, saved_objects: [ { type: 'visualization', @@ -189,6 +189,27 @@ export default function ({ getService }) { ], updated_at: '2017-09-21T18:51:23.794Z', }, + { + attributes: { + title: 'Count of requests', + }, + id: 'dd7caf20-9efd-11e7-acb3-3dab96693fab', + migrationVersion: { + visualization: '7.10.0', + }, + namespaces: ['foo-ns'], + references: [ + { + id: '91200a00-9efd-11e7-acb3-3dab96693fab', + name: 'kibanaSavedObjectMeta.searchSourceJSON.index', + type: 'index-pattern', + }, + ], + score: 0, + type: 'visualization', + updated_at: '2017-09-21T18:51:23.794Z', + version: 'WzYsMV0=', + }, ], }); expect(resp.body.saved_objects[0].migrationVersion).to.be.ok();