Skip to content

Commit

Permalink
Fix ConnectionUtil/ConnectionManager NPE (#145)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mateusz Rzeszutek authored Sep 8, 2021
1 parent e76d7c6 commit dc9cb2c
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 44 deletions.
44 changes: 16 additions & 28 deletions splunk-otel-android/src/main/java/com/splunk/rum/ConnectionUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.splunk.rum;

import android.content.Context;
import android.net.ConnectivityManager;
import android.net.Network;
import android.net.NetworkCapabilities;
Expand All @@ -26,7 +25,6 @@

import androidx.annotation.NonNull;

import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;

//note: based on ideas from stack overflow: https://stackoverflow.com/questions/32547006/connectivitymanager-getnetworkinfoint-deprecated
Expand All @@ -35,37 +33,33 @@ class ConnectionUtil {
static final CurrentNetwork NO_NETWORK = new CurrentNetwork(NetworkState.NO_NETWORK_AVAILABLE, null);
static final CurrentNetwork UNKNOWN_NETWORK = new CurrentNetwork(NetworkState.TRANSPORT_UNKNOWN, null);

private final ConnectionMonitor connectionMonitor;
private final NetworkDetector networkDetector;

private final AtomicReference<CurrentNetwork> currentNetwork = new AtomicReference<>();
private volatile CurrentNetwork currentNetwork;
private volatile ConnectionStateListener connectionStateListener;

ConnectionUtil(Context context) {
this(ConnectionUtil::createNetworkMonitoringRequest, NetworkDetector.create(context), (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE));
ConnectionUtil(NetworkDetector networkDetector) {
this.networkDetector = networkDetector;
}

//for testing, since building the NetworkRequest fails in junit due to .. Android.
ConnectionUtil(Supplier<NetworkRequest> createNetworkMonitoringRequest, NetworkDetector networkDetector, ConnectivityManager connectivityManager) {
this.connectionMonitor = new ConnectionMonitor();

void startMonitoring(Supplier<NetworkRequest> createNetworkMonitoringRequest, ConnectivityManager connectivityManager) {
refreshNetworkStatus();
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
connectivityManager.registerDefaultNetworkCallback(connectionMonitor);
connectivityManager.registerDefaultNetworkCallback(new ConnectionMonitor());
} else {
NetworkRequest networkRequest = createNetworkMonitoringRequest.get();
connectivityManager.registerNetworkCallback(networkRequest, connectionMonitor);
connectivityManager.registerNetworkCallback(networkRequest, new ConnectionMonitor());
}
this.networkDetector = networkDetector;
refreshNetworkStatus();
}

CurrentNetwork refreshNetworkStatus() {
CurrentNetwork activeNetwork = networkDetector.detectCurrentNetwork();
currentNetwork.set(activeNetwork);
currentNetwork = activeNetwork;
return activeNetwork;
}

private static NetworkRequest createNetworkMonitoringRequest() {
//todo: this throws an NPE when running in junit. what's up with that?
static NetworkRequest createNetworkMonitoringRequest() {
//note: this throws an NPE when running in junit without robolectric, due to Android
return new NetworkRequest.Builder()
.addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR)
.addTransportType(NetworkCapabilities.TRANSPORT_WIFI)
Expand All @@ -76,24 +70,18 @@ private static NetworkRequest createNetworkMonitoringRequest() {
}

boolean isOnline() {
return currentNetwork.get().isOnline();
return currentNetwork.isOnline();
}

CurrentNetwork getActiveNetwork() {
return currentNetwork.get();
return currentNetwork;
}

void setInternetStateListener(ConnectionStateListener listener) {
connectionMonitor.setOnConnectionStateListener(listener);
connectionStateListener = listener;
}

class ConnectionMonitor extends ConnectivityManager.NetworkCallback {

private ConnectionStateListener connectionStateListener;

void setOnConnectionStateListener(ConnectionStateListener connectionStateListener) {
this.connectionStateListener = connectionStateListener;
}
private class ConnectionMonitor extends ConnectivityManager.NetworkCallback {

@Override
public void onAvailable(@NonNull Network network) {
Expand All @@ -112,7 +100,7 @@ public void onLost(@NonNull Network network) {
//this method, we'll force it to be NO_NETWORK, rather than relying on the ConnectivityManager to have the right
//state at the right time during this event.
CurrentNetwork activeNetwork = NO_NETWORK;
currentNetwork.set(activeNetwork);
currentNetwork = activeNetwork;
if (connectionStateListener != null) {
connectionStateListener.onAvailable(false, activeNetwork);
Log.d(SplunkRum.LOG_TAG, " onLost: isConnected:" + false + ", activeNetwork: " + activeNetwork);
Expand Down
9 changes: 8 additions & 1 deletion splunk-otel-android/src/main/java/com/splunk/rum/SplunkRum.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import static io.opentelemetry.api.common.AttributeKey.stringKey;

import android.app.Application;
import android.content.Context;
import android.net.ConnectivityManager;
import android.os.Looper;
import android.util.Log;

Expand Down Expand Up @@ -90,7 +92,12 @@ public static Config.Builder newConfigBuilder() {
* @return A fully initialized {@link SplunkRum} instance, ready for use.
*/
public static SplunkRum initialize(Config config, Application application) {
return initialize(config, application, () -> new ConnectionUtil(application.getApplicationContext()));
return initialize(config, application, () -> {
Context context = application.getApplicationContext();
ConnectionUtil connectionUtil = new ConnectionUtil(NetworkDetector.create(context));
connectionUtil.startMonitoring(ConnectionUtil::createNetworkMonitoringRequest, (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE));
return connectionUtil;
});
}

//for testing purposes
Expand Down
72 changes: 57 additions & 15 deletions splunk-otel-android/src/test/java/com/splunk/rum/ConnectionUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,21 @@

package com.splunk.rum;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import android.net.ConnectivityManager;
import android.net.ConnectivityManager.NetworkCallback;
import android.net.Network;
import android.net.NetworkRequest;
import android.os.Build;

Expand All @@ -28,16 +42,6 @@

import java.util.concurrent.atomic.AtomicInteger;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@RunWith(RobolectricTestRunner.class)
public class ConnectionUtilTest {

Expand All @@ -52,12 +56,13 @@ public void lollipop() {
.thenReturn(new CurrentNetwork(NetworkState.TRANSPORT_WIFI, null)) //called on init
.thenReturn(new CurrentNetwork(NetworkState.TRANSPORT_CELLULAR, "LTE"));

ConnectionUtil connectionUtil = new ConnectionUtil(() -> networkRequest, networkDetector, connectivityManager);
ConnectionUtil connectionUtil = new ConnectionUtil(networkDetector);
connectionUtil.startMonitoring(() -> networkRequest, connectivityManager);

assertTrue(connectionUtil.isOnline());
assertEquals(new CurrentNetwork(NetworkState.TRANSPORT_WIFI, null), connectionUtil.getActiveNetwork());

ArgumentCaptor<ConnectionUtil.ConnectionMonitor> monitorCaptor = ArgumentCaptor.forClass(ConnectionUtil.ConnectionMonitor.class);
ArgumentCaptor<NetworkCallback> monitorCaptor = ArgumentCaptor.forClass(NetworkCallback.class);
verify(connectivityManager).registerNetworkCallback(eq(networkRequest), monitorCaptor.capture());

AtomicInteger notified = new AtomicInteger(0);
Expand Down Expand Up @@ -89,13 +94,14 @@ public void quiznos() {
.thenReturn(new CurrentNetwork(NetworkState.TRANSPORT_WIFI, null))
.thenReturn(new CurrentNetwork(NetworkState.TRANSPORT_CELLULAR, "LTE"));

ConnectionUtil connectionUtil = new ConnectionUtil(() -> networkRequest, networkDetector, connectivityManager);
ConnectionUtil connectionUtil = new ConnectionUtil(networkDetector);
connectionUtil.startMonitoring(() -> networkRequest, connectivityManager);

assertTrue(connectionUtil.isOnline());
assertEquals(new CurrentNetwork(NetworkState.TRANSPORT_WIFI, null), connectionUtil.getActiveNetwork());
verify(connectivityManager, never()).registerNetworkCallback(eq(networkRequest), isA(ConnectionUtil.ConnectionMonitor.class));
verify(connectivityManager, never()).registerNetworkCallback(eq(networkRequest), isA(NetworkCallback.class));

ArgumentCaptor<ConnectionUtil.ConnectionMonitor> monitorCaptor = ArgumentCaptor.forClass(ConnectionUtil.ConnectionMonitor.class);
ArgumentCaptor<NetworkCallback> monitorCaptor = ArgumentCaptor.forClass(NetworkCallback.class);
verify(connectivityManager).registerDefaultNetworkCallback(monitorCaptor.capture());

AtomicInteger notified = new AtomicInteger(0);
Expand All @@ -115,4 +121,40 @@ public void quiznos() {
monitorCaptor.getValue().onLost(null);
assertEquals(2, notified.get());
}

@Test
@Config(sdk = Build.VERSION_CODES.LOLLIPOP)
public void shouldNotFailOnImmediateConnectionManagerCall_lollipop() {
NetworkRequest networkRequest = mock(NetworkRequest.class);
NetworkDetector networkDetector = mock(NetworkDetector.class);
ConnectivityManager connectivityManager = mock(ConnectivityManager.class);

doAnswer(invocation -> {
NetworkCallback callback = invocation.getArgument(1);
callback.onAvailable(mock(Network.class));
return null;
}).when(connectivityManager)
.registerNetworkCallback(eq(networkRequest), any(NetworkCallback.class));

ConnectionUtil connectionUtil = new ConnectionUtil(networkDetector);
connectionUtil.startMonitoring(() -> networkRequest, connectivityManager);
}

@Test
@Config(sdk = Build.VERSION_CODES.Q)
public void shouldNotFailOnImmediateConnectionManagerCall_quiznos() {
NetworkRequest networkRequest = mock(NetworkRequest.class);
NetworkDetector networkDetector = mock(NetworkDetector.class);
ConnectivityManager connectivityManager = mock(ConnectivityManager.class);

doAnswer(invocation -> {
NetworkCallback callback = invocation.getArgument(0);
callback.onAvailable(mock(Network.class));
return null;
}).when(connectivityManager)
.registerDefaultNetworkCallback(any(NetworkCallback.class));

ConnectionUtil connectionUtil = new ConnectionUtil(networkDetector);
connectionUtil.startMonitoring(() -> networkRequest, connectivityManager);
}
}

0 comments on commit dc9cb2c

Please sign in to comment.