[RESTRICT AUTOMERGE] Replace SharedPrefs with flat file persistable bundle
This CL resolves the CarTelemetryService init path, replaces usages of
SharedPreferences with persistable bundle
Fixes: 198310102
Test: atest CarServiceUnitTest:StatsPublisherTest
Change-Id: I56db56abf3e7d69817d6000e4d6c5192ea04f0d9
diff --git a/service/src/com/android/car/telemetry/CarTelemetryService.java b/service/src/com/android/car/telemetry/CarTelemetryService.java
index 537ce45..8e694d0 100644
--- a/service/src/com/android/car/telemetry/CarTelemetryService.java
+++ b/service/src/com/android/car/telemetry/CarTelemetryService.java
@@ -95,9 +95,8 @@
mResultStore = new ResultStore(mRootDirectory);
mStatsManagerProxy = new StatsManagerImpl(
mContext.getSystemService(StatsManager.class));
- // TODO(b/197968695): delay initialization of stats publisher
mPublisherFactory = new PublisherFactory(mCarPropertyService, mTelemetryHandler,
- mStatsManagerProxy, null);
+ mStatsManagerProxy, mRootDirectory);
mDataBroker = new DataBrokerImpl(mContext, mPublisherFactory, mResultStore);
mSystemMonitor = SystemMonitor.create(mContext, mTelemetryHandler);
mDataBrokerController = new DataBrokerController(mDataBroker, mSystemMonitor);
diff --git a/service/src/com/android/car/telemetry/publisher/PublisherFactory.java b/service/src/com/android/car/telemetry/publisher/PublisherFactory.java
index 1009d25..2e08191 100644
--- a/service/src/com/android/car/telemetry/publisher/PublisherFactory.java
+++ b/service/src/com/android/car/telemetry/publisher/PublisherFactory.java
@@ -16,12 +16,12 @@
package com.android.car.telemetry.publisher;
-import android.content.SharedPreferences;
import android.os.Handler;
import com.android.car.CarPropertyService;
import com.android.car.telemetry.TelemetryProto;
+import java.io.File;
import java.util.function.BiConsumer;
/**
@@ -36,9 +36,9 @@
public class PublisherFactory {
private final Object mLock = new Object();
private final CarPropertyService mCarPropertyService;
+ private final File mRootDirectory;
private final Handler mTelemetryHandler;
private final StatsManagerProxy mStatsManager;
- private final SharedPreferences mSharedPreferences;
private VehiclePropertyPublisher mVehiclePropertyPublisher;
private CarTelemetrydPublisher mCarTelemetrydPublisher;
private StatsPublisher mStatsPublisher;
@@ -49,11 +49,11 @@
CarPropertyService carPropertyService,
Handler handler,
StatsManagerProxy statsManager,
- SharedPreferences sharedPreferences) {
+ File rootDirectory) {
mCarPropertyService = carPropertyService;
mTelemetryHandler = handler;
mStatsManager = statsManager;
- mSharedPreferences = sharedPreferences;
+ mRootDirectory = rootDirectory;
}
/** Returns the publisher by given type. */
@@ -76,7 +76,7 @@
case TelemetryProto.Publisher.STATS_FIELD_NUMBER:
if (mStatsPublisher == null) {
mStatsPublisher = new StatsPublisher(
- mFailureConsumer, mStatsManager, mSharedPreferences);
+ mFailureConsumer, mStatsManager, mRootDirectory);
}
return mStatsPublisher;
default:
diff --git a/service/src/com/android/car/telemetry/publisher/StatsPublisher.java b/service/src/com/android/car/telemetry/publisher/StatsPublisher.java
index 8d5f7ea..11e3b09 100644
--- a/service/src/com/android/car/telemetry/publisher/StatsPublisher.java
+++ b/service/src/com/android/car/telemetry/publisher/StatsPublisher.java
@@ -17,7 +17,6 @@
package com.android.car.telemetry.publisher;
import android.app.StatsManager.StatsUnavailableException;
-import android.content.SharedPreferences;
import android.os.Handler;
import android.os.Looper;
import android.os.PersistableBundle;
@@ -37,6 +36,10 @@
import com.google.protobuf.InvalidProtocolBufferException;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
@@ -61,17 +64,20 @@
@VisibleForTesting
static final int ATOM_APP_START_MEMORY_STATE_CAPTURED_ID = 55;
+ // The file that contains stats config key and stats config version
+ @VisibleForTesting
+ static final String SAVED_STATS_CONFIGS_FILE = "stats_config_keys_versions";
+
private static final Duration PULL_REPORTS_PERIOD = Duration.ofMinutes(10);
- private static final String SHARED_PREF_CONFIG_KEY_PREFIX = "statsd-publisher-config-id-";
- private static final String SHARED_PREF_CONFIG_VERSION_PREFIX =
- "statsd-publisher-config-version-";
+ private static final String BUNDLE_CONFIG_KEY_PREFIX = "statsd-publisher-config-id-";
+ private static final String BUNDLE_CONFIG_VERSION_PREFIX = "statsd-publisher-config-version-";
// TODO(b/197766340): remove unnecessary lock
private final Object mLock = new Object();
private final StatsManagerProxy mStatsManager;
- private final SharedPreferences mSharedPreferences;
+ private final File mSavedStatsConfigsFile;
private final Handler mTelemetryHandler;
// True if the publisher is periodically pulling reports from StatsD.
@@ -87,24 +93,51 @@
@GuardedBy("mLock")
private final LongSparseArray<DataSubscriber> mConfigKeyToSubscribers = new LongSparseArray<>();
+ private final PersistableBundle mSavedStatsConfigs;
+
// TODO(b/198331078): Use telemetry thread
StatsPublisher(
BiConsumer<AbstractPublisher, Throwable> failureConsumer,
StatsManagerProxy statsManager,
- SharedPreferences sharedPreferences) {
- this(failureConsumer, statsManager, sharedPreferences, new Handler(Looper.myLooper()));
+ File rootDirectory) {
+ this(failureConsumer, statsManager, rootDirectory, new Handler(Looper.myLooper()));
}
@VisibleForTesting
StatsPublisher(
BiConsumer<AbstractPublisher, Throwable> failureConsumer,
StatsManagerProxy statsManager,
- SharedPreferences sharedPreferences,
+ File rootDirectory,
Handler handler) {
super(failureConsumer);
mStatsManager = statsManager;
- mSharedPreferences = sharedPreferences;
mTelemetryHandler = handler;
+ mSavedStatsConfigsFile = new File(rootDirectory, SAVED_STATS_CONFIGS_FILE);
+ mSavedStatsConfigs = loadBundle();
+ }
+
+ /** Loads the PersistableBundle containing stats config keys and versions from disk. */
+ private PersistableBundle loadBundle() {
+ try (FileInputStream fileInputStream = new FileInputStream(mSavedStatsConfigsFile)) {
+ return PersistableBundle.readFromStream(fileInputStream);
+ } catch (IOException e) {
+ // TODO(b/199947533): handle failure
+ Slog.e(CarLog.TAG_TELEMETRY,
+ "Failed to read file " + mSavedStatsConfigsFile.getAbsolutePath(), e);
+ return new PersistableBundle();
+ }
+ }
+
+ /** Writes the PersistableBundle containing stats config keys and versions to disk. */
+ private void saveBundle() {
+ try (FileOutputStream fileOutputStream = new FileOutputStream(mSavedStatsConfigsFile)) {
+ mSavedStatsConfigs.writeToStream(fileOutputStream);
+ } catch (IOException e) {
+ // TODO(b/199947533): handle failure
+ Slog.e(CarLog.TAG_TELEMETRY,
+ "Cannot write to " + mSavedStatsConfigsFile.getAbsolutePath()
+ + ". Added stats config info is lost.", e);
+ }
}
@Override
@@ -161,12 +194,14 @@
private List<Long> getActiveConfigKeys() {
ArrayList<Long> result = new ArrayList<>();
synchronized (mLock) {
- mSharedPreferences.getAll().forEach((key, value) -> {
- if (!key.startsWith(SHARED_PREF_CONFIG_KEY_PREFIX)) {
- return;
+ for (String key : mSavedStatsConfigs.keySet()) {
+ // filter out all the config versions
+ if (!key.startsWith(BUNDLE_CONFIG_KEY_PREFIX)) {
+ continue;
}
- result.add((long) value);
- });
+ // the remaining values are config keys
+ result.add(mSavedStatsConfigs.getLong(key));
+ }
}
return result;
}
@@ -201,16 +236,18 @@
@Override
public void removeAllDataSubscribers() {
synchronized (mLock) {
- SharedPreferences.Editor editor = mSharedPreferences.edit();
- mSharedPreferences.getAll().forEach((key, value) -> {
- if (!key.startsWith(SHARED_PREF_CONFIG_KEY_PREFIX)) {
- return;
+ for (String key : mSavedStatsConfigs.keySet()) {
+ // filter out all the config versions
+ if (!key.startsWith(BUNDLE_CONFIG_KEY_PREFIX)) {
+ continue;
}
- long configKey = (long) value;
+ // the remaining values are config keys
+ long configKey = mSavedStatsConfigs.getLong(key);
try {
mStatsManager.removeConfig(configKey);
- String sharedPrefVersion = buildSharedPrefConfigVersionKey(configKey);
- editor.remove(key).remove(sharedPrefVersion);
+ String bundleVersion = buildBundleConfigVersionKey(configKey);
+ mSavedStatsConfigs.remove(key);
+ mSavedStatsConfigs.remove(bundleVersion);
} catch (StatsUnavailableException e) {
Slog.w(CarLog.TAG_TELEMETRY, "Failed to remove config " + configKey
+ ". Ignoring the failure. Will retry removing again when"
@@ -219,9 +256,9 @@
// retry. So we will just ignore the failures. The next call of this method
// will ry deleting StatsD configs again.
}
- });
- editor.apply();
- mConfigKeyToSubscribers.clear();
+ }
+ saveBundle();
+ mSavedStatsConfigs.clear();
}
mIsPullingReports.set(false);
mTelemetryHandler.removeCallbacks(mPullReportsPeriodically);
@@ -247,49 +284,48 @@
}
/**
- * Returns the key for SharedPreferences to store/retrieve configKey associated with the
+ * Returns the key for PersistableBundle to store/retrieve configKey associated with the
* subscriber.
*/
- private static String buildSharedPrefConfigKey(DataSubscriber subscriber) {
- return SHARED_PREF_CONFIG_KEY_PREFIX + subscriber.getMetricsConfig().getName() + "-"
+ private static String buildBundleConfigKey(DataSubscriber subscriber) {
+ return BUNDLE_CONFIG_KEY_PREFIX + subscriber.getMetricsConfig().getName() + "-"
+ subscriber.getSubscriber().getHandler();
}
/**
- * Returns the key for SharedPreferences to store/retrieve {@link TelemetryProto.MetricsConfig}
+ * Returns the key for PersistableBundle to store/retrieve {@link TelemetryProto.MetricsConfig}
* version associated with the configKey (which is generated per DataSubscriber).
*/
- private static String buildSharedPrefConfigVersionKey(long configKey) {
- return SHARED_PREF_CONFIG_VERSION_PREFIX + configKey;
+ private static String buildBundleConfigVersionKey(long configKey) {
+ return BUNDLE_CONFIG_VERSION_PREFIX + configKey;
}
/**
* This method can be called even if StatsdConfig was added to StatsD service before. It stores
- * previously added config_keys in the shared preferences and only updates StatsD when
+ * previously added config_keys in the persistable bundle and only updates StatsD when
* the MetricsConfig (of CarTelemetryService) has a new version.
*/
@GuardedBy("mLock")
private long addStatsConfigLocked(DataSubscriber subscriber) {
long configKey = buildConfigKey(subscriber);
// Store MetricsConfig (of CarTelemetryService) version per handler_function.
- String sharedPrefVersion = buildSharedPrefConfigVersionKey(configKey);
- if (mSharedPreferences.contains(sharedPrefVersion)) {
- int currentVersion = mSharedPreferences.getInt(sharedPrefVersion, 0);
+ String bundleVersion = buildBundleConfigVersionKey(configKey);
+ if (mSavedStatsConfigs.getInt(bundleVersion) != 0) {
+ int currentVersion = mSavedStatsConfigs.getInt(bundleVersion);
if (currentVersion >= subscriber.getMetricsConfig().getVersion()) {
// It's trying to add current or older MetricsConfig version, just ignore it.
return configKey;
} // if the subscriber's MetricsConfig version is newer, it will replace the old one.
}
- String sharedPrefConfigKey = buildSharedPrefConfigKey(subscriber);
+ String bundleConfigKey = buildBundleConfigKey(subscriber);
StatsdConfig config = buildStatsdConfig(subscriber, configKey);
try {
// It doesn't throw exception if the StatsdConfig is invalid. But it shouldn't happen,
// as we generate well-tested StatsdConfig in this service.
mStatsManager.addConfig(configKey, config.toByteArray());
- mSharedPreferences.edit()
- .putInt(sharedPrefVersion, subscriber.getMetricsConfig().getVersion())
- .putLong(sharedPrefConfigKey, configKey)
- .apply();
+ mSavedStatsConfigs.putInt(bundleVersion, subscriber.getMetricsConfig().getVersion());
+ mSavedStatsConfigs.putLong(bundleConfigKey, configKey);
+ saveBundle();
} catch (StatsUnavailableException e) {
Slog.w(CarLog.TAG_TELEMETRY, "Failed to add config" + configKey, e);
// TODO(b/189143813): if StatsManager is not ready, retry N times and hard fail after
@@ -304,13 +340,15 @@
/** Removes StatsdConfig and returns configKey. */
@GuardedBy("mLock")
private long removeStatsConfigLocked(DataSubscriber subscriber) {
- String sharedPrefConfigKey = buildSharedPrefConfigKey(subscriber);
+ String bundleConfigKey = buildBundleConfigKey(subscriber);
long configKey = buildConfigKey(subscriber);
// Store MetricsConfig (of CarTelemetryService) version per handler_function.
- String sharedPrefVersion = buildSharedPrefConfigVersionKey(configKey);
+ String bundleVersion = buildBundleConfigVersionKey(configKey);
try {
mStatsManager.removeConfig(configKey);
- mSharedPreferences.edit().remove(sharedPrefVersion).remove(sharedPrefConfigKey).apply();
+ mSavedStatsConfigs.remove(bundleVersion);
+ mSavedStatsConfigs.remove(bundleConfigKey);
+ saveBundle();
} catch (StatsUnavailableException e) {
Slog.w(CarLog.TAG_TELEMETRY, "Failed to remove config " + configKey
+ ". Ignoring the failure. Will retry removing again when"
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 6c41e4b..f7b48ca 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
@@ -30,7 +30,6 @@
import android.car.hardware.CarPropertyConfig;
import android.content.Context;
import android.content.ServiceConnection;
-import android.content.SharedPreferences;
import android.os.Handler;
import android.os.IBinder;
import android.os.PersistableBundle;
@@ -52,6 +51,7 @@
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
+import java.nio.file.Files;
import java.util.Collections;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.PriorityBlockingQueue;
@@ -105,20 +105,19 @@
@Mock
private StatsManagerProxy mMockStatsManager;
@Mock
- private SharedPreferences mMockSharedPreferences;
- @Mock
private IBinder mMockScriptExecutorBinder;
@Mock
private ResultStore mMockResultStore;
@Before
- public void setUp() {
+ public void setUp() throws Exception {
when(mMockCarPropertyService.getPropertyList())
.thenReturn(Collections.singletonList(PROP_CONFIG));
// bind service should return true, otherwise broker is disabled
when(mMockContext.bindServiceAsUser(any(), any(), anyInt(), any())).thenReturn(true);
PublisherFactory factory = new PublisherFactory(
- mMockCarPropertyService, mMockHandler, mMockStatsManager, mMockSharedPreferences);
+ mMockCarPropertyService, mMockHandler, mMockStatsManager,
+ Files.createTempDirectory("telemetry_test").toFile());
mDataBroker = new DataBrokerImpl(mMockContext, factory, mMockResultStore);
// add IdleHandler to get notified when all messages and posts are handled
mDataBroker.getTelemetryHandler().getLooper().getQueue().addIdleHandler(() -> {
diff --git a/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/StatsPublisherTest.java b/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/StatsPublisherTest.java
index a672bd5..738f541 100644
--- a/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/StatsPublisherTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/StatsPublisherTest.java
@@ -41,7 +41,6 @@
import com.android.car.telemetry.TelemetryProto;
import com.android.car.telemetry.databroker.DataSubscriber;
import com.android.car.test.FakeHandlerWrapper;
-import com.android.car.test.FakeSharedPreferences;
import com.google.common.collect.Range;
@@ -53,6 +52,10 @@
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
+import java.io.File;
+import java.io.FileInputStream;
+import java.nio.file.Files;
+
@RunWith(MockitoJUnitRunner.class)
public class StatsPublisherTest {
private static final TelemetryProto.Publisher STATS_PUBLISHER_PARAMS_1 =
@@ -92,10 +95,11 @@
private static final StatsLogProto.ConfigMetricsReportList EMPTY_STATS_REPORT =
StatsLogProto.ConfigMetricsReportList.newBuilder().build();
- private StatsPublisher mPublisher; // subject
- private final FakeSharedPreferences mFakeSharedPref = new FakeSharedPreferences();
private final FakeHandlerWrapper mFakeHandlerWrapper =
new FakeHandlerWrapper(Looper.getMainLooper(), FakeHandlerWrapper.Mode.QUEUEING);
+
+ private File mRootDirectory;
+ private StatsPublisher mPublisher; // subject
private Throwable mPublisherFailure;
@Mock private DataSubscriber mMockDataSubscriber;
@@ -105,6 +109,7 @@
@Before
public void setUp() throws Exception {
+ mRootDirectory = Files.createTempDirectory("telemetry_test").toFile();
mPublisher = createRestartedPublisher();
when(mMockDataSubscriber.getPublisherParam()).thenReturn(STATS_PUBLISHER_PARAMS_1);
when(mMockDataSubscriber.getMetricsConfig()).thenReturn(METRICS_CONFIG);
@@ -112,14 +117,14 @@
}
/**
- * Emulates a restart by creating a new StatsPublisher. StatsManager and SharedPreference
+ * Emulates a restart by creating a new StatsPublisher. StatsManager and PersistableBundle
* stays the same.
*/
- private StatsPublisher createRestartedPublisher() {
+ private StatsPublisher createRestartedPublisher() throws Exception {
return new StatsPublisher(
this::onPublisherFailure,
mStatsManager,
- mFakeSharedPref,
+ mRootDirectory,
mFakeHandlerWrapper.getMockHandler());
}
@@ -161,7 +166,7 @@
mPublisher.removeDataSubscriber(mMockDataSubscriber);
verify(mStatsManager, times(1)).removeConfig(SUBSCRIBER_1_HASH);
- assertThat(mFakeSharedPref.getAll().isEmpty()).isTrue(); // also removes from SharedPref.
+ assertThat(getSavedStatsConfigs().keySet()).isEmpty();
assertThat(mPublisher.hasDataSubscriber(mMockDataSubscriber)).isFalse();
}
@@ -185,7 +190,7 @@
publisher2.removeAllDataSubscribers();
verify(mStatsManager, times(1)).removeConfig(SUBSCRIBER_1_HASH);
- assertThat(mFakeSharedPref.getAll().isEmpty()).isTrue(); // also removes from SharedPref.
+ assertThat(getSavedStatsConfigs().keySet()).isEmpty();
assertThat(publisher2.hasDataSubscriber(mMockDataSubscriber)).isFalse();
}
@@ -258,6 +263,13 @@
assertThat(mBundleCaptor.getValue().getInt("reportsCount")).isEqualTo(2);
}
+ private PersistableBundle getSavedStatsConfigs() throws Exception {
+ File savedConfigsFile = new File(mRootDirectory, StatsPublisher.SAVED_STATS_CONFIGS_FILE);
+ try (FileInputStream fileInputStream = new FileInputStream(savedConfigsFile)) {
+ return PersistableBundle.readFromStream(fileInputStream);
+ }
+ }
+
// TODO(b/189143813): add test cases when connecting to Statsd fails
// TODO(b/189143813): add test cases for handling config version upgrades