Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[android] - check for null body on http request, cleanup code
Browse files Browse the repository at this point in the history
  • Loading branch information
@tobrun
tobrun committed Mar 8, 2018
1 parent 8ed540e commit e3071ab
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 77 deletions.
164 changes: 88 additions & 76 deletions platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
import android.content.Context;
import android.content.pm.PackageInfo;
import android.os.Build;
import android.support.annotation.NonNull;
import android.text.TextUtils;
import android.util.Log;

import com.mapbox.mapboxsdk.BuildConfig;
import com.mapbox.mapboxsdk.Mapbox;
Expand All @@ -27,54 +29,42 @@
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
import okhttp3.ResponseBody;
import okhttp3.internal.Util;
import timber.log.Timber;

import static android.util.Log.DEBUG;
import static android.util.Log.ERROR;
import static android.util.Log.INFO;
import static android.util.Log.VERBOSE;
import static android.util.Log.WARN;

class HTTPRequest implements Callback {

private static OkHttpClient mClient = new OkHttpClient.Builder().dispatcher(getDispatcher()).build();
private static boolean logEnabled = true;
private static boolean logRequestUrl = false;

private String USER_AGENT_STRING = null;

private static final int CONNECTION_ERROR = 0;
private static final int TEMPORARY_ERROR = 1;
private static final int PERMANENT_ERROR = 2;

// Reentrancy is not needed, but "Lock" is an
// abstract class.
private ReentrantLock mLock = new ReentrantLock();

private long mNativePtr = 0;

private Call mCall;
private Request mRequest;

private static Dispatcher getDispatcher() {
Dispatcher dispatcher = new Dispatcher();
// Matches core limit set on
// https://github.com/mapbox/mapbox-gl-native/blob/master/platform/android/src/http_file_source.cpp#L192
dispatcher.setMaxRequestsPerHost(20);
return dispatcher;
}

private native void nativeOnFailure(int type, String message);
private static OkHttpClient client = new OkHttpClient.Builder().dispatcher(getDispatcher()).build();
private static boolean logEnabled = true;
private static boolean logRequestUrl = false;

private native void nativeOnResponse(int code, String etag, String modified, String cacheControl, String expires,
String retryAfter, String xRateLimitReset, byte[] body);
// Reentrancy is not needed, but "Lock" is an abstract class.
private ReentrantLock lock = new ReentrantLock();
private String userAgentString;
private long nativePtr = 0;
private Call call;

private HTTPRequest(long nativePtr, String resourceUrl, String etag, String modified) {
mNativePtr = nativePtr;
this.nativePtr = nativePtr;

try {
HttpUrl httpUrl = HttpUrl.parse(resourceUrl);
final String host = httpUrl.host().toLowerCase(MapboxConstants.MAPBOX_LOCALE);
if (httpUrl == null) {
log(Log.ERROR, String.format("[HTTP] Unable to parse resourceUrl %s", resourceUrl));
}

final String host = httpUrl.host().toLowerCase(MapboxConstants.MAPBOX_LOCALE);
// Don't try a request to remote server if we aren't connected
if (!Mapbox.isConnected() && !host.equals("127.0.0.1") && !host.equals("localhost")) {
throw new NoRouteToHostException("No Internet connection available.");
Expand All @@ -99,56 +89,59 @@ private HTTPRequest(long nativePtr, String resourceUrl, String etag, String modi
} else if (modified.length() > 0) {
builder = builder.addHeader("If-Modified-Since", modified);
}
mRequest = builder.build();
mCall = mClient.newCall(mRequest);
mCall.enqueue(this);
Request request = builder.build();
call = client.newCall(request);
call.enqueue(this);
} catch (Exception exception) {
handleFailure(mCall, exception);
handleFailure(call, exception);
}
}

public void cancel() {
// mCall can be null if the constructor gets aborted (e.g, under a NoRouteToHostException).
if (mCall != null) {
mCall.cancel();
// call can be null if the constructor gets aborted (e.g, under a NoRouteToHostException).
if (call != null) {
call.cancel();
}

// TODO: We need a lock here because we can try
// to cancel at the same time the request is getting
// answered on the OkHTTP thread. We could get rid of
// this lock by using Runnable when we move Android
// implementation of mbgl::RunLoop to Looper.
mLock.lock();
mNativePtr = 0;
mLock.unlock();
lock.lock();
nativePtr = 0;
lock.unlock();
}

@Override
public void onResponse(Call call, Response response) throws IOException {
public void onResponse(@NonNull Call call, @NonNull Response response) throws IOException {
if (response.isSuccessful()) {
log(VERBOSE, String.format("[HTTP] Request was successful (code = %s).", response.code()));
} else {
// We don't want to call this unsuccessful because a 304 isn't really an error
String message = !TextUtils.isEmpty(response.message()) ? response.message() : "No additional information";
log(DEBUG, String.format("[HTTP] Request with response code = %s: %s", response.code(), message));
}

if (logEnabled) {
if (response.isSuccessful()) {
Timber.v("[HTTP] Request was successful (code = %s).", response.code());
} else {
// We don't want to call this unsuccessful because a 304 isn't really an error
String message = !TextUtils.isEmpty(response.message()) ? response.message() : "No additional information";
Timber.d("[HTTP] Request with response code = %s: %s", response.code(), message);
}
ResponseBody responseBody = response.body();
if (responseBody == null) {
log(ERROR, "[HTTP] Received empty response body");
return;
}

byte[] body;
try {
body = response.body().bytes();
body = responseBody.bytes();
} catch (IOException ioException) {
onFailure(call, ioException);
// throw ioException;
return;
} finally {
response.body().close();
response.close();
}

mLock.lock();
if (mNativePtr != 0) {
lock.lock();
if (nativePtr != 0) {
nativeOnResponse(response.code(),
response.header("ETag"),
response.header("Last-Modified"),
Expand All @@ -158,14 +151,34 @@ public void onResponse(Call call, Response response) throws IOException {
response.header("x-rate-limit-reset"),
body);
}
mLock.unlock();
lock.unlock();
}

@Override
public void onFailure(Call call, IOException e) {
public void onFailure(@NonNull Call call, @NonNull IOException e) {
handleFailure(call, e);
}

static void enableLog(boolean enabled) {
logEnabled = enabled;
}

static void enablePrintRequestUrlOnFailure(boolean enabled) {
logRequestUrl = enabled;
}

static void setOKHttpClient(OkHttpClient client) {
HTTPRequest.client = client;
}

private static Dispatcher getDispatcher() {
Dispatcher dispatcher = new Dispatcher();
// Matches core limit set on
// https://github.com/mapbox/mapbox-gl-native/blob/master/platform/android/src/http_file_source.cpp#L192
dispatcher.setMaxRequestsPerHost(20);
return dispatcher;
}

private void handleFailure(Call call, Exception e) {
String errorMessage = e.getMessage() != null ? e.getMessage() : "Error processing the request";
int type = getFailureType(e);
Expand All @@ -175,11 +188,11 @@ private void handleFailure(Call call, Exception e) {
logFailure(type, errorMessage, requestUrl);
}

mLock.lock();
if (mNativePtr != 0) {
lock.lock();
if (nativePtr != 0) {
nativeOnFailure(type, errorMessage);
}
mLock.unlock();
lock.unlock();
}

private int getFailureType(Exception e) {
Expand All @@ -192,29 +205,35 @@ private int getFailureType(Exception e) {
return PERMANENT_ERROR;
}

private void log(int type, String errorMessage) {
if (logEnabled) {
Timber.log(type, errorMessage);
}
}

private void logFailure(int type, String errorMessage, String requestUrl) {
Timber.log(
type == TEMPORARY_ERROR ? DEBUG : type == CONNECTION_ERROR ? INFO : WARN,
"Request failed due to a %s error: %s %s",
type == TEMPORARY_ERROR ? "temporary" : type == CONNECTION_ERROR ? "connection" : "permanent",
errorMessage,
logRequestUrl ? requestUrl : ""
log(type == TEMPORARY_ERROR ? DEBUG : type == CONNECTION_ERROR ? INFO : WARN,
String.format(
"Request failed due to a %s error: %s %s",
type == TEMPORARY_ERROR ? "temporary" : type == CONNECTION_ERROR ? "connection" : "permanent",
errorMessage,
logRequestUrl ? requestUrl : ""
)
);
}

private String getUserAgent() {
if (USER_AGENT_STRING == null) {
return USER_AGENT_STRING = Util.toHumanReadableAscii(
if (userAgentString == null) {
userAgentString = Util.toHumanReadableAscii(
String.format("%s %s (%s) Android/%s (%s)",
getApplicationIdentifier(),
BuildConfig.MAPBOX_VERSION_STRING,
BuildConfig.GIT_REVISION_SHORT,
Build.VERSION.SDK_INT,
Build.CPU_ABI)
);
} else {
return USER_AGENT_STRING;
}
return userAgentString;
}

private String getApplicationIdentifier() {
Expand All @@ -227,15 +246,8 @@ private String getApplicationIdentifier() {
}
}

static void enableLog(boolean enabled) {
logEnabled = enabled;
}

static void enablePrintRequestUrlOnFailure(boolean enabled) {
logRequestUrl = enabled;
}
private native void nativeOnFailure(int type, String message);

static void setOKHttpClient(OkHttpClient client) {
mClient = client;
}
private native void nativeOnResponse(int code, String etag, String modified, String cacheControl, String expires,
String retryAfter, String xRateLimitReset, byte[] body);
}
2 changes: 1 addition & 1 deletion platform/android/src/http_file_source.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ void RegisterNativeHTTPRequest(jni::JNIEnv& env) {

#define METHOD(MethodPtr, name) jni::MakeNativePeerMethod<decltype(MethodPtr), (MethodPtr)>(name)

jni::RegisterNativePeer<HTTPRequest>(env, HTTPRequest::javaClass, "mNativePtr",
jni::RegisterNativePeer<HTTPRequest>(env, HTTPRequest::javaClass, "nativePtr",
METHOD(&HTTPRequest::onFailure, "nativeOnFailure"),
METHOD(&HTTPRequest::onResponse, "nativeOnResponse"));
}
Expand Down

0 comments on commit e3071ab

Please sign in to comment.