[thread] refactor the settings module

The current settings APIs have a few issues:
1. It doesn't support default value which is derived from resource
   overlays
2. It exposes the default values of settings entries as public APIs
   while they shouldn't be

Bug: 401654864

Change-Id: I1bbe3f14507a497320401f02fa6da512558fe385
This commit is contained in:
Kangping Dong 2025-03-10 14:14:06 +08:00
commit d9bbec2fa1
6 changed files with 100 additions and 108 deletions

View file

@ -559,7 +559,7 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub
// The persistent setting keeps the desired enabled state, thus it's set regardless
// the otDaemon set enabled state operation succeeded or not, so that it can recover
// to the desired value after reboot.
mPersistentSettings.put(ThreadPersistentSettings.THREAD_ENABLED.key, isEnabled);
mPersistentSettings.put(ThreadPersistentSettings.KEY_THREAD_ENABLED, isEnabled);
}
try {
@ -743,7 +743,7 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub
private boolean shouldEnableThread() {
return !mForceStopOtDaemonEnabled
&& !mUserRestricted
&& mPersistentSettings.get(ThreadPersistentSettings.THREAD_ENABLED);
&& mPersistentSettings.get(ThreadPersistentSettings.KEY_THREAD_ENABLED);
}
private void requestUpstreamNetwork() {

View file

@ -16,7 +16,7 @@
package com.android.server.thread;
import static com.android.server.thread.ThreadPersistentSettings.THREAD_COUNTRY_CODE;
import static com.android.server.thread.ThreadPersistentSettings.KEY_COUNTRY_CODE;
import android.annotation.Nullable;
import android.annotation.StringDef;
@ -496,7 +496,7 @@ public class ThreadNetworkCountryCode {
return mLocationCountryCodeInfo;
}
String settingsCountryCode = mPersistentSettings.get(THREAD_COUNTRY_CODE);
String settingsCountryCode = mPersistentSettings.get(KEY_COUNTRY_CODE);
if (settingsCountryCode != null) {
return new CountryCodeInfo(settingsCountryCode, COUNTRY_CODE_SOURCE_SETTINGS);
}
@ -514,8 +514,7 @@ public class ThreadNetworkCountryCode {
public void onSuccess() {
synchronized ("ThreadNetworkCountryCode.this") {
mCurrentCountryCodeInfo = countryCodeInfo;
mPersistentSettings.put(
THREAD_COUNTRY_CODE.key, countryCodeInfo.getCountryCode());
mPersistentSettings.put(KEY_COUNTRY_CODE, countryCodeInfo.getCountryCode());
}
}

View file

@ -40,6 +40,8 @@ import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.HashMap;
import java.util.Map;
/**
* Store persistent data for Thread network settings. These are key (string) / value pairs that are
@ -53,54 +55,55 @@ public class ThreadPersistentSettings {
/** File name used for storing settings. */
private static final String FILE_NAME = "ThreadPersistentSettings.xml";
/** Current config store data version. This will be incremented for any additions. */
/** Current config store data version. This MUST be incremented for any incompatible changes. */
private static final int CURRENT_SETTINGS_STORE_DATA_VERSION = 1;
/**
* Stores the version of the data. This can be used to handle migration of data if some
* non-backward compatible change introduced.
*/
private static final String VERSION_KEY = "version";
private static final String KEY_VERSION = "version";
/******** Thread persistent setting keys ***************/
/** Stores the Thread feature toggle state, true for enabled and false for disabled. */
public static final Key<Boolean> THREAD_ENABLED = new Key<>("thread_enabled", true);
/**
* Saves the boolean flag for Thread being enabled. The value defaults to resource overlay value
* {@code R.bool.config_thread_default_enabled}.
*/
public static final Key<Boolean> KEY_THREAD_ENABLED = new Key<>("thread_enabled");
/**
* Saves the boolean flag for border router being enabled. The value defaults to resource
* overlay value {@code R.bool.config_thread_border_router_default_enabled}.
*/
private static final Key<Boolean> KEY_CONFIG_BORDER_ROUTER_ENABLED =
new Key<>("config_border_router_enabled");
/** Stores the Thread NAT64 feature toggle state, true for enabled and false for disabled. */
private static final Key<Boolean> KEY_CONFIG_NAT64_ENABLED = new Key<>("config_nat64_enabled");
/**
* Stores the Thread DHCPv6-PD feature toggle state, true for enabled and false for disabled.
*/
private static final Key<Boolean> KEY_CONFIG_DHCP6_PD_ENABLED =
new Key<>("config_dhcp6_pd_enabled");
/**
* Indicates that Thread was enabled (i.e. via the setEnabled() API) when the airplane mode is
* turned on in settings. When this value is {@code true}, the current airplane mode state will
* be ignored when evaluating the Thread enabled state.
*/
public static final Key<Boolean> THREAD_ENABLED_IN_AIRPLANE_MODE =
new Key<>("thread_enabled_in_airplane_mode", false);
public static final Key<Boolean> KEY_THREAD_ENABLED_IN_AIRPLANE_MODE =
new Key<>("thread_enabled_in_airplane_mode");
/** Stores the Thread country code, null if no country code is stored. */
public static final Key<String> THREAD_COUNTRY_CODE = new Key<>("thread_country_code", null);
/**
* Saves the boolean flag for border router being enabled. The value defaults to {@code true} if
* this config is missing.
*/
private static final Key<Boolean> CONFIG_BORDER_ROUTER_ENABLED =
new Key<>("config_border_router_enabled", false);
/** Stores the Thread NAT64 feature toggle state, true for enabled and false for disabled. */
private static final Key<Boolean> CONFIG_NAT64_ENABLED =
new Key<>("config_nat64_enabled", false);
/**
* Stores the Thread DHCPv6-PD feature toggle state, true for enabled and false for disabled.
*/
private static final Key<Boolean> CONFIG_DHCP6_PD_ENABLED =
new Key<>("config_dhcp6_pd_enabled", false);
/******** Thread persistent setting keys ***************/
public static final Key<String> KEY_COUNTRY_CODE = new Key<>("thread_country_code");
@GuardedBy("mLock")
private final AtomicFile mAtomicFile;
private final Object mLock = new Object();
private final Map<String, Object> mDefaultValues = new HashMap<>();
@GuardedBy("mLock")
private final PersistableBundle mSettings = new PersistableBundle();
@ -116,32 +119,22 @@ public class ThreadPersistentSettings {
ThreadPersistentSettings(AtomicFile atomicFile, ConnectivityResources resources) {
mAtomicFile = atomicFile;
mResources = resources;
mDefaultValues.put(
KEY_THREAD_ENABLED.key,
mResources.get().getBoolean(R.bool.config_thread_default_enabled));
mDefaultValues.put(
KEY_CONFIG_BORDER_ROUTER_ENABLED.key,
mResources.get().getBoolean(R.bool.config_thread_border_router_default_enabled));
mDefaultValues.put(KEY_CONFIG_NAT64_ENABLED.key, false);
mDefaultValues.put(KEY_CONFIG_DHCP6_PD_ENABLED.key, false);
mDefaultValues.put(KEY_THREAD_ENABLED_IN_AIRPLANE_MODE.key, false);
mDefaultValues.put(KEY_COUNTRY_CODE.key, null);
}
/** Initialize the settings by reading from the settings file. */
public void initialize() {
readFromStoreFile();
synchronized (mLock) {
if (!mSettings.containsKey(THREAD_ENABLED.key)) {
boolean enabled = mResources.get().getBoolean(R.bool.config_thread_default_enabled);
LOG.i(
"\"thread_enabled\" is missing in settings file, using default value: "
+ enabled);
put(THREAD_ENABLED.key, enabled);
}
if (!mSettings.containsKey(CONFIG_BORDER_ROUTER_ENABLED.key)) {
boolean borderRouterEnabled =
mResources
.get()
.getBoolean(R.bool.config_thread_border_router_default_enabled);
LOG.i(
"\"thread_border_router_enabled\" is missing in settings file, using"
+ " default value: "
+ borderRouterEnabled);
put(CONFIG_BORDER_ROUTER_ENABLED.key, borderRouterEnabled);
}
}
}
private void putObject(String key, @Nullable Object value) {
@ -186,25 +179,17 @@ public class ThreadPersistentSettings {
return (T) value;
}
/**
* Store a value to the stored settings.
*
* @param key One of the settings keys.
* @param value Value to be stored.
*/
public <T> void put(String key, @Nullable T value) {
putObject(key, value);
/** Stores a value to the stored settings. */
public <T> void put(Key<T> key, @Nullable T value) {
putObject(key.key, value);
writeToStoreFile();
}
/**
* Retrieve a value from the stored settings.
*
* @param key One of the settings keys.
* @return value stored in settings, defValue if the key does not exist.
*/
/** Retrieves a value from the stored settings. */
@Nullable
public <T> T get(Key<T> key) {
return getObject(key.key, key.defaultValue);
T defaultValue = (T) mDefaultValues.get(key.key);
return getObject(key.key, defaultValue);
}
/**
@ -217,9 +202,9 @@ public class ThreadPersistentSettings {
if (getConfiguration().equals(configuration)) {
return false;
}
putObject(CONFIG_BORDER_ROUTER_ENABLED.key, configuration.isBorderRouterEnabled());
putObject(CONFIG_NAT64_ENABLED.key, configuration.isNat64Enabled());
putObject(CONFIG_DHCP6_PD_ENABLED.key, configuration.isDhcpv6PdEnabled());
put(KEY_CONFIG_BORDER_ROUTER_ENABLED, configuration.isBorderRouterEnabled());
put(KEY_CONFIG_NAT64_ENABLED, configuration.isNat64Enabled());
put(KEY_CONFIG_DHCP6_PD_ENABLED, configuration.isDhcpv6PdEnabled());
writeToStoreFile();
return true;
}
@ -227,9 +212,9 @@ public class ThreadPersistentSettings {
/** Retrieve the {@link ThreadConfiguration} from the persistent settings. */
public ThreadConfiguration getConfiguration() {
return new ThreadConfiguration.Builder()
.setBorderRouterEnabled(get(CONFIG_BORDER_ROUTER_ENABLED))
.setNat64Enabled(get(CONFIG_NAT64_ENABLED))
.setDhcpv6PdEnabled(get(CONFIG_DHCP6_PD_ENABLED))
.setBorderRouterEnabled(get(KEY_CONFIG_BORDER_ROUTER_ENABLED))
.setNat64Enabled(get(KEY_CONFIG_NAT64_ENABLED))
.setDhcpv6PdEnabled(get(KEY_CONFIG_DHCP6_PD_ENABLED))
.build();
}
@ -238,18 +223,11 @@ public class ThreadPersistentSettings {
*
* @param <T> Type of the value.
*/
public static class Key<T> {
public final String key;
public final T defaultValue;
public static final class Key<T> {
@VisibleForTesting final String key;
private Key(String key, T defaultValue) {
private Key(String key) {
this.key = key;
this.defaultValue = defaultValue;
}
@Override
public String toString() {
return "[Key: " + key + ", DefaultValue: " + defaultValue + "]";
}
}
@ -260,7 +238,7 @@ public class ThreadPersistentSettings {
synchronized (mLock) {
bundleToWrite = new PersistableBundle(mSettings);
}
bundleToWrite.putInt(VERSION_KEY, CURRENT_SETTINGS_STORE_DATA_VERSION);
bundleToWrite.putInt(KEY_VERSION, CURRENT_SETTINGS_STORE_DATA_VERSION);
bundleToWrite.writeToStream(outputStream);
synchronized (mLock) {
writeToAtomicFile(mAtomicFile, outputStream.toByteArray());
@ -280,7 +258,7 @@ public class ThreadPersistentSettings {
final ByteArrayInputStream inputStream = new ByteArrayInputStream(readData);
final PersistableBundle bundleRead = PersistableBundle.readFromStream(inputStream);
// Version unused for now. May be needed in the future for handling migrations.
bundleRead.remove(VERSION_KEY);
bundleRead.remove(KEY_VERSION);
synchronized (mLock) {
mSettings.putAll(bundleRead);
}

View file

@ -35,6 +35,7 @@ import static android.net.thread.ThreadNetworkManager.PERMISSION_THREAD_NETWORK_
import static android.net.thread.ThreadNetworkManager.PERMISSION_THREAD_NETWORK_TESTING;
import static com.android.server.thread.ThreadNetworkCountryCode.DEFAULT_COUNTRY_CODE;
import static com.android.server.thread.ThreadPersistentSettings.KEY_THREAD_ENABLED;
import static com.android.server.thread.openthread.IOtDaemon.ErrorCode.OT_ERROR_INVALID_STATE;
import static com.android.testutils.TestPermissionUtil.runAsShell;
@ -580,7 +581,7 @@ public final class ThreadNetworkControllerServiceTest {
mTestLooper.dispatchAll();
assertThat(mFakeOtDaemon.getEnabledState()).isEqualTo(STATE_DISABLED);
assertThat(mPersistentSettings.get(ThreadPersistentSettings.THREAD_ENABLED)).isTrue();
assertThat(mPersistentSettings.get(KEY_THREAD_ENABLED)).isTrue();
}
@Test

View file

@ -19,7 +19,7 @@ package com.android.server.thread;
import static android.net.thread.ThreadNetworkException.ERROR_INTERNAL_ERROR;
import static com.android.server.thread.ThreadNetworkCountryCode.DEFAULT_COUNTRY_CODE;
import static com.android.server.thread.ThreadPersistentSettings.THREAD_COUNTRY_CODE;
import static com.android.server.thread.ThreadPersistentSettings.KEY_COUNTRY_CODE;
import static com.google.common.truth.Truth.assertThat;
@ -454,7 +454,7 @@ public class ThreadNetworkCountryCodeTest {
@Test
public void settingsCountryCode_settingsCountryCodeIsActive_settingsCountryCodeIsUsed() {
when(mPersistentSettings.get(THREAD_COUNTRY_CODE)).thenReturn(TEST_COUNTRY_CODE_CN);
when(mPersistentSettings.get(KEY_COUNTRY_CODE)).thenReturn(TEST_COUNTRY_CODE_CN);
mThreadNetworkCountryCode.initialize();
assertThat(mThreadNetworkCountryCode.getCountryCode()).isEqualTo(TEST_COUNTRY_CODE_CN);

View file

@ -16,8 +16,8 @@
package com.android.server.thread;
import static com.android.server.thread.ThreadPersistentSettings.THREAD_COUNTRY_CODE;
import static com.android.server.thread.ThreadPersistentSettings.THREAD_ENABLED;
import static com.android.server.thread.ThreadPersistentSettings.KEY_COUNTRY_CODE;
import static com.android.server.thread.ThreadPersistentSettings.KEY_THREAD_ENABLED;
import static com.google.common.truth.Truth.assertThat;
@ -35,6 +35,7 @@ import androidx.test.runner.AndroidJUnit4;
import com.android.connectivity.resources.R;
import com.android.server.connectivity.ConnectivityResources;
import com.android.server.thread.ThreadPersistentSettings.Key;
import org.junit.After;
import org.junit.Before;
@ -83,68 +84,81 @@ public class ThreadPersistentSettingsTest {
@Test
public void initialize_readsFromFile() throws Exception {
byte[] data = createXmlForParsing(THREAD_ENABLED.key, false);
byte[] data = createXmlForParsing(KEY_THREAD_ENABLED, false);
setupAtomicFileForRead(data);
mThreadPersistentSettings.initialize();
assertThat(mThreadPersistentSettings.get(THREAD_ENABLED)).isFalse();
assertThat(mThreadPersistentSettings.get(KEY_THREAD_ENABLED)).isFalse();
}
@Test
public void initialize_ThreadDisabledInResources_returnsThreadDisabled() throws Exception {
when(mResources.getBoolean(eq(R.bool.config_thread_default_enabled))).thenReturn(false);
setupAtomicFileForRead(new byte[0]);
mThreadPersistentSettings =
new ThreadPersistentSettings(mAtomicFile, mConnectivityResources);
mThreadPersistentSettings.initialize();
assertThat(mThreadPersistentSettings.get(THREAD_ENABLED)).isFalse();
assertThat(mThreadPersistentSettings.get(KEY_THREAD_ENABLED)).isFalse();
}
@Test
public void initialize_ThreadEnabledInResources_returnsThreadEnabled() throws Exception {
when(mResources.getBoolean(eq(R.bool.config_thread_default_enabled))).thenReturn(true);
mThreadPersistentSettings =
new ThreadPersistentSettings(mAtomicFile, mConnectivityResources);
mThreadPersistentSettings.initialize();
assertThat(mThreadPersistentSettings.get(KEY_THREAD_ENABLED)).isTrue();
}
@Test
public void initialize_ThreadDisabledInResourcesButEnabledInXml_returnsThreadEnabled()
throws Exception {
when(mResources.getBoolean(eq(R.bool.config_thread_default_enabled))).thenReturn(false);
byte[] data = createXmlForParsing(THREAD_ENABLED.key, true);
setupAtomicFileForRead(data);
setupAtomicFileForRead(createXmlForParsing(KEY_THREAD_ENABLED, true));
mThreadPersistentSettings =
new ThreadPersistentSettings(mAtomicFile, mConnectivityResources);
mThreadPersistentSettings.initialize();
assertThat(mThreadPersistentSettings.get(THREAD_ENABLED)).isTrue();
assertThat(mThreadPersistentSettings.get(KEY_THREAD_ENABLED)).isTrue();
}
@Test
public void put_ThreadFeatureEnabledTrue_returnsTrue() throws Exception {
mThreadPersistentSettings.put(THREAD_ENABLED.key, true);
mThreadPersistentSettings.put(KEY_THREAD_ENABLED, true);
assertThat(mThreadPersistentSettings.get(THREAD_ENABLED)).isTrue();
assertThat(mThreadPersistentSettings.get(KEY_THREAD_ENABLED)).isTrue();
}
@Test
public void put_ThreadFeatureEnabledFalse_returnsFalse() throws Exception {
mThreadPersistentSettings.put(THREAD_ENABLED.key, false);
mThreadPersistentSettings.put(KEY_THREAD_ENABLED, false);
assertThat(mThreadPersistentSettings.get(THREAD_ENABLED)).isFalse();
assertThat(mThreadPersistentSettings.get(KEY_THREAD_ENABLED)).isFalse();
mThreadPersistentSettings.initialize();
assertThat(mThreadPersistentSettings.get(THREAD_ENABLED)).isFalse();
assertThat(mThreadPersistentSettings.get(KEY_THREAD_ENABLED)).isFalse();
}
@Test
public void put_ThreadCountryCodeString_returnsString() throws Exception {
mThreadPersistentSettings.put(THREAD_COUNTRY_CODE.key, TEST_COUNTRY_CODE);
mThreadPersistentSettings.put(KEY_COUNTRY_CODE, TEST_COUNTRY_CODE);
assertThat(mThreadPersistentSettings.get(THREAD_COUNTRY_CODE)).isEqualTo(TEST_COUNTRY_CODE);
assertThat(mThreadPersistentSettings.get(KEY_COUNTRY_CODE)).isEqualTo(TEST_COUNTRY_CODE);
mThreadPersistentSettings.initialize();
assertThat(mThreadPersistentSettings.get(THREAD_COUNTRY_CODE)).isEqualTo(TEST_COUNTRY_CODE);
assertThat(mThreadPersistentSettings.get(KEY_COUNTRY_CODE)).isEqualTo(TEST_COUNTRY_CODE);
}
@Test
public void put_ThreadCountryCodeNull_returnsNull() throws Exception {
mThreadPersistentSettings.put(THREAD_COUNTRY_CODE.key, null);
mThreadPersistentSettings.put(KEY_COUNTRY_CODE, null);
assertThat(mThreadPersistentSettings.get(THREAD_COUNTRY_CODE)).isNull();
assertThat(mThreadPersistentSettings.get(KEY_COUNTRY_CODE)).isNull();
mThreadPersistentSettings.initialize();
assertThat(mThreadPersistentSettings.get(THREAD_COUNTRY_CODE)).isNull();
assertThat(mThreadPersistentSettings.get(KEY_COUNTRY_CODE)).isNull();
}
@Test
@ -202,10 +216,10 @@ public class ThreadPersistentSettingsTest {
return new AtomicFile(mTemporaryFolder.newFile());
}
private byte[] createXmlForParsing(String key, Boolean value) throws Exception {
private byte[] createXmlForParsing(Key<Boolean> key, Boolean value) throws Exception {
PersistableBundle bundle = new PersistableBundle();
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
bundle.putBoolean(key, value);
bundle.putBoolean(key.key, value);
bundle.writeToStream(outputStream);
return outputStream.toByteArray();
}