expose retry_on_conflict for SOR.update (#131371)

* expose `retry_on_conflict` for `SOR.update`

* update generated doc

* stop using preflight check for version check for other methods too.

* remove unused ignore
This commit is contained in:
Pierre Gayvallet 2022-05-18 16:27:10 +02:00 committed by GitHub
parent 3409ea325f
commit 31bb2c7fc5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 59 additions and 33 deletions

View file

@ -18,6 +18,7 @@ export interface SavedObjectsUpdateOptions<Attributes = unknown> extends SavedOb
| --- | --- | --- |
| [references?](./kibana-plugin-core-server.savedobjectsupdateoptions.references.md) | SavedObjectReference\[\] | <i>(Optional)</i> A reference to another saved object. |
| [refresh?](./kibana-plugin-core-server.savedobjectsupdateoptions.refresh.md) | MutatingOperationRefreshSetting | <i>(Optional)</i> The Elasticsearch Refresh setting for this operation |
| [retryOnConflict?](./kibana-plugin-core-server.savedobjectsupdateoptions.retryonconflict.md) | number | <i>(Optional)</i> The Elasticsearch <code>retry_on_conflict</code> setting for this operation. Defaults to <code>0</code> when <code>version</code> is provided, <code>3</code> otherwise. |
| [upsert?](./kibana-plugin-core-server.savedobjectsupdateoptions.upsert.md) | Attributes | <i>(Optional)</i> If specified, will be used to perform an upsert if the document doesn't exist |
| [version?](./kibana-plugin-core-server.savedobjectsupdateoptions.version.md) | string | <i>(Optional)</i> An opaque version number which changes on each successful write operation. Can be used for implementing optimistic concurrency control. |

View file

@ -1688,20 +1688,6 @@ describe('SavedObjectsRepository', () => {
);
});
it(`defaults to the version of the existing document for multi-namespace types`, async () => {
// only multi-namespace documents are obtained using a pre-flight mget request
const objects = [
{ ...obj1, type: MULTI_NAMESPACE_ISOLATED_TYPE },
{ ...obj2, type: MULTI_NAMESPACE_ISOLATED_TYPE },
];
await bulkUpdateSuccess(objects);
const overrides = {
if_seq_no: mockVersionProps._seq_no,
if_primary_term: mockVersionProps._primary_term,
};
expectClientCallArgsAction(objects, { method: 'update', overrides });
});
it(`defaults to no version for types that are not multi-namespace`, async () => {
const objects = [obj1, { ...obj2, type: NAMESPACE_AGNOSTIC_TYPE }];
await bulkUpdateSuccess(objects);
@ -1759,12 +1745,6 @@ describe('SavedObjectsRepository', () => {
it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => {
const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix)
const overrides = {
// bulkUpdate uses a preflight `get` request for multi-namespace saved objects, and specifies that version on `update`
// we aren't testing for this here, but we need to include Jest assertions so this test doesn't fail
if_primary_term: expect.any(Number),
if_seq_no: expect.any(Number),
};
const _obj1 = { ...obj1, type: NAMESPACE_AGNOSTIC_TYPE };
const _obj2 = { ...obj2, type: MULTI_NAMESPACE_ISOLATED_TYPE };
@ -1772,7 +1752,7 @@ describe('SavedObjectsRepository', () => {
expectClientCallArgsAction([_obj1], { method: 'update', getId });
client.bulk.mockClear();
await bulkUpdateSuccess([_obj2], { namespace });
expectClientCallArgsAction([_obj2], { method: 'update', getId, overrides });
expectClientCallArgsAction([_obj2], { method: 'update', getId });
jest.clearAllMocks();
// test again with object namespace string that supersedes the operation's namespace ID
@ -1780,7 +1760,7 @@ describe('SavedObjectsRepository', () => {
expectClientCallArgsAction([_obj1], { method: 'update', getId });
client.bulk.mockClear();
await bulkUpdateSuccess([{ ..._obj2, namespace }]);
expectClientCallArgsAction([_obj2], { method: 'update', getId, overrides });
expectClientCallArgsAction([_obj2], { method: 'update', getId });
});
});
@ -2723,14 +2703,14 @@ describe('SavedObjectsRepository', () => {
expect(client.delete).toHaveBeenCalledTimes(1);
});
it(`includes the version of the existing document when using a multi-namespace type`, async () => {
it(`does not includes the version of the existing document when using a multi-namespace type`, async () => {
await deleteSuccess(MULTI_NAMESPACE_ISOLATED_TYPE, id);
const versionProperties = {
if_seq_no: mockVersionProps._seq_no,
if_primary_term: mockVersionProps._primary_term,
};
expect(client.delete).toHaveBeenCalledWith(
expect.objectContaining(versionProperties),
expect.not.objectContaining(versionProperties),
expect.anything()
);
});
@ -4605,14 +4585,14 @@ describe('SavedObjectsRepository', () => {
);
});
it(`defaults to the version of the existing document when type is multi-namespace`, async () => {
it(`does not default to the version of the existing document when type is multi-namespace`, async () => {
await updateSuccess(MULTI_NAMESPACE_ISOLATED_TYPE, id, attributes, { references });
const versionProperties = {
if_seq_no: mockVersionProps._seq_no,
if_primary_term: mockVersionProps._primary_term,
};
expect(client.update).toHaveBeenCalledWith(
expect.objectContaining(versionProperties),
expect.not.objectContaining(versionProperties),
expect.anything()
);
});
@ -4627,6 +4607,35 @@ describe('SavedObjectsRepository', () => {
);
});
it('default to a `retry_on_conflict` setting of `3` when `version` is not provided', async () => {
await updateSuccess(type, id, attributes, {});
expect(client.update).toHaveBeenCalledWith(
expect.objectContaining({ retry_on_conflict: 3 }),
expect.anything()
);
});
it('default to a `retry_on_conflict` setting of `0` when `version` is provided', async () => {
await updateSuccess(type, id, attributes, {
version: encodeHitVersion({ _seq_no: 100, _primary_term: 200 }),
});
expect(client.update).toHaveBeenCalledWith(
expect.objectContaining({ retry_on_conflict: 0, if_seq_no: 100, if_primary_term: 200 }),
expect.anything()
);
});
it('accepts a `retryOnConflict` option', async () => {
await updateSuccess(type, id, attributes, {
version: encodeHitVersion({ _seq_no: 100, _primary_term: 200 }),
retryOnConflict: 42,
});
expect(client.update).toHaveBeenCalledWith(
expect.objectContaining({ retry_on_conflict: 42, if_seq_no: 100, if_primary_term: 200 }),
expect.anything()
);
});
it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => {
await updateSuccess(type, id, attributes, { namespace });
expect(client.update).toHaveBeenCalledWith(

View file

@ -151,6 +151,7 @@ export interface SavedObjectsDeleteByNamespaceOptions extends SavedObjectsBaseOp
}
export const DEFAULT_REFRESH_SETTING = 'wait_for';
export const DEFAULT_RETRY_COUNT = 3;
/**
* See {@link SavedObjectsRepository}
@ -523,7 +524,7 @@ export class SavedObjectsRepository {
}
savedObjectNamespaces =
initialNamespaces || getSavedObjectNamespaces(namespace, existingDocument);
versionProperties = getExpectedVersionProperties(version, existingDocument);
versionProperties = getExpectedVersionProperties(version);
} else {
if (this._registry.isSingleNamespace(object.type)) {
savedObjectNamespace = initialNamespaces
@ -761,7 +762,7 @@ export class SavedObjectsRepository {
{
id: rawId,
index: this.getIndexForType(type),
...getExpectedVersionProperties(undefined, preflightResult?.rawDocSource),
...getExpectedVersionProperties(undefined),
refresh,
},
{ ignore: [404], meta: true }
@ -1312,7 +1313,13 @@ export class SavedObjectsRepository {
throw SavedObjectsErrorHelpers.createBadRequestError('id cannot be empty'); // prevent potentially upserting a saved object with an empty ID
}
const { version, references, upsert, refresh = DEFAULT_REFRESH_SETTING } = options;
const {
version,
references,
upsert,
refresh = DEFAULT_REFRESH_SETTING,
retryOnConflict = version ? 0 : DEFAULT_RETRY_COUNT,
} = options;
const namespace = normalizeNamespace(options.namespace);
let preflightResult: PreflightCheckNamespacesResult | undefined;
@ -1373,8 +1380,9 @@ export class SavedObjectsRepository {
.update<unknown, unknown, SavedObjectsRawDocSource>({
id: this._serializer.generateRawId(namespace, type, id),
index: this.getIndexForType(type),
...getExpectedVersionProperties(version, preflightResult?.rawDocSource),
...getExpectedVersionProperties(version),
refresh,
retry_on_conflict: retryOnConflict,
body: {
doc,
...(rawUpsert && { upsert: rawUpsert._source }),
@ -1608,8 +1616,7 @@ export class SavedObjectsRepository {
// @ts-expect-error MultiGetHit is incorrectly missing _id, _source
SavedObjectsUtils.namespaceIdToString(actualResult!._source.namespace),
];
// @ts-expect-error MultiGetHit is incorrectly missing _id, _source
versionProperties = getExpectedVersionProperties(version, actualResult!);
versionProperties = getExpectedVersionProperties(version);
} else {
if (this._registry.isSingleNamespace(type)) {
// if `objectNamespace` is undefined, fall back to `options.namespace`

View file

@ -221,7 +221,10 @@ export interface SavedObjectsCheckConflictsResponse {
* @public
*/
export interface SavedObjectsUpdateOptions<Attributes = unknown> extends SavedObjectsBaseOptions {
/** An opaque version number which changes on each successful write operation. Can be used for implementing optimistic concurrency control. */
/**
* An opaque version number which changes on each successful write operation.
* Can be used for implementing optimistic concurrency control.
*/
version?: string;
/** {@inheritdoc SavedObjectReference} */
references?: SavedObjectReference[];
@ -229,6 +232,11 @@ export interface SavedObjectsUpdateOptions<Attributes = unknown> extends SavedOb
refresh?: MutatingOperationRefreshSetting;
/** If specified, will be used to perform an upsert if the document doesn't exist */
upsert?: Attributes;
/**
* The Elasticsearch `retry_on_conflict` setting for this operation.
* Defaults to `0` when `version` is provided, `3` otherwise.
*/
retryOnConflict?: number;
}
/**

View file

@ -2939,6 +2939,7 @@ export interface SavedObjectsUpdateObjectsSpacesResponseObject {
export interface SavedObjectsUpdateOptions<Attributes = unknown> extends SavedObjectsBaseOptions {
references?: SavedObjectReference[];
refresh?: MutatingOperationRefreshSetting;
retryOnConflict?: number;
upsert?: Attributes;
version?: string;
}