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

Unpaged instances not Jackson-serializable out of the box #2987

Closed
Nowheresly opened this issue Nov 24, 2023 · 16 comments
Closed

Unpaged instances not Jackson-serializable out of the box #2987

Nowheresly opened this issue Nov 24, 2023 · 16 comments
Assignees
Labels
type: bug A general bug

Comments

@Nowheresly
Copy link

Hi,

after upgrading to spring boot 3.2.0, some unit tests failed with the error:

16:10:11.695 [main] INFO org.springframework.mock.web.MockServletContext -- Initializing Spring TestDispatcherServlet ''
16:10:11.695 [main] INFO org.springframework.test.web.servlet.TestDispatcherServlet -- Initializing Servlet ''
16:10:11.695 [main] INFO org.springframework.test.web.servlet.TestDispatcherServlet -- Completed initialization in 0 ms
16:10:11.701 [main] WARN org.springframework.web.servlet.mvc.support.DefaultHandlerExceptionResolver -- Resolved [org.springframework.http.converter.HttpMessageNotWritableException: Could not write JSON: (was java.lang.UnsupportedOperationException)]

java.lang.AssertionError: Status expected:<200> but was:<500>
Expected :200
Actual   :500

Here is the test:

    @Test
    void findAll() throws Exception {
        doReturn(Page.empty()).when(activityLogService).findAll(any(Predicate.class),any(Pageable.class),any());
        mockMvc.perform(get("/api/v1/activity-logs"))
                .andExpect(status().isOk())
                .andReturn();
    }

If I replace

doReturn(Page.empty()).when(activityLogService).findAll(any(Predicate.class),any(Pageable.class),any());
with
doReturn(Page.empty(PageRequest.of(1, 10, Sort.by("id")))).when(activityLogService).findAll(any(Predicate.class),any(Pageable.class),any());
the error is gone.

It seems Page.empty() is not serializable while Page.empty(PageRequest.of(1, 10, Sort.by("id"))) is.

Is it a regression or did I miss something ?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 24, 2023
@jsadeli
Copy link

jsadeli commented Nov 25, 2023

similar issue, my error message is:

(was java.lang.UnsupportedOperationException) (through reference chain: org.springframework.data.domain.PageImpl[\"pageable\"]->org.springframework.data.domain.Unpaged[\"offset\"])

@smyth90
Copy link

smyth90 commented Nov 25, 2023

You can return a PagedModel<EntityModel<MyEntity>> instead of Page<MyEntity>:

@Autowired
private final PagedResourcesAssembler<MyEntity> assembler;

public ResponseEntity<PagedModel<EntityModel<MyEntity>>> getEntities(
@RequestParam(value = "page", defaultValue = "0") final int page, 
@RequestParam(value = "size", defaultValue = "2000") final int size) {
			final var pageable = PageRequest.of(page, size);
			final var entities = repository.findAll(pageable);
			// Convert to PagedModel and return it
			return new ResponseEntity<>(assembler.toModel(entities), OK);
}

@Carla-de-Beer
Copy link

Carla-de-Beer commented Nov 25, 2023

I have the same issue. All of my mockMvc tests are affected where Page results are returned.

Caused by: org.springframework.http.converter.HttpMessageNotWritableException: Could not write JSON: (was java.lang.UnsupportedOperationException)
	at org.springframework.http.converter.json.AbstractJackson2HttpMessageConverter.writeInternal(AbstractJackson2HttpMessageConverter.java:492)
	at org.springframework.http.converter.AbstractGenericHttpMessageConverter.write(AbstractGenericHttpMessageConverter.java:114)
	at org.springframework.web.servlet.mvc.method.annotation.AbstractMessageConverterMethodProcessor.writeWithMessageConverters(AbstractMessageConverterMethodProcessor.java:297)
	at org.springframework.web.servlet.mvc.method.annotation.RequestResponseBodyMethodProcessor.handleReturnValue(RequestResponseBodyMethodProcessor.java:190)
	at 
```org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite.handleReturnValue(HandlerMethodReturnValueHandlerComposite.java:78)
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:136)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:917)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:829)
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1089)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:979)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1014)
	... 12 more
Caused by: com.fasterxml.jackson.databind.JsonMappingException: (was java.lang.UnsupportedOperationException) (through reference chain: org.springframework.data.domain.PageImpl["pageable"]->org.springframework.data.domain.Unpaged["offset"])

@mp911de mp911de added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 27, 2023
@mp911de
Copy link
Member

mp911de commented Nov 27, 2023

Our domain objects such as Pageable were never intended to be Jackson-serializable, see #1683

@mp911de mp911de closed this as not planned Won't fix, can't repro, duplicate, stale Nov 27, 2023
@wutzebaer
Copy link

wutzebaer commented Nov 27, 2023

Explaination why this fails: If you instanciate PageImpl without Pageable it uses the "Unpaged" isnatnce.
Before it was an enum, even when it implemented getOffset with "UnsupportedOperationException" it was not called because enums just got serialized as string:

"pageable" : "INSTANCE",

Now unpaged is an Object, and jackson tries to seralize all the fields.

My solution unitl SpringBoot offers a better way to return pages:

Instanciate PageImpl with Pageable as parameter (Of course only in tests):

new PageImpl<>(findAllResult, (Pageable) params.getArgument(1), 1)

@mp911de
Copy link
Member

mp911de commented Nov 27, 2023

The unpaged object is represented by an enum. As this is an implementation detail, Jackson tends to apply its enum serialization.

@wutzebaer
Copy link

wutzebaer commented Nov 27, 2023

In the newest version it is no more an enum

final class Unpaged implements Pageable {

Thats why there errors come up now

@jsadeli
Copy link

jsadeli commented Nov 28, 2023

my workaround to this issue (using custom JsonSerializer component):
https://gist.github.com/jsadeli/3cb5c2d788c853099f24c68709931b50

@claudioaze
Copy link

I've solved my tests changing
PageImpl page = new PageImpl<>(myArraylist);

by
Pageable pageable = PageRequest.of(0, 10);
PageImpl page = new PageImpl<>(myArraylist, pageable, myArraylist.size());

@christophstrobl christophstrobl removed the status: invalid An issue that we don't feel is valid label Dec 1, 2023
@christophstrobl
Copy link
Member

We're looking into this evaluating different strategies to mitigate the issue.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 1, 2023
@LauroSilveira
Copy link

Good morning,

I am facing the same error.
Captura de pantalla 2023-12-05 a las 12 50 01
Url repository of error: https://github.com/LauroSilveira/alura-flix-api.git

@bjornharvold
Copy link

Adding Pageable to all our tests solved this for us. We do not have use cases where we request a Page without a pageable FYI.

@LauroSilveira
Copy link

Hello @bjornharvold,
I solved adding Pageable on my tests also.
Thank all for your help.

@odrotbohm odrotbohm removed the status: waiting-for-triage An issue we've not yet triaged label Jan 11, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 11, 2024
odrotbohm added a commit that referenced this issue Jan 11, 2024
We now reinstantiate the serialization of Unpaged instances as plain INSTANCE strings as they were rendered before Unpaged was changed from an enum into a class.

Note, that we still strongly advise, not to serialize Page instances directly as it's a domain type, its Jackson-level surface is subject to change if we need to change the type's API for unrelated reasons.
odrotbohm added a commit that referenced this issue Jan 11, 2024
We now reinstantiate the serialization of Unpaged instances as plain INSTANCE strings as they were rendered before Unpaged was changed from an enum into a class.

Note, that we still strongly advise, not to serialize Page instances directly as it's a domain type, its Jackson-level surface is subject to change if we need to change the type's API for unrelated reasons.
odrotbohm added a commit that referenced this issue Jan 11, 2024
…tly.

We now issue a one-time warning log if Jackson gets handed a PageImpl to serialize as the Jackson structure might need to change for arbitrary other reasons.
@odrotbohm
Copy link
Member

The serialization of Unpaged instances is now back to simply result in a string INSTANCE. For 3.3 we're now adding a warning log if user code ends up trying to serialize a PageImpl instance directly and hint at the build in support to create proper, stable representations.

Accepting that having to use Spring HATEOAS might be a bit of a tough burden, we're currently looking into options to allow creating the kind of same representation but not add the page navigation elements via links as that's what both requires Spring HATEOAS and deciding on a hypermedia enabled media type.

@odrotbohm odrotbohm changed the title [possible regression spring boot 3.2.0] Page.empty() is not json serializable Unpaged instances not Jackson-serializable out of the box Jan 11, 2024
@odrotbohm odrotbohm added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 11, 2024
@odrotbohm odrotbohm added this to the 3.2.2 (2023.1.2) milestone Jan 11, 2024
odrotbohm added a commit that referenced this issue Jan 12, 2024
We now reinstantiate the serialization of Unpaged instances as plain INSTANCE strings as they were rendered before Unpaged was changed from an enum into a class.

Note, that we still strongly advise, not to serialize Page instances directly as it's a domain type, its Jackson-level surface is subject to change if we need to change the type's API for unrelated reasons.

Fixes: GH-2987.
odrotbohm added a commit that referenced this issue Jan 12, 2024
We now issue a one-time warning log if Jackson gets handed a PageImpl to serialize as the Jackson structure might need to change for arbitrary other reasons.

Fixes GH-2987.
@vbliferniz
Copy link

vbliferniz commented Apr 23, 2024

For those who are still looking for the fix.

We are using Spring Boot 3.2.4. Adding the SpringDataJacksonConfiguration.PageModule to the Jackson2ObjectMapperBuilder solved the Problem.

SpringDataJacksonConfiguration.PageModule comes from spring-data-commons-3.2.4

@Bean
public Jackson2ObjectMapperBuilder objectMapperBuilder(SpringDataJacksonConfiguration.PageModule pageModule) {
Jackson2ObjectMapperBuilder builder = new Jackson2ObjectMapperBuilder();
builder.modules(pageModule);
return builder;
}

@odrotbohm
Copy link
Member

Shouldn't all beans implementing Jackson SimpleModule implementations automatically be registered with the ObjectMapper bean Spring Boot autoconfigures?

yaskovdev added a commit to motivepick/motive-back-end that referenced this issue Aug 10, 2024
- Fix the warning about PageImpl (see spring-projects/spring-data-commons#2987).
- Do not call repositories directly from controllers.
- Cleanup TaskListEntity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests