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