SavedObjects bulkCreate API should return migrationVersion and strip …

…the type & namespace from the id (#65150)

* Deserialize bulkCreate response to remove namespace type from id

* Index operations don't return _source in response

* Fix integration tests

* repository: make id generation and seq_no/primary_term spreading more explicit

* API Integration test for bulk create without ids

* Fix copy_to_space snapshot

* Revert "Fix copy_to_space snapshot"

This reverts commit 9c2b743.

* Move test into returns block

* repository.test.js stricter regexp matching

src/core/server/saved_objects/service/lib/repository.test.js

@@ -23,6 +23,7 @@ import { SavedObjectsErrorHelpers } from './errors';
 import { SavedObjectsSerializer } from '../../serialization';
 import { encodeHitVersion } from '../../version';
 import { SavedObjectTypeRegistry } from '../../saved_objects_type_registry';
+import { DocumentMigrator } from '../../migrations/core/document_migrator';
 
 jest.mock('./search_dsl/search_dsl', () => ({ getSearchDsl: jest.fn() }));
 
@@ -115,6 +116,7 @@ describe('SavedObjectsRepository', () => {
   const createType = type => ({
     name: type,
     mappings: { properties: mappings.properties[type].properties },
+    migrations: { '1.1.1': doc => doc },
   });
 
   const registry = new SavedObjectTypeRegistry();
@@ -144,6 +146,13 @@ describe('SavedObjectsRepository', () => {
     namespaceType: 'agnostic',
   });
 
+  const documentMigrator = new DocumentMigrator({
+    typeRegistry: registry,
+    kibanaVersion: '2.0.0',
+    log: {},
+    validateDoc: jest.fn(),
+  });
+
   const getMockGetResponse = ({ type, id, references, namespace }) => ({
     // NOTE: Elasticsearch returns more fields (_index, _type) but the SavedObjectsRepository method ignores these
     found: true,
@@ -207,7 +216,7 @@ describe('SavedObjectsRepository', () => {
   beforeEach(() => {
     callAdminCluster = jest.fn();
     migrator = {
-      migrateDocument: jest.fn(doc => doc),
+      migrateDocument: jest.fn().mockImplementation(documentMigrator.migrate),
       runMigrations: async () => ({ status: 'skipped' }),
     };
 
@@ -424,9 +433,17 @@ describe('SavedObjectsRepository', () => {
 
     const getMockBulkCreateResponse = (objects, namespace) => {
       return {
-        items: objects.map(({ type, id }) => ({
+        items: objects.map(({ type, id, attributes, references, migrationVersion }) => ({
           create: {
             _id: `${namespace ? `${namespace}:` : ''}${type}:${id}`,
+            _source: {
+              [type]: attributes,
+              type,
+              namespace,
+              references,
+              ...mockTimestampFields,
+              migrationVersion: migrationVersion || { [type]: '1.1.1' },
+            },
             ...mockVersionProps,
           },
         })),
@@ -474,7 +491,7 @@ describe('SavedObjectsRepository', () => {
 
     const expectSuccessResult = obj => ({
       ...obj,
-      migrationVersion: undefined,
+      migrationVersion: { [obj.type]: '1.1.1' },
       version: mockVersion,
       ...mockTimestampFields,
     });
@@ -619,13 +636,16 @@ describe('SavedObjectsRepository', () => {
       };
 
       const bulkCreateError = async (obj, esError, expectedError) => {
-        const objects = [obj1, obj, obj2];
-        const response = getMockBulkCreateResponse(objects);
+        let response;
         if (esError) {
+          response = getMockBulkCreateResponse([obj1, obj, obj2]);
           response.items[1].create = { error: esError };
+        } else {
+          response = getMockBulkCreateResponse([obj1, obj2]);
         }
         callAdminCluster.mockResolvedValue(response); // this._writeToCluster('bulk', ...)
 
+        const objects = [obj1, obj, obj2];
         const result = await savedObjectsRepository.bulkCreate(objects);
         expectClusterCalls('bulk');
         const objCall = esError ? expectObjArgs(obj) : [];
@@ -781,14 +801,40 @@ describe('SavedObjectsRepository', () => {
           id: 'three',
         };
         const objects = [obj1, obj, obj2];
-        const response = getMockBulkCreateResponse(objects);
+        const response = getMockBulkCreateResponse([obj1, obj2]);
         callAdminCluster.mockResolvedValue(response); // this._writeToCluster('bulk', ...)
         const result = await savedObjectsRepository.bulkCreate(objects);
         expect(callAdminCluster).toHaveBeenCalledTimes(1);
         expect(result).toEqual({
           saved_objects: [expectSuccessResult(obj1), expectError(obj), expectSuccessResult(obj2)],
         });
       });
+
+      it(`a deserialized saved object`, async () => {
+        // Test for fix to https://github.com/elastic/kibana/issues/65088 where
+        // we returned raw ID's when an object without an id was created.
+        const namespace = 'myspace';
+        const response = getMockBulkCreateResponse([obj1, obj2], namespace);
+        callAdminCluster.mockResolvedValueOnce(response); // this._writeToCluster('bulk', ...)
+
+        // Bulk create one object with id unspecified, and one with id specified
+        const result = await savedObjectsRepository.bulkCreate([{ ...obj1, id: undefined }, obj2], {
+          namespace,
+        });
+
+        // Assert that both raw docs from the ES response are deserialized
+        expect(serializer.rawToSavedObject).toHaveBeenNthCalledWith(1, {
+          ...response.items[0].create,
+          _id: expect.stringMatching(/^myspace:config:[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$/),
+        });
+        expect(serializer.rawToSavedObject).toHaveBeenNthCalledWith(2, response.items[1].create);
+
+        // Assert that ID's are deserialized to remove the type and namespace
+        expect(result.saved_objects[0].id).toEqual(
+          expect.stringMatching(/^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$/)
+        );
+        expect(result.saved_objects[1].id).toEqual(obj2.id);
+      });
     });
   });
 
@@ -1604,6 +1650,7 @@ describe('SavedObjectsRepository', () => {
           version: mockVersion,
           attributes,
           references,
+          migrationVersion: { [type]: '1.1.1' },
         });
       });
     });

src/core/server/saved_objects/service/lib/repository.ts

@@ -18,6 +18,7 @@
  */
 
 import { omit } from 'lodash';
+import uuid from 'uuid';
 import { retryCallCluster } from '../../../elasticsearch/retry_call_cluster';
 import { APICaller } from '../../../elasticsearch/';
 
@@ -299,6 +300,8 @@ export class SavedObjectsRepository {
       const requiresNamespacesCheck =
         method === 'index' && this._registry.isMultiNamespace(object.type);
 
+      if (object.id == null) object.id = uuid.v1();
+
       return {
         tag: 'Right' as 'Right',
         value: {
@@ -404,35 +407,25 @@ export class SavedObjectsRepository {
         }
 
         const { requestedId, rawMigratedDoc, esRequestIndex } = expectedResult.value;
-        const response = bulkResponse.items[esRequestIndex];
-        const {
-          error,
-          _id: responseId,
-          _seq_no: seqNo,
-          _primary_term: primaryTerm,
-        } = Object.values(response)[0] as any;
-
-        const {
-          _source: { type, [type]: attributes, references = [], namespaces },
-        } = rawMigratedDoc;
-
-        const id = requestedId || responseId;
+        const { error, ...rawResponse } = Object.values(
+          bulkResponse.items[esRequestIndex]
+        )[0] as any;
+
         if (error) {
           return {
-            id,
-            type,
-            error: getBulkOperationError(error, type, id),
+            id: requestedId,
+            type: rawMigratedDoc._source.type,
+            error: getBulkOperationError(error, rawMigratedDoc._source.type, requestedId),
           };
         }
-        return {
-          id,
-          type,
-          ...(namespaces && { namespaces }),
-          updated_at: time,
-          version: encodeVersion(seqNo, primaryTerm),
-          attributes,
-          references,
-        };
+
+        // When method == 'index' the bulkResponse doesn't include the indexed
+        // _source so we return rawMigratedDoc but have to spread the latest
+        // _seq_no and _primary_term values from the rawResponse.
+        return this._serializer.rawToSavedObject({
+          ...rawMigratedDoc,
+          ...{ _seq_no: rawResponse._seq_no, _primary_term: rawResponse._primary_term },
+        });
       }),
     };
   }

test/api_integration/apis/saved_objects/bulk_create.js

@@ -72,11 +72,26 @@ export default function({ getService }) {
                   attributes: {
                     title: 'A great new dashboard',
                   },
+                  migrationVersion: {
+                    dashboard: resp.body.saved_objects[1].migrationVersion.dashboard,
+                  },
                   references: [],
                 },
               ],
             });
           }));
+
+      it('should not return raw id when object id is unspecified', async () =>
+        await supertest
+          .post(`/api/saved_objects/_bulk_create`)
+          // eslint-disable-next-line no-unused-vars
+          .send(BULK_REQUESTS.map(({ id, ...rest }) => rest))
+          .expect(200)
+          .then(resp => {
+            resp.body.saved_objects.map(({ id }) =>
+              expect(id).not.match(/visualization|dashboard/)
+            );
+          }));
     });
 
     describe('without kibana index', () => {
@@ -106,6 +121,9 @@ export default function({ getService }) {
                     title: 'An existing visualization',
                   },
                   references: [],
+                  migrationVersion: {
+                    visualization: resp.body.saved_objects[0].migrationVersion.visualization,
+                  },
                 },
                 {
                   type: 'dashboard',
@@ -116,6 +134,9 @@ export default function({ getService }) {
                     title: 'A great new dashboard',
                   },
                   references: [],
+                  migrationVersion: {
+                    dashboard: resp.body.saved_objects[1].migrationVersion.dashboard,
+                  },
                 },
               ],
             });