Preserve originId when overwriting existing objects

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

@@ -255,7 +255,7 @@ async function bulkGetObjectsAndAliases(
  docsToBulkGet.push({
  _id: serializer.generateRawId(undefined, type, id), // namespace is intentionally undefined because multi-namespace objects don't have a namespace in their raw ID
  _index: getIndexForType(type),
- _source: ['type', 'namespaces'],
+ _source: ['type', 'namespaces', 'originId'],
  });
  if (checkAliases) {
  for (const space of spaces) {

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

@@ -46,6 +46,7 @@ import {
 import { ALL_NAMESPACES_STRING } from './utils';
 import { loggerMock } from '@kbn/logging-mocks';
 import {
+ SavedObjectsRawDoc,
  SavedObjectsRawDocSource,
  SavedObjectsSerializer,
  SavedObjectUnsanitizedDoc,
@@ -431,7 +432,6 @@ describe('SavedObjectsRepository', () => {
  id: '6.0.0-alpha1',
  attributes: { title: 'Test One' },
  references: [{ name: 'ref_0', type: 'test', id: '1' }],
- originId: 'some-origin-id', // only one of the object args has an originId, this is intentional to test both a positive and negative case
  };
  const obj2 = {
  type: 'index-pattern',
@@ -613,6 +613,96 @@ describe('SavedObjectsRepository', () => {
  );
  });
 
+ describe('originId', () => {
+ it(`returns error if originId is set for non-multi-namespace type`, async () => {
+ const result = await savedObjectsRepository.bulkCreate([
+ { ...obj1, originId: 'some-originId' },
+ { ...obj2, type: NAMESPACE_AGNOSTIC_TYPE, originId: 'some-originId' },
+ ]);
+ expect(result.saved_objects).toEqual([
+ expect.objectContaining({ id: obj1.id, type: obj1.type, error: expect.anything() }),
+ expect.objectContaining({
+ id: obj2.id,
+ type: NAMESPACE_AGNOSTIC_TYPE,
+ error: expect.anything(),
+ }),
+ ]);
+ expect(client.bulk).not.toHaveBeenCalled();
+ });
+
+ it(`defaults to no originId`, async () => {
+ const objects = [
+ { ...obj1, type: MULTI_NAMESPACE_TYPE },
+ { ...obj2, type: MULTI_NAMESPACE_ISOLATED_TYPE },
+ ];
+
+ await bulkCreateSuccess(objects);
+ const expected = expect.not.objectContaining({ originId: expect.anything() });
+ const body = [expect.any(Object), expected, expect.any(Object), expected];
+ expect(client.bulk).toHaveBeenCalledWith(
+ expect.objectContaining({ body }),
+ expect.anything()
+ );
+ });
+
+ describe('with existing originId', () => {
+ beforeEach(() => {
+ mockPreflightCheckForCreate.mockImplementation(({ objects }) => {
+ const existingDocument = {
+ _source: { originId: 'existing-originId' },
+ } as SavedObjectsRawDoc;
+ return Promise.resolve(
+ objects.map(({ type, id }) => ({ type, id, existingDocument }))
+ );
+ });
+ });
+
+ it(`accepts custom originId for multi-namespace type`, async () => {
+ // The preflight result has `existing-originId`, but that is discarded
+ const objects = [
+ { ...obj1, type: MULTI_NAMESPACE_TYPE, originId: 'some-originId' },
+ { ...obj2, type: MULTI_NAMESPACE_ISOLATED_TYPE, originId: 'some-originId' },
+ ];
+ await bulkCreateSuccess(objects);
+ const expected = expect.objectContaining({ originId: 'some-originId' });
+ const body = [expect.any(Object), expected, expect.any(Object), expected];
+ expect(client.bulk).toHaveBeenCalledWith(
+ expect.objectContaining({ body }),
+ expect.anything()
+ );
+ });
+
+ it(`accepts undefined originId`, async () => {
+ // The preflight result has `existing-originId`, but that is discarded
+ const objects = [
+ { ...obj1, type: MULTI_NAMESPACE_TYPE, originId: undefined },
+ { ...obj2, type: MULTI_NAMESPACE_ISOLATED_TYPE, originId: undefined },
+ ];
+ await bulkCreateSuccess(objects);
+ const expected = expect.not.objectContaining({ originId: expect.anything() });
+ const body = [expect.any(Object), expected, expect.any(Object), expected];
+ expect(client.bulk).toHaveBeenCalledWith(
+ expect.objectContaining({ body }),
+ expect.anything()
+ );
+ });
+
+ it(`preserves existing originId if originId option is not set`, async () => {
+ const objects = [
+ { ...obj1, type: MULTI_NAMESPACE_TYPE },
+ { ...obj2, type: MULTI_NAMESPACE_ISOLATED_TYPE },
+ ];
+ await bulkCreateSuccess(objects);
+ const expected = expect.objectContaining({ originId: 'existing-originId' });
+ const body = [expect.any(Object), expected, expect.any(Object), expected];
+ expect(client.bulk).toHaveBeenCalledWith(
+ expect.objectContaining({ body }),
+ expect.anything()
+ );
+ });
+ });
+ });
+
  it(`adds namespace to request body for any types that are single-namespace`, async () => {
  await bulkCreateSuccess([obj1, obj2], { namespace });
  const expected = expect.objectContaining({ namespace });
@@ -2116,7 +2206,6 @@ describe('SavedObjectsRepository', () => {
  const attributes = { title: 'Logstash' };
  const id = 'logstash-*';
  const namespace = 'foo-namespace';
- const originId = 'some-origin-id';
  const references = [
  {
  name: 'ref_0',
@@ -2226,24 +2315,73 @@ describe('SavedObjectsRepository', () => {
  await test(null);
  });
 
- it(`defaults to no originId`, async () => {
- await createSuccess(type, attributes, { id });
- expect(client.create).toHaveBeenCalledWith(
- expect.objectContaining({
- body: expect.not.objectContaining({ originId: expect.anything() }),
- }),
- expect.anything()
- );
- });
+ describe('originId', () => {
+ for (const objType of [type, NAMESPACE_AGNOSTIC_TYPE]) {
+ it(`throws an error if originId is set for non-multi-namespace type`, async () => {
+ await expect(
+ savedObjectsRepository.create(objType, attributes, { originId: 'some-originId' })
+ ).rejects.toThrowError(
+ createBadRequestError('"originId" can only be set for multi-namespace object types')
+ );
+ });
+ }
 
- it(`accepts custom originId`, async () => {
- await createSuccess(type, attributes, { id, originId });
- expect(client.create).toHaveBeenCalledWith(
- expect.objectContaining({
- body: expect.objectContaining({ originId }),
- }),
- expect.anything()
- );
+ for (const objType of [MULTI_NAMESPACE_TYPE, MULTI_NAMESPACE_ISOLATED_TYPE]) {
+ it(`${objType} defaults to no originId`, async () => {
+ await createSuccess(objType, attributes, { id });
+ expect(client.create).toHaveBeenCalledWith(
+ expect.objectContaining({
+ body: expect.not.objectContaining({ originId: expect.anything() }),
+ }),
+ expect.anything()
+ );
+ });
+
+ describe(`${objType} with existing originId`, () => {
+ beforeEach(() => {
+ mockPreflightCheckForCreate.mockImplementation(({ objects }) => {
+ const existingDocument = {
+ _source: { originId: 'existing-originId' },
+ } as SavedObjectsRawDoc;
+ return Promise.resolve(
+ objects.map(({ type, id }) => ({ type, id, existingDocument }))
+ );
+ });
+ });
+
+ it(`accepts custom originId for multi-namespace type`, async () => {
+ // The preflight result has `existing-originId`, but that is discarded
+ await createSuccess(objType, attributes, { id, originId: 'some-originId' });
+ expect(client.create).toHaveBeenCalledWith(
+ expect.objectContaining({
+ body: expect.objectContaining({ originId: 'some-originId' }),
+ }),
+ expect.anything()
+ );
+ });
+
+ it(`accepts undefined originId`, async () => {
+ // The preflight result has `existing-originId`, but that is discarded
+ await createSuccess(objType, attributes, { id, originId: undefined });
+ expect(client.create).toHaveBeenCalledWith(
+ expect.objectContaining({
+ body: expect.not.objectContaining({ originId: expect.anything() }),
+ }),
+ expect.anything()
+ );
+ });
+
+ it(`preserves existing originId if originId option is not set`, async () => {
+ await createSuccess(objType, attributes, { id });
+ expect(client.create).toHaveBeenCalledWith(
+ expect.objectContaining({
+ body: expect.objectContaining({ originId: 'existing-originId' }),
+ }),
+ expect.anything()
+ );
+ });
+ });
+ }
  });
 
  it(`defaults to a refresh setting of wait_for`, async () => {
@@ -2638,22 +2776,20 @@ describe('SavedObjectsRepository', () => {
 
  describe('returns', () => {
  it(`formats the ES response`, async () => {
- const result = await createSuccess(type, attributes, {
+ const result = await createSuccess(MULTI_NAMESPACE_TYPE, attributes, {
  id,
  namespace,
  references,
- originId,
  });
  expect(result).toEqual({
- type,
+ type: MULTI_NAMESPACE_TYPE,
  id,
- originId,
  ...mockTimestampFields,
  version: mockVersion,
  attributes,
  references,
  namespaces: [namespace ?? 'default'],
- migrationVersion: { [type]: '1.1.1' },
+ migrationVersion: { [MULTI_NAMESPACE_TYPE]: '1.1.1' },
  coreMigrationVersion: KIBANA_VERSION,
  });
  });

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

@@ -315,14 +315,14 @@ export class SavedObjectsRepository {
  overwrite = false,
  references = [],
  refresh = DEFAULT_REFRESH_SETTING,
- originId,
  initialNamespaces,
  version,
  } = options;
  const id = options.id || SavedObjectsUtils.generateId();
  const namespace = normalizeNamespace(options.namespace);
 
  this.validateInitialNamespaces(type, initialNamespaces);
+ this.validateOriginId(type, options);
 
  if (!this._allowedTypes.includes(type)) {
  throw SavedObjectsErrorHelpers.createUnsupportedTypeError(type);
@@ -331,6 +331,7 @@ export class SavedObjectsRepository {
  const time = getCurrentTime();
  let savedObjectNamespace: string | undefined;
  let savedObjectNamespaces: string[] | undefined;
+ let existingOriginId: string | undefined;
 
  if (this._registry.isSingleNamespace(type)) {
  savedObjectNamespace = initialNamespaces
@@ -354,11 +355,17 @@ export class SavedObjectsRepository {
  }
  savedObjectNamespaces =
  initialNamespaces || getSavedObjectNamespaces(namespace, existingDocument);
+ existingOriginId = existingDocument?._source?.originId;
  } else {
  savedObjectNamespaces = initialNamespaces || getSavedObjectNamespaces(namespace);
  }
  }
 
+ // 1. If the originId has been *explicitly set* in the options (defined or undefined), respect that.
+ // 2. Otherwise, preserve the originId of the existing object that is being overwritten, if any.
+ const originId = Object.keys(options).includes('originId')
+ ? options.originId
+ : existingOriginId;
  const migrated = this._migrator.migrateDocument({
  id,
  type,
@@ -442,6 +449,7 @@ export class SavedObjectsRepository {
  } else {
  try {
  this.validateInitialNamespaces(type, initialNamespaces);
+ this.validateOriginId(type, object);
  } catch (e) {
  error = e;
  }
@@ -499,6 +507,7 @@ export class SavedObjectsRepository {
 
  let savedObjectNamespace: string | undefined;
  let savedObjectNamespaces: string[] | undefined;
+ let existingOriginId: string | undefined;
  let versionProperties;
  const {
  preflightCheckIndex,
@@ -525,6 +534,7 @@ export class SavedObjectsRepository {
  savedObjectNamespaces =
  initialNamespaces || getSavedObjectNamespaces(namespace, existingDocument);
  versionProperties = getExpectedVersionProperties(version);
+ existingOriginId = existingDocument?._source?.originId;
  } else {
  if (this._registry.isSingleNamespace(object.type)) {
  savedObjectNamespace = initialNamespaces
@@ -536,6 +546,11 @@ export class SavedObjectsRepository {
  versionProperties = getExpectedVersionProperties(version);
  }
 
+ // 1. If the originId has been *explicitly set* for the object (defined or undefined), respect that.
+ // 2. Otherwise, preserve the originId of the existing object that is being overwritten, if any.
+ const originId = Object.keys(object).includes('originId')
+ ? object.originId
+ : existingOriginId;
  const migrated = this._migrator.migrateDocument({
  id: object.id,
  type: object.type,
@@ -546,7 +561,7 @@ export class SavedObjectsRepository {
  ...(savedObjectNamespaces && { namespaces: savedObjectNamespaces }),
  updated_at: time,
  references: object.references || [],
- originId: object.originId,
+ originId,
  }) as SavedObjectSanitizedDoc<T>;
 
  /**
@@ -2335,6 +2350,18 @@ export class SavedObjectsRepository {
  throw SavedObjectsErrorHelpers.createBadRequestError(error.message);
  }
  }
+
+ /** This is used when objects are created. */
+ private validateOriginId(type: string, objectOrOptions: { originId?: string }) {
+ if (
+ Object.keys(objectOrOptions).includes('originId') &&
+ !this._registry.isMultiNamespace(type)
+ ) {
+ throw SavedObjectsErrorHelpers.createBadRequestError(
+ '"originId" can only be set for multi-namespace object types'
+ );
+ }
+ }
 }
 
 /**