Address review followup comments for NetworkStats
This change includes:
1. Mark groupedBy*() deprecated and throw after android U.
2. Modify map() to mapKeysNotNull().
3. rename clearInterfaces to withoutInterfaces and refactoring.
4. Modify tests accordingly.
Test: atest FrameworksNetTests:android.net.connectivity.android.net.NetworkStatsTest
Fix: 296149902
Fix: 296150018
Change-Id: I5d97422ba957a212c0c5fbc1eee3f8b174343348
diff --git a/framework-t/src/android/net/NetworkStats.java b/framework-t/src/android/net/NetworkStats.java
index 4f816c5..e9a3f58 100644
--- a/framework-t/src/android/net/NetworkStats.java
+++ b/framework-t/src/android/net/NetworkStats.java
@@ -32,6 +32,7 @@
import android.util.SparseBooleanArray;
import com.android.internal.annotations.VisibleForTesting;
+import com.android.modules.utils.build.SdkLevel;
import com.android.net.module.util.CollectionUtils;
import libcore.util.EmptyArray;
@@ -1245,18 +1246,21 @@
* Return total statistics grouped by {@link #iface}; doesn't mutate the
* original structure.
* @hide
+ * @deprecated Use {@link #mapKeysNotNull(Function)} instead.
*/
+ @Deprecated
public NetworkStats groupedByIface() {
+ if (SdkLevel.isAtLeastV()) {
+ throw new UnsupportedOperationException("groupedByIface is not supported");
+ }
// Keep backward compatibility where the method filtered out tagged stats and keep the
// operation counts as 0. The method used to deal with uid snapshot where tagged and
// non-tagged stats were mixed. And this method was also in Android O API list,
// so it is possible OEM can access it.
- final NetworkStats copiedStats = this.clone();
- copiedStats.filter(e -> e.getTag() == TAG_NONE);
-
final Entry temp = new Entry();
- final NetworkStats mappedStats = copiedStats.map(entry -> temp.setKeys(entry.getIface(),
- UID_ALL, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL));
+ final NetworkStats mappedStats = this.mapKeysNotNull(entry -> entry.getTag() != TAG_NONE
+ ? null : temp.setKeys(entry.getIface(), UID_ALL,
+ SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL));
for (int i = 0; i < mappedStats.size; i++) {
mappedStats.operations[i] = 0L;
@@ -1268,17 +1272,20 @@
* Return total statistics grouped by {@link #uid}; doesn't mutate the
* original structure.
* @hide
+ * @deprecated Use {@link #mapKeysNotNull(Function)} instead.
*/
+ @Deprecated
public NetworkStats groupedByUid() {
+ if (SdkLevel.isAtLeastV()) {
+ throw new UnsupportedOperationException("groupedByUid is not supported");
+ }
// Keep backward compatibility where the method filtered out tagged stats. The method used
// to deal with uid snapshot where tagged and non-tagged stats were mixed. And
// this method is also in Android O API list, so it is possible OEM can access it.
- final NetworkStats copiedStats = this.clone();
- copiedStats.filter(e -> e.getTag() == TAG_NONE);
-
final Entry temp = new Entry();
- return copiedStats.map(entry -> temp.setKeys(IFACE_ALL,
- entry.getUid(), SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL));
+ return this.mapKeysNotNull(entry -> entry.getTag() != TAG_NONE
+ ? null : temp.setKeys(IFACE_ALL, entry.getUid(),
+ SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL));
}
/**
@@ -1304,14 +1311,14 @@
}
/**
- * Removes the interface name from all entries.
- * This returns a newly constructed object instead of mutating the original structure.
+ * Returns a copy of this NetworkStats, replacing iface with IFACE_ALL in all entries.
+ *
* @hide
*/
@NonNull
- public NetworkStats clearInterfaces() {
+ public NetworkStats withoutInterfaces() {
final Entry temp = new Entry();
- return map(entry -> temp.setKeys(IFACE_ALL, entry.getUid(), entry.getSet(),
+ return mapKeysNotNull(entry -> temp.setKeys(IFACE_ALL, entry.getUid(), entry.getSet(),
entry.getTag(), entry.getMetered(), entry.getRoaming(), entry.getDefaultNetwork()));
}
@@ -1321,13 +1328,16 @@
* Note that because NetworkStats is more akin to a map than to a list,
* the entries will be grouped after they are mapped by the key fields,
* e.g. uid, set, tag, defaultNetwork.
- * Value fields with the same keys will be added together.
+ * Only the key returned by the function is used ; values will be forcefully
+ * copied from the original entry. Entries that map to the same set of keys
+ * will be added together.
*/
@NonNull
- private NetworkStats map(@NonNull Function<Entry, Entry> f) {
+ private NetworkStats mapKeysNotNull(@NonNull Function<Entry, Entry> f) {
final NetworkStats ret = new NetworkStats(0, 1);
for (Entry e : this) {
final NetworkStats.Entry transformed = f.apply(e);
+ if (transformed == null) continue;
if (transformed == e) {
throw new IllegalStateException("A new entry must be created.");
}
diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java
index c46eada..25e59d5 100644
--- a/service-t/src/com/android/server/net/NetworkStatsService.java
+++ b/service-t/src/com/android/server/net/NetworkStatsService.java
@@ -1745,7 +1745,7 @@
// information. This is because no caller needs this information for now, and it
// makes it easier to change the implementation later by using the histories in the
// recorder.
- return stats.clearInterfaces();
+ return stats.withoutInterfaces();
} catch (RemoteException e) {
Log.wtf(TAG, "Error compiling UID stats", e);
return new NetworkStats(0L, 0);
diff --git a/tests/unit/java/android/net/NetworkStatsTest.java b/tests/unit/java/android/net/NetworkStatsTest.java
index 4ff131b..70ddc17 100644
--- a/tests/unit/java/android/net/NetworkStatsTest.java
+++ b/tests/unit/java/android/net/NetworkStatsTest.java
@@ -51,6 +51,7 @@
import com.google.android.collect.Sets;
+import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -63,7 +64,8 @@
@SmallTest
@DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2)
public class NetworkStatsTest {
-
+ @Rule
+ public final DevSdkIgnoreRule mIgnoreRule = new DevSdkIgnoreRule();
private static final String TEST_IFACE = "test0";
private static final String TEST_IFACE2 = "test2";
private static final int TEST_UID = 1001;
@@ -339,6 +341,7 @@
assertEquals(96L, uidRoaming.getTotalBytes());
}
+ @DevSdkIgnoreRule.IgnoreAfter(Build.VERSION_CODES.UPSIDE_DOWN_CAKE)
@Test
public void testGroupedByIfaceEmpty() throws Exception {
final NetworkStats uidStats = new NetworkStats(TEST_START, 3);
@@ -348,6 +351,7 @@
assertEquals(0, grouped.size());
}
+ @DevSdkIgnoreRule.IgnoreAfter(Build.VERSION_CODES.UPSIDE_DOWN_CAKE)
@Test
public void testGroupedByIfaceAll() throws Exception {
final NetworkStats uidStats = new NetworkStats(TEST_START, 3)
@@ -366,6 +370,7 @@
DEFAULT_NETWORK_ALL, 384L, 24L, 0L, 6L, 0L);
}
+ @DevSdkIgnoreRule.IgnoreAfter(Build.VERSION_CODES.UPSIDE_DOWN_CAKE)
@Test
public void testGroupedByIface() throws Exception {
final NetworkStats uidStats = new NetworkStats(TEST_START, 7)
@@ -1068,7 +1073,7 @@
}
@Test
- public void testClearInterfaces() {
+ public void testWithoutInterfaces() {
final NetworkStats stats = new NetworkStats(TEST_START, 1);
final NetworkStats.Entry entry1 = new NetworkStats.Entry(
"test1", 10100, SET_DEFAULT, TAG_NONE, METERED_NO, ROAMING_NO,
@@ -1093,10 +1098,10 @@
assertValues(stats, 2, "test3", 10101, SET_DEFAULT, 0xF0DD, METERED_NO,
ROAMING_NO, DEFAULT_NETWORK_NO, 1, 2L, 3L, 4L, 5L);
- // Clear interfaces.
- final NetworkStats ifaceClearedStats = stats.clearInterfaces();
+ // Get stats without interfaces.
+ final NetworkStats ifaceClearedStats = stats.withoutInterfaces();
- // Verify that the interfaces are cleared, and key-duplicated items are merged.
+ // Verify that the interfaces do not exist, and key-duplicated items are merged.
assertEquals(2, ifaceClearedStats.size());
assertValues(ifaceClearedStats, 0, null /* iface */, 10100, SET_DEFAULT, TAG_NONE,
METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 1024L, 50L, 100L, 20L, 0L);