Add more tests and docs

Author: markushi

sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java

@@ -18,6 +18,7 @@
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.TestOnly;
 
 /**
  * Note: ConnectivityManager sometimes throws SecurityExceptions on Android 11. Hence all relevant
@@ -64,7 +65,7 @@ public AndroidConnectionStatusProvider(
 
   @RequiresApi(api = Build.VERSION_CODES.LOLLIPOP)
   @Override
-  public void addConnectionStatusObserver(@NotNull IConnectionStatusObserver observer) {
+  public boolean addConnectionStatusObserver(@NotNull IConnectionStatusObserver observer) {
     final ConnectivityManager.NetworkCallback callback =
         new ConnectivityManager.NetworkCallback() {
           @Override
@@ -89,7 +90,7 @@ public void onUnavailable() {
         };
 
     registeredCallbacks.put(observer, callback);
-    registerNetworkCallback(context, logger, buildInfoProvider, callback);
+    return registerNetworkCallback(context, logger, buildInfoProvider, callback);
   }
 
   @Override
@@ -326,4 +327,11 @@ public static void unregisterNetworkCallback(
       logger.log(SentryLevel.ERROR, "unregisterNetworkCallback failed", t);
     }
   }
+
+  @TestOnly
+  @NotNull
+  public Map<IConnectionStatusObserver, ConnectivityManager.NetworkCallback>
+      getRegisteredCallbacks() {
+    return registeredCallbacks;
+  }
 }

sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt

@@ -20,6 +20,7 @@ import org.mockito.kotlin.any
 import org.mockito.kotlin.eq
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.never
+import org.mockito.kotlin.times
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
 import kotlin.test.BeforeTest
@@ -31,7 +32,7 @@ import kotlin.test.assertTrue
 
 class AndroidConnectionStatusProviderTest {
 
-    private lateinit var connectionStatusProvider: IConnectionStatusProvider
+    private lateinit var connectionStatusProvider: AndroidConnectionStatusProvider
     private lateinit var contextMock: Context
     private lateinit var connectivityManager: ConnectivityManager
     private lateinit var networkInfo: NetworkInfo
@@ -294,4 +295,52 @@ class AndroidConnectionStatusProviderTest {
         }
         assertFalse(failed)
     }
+
+    @Test
+    fun `connectionStatus returns NO_PERMISSIONS when context does not hold the permission`() {
+        whenever(contextMock.checkPermission(any(), any(), any())).thenReturn(PERMISSION_DENIED)
+        assertEquals(IConnectionStatusProvider.ConnectionStatus.NO_PERMISSION, connectionStatusProvider.connectionStatus)
+    }
+
+    @Test
+    fun `connectionStatus returns ethernet when underlying mechanism provides ethernet`() {
+        whenever(networkCapabilities.hasTransport(eq(TRANSPORT_ETHERNET))).thenReturn(true)
+        assertEquals(
+            "ethernet",
+            connectionStatusProvider.connectionType
+        )
+    }
+
+    @Test
+    fun `adding and removing an observer works correctly`() {
+        whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N)
+
+        val observer = IConnectionStatusProvider.IConnectionStatusObserver { }
+        val addResult = connectionStatusProvider.addConnectionStatusObserver(observer)
+        assertTrue(addResult)
+
+        connectionStatusProvider.removeConnectionStatusObserver(observer)
+        assertTrue(connectionStatusProvider.registeredCallbacks.isEmpty())
+    }
+
+    @Test
+    fun `underlying callbacks correctly trigger update`() {
+        whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N)
+
+        var callback: NetworkCallback? = null
+        whenever(connectivityManager.registerDefaultNetworkCallback(any())).then { invocation ->
+            callback = invocation.getArgument(0, NetworkCallback::class.java)
+            Unit
+        }
+        val observer = mock<IConnectionStatusProvider.IConnectionStatusObserver>()
+        connectionStatusProvider.addConnectionStatusObserver(observer)
+        callback!!.onAvailable(mock<Network>())
+        callback!!.onUnavailable()
+        callback!!.onLosing(mock<Network>(), 0)
+        callback!!.onLost(mock<Network>())
+        callback!!.onUnavailable()
+        connectionStatusProvider.removeConnectionStatusObserver(observer)
+
+        verify(observer, times(5)).onConnectionStatusChanged(any())
+    }
 }

sentry/api/sentry.api

@@ -441,7 +441,7 @@ public abstract interface class io/sentry/ICollector {
 }
 
 public abstract interface class io/sentry/IConnectionStatusProvider {
-	public abstract fun addConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)V
+	public abstract fun addConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)Z
 	public abstract fun getConnectionStatus ()Lio/sentry/IConnectionStatusProvider$ConnectionStatus;
 	public abstract fun getConnectionType ()Ljava/lang/String;
 	public abstract fun removeConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)V
@@ -863,7 +863,7 @@ public final class io/sentry/MemoryCollectionData {
 
 public final class io/sentry/NoOpConnectionStatusProvider : io/sentry/IConnectionStatusProvider {
 	public fun <init> ()V
-	public fun addConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)V
+	public fun addConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)Z
 	public fun getConnectionStatus ()Lio/sentry/IConnectionStatusProvider$ConnectionStatus;
 	public fun getConnectionType ()Ljava/lang/String;
 	public fun removeConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)V

sentry/src/main/java/io/sentry/IConnectionStatusProvider.java

@@ -15,16 +15,43 @@ enum ConnectionStatus {
   }
 
   interface IConnectionStatusObserver {
+    /**
+     * Invoked whenever the connection status changed.
+     *
+     * @param status the new connection status
+     */
     void onConnectionStatusChanged(ConnectionStatus status);
   }
 
+  /**
+   * Gets the connection status.
+   *
+   * @return the current connection status
+   */
   @NotNull
   ConnectionStatus getConnectionStatus();
 
+  /**
+   * Gets the connection type.
+   *
+   * @return the current connection type. E.g. "ethernet", "wifi" or "cellular"
+   */
   @Nullable
   String getConnectionType();
 
-  void addConnectionStatusObserver(@NotNull final IConnectionStatusObserver observer);
-
+  /**
+   * Adds an observer for listening to connection status changes.
+   *
+   * @param observer the observer to register
+   * @return true if the observer was sucessfully registered
+   */
+  boolean addConnectionStatusObserver(@NotNull final IConnectionStatusObserver observer);
+
+  /**
+   * Removes an observer.
+   *
+   * @param observer a previously added observer via {@link
+   *     #addConnectionStatusObserver(IConnectionStatusObserver)}
+   */
   void removeConnectionStatusObserver(@NotNull final IConnectionStatusObserver observer);
 }

sentry/src/main/java/io/sentry/NoOpConnectionStatusProvider.java

@@ -17,8 +17,8 @@ public final class NoOpConnectionStatusProvider implements IConnectionStatusProv
   }
 
   @Override
-  public void addConnectionStatusObserver(@NotNull IConnectionStatusObserver observer) {
-    // no-op
+  public boolean addConnectionStatusObserver(@NotNull IConnectionStatusObserver observer) {
+    return false;
   }
 
   @Override

sentry/src/test/java/io/sentry/NoOpConnectionStatusProviderTest.kt

@@ -0,0 +1,33 @@
+package io.sentry
+
+import org.mockito.kotlin.mock
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertNull
+
+class NoOpConnectionStatusProviderTest {
+
+    private val provider = NoOpConnectionStatusProvider()
+
+    @Test
+    fun `provider returns unknown status`() {
+        assertEquals(IConnectionStatusProvider.ConnectionStatus.UNKNOWN, provider.connectionStatus)
+    }
+
+    @Test
+    fun `connection type returns null`() {
+        assertNull(provider.connectionType)
+    }
+
+    @Test
+    fun `adding a listener is a no-op and returns false`() {
+        val result = provider.addConnectionStatusObserver(mock<IConnectionStatusProvider.IConnectionStatusObserver>())
+        assertFalse(result)
+    }
+
+    @Test
+    fun `removing a listener is a no-op`() {
+        provider.addConnectionStatusObserver(mock<IConnectionStatusProvider.IConnectionStatusObserver>())
+    }
+}

sentry/src/test/java/io/sentry/SentryOptionsTest.kt

@@ -478,4 +478,32 @@ class SentryOptionsTest {
     fun `when options are initialized, FullyDrawnReporter is set`() {
         assertEquals(FullyDisplayedReporter.getInstance(), SentryOptions().fullyDisplayedReporter)
     }
+
+    @Test
+    fun `when options is initialized, connectionStatusProvider is not null and default to noop`() {
+        assertNotNull(SentryOptions().connectionStatusProvider)
+        assertTrue(SentryOptions().connectionStatusProvider is NoOpConnectionStatusProvider)
+    }
+
+    @Test
+    fun `when connectionStatusProvider is set, its returned as well`() {
+        val options = SentryOptions()
+        val customProvider = object : IConnectionStatusProvider {
+            override fun getConnectionStatus(): IConnectionStatusProvider.ConnectionStatus {
+                return IConnectionStatusProvider.ConnectionStatus.UNKNOWN
+            }
+
+            override fun getConnectionType(): String? = null
+
+            override fun addConnectionStatusObserver(observer: IConnectionStatusProvider.IConnectionStatusObserver) {
+                // no-op
+            }
+
+            override fun removeConnectionStatusObserver(observer: IConnectionStatusProvider.IConnectionStatusObserver) {
+                // no-op
+            }
+        }
+        options.connectionStatusProvider = customProvider
+        assertEquals(customProvider, options.connectionStatusProvider)
+    }
 }