[MS10] Address leftover comments on MS03 and MS07

- Fix the copyright year in IPMSDatabase.java.
- Add missing Override annotations.
- Remove random l2keys, use fixed strings.
- Rename the method in OnNetworkAttributesRetrieved that puzzlingly
  nobody noticed was wrong.

Test: atest IpMemoryStoreServiceTest
Bug: 113554482
Change-Id: If71fadd23e158a4be299d112bfce75690b1ed8e8

diff --git a/core/java/android/net/ipmemorystore/IOnNetworkAttributesRetrieved.aidl b/core/java/android/net/ipmemorystore/IOnNetworkAttributesRetrieved.aidl
index 57f59a1..fb4ca3b 100644 (file)
--- a/core/java/android/net/ipmemorystore/IOnNetworkAttributesRetrieved.aidl
+++ b/core/java/android/net/ipmemorystore/IOnNetworkAttributesRetrieved.aidl
@@ -25,6 +25,6 @@ oneway interface IOnNetworkAttributesRetrieved {
      * Network attributes were fetched for the specified L2 key. While the L2 key will never
      * be null, the attributes may be if no data is stored about this L2 key.
      */
-     void onL2KeyResponse(in StatusParcelable status, in String l2Key,
+     void onNetworkAttributesRetrieved(in StatusParcelable status, in String l2Key,
              in NetworkAttributesParcelable attributes);
 }

diff --git a/services/ipmemorystore/java/com/android/server/net/ipmemorystore/IpMemoryStoreDatabase.java b/services/ipmemorystore/java/com/android/server/net/ipmemorystore/IpMemoryStoreDatabase.java
index 238f077..32513c1 100644 (file)
--- a/services/ipmemorystore/java/com/android/server/net/ipmemorystore/IpMemoryStoreDatabase.java
+++ b/services/ipmemorystore/java/com/android/server/net/ipmemorystore/IpMemoryStoreDatabase.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018 The Android Open Source Project
+ * Copyright (C) 2019 The Android Open Source Project
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -134,12 +134,14 @@ public class IpMemoryStoreDatabase {
         }
 
         /** Called when the database is created */
+        @Override
         public void onCreate(@NonNull final SQLiteDatabase db) {
             db.execSQL(NetworkAttributesContract.CREATE_TABLE);
             db.execSQL(PrivateDataContract.CREATE_TABLE);
         }
 
         /** Called when the database is upgraded */
+        @Override
         public void onUpgrade(@NonNull final SQLiteDatabase db, final int oldVersion,
                 final int newVersion) {
             // No upgrade supported yet.
@@ -149,6 +151,7 @@ public class IpMemoryStoreDatabase {
         }
 
         /** Called when the database is downgraded */
+        @Override
         public void onDowngrade(@NonNull final SQLiteDatabase db, final int oldVersion,
                 final int newVersion) {
             // Downgrades always nuke all data and recreate an empty table.
@@ -247,7 +250,9 @@ public class IpMemoryStoreDatabase {
         // result here. 0 results means the key was not found.
         if (cursor.getCount() != 1) return EXPIRY_ERROR;
         cursor.moveToFirst();
-        return cursor.getLong(0); // index in the EXPIRY_COLUMN array
+        final long result = cursor.getLong(0); // index in the EXPIRY_COLUMN array
+        cursor.close();
+        return result;
     }
 
     static final int RELEVANCE_ERROR = -1; // Legal values for relevance are positive
@@ -321,6 +326,8 @@ public class IpMemoryStoreDatabase {
         final byte[] dnsAddressesBlob =
                 getBlob(cursor, NetworkAttributesContract.COLNAME_DNSADDRESSES);
         final int mtu = getInt(cursor, NetworkAttributesContract.COLNAME_MTU, -1);
+        cursor.close();
+
         if (0 != assignedV4AddressInt) {
             builder.setAssignedV4Address(NetworkUtils.intToInet4AddressHTH(assignedV4AddressInt));
         }
@@ -353,7 +360,9 @@ public class IpMemoryStoreDatabase {
         // get more than one result here. 0 results means the key was not found.
         if (cursor.getCount() != 1) return null;
         cursor.moveToFirst();
-        return cursor.getBlob(0); // index in the DATA_COLUMN array
+        final byte[] result = cursor.getBlob(0); // index in the DATA_COLUMN array
+        cursor.close();
+        return result;
     }
 
     // Helper methods

diff --git a/services/ipmemorystore/java/com/android/server/net/ipmemorystore/IpMemoryStoreService.java b/services/ipmemorystore/java/com/android/server/net/ipmemorystore/IpMemoryStoreService.java
index c4d1657..8b521f4 100644 (file)
--- a/services/ipmemorystore/java/com/android/server/net/ipmemorystore/IpMemoryStoreService.java
+++ b/services/ipmemorystore/java/com/android/server/net/ipmemorystore/IpMemoryStoreService.java
@@ -317,21 +317,22 @@ public class IpMemoryStoreService extends IIpMemoryStore.Stub {
         mExecutor.execute(() -> {
             try {
                 if (null == l2Key) {
-                    listener.onL2KeyResponse(makeStatus(ERROR_ILLEGAL_ARGUMENT), l2Key, null);
+                    listener.onNetworkAttributesRetrieved(
+                            makeStatus(ERROR_ILLEGAL_ARGUMENT), l2Key, null);
                     return;
                 }
                 if (null == mDb) {
-                    listener.onL2KeyResponse(makeStatus(ERROR_DATABASE_CANNOT_BE_OPENED), l2Key,
-                            null);
+                    listener.onNetworkAttributesRetrieved(
+                            makeStatus(ERROR_DATABASE_CANNOT_BE_OPENED), l2Key, null);
                     return;
                 }
                 try {
                     final NetworkAttributes attributes =
                             IpMemoryStoreDatabase.retrieveNetworkAttributes(mDb, l2Key);
-                    listener.onL2KeyResponse(makeStatus(SUCCESS), l2Key,
+                    listener.onNetworkAttributesRetrieved(makeStatus(SUCCESS), l2Key,
                             null == attributes ? null : attributes.toParcelable());
                 } catch (final Exception e) {
-                    listener.onL2KeyResponse(makeStatus(ERROR_GENERIC), l2Key, null);
+                    listener.onNetworkAttributesRetrieved(makeStatus(ERROR_GENERIC), l2Key, null);
                 }
             } catch (final RemoteException e) {
                 // Client at the other end died

diff --git a/tests/net/java/com/android/server/net/ipmemorystore/IpMemoryStoreServiceTest.java b/tests/net/java/com/android/server/net/ipmemorystore/IpMemoryStoreServiceTest.java
index c58941a..c748d0f 100644 (file)
--- a/tests/net/java/com/android/server/net/ipmemorystore/IpMemoryStoreServiceTest.java
+++ b/tests/net/java/com/android/server/net/ipmemorystore/IpMemoryStoreServiceTest.java
@@ -56,7 +56,6 @@ import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.Arrays;
-import java.util.UUID;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
@@ -68,6 +67,8 @@ public class IpMemoryStoreServiceTest {
     private static final String TEST_CLIENT_ID = "testClientId";
     private static final String TEST_DATA_NAME = "testData";
 
+    private static final String[] FAKE_KEYS = { "fakeKey1", "fakeKey2", "fakeKey3", "fakeKey4" };
+
     @Mock
     private Context mMockContext;
     private File mDbFile;
@@ -133,8 +134,8 @@ public class IpMemoryStoreServiceTest {
             final OnNetworkAttributesRetrievedListener functor) {
         return new IOnNetworkAttributesRetrieved() {
             @Override
-            public void onL2KeyResponse(final StatusParcelable status, final String l2Key,
-                    final NetworkAttributesParcelable attributes)
+            public void onNetworkAttributesRetrieved(final StatusParcelable status,
+                    final String l2Key, final NetworkAttributesParcelable attributes)
                     throws RemoteException {
                 functor.onNetworkAttributesRetrieved(new Status(status), l2Key,
                         null == attributes ? null : new NetworkAttributes(attributes));
@@ -202,7 +203,7 @@ public class IpMemoryStoreServiceTest {
         } catch (UnknownHostException e) { /* Can't happen */ }
         na.setGroupHint("hint1");
         na.setMtu(219);
-        final String l2Key = UUID.randomUUID().toString();
+        final String l2Key = FAKE_KEYS[0];
         NetworkAttributes attributes = na.build();
         storeAttributes(l2Key, attributes);
 
@@ -298,7 +299,7 @@ public class IpMemoryStoreServiceTest {
     public void testPrivateData() {
         final Blob b = new Blob();
         b.data = new byte[] { -3, 6, 8, -9, 12, -128, 0, 89, 112, 91, -34 };
-        final String l2Key = UUID.randomUUID().toString();
+        final String l2Key = FAKE_KEYS[0];
         doLatched("Did not complete storing private data", latch ->
                 mService.storeBlob(l2Key, TEST_CLIENT_ID, TEST_DATA_NAME, b,
                         onStatus(status -> {
@@ -353,29 +354,25 @@ public class IpMemoryStoreServiceTest {
         na.setMtu(219);
         na.setDnsAddresses(Arrays.asList(Inet6Address.getByName("0A1C:2E40:480A::1CA6")));
 
-        final String[] keys = new String[4];
-        for (int i = 0; i < keys.length; ++i) {
-            keys[i] = UUID.randomUUID().toString();
-        }
-        storeAttributes(keys[0], na.build());
+        storeAttributes(FAKE_KEYS[0], na.build());
         // 0 and 1 have identical attributes
-        storeAttributes(keys[1], na.build());
+        storeAttributes(FAKE_KEYS[1], na.build());
 
         // Hopefully only the MTU being different still means it's the same network
         na.setMtu(200);
-        storeAttributes(keys[2], na.build());
+        storeAttributes(FAKE_KEYS[2], na.build());
 
         // Hopefully different MTU, assigned V4 address and grouphint make a different network,
         // even with identical DNS addresses
         na.setAssignedV4Address(null);
         na.setGroupHint("hint2");
-        storeAttributes(keys[3], na.build());
+        storeAttributes(FAKE_KEYS[3], na.build());
 
-        assertNetworksSameness(keys[0], keys[1], SameL3NetworkResponse.NETWORK_SAME);
-        assertNetworksSameness(keys[0], keys[2], SameL3NetworkResponse.NETWORK_SAME);
-        assertNetworksSameness(keys[1], keys[2], SameL3NetworkResponse.NETWORK_SAME);
-        assertNetworksSameness(keys[0], keys[3], SameL3NetworkResponse.NETWORK_DIFFERENT);
-        assertNetworksSameness(keys[0], UUID.randomUUID().toString(),
+        assertNetworksSameness(FAKE_KEYS[0], FAKE_KEYS[1], SameL3NetworkResponse.NETWORK_SAME);
+        assertNetworksSameness(FAKE_KEYS[0], FAKE_KEYS[2], SameL3NetworkResponse.NETWORK_SAME);
+        assertNetworksSameness(FAKE_KEYS[1], FAKE_KEYS[2], SameL3NetworkResponse.NETWORK_SAME);
+        assertNetworksSameness(FAKE_KEYS[0], FAKE_KEYS[3], SameL3NetworkResponse.NETWORK_DIFFERENT);
+        assertNetworksSameness(FAKE_KEYS[0], "neverInsertedKey",
                 SameL3NetworkResponse.NETWORK_NEVER_CONNECTED);
 
         doLatched("Did not finish evaluating sameness", latch ->