diff --git a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java index cf25652d80..7063357d2c 100644 --- a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java +++ b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java @@ -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() { diff --git a/thread/service/java/com/android/server/thread/ThreadNetworkCountryCode.java b/thread/service/java/com/android/server/thread/ThreadNetworkCountryCode.java index 2cd34e86d7..ff0e2c19e8 100644 --- a/thread/service/java/com/android/server/thread/ThreadNetworkCountryCode.java +++ b/thread/service/java/com/android/server/thread/ThreadNetworkCountryCode.java @@ -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()); } } diff --git a/thread/service/java/com/android/server/thread/ThreadPersistentSettings.java b/thread/service/java/com/android/server/thread/ThreadPersistentSettings.java index 47cf20237b..fd6dec728c 100644 --- a/thread/service/java/com/android/server/thread/ThreadPersistentSettings.java +++ b/thread/service/java/com/android/server/thread/ThreadPersistentSettings.java @@ -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 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 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 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 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 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 THREAD_ENABLED_IN_AIRPLANE_MODE = - new Key<>("thread_enabled_in_airplane_mode", false); + public static final Key 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 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 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 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 CONFIG_DHCP6_PD_ENABLED = - new Key<>("config_dhcp6_pd_enabled", false); - - /******** Thread persistent setting keys ***************/ + public static final Key KEY_COUNTRY_CODE = new Key<>("thread_country_code"); @GuardedBy("mLock") private final AtomicFile mAtomicFile; private final Object mLock = new Object(); + private final Map 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 void put(String key, @Nullable T value) { - putObject(key, value); + /** Stores a value to the stored settings. */ + public void put(Key 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 get(Key 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 Type of the value. */ - public static class Key { - public final String key; - public final T defaultValue; + public static final class Key { + @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); } diff --git a/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java b/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java index 4360c7b2e3..95ebda5c5c 100644 --- a/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java +++ b/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java @@ -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 diff --git a/thread/tests/unit/src/com/android/server/thread/ThreadNetworkCountryCodeTest.java b/thread/tests/unit/src/com/android/server/thread/ThreadNetworkCountryCodeTest.java index ca9741df86..139f4c8fca 100644 --- a/thread/tests/unit/src/com/android/server/thread/ThreadNetworkCountryCodeTest.java +++ b/thread/tests/unit/src/com/android/server/thread/ThreadNetworkCountryCodeTest.java @@ -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); diff --git a/thread/tests/unit/src/com/android/server/thread/ThreadPersistentSettingsTest.java b/thread/tests/unit/src/com/android/server/thread/ThreadPersistentSettingsTest.java index ba489d9b1f..15f3d0b146 100644 --- a/thread/tests/unit/src/com/android/server/thread/ThreadPersistentSettingsTest.java +++ b/thread/tests/unit/src/com/android/server/thread/ThreadPersistentSettingsTest.java @@ -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 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(); }