Skip to content

Commit a61e908

Browse files
XiaofeiCaoCopilotCopilot
authored
Fix duplicate inherited discriminators in Java models (#11824)
## Summary - merge redundant leaf discriminator declarations with propagated parent discriminators - validate that duplicate declarations have matching constant values and Java types - emit fixed propagated discriminator fields as `final` without changing shared serialization behavior - regenerate Java clientcore and generator-test golden models Fixes #11802 ## Testing - Java generator build - Java emitter build - generator core tests (53 passed) - full `http-client-generator-clientcore-test/Generate.ps1` regeneration - full `http-client-generator-test/Generate.ps1` regeneration - generated the Foundry Agents service spec from #11802, including a partial-update pass, and compiled all 496 Java sources with Java 17 - Java formatting and lint checks --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Copilot-Session: 54b93dc2-48ce-4250-a6ea-5bd9b9a7ab2b Copilot-Session: 00e7fd30-d342-4b5a-ad7e-f16198c7dbde Copilot-Session: e6933b7d-0cee-4855-a585-2e15b24c70c5 Copilot-Session: d4e8259f-d938-4448-9321-8eaf17f8b53d
1 parent 7abc933 commit a61e908

44 files changed

Lines changed: 1940 additions & 65 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
changeKind: fix
3+
packages:
4+
- "@typespec/http-client-java"
5+
---
6+
7+
Prevent duplicate Java discriminator members while preserving inherited discriminators in stream-style XML serialization.

packages/http-client-java/generator/http-client-generator-clientcore-test/src/main/java/type/model/inheritance/nesteddiscriminator/GoblinShark.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public final class GoblinShark extends Shark {
1616
* Discriminator property for Fish.
1717
*/
1818
@Metadata(properties = { MetadataProperties.GENERATED })
19-
private String kind = "shark";
19+
private final String kind = "shark";
2020

2121
/*
2222
* The sharktype property.

packages/http-client-java/generator/http-client-generator-clientcore-test/src/main/java/type/model/inheritance/nesteddiscriminator/SawShark.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public final class SawShark extends Shark {
1616
* Discriminator property for Fish.
1717
*/
1818
@Metadata(properties = { MetadataProperties.GENERATED })
19-
private String kind = "shark";
19+
private final String kind = "shark";
2020

2121
/*
2222
* The sharktype property.

packages/http-client-java/generator/http-client-generator-clientcore-test/src/main/java/type/model/inheritance/nesteddiscriminator/Shark.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public class Shark extends Fish {
1616
* Discriminator property for Fish.
1717
*/
1818
@Metadata(properties = { MetadataProperties.GENERATED })
19-
private String kind = "shark";
19+
private final String kind = "shark";
2020

2121
/*
2222
* The sharktype property.

packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/implementation/ClientModelPropertiesManager.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.function.BiConsumer;
2323
import java.util.function.Consumer;
2424
import java.util.stream.Collectors;
25+
import java.util.stream.Stream;
2526

2627
/**
2728
* Manages metadata about properties in a {@link ClientModel} and how they correlate with model class generation.
@@ -124,10 +125,11 @@ public ClientModelPropertiesManager(ClientModel model, JavaSettings settings) {
124125
xmlRootElementNamespace = model.getXmlNamespace();
125126
}
126127

127-
Set<String> thisModelPropertySerializeNames = model.getProperties()
128-
.stream()
128+
Set<String> thisModelPropertySerializeNames = Stream.concat(
129129
// discriminator property is known to be redefined in subclass
130-
.filter(property -> !property.isPolymorphicDiscriminator())
130+
model.getProperties().stream().filter(property -> !property.isPolymorphicDiscriminator()),
131+
// Canonicalized parent discriminators mask inherited ordinary properties with the same wire name.
132+
model.getParentPolymorphicDiscriminators().stream())
131133
.map(ClientModelProperty::getSerializedName)
132134
.filter(name -> Objects.nonNull(name) && !name.isEmpty())
133135
.collect(Collectors.toSet());

packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/implementation/PolymorphicDiscriminatorHandler.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import com.microsoft.typespec.http.client.generator.core.model.javamodel.JavaFile;
1313
import com.microsoft.typespec.http.client.generator.core.model.javamodel.JavaVisibility;
1414
import com.microsoft.typespec.http.client.generator.core.util.ClientModelUtil;
15+
import java.util.Objects;
1516
import java.util.function.Consumer;
1617
import java.util.function.Function;
1718

@@ -116,7 +117,13 @@ private static void declareFieldInternal(ClientModelProperty discriminator, Clie
116117
&& settings.isShareJsonSerializableCode()) {
117118
classBlock.memberVariable(JavaVisibility.PackagePrivate, fieldSignature);
118119
} else if (!allPolymorphicModelsInSamePackage || !settings.isShareJsonSerializableCode()) {
119-
classBlock.privateMemberVariable(fieldSignature);
120+
// Fixed inherited parent discriminators are final; active discriminators remain mutable for fallback.
121+
if (discriminator.isConstant()
122+
&& !Objects.equals(discriminator.getSerializedName(), model.getPolymorphicDiscriminatorName())) {
123+
classBlock.privateFinalMemberVariable(fieldSignature);
124+
} else {
125+
classBlock.privateMemberVariable(fieldSignature);
126+
}
120127
}
121128
}
122129
}

packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/mapper/ModelMapper.java

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.Collection;
2929
import java.util.LinkedHashSet;
3030
import java.util.List;
31+
import java.util.ListIterator;
3132
import java.util.Objects;
3233
import java.util.Set;
3334
import java.util.function.Function;
@@ -355,19 +356,15 @@ public ClientModel map(ObjectSchema compositeType) {
355356
result = builder.build();
356357

357358
if (isPolymorphic && !CoreUtils.isNullOrEmpty(derivedTypes)) {
358-
// Walk the polymorphic hierarchy finding places where the parent model and child model have different
359-
// polymorphic discriminators. When this case is found add the parent polymorphic discriminator as a
360-
// parent
361-
// polymorphic discriminator to the child model. This is necessary to ensure that the child model
362-
// generates
363-
// the correct serialization in multi-level polymorphic structures.
359+
// Preserve the fixed outer discriminator when a child starts a nested discriminator hierarchy.
364360
for (ClientModel derivedType : derivedTypes) {
365361
if (!Objects.equals(polymorphicDiscriminator, derivedType.getPolymorphicDiscriminatorName())) {
366362
ClientModelProperty parentDiscriminator = result.getPolymorphicDiscriminator()
367363
.newBuilder()
368364
.defaultValue(result.getPolymorphicDiscriminator()
369365
.getClientType()
370366
.defaultValueExpression(derivedType.getSerializedName()))
367+
.constant(true)
371368
.build();
372369

373370
passPolymorphicDiscriminatorToChildren(parentDiscriminator, derivedType);
@@ -381,14 +378,60 @@ public ClientModel map(ObjectSchema compositeType) {
381378
return result;
382379
}
383380

381+
/**
382+
* Propagates a fixed discriminator from an outer hierarchy through a nested discriminator hierarchy.
383+
* <p>
384+
* The {@code parentDiscriminator} is the fixed outer selection, while {@code child}'s discriminator remains the
385+
* active discriminator for its own descendants. For example, given an outer {@code type} discriminator, a
386+
* {@code type="message"} child that introduces {@code role}, and a {@code role="assistant"} grandchild, both nested
387+
* models retain the canonical {@code type="message"} value while {@code role} controls nested dispatch.
388+
* <p>
389+
* A child can also declare an ordinary fixed property with the same wire name as the propagated discriminator.
390+
* Keeping both representations would generate duplicate fields and accessors. Such a property must match
391+
* {@code parentDiscriminator}'s Java name, wire type, client type, and fixed value, and must be constant; otherwise
392+
* mapping fails. A valid property is removed from {@link ClientModel#getProperties()}, leaving
393+
* {@code parentDiscriminator} as the canonical entry in
394+
* {@link ClientModel#getParentPolymorphicDiscriminators()}.
395+
* <p>
396+
* Parent models map after their children, so the canonical entry is inserted at index zero to retain
397+
* outer-to-inner discriminator order. The fixed discriminator is then recursively propagated to every descendant.
398+
* For current stream-style JSON, same-package hierarchies may serialize this metadata through shared
399+
* {@code toJsonShared} code. When hierarchy models are in different packages, or sharing is disabled, each model
400+
* serializes inherited discriminator metadata through {@code serializeParentJsonProperties}, which consumes
401+
* {@link ClientModel#getParentPolymorphicDiscriminators()}.
402+
*
403+
* @param parentDiscriminator the fixed discriminator selected by the outer hierarchy
404+
* @param child the nested-hierarchy model that receives the fixed discriminator
405+
* @throws IllegalStateException if the child declares the same wire name without matching constant status, Java
406+
* name, wire type, client type, and fixed value
407+
*/
384408
private static void passPolymorphicDiscriminatorToChildren(ClientModelProperty parentDiscriminator,
385409
ClientModel child) {
386-
// Due to the execution order of ModelMapper, where children models complete mapping before the parent model,
387-
// the parent polymorphic discriminator needs to be added at index 0. Reason, given an example where there are
388-
// three models, where model #1 is the root parent with discriminator type, model #2 is a child of model #2 with
389-
// discriminator kind, and model #3 is a child of model #3 with discriminator form. The order if this running
390-
// will have model #2 add its discriminator to model #3 before model #1 runs adding its discriminator to #2 and
391-
// #3. We want #3 to have the ordering of [type, kind], to represent the ordering of the parent models.
410+
ListIterator<ClientModelProperty> iterator = child.getProperties().listIterator();
411+
while (iterator.hasNext()) {
412+
ClientModelProperty childProperty = iterator.next();
413+
if (!Objects.equals(parentDiscriminator.getSerializedName(), childProperty.getSerializedName())) {
414+
continue;
415+
}
416+
417+
if (!childProperty.isConstant()
418+
|| !Objects.equals(parentDiscriminator.getName(), childProperty.getName())
419+
|| !Objects.equals(parentDiscriminator.getWireType(), childProperty.getWireType())
420+
|| !Objects.equals(parentDiscriminator.getClientType(), childProperty.getClientType())
421+
|| !Objects.equals(parentDiscriminator.getDefaultValue(), childProperty.getDefaultValue())) {
422+
throw new IllegalStateException("Property '" + childProperty.getSerializedName() + "' on model '"
423+
+ child.getName() + "' does not match its inherited polymorphic discriminator. Expected (name="
424+
+ parentDiscriminator.getName() + ", constant=true, type=" + parentDiscriminator.getClientType()
425+
+ ", value=" + String.valueOf(parentDiscriminator.getDefaultValue()) + "), but found (name="
426+
+ childProperty.getName() + ", constant=" + childProperty.isConstant() + ", type="
427+
+ childProperty.getClientType() + ", value=" + String.valueOf(childProperty.getDefaultValue())
428+
+ ").");
429+
}
430+
431+
iterator.remove();
432+
break;
433+
}
434+
392435
child.getParentPolymorphicDiscriminators().add(0, parentDiscriminator);
393436

394437
for (ClientModel derived : child.getDerivedModels()) {

packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/template/ModelTemplate.java

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -875,31 +875,19 @@ private void addModelConstructor(ClientModel model, ClientModelPropertiesManager
875875

876876
superProperties.append(property.getName());
877877
} else {
878-
/*
879-
* here because the property in superclass constructor is overwritten in this model
880-
* one example is
881-
*
882-
* model ParentModel {
883-
* property: string;
884-
* }
885-
* model Model extends ParentModel {
886-
* property: "constant";
887-
* }
888-
*
889-
* we use the property in this model to initiate the superclass
890-
*/
891-
ClientModelProperty propertyInThisModel = model.getProperties()
892-
.stream()
878+
// Canonicalized discriminators can supply a fixed inherited constructor argument.
879+
ClientModelProperty overridingProperty = Stream
880+
.concat(model.getProperties().stream(), model.getParentPolymorphicDiscriminators().stream())
893881
.filter(p -> Objects.equals(p.getSerializedName(), property.getSerializedName()))
894882
.findFirst()
895883
.orElse(null);
896-
if (propertyInThisModel != null) {
897-
if (propertyInThisModel.isConstant() && !property.isConstant()) {
884+
if (overridingProperty != null) {
885+
if (overridingProperty.isConstant() && !property.isConstant()) {
898886
// property changed to constant in this model, use constant value to initiate super
899887
// class
900-
superProperties.append(propertyInThisModel.getDefaultValue());
888+
superProperties.append(overridingProperty.getDefaultValue());
901889
} else {
902-
superProperties.append(propertyInThisModel.getName());
890+
superProperties.append(overridingProperty.getName());
903891
}
904892
} else {
905893
// this should not happen

packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/template/StreamSerializationModelTemplate.java

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2059,8 +2059,16 @@ private void writeToXml(JavaClass classBlock) {
20592059
+ propertiesManager.getXmlNamespaceConstant(namespace) + ");"));
20602060

20612061
// Assumption for XML is polymorphic discriminators are attributes.
2062-
if (propertiesManager.getDiscriminatorProperty() != null) {
2063-
serializeXml(methodBlock, propertiesManager.getDiscriminatorProperty().getProperty(), false);
2062+
ClientModelPropertyWithMetadata discriminatorProperty
2063+
= propertiesManager.getDiscriminatorProperty();
2064+
model.getParentPolymorphicDiscriminators()
2065+
.stream()
2066+
.filter(discriminator -> discriminatorProperty == null
2067+
|| !Objects.equals(discriminator.getSerializedName(),
2068+
discriminatorProperty.getProperty().getSerializedName()))
2069+
.forEach(discriminator -> serializeXml(methodBlock, discriminator, false));
2070+
if (discriminatorProperty != null) {
2071+
serializeXml(methodBlock, discriminatorProperty.getProperty(), false);
20642072
}
20652073

20662074
propertiesManager.forEachSuperXmlAttribute(property -> serializeXml(methodBlock, property, true));
@@ -2214,7 +2222,7 @@ private void writeSuperTypeFromXml(JavaClass classBlock) {
22142222
+ propertiesManager.getXmlNamespaceConstant(discriminatorProperty.getXmlNamespace()) + ", "
22152223
+ "\"" + discriminatorProperty.getSerializedName() + "\");");
22162224
} else {
2217-
methodBlock.line("String discriminatorValue = reader.getStringAttribute(" + "\""
2225+
methodBlock.line("String discriminatorValue = reader.getStringAttribute(null, " + "\""
22182226
+ discriminatorProperty.getSerializedName() + "\");");
22192227
}
22202228

@@ -2226,12 +2234,18 @@ private void writeSuperTypeFromXml(JavaClass classBlock) {
22262234
// Add deserialization for all child types.
22272235
List<ClientModel> childTypes = getAllChildTypes(model, new ArrayList<>());
22282236
for (ClientModel childType : childTypes) {
2237+
boolean sameDiscriminator = Objects.equals(childType.getPolymorphicDiscriminatorName(),
2238+
model.getPolymorphicDiscriminatorName());
2239+
if (!sameDiscriminator && !Objects.equals(childType.getParentModelName(), model.getName())) {
2240+
continue;
2241+
}
2242+
2243+
String deserializationMethod = (isSuperTypeWithDiscriminator(childType) && sameDiscriminator)
2244+
? ".fromXmlInternal(reader, finalRootElementName)"
2245+
: ".fromXml(reader, finalRootElementName)";
22292246
ifBlock = ifOrElseIf(methodBlock, ifBlock,
22302247
"\"" + childType.getSerializedName() + "\".equals(discriminatorValue)",
2231-
ifStatement -> ifStatement
2232-
.methodReturn(childType.getName() + (isSuperTypeWithDiscriminator(childType)
2233-
? ".fromXmlInternal(reader, finalRootElementName)"
2234-
: ".fromXml(reader, finalRootElementName)")));
2248+
ifStatement -> ifStatement.methodReturn(childType.getName() + deserializationMethod));
22352249
}
22362250

22372251
if (ifBlock == null) {
@@ -2439,6 +2453,10 @@ private void writeFromXmlDeserialization(JavaBlock methodBlock) {
24392453
}
24402454

24412455
private void deserializeXmlAttribute(JavaBlock methodBlock, ClientModelProperty attribute, boolean fromSuper) {
2456+
if (attribute.isRequired() && attribute.isConstant() && !attribute.isPolymorphicDiscriminator()) {
2457+
return;
2458+
}
2459+
24422460
String xmlAttributeDeserialization = getSimpleXmlDeserialization(attribute.getWireType(), null,
24432461
attribute.getXmlName(), propertiesManager.getXmlNamespaceConstant(attribute.getXmlNamespace()), true);
24442462

packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armstreamstyleserialization/models/GoblinShark.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public final class GoblinShark extends Shark {
2121
/*
2222
* Discriminator property for Fish.
2323
*/
24-
private String kind = "shark";
24+
private final String kind = "shark";
2525

2626
/*
2727
* The sharktype property.

0 commit comments

Comments
 (0)