Fix 1324 page snake case support 4.3.x#1329
Fix 1324 page snake case support 4.3.x#1329weslyvinicius wants to merge 2 commits intospring-cloud:4.3.xfrom
Conversation
…NAKE_CASE compatibility Signed-off-by: weslyvinicius <weslyvinicius@hotmail.com>
Signed-off-by: weslyvinicius <weslyvinicius@hotmail.com>
|
The code does not compile |
There was a problem hiding this comment.
Pull request overview
Adds Jackson aliases for Pageable fields in PageJacksonModule to support deserializing Page<T> when external APIs (or global naming strategies like SNAKE_CASE) provide page_number / page-size / etc. This addresses spring-cloud-openfeign issue #1324 where SimplePageable only accepted camelCase names.
Changes:
- Add
@JsonAliasvariants forpageNumberandpageSizeinPageJacksonModule.SimplePageable. - Add new JSON fixtures covering hyphen/underscore/lowercase/pascal-case pageable field names.
- Add tests asserting deserialization succeeds for those pageable aliases.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/PageJacksonModule.java | Adds @JsonAlias on SimplePageable constructor params to accept multiple casing/format variants. |
| spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/PageJacksonModuleTests.java | Adds new tests for pageable alias deserialization. |
| spring-cloud-openfeign-core/src/test/resources/withPageableAliasHyphen.json | Test fixture using page-number / page-size. |
| spring-cloud-openfeign-core/src/test/resources/withPageableAliasUnderscore.json | Test fixture using page_number / page_size. |
| spring-cloud-openfeign-core/src/test/resources/withPageableAliasLowercase.json | Test fixture using pagenumber / pagesize. |
| spring-cloud-openfeign-core/src/test/resources/withPageableAliasPascalCase.json | Test fixture using PageNumber / PageSize. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void deserializePageableWithUnderscoreAlias() { | ||
| // Given | ||
| File file = new File("./src/test/resources/withPageableAliasUnderscore.json"); | ||
| // When | ||
| Page<?> result = objectMapper.readValue(file, Page.class); | ||
| // Then |
There was a problem hiding this comment.
These alias tests use the default ObjectMapper configuration, but the reported regression occurs specifically when a global PropertyNamingStrategies.SNAKE_CASE strategy is enabled. Consider adding a test that builds/configures an ObjectMapper with SNAKE_CASE and verifies the same JSON (e.g., page_number / page_size) deserializes correctly, so the original issue is covered end-to-end.
| @Test | ||
| void deserializePageableWithHyphenatedAlias() { | ||
| // Given | ||
| File file = new File("./src/test/resources/withPageableAliasHyphen.json"); | ||
| // When | ||
| Page<?> result = objectMapper.readValue(file, Page.class); |
There was a problem hiding this comment.
objectMapper.readValue(File, ...) throws IOException, but this test method does not declare/handle it, so the test class will not compile. Add throws IOException (or throws Exception) to the method signature, or wrap the call in a try/catch and fail the test on exception.
| @Test | ||
| void deserializePageableWithUnderscoreAlias() { | ||
| // Given | ||
| File file = new File("./src/test/resources/withPageableAliasUnderscore.json"); | ||
| // When | ||
| Page<?> result = objectMapper.readValue(file, Page.class); |
There was a problem hiding this comment.
objectMapper.readValue(File, ...) throws IOException, but this test method does not declare/handle it, so the test class will not compile. Add throws IOException (or throws Exception) to the method signature, or wrap the call in a try/catch and fail the test on exception.
| @Test | ||
| void deserializePageableWithLowercaseAlias() { | ||
| // Given | ||
| File file = new File("./src/test/resources/withPageableAliasLowercase.json"); | ||
| // When | ||
| Page<?> result = objectMapper.readValue(file, Page.class); |
There was a problem hiding this comment.
objectMapper.readValue(File, ...) throws IOException, but this test method does not declare/handle it, so the test class will not compile. Add throws IOException (or throws Exception) to the method signature, or wrap the call in a try/catch and fail the test on exception.
| @Test | ||
| void deserializePageableWithPascalCaseAlias() { | ||
| // Given | ||
| File file = new File("./src/test/resources/withPageableAliasPascalCase.json"); | ||
| // When | ||
| Page<?> result = objectMapper.readValue(file, Page.class); |
There was a problem hiding this comment.
objectMapper.readValue(File, ...) throws IOException, but this test method does not declare/handle it, so the test class will not compile. Add throws IOException (or throws Exception) to the method signature, or wrap the call in a try/catch and fail the test on exception.
| @JsonProperty("pageNumber") @JsonAlias({"page-number", "page_number", "pagenumber", "PageNumber"}) int number, | ||
| @JsonProperty("pageSize") @JsonAlias({"page-size", "page_size", "pagesize", "PageSize"}) int size, |
There was a problem hiding this comment.
Minor formatting: these @JsonAlias array initializers omit the spaces used elsewhere in this file (e.g., @JsonAlias({ "total-elements", ... })). Consider formatting consistently to avoid style/linter churn.
| @JsonProperty("pageNumber") @JsonAlias({"page-number", "page_number", "pagenumber", "PageNumber"}) int number, | |
| @JsonProperty("pageSize") @JsonAlias({"page-size", "page_size", "pagesize", "PageSize"}) int size, | |
| @JsonProperty("pageNumber") @JsonAlias({ "page-number", "page_number", "pagenumber", "PageNumber" }) int number, | |
| @JsonProperty("pageSize") @JsonAlias({ "page-size", "page_size", "pagesize", "PageSize" }) int size, |
Thanks for pointing that out. I have now reworked the fix directly on top of the 4.3.x branch and opened a new PR with the corrected changes: |
Fixes #1324
When a global PropertyNamingStrategies.SNAKE_CASE is configured, deserialization of Page fails because SimplePageable only accepted camelCase property names.
This PR adds @JsonProperty + @JsonAlias to the constructor parameters, supporting:
pageNumber / page_number / page-number / pagenumber
pageSize / page_size / page-size / pagesize
Tested with the reproduction case provided in the issue (global SNAKE_CASE + WireMock returning snake_case JSON).