[RESTRICT AUTOMERGE] Send metrics config removal request to DataBroker
Currently config removal is only propagated to ResultStore and
MetricsConfigStore. This CL will remove config from DataBroker.
This CL also deletes the async return value of the removeMetricsConfig
API to make remove and removeAll APIs consistent.
Bug: 199540952
Test: atest CarServiceUnitTest:CarTelemetryServiceTest
Test: atest CarServiceUnitTest:DataBrokerTest
Test: atest CarServiceUnitTest:MetricsConfigStoreTest
Test: atest CarServiceUnitTest:ResultStoreTest
Test: atest CarSecurityPermissionTest:CarTelemetryManagerPermissionTest
Change-Id: I949d9571588a44455c04c19c5b1b2524a92b6663
diff --git a/car-lib/src/android/car/telemetry/CarTelemetryManager.java b/car-lib/src/android/car/telemetry/CarTelemetryManager.java
index b0067b1..5d2b4cc 100644
--- a/car-lib/src/android/car/telemetry/CarTelemetryManager.java
+++ b/car-lib/src/android/car/telemetry/CarTelemetryManager.java
@@ -137,14 +137,6 @@
*/
void onAddMetricsConfigStatus(@NonNull MetricsConfigKey key,
@MetricsConfigError int statusCode);
-
- /**
- * Sends the {@link #removeMetricsConfig(MetricsConfigKey)} status to the client.
- *
- * @param key the {@link MetricsConfigKey} that the status is associated with
- * @param success true for successful removal, false otherwise.
- */
- void onRemoveMetricsConfigStatus(@NonNull MetricsConfigKey key, boolean success);
}
/**
@@ -186,15 +178,6 @@
}
manager.onAddMetricsConfigStatus(key, statusCode);
}
-
- @Override
- public void onRemoveMetricsConfigStatus(@NonNull MetricsConfigKey key, boolean success) {
- CarTelemetryManager manager = mManager.get();
- if (manager == null) {
- return;
- }
- manager.onRemoveMetricsConfigStatus(key, success);
- }
}
private void onResult(MetricsConfigKey key, byte[] result) {
@@ -224,15 +207,6 @@
Binder.restoreCallingIdentity(token);
}
- private void onRemoveMetricsConfigStatus(MetricsConfigKey key, boolean success) {
- long token = Binder.clearCallingIdentity();
- synchronized (mLock) {
- // TODO(b/198824696): listener should be nonnull
- mExecutor.execute(() -> mResultsListener.onRemoveMetricsConfigStatus(key, success));
- }
- Binder.restoreCallingIdentity(token);
- }
-
/**
* Gets an instance of CarTelemetryManager.
*
@@ -336,7 +310,6 @@
* Removes a MetricsConfig from {@link com.android.car.telemetry.CarTelemetryService}. This
* will also remove outputs produced by the MetricsConfig. If the MetricsConfig does not exist,
* nothing will be removed.
- * The status of this API is sent back asynchronously via {@link CarTelemetryResultsListener}.
*
* @param key the unique key to identify the MetricsConfig. Name and version must be exact.
* @return true for success, false otherwise.
diff --git a/car-lib/src/android/car/telemetry/ICarTelemetryServiceListener.aidl b/car-lib/src/android/car/telemetry/ICarTelemetryServiceListener.aidl
index 4bd61fd..f77558c 100644
--- a/car-lib/src/android/car/telemetry/ICarTelemetryServiceListener.aidl
+++ b/car-lib/src/android/car/telemetry/ICarTelemetryServiceListener.aidl
@@ -51,12 +51,4 @@
* @param statusCode indicating add status.
*/
void onAddMetricsConfigStatus(in MetricsConfigKey key, in int statusCode);
-
- /**
- * Sends the {@link #remove(MetricsConfigKey)} status to the client.
- *
- * @param key the {@link MetricsConfigKey} that the status is associated with.
- * @param success true for successful removal, false otherwise.
- */
- void onRemoveMetricsConfigStatus(in MetricsConfigKey key, in boolean success);
}
\ No newline at end of file
diff --git a/service/src/com/android/car/telemetry/CarTelemetryService.java b/service/src/com/android/car/telemetry/CarTelemetryService.java
index f48d8d8..6f5ca1a 100644
--- a/service/src/com/android/car/telemetry/CarTelemetryService.java
+++ b/service/src/com/android/car/telemetry/CarTelemetryService.java
@@ -182,7 +182,7 @@
// If no error (a config is successfully added), script results from an older version
// should be deleted
if (status == ERROR_METRICS_CONFIG_NONE) {
- mResultStore.deleteResult(key.getName());
+ mResultStore.removeResult(key.getName());
}
try {
mListener.onAddMetricsConfigStatus(key, status);
@@ -194,29 +194,21 @@
/**
* Removes a metrics config based on the key. This will also remove outputs produced by the
- * MetricsConfig. This method assumes {@link #setListener(ICarTelemetryServiceListener)} is
- * called. Otherwise it does nothing.
+ * MetricsConfig.
*
* @param key the unique identifier of a MetricsConfig.
*/
@Override
public void removeMetricsConfig(@NonNull MetricsConfigKey key) {
+ mContext.enforceCallingOrSelfPermission(
+ Car.PERMISSION_USE_CAR_TELEMETRY_SERVICE, "removeMetricsConfig");
mTelemetryHandler.post(() -> {
- if (mListener == null) {
- Slog.w(CarLog.TAG_TELEMETRY, "ICarTelemetryServiceListener is not set");
- return;
- }
Slog.d(CarLog.TAG_TELEMETRY, "Removing metrics config " + key.getName()
+ " from car telemetry service");
- // TODO(b/198792767): Check both config name and config version for deletion
- // TODO(b/199540952): Stop and remove config from data broker
- mResultStore.deleteResult(key.getName()); // delete the config's script results
- boolean success = mMetricsConfigStore.deleteMetricsConfig(key.getName());
- try {
- mListener.onRemoveMetricsConfigStatus(key, success);
- } catch (RemoteException e) {
- Slog.w(CarLog.TAG_TELEMETRY, "error with ICarTelemetryServiceListener", e);
- }
+ // TODO(b/198792767): Check both config name and config version for removal
+ mDataBroker.removeMetricsConfiguration(key.getName());
+ mResultStore.removeResult(key.getName());
+ mMetricsConfigStore.removeMetricsConfig(key.getName());
});
}
@@ -226,13 +218,13 @@
@Override
public void removeAllMetricsConfigs() {
mContext.enforceCallingOrSelfPermission(
- Car.PERMISSION_USE_CAR_TELEMETRY_SERVICE, "removeAllMetricsConfig");
+ Car.PERMISSION_USE_CAR_TELEMETRY_SERVICE, "removeAllMetricsConfigs");
mTelemetryHandler.post(() -> {
- // TODO(b/199540952): Stop and remove all configs from DataBroker
Slog.d(CarLog.TAG_TELEMETRY,
"Removing all metrics config from car telemetry service");
- mMetricsConfigStore.deleteAllMetricsConfigs();
- mResultStore.deleteAllResults();
+ mDataBroker.removeAllMetricsConfigurations();
+ mMetricsConfigStore.removeAllMetricsConfigs();
+ mResultStore.removeAllResults();
});
}
diff --git a/service/src/com/android/car/telemetry/MetricsConfigStore.java b/service/src/com/android/car/telemetry/MetricsConfigStore.java
index ff17d0e..2d259d4 100644
--- a/service/src/com/android/car/telemetry/MetricsConfigStore.java
+++ b/service/src/com/android/car/telemetry/MetricsConfigStore.java
@@ -104,17 +104,21 @@
}
/** Deletes the MetricsConfig from disk. Returns the success status. */
- public boolean deleteMetricsConfig(String metricsConfigName) {
+ public void removeMetricsConfig(String metricsConfigName) {
mActiveConfigs.remove(metricsConfigName);
mNameVersionMap.remove(metricsConfigName);
- return new File(mConfigDirectory, metricsConfigName).delete();
+ if (!new File(mConfigDirectory, metricsConfigName).delete()) {
+ Slog.w(CarLog.TAG_TELEMETRY, "Failed to remove MetricsConfig: " + metricsConfigName);
+ }
}
/** Deletes all MetricsConfigs from disk. */
- public void deleteAllMetricsConfigs() {
+ public void removeAllMetricsConfigs() {
mActiveConfigs.clear();
for (File file : mConfigDirectory.listFiles()) {
- file.delete();
+ if (!file.delete()) {
+ Slog.w(CarLog.TAG_TELEMETRY, "Failed to remove MetricsConfig: " + file.getName());
+ }
}
}
}
diff --git a/service/src/com/android/car/telemetry/ResultStore.java b/service/src/com/android/car/telemetry/ResultStore.java
index 9c8b2fe..85aa23e 100644
--- a/service/src/com/android/car/telemetry/ResultStore.java
+++ b/service/src/com/android/car/telemetry/ResultStore.java
@@ -179,14 +179,14 @@
* Deletes script result associated with the given config name. If result does not exist, this
* method does not do anything.
*/
- public void deleteResult(String metricsConfigName) {
+ public void removeResult(String metricsConfigName) {
mInterimResultCache.remove(metricsConfigName);
deleteFileInDirectory(mInterimResultDirectory, metricsConfigName);
deleteFileInDirectory(mFinalResultDirectory, metricsConfigName);
}
/** Deletes all interim and final results stored in disk. */
- public void deleteAllResults() {
+ public void removeAllResults() {
mInterimResultCache.clear();
for (File interimResult : mInterimResultDirectory.listFiles()) {
interimResult.delete();
diff --git a/service/src/com/android/car/telemetry/databroker/DataBroker.java b/service/src/com/android/car/telemetry/databroker/DataBroker.java
index d7cc2e6..2b7fa01 100644
--- a/service/src/com/android/car/telemetry/databroker/DataBroker.java
+++ b/service/src/com/android/car/telemetry/databroker/DataBroker.java
@@ -36,8 +36,7 @@
/**
* Adds an active {@link com.android.car.telemetry.TelemetryProto.MetricsConfig} that is pending
* execution. When updating the MetricsConfig to a newer version, the caller must call
- * {@link #removeMetricsConfiguration(TelemetryProto.MetricsConfig)} first to clear the old
- * MetricsConfig.
+ * {@link #removeMetricsConfiguration(String)} first to clear the old MetricsConfig.
* TODO(b/191378559): Define behavior when metricsConfig contains invalid config
*
* @param metricsConfig to be added and queued for execution.
@@ -48,9 +47,15 @@
* Removes a {@link com.android.car.telemetry.TelemetryProto.MetricsConfig} and all its
* relevant subscriptions.
*
- * @param metricsConfig to be removed from DataBroker.
+ * @param metricsConfigName name of the MetricsConfig to be removed.
*/
- void removeMetricsConfiguration(TelemetryProto.MetricsConfig metricsConfig);
+ void removeMetricsConfiguration(String metricsConfigName);
+
+ /**
+ * Removes all {@link com.android.car.telemetry.TelemetryProto.MetricsConfig}s and
+ * subscriptions.
+ */
+ void removeAllMetricsConfigurations();
/**
* Adds a {@link ScriptExecutionTask} to the priority queue. This method will schedule the
diff --git a/service/src/com/android/car/telemetry/databroker/DataBrokerImpl.java b/service/src/com/android/car/telemetry/databroker/DataBrokerImpl.java
index 695254c..b53899b 100644
--- a/service/src/com/android/car/telemetry/databroker/DataBrokerImpl.java
+++ b/service/src/com/android/car/telemetry/databroker/DataBrokerImpl.java
@@ -56,7 +56,7 @@
/**
* Implementation of the data path component of CarTelemetryService. Forwards the published data
* from publishers to consumers subject to the Controller's decision.
- * TODO(b/187743369): Handle thread-safety of member variables.
+ * All methods should be called from the telemetry thread unless otherwise specified as thread-safe.
*/
public class DataBrokerImpl implements DataBroker {
@@ -208,7 +208,7 @@
// get the metrics config from the DataSubscriber and remove the metrics config
if (mSubscriptionMap.get(metricsConfigName).size() != 0) {
removeMetricsConfiguration(mSubscriptionMap.get(metricsConfigName).get(0)
- .getMetricsConfig());
+ .getMetricsConfig().getName());
}
}
mSubscriptionMap.clear();
@@ -248,13 +248,13 @@
}
@Override
- public void removeMetricsConfiguration(MetricsConfig metricsConfig) {
+ public void removeMetricsConfiguration(String metricsConfigName) {
// TODO(b/187743369): pass status back to caller
- if (!mSubscriptionMap.containsKey(metricsConfig.getName())) {
+ if (!mSubscriptionMap.containsKey(metricsConfigName)) {
return;
}
// get the subscriptions associated with this MetricsConfig, remove it from the map
- List<DataSubscriber> dataSubscribers = mSubscriptionMap.remove(metricsConfig.getName());
+ List<DataSubscriber> dataSubscribers = mSubscriptionMap.remove(metricsConfigName);
// for each subscriber, remove it from publishers
for (DataSubscriber subscriber : dataSubscribers) {
AbstractPublisher publisher = mPublisherFactory.getPublisher(
@@ -271,7 +271,14 @@
// iterating, so it may or may not reflect any updates since the iterator was created.
// But since adding & polling from queue should happen in the same thread, the task queue
// should not be changed while tasks are being iterated and removed.
- mTaskQueue.removeIf(task -> task.isAssociatedWithMetricsConfig(metricsConfig));
+ mTaskQueue.removeIf(task -> task.isAssociatedWithMetricsConfig(metricsConfigName));
+ }
+
+ @Override
+ public void removeAllMetricsConfigurations() {
+ mPublisherFactory.removeAllDataSubscribers();
+ mSubscriptionMap.clear();
+ mTaskQueue.clear();
}
@Override
diff --git a/service/src/com/android/car/telemetry/databroker/ScriptExecutionTask.java b/service/src/com/android/car/telemetry/databroker/ScriptExecutionTask.java
index 648fc79..afe050a 100644
--- a/service/src/com/android/car/telemetry/databroker/ScriptExecutionTask.java
+++ b/service/src/com/android/car/telemetry/databroker/ScriptExecutionTask.java
@@ -63,11 +63,10 @@
}
/**
- * Indicates whether the task is associated with the given
- * {@link com.android.car.telemetry.TelemetryProto.MetricsConfig).
+ * Indicates whether the task is associated with MetricsConfig specified by the name.
*/
- public boolean isAssociatedWithMetricsConfig(TelemetryProto.MetricsConfig metricsConfig) {
- return mSubscriber.getMetricsConfig().equals(metricsConfig);
+ public boolean isAssociatedWithMetricsConfig(String metricsConfigName) {
+ return mSubscriber.getMetricsConfig().getName().equals(metricsConfigName);
}
/**
diff --git a/service/src/com/android/car/telemetry/publisher/PublisherFactory.java b/service/src/com/android/car/telemetry/publisher/PublisherFactory.java
index 2e08191..d777847 100644
--- a/service/src/com/android/car/telemetry/publisher/PublisherFactory.java
+++ b/service/src/com/android/car/telemetry/publisher/PublisherFactory.java
@@ -31,7 +31,7 @@
* <p>It doesn't instantiate all the publishers right away, as in some cases some publishers are
* not needed.
*
- * <p>Thread-safe.
+ * <p>Methods in this class must be called on telemetry thread unless specified as thread-safe.
*/
public class PublisherFactory {
private final Object mLock = new Object();
@@ -56,7 +56,7 @@
mRootDirectory = rootDirectory;
}
- /** Returns the publisher by given type. */
+ /** Returns the publisher by given type. This method is thread-safe. */
public AbstractPublisher getPublisher(TelemetryProto.Publisher.PublisherCase type) {
// No need to optimize locks, as this method is infrequently called.
synchronized (mLock) {
@@ -87,6 +87,21 @@
}
/**
+ * Removes all {@link com.android.car.telemetry.databroker.DataSubscriber} from all publishers.
+ */
+ public void removeAllDataSubscribers() {
+ if (mVehiclePropertyPublisher != null) {
+ mVehiclePropertyPublisher.removeAllDataSubscribers();
+ }
+ if (mCarTelemetrydPublisher != null) {
+ mCarTelemetrydPublisher.removeAllDataSubscribers();
+ }
+ if (mStatsPublisher != null) {
+ mStatsPublisher.removeAllDataSubscribers();
+ }
+ }
+
+ /**
* Sets the publisher failure consumer for all the publishers. This is expected to be called
* before {@link #getPublisher} method. This is not the best approach, but it suits for this
* case.
diff --git a/tests/CarSecurityPermissionTest/src/com/android/car/telemetry/CarTelemetryManagerPermissionTest.java b/tests/CarSecurityPermissionTest/src/com/android/car/telemetry/CarTelemetryManagerPermissionTest.java
index 84409cf..de8b8d1 100644
--- a/tests/CarSecurityPermissionTest/src/com/android/car/telemetry/CarTelemetryManagerPermissionTest.java
+++ b/tests/CarSecurityPermissionTest/src/com/android/car/telemetry/CarTelemetryManagerPermissionTest.java
@@ -92,7 +92,7 @@
}
@Test
- public void testRemoveMetricsConfig() throws Exception {
+ public void testRemoveMetricsConfig() {
Exception e = expectThrows(SecurityException.class,
() -> mCarTelemetryManager.removeMetricsConfig(mMetricsConfigKey));
@@ -136,9 +136,5 @@
@Override
public void onAddMetricsConfigStatus(@NonNull MetricsConfigKey key, int statusCode) {
}
-
- @Override
- public void onRemoveMetricsConfigStatus(@NonNull MetricsConfigKey key, boolean success) {
- }
}
}
diff --git a/tests/EmbeddedKitchenSinkApp/src/com/google/android/car/kitchensink/telemetry/CarTelemetryTestFragment.java b/tests/EmbeddedKitchenSinkApp/src/com/google/android/car/kitchensink/telemetry/CarTelemetryTestFragment.java
index d2f011f..13392b4 100644
--- a/tests/EmbeddedKitchenSinkApp/src/com/google/android/car/kitchensink/telemetry/CarTelemetryTestFragment.java
+++ b/tests/EmbeddedKitchenSinkApp/src/com/google/android/car/kitchensink/telemetry/CarTelemetryTestFragment.java
@@ -161,10 +161,5 @@
public void onAddMetricsConfigStatus(@NonNull MetricsConfigKey key, int statusCode) {
showOutput("Add MetricsConfig status: " + statusCode);
}
-
- @Override
- public void onRemoveMetricsConfigStatus(@NonNull MetricsConfigKey key, boolean success) {
- showOutput("Remove MetricsConfig status: " + success);
- }
}
}
diff --git a/tests/carservice_test/src/com/android/car/CarTelemetryManagerTest.java b/tests/carservice_test/src/com/android/car/CarTelemetryManagerTest.java
index 88302de..11181de 100644
--- a/tests/carservice_test/src/com/android/car/CarTelemetryManagerTest.java
+++ b/tests/carservice_test/src/com/android/car/CarTelemetryManagerTest.java
@@ -131,19 +131,6 @@
ERROR_METRICS_CONFIG_VERSION_TOO_OLD);
}
- @Test
- public void testRemoveMetricsConfig() throws Exception {
- mCarTelemetryManager.removeMetricsConfig(KEY_V1);
- waitForHandlerThreadToFinish();
- assertThat(mListener.getRemoveConfigStatus(KEY_V1)).isFalse();
-
- mCarTelemetryManager.addMetricsConfig(KEY_V1, METRICS_CONFIG_V1.toByteArray());
- waitForHandlerThreadToFinish();
- mCarTelemetryManager.removeMetricsConfig(KEY_V1);
- waitForHandlerThreadToFinish();
- assertThat(mListener.getRemoveConfigStatus(KEY_V1)).isTrue();
- }
-
private void waitForHandlerThreadToFinish() throws Exception {
assertWithMessage("handler not idle in %sms", TIMEOUT_MS)
.that(mIdleHandlerLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)).isTrue();
@@ -156,7 +143,6 @@
private static final class FakeCarTelemetryResultsListener
implements CarTelemetryManager.CarTelemetryResultsListener {
- private Map<MetricsConfigKey, Boolean> mRemoveConfigStatusMap = new ArrayMap<>();
private Map<MetricsConfigKey, Integer> mAddConfigStatusMap = new ArrayMap<>();
@Override
@@ -172,17 +158,8 @@
mAddConfigStatusMap.put(key, statusCode);
}
- @Override
- public void onRemoveMetricsConfigStatus(@NonNull MetricsConfigKey key, boolean success) {
- mRemoveConfigStatusMap.put(key, success);
- }
-
public int getAddConfigStatus(MetricsConfigKey key) {
return mAddConfigStatusMap.getOrDefault(key, -100);
}
-
- public boolean getRemoveConfigStatus(MetricsConfigKey key) {
- return mRemoveConfigStatusMap.getOrDefault(key, false);
- }
}
}
diff --git a/tests/carservice_unit_test/src/com/android/car/telemetry/CarTelemetryServiceTest.java b/tests/carservice_unit_test/src/com/android/car/telemetry/CarTelemetryServiceTest.java
index 6b4254a..b177655 100644
--- a/tests/carservice_unit_test/src/com/android/car/telemetry/CarTelemetryServiceTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/telemetry/CarTelemetryServiceTest.java
@@ -175,27 +175,18 @@
}
@Test
- public void testRemoveMetricsConfig_configExists_shouldDeleteScriptResult() throws Exception {
+ public void testRemoveMetricsConfig_shouldDeleteConfigAndResult() throws Exception {
mService.addMetricsConfig(KEY_V1, METRICS_CONFIG_V1.toByteArray());
mResultStore.putInterimResult(KEY_V1.getName(), new PersistableBundle());
mService.removeMetricsConfig(KEY_V1);
waitForHandlerThreadToFinish();
- verify(mMockListener).onRemoveMetricsConfigStatus(eq(KEY_V1), eq(true));
assertThat(mMetricsConfigStore.getActiveMetricsConfigs()).isEmpty();
assertThat(mResultStore.getInterimResult(KEY_V1.getName())).isNull();
}
@Test
- public void testRemoveMetricsConfig_configDoesNotExist_shouldFail() throws Exception {
- mService.removeMetricsConfig(KEY_V1);
-
- waitForHandlerThreadToFinish();
- verify(mMockListener).onRemoveMetricsConfigStatus(eq(KEY_V1), eq(false));
- }
-
- @Test
public void testRemoveAllMetricsConfigs_shouldRemoveConfigsAndResults() throws Exception {
MetricsConfigKey key = new MetricsConfigKey("test config", 2);
TelemetryProto.MetricsConfig config =
diff --git a/tests/carservice_unit_test/src/com/android/car/telemetry/MetricsConfigStoreTest.java b/tests/carservice_unit_test/src/com/android/car/telemetry/MetricsConfigStoreTest.java
index daf83d8..d138247 100644
--- a/tests/carservice_unit_test/src/com/android/car/telemetry/MetricsConfigStoreTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/telemetry/MetricsConfigStoreTest.java
@@ -71,28 +71,20 @@
}
@Test
- public void testDeleteMetricsConfig_whenNoConfig_shouldReturnFalse() {
- boolean status = mMetricsConfigStore.deleteMetricsConfig(NAME_BAR);
-
- assertThat(status).isFalse();
- }
-
- @Test
- public void testDeleteMetricsConfig_shouldDeleteConfigFromDisk() throws Exception {
+ public void testRemoveMetricsConfig_shouldDeleteConfigFromDisk() throws Exception {
writeConfigToDisk(METRICS_CONFIG_BAR);
- boolean status = mMetricsConfigStore.deleteMetricsConfig(NAME_BAR);
+ mMetricsConfigStore.removeMetricsConfig(NAME_BAR);
- assertThat(status).isTrue();
assertThat(new File(mTestMetricsConfigDir, NAME_BAR).exists()).isFalse();
}
@Test
- public void testDeleteAllMetricsConfigs_shouldDeleteAll() throws Exception {
+ public void testRemoveAllMetricsConfigs_shouldDeleteAll() throws Exception {
writeConfigToDisk(METRICS_CONFIG_FOO);
writeConfigToDisk(METRICS_CONFIG_BAR);
- mMetricsConfigStore.deleteAllMetricsConfigs();
+ mMetricsConfigStore.removeAllMetricsConfigs();
assertThat(mTestMetricsConfigDir.listFiles()).isEmpty();
}
diff --git a/tests/carservice_unit_test/src/com/android/car/telemetry/ResultStoreTest.java b/tests/carservice_unit_test/src/com/android/car/telemetry/ResultStoreTest.java
index 6eb8fd6..942c3a8 100644
--- a/tests/carservice_unit_test/src/com/android/car/telemetry/ResultStoreTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/telemetry/ResultStoreTest.java
@@ -79,25 +79,6 @@
}
@Test
- public void testFlushToDisk_shouldWriteResultsToFileAndCheckContent() throws Exception {
- String testInterimFileName = "test_file_1";
- String testFinalFileName = "test_file_2";
- writeBundleToFile(mTestInterimResultDir, testInterimFileName, TEST_INTERIM_BUNDLE);
- writeBundleToFile(mTestFinalResultDir, testFinalFileName, TEST_FINAL_BUNDLE);
-
- mResultStore.flushToDisk();
-
- assertThat(new File(mTestInterimResultDir, testInterimFileName).exists()).isTrue();
- assertThat(new File(mTestFinalResultDir, testFinalFileName).exists()).isTrue();
- // the content check will need to be modified when data encryption is implemented
- PersistableBundle interimData =
- readBundleFromFile(mTestInterimResultDir, testInterimFileName);
- assertThat(interimData.toString()).isEqualTo(TEST_INTERIM_BUNDLE.toString());
- PersistableBundle finalData = readBundleFromFile(mTestFinalResultDir, testFinalFileName);
- assertThat(finalData.toString()).isEqualTo(TEST_FINAL_BUNDLE.toString());
- }
-
- @Test
public void testFlushToDisk_shouldRemoveStaleData() throws Exception {
File staleTestFile1 = new File(mTestInterimResultDir, "stale_test_file_1");
File staleTestFile2 = new File(mTestFinalResultDir, "stale_test_file_2");
@@ -119,6 +100,42 @@
assertThat(activeTestFile3.exists()).isTrue();
}
+
+ @Test
+ public void testPutInterimResultAndFlushToDisk_shouldReplaceExistingFile() throws Exception {
+ String newKey = "new key";
+ String newValue = "new value";
+ String metricsConfigName = "my_metrics_config";
+ writeBundleToFile(mTestInterimResultDir, metricsConfigName, TEST_INTERIM_BUNDLE);
+ TEST_INTERIM_BUNDLE.putString(newKey, newValue);
+
+ mResultStore.putInterimResult(metricsConfigName, TEST_INTERIM_BUNDLE);
+ mResultStore.flushToDisk();
+
+ PersistableBundle bundle = readBundleFromFile(mTestInterimResultDir, metricsConfigName);
+ assertThat(bundle.getString(newKey)).isEqualTo(newValue);
+ assertThat(bundle.toString()).isEqualTo(TEST_INTERIM_BUNDLE.toString());
+ }
+
+ @Test
+ public void testPutInterimResultAndFlushToDisk_shouldWriteDirtyResultsOnly() throws Exception {
+ File fileFoo = new File(mTestInterimResultDir, "foo");
+ File fileBar = new File(mTestInterimResultDir, "bar");
+ writeBundleToFile(fileFoo, TEST_INTERIM_BUNDLE);
+ writeBundleToFile(fileBar, TEST_INTERIM_BUNDLE);
+ mResultStore = new ResultStore(mTestRootDir); // re-load data
+ PersistableBundle newData = new PersistableBundle();
+ newData.putDouble("pi", 3.1415926);
+
+ mResultStore.putInterimResult("bar", newData); // make bar dirty
+ fileFoo.delete(); // delete the clean file from the file system
+ mResultStore.flushToDisk(); // write dirty data
+
+ // foo is a clean file that should not be written in flushToDisk()
+ assertThat(fileFoo.exists()).isFalse();
+ assertThat(readBundleFromFile(fileBar).toString()).isEqualTo(newData.toString());
+ }
+
@Test
public void testGetFinalResult_whenNoData_shouldReceiveNull() throws Exception {
String metricsConfigName = "my_metrics_config";
@@ -186,42 +203,7 @@
}
@Test
- public void testPutInterimResultAndFlushToDisk_shouldReplaceExistingFile() throws Exception {
- String newKey = "new key";
- String newValue = "new value";
- String metricsConfigName = "my_metrics_config";
- writeBundleToFile(mTestInterimResultDir, metricsConfigName, TEST_INTERIM_BUNDLE);
- TEST_INTERIM_BUNDLE.putString(newKey, newValue);
-
- mResultStore.putInterimResult(metricsConfigName, TEST_INTERIM_BUNDLE);
- mResultStore.flushToDisk();
-
- PersistableBundle bundle = readBundleFromFile(mTestInterimResultDir, metricsConfigName);
- assertThat(bundle.getString(newKey)).isEqualTo(newValue);
- assertThat(bundle.toString()).isEqualTo(TEST_INTERIM_BUNDLE.toString());
- }
-
- @Test
- public void testPutInterimResultAndFlushToDisk_shouldWriteDirtyResultsOnly() throws Exception {
- File fileFoo = new File(mTestInterimResultDir, "foo");
- File fileBar = new File(mTestInterimResultDir, "bar");
- writeBundleToFile(fileFoo, TEST_INTERIM_BUNDLE);
- writeBundleToFile(fileBar, TEST_INTERIM_BUNDLE);
- mResultStore = new ResultStore(mTestRootDir); // re-load data
- PersistableBundle newData = new PersistableBundle();
- newData.putDouble("pi", 3.1415926);
-
- mResultStore.putInterimResult("bar", newData); // make bar dirty
- fileFoo.delete(); // delete the clean file from the file system
- mResultStore.flushToDisk(); // write dirty data
-
- // foo is a clean file that should not be written in shutdown
- assertThat(fileFoo.exists()).isFalse();
- assertThat(readBundleFromFile(fileBar).toString()).isEqualTo(newData.toString());
- }
-
- @Test
- public void testPutFinalResult_shouldWriteResultAndRemoveInterimq() throws Exception {
+ public void testPutFinalResult_shouldWriteResultAndRemoveInterim() throws Exception {
String metricsConfigName = "my_metrics_config";
writeBundleToFile(mTestInterimResultDir, metricsConfigName, TEST_INTERIM_BUNDLE);
@@ -244,25 +226,38 @@
}
@Test
- public void testDeleteResult_whenInterimResult_shouldDelete() throws Exception {
+ public void testRemoveResult_whenInterimResult_shouldDelete() throws Exception {
String metricsConfigName = "my_metrics_config";
writeBundleToFile(mTestInterimResultDir, metricsConfigName, TEST_INTERIM_BUNDLE);
- mResultStore.deleteResult(metricsConfigName);
+ mResultStore.removeResult(metricsConfigName);
assertThat(new File(mTestInterimResultDir, metricsConfigName).exists()).isFalse();
}
@Test
- public void testDeleteResult_whenFinalResult_shouldDelete() throws Exception {
+ public void testRemoveResult_whenFinalResult_shouldDelete() throws Exception {
String metricsConfigName = "my_metrics_config";
writeBundleToFile(mTestFinalResultDir, metricsConfigName, TEST_FINAL_BUNDLE);
- mResultStore.deleteResult(metricsConfigName);
+ mResultStore.removeResult(metricsConfigName);
assertThat(new File(mTestFinalResultDir, metricsConfigName).exists()).isFalse();
}
+ @Test
+ public void testRemoveAllResults_shouldDeleteAll() throws Exception {
+ mResultStore.putInterimResult("config 1", TEST_INTERIM_BUNDLE);
+ mResultStore.putFinalResult("config 2", TEST_FINAL_BUNDLE);
+ mResultStore.putError("config 3", TEST_TELEMETRY_ERROR);
+ mResultStore.flushToDisk();
+
+ mResultStore.removeAllResults();
+
+ assertThat(mTestInterimResultDir.listFiles()).isEmpty();
+ assertThat(mTestFinalResultDir.listFiles()).isEmpty();
+ }
+
private void writeBundleToFile(
File dir, String fileName, PersistableBundle persistableBundle) throws Exception {
writeBundleToFile(new File(dir, fileName), persistableBundle);
diff --git a/tests/carservice_unit_test/src/com/android/car/telemetry/databroker/DataBrokerTest.java b/tests/carservice_unit_test/src/com/android/car/telemetry/databroker/DataBrokerTest.java
index 0f37187..3e71470 100644
--- a/tests/carservice_unit_test/src/com/android/car/telemetry/databroker/DataBrokerTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/telemetry/databroker/DataBrokerTest.java
@@ -446,7 +446,7 @@
taskQueue.add(taskWithMetricsConfigFoo); // associated with METRICS_CONFIG_FOO
assertThat(taskQueue).hasSize(3);
- mDataBroker.removeMetricsConfiguration(METRICS_CONFIG_FOO);
+ mDataBroker.removeMetricsConfiguration(METRICS_CONFIG_FOO.getName());
assertThat(taskQueue).hasSize(1);
assertThat(taskQueue.poll()).isEqualTo(mLowPriorityTask);
@@ -454,11 +454,25 @@
@Test
public void testRemoveMetricsConfiguration_whenMetricsConfigNonExistent_shouldDoNothing() {
- mDataBroker.removeMetricsConfiguration(METRICS_CONFIG_BAR);
+ mDataBroker.removeMetricsConfiguration(METRICS_CONFIG_BAR.getName());
assertThat(mDataBroker.getSubscriptionMap()).hasSize(0);
}
+ @Test
+ public void testRemoveAllMetricsConfigurations_shouldRemoveTasksAndClearSubscriptionMap() {
+ mDataBroker.addMetricsConfiguration(METRICS_CONFIG_FOO);
+ mDataBroker.addMetricsConfiguration(METRICS_CONFIG_BAR);
+ PriorityBlockingQueue<ScriptExecutionTask> taskQueue = mDataBroker.getTaskQueue();
+ taskQueue.add(mHighPriorityTask); // associated with METRICS_CONFIG_FOO
+ taskQueue.add(mLowPriorityTask); // associated with METRICS_CONFIG_BAR
+
+ mDataBroker.removeAllMetricsConfigurations();
+
+ assertThat(taskQueue).isEmpty();
+ assertThat(mDataBroker.getSubscriptionMap()).isEmpty();
+ }
+
private void waitForTelemetryThreadToFinish() throws Exception {
assertWithMessage("handler not idle in %sms", TIMEOUT_MS)
.that(mIdleHandlerLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)).isTrue();