Prevent legacy url conflicts (#116007)

docs/api/saved-objects/bulk_create.asciidoc

@@ -66,7 +66,8 @@ Saved objects that are unable to persist are replaced with an error object.
 ==== Response code
 
 `200`::
- Indicates a successful call.
+ Indicates a successful call. Note, this HTTP response code indicates that the bulk operation succeeded. Errors pertaining to individual
+ objects will be returned in the response body. See the example below for details.
 
 [[saved-objects-api-bulk-create-example]]
 ==== Example
@@ -122,3 +123,19 @@ The API returns the following:
 --------------------------------------------------
 
 There is already a saved object with the `my-dashboard` ID, so only the index pattern is created.
+
+[[saved-objects-api-bulk-create-conflict-errors]]
+==== Conflict errors
+
+Starting in {kib} 8.0, saved objects can exist in multiple spaces. As a result, you may encounter different types of conflict errors when
+attempting to create an object:
+
+* *Regular conflict*: This is a 409 error without any metadata. It means an object of that type/ID already exists. This can be
+ overridden by using the `overwrite: true` option.
+* *Unresolvable conflict*: This is a 409 error with `isNotOverwritable: true` in its metadata. It means an object of that type/ID already
+ exists in a different space, and it cannot be overridden with the given parameters. To successfully overwrite this object, you must do so
+ in at least one space where it exists. You can specify that using the `space_id` path parameter _or_ the `initialNamespaces` parameter.
+* *Alias conflict*: This is a 409 error with a `spacesWithConflictingAliases` string array in its metadata. It means a conflicting
+ <<legacy-url-aliases,legacy URL alias>> for this type/ID exists in the space(s) where you attempted to create this object. A conflicting
+ legacy URL alias is one that points to a different type/ID. To successfully create this object, you need to first use the
+ <<spaces-api-disable-legacy-url-aliases,`_disable_legacy_url_aliases`>> API to disable the problematic legacy URL alias(es).

docs/api/saved-objects/bulk_get.asciidoc

@@ -53,7 +53,8 @@ Saved objects that are unable to persist are replaced with an error object.
 ==== Response code
 
 `200`::
- Indicates a successful call.
+ Indicates a successful call. Note, this HTTP response code indicates that the bulk operation succeeded. Errors pertaining to individual
+ objects will be returned in the response body. See the example below for details.
 
 [[saved-objects-api-bulk-get-body-example]]
 ==== Example

docs/api/saved-objects/bulk_resolve.asciidoc

@@ -46,7 +46,8 @@ that "exactMatch" is the default outcome, and the outcome only changes if an ali
 ==== Response code
 
 `200`::
- Indicates a successful call.
+ Indicates a successful call. Note, this HTTP response code indicates that the bulk operation succeeded. Errors pertaining to individual
+ objects will be returned in the response body. See the example below for details.
 
 [[saved-objects-api-bulk-resolve-body-example]]
 ==== Example

docs/api/saved-objects/create.asciidoc

@@ -64,6 +64,9 @@ used to specify a single space, and the "All spaces" identifier (`'*'`) is not a
 `200`::
  Indicates a successful call.
 
+`409`::
+ Indicates a <<saved-objects-api-create-conflict-errors,conflict error>>.
+
 [[saved-objects-api-create-example]]
 ==== Example
 
@@ -93,3 +96,11 @@ The API returns the following:
 --------------------------------------------------
 
 <1> When `my-pattern` is unspecified in the path, a unique ID is automatically generated.
+
+[[saved-objects-api-create-conflict-errors]]
+==== Conflict errors
+
+Starting in {kib} 8.0, saved objects can exist in multiple spaces. As a result, you may encounter different types of conflict errors when
+attempting to create an object. If you encounter a 409 error that cannot be overridden by using the `overwrite: true` option, you are likely
+hitting a different type of conflict error. The Create API response is limited and does not include additional metadata. You can get more
+details about this error by using the <<saved-objects-api-bulk-create,Bulk create API>> instead.

docs/api/saved-objects/update.asciidoc

@@ -45,6 +45,12 @@ WARNING: When you update, attributes are not validated, which allows you to pass
 `200`::
  Indicates a successful call.
 
+`404`::
+ Indicates the object was not found.
+
+`409`::
+ Indicates a <<saved-objects-api-update-conflict-errors,conflict error>>.
+
 [[saved-objects-api-update-example]]
 ==== Example
 
@@ -74,3 +80,10 @@ The API returns the following:
  }
 }
 --------------------------------------------------
+
+[[saved-objects-api-update-conflict-errors]]
+==== Conflict errors
+
+Starting in {kib} 8.0, saved objects can exist in multiple spaces. As a result, you may encounter a 409 *alias conflict* error when using
+the `upsert` option. The Update API response is limited and does not include additional metadata. You can get more details about this error
+by using the <<saved-objects-api-bulk-create,Bulk create API>> instead.

src/core/server/saved_objects/service/lib/collect_multi_namespace_references.test.mock.ts

@@ -6,8 +6,17 @@
  * Side Public License, v 1.
  */
 
+import type { findLegacyUrlAliases } from './find_legacy_url_aliases';
 import type * as InternalUtils from './internal_utils';
 
+export const mockFindLegacyUrlAliases = jest.fn() as jest.MockedFunction<
+ typeof findLegacyUrlAliases
+>;
+
+jest.mock('./find_legacy_url_aliases', () => {
+ return { findLegacyUrlAliases: mockFindLegacyUrlAliases };
+});
+
 export const mockRawDocExistsInNamespace = jest.fn() as jest.MockedFunction<
  typeof InternalUtils['rawDocExistsInNamespace']
 >;

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

@@ -6,25 +6,25 @@
  * Side Public License, v 1.
  */
 
-import { mockRawDocExistsInNamespace } from './collect_multi_namespace_references.test.mock';
+import {
+ mockFindLegacyUrlAliases,
+ mockRawDocExistsInNamespace,
+} from './collect_multi_namespace_references.test.mock';
 
 import type { DeeplyMockedKeys } from '@kbn/utility-types/jest';
-import type { ElasticsearchClient } from 'src/core/server/elasticsearch';
-import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks';
 
-import { LegacyUrlAlias, LEGACY_URL_ALIAS_TYPE } from '../../object_types';
+import type { ElasticsearchClient } from '../../../elasticsearch';
+import { elasticsearchClientMock } from '../../../elasticsearch/client/mocks';
 import { typeRegistryMock } from '../../saved_objects_type_registry.mock';
 import { SavedObjectsSerializer } from '../../serialization';
-import type {
+import {
+ ALIAS_SEARCH_PER_PAGE,
  CollectMultiNamespaceReferencesParams,
  SavedObjectsCollectMultiNamespaceReferencesObject,
  SavedObjectsCollectMultiNamespaceReferencesOptions,
 } from './collect_multi_namespace_references';
 import { collectMultiNamespaceReferences } from './collect_multi_namespace_references';
-import { savedObjectsPointInTimeFinderMock } from './point_in_time_finder.mock';
-import { savedObjectsRepositoryMock } from './repository.mock';
-import { PointInTimeFinder } from './point_in_time_finder';
-import { ISavedObjectsRepository } from './repository';
+import type { CreatePointInTimeFinderFn } from './point_in_time_finder';
 
 const SPACES = ['default', 'another-space'];
 const VERSION_PROPS = { _seq_no: 1, _primary_term: 1 };
@@ -35,17 +35,14 @@ const NON_MULTI_NAMESPACE_OBJ_TYPE = 'type-c';
 const MULTI_NAMESPACE_HIDDEN_OBJ_TYPE = 'type-d';
 
 beforeEach(() => {
+ mockFindLegacyUrlAliases.mockReset();
+ mockFindLegacyUrlAliases.mockResolvedValue(new Map()); // return an empty map by default
  mockRawDocExistsInNamespace.mockReset();
  mockRawDocExistsInNamespace.mockReturnValue(true); // return true by default
 });
 
 describe('collectMultiNamespaceReferences', () => {
  let client: DeeplyMockedKeys<ElasticsearchClient>;
- let savedObjectsMock: jest.Mocked<ISavedObjectsRepository>;
- let createPointInTimeFinder: jest.MockedFunction<
- CollectMultiNamespaceReferencesParams['createPointInTimeFinder']
- >;
- let pointInTimeFinder: DeeplyMockedKeys<PointInTimeFinder>;
 
  /** Sets up the type registry, saved objects client, etc. and return the full parameters object to be passed to `collectMultiNamespaceReferences` */
  function setup(
@@ -67,20 +64,6 @@ describe('collectMultiNamespaceReferences', () => {
  client = elasticsearchClientMock.createElasticsearchClient();
 
  const serializer = new SavedObjectsSerializer(registry);
- savedObjectsMock = savedObjectsRepositoryMock.create();
- savedObjectsMock.find.mockResolvedValue({
- pit_id: 'foo',
- saved_objects: [],
- // the rest of these fields don't matter but are included for type safety
- total: 0,
- page: 1,
- per_page: 100,
- });
- createPointInTimeFinder = jest.fn();
- createPointInTimeFinder.mockImplementation((params) => {
- pointInTimeFinder = savedObjectsPointInTimeFinderMock.create({ savedObjectsMock })(params);
- return pointInTimeFinder;
- });
  return {
  registry,
  allowedTypes: [
@@ -91,7 +74,7 @@ describe('collectMultiNamespaceReferences', () => {
  client,
  serializer,
  getIndexForType: (type: string) => `index-for-${type}`,
- createPointInTimeFinder,
+ createPointInTimeFinder: jest.fn() as CreatePointInTimeFinderFn,
  objects,
  options,
  };
@@ -130,23 +113,6 @@ describe('collectMultiNamespaceReferences', () => {
  );
  }
 
- function mockFindResults(...results: LegacyUrlAlias[]) {
- savedObjectsMock.find.mockResolvedValueOnce({
- pit_id: 'foo',
- saved_objects: results.map((attributes) => ({
- id: 'doesnt-matter',
- type: LEGACY_URL_ALIAS_TYPE,
- attributes,
- references: [],
- score: 0, // doesn't matter
- })),
- // the rest of these fields don't matter but are included for type safety
- total: 0,
- page: 1,
- per_page: 100,
- });
- }
-
  /** Asserts that mget is called for the given objects */
  function expectMgetArgs(
  n: number,
@@ -320,45 +286,30 @@ describe('collectMultiNamespaceReferences', () => {
  });
 
  describe('legacy URL aliases', () => {
- it('uses the PointInTimeFinder to search for legacy URL aliases', async () => {
+ it('uses findLegacyUrlAliases to search for legacy URL aliases', async () => {
  const obj1 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-1' };
  const obj2 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-2' };
  const obj3 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-3' };
  const params = setup([obj1, obj2], {});
  mockMgetResults({ found: true, references: [obj3] }, { found: true, references: [] }); // results for obj1 and obj2
  mockMgetResults({ found: true, references: [] }); // results for obj3
- mockFindResults(
- // mock search results for four aliases for obj1, and none for obj2 or obj3
- ...[1, 2, 3, 4].map((i) => ({
- sourceId: obj1.id,
- targetId: 'doesnt-matter',
- targetType: obj1.type,
- targetNamespace: `space-${i}`,
- }))
+ mockFindLegacyUrlAliases.mockResolvedValue(
+ new Map([
+ [`${obj1.type}:${obj1.id}`, new Set(['space-1', 'space-2', 'space-3', 'space-4'])],
+ // the result map does not contain keys for obj2 or obj3 because we did not find any aliases for those objects
+ ])
  );
 
  const result = await collectMultiNamespaceReferences(params);
  expect(client.mget).toHaveBeenCalledTimes(2);
  expectMgetArgs(1, obj1, obj2);
  expectMgetArgs(2, obj3); // obj3 is retrieved in a second cluster call
- expect(createPointInTimeFinder).toHaveBeenCalledTimes(1);
- const kueryFilterArgs = createPointInTimeFinder.mock.calls[0][0].filter.arguments;
- expect(kueryFilterArgs).toHaveLength(2);
- const typeAndIdFilters = kueryFilterArgs[1].arguments;
- expect(typeAndIdFilters).toHaveLength(3);
- [obj1, obj2, obj3].forEach(({ type, id }, i) => {
- const typeAndIdFilter = typeAndIdFilters[i].arguments;
- expect(typeAndIdFilter).toEqual([
- expect.objectContaining({
- arguments: expect.arrayContaining([{ type: 'literal', value: type }]),
- }),
- expect.objectContaining({
- arguments: expect.arrayContaining([{ type: 'literal', value: id }]),
- }),
- ]);
- });
- expect(pointInTimeFinder.find).toHaveBeenCalledTimes(1);
- expect(pointInTimeFinder.close).toHaveBeenCalledTimes(2);
+ expect(mockFindLegacyUrlAliases).toHaveBeenCalledTimes(1);
+ expect(mockFindLegacyUrlAliases).toHaveBeenCalledWith(
+ expect.anything(),
+ [obj1, obj2, obj3],
+ ALIAS_SEARCH_PER_PAGE
+ );
  expect(result.objects).toEqual([
  {
  ...obj1,
@@ -371,74 +322,32 @@ describe('collectMultiNamespaceReferences', () => {
  ]);
  });
 
- it('does not create a PointInTimeFinder if no objects are passed in', async () => {
- const params = setup([]);
-
- await collectMultiNamespaceReferences(params);
- expect(params.createPointInTimeFinder).not.toHaveBeenCalled();
- });
-
- it('does not search for objects that have an empty spaces array (the object does not exist, or we are not sure)', async () => {
+ it('omits objects that have an empty spaces array (the object does not exist, or we are not sure)', async () => {
  const obj1 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-1' };
  const obj2 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-2' };
  const params = setup([obj1, obj2]);
  mockMgetResults({ found: true }, { found: false }); // results for obj1 and obj2
 
  await collectMultiNamespaceReferences(params);
- expect(createPointInTimeFinder).toHaveBeenCalledTimes(1);
-
- const kueryFilterArgs = createPointInTimeFinder.mock.calls[0][0].filter.arguments;
- expect(kueryFilterArgs).toHaveLength(2);
- const typeAndIdFilters = kueryFilterArgs[1].arguments;
- expect(typeAndIdFilters).toHaveLength(1);
- const typeAndIdFilter = typeAndIdFilters[0].arguments;
- expect(typeAndIdFilter).toEqual([
- expect.objectContaining({
- arguments: expect.arrayContaining([{ type: 'literal', value: obj1.type }]),
- }),
- expect.objectContaining({
- arguments: expect.arrayContaining([{ type: 'literal', value: obj1.id }]),
- }),
- ]);
- expect(pointInTimeFinder.find).toHaveBeenCalledTimes(1);
- expect(pointInTimeFinder.close).toHaveBeenCalledTimes(2);
- });
-
- it('does not search at all if all objects that have an empty spaces array (the object does not exist, or we are not sure)', async () => {
- const obj1 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-1' };
- const params = setup([obj1]);
- mockMgetResults({ found: false }); // results for obj1
-
- await collectMultiNamespaceReferences(params);
- expect(params.createPointInTimeFinder).not.toHaveBeenCalled();
- });
-
- it('handles PointInTimeFinder.find errors', async () => {
- const obj1 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-1' };
- const params = setup([obj1]);
- mockMgetResults({ found: true }); // results for obj1
- savedObjectsMock.find.mockRejectedValue(new Error('Oh no!'));
-
- await expect(() => collectMultiNamespaceReferences(params)).rejects.toThrow(
- 'Failed to retrieve legacy URL aliases: Oh no!'
+ expect(mockFindLegacyUrlAliases).toHaveBeenCalledTimes(1);
+ expect(mockFindLegacyUrlAliases).toHaveBeenCalledWith(
+ expect.anything(),
+ [obj1],
+ ALIAS_SEARCH_PER_PAGE
  );
- expect(createPointInTimeFinder).toHaveBeenCalledTimes(1);
- expect(pointInTimeFinder.find).toHaveBeenCalledTimes(1);
- expect(pointInTimeFinder.close).toHaveBeenCalledTimes(2); // we still close the point-in-time, even though the search failed
  });
 
- it('handles PointInTimeFinder.close errors', async () => {
+ it('handles findLegacyUrlAliases errors', async () => {
  const obj1 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-1' };
  const params = setup([obj1]);
  mockMgetResults({ found: true }); // results for obj1
- savedObjectsMock.closePointInTime.mockRejectedValue(new Error('Oh no!'));
+ mockFindLegacyUrlAliases.mockRejectedValue(
+ new Error('Failed to retrieve legacy URL aliases: Oh no!')
+ );
 
  await expect(() => collectMultiNamespaceReferences(params)).rejects.toThrow(
  'Failed to retrieve legacy URL aliases: Oh no!'
  );
- expect(createPointInTimeFinder).toHaveBeenCalledTimes(1);
- expect(pointInTimeFinder.find).toHaveBeenCalledTimes(1);
- expect(pointInTimeFinder.close).toHaveBeenCalledTimes(2);
  });
  });
 });