Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document changes in rendering of PageImpl instances in Spring Boot 3.3 migration guide #41856

Closed
samuelstein opened this issue Aug 14, 2024 · 9 comments
Labels
type: wiki-documentation A documentation update required on the wiki

Comments

@samuelstein
Copy link

samuelstein commented Aug 14, 2024

During the migration to Spring Boot 3.3 I recently stumbled over the changes when rendering org.springframework.data.domain.PageImpl instances to JSON. In my case an additional field wasn't any longer present in the response.

The prefered way is now to wrap the response in PagedModel class or use the @EnableSpringDataWebSupport annotation. To find this out it took me hours unfortunately.
I couldn't find a hint in Spring Boot 3.2 and Spring Boot 3.3 migration guide.
Could you please add a note there?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 14, 2024
@wilkinsona
Copy link
Member

Thanks for the suggestion. We'd prefer to link out to something provided by the Spring Data project rather than documenting it ourselves. I'll ask the Data team if they have such a thing.

@wilkinsona wilkinsona added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Aug 14, 2024
@wilkinsona
Copy link
Member

I assume, given that you've mentioned both Boot 3.2 and 3.3, that you're upgrading from 3.1. The Data team are a bit puzzled as "all we added for 3.3 is a warning and a mechanism to flip all renderings to a new, preferred model". Can you please share a minimal sample that works with Spring Boot 3.1.x and requires changes to continue to work in the same way with Spring Boot 3.3.x?

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Aug 14, 2024
@samuelstein
Copy link
Author

samuelstein commented Aug 14, 2024

Sure.
I did a upgrade from Spring Boot 3.2.5.
We had before the upgrade a ResponsePage which extendend PageImpl with an additional field for a header row:

public class ResponsePageWithHeaderRow<T, U extends Serializable> extends PageImpl<T> {

@Getter
private U headerRow;

...
}

After the upgrade to Spring Boot 3.3.2 the additional field headerRow was missing wheater I added and configured the @EnableSpringDataWebSupport annotation or not.
It only worked after extending and returnig PagedModel.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 14, 2024
@wilkinsona
Copy link
Member

Thank you. Could you please turn that snippet into a minimal example that we can build and run to observe the behaviour you have described?

@wilkinsona
Copy link
Member

wilkinsona commented Aug 14, 2024

It looks like the use of extension may be what's causing the problem. The Data team are going to investigate and add something to https://github.com/spring-projects/spring-data-commons/wiki/Spring-Data-2024.0-Release-Notes if necessary.

On the Boot side, I've updated the Dependency Upgrades section of our 3.3 release notes to link to Spring Data's release notes.

@wilkinsona wilkinsona added type: wiki-documentation A documentation update required on the wiki and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Aug 14, 2024
@odrotbohm
Copy link
Member

It looks like the problem is caused by our registration of a Jackson Converter to produce a warning log but not actually converting the source value. However, in BeanSerializerFactory.createSerializer(…)Jackson inspects the Converter's generics and builds a serializer for that type. As we, of course, declare that to Page, this process then only considers properties exposed on it and ultimately causes the custom properties not considered.

I think Jackson could fix this by using the more concrete type of the two, i.e. testing for assignability and sticking to the original type if that's assignable to the detected one. I'll go ahead and open a ticket with Jackson to get this fixed. Meanwhile, I'll investigate whether we can use different means to produce the log message, so that we might not have to wait for Jackson to fix this.

@odrotbohm
Copy link
Member

We found a less invasive way of issuing the warning, which will make it into the Spring Data releases tomorrow, picked up by the Boot releases next week. I'll make sure we leave a note about the changes in general in the official changelog.

@wilkinsona
Copy link
Member

Thanks, @odrotbohm.

@samuelstein
Copy link
Author

Thank you very much for the quick response, explanation and fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: wiki-documentation A documentation update required on the wiki
Projects
None yet
Development

No branches or pull requests

4 participants