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

Response can not end async method result #31

Closed
tnguyen-lh opened this issue Jul 13, 2018 · 4 comments
Closed

Response can not end async method result #31

tnguyen-lh opened this issue Jul 13, 2018 · 4 comments

Comments

@tnguyen-lh
Copy link

tnguyen-lh commented Jul 13, 2018

Thank you for reply and close the last issue I have! There is something like this in Vertx-Web I can not do with rest.vertx:

In Vertx-Web I call async method like below and it is not blocking io, it doesn't need to run under worker thread:

public void test(Vertx vertx) {
		Router.router(vertx).post("/product/save").handler(ctx -> {
	        Product product = new Product();
			CompletableFuture<Product> result = productService.saveProduct(product);
			result.whenComplete((p, e) -> vertx.runOnContext(none -> {
	            ctx.response().end(Json.encodePrettily(p));
	        }));

	    });
	}

And this above code work fine, async and not blocking any thread.

In rest.vertx, I do this:

@POST
	@Path("/save")
	@Consumes(MediaType.APPLICATION_JSON)
	public HttpServerResponse save(@Context HttpServerResponse response, Product product) throws InterruptedException, ExecutionException {
		CompletableFuture<Product> result = productService.saveProduct(product);
		result.whenComplete((p, e) -> {
			System.out.println("Product: " + p);
			response.putHeader("content-type", "application/json; charset=utf-8")
		      .end(Json.encodePrettily(p));
		});
		
		return response;
		
	}

If I put break point in return response, when I hit /product/save, it return me the json result. However if I release break point, empty result produced.

I also try return void, but it return empty:

@POST
	@Path("/save")
	@Consumes(MediaType.APPLICATION_JSON)
	@Produces(MediaType.APPLICATION_JSON)
	public void save(@Context HttpServerResponse response, Product product) throws InterruptedException, ExecutionException {
		CompletableFuture<Product> result = productService.saveProduct(product);
		result.whenComplete((p, e) -> {
			System.out.println("Product: " + p);
			response.putHeader("content-type", "application/json; charset=utf-8")
		      .end(Json.encodePrettily(p));
		});
		
	}

The problem now is how to have response.end( result) in whenComplete() of Future, but still produce the result like Vertx-Web.
Thanks!

@drejc
Copy link
Member

drejc commented Jul 13, 2018

OK I found the problem ... it seems rest.vertx doesn't recognize CompletableFuture<> as a async response .... must fix this to make it work.

Apart from that if you use Future<> instead it will work:

Take a look at this example service:
https://github.com/zandero/rest.vertx.example/blob/master/src/main/java/com/zandero/rest/example/service/SlowServiceImpl.java

And the REST API:
https://github.com/zandero/rest.vertx.example/blob/master/src/main/java/com/zandero/rest/example/rest/SlowRest.java

Both REST calls are essentially doing the same thing ... with the difference that one is using the vert.x thread pool the other has a custom thread pool associated.

If we now run this JMeter test:
https://github.com/zandero/rest.vertx.example/blob/master/src/test/resources/jmeter.test

One REST will have a throughput of 10req/s the other 20req/s ... as one has a thread pool of 10 and the other a thread pool of 20 associated. In both cases each request takes 1s to complete.

Now rest.vertx is just a wrapper around vertx to ease REST set up ... it can not sprinkle magic and make more of it as it is. You are always limited by the number of threads that fulfil the requests one or the other way.

So I would skip the Future<> part completely and make something like this:

@POST
	@Path("/save")
	@Consumes(MediaType.APPLICATION_JSON)
	@Produces(MediaType.APPLICATION_JSON)
	public HttpServerResponse save(@Context HttpServerResponse response, Product product) throws InterruptedException, ExecutionException {
		Product result = productService.saveProduct(product);
			response.putHeader("content-type", "application/json; charset=utf-8")
return response;		
	}

Jet better would be to return Product and associate a writer to produce the correct response ...

@POST
	@Path("/save")
	@Consumes(MediaType.APPLICATION_JSON)
@ResponseWriter(MyProductWriter.class)
	public Product save(@Context HttpServerResponse response, Product product) throws InterruptedException, ExecutionException {
		return productService.saveProduct(product);
	}

@Produces(MediaType.APPLICATION_JSON)
public class MyProductWriter implements HttpResponseWriter<Product> {

	@Override
	public void write(Product product, HttpServerRequest request, HttpServerResponse response) throws FileNotFoundException {
                response.putHeader("content-type", "application/json; charset=utf-8")
	}
}

@drejc
Copy link
Member

drejc commented Jul 13, 2018

OK ... after looking into it ... this is actually not a bug.

CompletableFuture is a java.util.concurrent class ... rest.vertx support only vertx Future class implementations.

So if you want to use CompletableFuture you need to make sure you complete your action and then deliver the result to rest.vertx ...

Here is a working example:

@GET
	@Path("completable")
	@Produces(MediaType.APPLICATION_JSON)
	@ResponseWriter(FutureDummyWriter.class)
	public CompletableFuture<Dummy> complete(@Context Vertx vertx, @Context HttpServerResponse response) throws InterruptedException {

		System.out.println(Thread.currentThread().getName() + " REST invoked");

		CompletableFuture<Dummy> res = getFuture();
		System.out.println("Rest finished");
		return res;
	}

	private CompletableFuture<Dummy> getFuture() throws InterruptedException {

		CompletableFuture<Dummy> future = new CompletableFuture<>();
		future.complete(getDummy());
		return future;
	}

	private Dummy getDummy() throws InterruptedException {
		Thread.sleep(1000);
		return new Dummy("hello", "world");
	}

public class FutureDummyWriter implements HttpResponseWriter<CompletableFuture<Dummy>> {

	@Override
	public void write(CompletableFuture<Dummy> result, HttpServerRequest request, HttpServerResponse response) throws ExecutionException, InterruptedException {
		System.out.println("produce result");

		if (result != null){
			Dummy dummy = result.get(); // we wait to get the result

			response.setStatusCode(200)
			        .putHeader("content-type", "application/json")
			        .end("{\"name\": \"" + dummy.name + "\", \"value\": \"" + dummy.value + "\"}");
		}
		else {
			response.setStatusCode(204).end();
		}
	}
}

Still not sure why you would need to do this with a CompletableFuture ... but anyway it can be done.

@drejc drejc closed this as completed Jul 13, 2018
@tnguyen-lh
Copy link
Author

tnguyen-lh commented Jul 14, 2018

Thanks a lot drejc! I would like to participate in rest.vertx, do you have any task that you want me to do.
I am able to make it work using vertx future like this, confirmed this way is not blocking the main thread. The reason I have to use CompletableFuture because it is come from another api that return me CompletableFuture, and it has its own ExcecutorService and thread pool.

@POST
	@Path("/save")
	@Consumes(MediaType.APPLICATION_JSON)
	@Produces(MediaType.APPLICATION_JSON)
	public Future<Product> save(Product product) {
	    Future<Product> res = Future.future();

		CompletableFuture<Product> result = productService.saveProduct(product);
		result.whenComplete((p, e) -> {
			res.complete(p);
		});
		
		return res;
	}

Another way with plain vertx-web, also non blocking as well

public void save(RoutingContext routingContext) {
		Product product = routingContext.getBodyAsJson().mapTo(Product.class);
		CompletableFuture<Product> result;
		result = productService.saveProduct(product);
		result.thenAccept(p -> {
			System.out.println("Product: " + p);
			routingContext.response().putHeader("content-type", "application/json; charset=utf-8")
					.end(JsonObject.mapFrom(p).encodePrettily());
		});

	}

@drejc
Copy link
Member

drejc commented Jul 14, 2018

Sure,
you can take a look at the two open issues #25 and #26 ... they are both connected.

The problem #25 is that I have used a RestEasy style of regular expression and do (if I remember correctly) some transformation from the RestEasy format to the vertx reg ex format.

Now the parsing is fairly simple and does not cover using "\" and other path related special characters.
In PathConverter class is the logic for converting convert(String path) ... this is where a better solution is needed. PathConverterTest needs additional test for such cases.

and:
TestRegExRest.java there is a test REST endpoint commented out that currently does not work

add a test to:
RouteWithRegExTest.java

#26 ... can be fairly easy fixed when #25 is fixed.

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

No branches or pull requests

2 participants