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

Can not serialize CompletionStage<Response> #5103

Closed
hantsy opened this issue Jul 20, 2022 · 6 comments · Fixed by #5105
Closed

Can not serialize CompletionStage<Response> #5103

hantsy opened this issue Jul 20, 2022 · 6 comments · Fixed by #5105
Milestone

Comments

@hantsy
Copy link

hantsy commented Jul 20, 2022

Glassfish 7.0.0-M7
Java 17
Windows 10

Sample Jaxrs resource return a CompletionStage<Response>.

@GET
public CompletionStage<Response> getAllTodos() {
    return todoService.getAllTodosAsync().thenApply(todos -> Response.ok(todos).build());
}

When running the test TodoResourceTest, and got the following exceptions in the server.log.

[2022-07-20T23:09:41.977+0800] [glassfish 7.0] [WARNING] [] [org.glassfish.jersey.server.ServerRuntime$Responder] [tid: _ThreadID=139 _ThreadName=java:module/concurrent/MyExecutor-managedThreadFactory-Thread-1] [timeMillis: 1658329781977] [levelValue: 900] [[
  An exception mapping did not successfully produce and processed a response. Logging the exception propagated to the default exception mapper.
jakarta.ws.rs.ProcessingException: Error writing JSON-B serialized object.
	at org.glassfish.jersey.jsonb.internal.JsonBindingProvider.writeTo(JsonBindingProvider.java:105)

Attached the completed server.log

@jansupol
Copy link
Contributor

Caused by: jakarta.json.bind.JsonbException: Unable to serialize property 'allowedMethods' from java.util.Vector
at org.eclipse.yasson.internal.serializer.ObjectSerializer.lambda$serialize$0(ObjectSerializer.java:43)
at java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:721)
at org.eclipse.yasson.internal.serializer.ObjectSerializer.serialize(ObjectSerializer.java:38)
at org.eclipse.yasson.internal.serializer.RecursionChecker.serialize(RecursionChecker.java:38)
at org.eclipse.yasson.internal.serializer.KeyWriter.serialize(KeyWriter.java:41)
at org.eclipse.yasson.internal.serializer.NullVisibilitySwitcher.serialize(NullVisibilitySwitcher.java:40)
at org.eclipse.yasson.internal.serializer.NullSerializer.serialize(NullSerializer.java:67)
at org.eclipse.yasson.internal.SerializationContextImpl.serializeObject(SerializationContextImpl.java:197)
at org.eclipse.yasson.internal.SerializationContextImpl.marshall(SerializationContextImpl.java:133)
at org.eclipse.yasson.internal.SerializationContextImpl.marshallWithoutClose(SerializationContextImpl.java:170)
at org.eclipse.yasson.internal.JsonBinding.toJson(JsonBinding.java:142)
at org.glassfish.jersey.jsonb.internal.JsonBindingProvider.writeTo(JsonBindingProvider.java:103)
... 32 more
Caused by: jakarta.json.bind.JsonbException: Error getting value on: [Todo{id=59eb3441-b774-4c4a-b1c6-c080fa0e2ea7, title='What's new in JPA 3.1?', completed=false}, Todo{id=e9fdd67f-c90d-4c1a-9f9b-05df8805bfb4, title='What's new in Jaxrs 3.1', completed=false}, Todo{id=160b83d3-5985-4063-a9df-cb1865fdf064, title='Learn new features in Faces 4.0', completed=false}]
at org.eclipse.yasson.internal.serializer.ValueGetterSerializer.serialize(ValueGetterSerializer.java:41)
at org.eclipse.yasson.internal.serializer.ObjectSerializer.lambda$serialize$0(ObjectSerializer.java:41)
... 43 more
Caused by: java.lang.ClassCastException: Cannot cast java.util.Vector to jakarta.ws.rs.core.Response
at java.base/java.lang.Class.cast(Class.java:3889)
at org.eclipse.yasson.internal.serializer.ValueGetterSerializer.serialize(ValueGetterSerializer.java:39)
... 44 more

This looks more like a Yasson issue.

@jansupol
Copy link
Contributor

The thing is that Jersey does not handle CompletableFuture<Response>, but it can handle CompletableFuture<List<Todo>>. If you change this, it will be working fine. Your example uses functionality not supported by Jersey, and Yasson has a bug in the error message, it should say Unable to serialize property 'allowedMethods' from Response.

@hantsy
Copy link
Author

hantsy commented Jul 21, 2022

Jsonb impl(Yasson) provides basic serialization for general purpose. But I do think it is the responsibility of Yasson.

As a Jaxrs impl, to support Async using Java 8 CompletionStage/CompletableFuture, Jersey should add its own serializer/desearializer for supporting CompletableFuture<Response>.

As you mentioned, to get a list(GET 200 status), use CompletableFuture<List<Todo>> is ok, how to handle 201 Created with created uri as locaiton header status and 204 no Content? In the async handling, I would like use CompletableFuture<Response> in all cases.

Quarkus supports such an approach in the early version, check my example CompletableFuture, in the new version, it is replaced by RactiveStreams(Small Mutiny).

In contrast to Spring, besides the async Completable<ResponseEntity>, it supports Spring WebFlux reactive Mono<ResponseEntity> and ResponseEntity<Mono or Flux> as return type.

So I think to support async in Jaxrs, it should add it own serializer to handle CompletableFuture<Response> and Response<CompetableFuture>, and for all JSON implementation providers(jsonb, jackson, json-p).

@jansupol
Copy link
Contributor

CompletionStage<Response> is fine to implement, Till then, for HTTP status other than 200, you can use AsyncResponse.

@hantsy
Copy link
Author

hantsy commented Jul 22, 2022

I would like use CompletionStage<Response> in all cases, like the sample codes in my legacy Quarkus sandboxs.

@GET
    @Produces(MediaType.APPLICATION_JSON)
    public CompletionStage<Response> getAllPosts() {
        return this.posts.findAll().thenApply(posts -> ok(posts).build());
    }

    @POST
    @Consumes(MediaType.APPLICATION_JSON)
    public CompletionStage<Response> savePost(@Valid Post post/*, @Context UriInfo uriInfo*/) {
        //uriInfo.getBaseUriBuilder().path("/posts/{id}").build(id.toString())
        return this.posts.save(post).thenApply(id -> created(URI.create("/posts/" + id.toString())).build());
    }

    @Path("{id}")
    @PUT
    @Consumes(MediaType.APPLICATION_JSON)
    public CompletionStage<Response> updatePost(@PathParam("id") final String id, @Valid Post post) {
        return this.posts.update(UUID.fromString(id), post)
                .thenApply(updated -> updated > 0 ? Status.NO_CONTENT : Status.NOT_FOUND)
                .thenApply(status -> status(status).build());
    }

    @Path("{id}")
    @GET
    @Produces(MediaType.APPLICATION_JSON)
    public CompletionStage<Response> getPostById(@PathParam("id") final String id) {
        return this.posts.findById(UUID.fromString(id))
                .thenApply(post -> ok(post).build())
                .exceptionally(throwable -> {
                    LOGGER.log(Level.WARNING, " failed to get post by id :", throwable);
                    return status(NOT_FOUND).build();
                });
    }

    @DELETE
    @Path("{id}")
    public CompletionStage<Response> delete(@PathParam("id") String id) {
        return this.posts.delete(id)
                .thenApply(deleted -> deleted > 0 ? Status.NO_CONTENT : Status.NOT_FOUND)
                .thenApply(status -> status(status).build());
    }

And we are moving to Java 11, Java 9 Flow(and ReactiveStreams, there are some utility available to convert from/to Java 9 Flow) Publisher also should be considered.

@jansupol jansupol linked a pull request Aug 1, 2022 that will close this issue
@jansupol jansupol added this to the 3.1.0 milestone Aug 1, 2022
@hantsy
Copy link
Author

hantsy commented Oct 1, 2022

It is not included in the Glassfish 7.0.0-M9? The detailed exception in Glassfish 7.0.0-M9: eclipse-ee4j/glassfish#24121

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants