Change saved object updateObjectsSpaces API to use deleteLegacyUrlAli…

…ases

x-pack/test/spaces_api_integration/common/fixtures/es_archiver/saved_objects/spaces/data.json

@@ -582,6 +582,94 @@
   }
 }
 
+{
+  "type": "doc",
+  "value": {
+    "id": "sharedtype:alias_delete_inclusive",
+    "index": ".kibana",
+    "source": {
+      "sharedtype": {
+        "title": "This is used to test that when an object is unshared from a space, inbound aliases for just those spaces are removed"
+      },
+      "type": "sharedtype",
+      "namespaces": ["default", "space_1", "space_2"],
+      "updated_at": "2017-09-21T18:59:16.270Z"
+    },
+    "type": "doc"
+  }
+}
+
+{
+  "type": "doc",
+  "value": {
+    "id": "legacy-url-alias:space_1:sharedtype:doesnt-matter",
+    "index": ".kibana",
+    "source": {
+      "type": "legacy-url-alias",
+      "updated_at": "2017-09-21T18:51:23.794Z",
+      "legacy-url-alias": {
+        "sourceId": "doesnt-matter",
+        "targetNamespace": "space_1",
+        "targetType": "sharedtype",
+        "targetId": "alias_delete_inclusive"
+      }
+    }
+  }
+}
+
+{
+  "type": "doc",
+  "value": {
+    "id": "legacy-url-alias:space_2:sharedtype:doesnt-matter",
+    "index": ".kibana",
+    "source": {
+      "type": "legacy-url-alias",
+      "updated_at": "2017-09-21T18:51:23.794Z",
+      "legacy-url-alias": {
+        "sourceId": "doesnt-matter",
+        "targetNamespace": "space_2",
+        "targetType": "sharedtype",
+        "targetId": "alias_delete_inclusive"
+      }
+    }
+  }
+}
+
+{
+  "type": "doc",
+  "value": {
+    "id": "sharedtype:alias_delete_exclusive",
+    "index": ".kibana",
+    "source": {
+      "sharedtype": {
+        "title": "This is used to test that when an object is unshared from all space, inbound aliases for all spaces are removed"
+      },
+      "type": "sharedtype",
+      "namespaces": ["*"],
+      "updated_at": "2017-09-21T18:59:16.270Z"
+    },
+    "type": "doc"
+  }
+}
+
+{
+  "type": "doc",
+  "value": {
+    "id": "legacy-url-alias:other_space:sharedtype:doesnt-matter",
+    "index": ".kibana",
+    "source": {
+      "type": "legacy-url-alias",
+      "updated_at": "2017-09-21T18:51:23.794Z",
+      "legacy-url-alias": {
+        "sourceId": "doesnt-matter",
+        "targetNamespace": "other_space",
+        "targetType": "sharedtype",
+        "targetId": "alias_delete_exclusive"
+      }
+    }
+  }
+}
+
 {
   "type": "doc",
   "value": {

x-pack/test/spaces_api_integration/common/lib/saved_object_test_cases.ts

@@ -38,6 +38,14 @@ export const MULTI_NAMESPACE_SAVED_OBJECT_TEST_CASES = Object.freeze({
     id: 'all_spaces',
     existingNamespaces: ['*'], // all current and future spaces
   }),
+  ALIAS_DELETE_INCLUSIVE: Object.freeze({
+    id: 'alias_delete_inclusive',
+    existingNamespaces: ['default', 'space_1', 'space_2'], // each individual space
+  }),
+  ALIAS_DELETE_EXCLUSIVE: Object.freeze({
+    id: 'alias_delete_exclusive',
+    existingNamespaces: ['*'], // all current and future spaces
+  }),
   DOES_NOT_EXIST: Object.freeze({
     id: 'does_not_exist',
     existingNamespaces: [] as string[],

x-pack/test/spaces_api_integration/common/suites/delete.ts

@@ -53,8 +53,8 @@ export function deleteTestSuiteFactory(es: Client, esArchiver: any, supertest: S
     // @ts-expect-error @elastic/elasticsearch doesn't defined `count.buckets`.
     const buckets = response.aggregations?.count.buckets;
 
-    // The test fixture contains three legacy URL aliases:
-    // (1) one for "space_1", (2) one for "space_2", and (3) one for "other_space", which is a non-existent space.
+    // The test fixture contains six legacy URL aliases:
+    // (1) two for "space_1", (2) two for "space_2", and (3) two for "other_space", which is a non-existent space.
     // Each test deletes "space_2", so the agg buckets should reflect that aliases (1) and (3) still exist afterwards.
 
     // Space 2 deleted, all others should exist
@@ -75,26 +75,26 @@ export function deleteTestSuiteFactory(es: Client, esArchiver: any, supertest: S
         },
       },
       {
-        doc_count: 6,
+        doc_count: 7,
         key: 'space_1',
         countByType: {
           doc_count_error_upper_bound: 0,
           sum_other_doc_count: 0,
           buckets: [
             { key: 'visualization', doc_count: 3 },
+            { key: 'legacy-url-alias', doc_count: 2 }, // aliases (1)
             { key: 'dashboard', doc_count: 1 },
             { key: 'index-pattern', doc_count: 1 },
-            { key: 'legacy-url-alias', doc_count: 1 }, // alias (1)
           ],
         },
       },
       {
-        doc_count: 1,
+        doc_count: 2,
         key: 'other_space',
         countByType: {
           doc_count_error_upper_bound: 0,
           sum_other_doc_count: 0,
-          buckets: [{ key: 'legacy-url-alias', doc_count: 1 }], // alias (3)
+          buckets: [{ key: 'legacy-url-alias', doc_count: 2 }], // aliases (3)
         },
       },
     ];
@@ -110,8 +110,8 @@ export function deleteTestSuiteFactory(es: Client, esArchiver: any, supertest: S
       body: { query: { terms: { type: ['sharedtype'] } } },
     });
     const docs = multiNamespaceResponse.hits.hits;
-    // Just 12 results, since spaces_2_only, conflict_1_space_2 and conflict_2_space_2 got deleted.
-    expect(docs).length(12);
+    // Just 14 results, since spaces_2_only, conflict_1_space_2 and conflict_2_space_2 got deleted.
+    expect(docs).length(14);
     docs.forEach((doc) => () => {
       const containsSpace2 = doc?._source?.namespaces.includes('space_2');
       expect(containsSpace2).to.eql(false);

x-pack/test/spaces_api_integration/common/suites/update_objects_spaces.ts

@@ -6,6 +6,8 @@
  */
 
 import expect from '@kbn/expect';
+import type { Client } from '@elastic/elasticsearch';
+import type { SearchTotalHits } from '@elastic/elasticsearch/lib/api/types';
 import { without, uniq } from 'lodash';
 import { SuperTest } from 'supertest';
 import {
@@ -35,6 +37,7 @@ export interface UpdateObjectsSpacesTestCase {
   objects: Array<{
     id: string;
     existingNamespaces: string[];
+    expectAliasDifference?: number;
     failure?: 400 | 404;
   }>;
   spacesToAdd: string[];
@@ -54,7 +57,11 @@ const getTestTitle = ({ objects, spacesToAdd, spacesToRemove }: UpdateObjectsSpa
   return `{objects: [${objStr}], spacesToAdd: [${addStr}], spacesToRemove: [${remStr}]}`;
 };
 
-export function updateObjectsSpacesTestSuiteFactory(esArchiver: any, supertest: SuperTest<any>) {
+export function updateObjectsSpacesTestSuiteFactory(
+  es: Client,
+  esArchiver: any,
+  supertest: SuperTest<any>
+) {
   const expectForbidden = expectResponses.forbiddenTypes('share_to_space');
   const expectResponseBody =
     (
@@ -68,7 +75,9 @@ export function updateObjectsSpacesTestSuiteFactory(esArchiver: any, supertest:
       } else {
         const { objects, spacesToAdd, spacesToRemove } = testCase;
         const apiResponse = response.body as SavedObjectsUpdateObjectsSpacesResponse;
-        objects.forEach(({ id, existingNamespaces, failure }, i) => {
+
+        for (let i = 0; i < objects.length; i++) {
+          const { id, existingNamespaces, expectAliasDifference, failure } = objects[i];
           const object = apiResponse.objects[i];
           if (failure === 404) {
             const error = SavedObjectsErrorHelpers.createGenericNotFoundError(TYPE, id);
@@ -84,8 +93,25 @@ export function updateObjectsSpacesTestSuiteFactory(esArchiver: any, supertest:
             expect(result.type).to.eql(TYPE);
             expect(result.id).to.eql(id);
             expect(result.spaces.sort()).to.eql(expectedSpaces.sort());
+
+            if (expectAliasDifference !== undefined) {
+              // if we deleted an object that had an alias pointing to it, the alias should have been deleted as well
+              await es.indices.refresh({ index: '.kibana' }); // alias deletion uses refresh: false, so we need to manually refresh the index before searching
+              const searchResponse = await es.search({
+                index: '.kibana',
+                body: {
+                  size: 0,
+                  query: { terms: { type: ['legacy-url-alias'] } },
+                  track_total_hits: true,
+                },
+              });
+              expect((searchResponse.hits.total as SearchTotalHits).value).to.eql(
+                // Six aliases exist in the test fixtures
+                6 + expectAliasDifference
+              );
+            }
           }
-        });
+        }
       }
     };
   const createTestDefinitions = (

x-pack/test/spaces_api_integration/security_and_spaces/apis/update_objects_spaces.ts

@@ -28,6 +28,8 @@ const { fail404 } = testCaseFailures;
 
 const createTestCases = (spaceId: string): UpdateObjectsSpacesTestCase[] => {
   const eachSpace = [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID];
+  // Note: we intentionally exclude ALIAS_DELETION test cases because they are already covered in spaces_only test suite, and there is no
+  // authZ-specific logic that affects alias deletion, all of that happens at the Saved Objects Repository level.
   return [
     // Test case to check adding and removing all spaces ("*") to a saved object
     {
@@ -125,8 +127,10 @@ const calculateSingleSpaceAuthZ = (testCases: UpdateObjectsSpacesTestCase[], spa
 export default function ({ getService }: FtrProviderContext) {
   const supertest = getService('supertestWithoutAuth');
   const esArchiver = getService('esArchiver');
+  const es = getService('es');
 
   const { addTests, createTestDefinitions } = updateObjectsSpacesTestSuiteFactory(
+    es,
     esArchiver,
     supertest
   );

x-pack/test/spaces_api_integration/spaces_only/apis/update_objects_spaces.ts

@@ -11,6 +11,7 @@ import {
   getTestScenarios,
 } from '../../../saved_object_api_integration/common/lib/saved_object_test_utils';
 import { MULTI_NAMESPACE_SAVED_OBJECT_TEST_CASES as CASES } from '../../common/lib/saved_object_test_cases';
+import type { UpdateObjectsSpacesTestCase } from '../../common/suites/update_objects_spaces';
 import { updateObjectsSpacesTestSuiteFactory } from '../../common/suites/update_objects_spaces';
 import { FtrProviderContext } from '../../common/ftr_provider_context';
 
@@ -51,7 +52,55 @@ const createSinglePartTestCases = (spaceId: string) => {
 const createMultiPartTestCases = () => {
   const nonExistentSpace = 'does_not_exist'; // space that doesn't exist
   const eachSpace = [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID];
-  const group1 = [
+  const group1: UpdateObjectsSpacesTestCase[] = [
+    // These test cases ensure that aliases are deleted when objects are unshared.
+    // For simplicity these are done separately, before the others.
+    {
+      objects: [
+        {
+          id: CASES.ALIAS_DELETE_INCLUSIVE.id,
+          existingNamespaces: eachSpace,
+          expectAliasDifference: -1, // one alias should have been deleted from space_2
+        },
+      ],
+      spacesToAdd: [],
+      spacesToRemove: [SPACE_2_ID],
+    },
+    {
+      objects: [
+        {
+          id: CASES.ALIAS_DELETE_INCLUSIVE.id,
+          existingNamespaces: [DEFAULT_SPACE_ID, SPACE_1_ID],
+          expectAliasDifference: -2, // one alias should have been deleted from space_1
+        },
+      ],
+      spacesToAdd: [],
+      spacesToRemove: [SPACE_1_ID],
+    },
+    {
+      objects: [
+        {
+          id: CASES.ALIAS_DELETE_INCLUSIVE.id,
+          existingNamespaces: [DEFAULT_SPACE_ID],
+          expectAliasDifference: -2, // no aliases can exist in the default space, so no aliases were deleted
+        },
+      ],
+      spacesToAdd: [],
+      spacesToRemove: [DEFAULT_SPACE_ID],
+    },
+    {
+      objects: [
+        {
+          id: CASES.ALIAS_DELETE_EXCLUSIVE.id,
+          existingNamespaces: [SPACE_1_ID],
+          expectAliasDifference: -3, // one alias should have been deleted from other_space
+        },
+      ],
+      spacesToAdd: [SPACE_1_ID],
+      spacesToRemove: ['*'],
+    },
+  ];
+  const group2 = [
     // first, add this object to each space and remove it from nonExistentSpace
     // this will succeed even though the object already exists in the default space and it doesn't exist in nonExistentSpace
     { objects: [CASES.DEFAULT_ONLY], spacesToAdd: eachSpace, spacesToRemove: [nonExistentSpace] },
@@ -87,7 +136,7 @@ const createMultiPartTestCases = () => {
       spacesToRemove: [SPACE_1_ID],
     },
   ];
-  const group2 = [
+  const group3 = [
     // first, add this object to space_2 and remove it from space_1
     {
       objects: [CASES.DEFAULT_AND_SPACE_1],
@@ -111,15 +160,17 @@ const createMultiPartTestCases = () => {
       spacesToRemove: [],
     },
   ];
-  return [...group1, ...group2];
+  return [...group1, ...group2, ...group3];
 };
 
 // eslint-disable-next-line import/no-default-export
 export default function ({ getService }: FtrProviderContext) {
   const supertest = getService('supertest');
   const esArchiver = getService('esArchiver');
+  const es = getService('es');
 
   const { addTests, createTestDefinitions } = updateObjectsSpacesTestSuiteFactory(
+    es,
     esArchiver,
     supertest
   );