Throw exception when trying to instantiate a change log token with no
record types.
Note that this does mean that any developers that were previously
requesting change logs without specifying record types will no longer be
able to. However, the APK doesn't allow this anyway, so it is likely ok.
Test: atest CtsHealthFitnessDeviceTestCases:HealthConnectChangeLogsTests#testGetChangeLogToken_emptyRecordTypes_throwsException
Bug: 327332482
(cherry picked from commit cd228a3e21c9c8df83bc3851736d6f4e19956e46)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:c47251b82509773719b4b797d508b03216ab0e71)
Merged-In: I5dd49131fbe5eaa8bc138be03550f7fc739786d9
Change-Id: I5dd49131fbe5eaa8bc138be03550f7fc739786d9
diff --git a/framework/java/android/health/connect/changelog/ChangeLogTokenRequest.java b/framework/java/android/health/connect/changelog/ChangeLogTokenRequest.java
index 28241f8..e630e3c 100644
--- a/framework/java/android/health/connect/changelog/ChangeLogTokenRequest.java
+++ b/framework/java/android/health/connect/changelog/ChangeLogTokenRequest.java
@@ -49,6 +49,9 @@
private ChangeLogTokenRequest(
@NonNull Set<DataOrigin> dataOriginFilters,
@NonNull Set<Class<? extends Record>> recordTypes) {
+ if (recordTypes.isEmpty()) {
+ throw new IllegalArgumentException("Requested record types must not be empty");
+ }
Objects.requireNonNull(recordTypes);
Objects.requireNonNull(dataOriginFilters);
@@ -161,8 +164,8 @@
private final Set<DataOrigin> mDataOriginFilters = new ArraySet<>();
/**
- * @param recordType type of record for which change log is required. If not set includes
- * all record types
+ * @param recordType type of record for which change log is required. At least one record
+ * type must be set.
*/
@NonNull
public Builder addRecordType(@NonNull Class<? extends Record> recordType) {
diff --git a/service/java/com/android/server/healthconnect/HealthConnectServiceImpl.java b/service/java/com/android/server/healthconnect/HealthConnectServiceImpl.java
index 578db33..9becd0c 100644
--- a/service/java/com/android/server/healthconnect/HealthConnectServiceImpl.java
+++ b/service/java/com/android/server/healthconnect/HealthConnectServiceImpl.java
@@ -798,6 +798,10 @@
mAppOpsManagerLocal.isUidInForeground(uid),
builder);
throwExceptionIfDataSyncInProgress();
+ if (request.getRecordTypes().isEmpty()) {
+ throw new IllegalArgumentException(
+ "Requested record types must not be empty.");
+ }
mDataPermissionEnforcer.enforceRecordIdsReadPermissions(
request.getRecordTypesList(), attributionSource);
callback.onResult(
@@ -816,6 +820,14 @@
builder.setHealthDataServiceApiStatusError(ERROR_SECURITY);
Slog.e(TAG, "SecurityException: ", securityException);
tryAndThrowException(callback, securityException, ERROR_SECURITY);
+ } catch (IllegalArgumentException illegalArgumentException) {
+ builder.setHealthDataServiceApiStatusError(
+ HealthConnectException.ERROR_INVALID_ARGUMENT);
+ Slog.e(TAG, "IllegalArgumentException: ", illegalArgumentException);
+ tryAndThrowException(
+ callback,
+ illegalArgumentException,
+ HealthConnectException.ERROR_INVALID_ARGUMENT);
} catch (HealthConnectException healthConnectException) {
builder.setHealthDataServiceApiStatusError(
healthConnectException.getErrorCode());
@@ -862,6 +874,10 @@
ChangeLogsRequestHelper.TokenRequest changeLogsTokenRequest =
ChangeLogsRequestHelper.getRequest(
attributionSource.getPackageName(), token.getToken());
+ if (changeLogsTokenRequest.getRecordTypes().isEmpty()) {
+ throw new IllegalArgumentException(
+ "Requested record types must not be empty.");
+ }
mDataPermissionEnforcer.enforceRecordIdsReadPermissions(
changeLogsTokenRequest.getRecordTypes(), attributionSource);
boolean isInForeground = mAppOpsManagerLocal.isUidInForeground(uid);
diff --git a/tests/cts/hostsidetests/healthconnect/HealthConnectTestHelper/src/android/healthconnect/cts/testhelper/HealthConnectServiceLogsTests.java b/tests/cts/hostsidetests/healthconnect/HealthConnectTestHelper/src/android/healthconnect/cts/testhelper/HealthConnectServiceLogsTests.java
index 9eaa71a..d983442 100644
--- a/tests/cts/hostsidetests/healthconnect/HealthConnectTestHelper/src/android/healthconnect/cts/testhelper/HealthConnectServiceLogsTests.java
+++ b/tests/cts/hostsidetests/healthconnect/HealthConnectTestHelper/src/android/healthconnect/cts/testhelper/HealthConnectServiceLogsTests.java
@@ -43,6 +43,7 @@
import android.health.connect.changelog.ChangeLogsRequest;
import android.health.connect.changelog.ChangeLogsResponse;
import android.health.connect.datatypes.BloodPressureRecord;
+import android.health.connect.datatypes.HeartRateRecord;
import android.health.connect.datatypes.HeightRecord;
import android.health.connect.datatypes.Record;
import android.health.connect.datatypes.StepsRecord;
@@ -356,7 +357,10 @@
CountDownLatch latch = new CountDownLatch(1);
assertThat(mHealthConnectManager).isNotNull();
mHealthConnectManager.getChangeLogToken(
- new ChangeLogTokenRequest.Builder().build(),
+ new ChangeLogTokenRequest.Builder()
+ .addRecordType(BloodPressureRecord.class)
+ .addRecordType(HeartRateRecord.class)
+ .build(),
Executors.newSingleThreadExecutor(),
new OutcomeReceiver<>() {
diff --git a/tests/cts/hostsidetests/healthconnect/device/src/android/healthconnect/cts/device/HealthConnectDeviceTest.java b/tests/cts/hostsidetests/healthconnect/device/src/android/healthconnect/cts/device/HealthConnectDeviceTest.java
index ec1afc4..659c362 100644
--- a/tests/cts/hostsidetests/healthconnect/device/src/android/healthconnect/cts/device/HealthConnectDeviceTest.java
+++ b/tests/cts/hostsidetests/healthconnect/device/src/android/healthconnect/cts/device/HealthConnectDeviceTest.java
@@ -279,70 +279,4 @@
APP_A_WITH_READ_WRITE_PERMS, recordClassesToRead);
assertThat(bundle.getInt(READ_RECORDS_SIZE)).isEqualTo(noOfRecordsInsertedByAppA);
}
-
- @Test
- public void testAppCanReadChangeLogsUsingDataOriginFilters() throws Exception {
- Bundle bundle =
- getChangeLogTokenAs(
- APP_B_WITH_READ_WRITE_PERMS, APP_A_WITH_READ_WRITE_PERMS.getPackageName());
- String changeLogTokenForAppB = bundle.getString(CHANGE_LOG_TOKEN);
-
- bundle =
- getChangeLogTokenAs(
- APP_A_WITH_READ_WRITE_PERMS, APP_B_WITH_READ_WRITE_PERMS.getPackageName());
- String changeLogTokenForAppA = bundle.getString(CHANGE_LOG_TOKEN);
-
- bundle = insertRecordAs(APP_A_WITH_READ_WRITE_PERMS);
- assertThat(bundle.getBoolean(SUCCESS)).isTrue();
-
- List<TestUtils.RecordTypeAndRecordIds> listOfRecordIdsAndClass =
- (List<TestUtils.RecordTypeAndRecordIds>) bundle.getSerializable(RECORD_IDS);
-
- List<String> listOfRecordIdsInsertedByAppA = new ArrayList<>();
- int noOfRecordsInsertedByAppA = 0;
- for (TestUtils.RecordTypeAndRecordIds recordTypeAndRecordIds : listOfRecordIdsAndClass) {
- noOfRecordsInsertedByAppA += recordTypeAndRecordIds.getRecordIds().size();
- listOfRecordIdsInsertedByAppA.addAll(recordTypeAndRecordIds.getRecordIds());
- }
-
- updateRecordsAs(APP_A_WITH_READ_WRITE_PERMS, listOfRecordIdsAndClass);
-
- bundle = insertRecordAs(APP_B_WITH_READ_WRITE_PERMS);
- assertThat(bundle.getBoolean(SUCCESS)).isTrue();
-
- listOfRecordIdsAndClass =
- (List<TestUtils.RecordTypeAndRecordIds>) bundle.getSerializable(RECORD_IDS);
-
- int noOfRecordsInsertedByAppB = 0;
- for (TestUtils.RecordTypeAndRecordIds recordTypeAndRecordIds : listOfRecordIdsAndClass) {
- noOfRecordsInsertedByAppB += recordTypeAndRecordIds.getRecordIds().size();
- }
-
- deleteRecordsAs(APP_B_WITH_READ_WRITE_PERMS, listOfRecordIdsAndClass);
-
- bundle =
- readChangeLogsUsingDataOriginFiltersAs(
- APP_B_WITH_READ_WRITE_PERMS, changeLogTokenForAppB);
-
- ChangeLogsResponse response = bundle.getParcelable(CHANGE_LOGS_RESPONSE);
-
- assertThat(response.getUpsertedRecords().size()).isEqualTo(noOfRecordsInsertedByAppA);
- assertThat(
- response.getUpsertedRecords().stream()
- .map(Record::getMetadata)
- .map(Metadata::getId)
- .toList())
- .containsExactlyElementsIn(listOfRecordIdsInsertedByAppA);
-
- assertThat(response.getDeletedLogs().size()).isEqualTo(0);
-
- bundle =
- readChangeLogsUsingDataOriginFiltersAs(
- APP_A_WITH_READ_WRITE_PERMS, changeLogTokenForAppA);
-
- response = bundle.getParcelable(CHANGE_LOGS_RESPONSE);
-
- assertThat(response.getUpsertedRecords().size()).isEqualTo(0);
- assertThat(response.getDeletedLogs().size()).isEqualTo(noOfRecordsInsertedByAppB);
- }
}
diff --git a/tests/cts/src/android/healthconnect/cts/HealthConnectChangeLogsTests.java b/tests/cts/src/android/healthconnect/cts/HealthConnectChangeLogsTests.java
index 71e38ef..c58bae2 100644
--- a/tests/cts/src/android/healthconnect/cts/HealthConnectChangeLogsTests.java
+++ b/tests/cts/src/android/healthconnect/cts/HealthConnectChangeLogsTests.java
@@ -18,6 +18,8 @@
import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
+
import android.content.Context;
import android.health.connect.changelog.ChangeLogTokenRequest;
import android.health.connect.changelog.ChangeLogTokenResponse;
@@ -53,7 +55,8 @@
@Test
public void testGetChangeLogToken() throws InterruptedException {
- ChangeLogTokenRequest changeLogTokenRequest = new ChangeLogTokenRequest.Builder().build();
+ ChangeLogTokenRequest changeLogTokenRequest =
+ new ChangeLogTokenRequest.Builder().addRecordType(StepsRecord.class).build();
assertThat(TestUtils.getChangeLogToken(changeLogTokenRequest)).isNotNull();
assertThat(changeLogTokenRequest.getRecordTypes()).isNotNull();
assertThat(changeLogTokenRequest.getDataOriginFilters()).isNotNull();
@@ -62,7 +65,8 @@
@Test
public void testChangeLogs_insert_default() throws InterruptedException {
ChangeLogTokenResponse tokenResponse =
- TestUtils.getChangeLogToken(new ChangeLogTokenRequest.Builder().build());
+ TestUtils.getChangeLogToken(
+ TestUtils.getChangeLogTokenRequestForTestRecordTypes().build());
ChangeLogsRequest changeLogsRequest =
new ChangeLogsRequest.Builder(tokenResponse.getToken()).build();
assertThat(changeLogsRequest.getToken()).isNotNull();
@@ -82,7 +86,7 @@
public void testChangeLogs_insert_dataOrigin_filter_incorrect() throws InterruptedException {
ChangeLogTokenResponse tokenResponse =
TestUtils.getChangeLogToken(
- new ChangeLogTokenRequest.Builder()
+ TestUtils.getChangeLogTokenRequestForTestRecordTypes()
.addDataOriginFilter(
new DataOrigin.Builder().setPackageName("random").build())
.build());
@@ -106,7 +110,7 @@
Context context = ApplicationProvider.getApplicationContext();
ChangeLogTokenResponse tokenResponse =
TestUtils.getChangeLogToken(
- new ChangeLogTokenRequest.Builder()
+ TestUtils.getChangeLogTokenRequestForTestRecordTypes()
.addDataOriginFilter(
new DataOrigin.Builder()
.setPackageName(context.getPackageName())
@@ -160,7 +164,8 @@
@Test
public void testChangeLogs_insertAndDelete_default() throws InterruptedException {
ChangeLogTokenResponse tokenResponse =
- TestUtils.getChangeLogToken(new ChangeLogTokenRequest.Builder().build());
+ TestUtils.getChangeLogToken(
+ TestUtils.getChangeLogTokenRequestForTestRecordTypes().build());
ChangeLogsRequest changeLogsRequest =
new ChangeLogsRequest.Builder(tokenResponse.getToken()).build();
ChangeLogsResponse response = TestUtils.getChangeLogs(changeLogsRequest);
@@ -186,7 +191,8 @@
@Test
public void testChangeLogs_insertAndDelete_beforePermission() throws InterruptedException {
ChangeLogTokenResponse tokenResponse =
- TestUtils.getChangeLogToken(new ChangeLogTokenRequest.Builder().build());
+ TestUtils.getChangeLogToken(new ChangeLogTokenRequest.Builder().addRecordType(
+ StepsRecord.class).build());
ChangeLogsRequest changeLogsRequest =
new ChangeLogsRequest.Builder(tokenResponse.getToken()).build();
ChangeLogsResponse response = TestUtils.getChangeLogs(changeLogsRequest);
@@ -212,7 +218,7 @@
throws InterruptedException {
ChangeLogTokenResponse tokenResponse =
TestUtils.getChangeLogToken(
- new ChangeLogTokenRequest.Builder()
+ TestUtils.getChangeLogTokenRequestForTestRecordTypes()
.addDataOriginFilter(
new DataOrigin.Builder().setPackageName("random").build())
.build());
@@ -237,7 +243,7 @@
Context context = ApplicationProvider.getApplicationContext();
ChangeLogTokenResponse tokenResponse =
TestUtils.getChangeLogToken(
- new ChangeLogTokenRequest.Builder()
+ TestUtils.getChangeLogTokenRequestForTestRecordTypes()
.addDataOriginFilter(
new DataOrigin.Builder()
.setPackageName(context.getPackageName())
@@ -295,7 +301,8 @@
@Test
public void testChangeLogs_insert_default_withPageSize() throws InterruptedException {
ChangeLogTokenResponse tokenResponse =
- TestUtils.getChangeLogToken(new ChangeLogTokenRequest.Builder().build());
+ TestUtils.getChangeLogToken(
+ TestUtils.getChangeLogTokenRequestForTestRecordTypes().build());
ChangeLogsRequest changeLogsRequest =
new ChangeLogsRequest.Builder(tokenResponse.getToken()).setPageSize(1).build();
ChangeLogsResponse response = TestUtils.getChangeLogs(changeLogsRequest);
@@ -310,7 +317,8 @@
@Test
public void testChangeLogs_insert_default_withNextPageToken() throws InterruptedException {
ChangeLogTokenResponse tokenResponse =
- TestUtils.getChangeLogToken(new ChangeLogTokenRequest.Builder().build());
+ TestUtils.getChangeLogToken(
+ TestUtils.getChangeLogTokenRequestForTestRecordTypes().build());
ChangeLogsRequest changeLogsRequest =
new ChangeLogsRequest.Builder(tokenResponse.getToken()).setPageSize(1).build();
ChangeLogsResponse response = TestUtils.getChangeLogs(changeLogsRequest);
@@ -340,7 +348,8 @@
@Test
public void testChangeLogs_insert_default_withSamePageToken() throws InterruptedException {
ChangeLogTokenResponse tokenResponse =
- TestUtils.getChangeLogToken(new ChangeLogTokenRequest.Builder().build());
+ TestUtils.getChangeLogToken(
+ TestUtils.getChangeLogTokenRequestForTestRecordTypes().build());
ChangeLogsRequest changeLogsRequest =
new ChangeLogsRequest.Builder(tokenResponse.getToken()).build();
ChangeLogsResponse response = TestUtils.getChangeLogs(changeLogsRequest);
@@ -358,7 +367,8 @@
@Test
public void testChangeLogs_checkToken_hasMorePages_False() throws InterruptedException {
ChangeLogTokenResponse tokenResponse =
- TestUtils.getChangeLogToken(new ChangeLogTokenRequest.Builder().build());
+ TestUtils.getChangeLogToken(
+ TestUtils.getChangeLogTokenRequestForTestRecordTypes().build());
ChangeLogsRequest changeLogsRequest =
new ChangeLogsRequest.Builder(tokenResponse.getToken()).build();
ChangeLogsResponse response = TestUtils.getChangeLogs(changeLogsRequest);
diff --git a/tests/cts/src/android/healthconnect/cts/HealthConnectManagerTest.java b/tests/cts/src/android/healthconnect/cts/HealthConnectManagerTest.java
index c76dd1e..ca8d225 100644
--- a/tests/cts/src/android/healthconnect/cts/HealthConnectManagerTest.java
+++ b/tests/cts/src/android/healthconnect/cts/HealthConnectManagerTest.java
@@ -1560,7 +1560,8 @@
}
try {
- TestUtils.getChangeLogToken(new ChangeLogTokenRequest.Builder().build());
+ TestUtils.getChangeLogToken(
+ new ChangeLogTokenRequest.Builder().addRecordType(StepsRecord.class).build());
Assert.fail();
} catch (HealthConnectException exception) {
assertThat(exception).isNotNull();
diff --git a/tests/cts/src/android/healthconnect/cts/TestUtils.java b/tests/cts/src/android/healthconnect/cts/TestUtils.java
index 8f3ec4c..f89e81e 100644
--- a/tests/cts/src/android/healthconnect/cts/TestUtils.java
+++ b/tests/cts/src/android/healthconnect/cts/TestUtils.java
@@ -301,6 +301,14 @@
buildExerciseSession());
}
+ public static ChangeLogTokenRequest.Builder getChangeLogTokenRequestForTestRecordTypes() {
+ return new ChangeLogTokenRequest.Builder()
+ .addRecordType(StepsRecord.class)
+ .addRecordType(HeartRateRecord.class)
+ .addRecordType(BasalMetabolicRateRecord.class)
+ .addRecordType(ExerciseSessionRecord.class);
+ }
+
public static List<RecordAndIdentifier> getRecordsAndIdentifiers() {
return Arrays.asList(
new RecordAndIdentifier(RECORD_TYPE_STEPS, getStepsRecord()),
