- Notifications
You must be signed in to change notification settings - Fork 698
Description
With the change added in #3024 there seems to be couple of reported inconveniences for some users (#3264, #3320, #3327)
I am specifically opening this issue to discuss on what was the initial idea of this change (for which an input from @odrotbohm would be much appreciated) and trying to find a different solution for the problem. I completely agree that providing a stable JSON representation is very important and at the same time allow the implementation PageImpl
to evolve independently.
Currently it only targets Jackson as a json serialization library, but AFAIK Gson and others are also supported by the Spring Web project.
So I am having the following questions:
- Was Jackson the primary target ? Or, since probably it is used 99+% of the time, the change was implemented for it only ?
- Was it targeting a direct
PageImpl
serialization only ? Because current implementation also affects a nested property of any object that is of thePageImpl
type.
My overall issue is that now, when I opt in for the pageSerializationMode=VIA_DTO
, I am breaking the clients of my API. Which is also true, if the PageImpl
suddently changes (which was the primary goal of the initial change - stability).
I am currently thinking if we could use ResponseBodyAdvice that will apply the conversion from Page
to PagedModel
. This will give us the following benefits:
- The conversion will be independent from the serialization library (Even tho we can still target only Jackson initially by using a AbstractMappingJacksonResponseBodyAdvice)
- This will not affect serialization of bean properties of type
PageImpl
, only directly returned objects from@Controller
- The "converter" can be chosen/configured by the user/dev and the API stability will be transferred away from the maintainers
Potential implementation:
@Configuration(proxyBeanMethods = false) public class SpringDataWebConfiguration implements WebMvcConfigurer, BeanClassLoaderAware { ..... @Bean ResponseBodyAdvice<Object> pageResponseBodyAdvice(@Autowired(required = false) SpringDataWebSettings settings, ObjectProvider<PageResponseBodyConverter> pageConverter) { if (settings == null || settings.pageSerializationMode() == DIRECT) { return PageResponseBodyAdvice.NO_OP; } return new PageResponseBodyAdvice(pageConverter.getIfAvailable(() -> PagedModel::new)); } public interface PageResponseBodyConverter { <T> Object convert(Page<T> page); } @ControllerAdvice static final class PageResponseBodyAdvice extends AbstractMappingJacksonResponseBodyAdvice { static final ResponseBodyAdvice<Object> NO_OP = new ResponseBodyAdvice<>() { @Override public boolean supports(MethodParameter returnType, Class<? extends HttpMessageConverter<?>> converterType) { return false; } @Override public Object beforeBodyWrite(Object body, MethodParameter returnType, MediaType selectedContentType, Class<? extends HttpMessageConverter<?>> selectedConverterType, ServerHttpRequest request, ServerHttpResponse response) { return body; } }; private final PageResponseBodyConverter converter; public PageResponseBodyAdvice(PageResponseBodyConverter converter) { this.converter = converter; } @Override public void beforeBodyWriteInternal(MappingJacksonValue bodyContainer, MediaType contentType, MethodParameter returnType, ServerHttpRequest request, ServerHttpResponse response) { Object model = converter.convert((PageImpl<?>) bodyContainer.getValue()); bodyContainer.setValue(model); } @Override public boolean supports(MethodParameter returnType, Class<? extends HttpMessageConverter<?>> converterType) { return super.supports(returnType, converterType) && returnType.getParameterType().isAssignableFrom(PageImpl.class); } } }