fix: [SDK-4672] start location updates when permission is granted

OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/controller/impl/FusedLocationApiWrapperImpl.kt

@@ -24,16 +24,20 @@ internal class FusedLocationApiWrapperImpl : IFusedLocationApiWrapper {
         googleApiClient: GoogleApiClient,
         locationRequest: LocationRequest,
         locationListener: LocationListener,
-    ) {
-        try {
+    ): Boolean {
+        return try {
             if (Looper.myLooper() == null) {
                 Looper.prepare()
             }
             if (googleApiClient.isConnected) {
                 LocationServices.FusedLocationApi.requestLocationUpdates(googleApiClient, locationRequest, locationListener)
+                true
+            } else {
+                false
             }
         } catch (t: Throwable) {
             Logging.warn("FusedLocationApi.requestLocationUpdates failed!", t)
+            false
         }
     }
 

OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/controller/impl/GmsLocationController.kt

@@ -56,6 +56,10 @@ internal class GmsLocationController(
                             setLocationAndFire(localLastLocation)
                         }
                     }
+                    // Retry the subscription in case the first attempt failed because
+                    // permission was not yet granted, otherwise it would only recover on
+                    // the next app focus change.
+                    locationUpdateListener?.refreshRequest()
                     wasSuccessful = true
                 } else {
                     try {
@@ -193,7 +197,7 @@ internal class GmsLocationController(
             }
         }
 
-        private fun refreshRequest() {
+        internal fun refreshRequest() {
             if (!googleApiClient.isConnected) {
                 Logging.warn("Attempt to refresh location request but not currently connected!")
                 return
@@ -217,8 +221,7 @@ internal class GmsLocationController(
                     .setMaxWaitTime((updateInterval * 1.5).toLong())
                     .setPriority(LocationRequest.PRIORITY_BALANCED_POWER_ACCURACY)
             Logging.debug("GMSLocationController GoogleApiClient requestLocationUpdates!")
-            _fusedLocationApiWrapper.requestLocationUpdates(googleApiClient, locationRequest, this)
-            hasExistingRequest = true
+            hasExistingRequest = _fusedLocationApiWrapper.requestLocationUpdates(googleApiClient, locationRequest, this)
         }
 
         override fun onLocationChanged(location: Location) {

OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/controller/impl/IFusedLocationApiWrapper.kt

@@ -15,7 +15,7 @@ internal interface IFusedLocationApiWrapper {
         googleApiClient: GoogleApiClient,
         locationRequest: LocationRequest,
         locationListener: LocationListener,
-    )
+    ): Boolean
 
     fun getLastLocation(googleApiClient: GoogleApiClient): Location?
 }

OneSignalSDK/onesignal/location/src/test/java/com/onesignal/location/internal/controller/GmsLocationControllerTests.kt

@@ -173,4 +173,55 @@ class GmsLocationControllerTests : FunSpec({
             )
         }
     }
+
+    // Regression: when isShared is enabled before location permission is granted, the
+    // GoogleApiClient connects (which does not require permission) but the first
+    // requestLocationUpdates fails. A later start() (e.g. triggered by the permission grant)
+    // previously short-circuited and only fired a one-shot last location, leaving the live
+    // subscription dead until the next app focus change. It must now re-arm on that start().
+    test("start re-arms the live location subscription on a subsequent start") {
+        // Given
+        val location = Location("TEST_PROVIDER")
+        location.latitude = 123.45
+        location.longitude = 678.91
+        val fusedLocationApiWrapperMock = FusedLocationApiWrapperMock(listOf(location))
+
+        val applicationService = AndroidMockHelper.applicationService()
+        every { applicationService.isInForeground } returns true
+        val gmsLocationController = GmsLocationController(applicationService, fusedLocationApiWrapperMock)
+
+        // When
+        val response1 = gmsLocationController.start()
+        val response2 = gmsLocationController.start()
+
+        // Then
+        response1 shouldBe true
+        response2 shouldBe true
+        // Once on the initial init, and again when the second start re-arms the subscription.
+        fusedLocationApiWrapperMock.requestLocationUpdatesCallCount shouldBe 2
+    }
+
+    // Regression: a requestLocationUpdates that fails (e.g. a swallowed SecurityException
+    // when permission is missing) must not be recorded as an active request, otherwise
+    // close()/refresh would try to cancel a subscription that never existed.
+    test("a failed requestLocationUpdates is not treated as an active subscription") {
+        // Given
+        val location = Location("TEST_PROVIDER")
+        location.latitude = 123.45
+        location.longitude = 678.91
+        val fusedLocationApiWrapperMock = FusedLocationApiWrapperMock(listOf(location))
+        fusedLocationApiWrapperMock.requestLocationUpdatesResult = false
+
+        val applicationService = AndroidMockHelper.applicationService()
+        every { applicationService.isInForeground } returns true
+        val gmsLocationController = GmsLocationController(applicationService, fusedLocationApiWrapperMock)
+
+        // When
+        gmsLocationController.start()
+        gmsLocationController.stop()
+
+        // Then
+        fusedLocationApiWrapperMock.requestLocationUpdatesCallCount shouldBe 1
+        fusedLocationApiWrapperMock.cancelLocationUpdatesCallCount shouldBe 0
+    }
 })

OneSignalSDK/onesignal/location/src/test/java/com/onesignal/location/mocks/FusedLocationApiWrapperMock.kt

@@ -11,20 +11,35 @@ import java.util.Queue
 internal class FusedLocationApiWrapperMock(locationsList: List<Location>) : IFusedLocationApiWrapper {
     private val locations: Queue<Location>
 
+    // Controls the value returned by [requestLocationUpdates] to simulate the subscription
+    // succeeding or failing (e.g. a swallowed SecurityException when permission is missing).
+    var requestLocationUpdatesResult: Boolean = true
+
+    var requestLocationUpdatesCallCount: Int = 0
+        private set
+
+    var cancelLocationUpdatesCallCount: Int = 0
+        private set
+
     init {
         this.locations = LinkedList(locationsList)
     }
 
     override fun cancelLocationUpdates(
         googleApiClient: GoogleApiClient,
         locationListener: LocationListener,
-    ) {}
+    ) {
+        cancelLocationUpdatesCallCount++
+    }
 
     override fun requestLocationUpdates(
         googleApiClient: GoogleApiClient,
         locationRequest: LocationRequest,
         locationListener: LocationListener,
-    ) {}
+    ): Boolean {
+        requestLocationUpdatesCallCount++
+        return requestLocationUpdatesResult
+    }
 
     override fun getLastLocation(googleApiClient: GoogleApiClient): Location? {
         return locations.remove()