Honor PropertyNamingStrategy for is-prefixed record components#5215
Closed
seonwooj0810 wants to merge 1 commit into
Closed
Honor PropertyNamingStrategy for is-prefixed record components#5215seonwooj0810 wants to merge 1 commit into
seonwooj0810 wants to merge 1 commit into
Conversation
The get/is name-clobbering hack (issue swagger-api#415) overwrites the strategy-translated property name with the raw member name when an accessor starts with "is" followed by a lowercase letter. For records the accessor name equals the component name, so non-boolean components such as issuanceDate / issuingCountry / issuingAuthority were left in camelCase under a SNAKE_CASE (or any) PropertyNamingStrategy, while other fields were translated correctly. Skip the hack when a PropertyNamingStrategy is configured: in that case propDef.getName() already reflects the strategy and must not be overridden. Default (no-strategy) behavior is unchanged, so the swagger-api#415 workaround is preserved. Signed-off-by: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.com>
Contributor
|
Hi, first I would like to clarify that I am not the maintainer of springdoc, only a contributor. So take everything I say with a grain of salt. Second, this looks like to be a duplicate of your previous PR #5192? |
Author
|
You're right, thanks for catching this @Mattias-Sehlstedt. This does overlap with my earlier #5192, which targets the same |
Author
|
Closing as a duplicate of #5192, which addresses the same root cause more comprehensively. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a Jackson
PropertyNamingStrategy(e.g.SNAKE_CASE) is configured on theObjectMapperused byModelResolver, non-boolean record components whose name starts withisare left in camelCase in the generated schema, while every other field is translated correctly.Given:
with
SNAKE_CASE, the resolved schema containsfamily_name,expiry_date— butissuanceDate,issuingCountry,issuingAuthority(untranslated).This was diagnosed as a swagger-core issue by the springdoc-openapi maintainer in springdoc/springdoc-openapi#3293 (reproducible directly against
ModelResolver, no Spring involved).Root cause
The get/is name-clobbering hack (added for #415) in
ModelResolverfalls back to the raw member name when an accessor starts withget/isfollowed by a lowercase letter:For a record, the accessor name equals the component name (
issuanceDate), so"is"+ lowercase matches andpropNameis overwritten with the camelCase member name — discarding the strategy-translated value that Jackson already computed inpropDef.getName(). Classic getters (getIssuanceDate/isActive) are unaffected because the character after the prefix is uppercase.Fix
Skip the hack when a
PropertyNamingStrategyis configured. In that casepropDef.getName()is already authoritative and must not be overridden. Default (no-strategy) behavior is unchanged, so the #415 workaround is fully preserved.Tests
Added
Ticket3293Testin theswagger-java17-supportmodule (records require Java 17). It resolves the record above with aSNAKE_CASEModelResolverand asserts all five string components plus a genuine boolean component (isActive→is_active) are snake_cased.Verified the test fails without the fix (the three
is*components leak asissuanceDate/issuingCountry/issuingAuthority) and passes with it, withis_activecorrect in both runs (no boolean-getter regression). Fullswagger-java17-supportmodule and theio.swagger.v3.core.resolving.*package pass.Refs springdoc/springdoc-openapi#3293