AttributeService API deviates from Saved Object client API

[nreese]

Saved Object client API places attributes and references as siblings.

AttributeService API places references as a child to attributes.

AttributeService API usage of references as a child to attributes deviates from what is expected by the saved object client and makes using the AttributeService API much harder then it should be.

I would recommend changing the AttributeService API so that references is passed in as a top level argument that is a sibling to attributes so its more similar to saved object client API.

Details

The wrapAttributes function is defined as

public async wrapAttributes(
    newAttributes: SavedObjectAttributes,
    useRefType: boolean,
    input?: ValType | RefType
  ): Promise<Omit<ValType | RefType, 'id'>> {

Consumers of the API put references in newAttributes. For example, lens puts "reference" into "newAttributes" as part of runSave function.

    let references = lastKnownDoc.references;
    if (savedObjectsTagging && saveProps.newTags) {
      references = savedObjectsTagging.ui.updateTagsReferences(references, saveProps.newTags);
    }

    const docToSave = {
      ...getLastKnownDocWithoutPinnedFilters()!,
      description: saveProps.newDescription,
      title: saveProps.newTitle,
      references,
    };

      const newInput = (await attributeService.wrapAttributes(
        docToSave,
        options.saveToLibrary,
        originalInput
      )) as LensEmbeddableInput;

Then, in the depths of lens, weird stuff like below has to happen when working with the saved objects client. Its very confusing that attributes means different things in different APIs.

  save = async (vis: Document) => {
    const { savedObjectId, type, references, ...rest } = vis;
    // TODO: SavedObjectAttributes should support this kind of object,
    // remove this workaround when SavedObjectAttributes is updated.
    const attributes = (rest as unknown) as SavedObjectAttributes;

    const result = await (savedObjectId
      ? this.safeUpdate(savedObjectId, attributes, references)
      : this.client.create(DOC_TYPE, attributes, {
          references,
        }));

    return { ...vis, savedObjectId: result.id };
  };

cc @ThomThomson @flash1293

[flash1293]

+1 for establishing a common wording for what "attributes" mean exactly. I'm fine with following saved object clients lead here.

[ThomThomson]

As discussed with @nreese on zoom - The initial reason the API is structured this way was to work more easily with the lens Document type, which is essentially all of the attribute keys, plus the type, references etc. all in one. That said, since the Attribute Service is a common element, its API should absolutely be changed to be more in line with what is expected / standard in the saved object landscape.

My plan going forward is to change the API to match, and just do a little bit more fiddling with the lens_attribute_service to make the new API shape work with the Document type.

Longer term though, I think it would be helpful for us to replace the Document type with something more standard - like using the LensByReferenceInput - or some sort of LensEmbeddablePackage with Attributes, type, savedObjectId, and References, all as siblings.

@flash1293 - how feasible does that sound to you?

[flash1293]

@ThomThomson I did a quick code search and I don't think it would be a big issue to change the nesting of the Document type to put actual attributes and references side by side instead of one nested within the other (which is essentially what this would result in if I'm understanding correctly) as Document objects are unpacked relatively low in the call stack anyway.

[ThomThomson]

Great news, thanks for looking into that! Maybe it would be good to combine both changes into one PR. I am pretty swamped for the 7.11.0 timeframe, but might be able to squeeze this in.

[elasticmachine]

Pinging @elastic/kibana-app (Team:KibanaApp)