Remove pass discriminator to children setting

[alzimmermsft]

Removes the pass-discriminator-to-child-deserialization setting as the discriminator is now always passed to children models.

[weidongxu-microsoft]

This is a wide API change? It adds a getter to all of these models. There would be hundreds of them in mgmt.

Also there appear to be a bug with this, when serialize it to JSON #2411

[weidongxu-microsoft]

And I do not see why "pass to children" is the default.

If user does not use this getter, it does not need to be there. So far, no mgmt user asks for it.

[alzimmermsft]

That's good feedback @weidongxu-microsoft.

My thinking is this feature is very confusing and not well used. It'd be better for serialization to always have the discriminator property in the model so it isn't lost, which could happen when we're deserializing an unknown polymorphic type. And I agree, adding a getter everywhere won't be great but this may be nice to just have so our models aren't as mysterious. But if it's going to add a lot of noise, I think we can still remove the current setting and add a much clearer setting hide-polymorphic-getter which can default to true.

[weidongxu-microsoft]

Yeah, maybe its fine. Please get a sign off from Srikanta as well, because it affect API shape of all polymorphic classes (I see it already applied to stream serialization, hence there is few changes in test module now -- the remaining Jackson ones).

I see now the getter is added to the base class, not the terminal class. Again, no major objection.

The current CI error may relate to the issue I've linked.

Mgmt would generate model test cases like this
https://github.com/Azure/azure-sdk-for-java/blob/main/sdk/security/azure-resourcemanager-security/src/test/java/com/azure/resourcemanager/security/generated/ContainerRegistryVulnerabilityPropertiesTests.java

And it seems the test of the serialization fails with the Jackson form of this.

[weidongxu-microsoft]

Given what's merged to main branch, we now having another issue (that we need to solve before next typespec-java release)

tsp https://github.com/Azure/cadl-ranch/blob/main/packages/cadl-ranch-specs/http/type/model/inheritance/enum-discriminator/main.tsp#L12-L16

But code only return a String on getKind https://github.com/Azure/autorest.java/blob/main/typespec-tests/src/main/java/com/type/model/inheritance/enumdiscriminator/models/Dog.java#L49-L51

It should return a DogKind as in tsp.

I would go figure it out way sometime later as this is typespec. And the cause likely in typescript part.
#2571

---

It may apply to Swagger as well, since the kind there could also be an enum.

[alzimmermsft]

Given what's merged to main branch, we now having another issue (that we need to solve before next typespec-java release)

tsp https://github.com/Azure/cadl-ranch/blob/main/packages/cadl-ranch-specs/http/type/model/inheritance/enum-discriminator/main.tsp#L12-L16

But code only return a String on getKind https://github.com/Azure/autorest.java/blob/main/typespec-tests/src/main/java/com/type/model/inheritance/enumdiscriminator/models/Dog.java#L49-L51

It should return a DogKind as in tsp.

I would go figure it out way sometime later as this is typespec. And the cause likely in typescript part. #2571

It may apply to Swagger as well, since the kind there could also be an enum.

It looks like this does get handled as expected, returning a DogKind. It looks to be breaking as the old value was String, but is that due to a change in the tsp or due to code generation changes?

[srnagar]

I think it's okay to remove this option and always have the getter for the discriminator property added to the base type of polymorphic models. Having this getter is quite useful for troubleshooting and in general represents the service response more accurately. .NET always makes the discriminator visible. So, this will add parity in Java.

While this may cause a lot of classes in mgmt plane to be updated with a getter, it's not a breaking change and is a read-only method.

[weidongxu-microsoft]

It looks like this does get handled as expected, returning a DogKind. It looks to be breaking as the old value was String, but is that due to a change in the tsp or due to code generation changes?

It is due to change in codegen #2571 which I did last Friday.

It won't break SDK or customer. The getKind API would not be generated in any released typespec-java (and no flag exists), so no one would see the getKind and its type (and there is no need to expose the DogKind if no one see the getter).

I just find that the enabling of stream serialization also enable this "getter", so I had to make the type correct (without the getter, type does not matter).

[weidongxu-microsoft]

also some error in test code gen
https://github.com/weidongxu-microsoft/autorest.java_ci/actions/runs/8039220851/job/21956004744#step:11:6672

I will have Xiaofei take a look. #2574

[weidongxu-microsoft]

I've check the Swagger case, it would also generate the XXXKind class (via autorest), if that is an enum
https://cadlplayground.z22.web.core.windows.net/cadl-azure/?c=aW1wb3J0ICJAdHlwZXNwZWMvcmVzdCI7Cgp1c2luZyBUeXBlU3BlYy5IdHRwOwoKQHN1bW1hcnkoIscJIG9mIE5hbWluZyIpCkBkb2MoImRlc2NyaXB0aW9uzh5zZXJ2aWNlKHsKICB0aXRsZTogIsceLAp9KQpuYW1lc3BhY2UgQ2FkbC7GG%2BQAi27ES0RhdGFLaW5kIMQ%2Fc3RyaW5nLAogIGJ5dGVzOiAixQjESAoKQGRp5ACCbWluYXRvcigia2luZCIpCm1vZGVsxUfFQ8QWOslWO8Q5xiJCxErFJ2V4dGVuZHPYNC7FdDsKCiAgZGF0YTrmAIzFSkByb3V0ZSgiL27oAP5pbnRlcmbkAN3mANhPcMVSZ2V0KCnGU8Q5&e=%40azure-tools%2Ftypespec-autorest&options=%7B%7D

So we are good on parity of TypeSpec and Swagger (mgmt needs this) @XiaofeiCao

see #2599