From 839e10f8010d22ec22761b956370e162fe1b8f06 Mon Sep 17 00:00:00 2001 From: Zekan Qian Date: Mon, 16 Jan 2023 13:55:20 +0800 Subject: [PATCH] Add isEnabled API in SPP. API isEnabled is used to specify whether the page is enabled during migration. Once it is disabled, all its functionalities, such as browsing / search / slice provider are disabled. Bug: 244122804 Test: unit-test & local build gallery Change-Id: I4f3f83e314b624fd2f6f83e0d1f64f426efb0350 --- .../spa/framework/BrowseActivity.kt | 19 ++-- .../spa/framework/common/SettingsEntry.kt | 6 +- .../spa/framework/common/SettingsPage.kt | 6 ++ .../framework/common/SettingsPageProvider.kt | 8 ++ .../spa/framework/util/PageLogger.kt | 4 +- .../spa/framework/BrowseActivityTest.kt | 23 ++++- .../spa/framework/common/SettingsEntryTest.kt | 97 +++++++++++++------ .../slice/SettingsSliceDataRepositoryTest.kt | 5 + .../tests/testutils/SpaEnvironmentForTest.kt | 19 +++- 9 files changed, 137 insertions(+), 50 deletions(-) diff --git a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/BrowseActivity.kt b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/BrowseActivity.kt index a81e2e330b0f4..4d8b89bf12aa6 100644 --- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/BrowseActivity.kt +++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/BrowseActivity.kt @@ -49,7 +49,7 @@ import com.android.settingslib.spa.framework.compose.composable import com.android.settingslib.spa.framework.compose.localNavController import com.android.settingslib.spa.framework.compose.rememberAnimatedNavController import com.android.settingslib.spa.framework.theme.SettingsTheme -import com.android.settingslib.spa.framework.util.PageEvent +import com.android.settingslib.spa.framework.util.PageWithEvent import com.android.settingslib.spa.framework.util.getDestination import com.android.settingslib.spa.framework.util.getEntryId import com.android.settingslib.spa.framework.util.getSessionName @@ -118,32 +118,25 @@ private fun NavControllerWrapperImpl.NavContent(allProvider: Collection - spp.PageEvent(navBackStackEntry.arguments) - spp.Page(navBackStackEntry.arguments) - } + ) { navBackStackEntry -> spp.PageWithEvent(navBackStackEntry.arguments) } } } } diff --git a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsEntry.kt b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsEntry.kt index 0871304fa873e..2175e55cd49e3 100644 --- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsEntry.kt +++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsEntry.kt @@ -185,6 +185,8 @@ class SettingsEntryBuilder(private val name: String, private val owner: Settings private var sliceDataFn: SliceDataGetter = { _: Uri, _: Bundle? -> null } fun build(): SettingsEntry { + val page = fromPage ?: owner + val isEnabled = page.isEnabled() return SettingsEntry( id = id(), name = name, @@ -196,10 +198,10 @@ class SettingsEntryBuilder(private val name: String, private val owner: Settings toPage = toPage, // attributes - isAllowSearch = isAllowSearch, + isAllowSearch = isEnabled && isAllowSearch, isSearchDataDynamic = isSearchDataDynamic, hasMutableStatus = hasMutableStatus, - hasSliceSupport = hasSliceSupport, + hasSliceSupport = isEnabled && hasSliceSupport, // functions statusDataImpl = statusDataFn, diff --git a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsPage.kt b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsPage.kt index 2bfa2a4375da7..a362877fe88cd 100644 --- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsPage.kt +++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsPage.kt @@ -94,6 +94,12 @@ data class SettingsPage( return !isCreateBy(NULL_PAGE_NAME) && !hasRuntimeParam() } + + fun isEnabled(): Boolean { + if (!SpaEnvironmentFactory.isReady()) return false + val pageProviderRepository by SpaEnvironmentFactory.instance.pageProviderRepository + return pageProviderRepository.getProviderOrNull(sppName)?.isEnabled(arguments) ?: false + } } fun SettingsPageProvider.createSettingsPage(arguments: Bundle? = null): SettingsPage { diff --git a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsPageProvider.kt b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsPageProvider.kt index 940005d0c514a..42e5f7ed6aae5 100644 --- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsPageProvider.kt +++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsPageProvider.kt @@ -37,6 +37,14 @@ interface SettingsPageProvider { val parameter: List get() = emptyList() + /** + * The API to indicate whether the page is enabled or not. + * During SPA page migration, one can use it to enable certain pages in one release. + * When the page is disabled, all its related functionalities, such as browsing, search, + * slice provider, are disabled as well. + */ + fun isEnabled(arguments: Bundle?): Boolean = true + fun getTitle(arguments: Bundle?): String = displayName fun buildEntry(arguments: Bundle?): List = emptyList() diff --git a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/util/PageLogger.kt b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/util/PageLogger.kt index 73eae07a4ba9d..22a4563748d5a 100644 --- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/util/PageLogger.kt +++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/util/PageLogger.kt @@ -33,13 +33,15 @@ import com.android.settingslib.spa.framework.compose.LocalNavController import com.android.settingslib.spa.framework.compose.NavControllerWrapper @Composable -internal fun SettingsPageProvider.PageEvent(arguments: Bundle? = null) { +internal fun SettingsPageProvider.PageWithEvent(arguments: Bundle? = null) { + if (!isEnabled(arguments)) return val page = remember(arguments) { createSettingsPage(arguments) } val navController = LocalNavController.current LifecycleEffect( onStart = { page.logPageEvent(LogEvent.PAGE_ENTER, navController) }, onStop = { page.logPageEvent(LogEvent.PAGE_LEAVE, navController) }, ) + Page(arguments) } private fun SettingsPage.logPageEvent(event: LogEvent, navController: NavControllerWrapper) { diff --git a/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/framework/BrowseActivityTest.kt b/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/framework/BrowseActivityTest.kt index bd5884d6f2d90..218f5691e8819 100644 --- a/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/framework/BrowseActivityTest.kt +++ b/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/framework/BrowseActivityTest.kt @@ -30,6 +30,7 @@ import com.android.settingslib.spa.framework.common.SpaEnvironmentFactory import com.android.settingslib.spa.framework.common.createSettingsPage import com.android.settingslib.spa.tests.testutils.SpaEnvironmentForTest import com.android.settingslib.spa.tests.testutils.SpaLoggerForTest +import com.android.settingslib.spa.tests.testutils.SppDisabled import com.android.settingslib.spa.tests.testutils.SppHome import com.android.settingslib.spa.testutils.waitUntil import com.google.common.truth.Truth @@ -46,12 +47,12 @@ class BrowseActivityTest { private val context: Context = ApplicationProvider.getApplicationContext() private val spaLogger = SpaLoggerForTest() - private val spaEnvironment = - SpaEnvironmentForTest(context, listOf(SppHome.createSettingsPage()), logger = spaLogger) @Test fun testBrowsePage() { spaLogger.reset() + val spaEnvironment = + SpaEnvironmentForTest(context, listOf(SppHome.createSettingsPage()), logger = spaLogger) SpaEnvironmentFactory.reset(spaEnvironment) val sppRepository by spaEnvironment.pageProviderRepository @@ -75,6 +76,24 @@ class BrowseActivityTest { spaLogger.verifyPageEvent(pageHome.id, 1, 1) spaLogger.verifyPageEvent(pageLayer1.id, 1, 0) } + + @Test + fun testBrowseDisabledPage() { + spaLogger.reset() + val spaEnvironment = SpaEnvironmentForTest( + context, listOf(SppDisabled.createSettingsPage()), logger = spaLogger + ) + SpaEnvironmentFactory.reset(spaEnvironment) + + val sppRepository by spaEnvironment.pageProviderRepository + val sppDisabled = sppRepository.getProviderOrNull("SppDisabled")!! + val pageDisabled = sppDisabled.createSettingsPage() + + composeTestRule.setContent { BrowseContent(sppRepository) } + + composeTestRule.onNodeWithText(sppDisabled.getTitle(null)).assertDoesNotExist() + spaLogger.verifyPageEvent(pageDisabled.id, 0, 0) + } } private fun SpaLoggerForTest.verifyPageEvent(id: String, entryCount: Int, leaveCount: Int) { diff --git a/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/framework/common/SettingsEntryTest.kt b/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/framework/common/SettingsEntryTest.kt index b600ac6cd322f..6de1ae58a3c1d 100644 --- a/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/framework/common/SettingsEntryTest.kt +++ b/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/framework/common/SettingsEntryTest.kt @@ -16,13 +16,16 @@ package com.android.settingslib.spa.framework.common +import android.content.Context import android.net.Uri import androidx.compose.runtime.Composable import androidx.compose.ui.test.junit4.createComposeRule import androidx.core.os.bundleOf +import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.android.settingslib.spa.slice.appendSpaParams import com.android.settingslib.spa.slice.getEntryId +import com.android.settingslib.spa.tests.testutils.SpaEnvironmentForTest import com.android.settingslib.spa.tests.testutils.getUniqueEntryId import com.android.settingslib.spa.tests.testutils.getUniquePageId import com.google.common.truth.Truth.assertThat @@ -53,6 +56,9 @@ class MacroForTest(private val pageId: String, private val entryId: String) : En @RunWith(AndroidJUnit4::class) class SettingsEntryTest { + private val context: Context = ApplicationProvider.getApplicationContext() + private val spaEnvironment = SpaEnvironmentForTest(context) + @get:Rule val composeTestRule = createComposeRule() @@ -77,15 +83,15 @@ class SettingsEntryTest { val owner = SettingsPage.create("mySpp") val fromPage = SettingsPage.create("fromSpp") val toPage = SettingsPage.create("toSpp") - val entryFrom = SettingsEntryBuilder.createLinkFrom("myEntry", owner) - .setLink(toPage = toPage).build() + val entryFrom = + SettingsEntryBuilder.createLinkFrom("myEntry", owner).setLink(toPage = toPage).build() assertThat(entryFrom.id).isEqualTo(getUniqueEntryId("myEntry", owner, owner, toPage)) assertThat(entryFrom.displayName).isEqualTo("myEntry") assertThat(entryFrom.fromPage!!.sppName).isEqualTo("mySpp") assertThat(entryFrom.toPage!!.sppName).isEqualTo("toSpp") - val entryTo = SettingsEntryBuilder.createLinkTo("myEntry", owner) - .setLink(fromPage = fromPage).build() + val entryTo = + SettingsEntryBuilder.createLinkTo("myEntry", owner).setLink(fromPage = fromPage).build() assertThat(entryTo.id).isEqualTo(getUniqueEntryId("myEntry", owner, fromPage, owner)) assertThat(entryTo.displayName).isEqualTo("myEntry") assertThat(entryTo.fromPage!!.sppName).isEqualTo("fromSpp") @@ -98,9 +104,7 @@ class SettingsEntryTest { val entryInject = SettingsEntryBuilder.createInject(owner).build() assertThat(entryInject.id).isEqualTo( getUniqueEntryId( - INJECT_ENTRY_NAME_TEST, - owner, - toPage = owner + INJECT_ENTRY_NAME_TEST, owner, toPage = owner ) ) assertThat(entryInject.displayName).isEqualTo("${INJECT_ENTRY_NAME_TEST}_mySpp") @@ -114,9 +118,7 @@ class SettingsEntryTest { val entryInject = SettingsEntryBuilder.createRoot(owner, "myRootEntry").build() assertThat(entryInject.id).isEqualTo( getUniqueEntryId( - ROOT_ENTRY_NAME_TEST, - owner, - toPage = owner + ROOT_ENTRY_NAME_TEST, owner, toPage = owner ) ) assertThat(entryInject.displayName).isEqualTo("myRootEntry") @@ -126,13 +128,15 @@ class SettingsEntryTest { @Test fun testSetAttributes() { - val owner = SettingsPage.create("mySpp") - val entryBuilder = SettingsEntryBuilder.create(owner, "myEntry") - .setDisplayName("myEntryDisplay") - .setIsSearchDataDynamic(false) - .setHasMutableStatus(true) - .setSearchDataFn { null } - .setSliceDataFn { _, _ -> null } + SpaEnvironmentFactory.reset(spaEnvironment) + val owner = SettingsPage.create("SppHome") + val entryBuilder = + SettingsEntryBuilder.create(owner, "myEntry") + .setDisplayName("myEntryDisplay") + .setIsSearchDataDynamic(false) + .setHasMutableStatus(true) + .setSearchDataFn { null } + .setSliceDataFn { _, _ -> null } val entry = entryBuilder.build() assertThat(entry.id).isEqualTo(getUniqueEntryId("myEntry", owner)) assertThat(entry.displayName).isEqualTo("myEntryDisplay") @@ -143,21 +147,52 @@ class SettingsEntryTest { assertThat(entry.hasMutableStatus).isTrue() assertThat(entry.hasSliceSupport).isTrue() + // Test disabled Spp + val ownerDisabled = SettingsPage.create("SppDisabled") + val entryBuilderDisabled = + SettingsEntryBuilder.create(ownerDisabled, "myEntry") + .setDisplayName("myEntryDisplay") + .setIsSearchDataDynamic(false) + .setHasMutableStatus(true) + .setSearchDataFn { null } + .setSliceDataFn { _, _ -> null } + val entryDisabled = entryBuilderDisabled.build() + assertThat(entryDisabled.id).isEqualTo(getUniqueEntryId("myEntry", ownerDisabled)) + assertThat(entryDisabled.displayName).isEqualTo("myEntryDisplay") + assertThat(entryDisabled.fromPage).isNull() + assertThat(entryDisabled.toPage).isNull() + assertThat(entryDisabled.isAllowSearch).isFalse() + assertThat(entryDisabled.isSearchDataDynamic).isFalse() + assertThat(entryDisabled.hasMutableStatus).isTrue() + assertThat(entryDisabled.hasSliceSupport).isFalse() + + // Clear search data fn val entry2 = entryBuilder.clearSearchDataFn().build() assertThat(entry2.isAllowSearch).isFalse() + + // Clear SppHome in spa environment + SpaEnvironmentFactory.reset() + val entry3 = entryBuilder.build() + assertThat(entry3.id).isEqualTo(getUniqueEntryId("myEntry", owner)) + assertThat(entry3.displayName).isEqualTo("myEntryDisplay") + assertThat(entry3.fromPage).isNull() + assertThat(entry3.toPage).isNull() + assertThat(entry3.isAllowSearch).isFalse() + assertThat(entry3.isSearchDataDynamic).isFalse() + assertThat(entry3.hasMutableStatus).isTrue() + assertThat(entry3.hasSliceSupport).isFalse() } @Test fun testSetMarco() { - val owner = SettingsPage.create("mySpp", arguments = bundleOf("param" to "v1")) - val entry = SettingsEntryBuilder.create(owner, "myEntry") - .setMacro { - assertThat(it?.getString("param")).isEqualTo("v1") - assertThat(it?.getString("rtParam")).isEqualTo("v2") - assertThat(it?.getString("unknown")).isNull() - MacroForTest(getUniquePageId("mySpp"), getUniqueEntryId("myEntry", owner)) - } - .build() + SpaEnvironmentFactory.reset(spaEnvironment) + val owner = SettingsPage.create("SppHome", arguments = bundleOf("param" to "v1")) + val entry = SettingsEntryBuilder.create(owner, "myEntry").setMacro { + assertThat(it?.getString("param")).isEqualTo("v1") + assertThat(it?.getString("rtParam")).isEqualTo("v2") + assertThat(it?.getString("unknown")).isNull() + MacroForTest(getUniquePageId("SppHome"), getUniqueEntryId("myEntry", owner)) + }.build() val rtArguments = bundleOf("rtParam" to "v2") composeTestRule.setContent { entry.UiLayout(rtArguments) } @@ -175,14 +210,14 @@ class SettingsEntryTest { @Test fun testSetSliceDataFn() { - val owner = SettingsPage.create("mySpp") + SpaEnvironmentFactory.reset(spaEnvironment) + val owner = SettingsPage.create("SppHome") val entryId = getUniqueEntryId("myEntry", owner) val emptySliceData = EntrySliceData() - val entryBuilder = SettingsEntryBuilder.create(owner, "myEntry") - .setSliceDataFn { uri, _ -> - return@setSliceDataFn if (uri.getEntryId() == entryId) emptySliceData else null - } + val entryBuilder = SettingsEntryBuilder.create(owner, "myEntry").setSliceDataFn { uri, _ -> + return@setSliceDataFn if (uri.getEntryId() == entryId) emptySliceData else null + } val entry = entryBuilder.build() assertThat(entry.id).isEqualTo(entryId) assertThat(entry.hasSliceSupport).isTrue() diff --git a/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/slice/SettingsSliceDataRepositoryTest.kt b/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/slice/SettingsSliceDataRepositoryTest.kt index 1bdba299dc98b..530d2ed575621 100644 --- a/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/slice/SettingsSliceDataRepositoryTest.kt +++ b/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/slice/SettingsSliceDataRepositoryTest.kt @@ -23,6 +23,7 @@ import androidx.lifecycle.Observer import androidx.slice.Slice import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.android.settingslib.spa.framework.common.SpaEnvironmentFactory import com.android.settingslib.spa.framework.common.createSettingsPage import com.android.settingslib.spa.tests.testutils.SpaEnvironmentForTest import com.android.settingslib.spa.tests.testutils.SppHome @@ -44,6 +45,8 @@ class SettingsSliceDataRepositoryTest { @Test fun getOrBuildSliceDataTest() { + SpaEnvironmentFactory.reset(spaEnvironment) + // Slice empty assertThat(sliceDataRepository.getOrBuildSliceData(Uri.EMPTY)).isNull() @@ -67,6 +70,8 @@ class SettingsSliceDataRepositoryTest { @Test fun getActiveSliceDataTest() { + SpaEnvironmentFactory.reset(spaEnvironment) + val page = SppLayer2.createSettingsPage() val entryId = getUniqueEntryId("Layer2Entry1", page) val sliceUri = Uri.Builder().appendSpaParams(page.buildRoute(), entryId).build() diff --git a/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/tests/testutils/SpaEnvironmentForTest.kt b/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/tests/testutils/SpaEnvironmentForTest.kt index f38bd088060a7..2755b4e181548 100644 --- a/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/tests/testutils/SpaEnvironmentForTest.kt +++ b/packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/tests/testutils/SpaEnvironmentForTest.kt @@ -92,6 +92,23 @@ object SppHome : SettingsPageProvider { } } +object SppDisabled : SettingsPageProvider { + override val name = "SppDisabled" + + override fun isEnabled(arguments: Bundle?): Boolean = false + + override fun getTitle(arguments: Bundle?): String { + return "TitleDisabled" + } + + override fun buildEntry(arguments: Bundle?): List { + val owner = this.createSettingsPage() + return listOf( + SppLayer1.buildInject().setLink(fromPage = owner).build(), + ) + } +} + object SppLayer1 : SettingsPageProvider { override val name = "SppLayer1" @@ -190,7 +207,7 @@ class SpaEnvironmentForTest( SettingsPageProviderRepository( listOf( SppHome, SppLayer1, SppLayer2, - SppForSearch, + SppForSearch, SppDisabled, object : SettingsPageProvider { override val name = "SppWithParam" override val parameter = listOf(