fixes in the logic of MarshmallowNetworkObservingStrategy and tests according to PR #455 related to utilization of the NetworkState class

Author: pwittchen

library/src/main/java/com/github/pwittchen/reactivenetwork/library/rx2/Connectivity.java

@@ -17,45 +17,44 @@
 
 import android.content.Context;
 import android.net.ConnectivityManager;
-import android.net.Network;
-import android.net.NetworkCapabilities;
 import android.net.NetworkInfo;
 import android.os.Build;
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import androidx.annotation.RequiresApi;
-
 import com.github.pwittchen.reactivenetwork.library.rx2.info.NetworkState;
 
 /**
  * Connectivity class represents current connectivity status. It wraps NetworkInfo object.
  */
-@RequiresApi(api = Build.VERSION_CODES.CUPCAKE)
 public final class Connectivity {
   static final int UNKNOWN_TYPE = -1;
   static final int UNKNOWN_SUB_TYPE = -1;
-  private NetworkInfo.State state; // NOPMD
-  private NetworkInfo.DetailedState detailedState; // NOPMD
-  private int type; // NOPMD
-  private int subType; // NOPMD
-  private boolean available; // NOPMD
-  private boolean failover; // NOPMD
-  private boolean roaming; // NOPMD
-  private String typeName; // NOPMD
-  private String subTypeName; // NOPMD
-  private String reason; // NOPMD
-  private String extraInfo; // NOPMD
-  @RequiresApi(api = Build.VERSION_CODES.LOLLIPOP)
-  private NetworkState networkState;
+  @Nullable private NetworkInfo.State state; // NOPMD
+  @Nullable private NetworkInfo.DetailedState detailedState; // NOPMD
+  @Nullable @RequiresApi(api = Build.VERSION_CODES.LOLLIPOP)
+  private NetworkState networkState; // NOPMD
+  private final int type; // NOPMD
+  private final int subType; // NOPMD
+  private final boolean available; // NOPMD
+  private final boolean failover; // NOPMD
+  private final boolean roaming; // NOPMD
+  private final String typeName; // NOPMD
+  private final String subTypeName; // NOPMD
+  private final String reason; // NOPMD
+  private final String extraInfo; // NOPMD
 
   public static Connectivity create() {
     return builder().build();
   }
 
+  @SuppressWarnings("PMD")
   public static Connectivity create(@NonNull Context context) {
     Preconditions.checkNotNull(context, "context == null");
     return create(context, getConnectivityManager(context));
   }
 
+  @SuppressWarnings("PMD")
   @RequiresApi(api = Build.VERSION_CODES.LOLLIPOP)
   public static Connectivity create(@NonNull Context context, NetworkState networkState) {
     Preconditions.checkNotNull(context, "context == null");
@@ -67,6 +66,7 @@ private static ConnectivityManager getConnectivityManager(Context context) {
     return (ConnectivityManager) context.getSystemService(service);
   }
 
+  @SuppressWarnings("PMD")
   protected static Connectivity create(@NonNull Context context, ConnectivityManager manager) {
     Preconditions.checkNotNull(context, "context == null");
 
@@ -78,8 +78,11 @@ protected static Connectivity create(@NonNull Context context, ConnectivityManag
     return (networkInfo == null) ? create() : create(networkInfo);
   }
 
+  @SuppressWarnings("PMD")
   @RequiresApi(api = Build.VERSION_CODES.LOLLIPOP)
-  protected static Connectivity create(@NonNull Context context, ConnectivityManager manager, NetworkState networkState) {
+  protected static Connectivity create(
+      @NonNull Context context, ConnectivityManager manager, NetworkState networkState
+  ) {
     Preconditions.checkNotNull(context, "context == null");
 
     if (manager == null) {
@@ -109,16 +112,22 @@ private static Connectivity create(NetworkInfo networkInfo) {
   @RequiresApi(api = Build.VERSION_CODES.LOLLIPOP)
   private static Connectivity create(NetworkState networkState) {
     return new Builder()
-            .networkState(networkState)
-            .build();
+        .networkState(networkState)
+        .build();
   }
 
   private Connectivity(Builder builder) {
-    if(Preconditions.isAtLeastAndroidLollipop()) {
-      networkState = builder.networkState;
+    if (Preconditions.isAtLeastAndroidLollipop()) {
+      if (builder.networkState != null) {
+        networkState = builder.networkState;
+      }
     } else {
-      state = builder.state;
-      detailedState = builder.detailedState;
+      if (builder.state != null) {
+        state = builder.state;
+      }
+      if (builder.detailedState != null) {
+        detailedState = builder.detailedState;
+      }
     }
     type = builder.type;
     subType = builder.subType;
@@ -139,22 +148,28 @@ private static Builder builder() {
     return new Connectivity.Builder();
   }
 
-  public NetworkInfo.State state() {
+  public @Nullable NetworkInfo.State state() {
     return state;
   }
 
   public static Builder state(NetworkInfo.State state) {
     return builder().state(state);
   }
 
-  public NetworkInfo.DetailedState detailedState() {
+  public @Nullable NetworkInfo.DetailedState detailedState() {
     return detailedState;
   }
 
   public static Builder state(NetworkInfo.DetailedState detailedState) {
     return builder().detailedState(detailedState);
   }
 
+  @RequiresApi(api = Build.VERSION_CODES.LOLLIPOP)
+  @Nullable
+  public NetworkState getNetworkState() {
+    return networkState;
+  }
+
   public int type() {
     return type;
   }
@@ -227,11 +242,6 @@ public static Builder extraInfo(String extraInfo) {
     return builder().extraInfo(extraInfo);
   }
 
-  @RequiresApi(api = Build.VERSION_CODES.LOLLIPOP)
-  public NetworkState getNetworkState() {
-    return networkState;
-  }
-
   @Override public boolean equals(Object o) {
     if (this == o) {
       return true;
@@ -277,7 +287,7 @@ public NetworkState getNetworkState() {
   }
 
   @Override public int hashCode() {
-    int result = state.hashCode();
+    int result = state != null ? state.hashCode() : 0;
     result = 31 * result + (detailedState != null ? detailedState.hashCode() : 0);
     result = 31 * result + type;
     result = 31 * result + subType;
@@ -338,7 +348,7 @@ public final static class Builder {
     private String subTypeName = "NONE"; // NOPMD
     private String reason = ""; // NOPMD
     private String extraInfo = ""; // NOPMD
-    private NetworkState networkState = new NetworkState();
+    private NetworkState networkState = new NetworkState(); // NOPMD
 
     public Builder state(NetworkInfo.State state) {
       this.state = state;

library/src/main/java/com/github/pwittchen/reactivenetwork/library/rx2/info/NetworkState.java

@@ -3,16 +3,17 @@
 import android.net.LinkProperties;
 import android.net.Network;
 import android.net.NetworkCapabilities;
+import androidx.annotation.Nullable;
+import com.jakewharton.nopen.annotation.Open;
 
-/**
- * NetworkState data object
- */
+@Open
 public class NetworkState {
-    private boolean isConnected = false;
-    private Network network = null;
-    private NetworkCapabilities networkCapabilities = null;
-    private LinkProperties linkProperties = null;
+    @SuppressWarnings("PMD") private boolean isConnected = false;
+    @Nullable private Network network = null;
+    @Nullable private NetworkCapabilities networkCapabilities = null;
+    @Nullable private LinkProperties linkProperties = null;
 
+    @SuppressWarnings("PMD")
     public boolean isConnected() {
         return isConnected;
     }
@@ -21,27 +22,27 @@ public void setConnected(boolean connected) {
         isConnected = connected;
     }
 
-    public Network getNetwork() {
+    @Nullable public Network getNetwork() {
         return network;
     }
 
-    public void setNetwork(Network network) {
+    public void setNetwork(@Nullable Network network) {
         this.network = network;
     }
 
-    public NetworkCapabilities getNetworkCapabilities() {
+    @Nullable public NetworkCapabilities getNetworkCapabilities() {
         return networkCapabilities;
     }
 
-    public void setNetworkCapabilities(NetworkCapabilities networkCapabilities) {
+    public void setNetworkCapabilities(@Nullable NetworkCapabilities networkCapabilities) {
         this.networkCapabilities = networkCapabilities;
     }
 
-    public LinkProperties getLinkProperties() {
+    @Nullable public LinkProperties getLinkProperties() {
         return linkProperties;
     }
 
-    public void setLinkProperties(LinkProperties linkProperties) {
+    public void setLinkProperties(@Nullable LinkProperties linkProperties) {
         this.linkProperties = linkProperties;
     }
 }

library/src/main/java/com/github/pwittchen/reactivenetwork/library/rx2/network/observing/strategy/MarshmallowNetworkObservingStrategy.java

@@ -49,18 +49,19 @@
  * Network observing strategy for devices with Android Marshmallow (API 23) or higher.
  * Uses Network Callback API and handles Doze mode.
  */
-@Open @TargetApi(23) public class MarshmallowNetworkObservingStrategy
-    implements NetworkObservingStrategy {
+@Open
+@TargetApi(23)
+public class MarshmallowNetworkObservingStrategy implements NetworkObservingStrategy {
   protected static final String ERROR_MSG_NETWORK_CALLBACK =
       "could not unregister network callback";
   protected static final String ERROR_MSG_RECEIVER = "could not unregister receiver";
-
   @SuppressWarnings("NullAway") // it has to be initialized in the Observable due to Context
   private ConnectivityManager.NetworkCallback networkCallback;
   private final Subject<Connectivity> connectivitySubject;
   private final BroadcastReceiver idleReceiver;
   private Connectivity lastConnectivity = Connectivity.create();
-  private NetworkState networkState = new NetworkState();
+
+  @SuppressWarnings("FieldMayBeFinal") private NetworkState networkState = new NetworkState();
 
   @SuppressWarnings("NullAway") // networkCallback cannot be initialized here
   public MarshmallowNetworkObservingStrategy() {
@@ -76,7 +77,9 @@ public MarshmallowNetworkObservingStrategy() {
     registerIdleReceiver(context);
 
     final NetworkRequest request =
-        new NetworkRequest.Builder().addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
+        new NetworkRequest
+            .Builder()
+            .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
             .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED)
             .build();
 
@@ -100,12 +103,32 @@ public Publisher<Connectivity> apply(final Connectivity connectivity) {
     }).startWith(Connectivity.create(context)).distinctUntilChanged().toObservable();
   }
 
-  protected Publisher<Connectivity> propagateAnyConnectedState(final Connectivity last,
-      final Connectivity current) {
+  @SuppressWarnings("NullAway")
+  protected Publisher<Connectivity> propagateAnyConnectedState(
+      final Connectivity last,
+      final Connectivity current
+  ) {
+    final boolean hasNetworkState
+        = last.getNetworkState() != null
+        && current.getNetworkState() != null;
+
     final boolean typeChanged = last.type() != current.type();
-    final boolean wasConnected = last.state() == NetworkInfo.State.CONNECTED;
-    final boolean isDisconnected = current.state() == NetworkInfo.State.DISCONNECTED;
-    final boolean isNotIdle = current.detailedState() != NetworkInfo.DetailedState.IDLE;
+
+    boolean wasConnected;
+    boolean isDisconnected;
+    boolean isNotIdle;
+
+    if (hasNetworkState) {
+      // handling new NetworkState API
+      wasConnected = last.getNetworkState().isConnected();
+      isDisconnected = !current.getNetworkState().isConnected();
+      isNotIdle = true;
+    } else {
+      // handling legacy, deprecated NetworkInfo API
+      wasConnected = last.state() == NetworkInfo.State.CONNECTED;
+      isDisconnected = current.state() == NetworkInfo.State.DISCONNECTED;
+      isNotIdle = current.detailedState() != NetworkInfo.DetailedState.IDLE;
+    }
 
     if (typeChanged && wasConnected && isDisconnected && isNotIdle) {
       return Flowable.fromArray(current, last);
@@ -161,26 +184,28 @@ protected void tryToUnregisterReceiver(Context context) {
   protected ConnectivityManager.NetworkCallback createNetworkCallback(final Context context) {
     return new ConnectivityManager.NetworkCallback() {
       @Override
-      public void onCapabilitiesChanged(@NonNull Network network, @NonNull NetworkCapabilities networkCapabilities) {
+      public void onCapabilitiesChanged(@NonNull Network network,
+          @NonNull NetworkCapabilities networkCapabilities) {
         networkState.setNetwork(network);
         networkState.setNetworkCapabilities(networkCapabilities);
         onNext(Connectivity.create(context, networkState));
       }
 
       @Override
-      public void onLinkPropertiesChanged(@NonNull Network network, @NonNull LinkProperties linkProperties) {
+      public void onLinkPropertiesChanged(@NonNull Network network,
+          @NonNull LinkProperties linkProperties) {
         networkState.setNetwork(network);
         networkState.setLinkProperties(linkProperties);
         onNext(Connectivity.create(context, networkState));
       }
 
-      @Override public void onAvailable(Network network) {
+      @Override public void onAvailable(@NonNull Network network) {
         networkState.setNetwork(network);
         networkState.setConnected(true);
         onNext(Connectivity.create(context, networkState));
       }
 
-      @Override public void onLost(Network network) {
+      @Override public void onLost(@NonNull Network network) {
         networkState.setNetwork(network);
         networkState.setConnected(false);
         onNext(Connectivity.create(context, networkState));

library/src/test/java/com/github/pwittchen/reactivenetwork/library/rx2/ConnectivityTest.java

@@ -23,6 +23,7 @@
 import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.RuntimeEnvironment;
+import org.robolectric.annotation.Config;
 
 import static com.google.common.truth.Truth.assertThat;
 
@@ -32,7 +33,7 @@
   private static final String TYPE_NAME_MOBILE = "MOBILE";
   private static final String TYPE_NAME_NONE = "NONE";
 
-  @Test public void shouldCreateConnectivity() {
+  @Test @Config(sdk = 19) public void shouldCreateConnectivity() {
     // when
     Connectivity connectivity = Connectivity.create();
 
@@ -66,7 +67,8 @@
     assertThat(shouldBeEqualToGivenStatus).isTrue();
   }
 
-  @Test public void stateShouldBeEqualToOneOfGivenMultipleValues() throws Exception {
+  @Test @Config(sdk = 19)
+  public void stateShouldBeEqualToOneOfGivenMultipleValues() throws Exception {
     // given
     final Connectivity connectivity = Connectivity.state(NetworkInfo.State.CONNECTING)
         .type(ConnectivityManager.TYPE_WIFI)
@@ -174,7 +176,7 @@ public void createShouldThrowAnExceptionWhenContextIsNull() {
     // an exception is thrown
   }
 
-  @Test public void shouldReturnProperToStringValue() {
+  @Test @Config(sdk = 19) public void shouldReturnProperToStringValue() {
     // given
     final String expectedToString = "Connectivity{"
         + "state=DISCONNECTED, "
@@ -247,7 +249,8 @@ public void shouldAppendUnknownTypeWhileFilteringNetworkTypesInsidePredicateForE
     assertThat(outputTypes).isEqualTo(expectedOutputTypes);
   }
 
-  @Test public void shouldCreateConnectivityWithBuilder() {
+  @Test @Config(sdk = 19)
+  public void shouldCreateConnectivityWithBuilder() {
     // given
     NetworkInfo.State state = NetworkInfo.State.CONNECTED;
     NetworkInfo.DetailedState detailedState = NetworkInfo.DetailedState.CONNECTED;
@@ -321,7 +324,8 @@ public void shouldAppendUnknownTypeWhileFilteringNetworkTypesInsidePredicateForE
     assertThat(isAnotherConnectivityTheSame).isFalse();
   }
 
-  @Test public void shouldCreateDefaultConnectivityWhenConnectivityManagerIsNull() {
+  @Test @Config(sdk = 19)
+  public void shouldCreateDefaultConnectivityWhenConnectivityManagerIsNull() {
     // given
     final Context context = RuntimeEnvironment.getApplication().getApplicationContext();
     final ConnectivityManager connectivityManager = null;