Change updateObjectsSpaces API to prevent multiple objects w/ same or…

…igin (#128269)
elastic · Apr 19, 2022 · cde4288 · cde4288
1 parent 5872b5e
commit cde4288
Show file tree

Hide file tree

Showing 25 changed files with 659 additions and 54 deletions.
diff --git a/...opment/core/public/kibana-plugin-core-public.savedobjectreferencewithcontext.md b/...opment/core/public/kibana-plugin-core-public.savedobjectreferencewithcontext.md
@@ -19,7 +19,9 @@ export interface SavedObjectReferenceWithContext
 |  [id](./kibana-plugin-core-public.savedobjectreferencewithcontext.id.md) | string | The ID of the referenced object |
 |  [inboundReferences](./kibana-plugin-core-public.savedobjectreferencewithcontext.inboundreferences.md) | Array&lt;{ type: string; id: string; name: string; }&gt; | References to this object; note that this does not contain \_all inbound references everywhere for this object\_, it only contains inbound references for the scope of this operation |
 |  [isMissing?](./kibana-plugin-core-public.savedobjectreferencewithcontext.ismissing.md) | boolean | <i>(Optional)</i> Whether or not this object or reference is missing |
+|  [originId?](./kibana-plugin-core-public.savedobjectreferencewithcontext.originid.md) | string | <i>(Optional)</i> The origin ID of the referenced object (if it has one) |
 |  [spaces](./kibana-plugin-core-public.savedobjectreferencewithcontext.spaces.md) | string\[\] | The space(s) that the referenced object exists in |
 |  [spacesWithMatchingAliases?](./kibana-plugin-core-public.savedobjectreferencewithcontext.spaceswithmatchingaliases.md) | string\[\] | <i>(Optional)</i> The space(s) that legacy URL aliases matching this type/id exist in |
+|  [spacesWithMatchingOrigins?](./kibana-plugin-core-public.savedobjectreferencewithcontext.spaceswithmatchingorigins.md) | string\[\] | <i>(Optional)</i> The space(s) that objects matching this origin exist in (including this one) |
 |  [type](./kibana-plugin-core-public.savedobjectreferencewithcontext.type.md) | string | The type of the referenced object |
 
diff --git a/...re/public/kibana-plugin-core-public.savedobjectreferencewithcontext.originid.md b/...re/public/kibana-plugin-core-public.savedobjectreferencewithcontext.originid.md
@@ -0,0 +1,13 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-core-public](./kibana-plugin-core-public.md) &gt; [SavedObjectReferenceWithContext](./kibana-plugin-core-public.savedobjectreferencewithcontext.md) &gt; [originId](./kibana-plugin-core-public.savedobjectreferencewithcontext.originid.md)
+
+## SavedObjectReferenceWithContext.originId property
+
+The origin ID of the referenced object (if it has one)
+
+<b>Signature:</b>
+
+```typescript
+originId?: string;
+```
diff --git a/...plugin-core-public.savedobjectreferencewithcontext.spaceswithmatchingorigins.md b/...plugin-core-public.savedobjectreferencewithcontext.spaceswithmatchingorigins.md
@@ -0,0 +1,13 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-core-public](./kibana-plugin-core-public.md) &gt; [SavedObjectReferenceWithContext](./kibana-plugin-core-public.savedobjectreferencewithcontext.md) &gt; [spacesWithMatchingOrigins](./kibana-plugin-core-public.savedobjectreferencewithcontext.spaceswithmatchingorigins.md)
+
+## SavedObjectReferenceWithContext.spacesWithMatchingOrigins property
+
+The space(s) that objects matching this origin exist in (including this one)
+
+<b>Signature:</b>
+
+```typescript
+spacesWithMatchingOrigins?: string[];
+```
diff --git a/...opment/core/server/kibana-plugin-core-server.savedobjectreferencewithcontext.md b/...opment/core/server/kibana-plugin-core-server.savedobjectreferencewithcontext.md
@@ -19,7 +19,9 @@ export interface SavedObjectReferenceWithContext
 |  [id](./kibana-plugin-core-server.savedobjectreferencewithcontext.id.md) | string | The ID of the referenced object |
 |  [inboundReferences](./kibana-plugin-core-server.savedobjectreferencewithcontext.inboundreferences.md) | Array&lt;{ type: string; id: string; name: string; }&gt; | References to this object; note that this does not contain \_all inbound references everywhere for this object\_, it only contains inbound references for the scope of this operation |
 |  [isMissing?](./kibana-plugin-core-server.savedobjectreferencewithcontext.ismissing.md) | boolean | <i>(Optional)</i> Whether or not this object or reference is missing |
+|  [originId?](./kibana-plugin-core-server.savedobjectreferencewithcontext.originid.md) | string | <i>(Optional)</i> The origin ID of the referenced object (if it has one) |
 |  [spaces](./kibana-plugin-core-server.savedobjectreferencewithcontext.spaces.md) | string\[\] | The space(s) that the referenced object exists in |
 |  [spacesWithMatchingAliases?](./kibana-plugin-core-server.savedobjectreferencewithcontext.spaceswithmatchingaliases.md) | string\[\] | <i>(Optional)</i> The space(s) that legacy URL aliases matching this type/id exist in |
+|  [spacesWithMatchingOrigins?](./kibana-plugin-core-server.savedobjectreferencewithcontext.spaceswithmatchingorigins.md) | string\[\] | <i>(Optional)</i> The space(s) that objects matching this origin exist in (including this one) |
 |  [type](./kibana-plugin-core-server.savedobjectreferencewithcontext.type.md) | string | The type of the referenced object |
 
diff --git a/...re/server/kibana-plugin-core-server.savedobjectreferencewithcontext.originid.md b/...re/server/kibana-plugin-core-server.savedobjectreferencewithcontext.originid.md
@@ -0,0 +1,13 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [SavedObjectReferenceWithContext](./kibana-plugin-core-server.savedobjectreferencewithcontext.md) &gt; [originId](./kibana-plugin-core-server.savedobjectreferencewithcontext.originid.md)
+
+## SavedObjectReferenceWithContext.originId property
+
+The origin ID of the referenced object (if it has one)
+
+<b>Signature:</b>
+
+```typescript
+originId?: string;
+```
diff --git a/...plugin-core-server.savedobjectreferencewithcontext.spaceswithmatchingorigins.md b/...plugin-core-server.savedobjectreferencewithcontext.spaceswithmatchingorigins.md
@@ -0,0 +1,13 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [SavedObjectReferenceWithContext](./kibana-plugin-core-server.savedobjectreferencewithcontext.md) &gt; [spacesWithMatchingOrigins](./kibana-plugin-core-server.savedobjectreferencewithcontext.spaceswithmatchingorigins.md)
+
+## SavedObjectReferenceWithContext.spacesWithMatchingOrigins property
+
+The space(s) that objects matching this origin exist in (including this one)
+
+<b>Signature:</b>
+
+```typescript
+spacesWithMatchingOrigins?: string[];
+```
diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md
@@ -1089,8 +1089,10 @@ export interface SavedObjectReferenceWithContext {
         name: string;
     }>;
     isMissing?: boolean;
+    originId?: string;
     spaces: string[];
     spacesWithMatchingAliases?: string[];
+    spacesWithMatchingOrigins?: string[];
     type: string;
 }
 

diff --git a/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.test.mock.ts b/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.test.mock.ts
@@ -7,6 +7,7 @@
  */
 
 import type { findLegacyUrlAliases } from './legacy_url_aliases';
+import type { findSharedOriginObjects } from './find_shared_origin_objects';
 import type * as InternalUtils from './internal_utils';
 
 export const mockFindLegacyUrlAliases = jest.fn() as jest.MockedFunction<
@@ -17,6 +18,14 @@ jest.mock('./legacy_url_aliases', () => {
   return { findLegacyUrlAliases: mockFindLegacyUrlAliases };
 });
 
+export const mockFindSharedOriginObjects = jest.fn() as jest.MockedFunction<
+  typeof findSharedOriginObjects
+>;
+
+jest.mock('./find_shared_origin_objects', () => {
+  return { findSharedOriginObjects: mockFindSharedOriginObjects };
+});
+
 export const mockRawDocExistsInNamespace = jest.fn() as jest.MockedFunction<
   typeof InternalUtils['rawDocExistsInNamespace']
 >;

diff --git a/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.test.ts b/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.test.ts
@@ -8,14 +8,15 @@
 
 import {
   mockFindLegacyUrlAliases,
+  mockFindSharedOriginObjects,
   mockRawDocExistsInNamespace,
 } from './collect_multi_namespace_references.test.mock';
 
 import { elasticsearchClientMock } from '../../../elasticsearch/client/mocks';
 import { typeRegistryMock } from '../../saved_objects_type_registry.mock';
 import { SavedObjectsSerializer } from '../../serialization';
 import {
-  ALIAS_SEARCH_PER_PAGE,
+  ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE,
   CollectMultiNamespaceReferencesParams,
   SavedObjectsCollectMultiNamespaceReferencesObject,
   SavedObjectsCollectMultiNamespaceReferencesOptions,
@@ -35,6 +36,8 @@ const MULTI_NAMESPACE_HIDDEN_OBJ_TYPE = 'type-d';
 beforeEach(() => {
   mockFindLegacyUrlAliases.mockReset();
   mockFindLegacyUrlAliases.mockResolvedValue(new Map()); // return an empty map by default
+  mockFindSharedOriginObjects.mockReset();
+  mockFindSharedOriginObjects.mockResolvedValue(new Map()); // return an empty map by default
   mockRawDocExistsInNamespace.mockReset();
   mockRawDocExistsInNamespace.mockReturnValue(true); // return true by default
 });
@@ -82,6 +85,7 @@ describe('collectMultiNamespaceReferences', () => {
   function mockMgetResults(
     ...results: Array<{
       found: boolean;
+      originId?: string;
       references?: Array<{ type: string; id: string }>;
     }>
   ) {
@@ -95,6 +99,7 @@ describe('collectMultiNamespaceReferences', () => {
               _index: 'doesnt-matter',
               _source: {
                 namespaces: SPACES,
+                originId: x.originId,
                 references,
               },
               ...VERSION_PROPS,
@@ -321,7 +326,7 @@ describe('collectMultiNamespaceReferences', () => {
       expect(mockFindLegacyUrlAliases).toHaveBeenCalledWith(
         expect.anything(),
         [obj1, obj2, obj3],
-        ALIAS_SEARCH_PER_PAGE
+        ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE
       );
       expect(result.objects).toEqual([
         {
@@ -346,7 +351,7 @@ describe('collectMultiNamespaceReferences', () => {
       expect(mockFindLegacyUrlAliases).toHaveBeenCalledWith(
         expect.anything(),
         [obj1],
-        ALIAS_SEARCH_PER_PAGE
+        ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE
       );
     });
 
@@ -363,4 +368,81 @@ describe('collectMultiNamespaceReferences', () => {
       );
     });
   });
+
+  describe('shared origins', () => {
+    it('uses findSharedOriginObjects to search for objects with shared origins', async () => {
+      const obj1 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-1' };
+      const obj2 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-x', originId: 'id-2' };
+      const obj3 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-3' };
+      const params = setup([obj1, obj2], {});
+      mockMgetResults(
+        // results for obj1 and obj2
+        { found: true, references: [obj3] },
+        { found: true, originId: obj2.originId, references: [] }
+      );
+      mockMgetResults({ found: true, references: [] }); // results for obj3
+      mockFindSharedOriginObjects.mockResolvedValue(
+        new Map([
+          [`${obj1.type}:${obj1.id}`, new Set(['space-1'])],
+          [`${obj2.type}:${obj2.originId}`, new Set(['*'])],
+          [`${obj3.type}:${obj3.id}`, new Set(['space-1', 'space-2'])],
+        ])
+      );
+
+      const result = await collectMultiNamespaceReferences(params);
+      expect(client.mget).toHaveBeenCalledTimes(2);
+      expectMgetArgs(1, obj1, obj2);
+      expectMgetArgs(2, obj3); // obj3 is retrieved in a second cluster call
+      expect(mockFindSharedOriginObjects).toHaveBeenCalledTimes(1);
+      expect(mockFindSharedOriginObjects).toHaveBeenCalledWith(
+        expect.anything(),
+        [
+          { type: obj1.type, origin: obj1.id },
+          { type: obj2.type, origin: obj2.originId }, // If the found object has an `originId`, that is used instead of the object's `id`.
+          { type: obj3.type, origin: obj3.id },
+        ],
+        ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE
+      );
+      expect(result.objects).toEqual([
+        // Note: in a realistic scenario, `spacesWithMatchingOrigins` would be a superset of `spaces`. But for the purposes of this unit
+        // test, it doesn't matter if they are different.
+        { ...obj1, spaces: SPACES, inboundReferences: [], spacesWithMatchingOrigins: ['space-1'] },
+        { ...obj2, spaces: SPACES, inboundReferences: [], spacesWithMatchingOrigins: ['*'] },
+        {
+          ...obj3,
+          spaces: SPACES,
+          inboundReferences: [{ ...obj1, name: 'ref-name' }],
+          spacesWithMatchingOrigins: ['space-1', 'space-2'],
+        },
+      ]);
+    });
+
+    it('omits objects that have an empty spaces array (the object does not exist, or we are not sure)', async () => {
+      const obj1 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-1' };
+      const obj2 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-2' };
+      const params = setup([obj1, obj2]);
+      mockMgetResults({ found: true }, { found: false }); // results for obj1 and obj2
+
+      await collectMultiNamespaceReferences(params);
+      expect(mockFindSharedOriginObjects).toHaveBeenCalledTimes(1);
+      expect(mockFindSharedOriginObjects).toHaveBeenCalledWith(
+        expect.anything(),
+        [{ type: obj1.type, origin: obj1.id }],
+        ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE
+      );
+    });
+
+    it('handles findSharedOriginObjects errors', async () => {
+      const obj1 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-1' };
+      const params = setup([obj1]);
+      mockMgetResults({ found: true }); // results for obj1
+      mockFindSharedOriginObjects.mockRejectedValue(
+        new Error('Failed to retrieve shared origin objects: Oh no!')
+      );
+
+      await expect(() => collectMultiNamespaceReferences(params)).rejects.toThrow(
+        'Failed to retrieve shared origin objects: Oh no!'
+      );
+    });
+  });
 });
diff --git a/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.ts b/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.ts
@@ -21,20 +21,21 @@ import {
 } from './internal_utils';
 import type { CreatePointInTimeFinderFn } from './point_in_time_finder';
 import type { RepositoryEsClient } from './repository_es_client';
+import { findSharedOriginObjects } from './find_shared_origin_objects';
 
 /**
  * When we collect an object's outbound references, we will only go a maximum of this many levels deep before we throw an error.
  */
 const MAX_REFERENCE_GRAPH_DEPTH = 20;
 
 /**
- * How many aliases to search for per page. This is smaller than the PointInTimeFinder's default of 1000. We specify 100 for the page count
- * because this is a relatively unimportant operation, and we want to avoid blocking the Elasticsearch thread pool for longer than
- * necessary.
+ * How many aliases or objects with shared origins to search for per page. This is smaller than the PointInTimeFinder's default of 1000. We
+ * specify 100 for the page count because this is a relatively unimportant operation, and we want to avoid blocking the Elasticsearch thread
+ * pool for longer than necessary.
  *
  * @internal
  */
-export const ALIAS_SEARCH_PER_PAGE = 100;
+export const ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE = 100;
 
 /**
  * An object to collect references for. It must be a multi-namespace type (in other words, the object type must be registered with the
@@ -71,6 +72,8 @@ export interface SavedObjectReferenceWithContext {
   type: string;
   /** The ID of the referenced object */
   id: string;
+  /** The origin ID of the referenced object (if it has one) */
+  originId?: string;
   /** The space(s) that the referenced object exists in */
   spaces: string[];
   /**
@@ -89,6 +92,8 @@ export interface SavedObjectReferenceWithContext {
   isMissing?: boolean;
   /** The space(s) that legacy URL aliases matching this type/id exist in */
   spacesWithMatchingAliases?: string[];
+  /** The space(s) that objects matching this origin exist in (including this one) */
+  spacesWithMatchingOrigins?: string[];
 }
 
 /**
@@ -140,8 +145,16 @@ export async function collectMultiNamespaceReferences(
     });
     const { type, id } = parseObjectKey(referenceKey);
     const object = objectMap.get(referenceKey);
+    const originId = object?.originId;
     const spaces = object?.namespaces ?? [];
-    return { type, id, spaces, inboundReferences, ...(object === null && { isMissing: true }) };
+    return {
+      type,
+      id,
+      originId,
+      spaces,
+      inboundReferences,
+      ...(object === null && { isMissing: true }),
+    };
   });
 
   const objectsToFindAliasesFor = objectsWithContext
@@ -150,13 +163,22 @@ export async function collectMultiNamespaceReferences(
   const aliasesMap = await findLegacyUrlAliases(
     createPointInTimeFinder,
     objectsToFindAliasesFor,
-    ALIAS_SEARCH_PER_PAGE
+    ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE
+  );
+  const objectOriginsToSearchFor = objectsWithContext
+    .filter(({ spaces }) => spaces.length !== 0)
+    .map(({ type, id, originId }) => ({ type, origin: originId || id }));
+  const originsMap = await findSharedOriginObjects(
+    createPointInTimeFinder,
+    objectOriginsToSearchFor,
+    ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE
   );
   const results = objectsWithContext.map((obj) => {
-    const key = getObjectKey(obj);
-    const val = aliasesMap.get(key);
-    const spacesWithMatchingAliases = val && Array.from(val);
-    return { ...obj, spacesWithMatchingAliases };
+    const aliasesVal = aliasesMap.get(getObjectKey(obj));
+    const spacesWithMatchingAliases = aliasesVal && Array.from(aliasesVal).sort();
+    const originsVal = originsMap.get(getObjectKey({ type: obj.type, id: obj.originId || obj.id }));
+    const spacesWithMatchingOrigins = originsVal && Array.from(originsVal).sort();
+    return { ...obj, spacesWithMatchingAliases, spacesWithMatchingOrigins };
   });
 
   return {