Rename update function in saved object client and provide index function

[flash1293]

The saved object client includes an update function which uses the update Elasticsearch API to update the saved object document. The semantics of update are to merge the passed in parameter into the existing saved object, leaving all keys left out of the update in their current state. When a user is unaware of this, usage of update can cause corrupted saved objects (e.g. #74483 )

This is a valid and documented behavior (at least in Elasticsearch), but it's surprising for update on the saved object client to behave like this - from the interface it can be assumed update will completely replace the document:

  public update<T = unknown>(
    type: string,
    id: string,
    attributes: T,
    { version, migrationVersion, references }: SavedObjectsUpdateOptions = {}
  ): Promise<SimpleSavedObject<T>> {

There is just a single type parameter for the return value and the attributes to update. Also, IMHO in the context of a simple key/value object store like saved object client, update is often a replacement operation. Personally I don't know of any place in Kibana where update is used with just a subset of its attributes. Pretty sure it is used this way in some places, but the more common use case is definitely to pass in the whole document.

Currently it is not possible to completely replace an object - the index API in Elasticsearch would provide this functionality, but it's not exposed in the saved object client.

I can think of two ways how to improve the current state, but I'm pretty sure there are others. Leaving them here as a first idea.

Option 1 (non-breaking)

• Introduce a new method index to have an easy way to replace a whole object
• Make it clear in documentation and types what update is doing

This would have the advantages of staying close to Elasticsearch semantics and not breaking existing usage

Option 2 (breaking)

• Rename update to merge
• Introduce update method using index Elasticsearch API internally

This would have the advantage of staying close to the "expected" behavior of a key/value store, but that's just a personal opinion.

[elasticmachine]

Pinging @elastic/kibana-platform (Team:Platform)

[mshustov]

another complaint about the semantic of the method #71995 (comment)

[pgayvallet]

Note that the client-side signature of update is misleadingly declaring attributes: T instead of Partial<T>:

kibana/src/core/server/saved_objects/service/saved_objects_client.ts

Lines 382 to 389 in 0a02ac1

	async update<T = unknown>(
	type: string,
	id: string,
	attributes: Partial<T>,
	options: SavedObjectsUpdateOptions = {}
	): Promise<SavedObjectsUpdateResponse<T>> {
	return await this._repository.update(type, id, attributes, options);
	}

kibana/src/core/public/saved_objects/saved_objects_client.ts

Lines 403 to 408 in caa5da2

	public update<T = unknown>(
	type: string,
	id: string,
	attributes: T,
	{ version, migrationVersion, references }: SavedObjectsUpdateOptions = {}
	): Promise<SimpleSavedObject<T>> {

[rudolf]

Note: it's currently possible to create an object with overwrite=true which will use the Elasticsearch Index API under the hood.

I think the HTTP PATCH verb would have better described this API, but like the issue points out, there's a tension between what a developer expects the Saved Objects API to do and how the Elasticsearch API's behave.

We should also clarify this behaviour in the HTTP API docs: https://github.com/elastic/kibana/blob/master/docs/api/saved-objects/update.asciidoc

Given the conversation in #84729 I'm inclined to say long term we should let Saved Objects be just a thin powerful wrapper around Elasticsearch (and not a simple key value store abstraction). That way you don't need to learn Saved Objects AND Elasticsearch, for the most part you just need to understand how Elasticsearch works and understand the kibana-specific concepts of "spaces", "saved object types" and "references".