Refactor CompareResult<> class and its call sites
Move all corner case logic from call sites to CompareResult's implementation,
add a constructor to directly do the comparison.
Test: runtest frameworks-core -c android.net.LinkPropertiesTest
Change-Id: I95bba82ec38d295b18c49c025dffab5f17271cbd
diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java
index f527f77..2c9fb23 100644
--- a/core/java/android/net/LinkProperties.java
+++ b/core/java/android/net/LinkProperties.java
@@ -70,8 +70,23 @@
* @hide
*/
public static class CompareResult<T> {
- public List<T> removed = new ArrayList<T>();
- public List<T> added = new ArrayList<T>();
+ public final List<T> removed = 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 @@
* are in target but not in mLinkAddresses are placed in the
* addedAddresses.
*/
- CompareResult<LinkAddress> result = new CompareResult<LinkAddress>();
- result.removed = new ArrayList<LinkAddress>(mLinkAddresses);
- result.added.clear();
- if (target != null) {
- for (LinkAddress newAddress : target.getLinkAddresses()) {
- if (! result.removed.remove(newAddress)) {
- result.added.add(newAddress);
- }
- }
- }
- return result;
+ return new CompareResult<>(mLinkAddresses,
+ target != null ? target.getLinkAddresses() : null);
}
/**
@@ -1029,18 +1035,7 @@
* are in target but not in mDnses are placed in the
* addedAddresses.
*/
- CompareResult<InetAddress> result = new CompareResult<InetAddress>();
-
- 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;
+ return new CompareResult<>(mDnses, target != null ? target.getDnsServers() : null);
}
/**
@@ -1058,18 +1053,7 @@
* 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>();
-
- 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;
+ return new CompareResult<>(getAllRoutes(), target != null ? target.getAllRoutes() : null);
}
/**
@@ -1087,18 +1071,8 @@
* 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>();
-
- 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;
+ return new CompareResult<>(getAllInterfaceNames(),
+ target != null ? target.getAllInterfaceNames() : null);
}
diff --git a/core/tests/coretests/src/android/net/LinkPropertiesTest.java b/core/tests/coretests/src/android/net/LinkPropertiesTest.java
index 9686dd9..bc2b9a6 100644
--- a/core/tests/coretests/src/android/net/LinkPropertiesTest.java
+++ b/core/tests/coretests/src/android/net/LinkPropertiesTest.java
@@ -19,6 +19,7 @@
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 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 @@
}
+ @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 @@
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)));
+ }
}
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 1703bd5..cee81a8 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -4435,12 +4435,9 @@
private void updateInterfaces(LinkProperties newLp, LinkProperties oldLp, int netId,
NetworkCapabilities caps) {
- CompareResult<String> interfaceDiff = new CompareResult<String>();
- if (oldLp != null) {
- interfaceDiff = oldLp.compareAllInterfaceNames(newLp);
- } else if (newLp != null) {
- interfaceDiff.added = newLp.getAllInterfaceNames();
- }
+ CompareResult<String> interfaceDiff = new CompareResult<String>(
+ oldLp != null ? oldLp.getAllInterfaceNames() : null,
+ newLp != null ? newLp.getAllInterfaceNames() : null);
for (String iface : interfaceDiff.added) {
try {
if (DBG) log("Adding iface " + iface + " to network " + netId);
@@ -4466,12 +4463,10 @@
* @return true if routes changed between oldLp and newLp
*/
private boolean updateRoutes(LinkProperties newLp, LinkProperties oldLp, int netId) {
- CompareResult<RouteInfo> routeDiff = new CompareResult<RouteInfo>();
- if (oldLp != null) {
- routeDiff = oldLp.compareAllRoutes(newLp);
- } else if (newLp != null) {
- routeDiff.added = newLp.getAllRoutes();
- }
+ // Compare the route diff to determine which routes should be added and removed.
+ CompareResult<RouteInfo> routeDiff = new CompareResult<RouteInfo>(
+ oldLp != null ? oldLp.getAllRoutes() : null,
+ newLp != null ? newLp.getAllRoutes() : null);
// add routes before removing old in case it helps with continuous connectivity