From 82988c3d98bf70ed6e9d8ad6f6a0c262b38189af Mon Sep 17 00:00:00 2001
From: Kangping Dong <wgtdkp@google.com>
Date: Sat, 8 Mar 2025 12:50:09 +0800
Subject: [PATCH 1/2] [thread] Support non-BR mode

This commit adds a resource overlay config for OEMs to disable the
Border Router feature by default. Also updates the tests to cover both
BR and non-BR use cases

Bug: 401654864
Change-Id: I7dd4363836269a08853b58304f62fe52f9021a9e
---
 .../res/values/config_thread.xml              |   4 +
 .../net/thread/ThreadNetworkController.java   |   7 +-
 .../ThreadNetworkControllerService.java       |  13 +-
 .../thread/ThreadPersistentSettings.java      |  23 ++-
 .../cts/ThreadNetworkControllerTest.java      |  56 +++++--
 .../android/net/thread/BorderRoutingTest.java |   4 +-
 .../net/thread/ServiceDiscoveryTest.java      | 140 ++++++++++++++++++
 .../net/thread/ThreadIntegrationTest.java     |  41 +++--
 .../thread/ThreadNetworkShellCommandTest.java |   2 +
 .../net/thread/utils/FullThreadDevice.java    |   6 +
 .../net/thread/utils/IntegrationTestUtils.kt  |  26 ++++
 .../utils/ThreadNetworkControllerWrapper.java |   5 +-
 .../ThreadNetworkControllerServiceTest.java   |  30 +++-
 13 files changed, 306 insertions(+), 51 deletions(-)

diff --git a/service/ServiceConnectivityResources/res/values/config_thread.xml b/service/ServiceConnectivityResources/res/values/config_thread.xml
index d1d9e5292e..128a98f6d4 100644
--- a/service/ServiceConnectivityResources/res/values/config_thread.xml
+++ b/service/ServiceConnectivityResources/res/values/config_thread.xml
@@ -26,6 +26,10 @@
     -->
     <bool name="config_thread_default_enabled">true</bool>
 
+    <!-- Sets to {@code true} to enable Thread Border Router on the device by default.
+    -->
+    <bool name="config_thread_border_router_default_enabled">false</bool>
+
     <!-- Whether to use location APIs in the algorithm to determine country code or not.
     If disabled, will use other sources (telephony, wifi, etc) to determine device location for
     Thread Network regulatory purposes.
diff --git a/thread/framework/java/android/net/thread/ThreadNetworkController.java b/thread/framework/java/android/net/thread/ThreadNetworkController.java
index 73a6bda60c..b64971614d 100644
--- a/thread/framework/java/android/net/thread/ThreadNetworkController.java
+++ b/thread/framework/java/android/net/thread/ThreadNetworkController.java
@@ -227,8 +227,8 @@ public final class ThreadNetworkController {
      * specific error:
      *
      * <ul>
-     *   <li>{@link ThreadNetworkException#ERROR_FAILED_PRECONDITION} when this device is not
-     *       attached to Thread network
+     *   <li>{@link ThreadNetworkException#ERROR_FAILED_PRECONDITION} when this device is not a
+     *       Border Router or not attached to Thread network
      *   <li>{@link ThreadNetworkException#ERROR_BUSY} when ephemeral key mode is already activated
      *       on the device, caller can recover from this error when the ephemeral key mode gets
      *       deactivated
@@ -267,7 +267,8 @@ public final class ThreadNetworkController {
      * connection will be terminated.
      *
      * <p>On success, {@link OutcomeReceiver#onResult} of {@code receiver} is called. The call will
-     * always succeed if the device is not in ephemeral key mode.
+     * always succeed if the device is not in ephemeral key mode. It returns an error {@link
+     * ThreadNetworkException#ERROR_FAILED_PRECONDITION} if this device is not a Border Router.
      *
      * @param executor the executor to execute {@code receiver}
      * @param receiver the receiver to receive the result of this operation
diff --git a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
index af16d19ea2..cf25652d80 100644
--- a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
+++ b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
@@ -879,10 +879,8 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub
                         .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED);
         final var scoreBuilder = new NetworkScore.Builder();
 
-        if (isBorderRouterMode()) {
-            netCapsBuilder.addCapability(NetworkCapabilities.NET_CAPABILITY_LOCAL_NETWORK);
-            scoreBuilder.setKeepConnectedReason(NetworkScore.KEEP_CONNECTED_LOCAL_NETWORK);
-        }
+        netCapsBuilder.addCapability(NetworkCapabilities.NET_CAPABILITY_LOCAL_NETWORK);
+        scoreBuilder.setKeepConnectedReason(NetworkScore.KEEP_CONNECTED_LOCAL_NETWORK);
 
         return new NetworkAgent(
                 mContext,
@@ -890,7 +888,7 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub
                 LOG.getTag(),
                 netCapsBuilder.build(),
                 getTunIfLinkProperties(),
-                isBorderRouterMode() ? newLocalNetworkConfig() : null,
+                newLocalNetworkConfig(),
                 scoreBuilder.build(),
                 new NetworkAgentConfig.Builder().build(),
                 mNetworkProvider) {
@@ -899,9 +897,8 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub
             @Override
             public void onNetworkUnwanted() {
                 LOG.i("Thread network is unwanted by ConnectivityService");
-                if (!isBorderRouterMode()) {
-                    leave(false /* eraseDataset */, new LoggingOperationReceiver("leave"));
-                }
+                // TODO(b/374037595): leave() the current network when the new APIs for mobile
+                // is available
             }
         };
     }
diff --git a/thread/service/java/com/android/server/thread/ThreadPersistentSettings.java b/thread/service/java/com/android/server/thread/ThreadPersistentSettings.java
index 746b587553..47cf20237b 100644
--- a/thread/service/java/com/android/server/thread/ThreadPersistentSettings.java
+++ b/thread/service/java/com/android/server/thread/ThreadPersistentSettings.java
@@ -82,7 +82,7 @@ public class ThreadPersistentSettings {
      * this config is missing.
      */
     private static final Key<Boolean> CONFIG_BORDER_ROUTER_ENABLED =
-            new Key<>("config_border_router_enabled", true);
+            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 =
@@ -123,10 +123,23 @@ public class ThreadPersistentSettings {
         readFromStoreFile();
         synchronized (mLock) {
             if (!mSettings.containsKey(THREAD_ENABLED.key)) {
-                LOG.i("\"thread_enabled\" is missing in settings file, using default value");
-                put(
-                        THREAD_ENABLED.key,
-                        mResources.get().getBoolean(R.bool.config_thread_default_enabled));
+                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);
             }
         }
     }
diff --git a/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java b/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java
index a97972181b..2d681195c2 100644
--- a/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java
+++ b/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java
@@ -49,6 +49,8 @@ import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
 
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeTrue;
 
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
@@ -94,12 +96,15 @@ import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -118,6 +123,7 @@ import java.util.function.Predicate;
 /** CTS tests for {@link ThreadNetworkController}. */
 @LargeTest
 @RequiresThreadFeature
+@RunWith(Parameterized.class)
 public class ThreadNetworkControllerTest {
     private static final int JOIN_TIMEOUT_MILLIS = 30 * 1000;
     private static final int LEAVE_TIMEOUT_MILLIS = 2_000;
@@ -134,8 +140,6 @@ public class ThreadNetworkControllerTest {
     private static final String MESHCOP_SERVICE_TYPE = "_meshcop._udp";
     private static final String THREAD_NETWORK_PRIVILEGED =
             "android.permission.THREAD_NETWORK_PRIVILEGED";
-    private static final ThreadConfiguration DEFAULT_CONFIG =
-            new ThreadConfiguration.Builder().build();
     private static final SparseIntArray CHANNEL_MAX_POWERS =
             new SparseIntArray() {
                 {
@@ -161,6 +165,22 @@ public class ThreadNetworkControllerTest {
     private final List<Consumer<ThreadConfiguration>> mConfigurationCallbacksToCleanUp =
             new ArrayList<>();
 
+    public final boolean mIsBorderRouterEnabled;
+    private final ThreadConfiguration mDefaultConfig;
+
+    @Parameterized.Parameters
+    public static Collection configArguments() {
+        return Arrays.asList(new Object[][] {{false}, {true}});
+    }
+
+    public ThreadNetworkControllerTest(boolean isBorderRouterEnabled) {
+        mIsBorderRouterEnabled = isBorderRouterEnabled;
+        mDefaultConfig =
+                new ThreadConfiguration.Builder()
+                        .setBorderRouterEnabled(isBorderRouterEnabled)
+                        .build();
+    }
+
     @Before
     public void setUp() throws Exception {
         mController =
@@ -175,8 +195,10 @@ public class ThreadNetworkControllerTest {
         mHandlerThread.start();
 
         setEnabledAndWait(mController, true);
-        setConfigurationAndWait(mController, DEFAULT_CONFIG);
-        deactivateEphemeralKeyModeAndWait(mController);
+        setConfigurationAndWait(mController, mDefaultConfig);
+        if (mDefaultConfig.isBorderRouterEnabled()) {
+            deactivateEphemeralKeyModeAndWait(mController);
+        }
     }
 
     @After
@@ -185,7 +207,7 @@ public class ThreadNetworkControllerTest {
         setEnabledAndWait(mController, true);
         leaveAndWait(mController);
         tearDownTestNetwork();
-        setConfigurationAndWait(mController, DEFAULT_CONFIG);
+        setConfigurationAndWait(mController, mDefaultConfig);
         for (Consumer<ThreadConfiguration> configurationCallback :
                 mConfigurationCallbacksToCleanUp) {
             try {
@@ -197,7 +219,9 @@ public class ThreadNetworkControllerTest {
             }
         }
         mConfigurationCallbacksToCleanUp.clear();
-        deactivateEphemeralKeyModeAndWait(mController);
+        if (mDefaultConfig.isBorderRouterEnabled()) {
+            deactivateEphemeralKeyModeAndWait(mController);
+        }
     }
 
     @Test
@@ -573,7 +597,7 @@ public class ThreadNetworkControllerTest {
                     @Override
                     public void onActiveOperationalDatasetChanged(
                             ActiveOperationalDataset activeDataset) {
-                        if (activeDataset.equals(activeDataset2)) {
+                        if (Objects.equals(activeDataset, activeDataset2)) {
                             dataset2IsApplied.complete(true);
                         }
                     }
@@ -843,6 +867,7 @@ public class ThreadNetworkControllerTest {
     @Test
     @RequiresFlagsEnabled({Flags.FLAG_EPSKC_ENABLED})
     public void activateEphemeralKeyMode_withPrivilegedPermission_succeeds() throws Exception {
+        assumeTrue(mDefaultConfig.isBorderRouterEnabled());
         joinRandomizedDatasetAndWait(mController);
         CompletableFuture<Void> startFuture = new CompletableFuture<>();
 
@@ -861,6 +886,7 @@ public class ThreadNetworkControllerTest {
     @RequiresFlagsEnabled({Flags.FLAG_EPSKC_ENABLED})
     public void activateEphemeralKeyMode_withoutPrivilegedPermission_throwsSecurityException()
             throws Exception {
+        assumeTrue(mDefaultConfig.isBorderRouterEnabled());
         dropAllPermissions();
 
         assertThrows(
@@ -874,6 +900,7 @@ public class ThreadNetworkControllerTest {
     @RequiresFlagsEnabled({Flags.FLAG_EPSKC_ENABLED})
     public void activateEphemeralKeyMode_withZeroLifetime_throwsIllegalArgumentException()
             throws Exception {
+        assumeTrue(mDefaultConfig.isBorderRouterEnabled());
         grantPermissions(THREAD_NETWORK_PRIVILEGED);
 
         assertThrows(
@@ -885,6 +912,7 @@ public class ThreadNetworkControllerTest {
     @RequiresFlagsEnabled({Flags.FLAG_EPSKC_ENABLED})
     public void activateEphemeralKeyMode_withInvalidLargeLifetime_throwsIllegalArgumentException()
             throws Exception {
+        assumeTrue(mDefaultConfig.isBorderRouterEnabled());
         grantPermissions(THREAD_NETWORK_PRIVILEGED);
         Duration lifetime = mController.getMaxEphemeralKeyLifetime().plusMillis(1);
 
@@ -897,6 +925,7 @@ public class ThreadNetworkControllerTest {
     @RequiresFlagsEnabled({Flags.FLAG_EPSKC_ENABLED})
     public void activateEphemeralKeyMode_concurrentRequests_secondOneFailsWithBusyError()
             throws Exception {
+        assumeTrue(mDefaultConfig.isBorderRouterEnabled());
         joinRandomizedDatasetAndWait(mController);
         CompletableFuture<Void> future1 = new CompletableFuture<>();
         CompletableFuture<Void> future2 = new CompletableFuture<>();
@@ -945,6 +974,7 @@ public class ThreadNetworkControllerTest {
     @RequiresFlagsEnabled({Flags.FLAG_EPSKC_ENABLED})
     public void deactivateEphemeralKeyMode_withoutPrivilegedPermission_throwsSecurityException()
             throws Exception {
+        assumeTrue(mDefaultConfig.isBorderRouterEnabled());
         dropAllPermissions();
 
         assertThrows(
@@ -956,9 +986,7 @@ public class ThreadNetworkControllerTest {
     @RequiresFlagsEnabled({Flags.FLAG_EPSKC_ENABLED})
     public void deactivateEphemeralKeyMode_notBorderRouter_failsWithFailedPrecondition()
             throws Exception {
-        setConfigurationAndWait(
-                mController,
-                new ThreadConfiguration.Builder().setBorderRouterEnabled(false).build());
+        assumeFalse(mDefaultConfig.isBorderRouterEnabled());
         grantPermissions(THREAD_NETWORK_PRIVILEGED);
         CompletableFuture<Void> future = new CompletableFuture<>();
 
@@ -975,6 +1003,7 @@ public class ThreadNetworkControllerTest {
     @Test
     @RequiresFlagsEnabled({Flags.FLAG_EPSKC_ENABLED})
     public void subscribeEpskcState_permissionsGranted_returnsCurrentState() throws Exception {
+        assumeTrue(mDefaultConfig.isBorderRouterEnabled());
         CompletableFuture<Integer> stateFuture = new CompletableFuture<>();
         CompletableFuture<String> ephemeralKeyFuture = new CompletableFuture<>();
         CompletableFuture<Instant> expiryFuture = new CompletableFuture<>();
@@ -1011,6 +1040,7 @@ public class ThreadNetworkControllerTest {
     @RequiresFlagsEnabled({Flags.FLAG_EPSKC_ENABLED})
     public void subscribeEpskcState_withoutThreadPriviledgedPermission_returnsNullEphemeralKey()
             throws Exception {
+        assumeTrue(mDefaultConfig.isBorderRouterEnabled());
         CompletableFuture<Integer> stateFuture = new CompletableFuture<>();
         CompletableFuture<String> ephemeralKeyFuture = new CompletableFuture<>();
         CompletableFuture<Instant> expiryFuture = new CompletableFuture<>();
@@ -1050,6 +1080,7 @@ public class ThreadNetworkControllerTest {
     @Test
     @RequiresFlagsEnabled({Flags.FLAG_EPSKC_ENABLED})
     public void subscribeEpskcState_ephemralKeyStateChanged_returnsUpdatedState() throws Exception {
+        assumeTrue(mDefaultConfig.isBorderRouterEnabled());
         EphemeralKeyStateListener listener = new EphemeralKeyStateListener(mController);
         joinRandomizedDatasetAndWait(mController);
 
@@ -1068,6 +1099,7 @@ public class ThreadNetworkControllerTest {
     @Test
     @RequiresFlagsEnabled({Flags.FLAG_EPSKC_ENABLED})
     public void subscribeEpskcState_epskcEnabled_returnsSameExpiry() throws Exception {
+        assumeTrue(mDefaultConfig.isBorderRouterEnabled());
         EphemeralKeyStateListener listener1 = new EphemeralKeyStateListener(mController);
         Triple<Integer, String, Instant> epskc1;
         try {
@@ -1173,7 +1205,7 @@ public class ThreadNetworkControllerTest {
                 THREAD_NETWORK_PRIVILEGED,
                 () -> registerConfigurationCallback(mController, mExecutor, callback));
         assertThat(getConfigFuture.get(CALLBACK_TIMEOUT_MILLIS, MILLISECONDS))
-                .isEqualTo(DEFAULT_CONFIG);
+                .isEqualTo(mDefaultConfig);
     }
 
     @Test
@@ -1216,7 +1248,7 @@ public class ThreadNetworkControllerTest {
             setFuture1.get(ENABLED_TIMEOUT_MILLIS, MILLISECONDS);
             setFuture2.get(ENABLED_TIMEOUT_MILLIS, MILLISECONDS);
 
-            listener.expectConfiguration(DEFAULT_CONFIG);
+            listener.expectConfiguration(mDefaultConfig);
             listener.expectConfiguration(config1);
             listener.expectConfiguration(config2);
             listener.expectNoMoreConfiguration();
diff --git a/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java b/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java
index 875a4ad116..40f0089725 100644
--- a/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java
+++ b/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java
@@ -19,7 +19,7 @@ package android.net.thread;
 import static android.net.InetAddresses.parseNumericAddress;
 import static android.net.thread.utils.IntegrationTestUtils.DEFAULT_DATASET;
 import static android.net.thread.utils.IntegrationTestUtils.buildIcmpv4EchoReply;
-import static android.net.thread.utils.IntegrationTestUtils.enableThreadAndJoinNetwork;
+import static android.net.thread.utils.IntegrationTestUtils.enableBorderRouterAndJoinNetwork;
 import static android.net.thread.utils.IntegrationTestUtils.getIpv6LinkAddresses;
 import static android.net.thread.utils.IntegrationTestUtils.isExpectedIcmpv4Packet;
 import static android.net.thread.utils.IntegrationTestUtils.isExpectedIcmpv6Packet;
@@ -128,7 +128,7 @@ public class BorderRoutingTest {
 
     @BeforeClass
     public static void beforeClass() throws Exception {
-        enableThreadAndJoinNetwork(DEFAULT_DATASET);
+        enableBorderRouterAndJoinNetwork(DEFAULT_DATASET);
     }
 
     @AfterClass
diff --git a/thread/tests/integration/src/android/net/thread/ServiceDiscoveryTest.java b/thread/tests/integration/src/android/net/thread/ServiceDiscoveryTest.java
index f959ccfe58..62f9035916 100644
--- a/thread/tests/integration/src/android/net/thread/ServiceDiscoveryTest.java
+++ b/thread/tests/integration/src/android/net/thread/ServiceDiscoveryTest.java
@@ -21,6 +21,7 @@ import static android.net.nsd.NsdManager.PROTOCOL_DNS_SD;
 import static android.net.thread.utils.IntegrationTestUtils.SERVICE_DISCOVERY_TIMEOUT;
 import static android.net.thread.utils.IntegrationTestUtils.discoverForServiceLost;
 import static android.net.thread.utils.IntegrationTestUtils.discoverService;
+import static android.net.thread.utils.IntegrationTestUtils.joinNetworkAndWait;
 import static android.net.thread.utils.IntegrationTestUtils.joinNetworkAndWaitForOmr;
 import static android.net.thread.utils.IntegrationTestUtils.resolveService;
 import static android.net.thread.utils.IntegrationTestUtils.resolveServiceUntil;
@@ -115,6 +116,8 @@ public class ServiceDiscoveryTest {
     public void setUp() throws Exception {
         mController.setEnabledAndWait(true);
         mController.leaveAndWait();
+        var config = new ThreadConfiguration.Builder().setBorderRouterEnabled(true).build();
+        mController.setConfigurationAndWait(config);
         mController.joinAndWait(DEFAULT_DATASET);
         mNsdManager = mContext.getSystemService(NsdManager.class);
 
@@ -157,6 +160,143 @@ public class ServiceDiscoveryTest {
         mController.leaveAndWait();
     }
 
+    @Test
+    public void advertisingProxy_borderRouterDisabled_clientServiceRemovedWhenLeaveIsCalled()
+            throws Exception {
+        /*
+         * <pre>
+         * Topology:
+         *                            Thread
+         *  SRP Server / AD Proxy -------------- SRP Client
+         *
+         * </pre>
+         */
+
+        // The Border Router / SRP Server mode can only be changed when Thread is disconnected
+        mController.leaveAndWait();
+        var config = new ThreadConfiguration.Builder().setBorderRouterEnabled(false).build();
+        mController.setConfigurationAndWait(config);
+        mController.joinAndWait(DEFAULT_DATASET);
+
+        FullThreadDevice srpClient = mFtds.get(0);
+        joinNetworkAndWait(srpClient, DEFAULT_DATASET);
+        srpClient.setSrpHostname("thread-srp-client-host");
+        srpClient.setSrpHostAddresses(List.of(srpClient.getMlEid()));
+        srpClient.addSrpService(
+                "thread-srp-client-service",
+                "_matter._tcp",
+                List.of("_sub1", "_sub2"),
+                12345 /* port */,
+                Map.of("key1", bytes(1), "key2", bytes(2)));
+        NsdServiceInfo discoveredService = discoverService(mNsdManager, "_matter._tcp");
+        assertThat(discoveredService).isNotNull();
+
+        CompletableFuture<NsdServiceInfo> serviceLostFuture = new CompletableFuture<>();
+        NsdManager.DiscoveryListener listener =
+                discoverForServiceLost(mNsdManager, "_matter._tcp", serviceLostFuture);
+        mController.leaveAndWait();
+
+        // Verify the service becomes lost.
+        try {
+            serviceLostFuture.get(SERVICE_DISCOVERY_TIMEOUT.toMillis(), MILLISECONDS);
+        } finally {
+            mNsdManager.stopServiceDiscovery(listener);
+        }
+        assertThrows(TimeoutException.class, () -> discoverService(mNsdManager, "_matter._tcp"));
+    }
+
+    @Test
+    public void advertisingProxy_borderRouterDisabled_clientServiceRemovedWhen2ndSrpServerEnabled()
+            throws Exception {
+        /*
+         * <pre>
+         * Topology:
+         *                            Thread
+         *  SRP Server / AD Proxy -------------- SRP Client
+         *  (Cuttlefish)                |
+         *                              +------- 2nd SRP Server
+         *
+         * </pre>
+         */
+
+        // The Border Router / SRP Server mode can only be changed when Thread is disconnected
+        mController.leaveAndWait();
+        var config = new ThreadConfiguration.Builder().setBorderRouterEnabled(false).build();
+        mController.setConfigurationAndWait(config);
+        mController.joinAndWait(DEFAULT_DATASET);
+
+        FullThreadDevice srpClient = mFtds.get(0);
+        joinNetworkAndWait(srpClient, DEFAULT_DATASET);
+        srpClient.setSrpHostname("thread-srp-client-host");
+        srpClient.setSrpHostAddresses(List.of(srpClient.getMlEid()));
+        srpClient.addSrpService(
+                "thread-srp-client-service",
+                "_matter._tcp",
+                List.of("_sub1", "_sub2"),
+                12345 /* port */,
+                Map.of("key1", bytes(1), "key2", bytes(2)));
+        NsdServiceInfo discoveredService = discoverService(mNsdManager, "_matter._tcp");
+        assertThat(discoveredService).isNotNull();
+
+        FullThreadDevice srpServer2 = mFtds.get(1);
+        joinNetworkAndWait(srpServer2, DEFAULT_DATASET);
+        CompletableFuture<NsdServiceInfo> serviceLostFuture = new CompletableFuture<>();
+        NsdManager.DiscoveryListener listener =
+                discoverForServiceLost(mNsdManager, "_matter._tcp", serviceLostFuture);
+        srpServer2.setSrpServerEnabled(true);
+
+        // Verify the service becomes lost.
+        try {
+            serviceLostFuture.get(SERVICE_DISCOVERY_TIMEOUT.toMillis(), MILLISECONDS);
+        } finally {
+            mNsdManager.stopServiceDiscovery(listener);
+        }
+        assertThrows(TimeoutException.class, () -> discoverService(mNsdManager, "_matter._tcp"));
+    }
+
+    @Test
+    public void advertisingProxy_borderRouterDisabled_clientMleIdAddressIsAdvertised()
+            throws Exception {
+        /*
+         * <pre>
+         * Topology:
+         *                            Thread
+         *  SRP Server / AD Proxy -------------- SRP Client
+         *  (Cuttlefish)
+         *
+         * </pre>
+         */
+
+        // The Border Router / SRP Server mode can only be changed when Thread is disconnected
+        mController.leaveAndWait();
+        var config = new ThreadConfiguration.Builder().setBorderRouterEnabled(false).build();
+        mController.setConfigurationAndWait(config);
+        mController.joinAndWait(DEFAULT_DATASET);
+
+        FullThreadDevice srpClient = mFtds.getFirst();
+        joinNetworkAndWait(srpClient, DEFAULT_DATASET);
+        srpClient.setSrpHostname("thread-srp-client-host");
+        srpClient.setSrpHostAddresses(List.of(srpClient.getMlEid()));
+        srpClient.addSrpService(
+                "thread-srp-client-service",
+                "_matter._tcp",
+                List.of("_sub1", "_sub2"),
+                12345 /* port */,
+                Map.of("key1", bytes(1), "key2", bytes(2)));
+
+        NsdServiceInfo discoveredService = discoverService(mNsdManager, "_matter._tcp");
+        assertThat(discoveredService).isNotNull();
+        NsdServiceInfo resolvedService = resolveService(mNsdManager, discoveredService);
+        assertThat(resolvedService.getServiceName()).isEqualTo("thread-srp-client-service");
+        assertThat(resolvedService.getServiceType()).isEqualTo("_matter._tcp");
+        assertThat(resolvedService.getPort()).isEqualTo(12345);
+        assertThat(resolvedService.getAttributes())
+                .comparingValuesUsing(BYTE_ARRAY_EQUALITY)
+                .containsExactly("key1", bytes(1), "key2", bytes(2));
+        assertThat(resolvedService.getHostname()).isEqualTo("thread-srp-client-host");
+        assertThat(resolvedService.getHostAddresses()).containsExactly(srpClient.getMlEid());
+    }
+
     @Test
     public void advertisingProxy_multipleSrpClientsRegisterServices_servicesResolvableByMdns()
             throws Exception {
diff --git a/thread/tests/integration/src/android/net/thread/ThreadIntegrationTest.java b/thread/tests/integration/src/android/net/thread/ThreadIntegrationTest.java
index 0e8f824d7f..f070a86825 100644
--- a/thread/tests/integration/src/android/net/thread/ThreadIntegrationTest.java
+++ b/thread/tests/integration/src/android/net/thread/ThreadIntegrationTest.java
@@ -17,12 +17,10 @@
 package android.net.thread;
 
 import static android.Manifest.permission.ACCESS_NETWORK_STATE;
-import static android.net.NetworkCapabilities.NET_CAPABILITY_LOCAL_NETWORK;
 import static android.net.thread.ThreadNetworkController.DEVICE_ROLE_DETACHED;
 import static android.net.thread.ThreadNetworkController.DEVICE_ROLE_LEADER;
 import static android.net.thread.ThreadNetworkController.DEVICE_ROLE_STOPPED;
 import static android.net.thread.utils.IntegrationTestUtils.CALLBACK_TIMEOUT;
-import static android.net.thread.utils.IntegrationTestUtils.DEFAULT_CONFIG;
 import static android.net.thread.utils.IntegrationTestUtils.RESTART_JOIN_TIMEOUT;
 import static android.net.thread.utils.IntegrationTestUtils.getIpv6LinkAddresses;
 import static android.net.thread.utils.IntegrationTestUtils.getPrefixesFromNetData;
@@ -56,6 +54,7 @@ import android.net.NetworkRequest;
 import android.net.thread.utils.FullThreadDevice;
 import android.net.thread.utils.OtDaemonController;
 import android.net.thread.utils.ThreadFeatureCheckerRule;
+import android.net.thread.utils.ThreadFeatureCheckerRule.RequiresSimulationThreadDevice;
 import android.net.thread.utils.ThreadFeatureCheckerRule.RequiresThreadFeature;
 import android.net.thread.utils.ThreadNetworkControllerWrapper;
 import android.net.thread.utils.ThreadStateListener;
@@ -63,13 +62,13 @@ import android.os.SystemClock;
 
 import androidx.test.core.app.ApplicationProvider;
 import androidx.test.filters.LargeTest;
-import androidx.test.runner.AndroidJUnit4;
 
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 
 import java.io.IOException;
 import java.net.DatagramPacket;
@@ -78,6 +77,7 @@ import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.time.Duration;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutorService;
@@ -86,7 +86,7 @@ import java.util.concurrent.Executors;
 /** Tests for E2E Android Thread integration with ot-daemon, ConnectivityService, etc.. */
 @LargeTest
 @RequiresThreadFeature
-@RunWith(AndroidJUnit4.class)
+@RunWith(Parameterized.class)
 public class ThreadIntegrationTest {
     // The byte[] buffer size for UDP tests
     private static final int UDP_BUFFER_SIZE = 1024;
@@ -109,8 +109,6 @@ public class ThreadIntegrationTest {
                                     + "B9D351B40C0402A0FFF8");
     private static final ActiveOperationalDataset DEFAULT_DATASET =
             ActiveOperationalDataset.fromThreadTlvs(DEFAULT_DATASET_TLVS);
-    private static final ThreadConfiguration DEFAULT_CONFIG =
-            new ThreadConfiguration.Builder().build();
 
     private static final Inet6Address GROUP_ADDR_ALL_ROUTERS =
             (Inet6Address) InetAddresses.parseNumericAddress("ff02::2");
@@ -128,11 +126,28 @@ public class ThreadIntegrationTest {
     private OtDaemonController mOtCtl;
     private FullThreadDevice mFtd;
 
+    public final boolean mIsBorderRouterEnabled;
+    private final ThreadConfiguration mConfig;
+
+    @Parameterized.Parameters
+    public static Collection configArguments() {
+        return Arrays.asList(new Object[][] {{false}, {true}});
+    }
+
+    public ThreadIntegrationTest(boolean isBorderRouterEnabled) {
+        mIsBorderRouterEnabled = isBorderRouterEnabled;
+        mConfig =
+                new ThreadConfiguration.Builder()
+                        .setBorderRouterEnabled(isBorderRouterEnabled)
+                        .build();
+    }
+
     @Before
     public void setUp() throws Exception {
         mExecutor = Executors.newSingleThreadExecutor();
         mOtCtl = new OtDaemonController();
         mController.setEnabledAndWait(true);
+        mController.setConfigurationAndWait(mConfig);
         mController.leaveAndWait();
         mFtd = new FullThreadDevice(10 /* nodeId */);
     }
@@ -143,7 +158,6 @@ public class ThreadIntegrationTest {
 
         mController.setTestNetworkAsUpstreamAndWait(null);
         mController.leaveAndWait();
-        mController.setConfigurationAndWait(DEFAULT_CONFIG);
 
         mFtd.destroy();
         mExecutor.shutdownNow();
@@ -163,6 +177,7 @@ public class ThreadIntegrationTest {
     @Test
     public void otDaemonRestart_JoinedNetworkAndStopped_autoRejoinedAndTunIfStateConsistent()
             throws Exception {
+        assumeTrue(mController.getConfiguration().isBorderRouterEnabled());
         mController.joinAndWait(DEFAULT_DATASET);
 
         runShellCommand("stop ot-daemon");
@@ -228,6 +243,7 @@ public class ThreadIntegrationTest {
     }
 
     @Test
+    @RequiresSimulationThreadDevice
     public void udp_appStartEchoServer_endDeviceUdpEchoSuccess() throws Exception {
         // Topology:
         //   Test App ------ thread-wpan ------ End Device
@@ -287,6 +303,7 @@ public class ThreadIntegrationTest {
     }
 
     @Test
+    @RequiresSimulationThreadDevice
     public void edPingsMeshLocalAddresses_oneReplyPerRequest() throws Exception {
         mController.joinAndWait(DEFAULT_DATASET);
         startFtdChild(mFtd, DEFAULT_DATASET);
@@ -361,21 +378,15 @@ public class ThreadIntegrationTest {
     }
 
     @Test
-    public void setConfiguration_disableBorderRouter_noBrfunctionsEnabled() throws Exception {
-        NetworkRequest request =
-                new NetworkRequest.Builder()
-                        .addTransportType(NetworkCapabilities.TRANSPORT_THREAD)
-                        .build();
+    @RequiresSimulationThreadDevice
+    public void setConfiguration_disableBorderRouter_borderRoutingDisabled() throws Exception {
         startFtdLeader(mFtd, DEFAULT_DATASET);
 
         mController.setConfigurationAndWait(
                 new ThreadConfiguration.Builder().setBorderRouterEnabled(false).build());
         mController.joinAndWait(DEFAULT_DATASET);
-        NetworkCapabilities caps = registerNetworkCallbackAndWait(request);
 
-        assertThat(caps.hasCapability(NET_CAPABILITY_LOCAL_NETWORK)).isFalse();
         assertThat(mOtCtl.getBorderRoutingState()).ignoringCase().isEqualTo("disabled");
-        assertThat(mOtCtl.getSrpServerState()).ignoringCase().isNotEqualTo("disabled");
         // TODO: b/376217403 - enables / disables Border Agent at runtime
     }
 
diff --git a/thread/tests/integration/src/android/net/thread/ThreadNetworkShellCommandTest.java b/thread/tests/integration/src/android/net/thread/ThreadNetworkShellCommandTest.java
index dcccbf1135..804a332ab7 100644
--- a/thread/tests/integration/src/android/net/thread/ThreadNetworkShellCommandTest.java
+++ b/thread/tests/integration/src/android/net/thread/ThreadNetworkShellCommandTest.java
@@ -34,6 +34,7 @@ import android.content.Context;
 import android.net.thread.utils.FullThreadDevice;
 import android.net.thread.utils.OtDaemonController;
 import android.net.thread.utils.ThreadFeatureCheckerRule;
+import android.net.thread.utils.ThreadFeatureCheckerRule.RequiresSimulationThreadDevice;
 import android.net.thread.utils.ThreadFeatureCheckerRule.RequiresThreadFeature;
 import android.net.thread.utils.ThreadNetworkControllerWrapper;
 
@@ -167,6 +168,7 @@ public class ThreadNetworkShellCommandTest {
     }
 
     @Test
+    @RequiresSimulationThreadDevice
     public void handleOtCtlCommand_pingFtd_getValidResponse() throws Exception {
         mController.joinAndWait(DEFAULT_DATASET);
         startFtdChild(mFtd, DEFAULT_DATASET);
diff --git a/thread/tests/integration/src/android/net/thread/utils/FullThreadDevice.java b/thread/tests/integration/src/android/net/thread/utils/FullThreadDevice.java
index 38961a33b4..ed63fd0e09 100644
--- a/thread/tests/integration/src/android/net/thread/utils/FullThreadDevice.java
+++ b/thread/tests/integration/src/android/net/thread/utils/FullThreadDevice.java
@@ -239,6 +239,12 @@ public final class FullThreadDevice {
         executeCommand("udp send %s %d %s", serverAddr.getHostAddress(), serverPort, message);
     }
 
+    /** Sets `true` to enable SRP server on this device. */
+    public void setSrpServerEnabled(boolean enabled) {
+        String cmd = enabled ? "enable" : "disable";
+        executeCommand("srp server " + cmd);
+    }
+
     /** Enables the SRP client and run in autostart mode. */
     public void autoStartSrpClient() {
         executeCommand("srp client autostart enable");
diff --git a/thread/tests/integration/src/android/net/thread/utils/IntegrationTestUtils.kt b/thread/tests/integration/src/android/net/thread/utils/IntegrationTestUtils.kt
index f00c9cdf81..773167cc72 100644
--- a/thread/tests/integration/src/android/net/thread/utils/IntegrationTestUtils.kt
+++ b/thread/tests/integration/src/android/net/thread/utils/IntegrationTestUtils.kt
@@ -582,6 +582,17 @@ object IntegrationTestUtils {
         return networkFuture[timeout.toSeconds(), TimeUnit.SECONDS]
     }
 
+    /**
+     * Let the FTD join the specified Thread network and wait for it becomes a Child or Router.
+     */
+    @JvmStatic
+    @Throws(Exception::class)
+    fun joinNetworkAndWait(ftd: FullThreadDevice, dataset: ActiveOperationalDataset) {
+        ftd.factoryReset()
+        ftd.joinNetwork(dataset)
+        ftd.waitForStateAnyOf(listOf("router", "child"), JOIN_TIMEOUT)
+    }
+
     /**
      * Let the FTD join the specified Thread network and wait for border routing to be available.
      *
@@ -613,6 +624,21 @@ object IntegrationTestUtils {
         controller.joinAndWait(dataset);
     }
 
+    /** Enables Border Router and joins the specified Thread network. */
+    @JvmStatic
+    fun enableBorderRouterAndJoinNetwork(dataset: ActiveOperationalDataset) {
+        val context: Context = requireNotNull(ApplicationProvider.getApplicationContext());
+        val controller = requireNotNull(ThreadNetworkControllerWrapper.newInstance(context));
+
+        // TODO: b/323301831 - This is a workaround to avoid unnecessary delay to re-form a network
+        controller.leaveAndWait();
+
+        controller.setEnabledAndWait(true);
+        val config = ThreadConfiguration.Builder().setBorderRouterEnabled(true).build();
+        controller.setConfigurationAndWait(config);
+        controller.joinAndWait(dataset);
+    }
+
     /** Leaves the Thread network and disables Thread. */
     @JvmStatic
     fun leaveNetworkAndDisableThread() {
diff --git a/thread/tests/integration/src/android/net/thread/utils/ThreadNetworkControllerWrapper.java b/thread/tests/integration/src/android/net/thread/utils/ThreadNetworkControllerWrapper.java
index b6114f3ec4..c4150cb103 100644
--- a/thread/tests/integration/src/android/net/thread/utils/ThreadNetworkControllerWrapper.java
+++ b/thread/tests/integration/src/android/net/thread/utils/ThreadNetworkControllerWrapper.java
@@ -233,7 +233,10 @@ public final class ThreadNetworkControllerWrapper {
     public void setNat64EnabledAndWait(boolean enabled) throws Exception {
         final ThreadConfiguration config = getConfiguration();
         final ThreadConfiguration newConfig =
-                new ThreadConfiguration.Builder(config).setNat64Enabled(enabled).build();
+                new ThreadConfiguration.Builder(config)
+                        .setBorderRouterEnabled(true)
+                        .setNat64Enabled(enabled)
+                        .build();
         setConfigurationAndWait(newConfig);
     }
 
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 bc8da8b81b..4360c7b2e3 100644
--- a/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java
+++ b/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java
@@ -42,6 +42,7 @@ import static com.google.common.io.BaseEncoding.base16;
 import static com.google.common.truth.Truth.assertThat;
 
 import static org.junit.Assert.assertThrows;
+import static org.junit.Assume.assumeTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
@@ -93,7 +94,6 @@ import android.util.AtomicFile;
 
 import androidx.test.annotation.UiThreadTest;
 import androidx.test.core.app.ApplicationProvider;
-import androidx.test.ext.junit.runners.AndroidJUnit4;
 import androidx.test.filters.SmallTest;
 
 import com.android.connectivity.resources.R;
@@ -112,6 +112,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Captor;
 import org.mockito.InOrder;
@@ -124,6 +125,8 @@ import java.time.Clock;
 import java.time.DateTimeException;
 import java.time.Instant;
 import java.time.ZoneId;
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
@@ -132,7 +135,7 @@ import java.util.concurrent.atomic.AtomicReference;
 
 /** Unit tests for {@link ThreadNetworkControllerService}. */
 @SmallTest
-@RunWith(AndroidJUnit4.class)
+@RunWith(Parameterized.class)
 // This test doesn't really need to run on the UI thread, but @Before and @Test annotated methods
 // need to run in the same thread because there are code in {@code ThreadNetworkControllerService}
 // checking that all its methods are running in the thread of the handler it's using. This is due
@@ -200,6 +203,17 @@ public final class ThreadNetworkControllerServiceTest {
     @Rule(order = 1)
     public final TemporaryFolder tempFolder = new TemporaryFolder();
 
+    private final boolean mIsBorderRouterEnabled;
+
+    @Parameterized.Parameters
+    public static Collection configArguments() {
+        return Arrays.asList(new Object[][] {{false}, {true}});
+    }
+
+    public ThreadNetworkControllerServiceTest(boolean isBorderRouterEnabled) {
+        mIsBorderRouterEnabled = isBorderRouterEnabled;
+    }
+
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
@@ -231,6 +245,8 @@ public final class ThreadNetworkControllerServiceTest {
 
         when(mConnectivityResources.get()).thenReturn(mResources);
         when(mResources.getBoolean(eq(R.bool.config_thread_default_enabled))).thenReturn(true);
+        when(mResources.getBoolean(eq(R.bool.config_thread_border_router_default_enabled)))
+                .thenReturn(mIsBorderRouterEnabled);
         when(mResources.getBoolean(
                         eq(R.bool.config_thread_srp_server_wait_for_border_routing_enabled)))
                 .thenReturn(true);
@@ -920,7 +936,9 @@ public final class ThreadNetworkControllerServiceTest {
     }
 
     @Test
-    public void initialize_upstreamNetworkRequestHasCertainTransportTypesAndCapabilities() {
+    public void initialize_borderRouterEnabled_upstreamNetworkRequestHasExpectedTransportAndCaps() {
+        assumeTrue(mIsBorderRouterEnabled);
+
         mService.initialize();
         mTestLooper.dispatchAll();
 
@@ -999,7 +1017,8 @@ public final class ThreadNetworkControllerServiceTest {
     }
 
     @Test
-    public void activateEphemeralKeyMode_succeed() throws Exception {
+    public void activateEphemeralKeyMode_borderRouterEnabled_succeed() throws Exception {
+        assumeTrue(mIsBorderRouterEnabled);
         mService.initialize();
         final IOperationReceiver mockReceiver = mock(IOperationReceiver.class);
 
@@ -1010,7 +1029,8 @@ public final class ThreadNetworkControllerServiceTest {
     }
 
     @Test
-    public void deactivateEphemeralKeyMode_succeed() throws Exception {
+    public void deactivateEphemeralKeyMode_borderRouterEnabled_succeed() throws Exception {
+        assumeTrue(mIsBorderRouterEnabled);
         mService.initialize();
         final IOperationReceiver mockReceiver = mock(IOperationReceiver.class);
 

From d9bbec2fa134004d211f4c86b8aa11082d0cc737 Mon Sep 17 00:00:00 2001
From: Kangping Dong <wgtdkp@google.com>
Date: Mon, 10 Mar 2025 14:14:06 +0800
Subject: [PATCH 2/2] [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
---
 .../ThreadNetworkControllerService.java       |   4 +-
 .../thread/ThreadNetworkCountryCode.java      |   7 +-
 .../thread/ThreadPersistentSettings.java      | 140 ++++++++----------
 .../ThreadNetworkControllerServiceTest.java   |   3 +-
 .../thread/ThreadNetworkCountryCodeTest.java  |   4 +-
 .../thread/ThreadPersistentSettingsTest.java  |  58 +++++---
 6 files changed, 104 insertions(+), 112 deletions(-)

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<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);
             }
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<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();
     }