[Saved objects client - bulkUpdate] Passing the correct namespace to migrateInputDocument (#222313)

While working on [this
PR](https://github.com/elastic/kibana/pull/221515) I noticed an issue
when trying to bulk update saved objects overriding the current space
for the operation.

---------

Co-authored-by: “jeramysoucy” <jeramy.soucy@elastic.co>
Co-authored-by: Shahzad <shahzad31comp@gmail.com>
Co-authored-by: Rudolf Meijering <skaapgif@gmail.com>

src/core/packages/saved-objects/api-server-internal/src/lib/apis/bulk_update.test.ts

@@ -53,6 +53,7 @@ import {
   expectError,
   createBadRequestErrorPayload,
   expectUpdateResult,
+  MULTI_NAMESPACE_TYPE,
 } from '../../test_helpers/repository.test.common';
 import type { ISavedObjectsSecurityExtension } from '@kbn/core-saved-objects-server';
 import { savedObjectsExtensionsMock } from '../../mocks/saved_objects_extensions.mock';
@@ -620,6 +621,74 @@ describe('#bulkUpdate', () => {
           2
         );
       });
+
+      it('migrates single namespace objects using the object namespace', async () => {
+        const modifiedObj2 = {
+          ...obj2,
+          coreMigrationVersion: '8.0.0',
+          namespace: 'test',
+        };
+        const objects = [modifiedObj2];
+        migrator.migrateDocument.mockImplementationOnce((doc) => ({ ...doc, migrated: true }));
+
+        await bulkUpdateSuccess(client, repository, registry, objects);
+
+        expect(migrator.migrateDocument).toHaveBeenCalledTimes(2);
+        expectMigrationArgs(
+          {
+            id: modifiedObj2.id,
+            namespace: 'test',
+          },
+          true,
+          2
+        );
+      });
+
+      it('migrates multiple namespace objects using the object namespaces', async () => {
+        const modifiedObj2 = {
+          ...obj2,
+          type: MULTI_NAMESPACE_TYPE,
+          coreMigrationVersion: '8.0.0',
+          namespace: 'test',
+        };
+        const objects = [modifiedObj2];
+        migrator.migrateDocument.mockImplementationOnce((doc) => ({ ...doc, migrated: true }));
+
+        await bulkUpdateSuccess(client, repository, registry, objects);
+
+        expect(migrator.migrateDocument).toHaveBeenCalledTimes(2);
+        expectMigrationArgs(
+          {
+            id: modifiedObj2.id,
+            namespaces: ['test'],
+          },
+          true,
+          2
+        );
+      });
+
+      it('migrates namespace agnsostic objects', async () => {
+        const modifiedObj2 = {
+          ...obj2,
+          type: NAMESPACE_AGNOSTIC_TYPE,
+          coreMigrationVersion: '8.0.0',
+          namespace: 'test', // specify a namespace, but it should be ignored
+        };
+        const objects = [modifiedObj2];
+        migrator.migrateDocument.mockImplementationOnce((doc) => ({ ...doc, migrated: true }));
+
+        await bulkUpdateSuccess(client, repository, registry, objects);
+
+        expect(migrator.migrateDocument).toHaveBeenCalledTimes(2);
+        expectMigrationArgs(
+          {
+            id: modifiedObj2.id,
+            namespaces: [],
+          },
+          true,
+          2
+        );
+      });
     });
 
     describe('returns', () => {

src/core/packages/saved-objects/api-server-internal/src/lib/apis/bulk_update.ts

@@ -150,17 +150,11 @@ export const performBulkUpdate = async <T>(
     };
   }
 
-  // `objectNamespace` is a namespace string, while `namespace` is a namespace ID.
-  // The object namespace string, if defined, will supersede the operation's namespace ID.
-  const namespaceString = SavedObjectsUtils.namespaceIdToString(namespace);
-
   const getNamespaceId = (objectNamespace?: string) =>
     objectNamespace !== undefined
       ? SavedObjectsUtils.namespaceStringToId(objectNamespace)
       : namespace;
 
-  const getNamespaceString = (objectNamespace?: string) => objectNamespace ?? namespaceString;
-
   const bulkGetDocs = validObjects.map(({ value: { type, id, objectNamespace } }) => ({
     _id: serializer.generateRawId(getNamespaceId(objectNamespace), type, id),
     _index: commonHelper.getIndexForType(type),
@@ -235,7 +229,6 @@ export const performBulkUpdate = async <T>(
         mergeAttributes,
       } = expectedBulkGetResult.value;
 
-      let namespaces: string[] | undefined;
       const versionProperties = getExpectedVersionProperties(version);
       const indexFound = bulkGetResponse?.statusCode !== 404;
       const actualResult = indexFound ? bulkGetResponse?.body.docs[esRequestIndex] : undefined;
@@ -258,15 +251,18 @@ export const performBulkUpdate = async <T>(
         });
       }
 
+      let savedObjectNamespace: string | undefined;
+      let savedObjectNamespaces: string[] | undefined;
+
       if (isMultiNS) {
         // @ts-expect-error MultiGetHit is incorrectly missing _id, _source
-        namespaces = actualResult!._source.namespaces ?? [
+        savedObjectNamespaces = actualResult!._source.namespaces ?? [
           // @ts-expect-error MultiGetHit is incorrectly missing _id, _source
           SavedObjectsUtils.namespaceIdToString(actualResult!._source.namespace),
         ];
       } else if (registry.isSingleNamespace(type)) {
         // if `objectNamespace` is undefined, fall back to `options.namespace`
-        namespaces = [getNamespaceString(objectNamespace)];
+        savedObjectNamespace = objectNamespace ?? namespace;
       }
 
       const document = getSavedObjectFromSource<T>(
@@ -310,8 +306,8 @@ export const performBulkUpdate = async <T>(
         ...migrated!,
         id,
         type,
-        namespace,
-        namespaces,
+        ...(savedObjectNamespace && { namespace: savedObjectNamespace }),
+        ...(savedObjectNamespaces && { namespaces: savedObjectNamespaces }),
         attributes: updatedAttributes,
         updated_at: time,
         updated_by: updatedBy,
@@ -321,6 +317,9 @@ export const performBulkUpdate = async <T>(
         migratedUpdatedSavedObjectDoc as SavedObjectSanitizedDoc
       );
 
+      const namespaces =
+        savedObjectNamespaces ?? (savedObjectNamespace ? [savedObjectNamespace] : []);
+
       const expectedResult = {
         type,
         id,

src/core/packages/saved-objects/api-server-internal/src/lib/apis/update.test.ts

@@ -546,9 +546,13 @@ describe('#update', () => {
           namespace: 'default',
         });
         expect(client.index).toHaveBeenCalledWith(
-          expect.objectContaining({ id: expect.stringMatching(`${type}:${id}`) }),
+          expect.objectContaining({
+            id: expect.stringMatching(`${type}:${id}`),
+          }),
           expect.anything()
         );
+        // Assert that 'namespace' does not exist at all
+        expect(client.index.mock.calls[0][0]).not.toHaveProperty('namespace');
       });
 
       it(`doesn't prepend namespace to the id when using agnostic-namespace type`, async () => {

src/core/packages/saved-objects/api-server-internal/src/test_helpers/repository.test.common.ts

@@ -716,7 +716,7 @@ export const expectUpdateResult = ({
   attributes,
   references,
   version: mockVersion,
-  namespaces: ['default'],
+  namespaces: [],
   ...mockTimestampFields,
 });