Refactor and add unit test

build.gradle

@@ -28,15 +28,14 @@ subprojects {
 
     dependencyManagement {
         imports {
-            mavenBom 'org.jboss.aerogear:aerogear-android-sdk-bom:1.1.9'
+            mavenBom 'org.jboss.aerogear:aerogear-android-sdk-bom:1.1.10'
         }
     }
 }
 
 // We want to expose the SDK version and name to the metrics subproject
-project(':metrics') {
+project(':core') {
     project.ext {
-        versionCode = VERSION_CODE
         versionName = VERSION_NAME
     }
 }

core/build.gradle

@@ -7,8 +7,8 @@ android {
         minSdkVersion project.ext.minSdkVersion
         targetSdkVersion project.ext.targetSdkVersion
 
-        versionCode 1
-        versionName "1.0"
+        // Required for metrics, exposed by the parent project
+        versionName project.ext.versionName
 
         testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner"
     }
@@ -43,7 +43,6 @@ tasks.withType(JavaCompile) {
     options.compilerArgs = ["-Xlint:unchecked"]
 }
 
-
 dependencies {
     implementation "com.android.support:appcompat-v7"
     implementation "com.squareup.okhttp3:okhttp"

core/src/main/java/org/aerogear/mobile/core/MobileCore.java

@@ -1,8 +1,10 @@
 package org.aerogear.mobile.core;
 
 import android.content.Context;
+import android.content.pm.PackageManager;
 import android.support.annotation.NonNull;
 
+import org.aerogear.android.core.BuildConfig;
 import org.aerogear.mobile.core.configuration.MobileCoreJsonParser;
 import org.aerogear.mobile.core.configuration.ServiceConfiguration;
 import org.aerogear.mobile.core.exception.ConfigurationNotFoundException;
@@ -23,9 +25,11 @@
  */
 public final class MobileCore {
 
+    private static Logger logger = new LoggerAdapter();
+
+    private final String appVersion;
     private final String configFileName;
     private final HttpServiceModule httpLayer;
-    private final Logger logger;
     private final Map<String, ServiceConfiguration> servicesConfig;
     private final Map<Class<? extends ServiceModule>, ServiceModule> services = new HashMap<>();
 
@@ -36,7 +40,11 @@ public final class MobileCore {
      */
     private MobileCore(Context context, Options options) throws InitializationException {
         this.configFileName = options.configFileName;
-        this.logger = options.logger;
+
+        // -- Allow to override the default logger
+        if (options.logger != null) {
+            this.logger = options.logger;
+        }
 
         // -- Parse JSON config file
         try (InputStream configStream = context.getAssets().open(configFileName)) {
@@ -46,8 +54,10 @@ private MobileCore(Context context, Options options) throws InitializationExcept
             throw new InitializationException(message, exception);
         }
 
-        // -- Setting default http layer
+        // -- Set the app version variable
+        this.appVersion = getAppVersion(context);
 
+        // -- Setting default http layer
         if (options.httpServiceModule == null) {
             OkHttpServiceModule httpServiceModule = new OkHttpServiceModule();
 
@@ -95,12 +105,14 @@ public void destroy() {
         }
     }
 
+    @SuppressWarnings("unchecked")
     public <T extends ServiceModule> T getInstance(Class<T> serviceClass) {
         return (T) getInstance(serviceClass, null);
     }
 
+    @SuppressWarnings("unchecked")
     public <T extends ServiceModule> T getInstance(Class<T> serviceClass,
-                                     ServiceConfiguration serviceConfiguration)
+                                                   ServiceConfiguration serviceConfiguration)
         throws InitializationException {
 
         if (services.containsKey(serviceClass)) {
@@ -140,15 +152,50 @@ private ServiceConfiguration getServiceConfiguration(String type) {
         return serviceConfiguration;
     }
 
+    /**
+     * Get the user app version from the package manager
+     *
+     * @param context Android application context
+     * @return String app version name
+     */
+    private String getAppVersion(final Context context) throws InitializationException {
+        try {
+            return context
+                .getPackageManager()
+                .getPackageInfo(context.getPackageName(), 0)
+                .versionName;
+        } catch (PackageManager.NameNotFoundException e) {
+            // Wrap in Initialization exception
+            throw new InitializationException("Failed to read app version", e);
+        }
+    }
 
     public HttpServiceModule getHttpLayer() {
         return this.httpLayer;
     }
 
-    public Logger getLogger() {
+    public static Logger getLogger() {
         return logger;
     }
 
+    /**
+     * Get the version name of the SDK itself
+     *
+     * @return String SDK version
+     */
+    public static String getSdkVersion() {
+        return BuildConfig.VERSION_NAME;
+    }
+
+    /**
+     * Get the version of the user app
+     *
+     * @return String App version name
+     */
+    public String getAppVersion() {
+        return appVersion;
+    }
+
     public static final class Options {
 
         private String configFileName = "mobile-services.json";
@@ -178,7 +225,5 @@ public Options setLogger(Logger logger) {
             this.logger = logger;
             return this;
         }
-
     }
-
 }

gradle-mvn-push.gradle

@@ -103,7 +103,7 @@ afterEvaluate { project ->
             source = variant.javaCompile.source
             ext.androidJar = project.files(android.getBootClasspath().join(File.pathSeparator))
             classpath = files(variant.javaCompile.classpath.files) + files(ext.androidJar)
-            exclude '**/BuildConfig.java'
+            exclude '**/BuildConfig'
             exclude '**/R.java'
         }
 

metrics/build.gradle

@@ -7,9 +7,6 @@ android {
         minSdkVersion project.ext.minSdkVersion
         targetSdkVersion project.ext.targetSdkVersion
         testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner"
-
-        // Required for metrics, exposed by the parent project
-        versionName project.ext.versionName
     }
 
     buildTypes {
@@ -18,6 +15,11 @@ android {
         }
     }
 
+    testOptions {
+        unitTests.returnDefaultValues = true
+        unitTests.includeAndroidResources = true
+    }
+
     compileOptions {
         sourceCompatibility JavaVersion.VERSION_1_8
         targetCompatibility JavaVersion.VERSION_1_8
@@ -26,12 +28,10 @@ android {
 
 dependencies {
     implementation project(path: ':core')
-    implementation fileTree(dir: 'libs', include: ['*.jar'])
 
-    implementation 'com.android.support:appcompat-v7:26.1.0'
-    testImplementation 'junit:junit:4.12'
-    androidTestImplementation 'com.android.support.test:runner:1.0.1'
-    androidTestImplementation 'com.android.support.test.espresso:espresso-core:3.0.1'
+    testImplementation 'junit:junit'
+    testImplementation 'org.mockito:mockito-core'
+    testImplementation 'org.json:json'
 }
 
 apply from: '../gradle-mvn-push.gradle'

metrics/src/main/AndroidManifest.xml

@@ -1,2 +1,2 @@
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
-    package="mobile.aerogear.org.metrics" />
+    package="org.aerogear.mobile.metrics" />

metrics/src/main/java/mobile/aerogear/org/metrics/MetricsService.java → metrics/src/main/java/org/aerogear/mobile/metrics/MetricsService.java

@@ -1,8 +1,7 @@
-package mobile.aerogear.org.metrics;
+package org.aerogear.mobile.metrics;
 
 import android.content.Context;
 import android.content.SharedPreferences;
-import android.content.pm.PackageManager;
 
 import org.aerogear.mobile.core.MobileCore;
 import org.aerogear.mobile.core.ServiceModule;
@@ -17,32 +16,37 @@
 import java.util.UUID;
 
 public class MetricsService implements ServiceModule {
+    public final static String STORAGE_NAME = "org.aerogear.mobile.metrics";
+    public final static String STORAGE_KEY = "metrics-sdk-installation-id";
+
     private final static String MODULE_NAME = "metrics";
-    private final static String LOG_TAG = "AEROGEAR/METRICS";
-    private final static String STORAGE_NAME = "mobile.aerogear.org.metrics";
-    private final static String STORAGE_KEY = "metrics-sdk-installation-id";
+    private final static String TAG = "AEROGEAR/METRICS";
 
     private HttpServiceModule httpService;
-    private MetricsConfig config;
     private Logger logger;
 
+    private String appVersion = null;
+    private String metricsUrl = null;
+
     /**
      * Get or create the client ID that identifies a device as long as the user doesn't
      * reinstall the app or delete the app storage. A random UUID is created and stored in the
      * application shared preferences.
      *
+     * Can be overridden to provide a different implementation for identification.
+     *
      * @param context Android app context
      * @return String Client ID
      */
-    private String getOrCreateClientId(final Context context) {
+    protected String getOrCreateClientId(final Context context) {
         final SharedPreferences preferences = context
             .getSharedPreferences(STORAGE_NAME, Context.MODE_PRIVATE);
 
         String clientId = preferences.getString(STORAGE_KEY, null);
         if (clientId == null) {
             clientId = UUID.randomUUID().toString();
 
-            logger.info(LOG_TAG, "Generated a new client ID: " + clientId);
+            logger.info(TAG, "Generated a new client ID: " + clientId);
 
             SharedPreferences.Editor editor = preferences.edit();
             editor.putString(STORAGE_KEY, clientId);
@@ -53,61 +57,56 @@ private String getOrCreateClientId(final Context context) {
     }
 
     /**
-     * Get the version of the app itself
+     * This method is called to create the JSON object containing the metrics data.
+     * Can be overridden to add more data points.
      *
-     * @param context Android application context
-     * @return String version name
+     * @param context Android app context
+     * @return JSONObject Metrics data
+     * @throws JSONException when any of the data results in invalid JSON
      */
-    private String getAppVersion(final Context context) {
-        try {
-            return context
-                .getPackageManager()
-                .getPackageInfo(context.getPackageName(), 0).versionName;
-        } catch (PackageManager.NameNotFoundException e) {
-            logger.error(LOG_TAG, e);
-            return null;
-        }
-    }
-
-    private byte[] getMetricsData(final Context context) throws JSONException {
+    protected JSONObject metricsData(final Context context) throws JSONException {
         final JSONObject result = new JSONObject();
         result.put("clientId", getOrCreateClientId(context));
         result.put("appId", context.getPackageName());
-        result.put("appVersion", getAppVersion(context));
-        result.put("sdkVersion", BuildConfig.VERSION_NAME);
-
-        return result.toString().getBytes();
+        result.put("appVersion", appVersion);
+        result.put("sdkVersion", MobileCore.getSdkVersion());
+        return result;
     }
 
     /**
      * Send the metrics data to the server. The data is contained in a JSON object with the
      * following properties: clientId, appId, sdkVersion and appVersion
      *
+     * Should not be overridden. Users can change the target URL in mobile-services.json
+     *
      * @param context Android application context
      */
-    public void init(final Context context) {
+    public final void init(final Context context) {
         try {
+            final JSONObject data = metricsData(context);
+
             // Send request to backend
-            final HttpRequest request = httpService.newRequest();
-            request.post(config.getUri(), getMetricsData(context));
-            final HttpResponse response = request.execute();
+            HttpRequest request = httpService.newRequest();
+            request.post(metricsUrl, data.toString().getBytes());
+            HttpResponse response = request.execute();
+
+            // Async response handling
             response.onComplete(() -> {
-                if (response.getStatus() == 200) {
-                    logger.info(LOG_TAG, "Metrics sent to server");
-                } else {
-                    logger.error(LOG_TAG, "Error sending metrics to server");
+                if (response.getStatus() != 200) {
+                    logger.error(TAG, "Error sending metrics data");
                 }
             });
         } catch (JSONException e) {
-            logger.error(LOG_TAG, e);
+            logger.error(TAG, e);
         }
     }
 
     @Override
-    public void configure(MobileCore core, ServiceConfiguration serviceConfiguration) {
-        config = new MetricsConfig(serviceConfiguration);
+    public void configure(final MobileCore core, final ServiceConfiguration serviceConfiguration) {
+        metricsUrl = serviceConfiguration.getUri();
+        appVersion = core.getAppVersion();
         httpService = core.getHttpLayer();
-        logger = core.getLogger();
+        logger = MobileCore.getLogger();
     }
 
     @Override
@@ -117,6 +116,5 @@ public String type() {
 
     @Override
     public void destroy() {
-        // Not used
     }
 }