Merge "Refactor CompareResult<> class and its call sites"

core/java/android/net/LinkProperties.java

@@ -70,8 +70,23 @@ public final class LinkProperties implements Parcelable {
      * @hide
      */
     public static class CompareResult<T> {
-        public List<T> removed = new ArrayList<T>();
+        public final List<T> removed = new ArrayList<T>();
-        public List<T> added = new ArrayList<T>();
+        public final List<T> added = new ArrayList<T>();
+
+        public CompareResult() {}
+
+        public CompareResult(Collection<T> oldItems, Collection<T> newItems) {
+            if (oldItems != null) {
+                removed.addAll(oldItems);
+            }
+            if (newItems != null) {
+                for (T newItem : newItems) {
+                    if (!removed.remove(newItem)) {
+                        added.add(newItem);
+                    }
+                }
+            }
+        }
 
         @Override
         public String toString() {
@@ -1000,17 +1015,8 @@ public final class LinkProperties implements Parcelable {
          * are in target but not in mLinkAddresses are placed in the
          * addedAddresses.
          */
-        CompareResult<LinkAddress> result = new CompareResult<LinkAddress>();
+        return new CompareResult<>(mLinkAddresses,
-        result.removed = new ArrayList<LinkAddress>(mLinkAddresses);
+                target != null ? target.getLinkAddresses() : null);
-        result.added.clear();
-        if (target != null) {
-            for (LinkAddress newAddress : target.getLinkAddresses()) {
-                if (! result.removed.remove(newAddress)) {
-                    result.added.add(newAddress);
-                }
-            }
-        }
-        return result;
     }
 
     /**
@@ -1029,18 +1035,7 @@ public final class LinkProperties implements Parcelable {
          * are in target but not in mDnses are placed in the
          * addedAddresses.
          */
-        CompareResult<InetAddress> result = new CompareResult<InetAddress>();
+        return new CompareResult<>(mDnses, target != null ? target.getDnsServers() : null);
-
-        result.removed = new ArrayList<InetAddress>(mDnses);
-        result.added.clear();
-        if (target != null) {
-            for (InetAddress newAddress : target.getDnsServers()) {
-                if (! result.removed.remove(newAddress)) {
-                    result.added.add(newAddress);
-                }
-            }
-        }
-        return result;
     }
 
     /**
@@ -1058,18 +1053,7 @@ public final class LinkProperties implements Parcelable {
          * leaving the routes that are different. And route address which
          * are in target but not in mRoutes are placed in added.
          */
-        CompareResult<RouteInfo> result = new CompareResult<RouteInfo>();
+        return new CompareResult<>(getAllRoutes(), target != null ? target.getAllRoutes() : null);
-
-        result.removed = getAllRoutes();
-        result.added.clear();
-        if (target != null) {
-            for (RouteInfo r : target.getAllRoutes()) {
-                if (! result.removed.remove(r)) {
-                    result.added.add(r);
-                }
-            }
-        }
-        return result;
     }
 
     /**
@@ -1087,18 +1071,8 @@ public final class LinkProperties implements Parcelable {
          * leaving the interface names that are different. And interface names which
          * are in target but not in this are placed in added.
          */
-        CompareResult<String> result = new CompareResult<String>();
+        return new CompareResult<>(getAllInterfaceNames(),
-
+                target != null ? target.getAllInterfaceNames() : null);
-        result.removed = getAllInterfaceNames();
-        result.added.clear();
-        if (target != null) {
-            for (String r : target.getAllInterfaceNames()) {
-                if (! result.removed.remove(r)) {
-                    result.added.add(r);
-                }
-            }
-        }
-        return result;
     }
 
 

core/tests/coretests/src/android/net/LinkPropertiesTest.java

@@ -19,6 +19,7 @@ package android.net;
 import android.net.IpPrefix;
 import android.net.LinkAddress;
 import android.net.LinkProperties;
+import android.net.LinkProperties.CompareResult;
 import android.net.LinkProperties.ProvisioningChange;
 import android.net.RouteInfo;
 import android.system.OsConstants;
@@ -29,9 +30,11 @@ import android.util.ArraySet;
 import junit.framework.TestCase;
 
 import java.net.InetAddress;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 import java.util.Set;
 
 
@@ -747,6 +750,27 @@ public class LinkPropertiesTest extends TestCase {
 
     }
 
+    @SmallTest
+    public void testCompareResult() {
+        // Either adding or removing items
+        testCompareResult(Arrays.asList(1), Arrays.asList(1, 2, 3, 4),
+                Arrays.asList(2, 3, 4), new ArrayList<>());
+        testCompareResult(Arrays.asList(1, 2), Arrays.asList(3, 2, 1, 4),
+                new ArrayList<>(), Arrays.asList(3, 4));
+
+
+        // adding and removing items at the same time
+        testCompareResult(Arrays.asList(1, 2, 3, 4), Arrays.asList(2, 3, 4, 5),
+                Arrays.asList(1), Arrays.asList(5));
+        testCompareResult(Arrays.asList(1, 2, 3), Arrays.asList(4, 5, 6),
+                Arrays.asList(1, 2, 3), Arrays.asList(4, 5, 6));
+
+        // null cases
+        testCompareResult(Arrays.asList(1, 2, 3), null, Arrays.asList(1, 2, 3), new ArrayList<>());
+        testCompareResult(null, Arrays.asList(3, 2, 1), new ArrayList<>(), Arrays.asList(1, 2, 3));
+        testCompareResult(null, null, new ArrayList<>(), new ArrayList<>());
+    }
+
     private void assertEqualRoutes(Collection<RouteInfo> expected, Collection<RouteInfo> actual) {
         Set<RouteInfo> expectedSet = new ArraySet<>(expected);
         Set<RouteInfo> actualSet = new ArraySet<>(actual);
@@ -755,4 +779,11 @@ public class LinkPropertiesTest extends TestCase {
 
         assertEquals(expectedSet, actualSet);
     }
+
+    private <T> void testCompareResult(List<T> oldItems, List<T> newItems, List<T> expectAdded,
+            List<T> expectRemoved) {
+        CompareResult<T> result = new CompareResult<>(newItems, oldItems);
+        assertEquals(new ArraySet<>(expectAdded), new ArraySet<>(result.added));
+        assertEquals(new ArraySet<>(expectRemoved), (new ArraySet<>(result.removed)));
+    }
 }

services/core/java/com/android/server/ConnectivityService.java

@@ -4435,12 +4435,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
 
     private void updateInterfaces(LinkProperties newLp, LinkProperties oldLp, int netId,
                                   NetworkCapabilities caps) {
-        CompareResult<String> interfaceDiff = new CompareResult<String>();
+        CompareResult<String> interfaceDiff = new CompareResult<String>(
-        if (oldLp != null) {
+                oldLp != null ? oldLp.getAllInterfaceNames() : null,
-            interfaceDiff = oldLp.compareAllInterfaceNames(newLp);
+                newLp != null ? newLp.getAllInterfaceNames() : null);
-        } else if (newLp != null) {
-            interfaceDiff.added = newLp.getAllInterfaceNames();
-        }
         for (String iface : interfaceDiff.added) {
             try {
                 if (DBG) log("Adding iface " + iface + " to network " + netId);
@@ -4466,12 +4463,10 @@ public class ConnectivityService extends IConnectivityManager.Stub
      * @return true if routes changed between oldLp and newLp
      */
     private boolean updateRoutes(LinkProperties newLp, LinkProperties oldLp, int netId) {
-        CompareResult<RouteInfo> routeDiff = new CompareResult<RouteInfo>();
+        // Compare the route diff to determine which routes should be added and removed.
-        if (oldLp != null) {
+        CompareResult<RouteInfo> routeDiff = new CompareResult<RouteInfo>(
-            routeDiff = oldLp.compareAllRoutes(newLp);
+                oldLp != null ? oldLp.getAllRoutes() : null,
-        } else if (newLp != null) {
+                newLp != null ? newLp.getAllRoutes() : null);
-            routeDiff.added = newLp.getAllRoutes();
-        }
 
         // add routes before removing old in case it helps with continuous connectivity