Skip to content

Preserve all discriminator aliases when comparing oneOf schemas - #929

Open
MoChiUaena wants to merge 1 commit into
OpenAPITools:masterfrom
MoChiUaena:codex/fix-discriminator-mapping-order
Open

MoChiUaena wants to merge 1 commit into
OpenAPITools:masterfrom
MoChiUaena:codex/fix-discriminator-mapping-order

Conversation

@MoChiUaena

@MoChiUaena MoChiUaena commented Sep 27, 2026 •

Copy link
Copy Markdown

When multiple discriminator values point to the same schema, reordering the mapping can incorrectly report a breaking change. The reverse mapping in ComposedSchemaDiffResult kept only the last alias for each reference, which also hid additions, removals, and target changes involving a discarded alias.

Keep the explicit alias-to-reference mapping intact and add implicit schema names for references without an explicit mapping. When scheduling aliases, check recursion against the references visited before entering this oneOf, while retaining the shared visited set for deferred traversal. This lets aliases reuse cached comparisons and compare changed targets regardless of mapping order, without introducing circular dependencies between mutually recursive schemas. The existing request/response compatibility rules remain in place.

Fixes #886.

Validation on Windows with the Maven wrapper (3.8.5):

  • Added two minimal YAML fixtures and 18 test cases covering request and response mappings: reorder in both directions, alias addition/removal, target changes in both mapping orders, shared schema changes, mixed implicit/explicit mappings, self-references, and mutually recursive schemas.
  • On unmodified master, all 18 added cases fail while the 9 existing cases pass. All 27 OneOfDiffTest cases pass with the final fix.
  • ./mvnw.cmd -B -ntp -pl core com.coveo:fmt-maven-plugin:format -Dtest=OneOfDiffTest test
  • ./mvnw.cmd -V -B -ntp -ff clean verify — all modules passed on JDK 8 (Corretto 8u504), 11 and 21; core: 288 tests, 2 existing disabled tests; Maven plugin: 13 tests; no failures or errors.
  • ./mvnw.cmd -V -B -ntp -ff -Dmaven.compiler.release=8 clean verify — all modules passed on JDK 17, compiling against the Java 8 API.
  • ./mvnw.cmd -B -ntp com.coveo:fmt-maven-plugin:check — all modules passed.
  • Packaged CLI checks: reordered mappings report no changes (exit 0); removing a request alias reports incompatibility (exit 1); adding a request alias remains compatible (exit 0).
  • The packaged CLI reorder and removal checks also passed when run with the JDK 8 JVM.

The upstream Linux CI run is still pending.


Summary by cubic

Fixes false breaking changes when multiple discriminator aliases point to the same schema and their mapping order changes.

  • Keeps every explicit alias-to-reference mapping and adds implicit schema names only for unmapped references.
  • Checks recursion against references visited before entering the oneOf while keeping the shared visited set for deferred traversal.
  • Adds 18 test cases covering reordering, alias add/remove, target changes, self-recursive, and mutually recursive schemas.

Written for commit 23699ce. Summary will update on new commits.

Review in cubic

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 5 files

Re-trigger cubic

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] Backward compatibility check fails on reordered discriminator mappings

1 participant