Merge "Minor improvements on UserHalHelper / UserHalService." into rvc-dev
diff --git a/service/src/com/android/car/hal/UserHalService.java b/service/src/com/android/car/hal/UserHalService.java
index 9abe92b..7036912 100644
--- a/service/src/com/android/car/hal/UserHalService.java
+++ b/service/src/com/android/car/hal/UserHalService.java
@@ -80,8 +80,7 @@
             USER_IDENTIFICATION_ASSOCIATION
     };
 
-    // TODO(b/150413515): STOPSHIP - change to false before R is launched
-    private static final boolean DBG = true;
+    private static final boolean DBG = false;
 
     private final Object mLock = new Object();
 
@@ -183,8 +182,7 @@
             Log.w(TAG, UNSUPPORTED_MSG);
             return;
         }
-        // TODO(b/150413515): increase capacity as more properties are added
-        SparseArray<VehiclePropConfig> supportedProperties = new SparseArray<>(2);
+        SparseArray<VehiclePropConfig> supportedProperties = new SparseArray<>(5);
         for (VehiclePropConfig config : properties) {
             supportedProperties.put(config.prop, config);
         }
@@ -224,7 +222,7 @@
         if (DBG) Log.d(TAG, "getInitialInfo(" + requestType + ")");
         Preconditions.checkArgumentPositive(timeoutMs, "timeout must be positive");
         Objects.requireNonNull(usersInfo);
-        // TODO(b/150413515): use helper method to check usersInfo is valid
+        UserHalHelper.checkValid(usersInfo);
         Objects.requireNonNull(callback);
 
         VehiclePropValue propRequest;
@@ -273,11 +271,11 @@
     public void switchUser(@NonNull UserInfo targetInfo, int timeoutMs,
             @NonNull UsersInfo usersInfo, @NonNull HalCallback<SwitchUserResponse> callback) {
         if (DBG) Log.d(TAG, "switchUser(" + targetInfo + ")");
-        // TODO(b/150413515): check that targetInfo is not null / add unit test
+
+        Objects.requireNonNull(targetInfo);
         Preconditions.checkArgumentPositive(timeoutMs, "timeout must be positive");
-        Objects.requireNonNull(usersInfo);
-        // TODO(b/150413515): use helper method to check usersInfo is valid
         Objects.requireNonNull(callback);
+        UserHalHelper.checkValid(usersInfo);
 
         VehiclePropValue propRequest;
         int requestId;
@@ -307,8 +305,7 @@
         EventLog.writeEvent(EventLogTags.CAR_USER_HAL_POST_SWITCH_USER_REQ, requestId,
                 targetInfo.userId, usersInfo.currentUser.userId);
         if (DBG) Log.d(TAG, "postSwitchResponse(" + targetInfo + ")");
-        Objects.requireNonNull(usersInfo);
-        // TODO(b/150413515): use helper method to check usersInfo is valid
+        UserHalHelper.checkValid(usersInfo);
 
         VehiclePropValue propRequest;
         synchronized (mLock) {
@@ -335,8 +332,7 @@
      */
     public void legacyUserSwitch(@NonNull UserInfo targetInfo, @NonNull UsersInfo usersInfo) {
         if (DBG) Log.d(TAG, "userSwitchLegacy(" + targetInfo + ")");
-        Objects.requireNonNull(usersInfo);
-        // TODO(b/150413515): use helper method to check usersInfo is valid
+        UserHalHelper.checkValid(usersInfo);
 
         VehiclePropValue propRequest;
         synchronized (mLock) {
@@ -356,6 +352,7 @@
         }
     }
 
+    // TODO(b/157699720): move to UserHalHelper
     private static VehiclePropValue getPropRequestForSwitchUserLocked(int requestId,
             int requestType, @NonNull UserInfo targetInfo, @NonNull UsersInfo usersInfo) {
         VehiclePropValue propRequest =
@@ -625,7 +622,6 @@
     }
 
     private void handleOnInitialUserInfoResponse(VehiclePropValue value) {
-        // TODO(b/150413515): record (for dumping()) the last N responses.
         int requestId = value.value.int32Values.get(0);
         HalCallback<InitialUserInfoResponse> callback = handleGetPendingCallback(requestId,
                 InitialUserInfoResponse.class);
diff --git a/tests/carservice_unit_test/src/android/car/userlib/UserHalHelperTest.java b/tests/carservice_unit_test/src/android/car/userlib/UserHalHelperTest.java
index 3ae1eb5..9487871 100644
--- a/tests/carservice_unit_test/src/android/car/userlib/UserHalHelperTest.java
+++ b/tests/carservice_unit_test/src/android/car/userlib/UserHalHelperTest.java
@@ -926,6 +926,63 @@
         assertThat(usersInfo.existingUsers.get(1).flags).isEqualTo(UserFlags.NONE);
     }
 
+    @Test
+    public void testCheckValidUsersInfo_null() {
+        assertThrows(IllegalArgumentException.class, () -> UserHalHelper.checkValid(null));
+    }
+
+    @Test
+    public void testCheckValidUsersInfo_empty() {
+        UsersInfo usersInfo = new UsersInfo();
+        assertThrows(IllegalArgumentException.class, () -> UserHalHelper.checkValid(usersInfo));
+    }
+
+    @Test
+    public void testCheckValidUsersInfo_sizeMismatch() {
+        UsersInfo usersInfo = new UsersInfo();
+        usersInfo.numberUsers = 1;
+        assertThrows(IllegalArgumentException.class, () -> UserHalHelper.checkValid(usersInfo));
+    }
+
+    @Test
+    public void testCheckValidUsersInfo_currentUserMissing() {
+        UsersInfo usersInfo = new UsersInfo();
+        usersInfo.numberUsers = 1;
+        usersInfo.currentUser.userId = 10;
+        usersInfo.existingUsers.add(new android.hardware.automotive.vehicle.V2_0.UserInfo());
+
+        assertThrows(IllegalArgumentException.class, () -> UserHalHelper.checkValid(usersInfo));
+    }
+
+    @Test
+    public void testCheckValidUsersInfo_currentUserFlagsMismatch() {
+        UsersInfo usersInfo = new UsersInfo();
+        usersInfo.numberUsers = 1;
+        usersInfo.currentUser.userId = 10;
+        usersInfo.currentUser.flags = UserFlags.ADMIN;
+        android.hardware.automotive.vehicle.V2_0.UserInfo currentUser =
+                new android.hardware.automotive.vehicle.V2_0.UserInfo();
+        currentUser.userId = 10;
+        currentUser.flags = UserFlags.SYSTEM;
+        usersInfo.existingUsers.add(currentUser);
+
+        assertThrows(IllegalArgumentException.class, () -> UserHalHelper.checkValid(usersInfo));
+    }
+
+    @Test
+    public void testCheckValidUsersInfo_ok() {
+        UsersInfo usersInfo = new UsersInfo();
+        usersInfo.numberUsers = 1;
+        usersInfo.currentUser.userId = 10;
+
+        android.hardware.automotive.vehicle.V2_0.UserInfo currentUser =
+                new android.hardware.automotive.vehicle.V2_0.UserInfo();
+        currentUser.userId = 10;
+        usersInfo.existingUsers.add(currentUser);
+
+        UserHalHelper.checkValid(usersInfo);
+    }
+
     private static void assertEmptyUsersInfo(UsersInfo usersInfo) {
         assertThat(usersInfo).isNotNull();
         assertThat(usersInfo.currentUser.userId).isEqualTo(UserHandle.USER_NULL);
diff --git a/tests/carservice_unit_test/src/com/android/car/hal/UserHalServiceTest.java b/tests/carservice_unit_test/src/com/android/car/hal/UserHalServiceTest.java
index 56d5395..b001a96 100644
--- a/tests/carservice_unit_test/src/com/android/car/hal/UserHalServiceTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/hal/UserHalServiceTest.java
@@ -199,20 +199,16 @@
 
     @Test
     public void testGetUserInfo_invalidTimeout() {
-        assertThrows(IllegalArgumentException.class,
-                () -> mUserHalService.getInitialUserInfo(COLD_BOOT, 0, mUsersInfo, (i, r) -> {
-                }));
-        assertThrows(IllegalArgumentException.class,
-                () -> mUserHalService.getInitialUserInfo(COLD_BOOT, -1, mUsersInfo, (i, r) -> {
-                }));
+        assertThrows(IllegalArgumentException.class, () ->
+                mUserHalService.getInitialUserInfo(COLD_BOOT, 0, mUsersInfo, noOpCallback()));
+        assertThrows(IllegalArgumentException.class, () ->
+                mUserHalService.getInitialUserInfo(COLD_BOOT, -1, mUsersInfo, noOpCallback()));
     }
 
     @Test
     public void testGetUserInfo_noUsersInfo() {
-        assertThrows(NullPointerException.class,
-                () -> mUserHalService.getInitialUserInfo(COLD_BOOT, TIMEOUT_MS, null,
-                        (i, r) -> {
-                        }));
+        assertThrows(NullPointerException.class, () ->
+                mUserHalService.getInitialUserInfo(COLD_BOOT, TIMEOUT_MS, null, noOpCallback()));
     }
 
     @Test
@@ -414,26 +410,27 @@
     @Test
     public void testSwitchUser_invalidTimeout() {
         assertThrows(IllegalArgumentException.class,
-                () -> mUserHalService.switchUser(mUser10, 0, mUsersInfo, (i, r) -> {
-                }));
+                () -> mUserHalService.switchUser(mUser10, 0, mUsersInfo, noOpCallback()));
         assertThrows(IllegalArgumentException.class,
-                () -> mUserHalService.switchUser(mUser10, -1, mUsersInfo, (i, r) -> {
-                }));
+                () -> mUserHalService.switchUser(mUser10, -1, mUsersInfo, noOpCallback()));
     }
 
     @Test
     public void testSwitchUser_noUsersInfo() {
-        assertThrows(NullPointerException.class,
-                () -> mUserHalService.switchUser(mUser10, TIMEOUT_MS, null,
-                        (i, r) -> {
-                        }));
+        assertThrows(IllegalArgumentException.class,
+                () -> mUserHalService.switchUser(mUser10, TIMEOUT_MS, null, noOpCallback()));
     }
 
     @Test
     public void testSwitchUser_noCallback() {
         assertThrows(NullPointerException.class,
-                () -> mUserHalService.switchUser(mUser10, TIMEOUT_MS,
-                        mUsersInfo, null));
+                () -> mUserHalService.switchUser(mUser10, TIMEOUT_MS, mUsersInfo, null));
+    }
+
+    @Test
+    public void testSwitchUser_noTarget() {
+        assertThrows(NullPointerException.class,
+                () -> mUserHalService.switchUser(null, TIMEOUT_MS, mUsersInfo, noOpCallback()));
     }
 
     @Test
@@ -643,7 +640,7 @@
 
     @Test
     public void testUserSwitchLegacy_noUsersInfo() {
-        assertThrows(NullPointerException.class,
+        assertThrows(IllegalArgumentException.class,
                 () -> mUserHalService.legacyUserSwitch(mUser10, null));
     }
 
@@ -771,15 +768,15 @@
     public void testSetUserAssociation_invalidTimeout() {
         UserIdentificationSetRequest request = new UserIdentificationSetRequest();
         assertThrows(IllegalArgumentException.class, () ->
-                mUserHalService.setUserAssociation(0, request, (i, r) -> { }));
+                mUserHalService.setUserAssociation(0, request, noOpCallback()));
         assertThrows(IllegalArgumentException.class, () ->
-                mUserHalService.setUserAssociation(-1, request, (i, r) -> { }));
+                mUserHalService.setUserAssociation(-1, request, noOpCallback()));
     }
 
     @Test
     public void testSetUserAssociation_nullRequest() {
         assertThrows(NullPointerException.class, () ->
-                mUserHalService.setUserAssociation(TIMEOUT_MS, null, (i, r) -> { }));
+                mUserHalService.setUserAssociation(TIMEOUT_MS, null, noOpCallback()));
     }
 
     @Test
@@ -800,7 +797,7 @@
         request.associations.add(association1);
 
         assertThrows(IllegalArgumentException.class, () ->
-                mUserHalService.setUserAssociation(TIMEOUT_MS, request, (i, r) -> { }));
+                mUserHalService.setUserAssociation(TIMEOUT_MS, request, noOpCallback()));
     }
 
     @Test
@@ -1157,6 +1154,10 @@
                 /* numberAssociations= */ 1, KEY_FOB, ASSOCIATE_CURRENT_USER);
     }
 
+    private static <T> HalCallback<T> noOpCallback() {
+        return (i, r) -> { };
+    }
+
     private final class GenericHalCallback<R> implements HalCallback<R> {
 
         private final CountDownLatch mLatch = new CountDownLatch(1);
diff --git a/user/car-user-lib/src/android/car/userlib/UserHalHelper.java b/user/car-user-lib/src/android/car/userlib/UserHalHelper.java
index 65e6196..5537245 100644
--- a/user/car-user-lib/src/android/car/userlib/UserHalHelper.java
+++ b/user/car-user-lib/src/android/car/userlib/UserHalHelper.java
@@ -509,6 +509,32 @@
         return usersInfo;
     }
 
+    /**
+     * Checks if the given {@code usersInfo} is valid.
+     *
+     * @throws IllegalArgumentException if it isn't.
+     */
+    public static void checkValid(@NonNull UsersInfo usersInfo) {
+        Preconditions.checkArgument(usersInfo != null);
+        Preconditions.checkArgument(usersInfo.numberUsers == usersInfo.existingUsers.size(),
+                "sizes mismatch: numberUsers=%d, existingUsers.size=%d", usersInfo.numberUsers,
+                usersInfo.existingUsers.size());
+        boolean hasCurrentUser = false;
+        int currentUserFlags = UserFlags.NONE;
+        for (int i = 0; i < usersInfo.numberUsers; i++) {
+            android.hardware.automotive.vehicle.V2_0.UserInfo user = usersInfo.existingUsers.get(i);
+            if (user.userId == usersInfo.currentUser.userId) {
+                hasCurrentUser = true;
+                currentUserFlags = user.flags;
+                break;
+            }
+        }
+        Preconditions.checkArgument(hasCurrentUser,
+                "current user not found on existing users on %s", usersInfo);
+        Preconditions.checkArgument(usersInfo.currentUser.flags == currentUserFlags,
+                "current user flags mismatch on existing users on %s", usersInfo);
+    }
+
     @NonNull
     private static UsersInfo emptyUsersInfo() {
         UsersInfo usersInfo = new UsersInfo();