From 225c7b960ae526bf904e7e5337b341b024cabaf0 Mon Sep 17 00:00:00 2001 From: Tobrun Van Nuland Date: Thu, 24 Aug 2017 14:28:39 +0200 Subject: [PATCH] Revert "[android] - configure loggin of HttpRequest, cleanup HttpRequest" This reverts commit a059c74a54302f9604f3e3b28143be7645e6204f. --- .../mapbox/mapboxsdk/http/HTTPRequest.java | 200 +++++++++++++++ .../mapbox/mapboxsdk/http/HttpRequest.java | 238 ------------------ .../mapboxsdk/http/HttpRequestUtil.java | 19 -- .../activity/offline/OfflineActivity.java | 3 - platform/android/src/http_file_source.cpp | 4 +- 5 files changed, 202 insertions(+), 262 deletions(-) create mode 100644 platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java delete mode 100644 platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HttpRequest.java delete mode 100644 platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HttpRequestUtil.java diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java new file mode 100644 index 00000000000..91a235616a4 --- /dev/null +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java @@ -0,0 +1,200 @@ +package com.mapbox.mapboxsdk.http; + + +import android.content.Context; +import android.content.pm.PackageInfo; +import android.os.Build; +import android.text.TextUtils; + +import com.mapbox.mapboxsdk.BuildConfig; +import com.mapbox.mapboxsdk.Mapbox; +import com.mapbox.mapboxsdk.constants.MapboxConstants; + +import java.io.IOException; +import java.io.InterruptedIOException; +import java.net.NoRouteToHostException; +import java.net.ProtocolException; +import java.net.SocketException; +import java.net.UnknownHostException; +import java.util.concurrent.locks.ReentrantLock; + +import javax.net.ssl.SSLException; + +import okhttp3.Call; +import okhttp3.Callback; +import okhttp3.HttpUrl; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.Response; +import okhttp3.internal.Util; +import timber.log.Timber; + +class HTTPRequest implements Callback { + + private static OkHttpClient mClient = new OkHttpClient(); + 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 native void nativeOnFailure(int type, String message); + + private native void nativeOnResponse(int code, String etag, String modified, String cacheControl, String expires, + String retryAfter, String xRateLimitReset, byte[] body); + + private HTTPRequest(long nativePtr, String resourceUrl, String etag, String modified) { + mNativePtr = nativePtr; + + try { + // Don't try a request if we aren't connected + if (!Mapbox.isConnected()) { + throw new NoRouteToHostException("No Internet connection available."); + } + + HttpUrl httpUrl = HttpUrl.parse(resourceUrl); + final String host = httpUrl.host().toLowerCase(MapboxConstants.MAPBOX_LOCALE); + if (host.equals("mapbox.com") || host.endsWith(".mapbox.com") || host.equals("mapbox.cn") + || host.endsWith(".mapbox.cn")) { + if (httpUrl.querySize() == 0) { + resourceUrl = resourceUrl + "?"; + } else { + resourceUrl = resourceUrl + "&"; + } + resourceUrl = resourceUrl + "events=true"; + } + + Request.Builder builder = new Request.Builder() + .url(resourceUrl) + .tag(resourceUrl.toLowerCase(MapboxConstants.MAPBOX_LOCALE)) + .addHeader("User-Agent", getUserAgent()); + if (etag.length() > 0) { + builder = builder.addHeader("If-None-Match", etag); + } else if (modified.length() > 0) { + builder = builder.addHeader("If-Modified-Since", modified); + } + mRequest = builder.build(); + mCall = mClient.newCall(mRequest); + mCall.enqueue(this); + } catch (Exception exception) { + onFailure(exception); + } + } + + public void cancel() { + // mCall can be null if the constructor gets aborted (e.g, under a NoRouteToHostException). + if (mCall != null) { + mCall.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(); + } + + @Override + public void onResponse(Call call, Response response) throws IOException { + 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); + } + + byte[] body; + try { + body = response.body().bytes(); + } catch (IOException ioException) { + onFailure(ioException); + // throw ioException; + return; + } finally { + response.body().close(); + } + + mLock.lock(); + if (mNativePtr != 0) { + nativeOnResponse(response.code(), + response.header("ETag"), + response.header("Last-Modified"), + response.header("Cache-Control"), + response.header("Expires"), + response.header("Retry-After"), + response.header("x-rate-limit-reset"), + body); + } + mLock.unlock(); + } + + @Override + public void onFailure(Call call, IOException e) { + onFailure(e); + } + + private void onFailure(Exception e) { + int type = PERMANENT_ERROR; + if ((e instanceof NoRouteToHostException) || (e instanceof UnknownHostException) || (e instanceof SocketException) + || (e instanceof ProtocolException) || (e instanceof SSLException)) { + type = CONNECTION_ERROR; + } else if ((e instanceof InterruptedIOException)) { + type = TEMPORARY_ERROR; + } + + String errorMessage = e.getMessage() != null ? e.getMessage() : "Error processing the request"; + + if (type == TEMPORARY_ERROR) { + Timber.d("Request failed due to a temporary error: %s", errorMessage); + } else if (type == CONNECTION_ERROR) { + Timber.i("Request failed due to a connection error: %s", errorMessage); + } else { + // PERMANENT_ERROR + Timber.w("Request failed due to a permanent error: %s", errorMessage); + } + + mLock.lock(); + if (mNativePtr != 0) { + nativeOnFailure(type, errorMessage); + } + mLock.unlock(); + } + + private String getUserAgent() { + if (USER_AGENT_STRING == null) { + return USER_AGENT_STRING = 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; + } + } + + private String getApplicationIdentifier() { + try { + Context context = Mapbox.getApplicationContext(); + PackageInfo packageInfo = context.getPackageManager().getPackageInfo(context.getPackageName(), 0); + return String.format("%s/%s (%s)", context.getPackageName(), packageInfo.versionName, packageInfo.versionCode); + } catch (Exception exception) { + return ""; + } + } +} diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HttpRequest.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HttpRequest.java deleted file mode 100644 index 4f899cf6961..00000000000 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HttpRequest.java +++ /dev/null @@ -1,238 +0,0 @@ -package com.mapbox.mapboxsdk.http; - -import android.content.Context; -import android.content.pm.PackageInfo; -import android.os.Build; -import android.support.annotation.IntDef; -import android.support.annotation.NonNull; -import android.text.TextUtils; - -import com.mapbox.mapboxsdk.BuildConfig; -import com.mapbox.mapboxsdk.Mapbox; -import com.mapbox.mapboxsdk.constants.MapboxConstants; - -import java.io.IOException; -import java.io.InterruptedIOException; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.net.NoRouteToHostException; -import java.net.ProtocolException; -import java.net.SocketException; -import java.net.UnknownHostException; -import java.util.concurrent.locks.ReentrantLock; - -import javax.net.ssl.SSLException; - -import okhttp3.Call; -import okhttp3.Callback; -import okhttp3.HttpUrl; -import okhttp3.OkHttpClient; -import okhttp3.Request; -import okhttp3.Response; -import okhttp3.internal.Util; -import timber.log.Timber; - -/** - * Gateway to request and return HttpRequest to c++ - *

- * Reentrancy lock is not needed, but "Lock" is an abstract class. - *

- */ -class HttpRequest implements Callback { - - @IntDef( {CONNECTION_ERROR, TEMPORARY_ERROR, PERMANENT_ERROR}) - @Retention(RetentionPolicy.SOURCE) - @interface HttpError { - } - - private static final int CONNECTION_ERROR = 0; - private static final int TEMPORARY_ERROR = 1; - private static final int PERMANENT_ERROR = 2; - - private static final ReentrantLock LOCK = new ReentrantLock(); - private static final OkHttpClient HTTP_CLIENT = new OkHttpClient(); - - private static boolean logEnabled = true; - private static String userAgentString; - - private long nativePtr; - private Call call; - - private native void nativeOnFailure(@HttpError int type, String message); - - private native void nativeOnResponse(int code, String etag, String modified, String cacheControl, String expires, - String retryAfter, String xRateLimitReset, byte[] body); - - private HttpRequest(long nativePtr, String resourceUrl, String etag, String modified) { - this.nativePtr = nativePtr; - - try { - // Don't try a request if we aren't connected - if (!Mapbox.isConnected()) { - throw new NoRouteToHostException("No Internet connection available."); - } - - resourceUrl = adaptResourceUrl(resourceUrl); - call = HTTP_CLIENT.newCall(buildRequest(resourceUrl, etag, modified)); - call.enqueue(this); - } catch (Exception exception) { - onFailure(exception); - } - } - - private String adaptResourceUrl(String resourceUrl) { - HttpUrl httpUrl = HttpUrl.parse(resourceUrl); - if (httpUrl != null && isMapboxHost(httpUrl)) { - HttpUrl.Builder builder = httpUrl.newBuilder(); - builder.addQueryParameter("events", "true"); - resourceUrl = builder.build().toString(); - } - return resourceUrl; - } - - private boolean isMapboxHost(@NonNull HttpUrl httpUrl) { - final String host = httpUrl.host().toLowerCase(MapboxConstants.MAPBOX_LOCALE); - return host.equals("mapbox.com") || host.endsWith(".mapbox.com") - || host.equals("mapbox.cn") || host.endsWith(".mapbox.cn"); - } - - private Request buildRequest(String resourceUrl, String etag, String modified) { - Request.Builder builder = new Request.Builder() - .url(resourceUrl) - .tag(resourceUrl.toLowerCase(MapboxConstants.MAPBOX_LOCALE)) - .addHeader("User-Agent", getUserAgentString()); - if (etag.length() > 0) { - builder = builder.addHeader("If-None-Match", etag); - } else if (modified.length() > 0) { - builder = builder.addHeader("If-Modified-Since", modified); - } - return builder.build(); - } - - public void 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. - LOCK.lock(); - nativePtr = 0; - LOCK.unlock(); - } - - @Override - public void onResponse(@NonNull Call call, @NonNull Response response) throws IOException { - if (logEnabled) { - logOnResponse(response); - } - - byte[] body; - try { - body = response.body().bytes(); - } catch (IOException ioException) { - onFailure(ioException); - return; - } finally { - response.body().close(); - } - - LOCK.lock(); - if (nativePtr != 0) { - nativeOnResponse(response.code(), - response.header("ETag"), - response.header("Last-Modified"), - response.header("Cache-Control"), - response.header("Expires"), - response.header("Retry-After"), - response.header("x-rate-limit-reset"), - body); - } - LOCK.unlock(); - } - - private void logOnResponse(Response response) { - if (response.isSuccessful()) { - Timber.v("[HTTP] Request was successful (code = %s).", response.code()); - } else if (logEnabled) { - // 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); - } - } - - @Override - public void onFailure(@NonNull Call call, @NonNull IOException exception) { - onFailure(exception); - } - - private void onFailure(Exception exception) { - int failType = resolveFailureType(exception); - String errorMessage = exception.getMessage() != null ? exception.getMessage() : "Error processing the request"; - if (logEnabled) { - logOnFailure(failType, errorMessage); - } - - LOCK.lock(); - if (nativePtr != 0) { - nativeOnFailure(failType, errorMessage); - } - LOCK.unlock(); - } - - @HttpError - private int resolveFailureType(Exception exception) { - int type = PERMANENT_ERROR; - if ((exception instanceof NoRouteToHostException) || (exception instanceof UnknownHostException) - || (exception instanceof SocketException) || (exception instanceof ProtocolException) - || (exception instanceof SSLException)) { - type = CONNECTION_ERROR; - } else if ((exception instanceof InterruptedIOException)) { - type = TEMPORARY_ERROR; - } - return type; - } - - private void logOnFailure(@HttpError int type, String errorMessage) { - if (type == TEMPORARY_ERROR) { - Timber.d("Request failed due to a temporary error: %s", errorMessage); - } else if (type == CONNECTION_ERROR) { - Timber.i("Request failed due to a connection error: %s", errorMessage); - } else { - // PERMANENT_ERROR - Timber.w("Request failed due to a permanent error: %s", errorMessage); - } - } - - private String getUserAgentString() { - 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) - ); - } - return userAgentString; - } - - private String getApplicationIdentifier() { - try { - Context context = Mapbox.getApplicationContext(); - PackageInfo packageInfo = context.getPackageManager().getPackageInfo(context.getPackageName(), 0); - return String.format("%s/%s (%s)", context.getPackageName(), packageInfo.versionName, packageInfo.versionCode); - } catch (Exception exception) { - return ""; - } - } - - static void enableLog(boolean enabled) { - logEnabled = enabled; - } -} diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HttpRequestUtil.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HttpRequestUtil.java deleted file mode 100644 index 46359572e63..00000000000 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HttpRequestUtil.java +++ /dev/null @@ -1,19 +0,0 @@ -package com.mapbox.mapboxsdk.http; - -/** - * Utility class for setting HttpRequest configurations - */ -public class HttpRequestUtil { - - /** - * Set the log state of HttpRequest. - *

- * This configuration will outlast the lifecycle of the Map. - *

- * - * @param enabled True will enable logging, false will disable - */ - public static void setLogEnabled(boolean enabled) { - HttpRequest.enableLog(enabled); - } -} diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/offline/OfflineActivity.java b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/offline/OfflineActivity.java index 3f0c2db81f0..5bffd4d930f 100644 --- a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/offline/OfflineActivity.java +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/offline/OfflineActivity.java @@ -15,7 +15,6 @@ import com.mapbox.mapboxsdk.constants.Style; import com.mapbox.mapboxsdk.geometry.LatLng; import com.mapbox.mapboxsdk.geometry.LatLngBounds; -import com.mapbox.mapboxsdk.http.HttpRequestUtil; import com.mapbox.mapboxsdk.maps.MapView; import com.mapbox.mapboxsdk.maps.MapboxMap; import com.mapbox.mapboxsdk.maps.OnMapReadyCallback; @@ -69,7 +68,6 @@ public class OfflineActivity extends AppCompatActivity @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); - HttpRequestUtil.setLogEnabled(false); setContentView(R.layout.activity_offline); // You can use Mapbox.setConnected(Boolean) to manually set the connectivity @@ -157,7 +155,6 @@ protected void onSaveInstanceState(Bundle outState) { protected void onDestroy() { super.onDestroy(); mapView.onDestroy(); - HttpRequestUtil.setLogEnabled(true); } @Override diff --git a/platform/android/src/http_file_source.cpp b/platform/android/src/http_file_source.cpp index 1830923bbe5..8eb9416e9dc 100644 --- a/platform/android/src/http_file_source.cpp +++ b/platform/android/src/http_file_source.cpp @@ -20,7 +20,7 @@ class HTTPFileSource::Impl { class HTTPRequest : public AsyncRequest { public: - static constexpr auto Name() { return "com/mapbox/mapboxsdk/http/HttpRequest"; }; + static constexpr auto Name() { return "com/mapbox/mapboxsdk/http/HTTPRequest"; }; HTTPRequest(jni::JNIEnv&, const Resource&, FileSource::Callback); ~HTTPRequest(); @@ -61,7 +61,7 @@ void RegisterNativeHTTPRequest(jni::JNIEnv& env) { #define METHOD(MethodPtr, name) jni::MakeNativePeerMethod(name) - jni::RegisterNativePeer(env, HTTPRequest::javaClass, "nativePtr", + jni::RegisterNativePeer(env, HTTPRequest::javaClass, "mNativePtr", METHOD(&HTTPRequest::onFailure, "nativeOnFailure"), METHOD(&HTTPRequest::onResponse, "nativeOnResponse")); }