glassfish icon indicating copy to clipboard operation
glassfish copied to clipboard

CompletionStage return type in Rest

Open hantsy opened this issue 3 years ago • 2 comments

Environment Details

  • GlassFish Version (and build number): 7.0.0-M9
  • JDK version: 17
  • OS: Windows 64
  • Database: built-in Derby

I have request to support CompletionStage in REST, see https://github.com/eclipse-ee4j/jersey/issues/5103

But it still does not work in the latest Glassfish 7.0.0-M9.

Given the following resources.

@Path("todos")
@RequestScoped
public class TodoResources {

    private static final Logger LOGGER = Logger.getLogger(TodoResources.class.getName());

    @Context
    ResourceContext resourceContext;

    @Context
    UriInfo uriInfo;

    @Inject
    TodoService todoService;

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

    @POST
    public CompletionStage<Response> createTodo(Todo todo) throws Exception {
        return todoService.createAsync(todo)
                .thenApply(saved ->
                        Response.created(uriInfo.getBaseUriBuilder().path("todos/{id}").build(saved.getId())).build()
                );
    }
}

When running an Arquillian test TodoResourceTest to verify it, failed.

There is exceptions in the console.

ERROR] com.example.it.TodoResourceTest.testCreateTodo  Time elapsed: 0.525 s  <<< ERROR!
java.util.concurrent.CompletionException: org.opentest4j.AssertionFailedError: expected: <201> but was: <500>
        at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315)
        at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:320)
        at java.base/java.util.concurrent.CompletableFuture$UniAccept.tryFire(CompletableFuture.java:722)
        at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
        at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2147)
        at org.glassfish.jersey.client.JerseyInvocation$InvocationResponseCallback.completed(JerseyInvocation.java:1029)
        at org.glassfish.jersey.client.ClientRuntime.processResponse(ClientRuntime.java:229)
        at org.glassfish.jersey.client.ClientRuntime.access$200(ClientRuntime.java:62)
        at org.glassfish.jersey.client.ClientRuntime$2.lambda$response$0(ClientRuntime.java:173)
        at org.glassfish.jersey.internal.Errors$1.call(Errors.java:248)
        at org.glassfish.jersey.internal.Errors$1.call(Errors.java:244)
        at org.glassfish.jersey.internal.Errors.process(Errors.java:292)
        at org.glassfish.jersey.internal.Errors.process(Errors.java:274)
        at org.glassfish.jersey.internal.Errors.process(Errors.java:244)
        at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:288)
        at org.glassfish.jersey.client.ClientRuntime$2.response(ClientRuntime.java:173)
        at org.glassfish.jersey.client.internal.HttpUrlConnector.apply(HttpUrlConnector.java:278)
        at org.glassfish.jersey.client.ClientRuntime.lambda$createRunnableForAsyncProcessing$6(ClientRuntime.java:182)
        at org.glassfish.jersey.internal.Errors$1.call(Errors.java:248)
        at org.glassfish.jersey.internal.Errors$1.call(Errors.java:244)
        at org.glassfish.jersey.internal.Errors.process(Errors.java:292)
        at org.glassfish.jersey.internal.Errors.process(Errors.java:274)
        at org.glassfish.jersey.internal.Errors.process(Errors.java:244)
        at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:288)
        at org.glassfish.jersey.client.ClientRuntime.lambda$createRunnableForAsyncProcessing$7(ClientRuntime.java:156)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: org.opentest4j.AssertionFailedError: expected: <201> but was: <500>
        at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
        at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
        at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
        at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
        at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:528)
        at com.example.it.TodoResourceTest.lambda$testCreateTodo$1(TodoResourceTest.java:115)
        at java.base/java.util.concurrent.CompletableFuture$UniAccept.tryFire(CompletableFuture.java:718)
        ... 27 more

The complete server log when running the tests: server.log

hantsy avatar Oct 01 '22 02:10 hantsy

I do not know what todoService.getAllTodosAsync() looks like, but the following (a bit artificial code to follow your example)

    @GET
    public CompletionStage<Response> getAllTodos() {
        var todos = todoService.findAll();
        return CompletableFuture.supplyAsync(() -> todos).thenApply(ts -> Response.ok(ts).build());
    }

works well.

jansupol avatar Oct 12 '22 11:10 jansupol

@jansupol Do you used GF7 M9 without updating the jersey jars?

hantsy avatar Oct 13 '22 00:10 hantsy

The exception cause you report is >java.lang.IllegalStateException: Not inside a request scope.

It is caused by the CompletableFuture, which runs uriInfo.getBaseUriBuilder() in its own thread. But the thread is not aware of the ThreadLocal RequestScopes, so the scope is not found. I do not think we may be able to fix this on our side. The change can be done on the app level:

    @POST
    public CompletionStage<Response> createTodo(Todo todo) throws Exception {
        UriBuilder builder = uriInfo.getBaseUriBuilder();
        return todoService.createAsync(todo)
                .thenApply(saved ->
                        Response.created(builder.path("todos/{id}").build(saved.getId())).build()
                );
    }

works well.

jansupol avatar Oct 19 '22 15:10 jansupol

I do not know what todoService.getAllTodosAsync() looks like, but the following (a bit artificial code to follow your example)

    @GET
    public CompletionStage<Response> getAllTodos() {
        var todos = todoService.findAll();
        return CompletableFuture.supplyAsync(() -> todos).thenApply(ts -> Response.ok(ts).build());
    }

works well.

Yes, your example works.

My todoService.getAllTodosAsync() code fragment is like the following, it returns a @Asynchronous annotated CompletableFuture in the TodoService, and executorService is a customized ExecutorService.

@Asynchronous
CompletableFuture<List<Todo>> getAllTodosAsync() {
    return CompletableFuture
            .supplyAsync(
                    () -> entityManager.createQuery("select t from Todo t", Todo.class).getResultList(),
                    executorService
            );
}

hantsy avatar Nov 06 '22 10:11 hantsy

But the thread is not aware of the ThreadLocal RequestScopes, so the scope is not found.

Why the context can not be propagated automatically between threads like Spring framework.

hantsy avatar Nov 06 '22 10:11 hantsy

In Spring, I assume the random customer-provided thread is not used, the way you use it in your example. In the case you reference, there would be a Spring thread factory used I assume?

jansupol avatar Nov 06 '22 18:11 jansupol

OK, let's have a look at an example of Spring and Jaxrs.

I have created a sample before to demo @Async in Spring, https://github.com/hantsy/spring-puzzles/tree/master/webmvc-async

The repository and controller is like this. The executor is also can be customized.

// Repository 
public interface PostRepository extends JpaRepository<Post, Long> {

    @Async
    CompletableFuture<List<Post>> readAllBy();

}

// controller codes
@RestController
@RequestMapping(value = "/posts")
@RequiredArgsConstructor
public class PostController {
    private final PostRepository posts;

    @GetMapping
    public CompletableFuture<List<Post>> all() {
        return this.posts.readAllBy();
    }

}

Compare to the Jaxrs/Jakarta EE 10 example here.

// in a CDI bean
@Asynchronous
CompletableFuture<List<Todo>> getAllTodosAsync() {
    return CompletableFuture
            .supplyAsync(
                    () -> entityManager.createQuery("select t from Todo t", Todo.class).getResultList(),
                    executorService
            );
}

// in a Jaxrs resource
@GET
public CompletableFuture<Response> getAllTodos() {
    return todoService.getAllTodosAsync().thenApply(todos -> Response.ok(todos).build());
}

If the CDI bean used custom Asynchronous ContextService, ExecutorService, ThreadFactory, etc can not be called freely, what is the meaning of concurrency?

hantsy avatar Nov 07 '22 02:11 hantsy

But all these things you mention here work fine. The request-scoped specific functionality uriInfo.getBaseUriBuilder() does not work in the new thread; the new thread does not know about the request and hence the base.

jansupol avatar Nov 07 '22 06:11 jansupol

But this code fragment does not work.

// in a CDI bean
@Asynchronous
CompletableFuture<List<Todo>> getAllTodosAsync() {
    return CompletableFuture
            .supplyAsync(
                    () -> entityManager.createQuery("select t from Todo t", Todo.class).getResultList(),
                    executorService
            );
}

// in a Jaxrs resource
@GET
public CompletableFuture<Response> getAllTodos() {
    return todoService.getAllTodosAsync().thenApply(todos -> Response.ok(todos).build());
}

hantsy avatar Nov 08 '22 10:11 hantsy

Your Jakarta EE and Spring code isn't the same. Your Spring code returns a list of Post entities, which are then mapped by Jersey to the response. Your EE code returns a Response object, uses the injected uriInfo reference, which isn't available in the synchronous thread.

You can rewrite your code to use the uriInfo reference in the original thread and pass the value of uriInfo.getBaseUriBuilder() to the other thread, like this:

@POST
    public CompletionStage<Response> createTodo(Todo todo) throws Exception {
        final UriBuilder uriBuilder = uriInfo.getBaseUriBuilder();
        return todoService.createAsync(todo)
                .thenApply(saved ->
                        Response.created( uriBuilder.path("todos/{id}").build(saved.getId())).build()
                );
    }

I think that the path and build methods of UriBuilder shouldn't need to have access to a request scope.

OndroMih avatar Nov 08 '22 13:11 OndroMih

There are several examples demonstrates different combinations in my Jakarta EE example.

I think what I talked is a different example in the Jakarta EE and Spring comparison, https://github.com/eclipse-ee4j/glassfish/issues/24121#issuecomment-1304991289, there is no UriBuilder in this example.

hantsy avatar Nov 08 '22 13:11 hantsy

OK, I have updated my Jakarta EE Jaxrs Example.

See the following codes, I marked the not working on the method.

@Path("todos")
@RequestScoped
public class TodoResources {

    private static final Logger LOGGER = Logger.getLogger(TodoResources.class.getName());

    @Context
    ResourceContext resourceContext;

    @Context
    UriInfo uriInfo;

    @Inject
    TodoService todoService;

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

    @GET
    @Path("async1") // works
    public CompletableFuture<Response> getAllTodosAndAsync() {
        var todos = todoService.getAllTodos();
        return CompletableFuture.supplyAsync(() -> todos).thenApply(data -> Response.ok(data).build());
    }

    @POST // extract the UriBuilder, it works
    public CompletionStage<Response> createTodo(Todo todo) throws Exception {
        var uriBuilder = uriInfo.getBaseUriBuilder();
        return todoService.createAsync(todo)
                .thenApply(saved -> Response.created(uriBuilder.path("todos/{id}").build(saved.getId())).build());
    }
}

The todoService.getAllTodosAsync() uses a custom ExectorService from the concurrency spec.

hantsy avatar Nov 08 '22 14:11 hantsy

What exception you get in server.log now?

OndroMih avatar Nov 08 '22 14:11 OndroMih

This issue has been marked as inactive and old and will be closed in 7 days if there is no further activity. If you want the issue to remain open please add a comment

github-actions[bot] avatar Nov 09 '23 01:11 github-actions[bot]