Skip to content

[kotlin-spring] allOf discriminator children get default discriminator value - #23952

Open
rvlasveld wants to merge 1 commit into
OpenAPITools:masterfrom
rvlasveld:fix/kotlin-spring-allof-discriminator-defaults
Open

rvlasveld wants to merge 1 commit into
OpenAPITools:masterfrom
rvlasveld:fix/kotlin-spring-allof-discriminator-defaults

Conversation

@rvlasveld

@rvlasveld rvlasveld commented Jun 5, 2026 •

Copy link
Copy Markdown
Contributor

What

A child of a discriminator parent already knows its own discriminating value, but kotlin-spring generated that property as an ordinary required constructor parameter. Callers had to pass it on every construction, and nothing stopped them passing a wrong one:

// before
Dog(petType = "Cat", ...)   // compiles, serialises as a Cat

The discriminator property now carries a default taken from the discriminator mapping:

// after
data class Dog(
    @get:JsonProperty("petType", required = true) override val petType: kotlin.String = "Dog",
    ...
)

The oneOf path already did this. This PR generalises that loop to every discriminator parent, so allOf parents are handled by the same code rather than a parallel implementation.

The emitted value is the mapping key, not the schema name

Worth highlighting, because it is easy to get wrong. Given:

discriminator:
  propertyName: className
  mapping:
    DOG: '#/components/schemas/Dog'

the generated default is "DOG", not "Dog". This falls out of CodegenDiscriminator.MappedModel ordering and the de-duplication in DefaultCodegen.createDiscriminator() — subtle enough that it is now pinned by a dedicated test (testAllOfDiscriminatorMappingKeyDiffersFromSchemaName), using the existing petstore-with-fake-endpoints-for-testing-with-cookie.yaml spec which maps DOG/CAT onto Dog/Cat.

Behaviour change

The discriminator property gains a default value, so it becomes optional at the call site. This is source-compatible for existing callers that still pass it explicitly. Callers who passed a wrong value keep compiling — the default only changes what happens when the argument is omitted.

The rewritten loop is also no longer gated on x-is-one-of-interface: it now runs for every model with a discriminator and a non-empty mapped-model set. In practice that widens coverage from oneOf to allOf (the point of the PR) and to anyOf, which shares the same mapped-model machinery.

Tests

KotlinSpringServerCodegenTest: 286 passing, 3 of them new (default from schema name, default from explicit mapping, mapping key ≠ schema name).

Samples

Regenerated all kotlin-spring sample configs. Three samples change — note kotlin-springboot-request-cookie is included, which the earlier revision of this PR had missed:

  • kotlin-springboot-x-kotlin-implements — Cat/Dog
  • kotlin-springboot-include-http-request-context-delegate — Cat/Dog
  • kotlin-springboot-request-cookie — Cat/Dog (shows = "DOG" / = "CAT")

For maintainers

  • Independent PR. No ordering relationship with [kotlin-spring] seal allOf discriminator parent interfaces by default #23953; either merge order works. The two touch
    different regions of KotlinSpringServerCodegen.java and different sample files, so the only
    overlap is a both-added resolution at the tail of KotlinSpringServerCodegenTest.java.
  • Rebased on master after [kotlin-spring] fix multi-level allOf inheritance (fixes #18206) #23954 merged. That PR added the allOf-multilevel sample, whose
    Animal children are allOf discriminator children — so this PR now also gives Dog, Cat and
    BigDog their className defaults in that sample. This is the regeneration [kotlin-spring] fix multi-level allOf inheritance (fixes #18206) #23954 flagged as
    due once this landed, not a new behaviour.
  • Should this be opt-in? It ships on by default with no flag. That seems right — emitting a value the schema already fixes is a bug fix, not a preference — but say the word and I will put it behind an option.
  • anyOf is now in scope as described above. It is untested here because no kotlin-spring sample exercises an anyOf discriminator; happy to add one if you would rather have it covered explicitly.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VBn7VWUxZL6mTUbg21xNjm

@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 6 files

Re-trigger cubic

@rvlasveld
rvlasveld marked this pull request as draft June 5, 2026 11:11
@rvlasveld

This comment was marked as outdated.

@rvlasveld
rvlasveld marked this pull request as ready for review June 5, 2026 12:35

@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 6 files

Re-trigger cubic

@rvlasveld

Copy link
Copy Markdown
Contributor Author

@wing328, could you help get this reviewed?

@wing328

wing328 commented Jul 30, 2026

Copy link
Copy Markdown
Member

@rvlasveld thanks for the PR

can you please PM me via Slack to go through your PRs and get them merged?

https://join.slack.com/t/openapi-generator/shared_invite/zt-36ucx4ybl-jYrN6euoYn6zxXNZdldoZA

@wing328 wing328 added this to the 7.25.0 milestone Jul 30, 2026
@wing328 wing328 modified the milestones: 7.25.0, 7.26.0 Aug 24, 2026
@rvlasveld
rvlasveld force-pushed the fix/kotlin-spring-allof-discriminator-defaults branch from 549b8fc to 43abab4 Compare September 20, 2026 13:58
@wing328

wing328 commented Sep 27, 2026

Copy link
Copy Markdown
Member

please resolve the merge conflicts when you've time. will try to review, test it before the upcoming stable release (due later this coming week)

…r value

A child of a discriminator parent already knows its own discriminating
value, but the generated property was an ordinary required constructor
parameter, so every caller had to pass it and could pass a wrong one.

The discriminator property now gets a default value taken from the
discriminator mapping, matching what the oneOf path already did. The
oneOf-specific loop is generalised to every discriminator parent, so
allOf parents are covered by the same code path.

The value is the mapping key, which is not always the schema name: a
spec mapping DOG -> Dog makes "DOG" the discriminating value.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VBn7VWUxZL6mTUbg21xNjm
@rvlasveld
rvlasveld force-pushed the fix/kotlin-spring-allof-discriminator-defaults branch from 43abab4 to 2a85d55 Compare September 27, 2026 21:07

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants