Merge pull request square#1117 from square/jwilson_1102_cancel

Don't skip the callback if a call is canceled.
huntervwang · Nov 2, 2014 · 912d09e · 912d09e
2 parents 108fd5c + 5991ab1
commit 912d09e
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 13 deletions.
diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/CallTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/CallTest.java
@@ -1105,8 +1105,10 @@ public final class CallTest {
     server2.enqueue(new MockResponse().setBody("Page 2"));
     server2.play();
 
-    server.enqueue(new MockResponse().setResponseCode(401));
-    server.enqueue(new MockResponse().setResponseCode(302)
+    server.enqueue(new MockResponse()
+        .setResponseCode(401));
+    server.enqueue(new MockResponse()
+        .setResponseCode(302)
         .addHeader("Location: " + server2.getUrl("/b")));
     server.play();
 
@@ -1230,6 +1232,18 @@ public final class CallTest {
     assertEquals(0, server.getRequestCount());
   }
 
+  @Test public void cancelTagImmediatelyAfterEnqueue() throws Exception {
+    server.play();
+    Call call = client.newCall(new Request.Builder()
+        .url(server.getUrl("/a"))
+        .tag("request")
+        .build());
+    call.enqueue(callback);
+    client.cancel("request");
+    assertEquals(0, server.getRequestCount());
+    callback.await(server.getUrl("/a")).assertFailure("Canceled");
+  }
+
   @Test public void cancelBeforeBodyIsRead() throws Exception {
     server.enqueue(new MockResponse().setBody("def").throttleBody(1, 750, TimeUnit.MILLISECONDS));
     server.play();

diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/DispatcherTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/DispatcherTest.java
@@ -112,15 +112,6 @@ public final class DispatcherTest {
     executor.assertJobs("http://a/1");
   }
 
-  @Test public void cancelingReadyJobPreventsItFromStarting() throws Exception {
-    dispatcher.setMaxRequestsPerHost(1);
-    client.newCall(newRequest("http://a/1")).enqueue(callback);
-    client.newCall(newRequest("http://a/2", "tag1")).enqueue(callback);
-    dispatcher.cancel("tag1");
-    executor.finishJob("http://a/1");
-    executor.assertJobs();
-  }
-
   @Test public void cancelingRunningJobTakesNoEffectUntilJobFinishes() throws Exception {
     dispatcher.setMaxRequests(1);
     client.newCall(newRequest("http://a/1", "tag1")).enqueue(callback);

diff --git a/okhttp/src/main/java/com/squareup/okhttp/Call.java b/okhttp/src/main/java/com/squareup/okhttp/Call.java
@@ -150,6 +150,10 @@ Object tag() {
       return request.tag();
     }
 
+    void cancel() {
+      Call.this.cancel();
+    }
+
     Call get() {
       return Call.this;
     }

diff --git a/okhttp/src/main/java/com/squareup/okhttp/Dispatcher.java b/okhttp/src/main/java/com/squareup/okhttp/Dispatcher.java
@@ -115,8 +115,10 @@ synchronized void enqueue(AsyncCall call) {
 
   /** Cancel all calls with the tag {@code tag}. */
   public synchronized void cancel(Object tag) {
-    for (Iterator<AsyncCall> i = readyCalls.iterator(); i.hasNext(); ) {
-      if (Util.equal(tag, i.next().tag())) i.remove();
+    for (AsyncCall call : readyCalls) {
+      if (Util.equal(tag, call.tag())) {
+        call.cancel();
+      }
     }
 
     for (AsyncCall call : runningCalls) {