From 7aa986553704f283d0a5c59062ab585f721f5525 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 8 Nov 2021 11:30:51 -0500 Subject: [PATCH 1/5] - adds support for cancelling request --- .../http/CoreHttpCallbackFutureWrapper.java | 13 +++++- .../graph/http/CoreHttpProvider.java | 6 ++- .../CoreHttpCallbackFutureWrapperTests.java | 46 +++++++++++++++++++ 3 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java diff --git a/src/main/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapper.java b/src/main/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapper.java index 1bb930411..f4e2c3753 100644 --- a/src/main/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapper.java +++ b/src/main/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapper.java @@ -1,9 +1,12 @@ package com.microsoft.graph.http; import java.io.IOException; - +import java.util.Objects; +import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; +import javax.annotation.Nonnull; + import okhttp3.Call; import okhttp3.Callback; import okhttp3.Response; @@ -12,6 +15,14 @@ * Wraps the HTTP execution in a future, not public by intention */ class CoreHttpCallbackFutureWrapper implements Callback { + public CoreHttpCallbackFutureWrapper(@Nonnull final Call call) { + Objects.requireNonNull(call); + future.whenComplete((r, ex) -> { + if (ex != null && (ex instanceof InterruptedException || ex instanceof CancellationException)) { + call.cancel(); + } + }); + } final CompletableFuture future = new CompletableFuture<>(); @Override public void onFailure(Call arg0, IOException arg1) { diff --git a/src/main/java/com/microsoft/graph/http/CoreHttpProvider.java b/src/main/java/com/microsoft/graph/http/CoreHttpProvider.java index 3b50842c5..6ffa45028 100644 --- a/src/main/java/com/microsoft/graph/http/CoreHttpProvider.java +++ b/src/main/java/com/microsoft/graph/http/CoreHttpProvider.java @@ -51,6 +51,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import okhttp3.Call; import okhttp3.MediaType; import okhttp3.OkHttpClient; import okhttp3.Request; @@ -375,8 +376,9 @@ private java.util.concurrent.CompletableFuture handler) throws ClientException { final Request coreHttpRequest = getHttpRequest(request, resultClass, serializable); - final CoreHttpCallbackFutureWrapper wrapper = new CoreHttpCallbackFutureWrapper(); - corehttpClient.newCall(coreHttpRequest).enqueue(wrapper); + final Call call = corehttpClient.newCall(coreHttpRequest); + final CoreHttpCallbackFutureWrapper wrapper = new CoreHttpCallbackFutureWrapper(call); + call.enqueue(wrapper); return wrapper.future.thenApply(r -> processResponse(r, request, resultClass, serializable, handler)); } /** diff --git a/src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java b/src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java new file mode 100644 index 000000000..3e1be2e93 --- /dev/null +++ b/src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java @@ -0,0 +1,46 @@ +package com.microsoft.graph.http; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; + +import java.io.IOException; +import java.util.concurrent.ExecutionException; + +import org.junit.jupiter.api.Test; + +import okhttp3.Call; +import okhttp3.Response; + +public class CoreHttpCallbackFutureWrapperTests { + + @Test + public void ThrowsIfCallIsNull() { + assertThrows(NullPointerException.class, () -> new CoreHttpCallbackFutureWrapper(null)); + } + boolean isCanceled = false; + + @Test + public void CancelsCall() { + var call = mock(Call.class); + doAnswer(i -> { + isCanceled = true; + return null; + }).when(call).cancel(); + var wrapper = new CoreHttpCallbackFutureWrapper(call); + wrapper.future.cancel(true); + assertTrue(isCanceled); + } + + @Test + public void ReturnsResponseWhenCompleted() throws IOException, InterruptedException, ExecutionException { + var call = mock(Call.class); + var response = mock(Response.class); + var wrapper = new CoreHttpCallbackFutureWrapper(call); + wrapper.onResponse(call, response); + assertEquals(response, wrapper.future.get()); + } + +} From f62354b4aff555a2088b962dbf69957f953c69a7 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 8 Nov 2021 11:38:05 -0500 Subject: [PATCH 2/5] - fixes spot bugs issues --- .../graph/http/CoreHttpCallbackFutureWrapperTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java b/src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java index 3e1be2e93..d4107b5a8 100644 --- a/src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java +++ b/src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java @@ -17,13 +17,13 @@ public class CoreHttpCallbackFutureWrapperTests { @Test - public void ThrowsIfCallIsNull() { + public void throwsIfCallIsNull() { assertThrows(NullPointerException.class, () -> new CoreHttpCallbackFutureWrapper(null)); } boolean isCanceled = false; @Test - public void CancelsCall() { + public void cancelsCall() { var call = mock(Call.class); doAnswer(i -> { isCanceled = true; @@ -35,7 +35,7 @@ public void CancelsCall() { } @Test - public void ReturnsResponseWhenCompleted() throws IOException, InterruptedException, ExecutionException { + public void returnsResponseWhenCompleted() throws IOException, InterruptedException, ExecutionException { var call = mock(Call.class); var response = mock(Response.class); var wrapper = new CoreHttpCallbackFutureWrapper(call); From 1e29f4363c569cc13771a1c17f78d51b52a4346b Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 8 Nov 2021 11:44:52 -0500 Subject: [PATCH 3/5] - code linting --- .../graph/http/CoreHttpCallbackFutureWrapperTests.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java b/src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java index d4107b5a8..83b1d1f54 100644 --- a/src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java +++ b/src/test/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapperTests.java @@ -14,16 +14,16 @@ import okhttp3.Call; import okhttp3.Response; -public class CoreHttpCallbackFutureWrapperTests { +class CoreHttpCallbackFutureWrapperTests { @Test - public void throwsIfCallIsNull() { + void throwsIfCallIsNull() { assertThrows(NullPointerException.class, () -> new CoreHttpCallbackFutureWrapper(null)); } boolean isCanceled = false; @Test - public void cancelsCall() { + void cancelsCall() { var call = mock(Call.class); doAnswer(i -> { isCanceled = true; @@ -35,7 +35,7 @@ public void cancelsCall() { } @Test - public void returnsResponseWhenCompleted() throws IOException, InterruptedException, ExecutionException { + void returnsResponseWhenCompleted() throws IOException, InterruptedException, ExecutionException { var call = mock(Call.class); var response = mock(Response.class); var wrapper = new CoreHttpCallbackFutureWrapper(call); From d00b2dbb2762b9ee5551ebe2179c652e1a142e8a Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 11 Nov 2021 15:36:55 -0500 Subject: [PATCH 4/5] - adds changelog entry for requests cancellation --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 995516452..b099f8eb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Added support for cancelling requests #361 + ### Changed - Bumps Azure Core from 1.20.0 to 1.22.0 #359, #360, #341, #342 From 505c699c10ab617a055488180d1b20ee05ad46a5 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 15 Nov 2021 08:37:05 -0500 Subject: [PATCH 5/5] - applies PR suggestions --- .../com/microsoft/graph/http/CoreHttpCallbackFutureWrapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapper.java b/src/main/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapper.java index f4e2c3753..4cb926660 100644 --- a/src/main/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapper.java +++ b/src/main/java/com/microsoft/graph/http/CoreHttpCallbackFutureWrapper.java @@ -15,6 +15,7 @@ * Wraps the HTTP execution in a future, not public by intention */ class CoreHttpCallbackFutureWrapper implements Callback { + final CompletableFuture future = new CompletableFuture<>(); public CoreHttpCallbackFutureWrapper(@Nonnull final Call call) { Objects.requireNonNull(call); future.whenComplete((r, ex) -> { @@ -23,7 +24,6 @@ public CoreHttpCallbackFutureWrapper(@Nonnull final Call call) { } }); } - final CompletableFuture future = new CompletableFuture<>(); @Override public void onFailure(Call arg0, IOException arg1) { future.completeExceptionally(arg1);