From 86a806c05e209393e11e1ef567e490577908eb69 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 2 Sep 2021 20:19:49 +0200 Subject: [PATCH] use initialNamespaces when checking for conflict --- .../service/lib/repository.test.js | 82 ++++++++++++++++++- .../saved_objects/service/lib/repository.ts | 34 ++++++-- 2 files changed, 105 insertions(+), 11 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 427c28ceb326c..eaec06b42e053 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -197,7 +197,17 @@ describe('SavedObjectsRepository', () => { { type, id, references, namespace: objectNamespace, originId }, namespace ) => { - const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace; + let namespaces; + if (objectNamespace) { + namespaces = [objectNamespace]; + } else if (namespace) { + namespaces = Array.isArray(namespace) ? namespace : [namespace]; + } else { + namespaces = ['default']; + } + const namespaceId = namespaces[0] === 'default' ? undefined : namespaces[0]; + + // const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace; return { // NOTE: Elasticsearch returns more fields (_index, _type) but the SavedObjectsRepository method ignores these found: true, @@ -207,7 +217,7 @@ describe('SavedObjectsRepository', () => { ...mockVersionProps, _source: { ...(registry.isSingleNamespace(type) && { namespace: namespaceId }), - ...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }), + ...(registry.isMultiNamespace(type) && { namespaces }), ...(originId && { originId }), type, [type]: { title: 'Testing' }, @@ -219,7 +229,9 @@ describe('SavedObjectsRepository', () => { }; const getMockMgetResponse = (objects, namespace) => ({ - docs: objects.map((obj) => (obj.found === false ? obj : getMockGetResponse(obj, namespace))), + docs: objects.map((obj) => + obj.found === false ? obj : getMockGetResponse(obj, obj.initialNamespaces ?? namespace) + ), }); expect.extend({ @@ -778,6 +790,54 @@ describe('SavedObjectsRepository', () => { }); }); + it(`returns error when there is a conflict with an existing multi-namespace saved object with initialNamespaces (get)`, async () => { + const obj = { + ...obj3, + type: MULTI_NAMESPACE_TYPE, + initialNamespaces: ['foo-namespace', 'default'], + }; + const response1 = { + status: 200, + docs: [ + { + found: true, + _source: { + type: obj.type, + namespaces: ['bar-namespace'], + }, + }, + ], + }; + client.mget.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise(response1) + ); + const response2 = getMockBulkCreateResponse([obj1, obj2]); + client.bulk.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise(response2) + ); + + const options = { overwrite: true }; + const result = await savedObjectsRepository.bulkCreate([obj1, obj, obj2], options); + + expect(client.bulk).toHaveBeenCalled(); + expect(client.mget).toHaveBeenCalled(); + + const body1 = { docs: [expect.objectContaining({ _id: `${obj.type}:${obj.id}` })] }; + expect(client.mget).toHaveBeenCalledWith( + expect.objectContaining({ body: body1 }), + expect.anything() + ); + const body2 = [...expectObjArgs(obj1), ...expectObjArgs(obj2)]; + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ body: body2 }), + expect.anything() + ); + const expectedError = expectErrorConflict(obj, { metadata: { isNotOverwritable: true } }); + expect(result).toEqual({ + saved_objects: [expectSuccess(obj1), expectedError, expectSuccess(obj2)], + }); + }); + it(`returns bulk error`, async () => { const expectedErrorResult = { type: obj3.type, id: obj3.id, error: 'Oh no, a bulk error!' }; await bulkCreateError(obj3, true, expectedErrorResult); @@ -2160,6 +2220,22 @@ describe('SavedObjectsRepository', () => { expect(client.get).toHaveBeenCalled(); }); + it(`throws when there is a conflict with an existing multi-namespace saved object when using initialNamespaces (get)`, async () => { + const response = getMockGetResponse({ type: MULTI_NAMESPACE_ISOLATED_TYPE, id }, namespace); + client.get.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise(response) + ); + await expect( + savedObjectsRepository.create(MULTI_NAMESPACE_TYPE, attributes, { + id, + overwrite: true, + initialNamespaces: ['bar-ns', 'dolly-ns'], + namespace, + }) + ).rejects.toThrowError(createConflictError(MULTI_NAMESPACE_TYPE, id)); + expect(client.get).toHaveBeenCalled(); + }); + it.todo(`throws when automatic index creation fails`); it.todo(`throws when an unexpected failure occurs`); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 7ce3a55d057eb..09c0de4daca90 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -306,8 +306,12 @@ export class SavedObjectsRepository { if (id && overwrite) { // we will overwrite a multi-namespace saved object if it exists; if that happens, ensure we preserve its included namespaces // note: this check throws an error if the object is found but does not exist in this namespace - const existingNamespaces = await this.preflightGetNamespaces(type, id, namespace); - savedObjectNamespaces = initialNamespaces || existingNamespaces; + savedObjectNamespaces = await this.preflightGetNamespaces( + type, + id, + namespace, + initialNamespaces + ); } else { savedObjectNamespaces = initialNamespaces || getSavedObjectNamespaces(namespace); } @@ -453,8 +457,14 @@ export class SavedObjectsRepository { const indexFound = bulkGetResponse?.statusCode !== 404; const actualResult = indexFound ? bulkGetResponse?.body.docs[esRequestIndex] : undefined; const docFound = indexFound && actualResult?.found === true; - // @ts-expect-error MultiGetHit._source is optional - if (docFound && !this.rawDocExistsInNamespace(actualResult!, namespace)) { + if ( + docFound && + !this.rawDocExistsInNamespaces( + // @ts-expect-error MultiGetHit._source is optional + actualResult!, + initialNamespaces ?? [SavedObjectsUtils.namespaceIdToString(namespace)] + ) + ) { const { id, type } = object; return { tag: 'Left' as 'Left', @@ -2136,12 +2146,18 @@ export class SavedObjectsRepository { * @param type The type of the saved object. * @param id The ID of the saved object. * @param namespace The target namespace. + * @param initialNamespaces The target namespace(s) we intend to create the object in, if specified. * @returns Array of namespaces that this saved object currently includes, or (if the object does not exist yet) the namespaces that a * newly-created object will include. Value may be undefined if an existing saved object has no namespaces attribute; this should not * happen in normal operations, but it is possible if the Elasticsearch document is manually modified. * @throws Will throw an error if the saved object exists and it does not include the target namespace. */ - private async preflightGetNamespaces(type: string, id: string, namespace?: string) { + private async preflightGetNamespaces( + type: string, + id: string, + namespace: string | undefined, + initialNamespaces?: string[] + ) { if (!this._registry.isMultiNamespace(type)) { throw new Error(`Cannot make preflight get request for non-multi-namespace type '${type}'.`); } @@ -2156,17 +2172,19 @@ export class SavedObjectsRepository { } ); + const namespaces = initialNamespaces ?? [SavedObjectsUtils.namespaceIdToString(namespace)]; + const indexFound = statusCode !== 404; if (indexFound && isFoundGetResponse(body)) { - if (!this.rawDocExistsInNamespace(body, namespace)) { + if (!this.rawDocExistsInNamespaces(body, namespaces)) { throw SavedObjectsErrorHelpers.createConflictError(type, id); } - return getSavedObjectNamespaces(namespace, body); + return initialNamespaces ?? getSavedObjectNamespaces(namespace, body); } else if (isNotFoundFromUnsupportedServer({ statusCode, headers })) { // checking if the 404 is from Elasticsearch throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id); } - return getSavedObjectNamespaces(namespace); + return initialNamespaces ?? getSavedObjectNamespaces(namespace); } /**