maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[SOR] use initialNamespaces when checking for conflict for create and bulkCreate (#111023)

Browse source 
* use initialNamespaces when checking for conflict

* nits
This commit is contained in:
Pierre Gayvallet 2021-09-03 08:27:18 +02:00 committed by GitHub
parent 8f728977f2
commit 9d216cd312
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 104 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

81
src/core/server/saved_objects/service/lib/repository.test.js
View file

@ -197,7 +197,16 @@ 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];
return {
// NOTE: Elasticsearch returns more fields (_index, _type) but the SavedObjectsRepository method ignores these
found: true,
@ -207,7 +216,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 +228,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({
@ -797,6 +808,54 @@ describe('SavedObjectsRepository', () => {
});
});
it(`returns error when there is an unresolvable conflict with an existing multi-namespace saved object when using 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);
@ -2197,6 +2256,22 @@ describe('SavedObjectsRepository', () => {
expect(client.get).toHaveBeenCalled();
});
it(`throws when there is an unresolvable 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`);

34
src/core/server/saved_objects/service/lib/repository.ts
View file

@ -308,8 +308,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);
}
@ -455,8 +459,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',
@ -2140,12 +2150,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}'.`);
}
@ -2160,17 +2176,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);
}
/**