[7.9] Revert add support for version on create & bulkCreate when overwriting a document (#76280)

mikecote · web-flow · commit bbb51b5dcbfb · 2020-08-31T10:47:51.000-04:00
* Revert "Fix more broken usages of `bulkCreate` (#76005) (#76131)" This reverts commit 44a017b. * Revert "Filter saved object `version` during legacy import (#75597) (#75793)" This reverts commit 6e82885. * Revert "[7.9] [Saved objects] Add support for version on create & bulkCreate when overwriting a document (#75172) (#75498)" This reverts commit 00836e5.
diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkcreateobject.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkcreateobject.md
@@ -20,5 +20,4 @@ export interface SavedObjectsBulkCreateObject<T = unknown>
 |  [migrationVersion](./kibana-plugin-core-server.savedobjectsbulkcreateobject.migrationversion.md) | <code>SavedObjectsMigrationVersion</code> | Information about the migrations that have been applied to this SavedObject. When Kibana starts up, KibanaMigrator detects outdated documents and migrates them based on this value. For each migration that has been applied, the plugin's name is used as a key and the latest migration version as the value. |
 |  [references](./kibana-plugin-core-server.savedobjectsbulkcreateobject.references.md) | <code>SavedObjectReference[]</code> |  |
 |  [type](./kibana-plugin-core-server.savedobjectsbulkcreateobject.type.md) | <code>string</code> |  |
-|  [version](./kibana-plugin-core-server.savedobjectsbulkcreateobject.version.md) | <code>string</code> |  |
 
diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkcreateobject.version.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkcreateobject.version.md
diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectscreateoptions.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectscreateoptions.md
@@ -20,5 +20,4 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions
 |  [overwrite](./kibana-plugin-core-server.savedobjectscreateoptions.overwrite.md) | <code>boolean</code> | Overwrite existing documents (defaults to false) |
 |  [references](./kibana-plugin-core-server.savedobjectscreateoptions.references.md) | <code>SavedObjectReference[]</code> |  |
 |  [refresh](./kibana-plugin-core-server.savedobjectscreateoptions.refresh.md) | <code>MutatingOperationRefreshSetting</code> | The Elasticsearch Refresh setting for this operation |
-|  [version](./kibana-plugin-core-server.savedobjectscreateoptions.version.md) | <code>string</code> | An opaque version number which changes on each successful write operation. Can be used in conjunction with <code>overwrite</code> for implementing optimistic concurrency control. |
 
diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectscreateoptions.version.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectscreateoptions.version.md
diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts
@@ -25,7 +25,6 @@ import {
   SavedObjectsImportOptions,
 } from './types';
 import { validateReferences } from './validate_references';
-import { SavedObject } from '../types';
 
 /**
  * Import saved objects from given stream. See the {@link SavedObjectsImportOptions | options} for more
@@ -68,7 +67,7 @@ export async function importSavedObjectsFromStream({
   }
 
   // Create objects in bulk
-  const bulkCreateResult = await savedObjectsClient.bulkCreate(omitVersion(filteredObjects), {
+  const bulkCreateResult = await savedObjectsClient.bulkCreate(filteredObjects, {
     overwrite,
     namespace,
   });
@@ -83,7 +82,3 @@ export async function importSavedObjectsFromStream({
     ...(errorAccumulator.length ? { errors: errorAccumulator } : {}),
   };
 }
-
-export function omitVersion(objects: SavedObject[]): SavedObject[] {
-  return objects.map(({ version, ...object }) => object);
-}
diff --git a/src/core/server/saved_objects/import/resolve_import_errors.ts b/src/core/server/saved_objects/import/resolve_import_errors.ts
@@ -26,7 +26,6 @@ import {
   SavedObjectsResolveImportErrorsOptions,
 } from './types';
 import { validateReferences } from './validate_references';
-import { omitVersion } from './import_saved_objects';
 
 /**
  * Resolve and return saved object import errors.
@@ -92,7 +91,7 @@ export async function resolveSavedObjectsImportErrors({
   // Bulk create in two batches, overwrites and non-overwrites
   const { objectsToOverwrite, objectsToNotOverwrite } = splitOverwrites(filteredObjects, retries);
   if (objectsToOverwrite.length) {
-    const bulkCreateResult = await savedObjectsClient.bulkCreate(omitVersion(objectsToOverwrite), {
+    const bulkCreateResult = await savedObjectsClient.bulkCreate(objectsToOverwrite, {
       overwrite: true,
       namespace,
     });
@@ -103,12 +102,9 @@ export async function resolveSavedObjectsImportErrors({
     successCount += bulkCreateResult.saved_objects.filter((obj) => !obj.error).length;
   }
   if (objectsToNotOverwrite.length) {
-    const bulkCreateResult = await savedObjectsClient.bulkCreate(
-      omitVersion(objectsToNotOverwrite),
-      {
-        namespace,
-      }
-    );
+    const bulkCreateResult = await savedObjectsClient.bulkCreate(objectsToNotOverwrite, {
+      namespace,
+    });
     errorAccumulator = [
       ...errorAccumulator,
       ...extractErrors(bulkCreateResult.saved_objects, objectsToNotOverwrite),
diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js
@@ -472,16 +472,8 @@ describe('SavedObjectsRepository', () => {
       { method, _index = expect.any(String), getId = () => expect.any(String) }
     ) => {
       const body = [];
-      for (const { type, id, if_primary_term: ifPrimaryTerm, if_seq_no: ifSeqNo } of objects) {
-        body.push({
-          [method]: {
-            _index,
-            _id: getId(type, id),
-            ...(ifPrimaryTerm && ifSeqNo
-              ? { if_primary_term: expect.any(Number), if_seq_no: expect.any(Number) }
-              : {}),
-          },
-        });
+      for (const { type, id } of objects) {
+        body.push({ [method]: { _index, _id: getId(type, id) } });
         body.push(expect.any(Object));
       }
       expectClusterCallArgs({ body });
@@ -537,27 +529,6 @@ describe('SavedObjectsRepository', () => {
         expectClusterCallArgsAction([obj1, obj2], { method: 'index' });
       });
 
-      it(`should use the ES index method with version if ID and version are defined and overwrite=true`, async () => {
-        await bulkCreateSuccess(
-          [
-            {
-              ...obj1,
-              version: mockVersion,
-            },
-            obj2,
-          ],
-          { overwrite: true }
-        );
-
-        const obj1WithSeq = {
-          ...obj1,
-          if_seq_no: mockVersionProps._seq_no,
-          if_primary_term: mockVersionProps._primary_term,
-        };
-
-        expectClusterCallArgsAction([obj1WithSeq, obj2], { method: 'index' });
-      });
-
       it(`should use the ES create method if ID is defined and overwrite=false`, async () => {
         await bulkCreateSuccess([obj1, obj2]);
         expectClusterCallArgsAction([obj1, obj2], { method: 'create' });
@@ -1487,15 +1458,6 @@ describe('SavedObjectsRepository', () => {
         expectClusterCalls('index');
       });
 
-      it(`should use the ES index with version if ID and version are defined and overwrite=true`, async () => {
-        await createSuccess(type, attributes, { id, overwrite: true, version: mockVersion });
-        expectClusterCalls('index');
-        expectClusterCallArgs({
-          if_seq_no: mockVersionProps._seq_no,
-          if_primary_term: mockVersionProps._primary_term,
-        });
-      });
-
       it(`should use the ES create action if ID is defined and overwrite=false`, async () => {
         await createSuccess(type, attributes, { id });
         expectClusterCalls('create');
diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts
@@ -220,7 +220,6 @@ export class SavedObjectsRepository {
       overwrite = false,
       references = [],
       refresh = DEFAULT_REFRESH_SETTING,
-      version,
     } = options;
 
     if (!this._allowedTypes.includes(type)) {
@@ -261,7 +260,6 @@ export class SavedObjectsRepository {
       index: this.getIndexForType(type),
       refresh,
       body: raw._source,
-      ...(overwrite && version ? decodeRequestVersion(version) : {}),
     });
 
     return this._rawToSavedObject<T>({
@@ -341,12 +339,7 @@ export class SavedObjectsRepository {
 
       let savedObjectNamespace;
       let savedObjectNamespaces;
-      let versionProperties;
-      const {
-        esRequestIndex,
-        object: { version, ...object },
-        method,
-      } = expectedBulkGetResult.value;
+      const { esRequestIndex, object, method } = expectedBulkGetResult.value;
       if (esRequestIndex !== undefined) {
         const indexFound = bulkGetResponse.status !== 404;
         const actualResult = indexFound ? bulkGetResponse.docs[esRequestIndex] : undefined;
@@ -363,14 +356,12 @@ export class SavedObjectsRepository {
           };
         }
         savedObjectNamespaces = getSavedObjectNamespaces(namespace, docFound && actualResult);
-        versionProperties = getExpectedVersionProperties(version, actualResult);
       } else {
         if (this._registry.isSingleNamespace(object.type)) {
           savedObjectNamespace = namespace;
         } else if (this._registry.isMultiNamespace(object.type)) {
           savedObjectNamespaces = getSavedObjectNamespaces(namespace);
         }
-        versionProperties = getExpectedVersionProperties(version);
       }
 
       const expectedResult = {
@@ -395,7 +386,6 @@ export class SavedObjectsRepository {
           [method]: {
             _id: expectedResult.rawMigratedDoc._id,
             _index: this.getIndexForType(object.type),
-            ...(overwrite && versionProperties),
           },
         },
         expectedResult.rawMigratedDoc._source
diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts
@@ -37,11 +37,6 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions {
   id?: string;
   /** Overwrite existing documents (defaults to false) */
   overwrite?: boolean;
-  /**
-   * An opaque version number which changes on each successful write operation.
-   * Can be used in conjunction with `overwrite` for implementing optimistic concurrency control.
-   **/
-  version?: string;
   /** {@inheritDoc SavedObjectsMigrationVersion} */
   migrationVersion?: SavedObjectsMigrationVersion;
   references?: SavedObjectReference[];
@@ -57,7 +52,6 @@ export interface SavedObjectsBulkCreateObject<T = unknown> {
   id?: string;
   type: string;
   attributes: T;
-  version?: string;
   references?: SavedObjectReference[];
   /** {@inheritDoc SavedObjectsMigrationVersion} */
   migrationVersion?: SavedObjectsMigrationVersion;
diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md
@@ -1945,8 +1945,6 @@ export interface SavedObjectsBulkCreateObject<T = unknown> {
     references?: SavedObjectReference[];
     // (undocumented)
     type: string;
-    // (undocumented)
-    version?: string;
 }
 
 // @public (undocumented)
@@ -2081,7 +2079,6 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions {
     // (undocumented)
     references?: SavedObjectReference[];
     refresh?: MutatingOperationRefreshSetting;
-    version?: string;
 }
 
 // @public (undocumented)
diff --git a/src/plugins/home/public/application/components/tutorial/saved_objects_installer.js b/src/plugins/home/public/application/components/tutorial/saved_objects_installer.js
@@ -62,12 +62,9 @@ class SavedObjectsInstallerUi extends React.Component {
 
     let resp;
     try {
-      // Filter out the saved object version field, if present, to avoid inadvertently triggering optimistic concurrency control.
-      const objectsToCreate = this.props.savedObjects.map(
-        // eslint-disable-next-line no-unused-vars
-        ({ version, ...savedObject }) => savedObject
-      );
-      resp = await this.props.bulkCreate(objectsToCreate, { overwrite: this.state.overwrite });
+      resp = await this.props.bulkCreate(this.props.savedObjects, {
+        overwrite: this.state.overwrite,
+      });
     } catch (error) {
       if (!this._isMounted) {
         return;
diff --git a/src/plugins/home/public/application/components/tutorial/saved_objects_installer.test.js b/src/plugins/home/public/application/components/tutorial/saved_objects_installer.test.js
@@ -79,25 +79,4 @@ describe('bulkCreate', () => {
 
     expect(component).toMatchSnapshot();
   });
-
-  test('should filter out saved object version before calling bulkCreate', async () => {
-    const bulkCreateMock = jest.fn().mockResolvedValue({
-      savedObjects: [savedObject],
-    });
-    const component = mountWithIntl(
-      <SavedObjectsInstaller.WrappedComponent
-        bulkCreate={bulkCreateMock}
-        savedObjects={[{ ...savedObject, version: 'foo' }]}
-      />
-    );
-
-    findTestSubject(component, 'loadSavedObjects').simulate('click');
-
-    // Ensure all promises resolve
-    await new Promise((resolve) => process.nextTick(resolve));
-    // Ensure the state changes are reflected
-    component.update();
-
-    expect(bulkCreateMock).toHaveBeenCalledWith([savedObject], expect.any(Object));
-  });
 });
diff --git a/src/plugins/home/server/services/sample_data/routes/install.ts b/src/plugins/home/server/services/sample_data/routes/install.ts
@@ -154,7 +154,7 @@ export function createInstallRoute(
       let createResults;
       try {
         createResults = await context.core.savedObjects.client.bulkCreate(
-          sampleDataset.savedObjects.map(({ version, ...savedObject }) => savedObject),
+          sampleDataset.savedObjects,
           { overwrite: true }
         );
       } catch (err) {
diff --git a/src/plugins/legacy_export/server/lib/import/import_dashboards.test.ts b/src/plugins/legacy_export/server/lib/import/import_dashboards.test.ts
@@ -30,18 +30,12 @@ describe('importDashboards(req)', () => {
     savedObjectClient.bulkCreate.mockResolvedValue({ saved_objects: [] });
 
     importedObjects = [
-      {
-        id: 'dashboard-01',
-        type: 'dashboard',
-        attributes: { panelJSON: '{}' },
-        references: [],
-        version: 'foo',
-      },
+      { id: 'dashboard-01', type: 'dashboard', attributes: { panelJSON: '{}' }, references: [] },
       { id: 'panel-01', type: 'visualization', attributes: { visState: '{}' }, references: [] },
     ];
   });
 
-  test('should call bulkCreate with each asset, filtering out any version if present', async () => {
+  test('should call bulkCreate with each asset', async () => {
     await importDashboards(savedObjectClient, importedObjects, { overwrite: false, exclude: [] });
 
     expect(savedObjectClient.bulkCreate).toHaveBeenCalledTimes(1);
diff --git a/src/plugins/legacy_export/server/lib/import/import_dashboards.ts b/src/plugins/legacy_export/server/lib/import/import_dashboards.ts
@@ -31,8 +31,7 @@ export async function importDashboards(
   // docs are not seen as automatically up-to-date.
   const docs = objects
     .filter((item) => !exclude.includes(item.type))
-    // filter out any document version, if present
-    .map(({ version, ...doc }) => ({ ...doc, migrationVersion: doc.migrationVersion || {} }));
+    .map((doc) => ({ ...doc, migrationVersion: doc.migrationVersion || {} }));
 
   const results = await savedObjectsClient.bulkCreate(docs, { overwrite });
   return { objects: results.saved_objects };
diff --git a/x-pack/plugins/ingest_manager/server/services/epm/kibana/assets/install.ts b/x-pack/plugins/ingest_manager/server/services/epm/kibana/assets/install.ts
@@ -20,9 +20,7 @@ import {
 import { deleteKibanaSavedObjectsAssets } from '../../packages/remove';
 import { getInstallationObject, savedObjectTypes } from '../../packages';
 
-type SavedObjectToBe = Required<Omit<SavedObjectsBulkCreateObject, 'version'>> & {
-  type: AssetType;
-};
+type SavedObjectToBe = Required<SavedObjectsBulkCreateObject> & { type: AssetType };
 export type ArchiveAsset = Pick<
   SavedObject,
   'id' | 'attributes' | 'migrationVersion' | 'references'
diff --git a/x-pack/plugins/ml/server/models/data_recognizer/data_recognizer.ts b/x-pack/plugins/ml/server/models/data_recognizer/data_recognizer.ts
@@ -20,7 +20,6 @@ import { getAuthorizationHeader } from '../../lib/request_authorization';
 import { MlInfoResponse } from '../../../common/types/ml_server_info';
 import {
   KibanaObjects,
-  KibanaObjectConfig,
   ModuleDataFeed,
   ModuleJob,
   Module,
@@ -101,7 +100,7 @@ interface ObjectExistResponse {
   id: string;
   type: string;
   exists: boolean;
-  savedObject?: { id: string; type: string; attributes: KibanaObjectConfig };
+  savedObject?: any;
 }
 
 interface SaveResults {
@@ -679,14 +678,14 @@ export class DataRecognizer {
     let results = { saved_objects: [] as any[] };
     const filteredSavedObjects = objectExistResults
       .filter((o) => o.exists === false)
-      .map((o) => o.savedObject!);
+      .map((o) => o.savedObject);
     if (filteredSavedObjects.length) {
       results = await this.savedObjectsClient.bulkCreate(
         // Add an empty migrationVersion attribute to each saved object to ensure
         // it is automatically migrated to the 7.0+ format with a references attribute.
         filteredSavedObjects.map((doc) => ({
           ...doc,
-          migrationVersion: {},
+          migrationVersion: doc.migrationVersion || {},
         }))
       );
     }

Original file line number	Diff line number	Diff line change
`@@ -1945,8 +1945,6 @@ export interface SavedObjectsBulkCreateObject<T = unknown> {`
`1945`	`1945`	`references?: SavedObjectReference[];`
`1946`	`1946`	`// (undocumented)`
`1947`	`1947`	`type: string;`
`1948`		`- // (undocumented)`
`1949`		`- version?: string;`
`1950`	`1948`	`}`
`1951`	`1949`
`1952`	`1950`	`// @public (undocumented)`
`@@ -2081,7 +2079,6 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions {`
`2081`	`2079`	`// (undocumented)`
`2082`	`2080`	`references?: SavedObjectReference[];`
`2083`	`2081`	`refresh?: MutatingOperationRefreshSetting;`
`2084`		`- version?: string;`
`2085`	`2082`	`}`
`2086`	`2083`
`2087`	`2084`	`// @public (undocumented)`