Skip to content

Commit a387df4

Browse files
committed
Merge pull request android-async-http#675 from zxw1962/t/work
Get rid of null check of responseHandler, also add proper synchronizatio...
2 parents d3d577a + 29afb2c commit a387df4

File tree

3 files changed

+30
-29
lines changed

3 files changed

+30
-29
lines changed

library/src/main/java/com/loopj/android/http/AsyncHttpRequest.java

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.io.IOException;
3030
import java.net.MalformedURLException;
3131
import java.net.UnknownHostException;
32+
import java.util.concurrent.atomic.AtomicBoolean;
3233

3334
/**
3435
* Internal class, representing the HttpRequest, done in asynchronous manner
@@ -39,16 +40,16 @@ public class AsyncHttpRequest implements Runnable {
3940
private final HttpUriRequest request;
4041
private final ResponseHandlerInterface responseHandler;
4142
private int executionCount;
42-
private boolean isCancelled;
43+
private final AtomicBoolean isCancelled = new AtomicBoolean();
4344
private boolean cancelIsNotified;
44-
private boolean isFinished;
45+
private volatile boolean isFinished;
4546
private boolean isRequestPreProcessed;
4647

4748
public AsyncHttpRequest(AbstractHttpClient client, HttpContext context, HttpUriRequest request, ResponseHandlerInterface responseHandler) {
48-
this.client = client;
49-
this.context = context;
50-
this.request = request;
51-
this.responseHandler = responseHandler;
49+
this.client = Utils.notNull(client, "client");
50+
this.context = Utils.notNull(context, "context");
51+
this.request = Utils.notNull(request, "request");
52+
this.responseHandler = Utils.notNull(responseHandler, "responseHandler");
5253
}
5354

5455
/**
@@ -97,9 +98,7 @@ public void run() {
9798
return;
9899
}
99100

100-
if (responseHandler != null) {
101-
responseHandler.sendStartMessage();
102-
}
101+
responseHandler.sendStartMessage();
103102

104103
if (isCancelled()) {
105104
return;
@@ -108,20 +107,18 @@ public void run() {
108107
try {
109108
makeRequestWithRetries();
110109
} catch (IOException e) {
111-
if (!isCancelled() && responseHandler != null) {
110+
if (!isCancelled()) {
112111
responseHandler.sendFailureMessage(0, null, null, e);
113112
} else {
114-
Log.e("AsyncHttpRequest", "makeRequestWithRetries returned error, but handler is null", e);
113+
Log.e("AsyncHttpRequest", "makeRequestWithRetries returned error", e);
115114
}
116115
}
117116

118117
if (isCancelled()) {
119118
return;
120119
}
121120

122-
if (responseHandler != null) {
123-
responseHandler.sendFinishMessage();
124-
}
121+
responseHandler.sendFinishMessage();
125122

126123
if (isCancelled()) {
127124
return;
@@ -150,7 +147,7 @@ private void makeRequest() throws IOException {
150147

151148
HttpResponse response = client.execute(request, context);
152149

153-
if (isCancelled() || responseHandler == null) {
150+
if (isCancelled()) {
154151
return;
155152
}
156153

@@ -201,7 +198,7 @@ private void makeRequestWithRetries() throws IOException {
201198
cause = e;
202199
retry = retryHandler.retryRequest(cause, ++executionCount, context);
203200
}
204-
if (retry && (responseHandler != null)) {
201+
if (retry) {
205202
responseHandler.sendRetryMessage(executionCount);
206203
}
207204
}
@@ -216,17 +213,17 @@ private void makeRequestWithRetries() throws IOException {
216213
}
217214

218215
public boolean isCancelled() {
219-
if (isCancelled) {
216+
boolean cancelled = isCancelled.get();
217+
if (cancelled) {
220218
sendCancelNotification();
221219
}
222-
return isCancelled;
220+
return cancelled;
223221
}
224222

225223
private synchronized void sendCancelNotification() {
226-
if (!isFinished && isCancelled && !cancelIsNotified) {
224+
if (!isFinished && isCancelled.get() && !cancelIsNotified) {
227225
cancelIsNotified = true;
228-
if (responseHandler != null)
229-
responseHandler.sendCancelMessage();
226+
responseHandler.sendCancelMessage();
230227
}
231228
}
232229

@@ -235,7 +232,7 @@ public boolean isDone() {
235232
}
236233

237234
public boolean cancel(boolean mayInterruptIfRunning) {
238-
isCancelled = true;
235+
isCancelled.set(true);
239236
request.abort();
240237
return isCancelled();
241238
}

library/src/main/java/com/loopj/android/http/AsyncHttpResponseHandler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ protected void sendMessage(Message msg) {
357357
if (getUseSynchronousMode() || handler == null) {
358358
handleMessage(msg);
359359
} else if (!Thread.currentThread().isInterrupted()) { // do not send messages if request has been cancelled
360-
AssertUtils.asserts(handler != null, "handler should not be null!");
360+
Utils.asserts(handler != null, "handler should not be null!");
361361
handler.sendMessage(msg);
362362
}
363363
}
@@ -374,7 +374,7 @@ protected void postRunnable(Runnable runnable) {
374374
runnable.run();
375375
} else {
376376
// Otherwise, run on provided handler
377-
AssertUtils.asserts(handler != null, "handler should not be null!");
377+
Utils.asserts(handler != null, "handler should not be null!");
378378
handler.post(runnable);
379379
}
380380
}

library/src/main/java/com/loopj/android/http/AssertUtils.java renamed to library/src/main/java/com/loopj/android/http/Utils.java

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,21 @@
1818

1919
package com.loopj.android.http;
2020

21-
/**
22-
* Internal class, used to make some asserts, throw AssertError if asserts fail.
23-
*/
24-
class AssertUtils {
21+
class Utils {
2522

26-
private AssertUtils() {
23+
private Utils() {
2724
}
2825

2926
public static void asserts(final boolean expression, final String failedMessage) {
3027
if (!expression) {
3128
throw new AssertionError(failedMessage);
3229
}
3330
}
31+
32+
public static <T> T notNull(final T argument, final String name) {
33+
if (argument == null) {
34+
throw new IllegalArgumentException(name + " should not be null!");
35+
}
36+
return argument;
37+
}
3438
}

0 commit comments

Comments
 (0)