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);