Merge pull request #813 from ably/feature/add-more-logs

QuintinWillison · web-flow · commit 71fc7f5fcf1e · 2022-11-18T09:50:48.000Z
Add more logs to the SDKs
diff --git a/common/src/main/java/com/ably/tracking/common/Ably.kt b/common/src/main/java/com/ably/tracking/common/Ably.kt
diff --git a/common/src/main/java/com/ably/tracking/common/CallbackFunctionModels.kt b/common/src/main/java/com/ably/tracking/common/CallbackFunctionModels.kt
@@ -20,11 +20,17 @@ typealias ResultCallbackFunction<T> = CallbackFunction<Result<T>>
  * Utility function adapts [Continuation] to fit in [ResultCallbackFunction] signature. Callback result is unpacked and
  * used to resume the continuation.
  */
-fun <T> Continuation<T>.wrapInResultCallback(): ResultCallbackFunction<T> =
+fun <T> Continuation<T>.wrapInResultCallback(
+    onSuccess: () -> Unit = {},
+    onError: (Exception) -> Unit = {},
+): ResultCallbackFunction<T> =
     { result ->
         try {
-            resume(result.getOrThrow())
+            val resultValue = result.getOrThrow()
+            onSuccess()
+            resume(resultValue)
         } catch (exception: Exception) {
+            onError(exception)
             resumeWithException(exception)
         }
     }
diff --git a/publishing-sdk/src/main/java/com/ably/tracking/publisher/DefaultPublisher.kt b/publishing-sdk/src/main/java/com/ably/tracking/publisher/DefaultPublisher.kt
@@ -8,6 +8,9 @@ import com.ably.tracking.LocationUpdate
 import com.ably.tracking.Resolution
 import com.ably.tracking.TrackableState
 import com.ably.tracking.common.Ably
+import com.ably.tracking.common.logging.createLoggingTag
+import com.ably.tracking.common.logging.v
+import com.ably.tracking.common.logging.w
 import com.ably.tracking.common.wrapInResultCallback
 import com.ably.tracking.logging.LogHandler
 import kotlin.coroutines.suspendCoroutine
@@ -22,13 +25,14 @@ constructor(
     mapbox: Mapbox,
     resolutionPolicyFactory: ResolutionPolicy.Factory,
     routingProfile: RoutingProfile,
-    logHandler: LogHandler?,
+    private val logHandler: LogHandler?,
     areRawLocationsEnabled: Boolean?,
     sendResolutionEnabled: Boolean,
     constantLocationEngineResolution: Resolution?,
 ) :
     Publisher {
     private val core: CorePublisher
+    private val TAG = createLoggingTag(this)
 
     override val active: Trackable?
         get() = core.active
@@ -53,29 +57,58 @@ constructor(
             sendResolutionEnabled,
             constantLocationEngineResolution,
         )
+        logHandler?.v("$TAG Created a publisher instance")
     }
 
     override suspend fun track(trackable: Trackable): StateFlow<TrackableState> {
+        logHandler?.v("$TAG Publisher track operation started")
         return suspendCoroutine { continuation ->
-            core.trackTrackable(trackable, continuation.wrapInResultCallback())
+            core.trackTrackable(
+                trackable,
+                continuation.wrapInResultCallback(
+                    onSuccess = { logHandler?.v("$TAG Publisher track operation succeeded") },
+                    onError = { logHandler?.w("$TAG Publisher track operation failed", it) },
+                ),
+            )
         }
     }
 
     override suspend fun add(trackable: Trackable): StateFlow<TrackableState> {
+        logHandler?.v("$TAG Publisher add operation started")
         return suspendCoroutine { continuation ->
-            core.addTrackable(trackable, continuation.wrapInResultCallback())
+            core.addTrackable(
+                trackable,
+                continuation.wrapInResultCallback(
+                    onSuccess = { logHandler?.v("$TAG Publisher add operation succeeded") },
+                    onError = { logHandler?.w("$TAG Publisher add operation failed", it) },
+                ),
+            )
         }
     }
 
     override suspend fun remove(trackable: Trackable): Boolean {
+        logHandler?.v("$TAG Publisher remove operation started")
         return suspendCoroutine { continuation ->
-            core.removeTrackable(trackable, continuation.wrapInResultCallback())
+            core.removeTrackable(
+                trackable,
+                continuation.wrapInResultCallback(
+                    onSuccess = { logHandler?.v("$TAG Publisher remove operation succeeded") },
+                    onError = { logHandler?.w("$TAG Publisher remove operation failed", it) },
+                ),
+            )
         }
     }
 
     override suspend fun stop(timeoutInMilliseconds: Long) {
+        logHandler?.v("$TAG Publisher stop operation started")
         suspendCoroutine<Unit> { continuation ->
-            core.stop(timeoutInMilliseconds, continuation.wrapInResultCallback())
+            core.stop(
+                timeoutInMilliseconds,
+                continuation.wrapInResultCallback(
+                    onSuccess = { logHandler?.v("$TAG Publisher stop operation succeeded") },
+                    onError = { logHandler?.w("$TAG Publisher stop operation failed", it) },
+                ),
+            )
         }
     }
 
diff --git a/publishing-sdk/src/main/java/com/ably/tracking/publisher/PublisherBuilder.kt b/publishing-sdk/src/main/java/com/ably/tracking/publisher/PublisherBuilder.kt
@@ -7,6 +7,8 @@ import androidx.annotation.RequiresPermission
 import com.ably.tracking.BuilderConfigurationIncompleteException
 import com.ably.tracking.Resolution
 import com.ably.tracking.common.DefaultAbly
+import com.ably.tracking.common.logging.createLoggingTag
+import com.ably.tracking.common.logging.v
 import com.ably.tracking.connection.ConnectionConfiguration
 import com.ably.tracking.logging.LogHandler
 
@@ -26,6 +28,7 @@ internal data class PublisherBuilder(
     val constantLocationEngineResolution: Resolution? = null,
     val vehicleProfile: VehicleProfile = VehicleProfile.CAR,
 ) : Publisher.Builder {
+    private val TAG = createLoggingTag(this)
 
     override fun connection(configuration: ConnectionConfiguration): Publisher.Builder =
         this.copy(connectionConfiguration = configuration)
@@ -72,8 +75,10 @@ internal data class PublisherBuilder(
     @RequiresPermission(anyOf = [ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION])
     override fun start(): Publisher {
         if (isMissingRequiredFields()) {
+            logHandler?.v("$TAG Creating a publisher instance failed due to missing required fields")
             throw BuilderConfigurationIncompleteException()
         }
+        logHandler?.v("$TAG Creating a publisher instance")
         // All below fields are required and above code checks if they are nulls, so using !! should be safe from NPE
         return DefaultPublisher(
             DefaultAbly(connectionConfiguration!!, logHandler),
diff --git a/subscribing-sdk/src/main/java/com/ably/tracking/subscriber/DefaultSubscriber.kt b/subscribing-sdk/src/main/java/com/ably/tracking/subscriber/DefaultSubscriber.kt
@@ -5,7 +5,11 @@ import com.ably.tracking.Resolution
 import com.ably.tracking.TrackableState
 import com.ably.tracking.annotations.Experimental
 import com.ably.tracking.common.Ably
+import com.ably.tracking.common.logging.createLoggingTag
+import com.ably.tracking.common.logging.v
+import com.ably.tracking.common.logging.w
 import com.ably.tracking.common.wrapInResultCallback
+import com.ably.tracking.logging.LogHandler
 import com.ably.tracking.subscriber.workerqueue.WorkerSpecification
 import kotlin.coroutines.suspendCoroutine
 import kotlinx.coroutines.flow.SharedFlow
@@ -15,8 +19,10 @@ internal class DefaultSubscriber(
     ably: Ably,
     resolution: Resolution?,
     trackableId: String,
+    private val logHandler: LogHandler?,
 ) : Subscriber {
     private val core: CoreSubscriber
+    private val TAG = createLoggingTag(this)
 
     override val locations: SharedFlow<LocationUpdate>
         get() = core.enhancedLocations
@@ -39,33 +45,53 @@ internal class DefaultSubscriber(
 
     init {
         core = createCoreSubscriber(ably, resolution, trackableId)
+        logHandler?.v("$TAG Created a subscriber instance")
     }
 
     /**
      * This method must be run before running any other method from [DefaultSubscriber].
      */
     suspend fun start() {
+        logHandler?.v("$TAG Subscriber start operation started")
         suspendCoroutine<Unit> { continuation ->
             core.enqueue(
-                WorkerSpecification.StartConnection(continuation.wrapInResultCallback())
+                WorkerSpecification.StartConnection(
+                    continuation.wrapInResultCallback(
+                        onSuccess = { logHandler?.v("$TAG Subscriber start operation succeeded") },
+                        onError = { logHandler?.w("$TAG Subscriber start operation failed", it) },
+                    )
+                )
             )
         }
     }
 
     override suspend fun resolutionPreference(resolution: Resolution?) {
+        logHandler?.v("$TAG Subscriber resolutionPreference operation started")
         // send change request over channel and wait for the result
         suspendCoroutine<Unit> { continuation ->
             core.enqueue(
-                WorkerSpecification.ChangeResolution(resolution, continuation.wrapInResultCallback())
+                WorkerSpecification.ChangeResolution(
+                    resolution,
+                    continuation.wrapInResultCallback(
+                        onSuccess = { logHandler?.v("$TAG Subscriber resolutionPreference operation succeeded") },
+                        onError = { logHandler?.w("$TAG Subscriber resolutionPreference operation failed", it) },
+                    ),
+                )
             )
         }
     }
 
     override suspend fun stop() {
+        logHandler?.v("$TAG Subscriber stop operation started")
         // send stop request over channel and wait for the result
         suspendCoroutine<Unit> { continuation ->
             core.enqueue(
-                WorkerSpecification.StopConnection(continuation.wrapInResultCallback())
+                WorkerSpecification.StopConnection(
+                    continuation.wrapInResultCallback(
+                        onSuccess = { logHandler?.v("$TAG Subscriber stop operation succeeded") },
+                        onError = { logHandler?.w("$TAG Subscriber stop operation failed", it) },
+                    )
+                )
             )
         }
     }
diff --git a/subscribing-sdk/src/main/java/com/ably/tracking/subscriber/SubscriberBuilder.kt b/subscribing-sdk/src/main/java/com/ably/tracking/subscriber/SubscriberBuilder.kt
@@ -3,6 +3,8 @@ package com.ably.tracking.subscriber
 import com.ably.tracking.BuilderConfigurationIncompleteException
 import com.ably.tracking.Resolution
 import com.ably.tracking.common.DefaultAbly
+import com.ably.tracking.common.logging.createLoggingTag
+import com.ably.tracking.common.logging.v
 import com.ably.tracking.connection.ConnectionConfiguration
 import com.ably.tracking.logging.LogHandler
 
@@ -12,6 +14,7 @@ internal data class SubscriberBuilder(
     val logHandler: LogHandler? = null,
     val trackingId: String? = null,
 ) : Subscriber.Builder {
+    private val TAG = createLoggingTag(this)
 
     override fun connection(configuration: ConnectionConfiguration): Subscriber.Builder =
         this.copy(connectionConfiguration = configuration)
@@ -27,13 +30,16 @@ internal data class SubscriberBuilder(
 
     override suspend fun start(): Subscriber {
         if (isMissingRequiredFields()) {
+            logHandler?.v("$TAG Creating a subscriber instance failed due to missing required fields")
             throw BuilderConfigurationIncompleteException()
         }
+        logHandler?.v("$TAG Creating a subscriber instance")
         // All below fields are required and above code checks if they are nulls, so using !! should be safe from NPE
         return DefaultSubscriber(
             DefaultAbly(connectionConfiguration!!, logHandler),
             resolution,
             trackingId!!,
+            logHandler,
         ).apply {
             start()
         }
diff --git a/subscribing-sdk/src/test/java/com/ably/tracking/subscriber/DefaultSubscriberTest.kt b/subscribing-sdk/src/test/java/com/ably/tracking/subscriber/DefaultSubscriberTest.kt
@@ -15,7 +15,7 @@ import java.util.UUID
 class DefaultSubscriberTest {
     private val ably = mockk<Ably>(relaxed = true)
     private val trackableId = UUID.randomUUID().toString()
-    private val subscriber = DefaultSubscriber(ably, null, trackableId)
+    private val subscriber = DefaultSubscriber(ably, null, trackableId, null)
 
     @Test(expected = ConnectionException::class)
     fun `should return an error when starting the subscriber with subscribing to presence error`() {
