Improve SOR.find reference filter implementation (#121042) (#121177)

Co-authored-by: Pierre Gayvallet <pierre.gayvallet@elastic.co>

src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts

@@ -6,12 +6,15 @@
  * Side Public License, v 1.
  */
 
+import { getReferencesFilterMock } from './query_params.tests.mocks';
+
 import * as esKuery from '@kbn/es-query';
+
 type KueryNode = any;
 
 import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils';
 import { SavedObjectTypeRegistry } from '../../../saved_objects_type_registry';
-import { getQueryParams, getClauseForReference } from './query_params';
+import { getQueryParams } from './query_params';
 
 const registerTypes = (registry: SavedObjectTypeRegistry) => {
   registry.registerType({
@@ -85,6 +88,12 @@ describe('#getQueryParams', () => {
   beforeEach(() => {
     registry = new SavedObjectTypeRegistry();
     registerTypes(registry);
+
+    getReferencesFilterMock.mockReturnValue({ references_filter: true });
+  });
+
+  afterEach(() => {
+    getReferencesFilterMock.mockClear();
   });
 
   const createTypeClause = (type: string, namespaces?: string[]) => {
@@ -185,102 +194,42 @@ describe('#getQueryParams', () => {
 
   describe('reference filter clause', () => {
     describe('`hasReference` parameter', () => {
-      const getReferencesFilter = (result: any) => {
-        const filters = result.query.bool.filter;
-        return filters.find((filter: any) => {
-          const clauses = filter.bool?.must ?? filter.bool?.should;
-          if (!clauses) {
-            return false;
-          }
-          return clauses[0].nested?.path === 'references' ?? false;
-        });
-      };
-
-      it('does not include the clause when `hasReference` is not specified', () => {
-        const result = getQueryParams({
+      it('does not call `getReferencesFilter` when `hasReference` is not specified', () => {
+        getQueryParams({
           registry,
           hasReference: undefined,
         });
 
-        expect(getReferencesFilter(result)).toBeUndefined();
-      });
-
-      it('creates a should clause for specified reference when operator is `OR`', () => {
-        const hasReference = { id: 'foo', type: 'bar' };
-        const result = getQueryParams({
-          registry,
-          hasReference,
-          hasReferenceOperator: 'OR',
-        });
-        expect(getReferencesFilter(result)).toEqual({
-          bool: {
-            should: [getClauseForReference(hasReference)],
-            minimum_should_match: 1,
-          },
-        });
+        expect(getReferencesFilterMock).not.toHaveBeenCalled();
       });
 
-      it('creates a must clause for specified reference when operator is `AND`', () => {
+      it('calls `getReferencesFilter` with the correct parameters', () => {
         const hasReference = { id: 'foo', type: 'bar' };
-        const result = getQueryParams({
+        getQueryParams({
           registry,
           hasReference,
           hasReferenceOperator: 'AND',
         });
-        expect(getReferencesFilter(result)).toEqual({
-          bool: {
-            must: [getClauseForReference(hasReference)],
-          },
-        });
-      });
 
-      it('handles multiple references when operator is `OR`', () => {
-        const hasReference = [
-          { id: 'foo', type: 'bar' },
-          { id: 'hello', type: 'dolly' },
-        ];
-        const result = getQueryParams({
-          registry,
-          hasReference,
-          hasReferenceOperator: 'OR',
-        });
-        expect(getReferencesFilter(result)).toEqual({
-          bool: {
-            should: hasReference.map(getClauseForReference),
-            minimum_should_match: 1,
-          },
+        expect(getReferencesFilterMock).toHaveBeenCalledTimes(1);
+        expect(getReferencesFilterMock).toHaveBeenCalledWith({
+          references: [hasReference],
+          operator: 'AND',
         });
       });
 
-      it('handles multiple references when operator is `AND`', () => {
-        const hasReference = [
-          { id: 'foo', type: 'bar' },
-          { id: 'hello', type: 'dolly' },
-        ];
-        const result = getQueryParams({
-          registry,
-          hasReference,
-          hasReferenceOperator: 'AND',
-        });
-        expect(getReferencesFilter(result)).toEqual({
-          bool: {
-            must: hasReference.map(getClauseForReference),
-          },
-        });
-      });
+      it('includes the return of `getReferencesFilter` in the `filter` clause', () => {
+        getReferencesFilterMock.mockReturnValue({ references_filter: true });
 
-      it('defaults to `OR` when operator is not specified', () => {
         const hasReference = { id: 'foo', type: 'bar' };
         const result = getQueryParams({
           registry,
           hasReference,
+          hasReferenceOperator: 'AND',
         });
-        expect(getReferencesFilter(result)).toEqual({
-          bool: {
-            should: [getClauseForReference(hasReference)],
-            minimum_should_match: 1,
-          },
-        });
+
+        const filters: any[] = result.query.bool.filter;
+        expect(filters.some((filter) => filter.references_filter === true)).toBeDefined();
       });
     });
   });

src/core/server/saved_objects/service/lib/search_dsl/query_params.tests.mocks.ts

@@ -0,0 +1,13 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+export const getReferencesFilterMock = jest.fn();
+
+jest.doMock('./references_filter', () => ({
+  getReferencesFilter: getReferencesFilterMock,
+}));

src/core/server/saved_objects/service/lib/search_dsl/query_params.ts

@@ -7,10 +7,12 @@
  */
 
 import * as esKuery from '@kbn/es-query';
+
 type KueryNode = any;
 
 import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry';
 import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils';
+import { getReferencesFilter } from './references_filter';
 
 /**
  * Gets the types based on the type. Uses mappings to support
@@ -139,50 +141,6 @@ interface QueryParams {
   kueryNode?: KueryNode;
 }
 
-function getReferencesFilter(
-  references: HasReferenceQueryParams[],
-  operator: SearchOperator = 'OR'
-) {
-  if (operator === 'AND') {
-    return {
-      bool: {
-        must: references.map(getClauseForReference),
-      },
-    };
-  } else {
-    return {
-      bool: {
-        should: references.map(getClauseForReference),
-        minimum_should_match: 1,
-      },
-    };
-  }
-}
-
-export function getClauseForReference(reference: HasReferenceQueryParams) {
-  return {
-    nested: {
-      path: 'references',
-      query: {
-        bool: {
-          must: [
-            {
-              term: {
-                'references.id': reference.id,
-              },
-            },
-            {
-              term: {
-                'references.type': reference.type,
-              },
-            },
-          ],
-        },
-      },
-    },
-  };
-}
-
 // A de-duplicated set of namespaces makes for a more efficient query.
 const uniqNamespaces = (namespacesToNormalize?: string[]) =>
   namespacesToNormalize ? Array.from(new Set(namespacesToNormalize)) : undefined;
@@ -215,7 +173,14 @@ export function getQueryParams({
   const bool: any = {
     filter: [
       ...(kueryNode != null ? [esKuery.toElasticsearchQuery(kueryNode)] : []),
-      ...(hasReference?.length ? [getReferencesFilter(hasReference, hasReferenceOperator)] : []),
+      ...(hasReference?.length
+        ? [
+            getReferencesFilter({
+              references: hasReference,
+              operator: hasReferenceOperator,
+            }),
+          ]
+        : []),
       {
         bool: {
           should: types.map((shouldType) => {