http-client-java, fix list result property in parent#10017
Conversation
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
|
I may need to add another logic, that if a subclass is public without pageable, the parent class should not be in impl package. |
There was a problem hiding this comment.
Pull request overview
This PR addresses an issue in the http-client-java emitter/generator where pageable result/nextLink segments (e.g., "value") may not be found when the corresponding property is declared on a parent model, and it updates packaging behavior for paged models to avoid leaking implementation types into the public surface.
Changes:
- Update
findResponsePropertySegmentsto consider parent schema properties when locating paging-related response segments. - Adjust paged model package placement logic in the generator core to keep paged base models public when needed by public children.
- Add/refresh generator-test TypeSpec scenarios, generated Java outputs, and a JUnit test asserting correct model package placement.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/http-client-java/generator/http-client-generator-test/tsp/arm-stream-style-serialization.tsp | Extends the test TypeSpec with additional list/summary operations and inheritance cases for pageable models. |
| packages/http-client-java/generator/http-client-generator-test/src/test/java/tsptest/armstreamstyleserialization/PagedModelPackageTests.java | Adds a JUnit test to assert paged model classes land in the expected packages. |
| packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/azure-resourcemanager-armstreamstyleserialization-generated_metadata.json | Updates generated metadata to include new operations/models and file list. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armstreamstyleserialization/models/ListResultSummary2.java | Adds a public-facing model interface for a new summary response type. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armstreamstyleserialization/models/ListResult2.java | Adds a public paged wire model used as a base for a public child type. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armstreamstyleserialization/models/Items.java | Adds new client API surface for summary, list2, and summary2 operations. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armstreamstyleserialization/implementation/models/ListResultSummary.java | Adds an implementation-model subclass to validate inherited pageable properties. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armstreamstyleserialization/implementation/models/ListResult.java | Makes the paged base model extensible (non-final, protected ctor) and adds package-private setters for inheritance scenarios. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armstreamstyleserialization/implementation/ListResultSummary2Impl.java | Adds an implementation wrapper mapping an inner model to a public-facing interface. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armstreamstyleserialization/implementation/ItemsImpl.java | Wires new operations through the public Items facade and maps inner-to-public models for summary2. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armstreamstyleserialization/implementation/ItemsClientImpl.java | Adds REST proxy methods and pageable plumbing for the new operations. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armstreamstyleserialization/fluent/models/ListResultSummary2Inner.java | Adds the fluent inner model subclass extending the paged base model. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armstreamstyleserialization/fluent/ItemsClient.java | Exposes new fluent client methods for added operations. |
| packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/mapper/ObjectMapper.java | Changes paged model packaging logic to depend on child visibility. |
| packages/http-client-java/emitter/src/operation-utils.ts | Updates response-property segment resolution to include inherited properties. |
| packages/http-client-java/emitter/src/code-model-builder.ts | Removes an eslint suppression but introduces an untyped local variable. |
| .chronus/changes/http-client-java_fix-property-in-parent-2026-2-13-15-41-25.md | Adds a changelog entry describing the fix. |
Comments suppressed due to low confidence (2)
packages/http-client-java/emitter/src/operation-utils.ts:251
findResponsePropertySegmentsnow concatenates parent properties intocurrentSchemaProperties, but the inner loop doesn’t stop after the first match. If a property exists in both derived + parent schemas (or multiple parents), this will push multiple matches for the same segment and may leavecurrentSchemaPropertiespointing at the wrong schema for the next segment. Consider selecting the first match (derived-first) and breaking, and/or deduplicating by serialized name after merging parent properties.
let currentSchemaProperties: Property[] | undefined = schema.properties;
if (currentSchemaProperties && schema.parents && schema.parents.all) {
for (const parent of schema.parents.all) {
if (parent instanceof ObjectSchema && parent.properties) {
currentSchemaProperties = currentSchemaProperties.concat(parent.properties);
}
}
}
for (const propertySegment of propertySegments) {
// abort if no properties in current schema. this should not happen though
if (!currentSchemaProperties) {
break;
}
// skip non-property segments. again, this should not happen
if (propertySegment.kind === "property") {
const serializedName = getPropertySerializedName(propertySegment);
for (const property of currentSchemaProperties) {
if (property.serializedName === serializedName) {
propertyArray.push(property);
currentSchemaProperties =
property.schema instanceof ObjectSchema ? property.schema.properties : undefined;
}
}
}
packages/http-client-java/emitter/src/code-model-builder.ts:1894
coreNamespaceis declared without a type or initializer. With the repo tsconfig (noImplicitAny: true), this becomes an implicitanyand will fail TypeScript compilation. Declare it as astring(and keep the existing assignments in both branches) to satisfynoImplicitAny.
let coreNamespace;
if (this.isAzureV1()) {
coreNamespace = "com.azure.core.http";
} else {
coreNamespace = "io.clientcore.core.http.models";
}
You can also share your feedback on Copilot code review. Take the survey.
...ore/src/main/java/com/microsoft/typespec/http/client/generator/core/mapper/ObjectMapper.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
issue from billing service Azure/azure-rest-api-specs#40073