diff --git a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/internals/collect_multi_namespace_references.test.ts b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/internals/collect_multi_namespace_references.test.ts index 5450f0c739ce3..a82c18d42876b 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/internals/collect_multi_namespace_references.test.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/internals/collect_multi_namespace_references.test.ts @@ -411,11 +411,12 @@ describe('collectMultiNamespaceReferences', () => { 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 }, + { type: obj1.type, id: obj1.id }, + { type: obj2.type, id: obj2.id, origin: obj2.originId }, + { type: obj3.type, id: obj3.id }, ], - ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE + ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE, + undefined ); expect(result.objects).toEqual([ // Note: in a realistic scenario, `spacesWithMatchingOrigins` would be a superset of `spaces`. But for the purposes of this unit @@ -441,8 +442,9 @@ describe('collectMultiNamespaceReferences', () => { expect(mockFindSharedOriginObjects).toHaveBeenCalledTimes(1); expect(mockFindSharedOriginObjects).toHaveBeenCalledWith( expect.anything(), - [{ type: obj1.type, origin: obj1.id }], - ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE + [{ type: obj1.type, id: obj1.id }], + ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE, + undefined ); }); @@ -458,6 +460,25 @@ describe('collectMultiNamespaceReferences', () => { 'Failed to retrieve shared origin objects: Oh no!' ); }); + + it('passes options to findSharedOriginObjects', 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, + options: { purpose: 'updateObjectsSpaces' }, + }); + expect(mockFindSharedOriginObjects).toHaveBeenCalledTimes(1); + expect(mockFindSharedOriginObjects).toHaveBeenCalledWith( + expect.anything(), + [{ type: obj1.type, id: obj1.id }], + ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE, + 'updateObjectsSpaces' + ); + }); }); describe('with security enabled', () => { diff --git a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/internals/collect_multi_namespace_references.ts b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/internals/collect_multi_namespace_references.ts index 87254d1608515..ab11f76d9e967 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/internals/collect_multi_namespace_references.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/internals/collect_multi_namespace_references.ts @@ -108,12 +108,14 @@ export async function collectMultiNamespaceReferences( ); const objectOriginsToSearchFor = foundObjects.map(({ type, id, originId }) => ({ type, - origin: originId || id, + id, + origin: originId, })); const originsMap = await findSharedOriginObjects( createPointInTimeFinder, objectOriginsToSearchFor, - ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE + ALIAS_OR_SHARED_ORIGIN_SEARCH_PER_PAGE, + options?.purpose ); const results = objectsWithContext.map((obj) => { const aliasesVal = aliasesMap.get(getObjectKey(obj)); diff --git a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/utils/find_shared_origin_objects.test.ts b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/utils/find_shared_origin_objects.test.ts index c9f90073da24f..1622d953d7a36 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/utils/find_shared_origin_objects.test.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/utils/find_shared_origin_objects.test.ts @@ -59,10 +59,10 @@ describe('findSharedOriginObjects', () => { }); } - const obj1 = { type: 'type-1', origin: 'id-1' }; - const obj2 = { type: 'type-2', origin: 'id-2' }; - const obj3 = { type: 'type-3', origin: 'id-3' }; - const obj4 = { type: 'type-4', origin: 'id-4' }; + const obj1 = { type: 'type-1', id: 'id-1', origin: 'origin-1' }; + const obj2 = { type: 'type-2', id: 'id-2', origin: 'origin-2' }; + const obj3 = { type: 'type-3', id: 'id-3', origin: 'origin-3' }; + const obj4 = { type: 'type-4', id: 'id-4', origin: 'origin-4' }; it('uses the PointInTimeFinder to search for legacy URL aliases', async () => { mockFindResults( @@ -156,4 +156,220 @@ describe('findSharedOriginObjects', () => { expect(pointInTimeFinder.find).toHaveBeenCalledTimes(1); expect(pointInTimeFinder.close).toHaveBeenCalledTimes(2); }); + + describe(`when options.purpose is 'updateObjectsSpaces'`, () => { + it('calls createPointInTimeFinder with filter to ignore direct ID matches', async () => { + const objects = [obj1, obj2, obj3]; + await findSharedOriginObjects(createPointInTimeFinder, objects, 999, 'updateObjectsSpaces'); + expect(createPointInTimeFinder).toHaveBeenCalledTimes(1); + expect(createPointInTimeFinder).toHaveBeenCalledWith( + expect.objectContaining({ + filter: expect.objectContaining({ + arguments: expect.arrayContaining([ + expect.objectContaining({ + arguments: expect.arrayContaining([ + expect.objectContaining({ + arguments: [ + { + arguments: [ + { + isQuoted: false, + type: 'literal', + value: 'type-1.id', + }, + { + isQuoted: false, + type: 'literal', + value: 'type-1:id-1', + }, + ], + function: 'is', + type: 'function', + }, + ], + function: 'not', + type: 'function', + }), + ]), + }), + expect.objectContaining({ + arguments: expect.arrayContaining([ + expect.objectContaining({ + arguments: [ + { + arguments: [ + { + isQuoted: false, + type: 'literal', + value: 'type-2.id', + }, + { + isQuoted: false, + type: 'literal', + value: 'type-2:id-2', + }, + ], + function: 'is', + type: 'function', + }, + ], + function: 'not', + type: 'function', + }), + ]), + }), + expect.objectContaining({ + arguments: expect.arrayContaining([ + expect.objectContaining({ + arguments: [ + { + arguments: [ + { + isQuoted: false, + type: 'literal', + value: 'type-3.id', + }, + { + isQuoted: false, + type: 'literal', + value: 'type-3:id-3', + }, + ], + function: 'is', + type: 'function', + }, + ], + function: 'not', + type: 'function', + }), + ]), + }), + ]), + }), + }), + undefined, + { disableExtensions: true } + ); + }); + + it('calls createPointInTimeFinder without redundant filter when object does not have an origin ID', async () => { + const objects = [obj1, { ...obj2, origin: undefined }, obj3]; + await findSharedOriginObjects(createPointInTimeFinder, objects, 999, 'updateObjectsSpaces'); + expect(createPointInTimeFinder).toHaveBeenCalledTimes(1); + expect(createPointInTimeFinder).toHaveBeenCalledWith( + expect.objectContaining({ + filter: expect.objectContaining({ + arguments: expect.arrayContaining([ + expect.objectContaining({ + arguments: expect.arrayContaining([ + expect.objectContaining({ + arguments: [ + { + arguments: [ + { + isQuoted: false, + type: 'literal', + value: 'type-1.id', + }, + { + isQuoted: false, + type: 'literal', + value: 'type-1:origin-1', + }, + ], + function: 'is', + type: 'function', + }, + { + arguments: [ + { + isQuoted: false, + type: 'literal', + value: 'type-1.originId', + }, + { + isQuoted: false, + type: 'literal', + value: 'origin-1', + }, + ], + function: 'is', + type: 'function', + }, + ], + function: 'or', + type: 'function', + }), + ]), + }), + expect.objectContaining({ + arguments: expect.arrayContaining([ + expect.objectContaining({ + arguments: [ + { + isQuoted: false, + type: 'literal', + value: 'type-2.originId', + }, + { + isQuoted: false, + type: 'literal', + value: 'id-2', + }, + ], + function: 'is', + type: 'function', + }), + ]), + }), + expect.objectContaining({ + arguments: expect.arrayContaining([ + expect.objectContaining({ + arguments: [ + { + arguments: [ + { + isQuoted: false, + type: 'literal', + value: 'type-3.id', + }, + { + isQuoted: false, + type: 'literal', + value: 'type-3:origin-3', + }, + ], + function: 'is', + type: 'function', + }, + { + arguments: [ + { + isQuoted: false, + type: 'literal', + value: 'type-3.originId', + }, + { + isQuoted: false, + type: 'literal', + value: 'origin-3', + }, + ], + function: 'is', + type: 'function', + }, + ], + function: 'or', + type: 'function', + }), + ]), + }), + ]), + }), + }), + undefined, + { disableExtensions: true } + ); + }); + }); }); diff --git a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/utils/find_shared_origin_objects.ts b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/utils/find_shared_origin_objects.ts index 8ed1f0a3c965a..bdf1a11375763 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/utils/find_shared_origin_objects.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/utils/find_shared_origin_objects.ts @@ -9,13 +9,22 @@ import * as esKuery from '@kbn/es-query'; import { ALL_NAMESPACES_STRING } from '@kbn/core-saved-objects-utils-server'; import { getObjectKey } from '@kbn/core-saved-objects-base-server-internal'; +import { SavedObjectsCollectMultiNamespaceReferencesPurpose } from '@kbn/core-saved-objects-api-server/src/apis'; +import { + KQL_FUNCTION_AND, + KQL_FUNCTION_IS, + KQL_FUNCTION_NOT, + KQL_FUNCTION_OR, +} from '@kbn/es-query/src/kuery/functions'; import type { CreatePointInTimeFinderFn } from '../../point_in_time_finder'; interface ObjectOrigin { /** The object's type. */ type: string; - /** The object's origin is its `originId` field, or its `id` field if that is unavailable. */ - origin: string; + /** The object's ID. */ + id: string; + /** The object's origin is its `originId` field */ + origin: string | undefined; } /** @@ -26,14 +35,15 @@ interface ObjectOrigin { export async function findSharedOriginObjects( createPointInTimeFinder: CreatePointInTimeFinderFn, objects: ObjectOrigin[], - perPage?: number + perPage?: number, + purpose?: SavedObjectsCollectMultiNamespaceReferencesPurpose ) { if (!objects.length) { return new Map>(); } const uniqueObjectTypes = objects.reduce((acc, { type }) => acc.add(type), new Set()); - const filter = createAliasKueryFilter(objects); + const filter = createOriginKueryFilter(objects, purpose); const finder = createPointInTimeFinder( { type: [...uniqueObjectTypes], @@ -80,17 +90,54 @@ export async function findSharedOriginObjects( return objectsMap; } -function createAliasKueryFilter(objects: Array<{ type: string; origin: string }>) { +function createOriginKueryFilter( + objects: ObjectOrigin[], + purpose?: SavedObjectsCollectMultiNamespaceReferencesPurpose +) { const { buildNode } = esKuery.nodeTypes.function; // Note: these nodes include '.attributes' for type-level fields because these are eventually passed to `validateConvertFilterToKueryNode`, which requires it const kueryNodes = objects - .reduce((acc, { type, origin }) => { + .reduce((acc, { type, id, origin }) => { // Escape Kuery values to prevent parsing errors and unintended behavior (object types/IDs can contain KQL special characters/operators) - const match1 = buildNode('is', `${type}.id`, esKuery.escapeKuery(`${type}:${origin}`)); // here we are looking for the raw document `_id` field, which has a `type:` prefix - const match2 = buildNode('is', `${type}.originId`, esKuery.escapeKuery(origin)); // here we are looking for the saved object's `originId` field, which does not have a `type:` prefix - acc.push([match1, match2]); + + // Look for objects with an ID that matches the origin or ID (has a `type:` prefix) + const idMatchesOrigin = buildNode( + KQL_FUNCTION_IS, + `${type}.id`, + esKuery.escapeKuery(`${type}:${origin || id}`) + ); + + // Look for objects with an `originId` that matches the origin or ID (does not have a `type:` prefix) + const originMatch = buildNode( + KQL_FUNCTION_IS, + `${type}.originId`, + esKuery.escapeKuery(origin || id) + ); + + // If we are updating an object's spaces (as opposed to copying)... + if (purpose === 'updateObjectsSpaces') { + // we never want to match on the raw document `_id` fields. + // If they are equal, this just means that the object already exists in that space and it's ok. + const notIdMatch = buildNode( + KQL_FUNCTION_NOT, + buildNode(KQL_FUNCTION_IS, `${type}.id`, esKuery.escapeKuery(`${type}:${id}`)) + ); + + // If this object has an origin ID, then we do still want to match if another object's ID matches the + // object's origin (idMatchesOrigin), or if another object's origin matches the object's origin (originMatch). + // But if this object does not have an origin ID, we can skip the idMatchesOrigin part altogether + // and just check if another object's origin ID matches this object's ID (originMatch). + // (maybe slightly more efficient?) + acc.push( + buildNode(KQL_FUNCTION_AND, [ + notIdMatch, + origin ? buildNode(KQL_FUNCTION_OR, [idMatchesOrigin, originMatch]) : originMatch, + ]) + ); + } else acc.push([idMatchesOrigin, originMatch]); // If we are copying, things are much simpler + return acc; }, []) .flat(); - return buildNode('or', kueryNodes); + return buildNode(KQL_FUNCTION_OR, kueryNodes); } diff --git a/packages/core/saved-objects/core-saved-objects-api-server/src/apis/collect_multinamespace_references.ts b/packages/core/saved-objects/core-saved-objects-api-server/src/apis/collect_multinamespace_references.ts index fcd0d079961bd..f13d2f7261329 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server/src/apis/collect_multinamespace_references.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server/src/apis/collect_multinamespace_references.ts @@ -24,6 +24,13 @@ export interface SavedObjectsCollectMultiNamespaceReferencesObject { type: string; } +/** + * Purpose for collecting references. + */ +export type SavedObjectsCollectMultiNamespaceReferencesPurpose = + | 'collectMultiNamespaceReferences' + | 'updateObjectsSpaces'; + /** * Options for collecting references. * @@ -32,7 +39,7 @@ export interface SavedObjectsCollectMultiNamespaceReferencesObject { export interface SavedObjectsCollectMultiNamespaceReferencesOptions extends SavedObjectsBaseOptions { /** Optional purpose used to determine filtering and authorization checks; default is 'collectMultiNamespaceReferences' */ - purpose?: 'collectMultiNamespaceReferences' | 'updateObjectsSpaces'; + purpose?: SavedObjectsCollectMultiNamespaceReferencesPurpose; } /** diff --git a/packages/core/saved-objects/core-saved-objects-api-server/src/apis/index.ts b/packages/core/saved-objects/core-saved-objects-api-server/src/apis/index.ts index ab3c3ca12f572..d343e1472b86d 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server/src/apis/index.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server/src/apis/index.ts @@ -35,6 +35,7 @@ export type { SavedObjectReferenceWithContext, SavedObjectsCollectMultiNamespaceReferencesResponse, SavedObjectsCollectMultiNamespaceReferencesOptions, + SavedObjectsCollectMultiNamespaceReferencesPurpose, } from './collect_multinamespace_references'; export type { SavedObjectsCreateOptions } from './create'; export type { diff --git a/x-pack/test/spaces_api_integration/common/suites/get_shareable_references.ts b/x-pack/test/spaces_api_integration/common/suites/get_shareable_references.ts index fb6c22a761f1e..bedc8a52409b6 100644 --- a/x-pack/test/spaces_api_integration/common/suites/get_shareable_references.ts +++ b/x-pack/test/spaces_api_integration/common/suites/get_shareable_references.ts @@ -51,7 +51,7 @@ export const EXPECTED_RESULTS: Record { ...TEST_CASE_OBJECTS.SHAREABLE_TYPE, spaces: [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID], - spacesWithMatchingOrigins: [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID], + // No matching origins because there are no copies of the object in another space (we no longer consider a raw ID match to be an origin match) inboundReferences: [{ type: 'sharedtype', id: CASES.DEFAULT_ONLY.id, name: 'refname' }], // only reflects inbound reference that exist in the default space }, { @@ -65,7 +65,7 @@ export const EXPECTED_RESULTS: Record type: 'sharedtype', id: CASES.DEFAULT_ONLY.id, spaces: [DEFAULT_SPACE_ID], - spacesWithMatchingOrigins: [DEFAULT_SPACE_ID], // The first test assertion for spacesWithMatchingOrigins is an object that doesn't have any matching origins in other spaces + // No matching origins because there are no copies of the object in another space (we no longer consider a raw ID match to be an origin match) inboundReferences: [{ ...TEST_CASE_OBJECTS.SHAREABLE_TYPE, name: 'refname' }], }, { @@ -86,7 +86,7 @@ export const EXPECTED_RESULTS: Record type: 'sharedtype', id: CASES.ALL_SPACES.id, spaces: ['*'], - spacesWithMatchingOrigins: ['*'], + // No matching origins because there are no copies of the object in another space (we no longer consider a raw ID match to be an origin match) inboundReferences: [{ ...TEST_CASE_OBJECTS.SHAREABLE_TYPE, name: 'refname' }], }, ], @@ -94,7 +94,7 @@ export const EXPECTED_RESULTS: Record { ...TEST_CASE_OBJECTS.SHAREABLE_TYPE, spaces: [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID], - spacesWithMatchingOrigins: [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID], + // No matching origins because there are no copies of the object in another space (we no longer consider a raw ID match to be an origin match) inboundReferences: [{ type: 'sharedtype', id: CASES.SPACE_1_ONLY.id, name: 'refname' }], // only reflects inbound reference that exist in space 1 }, { @@ -116,7 +116,7 @@ export const EXPECTED_RESULTS: Record id: CASES.SPACE_1_ONLY.id, spaces: [SPACE_1_ID], spacesWithMatchingAliases: [DEFAULT_SPACE_ID, SPACE_2_ID], // aliases with a matching targetType and sourceId exist in two other spaces - spacesWithMatchingOrigins: ['other_space', SPACE_1_ID], // The second test assertion for spacesWithMatchingOrigins is an object that has a matching origin in one other space + spacesWithMatchingOrigins: ['other_space'], // The second test assertion for spacesWithMatchingOrigins is an object that has a matching origin in one other space inboundReferences: [{ ...TEST_CASE_OBJECTS.SHAREABLE_TYPE, name: 'refname' }], }, { @@ -130,7 +130,7 @@ export const EXPECTED_RESULTS: Record type: 'sharedtype', id: CASES.ALL_SPACES.id, spaces: ['*'], - spacesWithMatchingOrigins: ['*'], + // No matching origins because there are no copies of the object in another space (we no longer consider a raw ID match to be an origin match) inboundReferences: [{ ...TEST_CASE_OBJECTS.SHAREABLE_TYPE, name: 'refname' }], }, ], @@ -138,7 +138,7 @@ export const EXPECTED_RESULTS: Record { ...TEST_CASE_OBJECTS.SHAREABLE_TYPE, spaces: [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID], - spacesWithMatchingOrigins: [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID], + // No matching origins because there are no copies of the object in another space (we no longer consider a raw ID match to be an origin match) inboundReferences: [{ type: 'sharedtype', id: CASES.SPACE_2_ONLY.id, name: 'refname' }], // only reflects inbound reference that exist in space 2 }, { @@ -173,7 +173,7 @@ export const EXPECTED_RESULTS: Record type: 'sharedtype', id: CASES.ALL_SPACES.id, spaces: ['*'], - spacesWithMatchingOrigins: ['*'], + // No matching origins because there are no copies of the object in another space (we no longer consider a raw ID match to be an origin match) inboundReferences: [{ ...TEST_CASE_OBJECTS.SHAREABLE_TYPE, name: 'refname' }], }, ],