tsp, json-merge-patch, "kind" not set to JSON for polymorphic class

[weidongxu-microsoft]

Background

https://github.com/Azure/autorest.java/pull/2576#discussion_r1504012383

Resource resource = new Resource();
Fish salmon = new Salmon().setAge(1);
Assertions.assertEquals("salmon", salmon.getKind());
resource.setFish(salmon);
JsonMergePatchHelper.getResourceAccessor().prepareModelForJsonMergePatch(resource, true);
Fish shark = new Shark().setAge(2);
shark.setColor(null);
resource.setFish(shark);
// here when serialize for resource.fish, it calls shark.toJsonMergePatch(), but kind is not added to shark's updatedProperties, but to fish's updated properties.
+ String json = BinaryData.fromObject(resource).toString();
Assertions.assertEquals("shark", node.get("fish").get("kind"));

Shark has updatedProperties which shadows Fish's updatedProperties. And in the ctor of Shark, it calls Fish's setKind(), which updated Fish's updatedProperties. In the highlighted line in above code snippet, when serialize for resource.fish, it calls shark.toJsonMergePatch(), but as kind is not added to shark's updatedProperties, but to fish's updated properties. So kind is not serialized.

test: https://github.com/Azure/autorest.java/tree/main/typespec-tests/src/main/java/com/cadl/patch/models service using this: communication jobrouter

Design

Option1:

Make updatedProperties from base class as protected, and remove override updatedProperties from sub class. Thus this.updatedProperties in sub class will get base classs' updatedProperties.

Pros: Implementation is easy Cons: Once we make updatedProperties as protected, we can't change back.

Option2:

Check both base class and sub class's updatedProperties in sub class's toJsonMergePatch() logic.

Pros: Will not change the current class's public APIs. Cons: Implementation is a bit complex, we will need to check all the super classes' updatedProperties.

Option3:

Add kind(private) property, setKind()(private) and getKind()(public) method to sub class, and when calling setKind method, it will update sub class's updatedProperties.

Pros: generated code's logic is clear and simple, does not need to consider about the shaddowed property thing. Cons: We will need to add APIs to sub class. kind appears will both in sub class and base class.

Personaly I would prefer option 1, as we already make kind in parent class protected. Guess we can also make updatedProperties as `protected.

[weidongxu-microsoft]

package private won't work, as class could be in different package.

[weidongxu-microsoft]

option3, just add this.updatedProperties.add("kind"); to subclass ctor, after call superclass setKind.

[haolingdong-msft]

Final decision in 2024.3.1 Meeting:

In the subclass, e.g. Shark, we add updatedProperties.add("kind") to the ctor. Code like below:

  /**
   * Creates an instance of Shark class.
   */
  @Generated
  public Shark() {
     this.updatedProperties.add("kind");
     setKind("shark");
  }