Skip to content

fix(InlineModelResolver): do not merge untitled inline schemas differing only by description (#24004)#24007

Open
seonwooj0810 wants to merge 2 commits into
OpenAPITools:masterfrom
seonwooj0810:fix/issue-24004-untitled-inline-dedup
Open

fix(InlineModelResolver): do not merge untitled inline schemas differing only by description (#24004)#24007
seonwooj0810 wants to merge 2 commits into
OpenAPITools:masterfrom
seonwooj0810:fix/issue-24004-untitled-inline-dedup

Conversation

@seonwooj0810

@seonwooj0810 seonwooj0810 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #24004

Problem

A schema with two structurally-identical inline (untitled) object properties that differ only in their descriptions is generated differently in 7.22 vs 7.23. Given:

"properties": {
  "abc": { "type": "object", "description": "first container",
           "properties": { "result": { "type": "string", "description": "ABC Result" } } },
  "def": { "type": "object", "description": "second container",
           "properties": { "result": { "type": "string", "description": "DEF Result" } } }
}
  • 7.22 generates two distinct types — abc…Abc, def…Def.
  • 7.23 generates def with type …Abc, so code expecting …Def no longer compiles.

Root cause

The structural-signature dedup fallback introduced in #23856 (to prevent numbered duplicate models from multi-file OAS 3.1 specs) serialises schemas with description/type/example stripped at every level and was consulted/registered for all schemas in matchGenerated()/addGenerated(). Once those volatile fields are stripped, abc and def produce an identical signature, so def is unified with abc.

That fallback is only appropriate for titled named types (which should be reused wherever they appear, despite parser-induced volatility). Anonymous/untitled inline schemas may be intentionally distinct — exactly as the existing titled-only guards in flatten() pre-population and deduplicateComponents() already note ("anonymous schemas may be intentionally distinct").

Fix

Restrict the structural-signature fallback (both lookup in matchGenerated() and registration in addGenerated()) to schemas with a non-null title. Exact-signature matching is unchanged, so genuine duplicates are still reused, and the #23856 multi-file dedup behaviour (which operates on titled schemas) is preserved.

Test evidence

Added InlineModelResolverTest#resolveInlineModelKeepsUntitledSchemasDifferingOnlyByDescriptionDistinct, which reproduces the issue and asserts abc/def resolve to distinct $refs. Verified it fails on master and passes with this change.

Full InlineModelResolverTest suite (including the #23856 structural-dedup regression tests …MutatesPropertyDescriptions, …MutatesPropertyTypes, and the multi-file root.yaml cases) passes:

Tests run: 56, Failures: 0, Errors: 0, Skipped: 0

Verification done: built/tested on JDK 21 via ./mvnw -pl modules/openapi-generator -am test -Dtest=InlineModelResolverTest.

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore samples — N/A: this change does not alter generated output for any existing sample (no committed sample uses two untitled inline schemas differing only by description); full InlineModelResolverTest passes unchanged.
  • Filed the PR against the correct branch: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Summary by cubic

Stops merging untitled inline schemas that differ only by description by limiting structural-signature dedup to titled schemas, restoring distinct generated types and fixing the 7.23 regression. Regenerates samples (C# FormModels, crystal-qdrant, Python clients) to reflect corrected model/type emission.

  • Bug Fixes

    • Restrict structural-signature fallback to titled schemas in InlineModelResolver (matchGenerated(), addGenerated()); keep exact-signature dedup.
    • Add regression test asserting distinct $refs for abc/def; preserve multi-file OAS 3.1 reuse for titled schemas.
  • Migration

    • Regenerated samples now emit distinct types where untitled inline schemas were merged; update references if you relied on the unified type.
    • Examples: FilterMust/FilterMustNot/PrefetchPrefetch and TrackerStatusOneOf1 in crystal-qdrant; TestEnumParametersEnumHeaderStringParameter in C#; UploadFileWithAdditionalPropertiesRequestObject in Python clients.

Written for commit 44ac7ae. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

@seonwooj0810 seonwooj0810 force-pushed the fix/issue-24004-untitled-inline-dedup branch 2 times, most recently from e7bf897 to 9589d82 Compare June 23, 2026 09:03
@seonwooj0810

Copy link
Copy Markdown
Contributor Author

Rebased onto latest master and regenerated the affected samples (sequentially, to avoid the parallel-generation nondeterminism in generate-samples.sh). The core change is covered by InlineModelResolverTest.

The remaining red checks are generated-sample builds (.NET/PHP/Python) plus the csharp generichost .openapi-generator/FILES, which shows non-deterministic entries across generation runs (e.g. a test file alternately listed/omitted). Could a maintainer advise whether these are pre-existing sample-harness flakes vs. something this change should address?

@seonwooj0810

Copy link
Copy Markdown
Contributor Author

Correction to my earlier comment — after digging into the failing checks, my "remaining failures are unrelated / parallel-generation nondeterminism" note was wrong for most of them. Accurate breakdown:

Build .Net* (samples/client/petstore/csharp/generichost/.../FormModels) — caused by this PR, and not fixable by regeneration.
This change stops merging the enum_header_string / enum_query_string inline enums into the form enum, so the generated API (FakeApi.cs) now correctly types those parameters as TestEnumParametersEnumHeaderStringParameter. But the generated test (FakeApiTests.cs) still declares them as TestEnumParametersRequestEnumFormString, which is the CS1503 mismatch reported on arguments 3 and 7. Regenerating on this branch reproduces the same inconsistency, so it's not a stale-sample problem — the csharp generichost generator doesn't propagate the per-parameter model type into the api_test output. This is a pre-existing generator gap that this PR is simply the first to expose (it only surfaces once those parameters stop sharing a single model). Filed separately as #24112.

Test Python client (.../python) — also an effect of this PR. The change splits the multipart object schema into its own model (UploadFileWithAdditionalPropertiesRequestObject instead of the previously shared TestObjectForMultipartRequestsRequestMarker), which is what breaks test_multipart_requests_with_file_and_additional_properties.

Build PHP (.../php-laravel/) — this one does look unrelated: composer reports "requirements could not be resolved", i.e. a dependency/infra failure rather than anything this change generates.

Samples up-to-date — the .openapi-generator/FILES for the 5 csharp generichost FormModels samples (and crystal-qdrant) need regenerating; I'll push that.

Since the .NET breakage is a generator-side issue beyond the scope of this inline-resolver fix, I'd welcome maintainer guidance on the preferred direction here.

…ing only by description

The structural-signature dedup fallback added for multi-file OAS 3.1 specs
(OpenAPITools#23856) strips description/type/example at every level and was applied to all
schemas in matchGenerated()/addGenerated(). For untitled inline schemas that are
structurally identical once those fields are stripped but intentionally distinct
(e.g. two response properties differing only by description), this collapsed them
into a single generated type, changing one property's type and breaking compiling
user code (regression since 7.23).

Restrict the structural-signature fallback to titled schemas only, mirroring the
existing titled-only guards in flatten() pre-population and deduplicateComponents()
("anonymous schemas may be intentionally distinct"). Exact-signature matching is
unaffected, so genuine duplicates are still reused.

Fixes OpenAPITools#24004

Signed-off-by: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.com>
@seonwooj0810 seonwooj0810 force-pushed the fix/issue-24004-untitled-inline-dedup branch from 9589d82 to bcffcfa Compare June 24, 2026 01:53
Align the FakeApiTests enumHeaderString/enumQueryString parameter types
with the FakeApi signature (EnumHeaderStringParameter) so the samples
match CI-generated output and compile.

Signed-off-by: seonwooj0810 <seonwooj0810@gmail.com>
@seonwooj0810 seonwooj0810 force-pushed the fix/issue-24004-untitled-inline-dedup branch from bcffcfa to 44ac7ae Compare June 24, 2026 03:22
@seonwooj0810

Copy link
Copy Markdown
Contributor Author

Update + correction to my earlier comments — I was wrong about the .NET failures, and I've now pushed a fix that greens them.

Build .Net* (...generichost/.../FormModels) — fixed (now green).
My earlier "not fixable by regeneration" was incorrect. The committed FakeApiTests.cs typed enumHeaderString/enumQueryString as TestEnumParametersRequestEnumFormString, inconsistent with the FakeApi signature (...EnumHeaderStringParameter) → CS1503. The key finding is that this output is environment-dependent, not nondeterministic: on my machine (macOS) regeneration stably produces the inconsistent test types — sequential and batch alike — whereas CI generates the self-consistent EnumHeaderStringParameter for both the API and the test. I have not isolated the exact variable: it is not the JDK (11 and 25 behave identically here), not the locale, and a Docker/Linux check was inconclusive (Docker Desktop mounts the host macOS filesystem, so it wasn't a clean Linux run). I committed the CI-consistent samples, which greens both Samples up-to-date and the .NET builds.

One concrete clue for whoever digs deeper: within a single generation run, FakeApi.cs and FakeApiTests.cs disagree on the same parameter's model type, and FakeApi.cs is stable across environments — so the divergence is specific to the api_test generation path, not global model naming. (I'll reframe #24112 accordingly — the templates themselves are fine; CI proves they emit the correct output.)

To be clear on attribution: this is PR-caused in the sense that the split is what exposes the order/environment-dependent naming (master has no split, so it compiles everywhere) — but it is not "parallel-generation nondeterminism," which I'd earlier hand-waved at.

Build PHP (.../php-laravel/) — failing but unrelated to this PR: composer refuses laravel/framework ^10.0 because those versions are now flagged by packagist security advisories (PKSA-*). The php-laravel sample isn't touched by this change, so it's a repo-wide issue rather than anything this PR introduced.

Test Python client (.../python*) — these are caused by this PR. The change splits the multipart object schema into UploadFileWithAdditionalPropertiesRequestObject, and the manual test_rest.py multipart test no longer matches. Unlike the .NET issue this is a genuine behavioral effect of the split (it does not occur on #24009), so it needs a decision rather than a regeneration. I'd welcome maintainer guidance on whether the split should be narrowed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] [JAVA] Deduplication feature in 7.23 breaks existing code

1 participant