Rare Registration and location fields fixes, and open callback improv…

…ements * Fixed rare but were device would not register with OneSignal due to intermittent connection issues or GMS aar file version mismatches. * Fixes around NotificationOpenedHandler with the back button for Corona and Cordova. - Added new removeNotificationOpenedHandler method which will be called by the Corona/Cordova wrapper when its run time shutsdown. * Fixed issues with location fields.
OneSignal · Jan 16, 2016 · f513a8f · f513a8f
1 parent 8d9dce0
commit f513a8f
Show file tree

Hide file tree

Showing 11 changed files with 214 additions and 71 deletions.
diff --git a/OneSignalSDK/app/build.gradle b/OneSignalSDK/app/build.gradle
@@ -18,6 +18,7 @@ android {
     buildTypes {
         debug {
             // Test with proguard enabled to make sure our consumer rules are correct in the SDK.
+            // minify does not work with GMS 7.0.0 with the current settings. Use 8.4.0 to test.
             minifyEnabled true
             proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro'
         }
@@ -51,10 +52,12 @@ dependencies {
 
     // Test with 7.0.0 to make sure there are no breaking changes in Google's libraries.
     // This insure that the SDK will work if an app developer is using an older version of GMS.
-    compile "com.google.android.gms:play-services-gcm:7.0.0"
-    compile "com.google.android.gms:play-services-analytics:7.0.0"
-    compile "com.google.android.gms:play-services-location:7.0.0"
-//  compile 'com.android.support:support-v4:23.0.0'
+    compile "com.google.android.gms:play-services-gcm:8.4.0"
+
+    // play-services-analytics is required for AdvertisingIdClient when using GMS 8.1.0 or lower.
+    // 8.3.0 it is included in 'basement' which is required by 'base'.
+    //compile "com.google.android.gms:play-services-analytics:7.0.0"
+    compile "com.google.android.gms:play-services-location:8.4.0"
 
 //    testCompile 'junit:junit:4.12'
 //    testCompile 'org.robolectric:shadows-support-v4:3.0'

diff --git a/OneSignalSDK/app/src/test/java/com/onesignal/OneSignalPackagePrivateHelper.java b/OneSignalSDK/app/src/test/java/com/onesignal/OneSignalPackagePrivateHelper.java
@@ -9,7 +9,7 @@
 import static org.robolectric.Shadows.shadowOf;
 
 public class OneSignalPackagePrivateHelper {
-   public static void runAllNetworkRunnable() {
+   public static void runAllNetworkRunnables() {
       for (Map.Entry<Integer, OneSignalStateSynchronizer.NetworkHandlerThread> handlerThread : OneSignalStateSynchronizer.networkHandlerThreads.entrySet()) {
          Scheduler scheduler = shadowOf(handlerThread.getValue().getLooper()).getScheduler();
          while (scheduler.advanceToNextPostedRunnable()) {}

diff --git a/OneSignalSDK/app/src/test/java/com/onesignal/ShadowLocationGMS.java b/OneSignalSDK/app/src/test/java/com/onesignal/ShadowLocationGMS.java
@@ -0,0 +1,44 @@
+/**
+ * Modified MIT License
+ *
+ * Copyright 2015 OneSignal
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * 1. The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * 2. All copies of substantial portions of the Software may only be used in connection
+ * with services provided by OneSignal.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package com.onesignal;
+
+import android.content.Context;
+
+import org.robolectric.annotation.Implements;
+
+@Implements(LocationGMS.class)
+public class ShadowLocationGMS {
+
+   public static Double lat = 1.0, log = 2.0;
+   public static Float accuracy = 3.0f;
+   public static Integer type = 0;
+
+   static void getLocation(Context context, boolean promptLocation, LocationGMS.LocationHandler handler) {
+      handler.complete(lat, log, accuracy, type);
+   }
+}
diff --git a/OneSignalSDK/app/src/test/java/com/test/onesignal/CustomRobolectricTestRunner.java b/OneSignalSDK/app/src/test/java/com/test/onesignal/CustomRobolectricTestRunner.java
@@ -29,6 +29,7 @@
 
 import org.junit.runners.model.InitializationError;
 import org.robolectric.RobolectricGradleTestRunner;
+import org.robolectric.annotation.Config;
 import org.robolectric.internal.bytecode.InstrumentationConfiguration;
 
 public class CustomRobolectricTestRunner extends RobolectricGradleTestRunner {
@@ -37,7 +38,7 @@ public CustomRobolectricTestRunner(Class<?> klass) throws InitializationError {
     }
 
     @Override
-    public InstrumentationConfiguration createClassLoaderConfig() {
+    public InstrumentationConfiguration createClassLoaderConfig(Config config) {
         return InstrumentationConfiguration.newBuilder().addInstrumentedPackage("com.onesignal").build();
     }
 }
diff --git a/OneSignalSDK/app/src/test/java/com/test/onesignal/MainOneSignalClassRunner.java b/OneSignalSDK/app/src/test/java/com/test/onesignal/MainOneSignalClassRunner.java
@@ -36,6 +36,7 @@
 import com.onesignal.BuildConfig;
 import com.onesignal.NotificationBundleProcessor;
 import com.onesignal.OneSignal;
+import com.onesignal.ShadowLocationGMS;
 import com.onesignal.ShadowOSUtils;
 import com.onesignal.ShadowOneSignalRestClient;
 import com.onesignal.OneSignalPackagePrivateHelper;
@@ -66,7 +67,7 @@
 
 @Config(packageName = "com.onesignal.example",
       constants = BuildConfig.class,
-      shadows = {ShadowOneSignalRestClient.class, ShadowPushRegistratorGPS.class, ShadowPushRegistratorADM.class, ShadowOSUtils.class},
+      shadows = {ShadowOneSignalRestClient.class, ShadowPushRegistratorGPS.class, ShadowPushRegistratorADM.class, ShadowOSUtils.class, ShadowLocationGMS.class},
       sdk = 21)
 
 @RunWith(CustomRobolectricTestRunner.class)
@@ -155,6 +156,25 @@ public void notificationOpened(String message, JSONObject additionalData, boolea
       Assert.assertEquals("Robo test message", notificationOpenedMessage);
    }
 
+   @Test
+   public void shouldCorrectlyRemoveOpenedHandlerAndFireMissedOnesWhenAddedBack() throws Exception {
+      OneSignal.NotificationOpenedHandler notifHandler = new OneSignal.NotificationOpenedHandler() {
+         @Override
+         public void notificationOpened(String message, JSONObject additionalData, boolean isActive) {
+            notificationOpenedMessage = message;
+         }
+      };
+      OneSignal.init(blankActivity, "123456789", ONESIGNAL_APP_ID, notifHandler);
+      threadAndTaskWait();
+
+      OneSignal.removeNotificationOpenedHandler();
+      OneSignal.handleNotificationOpened(blankActivity, new JSONArray("[{ \"alert\": \"Robo test message\", \"custom\": { \"i\": \"UUID\" } }]"), false);
+      Assert.assertNull(notificationOpenedMessage);
+
+      OneSignal.init(blankActivity, "123456789", ONESIGNAL_APP_ID, notifHandler);
+      Assert.assertEquals("Robo test message", notificationOpenedMessage);
+   }
+
    @Test
    public void shouldNotFireNotificationOpenAgainAfterAppRestart() throws Exception {
       OneSignal.init(blankActivity, "123456789", ONESIGNAL_APP_ID, new OneSignal.NotificationOpenedHandler() {
@@ -619,6 +639,29 @@ public void shouldNotDoubleCountFocusTime() throws Exception {
    }
    */
 
+   // ####### Unit Test Location       ########
+
+   @Test
+   public void shouldUpdateAllLocationFieldsWhenAnyFieldsChange() throws Exception {
+      OneSignalInit();
+      threadAndTaskWait();
+      Assert.assertEquals(1.0, ShadowOneSignalRestClient.lastPost.getDouble("lat"));
+      Assert.assertEquals(2.0, ShadowOneSignalRestClient.lastPost.getDouble("long"));
+      Assert.assertEquals(3.0, ShadowOneSignalRestClient.lastPost.getDouble("loc_acc"));
+      Assert.assertEquals(0.0, ShadowOneSignalRestClient.lastPost.getDouble("loc_type"));
+
+      ShadowOneSignalRestClient.lastPost = null;
+      StaticResetHelper.restSetStaticFields();
+      ShadowLocationGMS.lat = 30.0;
+      ShadowLocationGMS.accuracy = 5.0f;
+
+      OneSignalInit();
+      threadAndTaskWait();
+      Assert.assertEquals(30.0, ShadowOneSignalRestClient.lastPost.getDouble("lat"));
+      Assert.assertEquals(2.0, ShadowOneSignalRestClient.lastPost.getDouble("long"));
+      Assert.assertEquals(5.0, ShadowOneSignalRestClient.lastPost.getDouble("loc_acc"));
+      Assert.assertEquals(0.0, ShadowOneSignalRestClient.lastPost.getDouble("loc_type"));
+   }
 
    // ####### Unit test helper methods ########
 
@@ -628,7 +671,7 @@ private static void threadWait() {
 
    private void threadAndTaskWait() {
       try {Thread.sleep(300);} catch (Throwable t) {}
-      OneSignalPackagePrivateHelper.runAllNetworkRunnable();
+      OneSignalPackagePrivateHelper.runAllNetworkRunnables();
       OneSignalPackagePrivateHelper.runFocusRunnables();
 
       Robolectric.getForegroundThreadScheduler().runOneTask();

diff --git a/OneSignalSDK/onesignal/build.gradle b/OneSignalSDK/onesignal/build.gradle
@@ -26,7 +26,6 @@ dependencies {
     provided fileTree(dir: 'libs', include: ['*.jar'])
 
     compile 'com.google.android.gms:play-services-gcm:8.4.0'
-    compile 'com.google.android.gms:play-services-analytics:8.4.0'
     compile 'com.google.android.gms:play-services-location:8.4.0'
 }
 

diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/ActivityLifecycleHandler.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/ActivityLifecycleHandler.java
@@ -84,6 +84,8 @@ static void onActivityPaused(Activity activity) {
    }
 
    static void onActivityStopped(Activity activity) {
+      OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "onActivityStopped: " + activity.getClass().getName());
+
       if (activity == curActivity) {
          curActivity = null;
          handleLostFocus();
@@ -93,6 +95,8 @@ static void onActivityStopped(Activity activity) {
    }
 
    static void onActivityDestroyed(Activity activity) {
+      OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "onActivityDestroyed: " + activity.getClass().getName());
+
       if (activity == curActivity) {
          curActivity = null;
          handleLostFocus();

diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/ActivityLifecycleListenerCompat.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/ActivityLifecycleListenerCompat.java
@@ -68,6 +68,7 @@ public void onPaused(Activity activity) {
 
                while (true) {
                   // Wait for new activity events, does not fire for pauses through.
+                  // waitForActivity should also return for onStop and onDestroy according to android/app/Instrumentation.java but this is not happening on a real device.
                   Activity currentActivity = allActivitiesMonitor.waitForActivity();
 
                   if (!currentActivity.isFinishing()) {

diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/LocationGMS.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/LocationGMS.java
@@ -35,6 +35,7 @@
 import android.location.Location;
 import android.os.Build;
 import android.os.Bundle;
+import android.os.SystemClock;
 
 import com.google.android.gms.common.ConnectionResult;
 import com.google.android.gms.common.api.GoogleApiClient;
@@ -49,25 +50,29 @@ class LocationGMS {
    private static GoogleApiClientCompatProxy mGoogleApiClient;
    static String requestPermission;
 
-   private static ActivityLifecycleHandler.ActivityAvailableListener activityAvailableListener;
-
    interface LocationHandler {
-      void complete(Double lat, Double log);
+      void complete(Double lat, Double log, Float accuracy, Integer type);
    }
 
    private static LocationHandler locationHandler;
 
+   private static Thread fallbackFailThread;
+
+   private static boolean locationCoarse;
+
    static void getLocation(Context context, boolean promptLocation, LocationHandler handler) {
       locationHandler = handler;
       int locationCoarsePermission = PackageManager.PERMISSION_DENIED;
 
       int locationFinePermission = ContextCompat.checkSelfPermission(context, "android.permission.ACCESS_FINE_LOCATION");
-      if (locationFinePermission == PackageManager.PERMISSION_DENIED)
+      if (locationFinePermission == PackageManager.PERMISSION_DENIED) {
          locationCoarsePermission = ContextCompat.checkSelfPermission(context, "android.permission.ACCESS_COARSE_LOCATION");
+         locationCoarse = true;
+      }
 
       if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
          if (locationFinePermission != PackageManager.PERMISSION_GRANTED && locationCoarsePermission != PackageManager.PERMISSION_GRANTED) {
-            handler.complete(null, null);
+            handler.complete(null, null, null, null);
             return;
          }
 
@@ -102,6 +107,15 @@ else if (locationCoarsePermission == PackageManager.PERMISSION_GRANTED)
 
    static void startGetLocation() {
       try {
+         fallbackFailThread = new Thread(new Runnable() {
+            public void run() {
+               SystemClock.sleep(30000);
+               OneSignal.Log(OneSignal.LOG_LEVEL.WARN, "Location permission exists but GoogleApiClient timedout. Maybe related to mismatch google-play aar versions.");
+               fireFailedComplete();
+            }
+         });
+         fallbackFailThread.start();
+
          GoogleApiClientListener googleApiClientListener = new GoogleApiClientListener();
          GoogleApiClient googleApiClient = new GoogleApiClient.Builder(OneSignal.appContext)
              .addApi(LocationServices.API)
@@ -120,22 +134,33 @@ static void startGetLocation() {
    static void fireFailedComplete() {
       PermissionsActivity.answered = false;
 
-      locationHandler.complete(null, null);
+      fireComplete(null, null, null, null);
       if (mGoogleApiClient != null)
          mGoogleApiClient.disconnect();
    }
 
+   private static void fireComplete(Double lat, Double log, Float accuracy, Integer type) {
+      locationHandler.complete(lat, log, accuracy, type);
+      if (fallbackFailThread != null && !Thread.currentThread().equals(fallbackFailThread))
+         fallbackFailThread.interrupt();
+   }
+
    private static class GoogleApiClientListener implements GoogleApiClient.ConnectionCallbacks, GoogleApiClient.OnConnectionFailedListener {
       @Override
       public void onConnected(Bundle bundle) {
          PermissionsActivity.answered = false;
 
          Location location = LocationServices.FusedLocationApi.getLastLocation(mGoogleApiClient.realInstance());
+
          // Coarse always gives out 14 digits and has an accuracy 2000. Always rounding to 7 as this is what fine returns.
-         if (location != null)
-            locationHandler.complete(new BigDecimal(location.getLatitude()).setScale(7, RoundingMode.HALF_UP).doubleValue(), new BigDecimal(location.getLongitude()).setScale(7, RoundingMode.HALF_UP).doubleValue());
+         if (location != null) {
+            location.getAccuracy();
+            Double lat = new BigDecimal(location.getLatitude()).setScale(7, RoundingMode.HALF_UP).doubleValue();
+            Double log = new BigDecimal(location.getLongitude()).setScale(7, RoundingMode.HALF_UP).doubleValue();
+            fireComplete(lat, log, location.getAccuracy(), locationCoarse ? 0 : 1);
+         }
          else
-            locationHandler.complete(null, null);
+            fireComplete(null, null, null, null);
 
          mGoogleApiClient.disconnect();
       }