Revert "[android] - configure loggin of HttpRequest, cleanup HttpRequest"

This reverts commit a059c74a54302f9604f3e3b28143be7645e6204f.

5 files changed, 202 insertions, 262 deletions

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 0000000000..91a235616a
--- /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 4f899cf696..0000000000
--- 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++
- * <p>
- * Reentrancy lock is not needed, but "Lock" is an abstract class.
- * </p>
- */
-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 46359572e6..0000000000
--- 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.
-   * <p>
-   * This configuration will outlast the lifecycle of the Map.
-   * </p>
-   *
-   * @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 3f0c2db81f..5bffd4d930 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.camera.CameraUpdateFactory;
 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 @@ public class OfflineActivity extends AppCompatActivity
   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 1830923bbe..8eb9416e9d 100644
--- a/platform/android/src/http_file_source.cpp
+++ b/platform/android/src/http_file_source.cpp
@@ -20,7 +20,7 @@ public:
 
 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<decltype(MethodPtr), (MethodPtr)>(name)
 
-    jni::RegisterNativePeer<HTTPRequest>(env, HTTPRequest::javaClass, "nativePtr",
+    jni::RegisterNativePeer<HTTPRequest>(env, HTTPRequest::javaClass, "mNativePtr",
         METHOD(&HTTPRequest::onFailure, "nativeOnFailure"),
         METHOD(&HTTPRequest::onResponse, "nativeOnResponse"));
 }