Skip to content
This repository was archived by the owner on May 28, 2018. It is now read-only.

Commit 60f3fa3

Browse files
Miroslav FuksaGerrit Code Review
authored andcommitted
Merge "JERSEY-2162 Cannot cancel pending async request"
2 parents 2cbceba + faa1d34 commit 60f3fa3

File tree

3 files changed

+155
-4
lines changed

3 files changed

+155
-4
lines changed

core-client/src/main/java/org/glassfish/jersey/client/JerseyInvocation.java

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.util.HashMap;
4545
import java.util.Locale;
4646
import java.util.Map;
47+
import java.util.concurrent.CancellationException;
4748
import java.util.concurrent.Future;
4849
import java.util.logging.Logger;
4950

@@ -708,12 +709,18 @@ public Future<Response> submit() {
708709

709710
@Override
710711
public void completed(ClientResponse response, RequestScope scope) {
711-
responseFuture.set(new InboundJaxrsResponse(response, scope));
712+
if (!responseFuture.isCancelled()) {
713+
responseFuture.set(new InboundJaxrsResponse(response, scope));
714+
} else {
715+
response.close();
716+
}
712717
}
713718

714719
@Override
715720
public void failed(ProcessingException error) {
716-
responseFuture.setException(error);
721+
if (!responseFuture.isCancelled()) {
722+
responseFuture.setException(error);
723+
}
717724
}
718725
});
719726

@@ -730,6 +737,10 @@ public <T> Future<T> submit(final Class<T> responseType) {
730737

731738
@Override
732739
public void completed(ClientResponse response, RequestScope scope) {
740+
if (responseFuture.isCancelled()) {
741+
response.close();
742+
return;
743+
}
733744
try {
734745
responseFuture.set(translate(response, scope, responseType));
735746
} catch (ProcessingException ex) {
@@ -739,6 +750,9 @@ public void completed(ClientResponse response, RequestScope scope) {
739750

740751
@Override
741752
public void failed(ProcessingException error) {
753+
if (responseFuture.isCancelled()) {
754+
return;
755+
}
742756
if (error.getCause() instanceof WebApplicationException) {
743757
responseFuture.setException(error.getCause());
744758
} else {
@@ -781,6 +795,11 @@ public <T> Future<T> submit(final GenericType<T> responseType) {
781795

782796
@Override
783797
public void completed(ClientResponse response, RequestScope scope) {
798+
if (responseFuture.isCancelled()) {
799+
response.close();
800+
return;
801+
}
802+
784803
try {
785804
responseFuture.set(translate(response, scope, responseType));
786805
} catch (ProcessingException ex) {
@@ -790,6 +809,9 @@ public void completed(ClientResponse response, RequestScope scope) {
790809

791810
@Override
792811
public void failed(ProcessingException error) {
812+
if (responseFuture.isCancelled()) {
813+
return;
814+
}
793815
if (error.getCause() instanceof WebApplicationException) {
794816
responseFuture.setException(error.getCause());
795817
} else {
@@ -838,6 +860,12 @@ public <T> Future<T> submit(final InvocationCallback<T> callback) {
838860

839861
@Override
840862
public void completed(ClientResponse response, RequestScope scope) {
863+
if (responseFuture.isCancelled()) {
864+
response.close();
865+
failed(new ProcessingException(new CancellationException(LocalizationMessages.ERROR_REQUEST_CANCELLED())));
866+
return;
867+
}
868+
841869
final T result;
842870
if (callbackParamClass == Response.class) {
843871
result = callbackParamClass.cast(new InboundJaxrsResponse(response, scope));
@@ -857,11 +885,11 @@ public void failed(ProcessingException error) {
857885
try {
858886
if (error.getCause() instanceof WebApplicationException) {
859887
responseFuture.setException(error.getCause());
860-
} else {
888+
} else if (!responseFuture.isCancelled()) {
861889
responseFuture.setException(error);
862890
}
863891
} finally {
864-
callback.failed(error);
892+
callback.failed(error.getCause() instanceof CancellationException ? error.getCause() : error);
865893
}
866894
}
867895
};

core-client/src/main/resources/org/glassfish/jersey/client/internal/localization.properties

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,4 @@ unexpected.error.response.processing=Unexpected error during response processing
8989
use.encoding.ignored=Value {1} of {0} client property will be ignored as it is not a valid supported encoding. \
9090
Valid supported encodings are: {2}
9191
using.fixed.async.threadpool=Using fixed-size thread pool of size [{0}] for asynchronous client invocations.
92+
error.request.cancelled=Request cancelled by the client call.
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/*
2+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
3+
*
4+
* Copyright (c) 2014 Oracle and/or its affiliates. All rights reserved.
5+
*
6+
* The contents of this file are subject to the terms of either the GNU
7+
* General Public License Version 2 only ("GPL") or the Common Development
8+
* and Distribution License("CDDL") (collectively, the "License"). You
9+
* may not use this file except in compliance with the License. You can
10+
* obtain a copy of the License at
11+
* http://glassfish.java.net/public/CDDL+GPL_1_1.html
12+
* or packager/legal/LICENSE.txt. See the License for the specific
13+
* language governing permissions and limitations under the License.
14+
*
15+
* When distributing the software, include this License Header Notice in each
16+
* file and include the License file at packager/legal/LICENSE.txt.
17+
*
18+
* GPL Classpath Exception:
19+
* Oracle designates this particular file as subject to the "Classpath"
20+
* exception as provided by Oracle in the GPL Version 2 section of the License
21+
* file that accompanied this code.
22+
*
23+
* Modifications:
24+
* If applicable, add the following below the License Header, with the fields
25+
* enclosed by brackets [] replaced by your own identifying information:
26+
* "Portions Copyright [year] [name of copyright owner]"
27+
*
28+
* Contributor(s):
29+
* If you wish your version of this file to be governed by only the CDDL or
30+
* only the GPL Version 2, indicate your decision by adding "[Contributor]
31+
* elects to include this software in this distribution under the [CDDL or GPL
32+
* Version 2] license." If you don't indicate a single choice of license, a
33+
* recipient has the option to distribute your version of this file under
34+
* either the CDDL, the GPL Version 2 or to extend the choice of license to
35+
* its licensees as provided above. However, if you add GPL Version 2 code
36+
* and therefore, elected the GPL Version 2 license, then the option applies
37+
* only if the new code is made subject to such option by the copyright
38+
* holder.
39+
*/
40+
41+
package org.glassfish.jersey.tests.e2e.client;
42+
43+
import org.glassfish.jersey.server.ResourceConfig;
44+
import org.glassfish.jersey.test.JerseyTest;
45+
import org.junit.Test;
46+
47+
import javax.ws.rs.GET;
48+
import javax.ws.rs.Path;
49+
import javax.ws.rs.client.InvocationCallback;
50+
import javax.ws.rs.core.Application;
51+
import javax.ws.rs.core.Response;
52+
import java.util.concurrent.CancellationException;
53+
import java.util.concurrent.CountDownLatch;
54+
import java.util.concurrent.Future;
55+
import java.util.concurrent.TimeUnit;
56+
import java.util.concurrent.TimeoutException;
57+
import java.util.concurrent.locks.Condition;
58+
import java.util.concurrent.locks.ReentrantLock;
59+
60+
import static org.junit.Assert.assertEquals;
61+
import static org.junit.Assert.assertTrue;
62+
import static org.junit.Assert.fail;
63+
64+
/**
65+
* Tests the behaviour of the Async client when the {@link java.util.concurrent.Future} is cancelled.
66+
*
67+
* <p>
68+
* Tests, that if the async request future is cancelled by the client,
69+
* the {@link javax.ws.rs.client.InvocationCallback#completed(Object)} callback is not invoked and that
70+
* {@link java.util.concurrent.CancellationException} is correctly returned (according to spec.) to
71+
* {@link javax.ws.rs.client.InvocationCallback#failed(Throwable)} callback method.
72+
* </p>
73+
*
74+
* @author Adam Lindenthal (adam.lindenthal at oracle.com)
75+
*/
76+
public class CancelFutureClientTest extends JerseyTest {
77+
78+
public static final long MAX_WAITING_SECONDS = 2L;
79+
final CountDownLatch countDownLatch = new CountDownLatch(1);
80+
81+
@Override
82+
protected Application configure() {
83+
return new ResourceConfig(TestResource.class);
84+
}
85+
86+
@Test
87+
public void testCancelFuture() throws InterruptedException, TimeoutException {
88+
Future<Response> future = target().path("test").request().async().get(
89+
new InvocationCallback<Response>() {
90+
public void completed(final Response response) {
91+
fail("[completed()] callback was invoked, although the Future should have been cancelled.");
92+
}
93+
94+
public void failed(final Throwable throwable) {
95+
assertEquals(CancellationException.class, throwable.getClass());
96+
countDownLatch.countDown();
97+
}
98+
}
99+
);
100+
if (!future.cancel(true)) {
101+
fail("The Future could not be canceled.");
102+
}
103+
104+
// prevent the test container to stop the method execution before the callbacks can be reached.
105+
if (!countDownLatch.await(MAX_WAITING_SECONDS, TimeUnit.SECONDS)) {
106+
throw new TimeoutException("Callback was not triggered within the time limit." + countDownLatch.getCount());
107+
}
108+
}
109+
110+
@Path("test")
111+
public static class TestResource {
112+
@GET
113+
public Response get() {
114+
try {
115+
Thread.sleep(1000);
116+
} catch (InterruptedException e) {
117+
e.printStackTrace();
118+
}
119+
return Response.noContent().build();
120+
}
121+
}
122+
}

0 commit comments

Comments
 (0)