From ff9dd81c55e823ac649ba55cfa3e5c4a39320943 Mon Sep 17 00:00:00 2001 From: Lucas Kivi <125697099+lucas-livefront@users.noreply.github.com> Date: Thu, 11 Jan 2024 13:22:41 -0600 Subject: [PATCH] Refactor autofill flow to partition fill data by cipher (#571) --- .../builder/FillResponseBuilderImpl.kt | 51 +++--- .../autofill/builder/FilledDataBuilderImpl.kt | 71 ++++++-- .../data/autofill/di/AutofillModule.kt | 4 +- .../data/autofill/model/FilledData.kt | 5 +- .../data/autofill/model/FilledPartition.kt | 11 ++ .../autofill/util/FilledItemExtensions.kt | 53 ++---- .../util/FilledPartitionExtensions.kt | 76 +++++++++ .../ui/autofill/BitwardenRemoteViews.kt | 5 +- .../builder/FillResponseBuilderTest.kt | 98 +++++------ .../autofill/builder/FilledDataBuilderTest.kt | 50 +++++- .../processor/AutofillProcessorTest.kt | 4 +- .../autofill/util/FilledItemExtensionsTest.kt | 36 +--- .../util/FilledPartitionExtensionsTest.kt | 155 ++++++++++++++++++ .../ui/autofill/BitwardenRemoteViewsTest.kt | 16 +- 14 files changed, 442 insertions(+), 193 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/autofill/model/FilledPartition.kt create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/autofill/util/FilledPartitionExtensions.kt create mode 100644 app/src/test/java/com/x8bit/bitwarden/data/autofill/util/FilledPartitionExtensionsTest.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FillResponseBuilderImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FillResponseBuilderImpl.kt index 45b8078cf..2a9bbc23f 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FillResponseBuilderImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FillResponseBuilderImpl.kt @@ -1,11 +1,9 @@ package com.x8bit.bitwarden.data.autofill.builder -import android.service.autofill.Dataset import android.service.autofill.FillResponse import com.x8bit.bitwarden.data.autofill.model.AutofillAppInfo import com.x8bit.bitwarden.data.autofill.model.FilledData -import com.x8bit.bitwarden.data.autofill.util.applyOverlayToDataset -import com.x8bit.bitwarden.ui.autofill.buildAutofillRemoteViews +import com.x8bit.bitwarden.data.autofill.util.buildDataset /** * The default implementation for [FillResponseBuilder]. This is a component for compiling fulfilled @@ -16,32 +14,35 @@ class FillResponseBuilderImpl : FillResponseBuilder { autofillAppInfo: AutofillAppInfo, filledData: FilledData, ): FillResponse? = - if (filledData.filledItems.isNotEmpty()) { - val remoteViewsPlaceholder = buildAutofillRemoteViews( - packageName = autofillAppInfo.packageName, - context = autofillAppInfo.context, - ) - val datasetBuilder = Dataset.Builder() + if (filledData.filledPartitions.any { it.filledItems.isNotEmpty() }) { + val fillResponseBuilder = FillResponse.Builder() - // Set UI for each valid autofill view. - filledData.filledItems.forEach { filledItem -> - filledItem.applyOverlayToDataset( - appInfo = autofillAppInfo, - datasetBuilder = datasetBuilder, - remoteViews = remoteViewsPlaceholder, - ) - } - val dataset = datasetBuilder.build() - FillResponse.Builder() - .addDataset(dataset) + filledData + .filledPartitions + .forEach { filledPartition -> + // It won't be empty but we really don't want to make an empty dataset, + // it causes a crash. + if (filledPartition.filledItems.isNotEmpty()) { + // We build a dataset for each filled partition. A filled partition is a + // copy of all the views that we are going to fill, loaded with the data + // from one of the ciphers that can fulfill this partition type. + val dataset = filledPartition.buildDataset( + autofillAppInfo = autofillAppInfo, + ) + + // Load the dataset into the fill request. + fillResponseBuilder.addDataset(dataset) + } + } + fillResponseBuilder .setIgnoredIds(*filledData.ignoreAutofillIds.toTypedArray()) .build() } else { - // It is impossible for an `AutofillPartition` to be empty due to the way it is - // constructed. However, the [FillRequest] requires at least one dataset or an - // authentication intent with a presentation view. Neither of these make sense in the - // case where we have no views to fill. What we are supposed to do when we cannot - // fulfill a request is replace [FillResponse] with null in order to avoid this crash. + // It is impossible for [filledData] to be empty due to the way it is constructed. + // However, the [FillRequest] requires at least one dataset or an authentication intent + // with a presentation view. Neither of these make sense in the case where we have no + // views to fill. What we are supposed to do when we cannot fulfill a request is + // replace [FillResponse] with null in order to avoid this crash. null } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt index 843b85277..0d5369bbf 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt @@ -1,9 +1,11 @@ package com.x8bit.bitwarden.data.autofill.builder +import com.x8bit.bitwarden.data.autofill.model.AutofillPartition import com.x8bit.bitwarden.data.autofill.model.AutofillRequest import com.x8bit.bitwarden.data.autofill.model.AutofillView import com.x8bit.bitwarden.data.autofill.model.FilledData import com.x8bit.bitwarden.data.autofill.model.FilledItem +import com.x8bit.bitwarden.data.autofill.model.FilledPartition /** * The default [FilledDataBuilder]. This converts parsed autofill data into filled data that is @@ -13,24 +15,65 @@ class FilledDataBuilderImpl : FilledDataBuilder { override suspend fun build(autofillRequest: AutofillRequest.Fillable): FilledData { // TODO: determine whether or not the vault is locked (BIT-1296) - val filledItems = autofillRequest - .partition - .views - .map(AutofillView::toFilledItem) + val filledPartitions = when (autofillRequest.partition) { + is AutofillPartition.Card -> { + // TODO: perform fulfillment with dummy data (BIT-1315) + listOf( + fillCardPartition( + autofillViews = autofillRequest.partition.views, + ), + ) + } - // TODO: perform fulfillment with dummy data (BIT-1315) + is AutofillPartition.Login -> { + // TODO: perform fulfillment with dummy data (BIT-1315) + listOf( + fillLoginPartition( + autofillViews = autofillRequest.partition.views, + ), + ) + } + } return FilledData( - filledItems = filledItems, + filledPartitions = filledPartitions, ignoreAutofillIds = autofillRequest.ignoreAutofillIds, ) } -} -/** - * Map this [AutofillView] to a [FilledItem]. - */ -private fun AutofillView.toFilledItem(): FilledItem = - FilledItem( - autofillId = autofillId, - ) + /** + * Construct a [FilledPartition] by fulfilling the card [autofillViews] with data. + */ + private fun fillCardPartition( + autofillViews: List, + ): FilledPartition { + val filledItems = autofillViews + .map { autofillView -> + FilledItem( + autofillId = autofillView.autofillId, + ) + } + + return FilledPartition( + filledItems = filledItems, + ) + } + + /** + * Construct a [FilledPartition] by fulfilling the login [autofillViews] with data. + */ + private fun fillLoginPartition( + autofillViews: List, + ): FilledPartition { + val filledItems = autofillViews + .map { autofillView -> + FilledItem( + autofillId = autofillView.autofillId, + ) + } + + return FilledPartition( + filledItems = filledItems, + ) + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt index 63fbaf55f..095bea150 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt @@ -1,9 +1,9 @@ package com.x8bit.bitwarden.data.autofill.di -import com.x8bit.bitwarden.data.autofill.builder.FilledDataBuilder -import com.x8bit.bitwarden.data.autofill.builder.FilledDataBuilderImpl import com.x8bit.bitwarden.data.autofill.builder.FillResponseBuilder import com.x8bit.bitwarden.data.autofill.builder.FillResponseBuilderImpl +import com.x8bit.bitwarden.data.autofill.builder.FilledDataBuilder +import com.x8bit.bitwarden.data.autofill.builder.FilledDataBuilderImpl import com.x8bit.bitwarden.data.autofill.parser.AutofillParser import com.x8bit.bitwarden.data.autofill.parser.AutofillParserImpl import com.x8bit.bitwarden.data.autofill.processor.AutofillProcessor diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/model/FilledData.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/model/FilledData.kt index 8c4adb30f..64cea126f 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/model/FilledData.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/model/FilledData.kt @@ -3,9 +3,10 @@ package com.x8bit.bitwarden.data.autofill.model import android.view.autofill.AutofillId /** - * The fulfilled autofill data to be loaded into the a fill response. + * A fulfilled autofill dataset. This is all of the data to fulfill each view of the autofill + * request for a given cipher. */ data class FilledData( - val filledItems: List, + val filledPartitions: List, val ignoreAutofillIds: List, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/model/FilledPartition.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/model/FilledPartition.kt new file mode 100644 index 000000000..ee41403f3 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/model/FilledPartition.kt @@ -0,0 +1,11 @@ +package com.x8bit.bitwarden.data.autofill.model + +/** + * All of the data required to build a `Dataset` for fulfilling a partition of data based on a + * cipher. + * + * @param filledItems A filled copy of each view from this partition. + */ +data class FilledPartition( + val filledItems: List, +) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/util/FilledItemExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/util/FilledItemExtensions.kt index 2a75d55d5..62399acd1 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/util/FilledItemExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/util/FilledItemExtensions.kt @@ -1,73 +1,40 @@ package com.x8bit.bitwarden.data.autofill.util import android.annotation.SuppressLint -import android.os.Build import android.service.autofill.Dataset import android.service.autofill.Field import android.service.autofill.Presentations -import android.view.autofill.AutofillId import android.widget.RemoteViews -import com.x8bit.bitwarden.data.autofill.model.AutofillAppInfo import com.x8bit.bitwarden.data.autofill.model.FilledItem /** - * Apply this [FilledItem] to the dataset being built by [datasetBuilder] in the form of an - * overlay presentation. - */ -fun FilledItem.applyOverlayToDataset( - appInfo: AutofillAppInfo, - datasetBuilder: Dataset.Builder, - remoteViews: RemoteViews, -) { - if (appInfo.sdkInt >= Build.VERSION_CODES.TIRAMISU) { - setOverlay( - autoFillId = autofillId, - datasetBuilder = datasetBuilder, - remoteViews = remoteViews, - ) - } else { - setOverlayPreTiramisu( - autoFillId = autofillId, - datasetBuilder = datasetBuilder, - remoteViews = remoteViews, - ) - } -} - -/** - * Set up an overlay presentation in the [datasetBuilder] for Android devices running on API - * Tiramisu or greater. + * Set up an overlay presentation for this [FilledItem] in the [datasetBuilder] for Android devices + * running on API Tiramisu or greater. */ @SuppressLint("NewApi") -private fun setOverlay( - autoFillId: AutofillId, +fun FilledItem.applyToDatasetPostTiramisu( datasetBuilder: Dataset.Builder, - remoteViews: RemoteViews, + presentations: Presentations, ) { - val presentation = Presentations.Builder() - .setMenuPresentation(remoteViews) - .build() - datasetBuilder.setField( - autoFillId, + autofillId, Field.Builder() - .setPresentations(presentation) + .setPresentations(presentations) .build(), ) } /** - * Set up an overlay presentation in the [datasetBuilder] for Android devices running on APIs that - * predate Tiramisu. + * Set up an overlay presentation for this [FilledItem] in the [datasetBuilder] for Android devices + * running on APIs that predate Tiramisu. */ @Suppress("Deprecation") -private fun setOverlayPreTiramisu( - autoFillId: AutofillId, +fun FilledItem.applyToDatasetPreTiramisu( datasetBuilder: Dataset.Builder, remoteViews: RemoteViews, ) { datasetBuilder.setValue( - autoFillId, + autofillId, null, remoteViews, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/util/FilledPartitionExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/util/FilledPartitionExtensions.kt new file mode 100644 index 000000000..359f400c1 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/util/FilledPartitionExtensions.kt @@ -0,0 +1,76 @@ +package com.x8bit.bitwarden.data.autofill.util + +import android.annotation.SuppressLint +import android.os.Build +import android.service.autofill.Dataset +import android.service.autofill.Presentations +import android.widget.RemoteViews +import com.x8bit.bitwarden.R +import com.x8bit.bitwarden.data.autofill.model.AutofillAppInfo +import com.x8bit.bitwarden.data.autofill.model.FilledPartition +import com.x8bit.bitwarden.ui.autofill.buildAutofillRemoteViews + +/** + * Build a [Dataset] to represent the [FilledPartition]. This dataset includes an overlay UI + * presentation for each filled item. + */ +fun FilledPartition.buildDataset( + autofillAppInfo: AutofillAppInfo, +): Dataset { + val remoteViewsPlaceholder = buildAutofillRemoteViews( + packageName = autofillAppInfo.packageName, + title = autofillAppInfo.context.resources.getString(R.string.app_name), + ) + val datasetBuilder = Dataset.Builder() + + if (autofillAppInfo.sdkInt >= Build.VERSION_CODES.TIRAMISU) { + applyToDatasetPostTiramisu( + datasetBuilder = datasetBuilder, + remoteViews = remoteViewsPlaceholder, + ) + } else { + buildDatasetPreTiramisu( + datasetBuilder = datasetBuilder, + remoteViews = remoteViewsPlaceholder, + ) + } + + return datasetBuilder.build() +} + +/** + * Apply this [FilledPartition] to the [datasetBuilder] on devices running OS version Tiramisu or + * greater. + */ +@SuppressLint("NewApi") +private fun FilledPartition.applyToDatasetPostTiramisu( + datasetBuilder: Dataset.Builder, + remoteViews: RemoteViews, +) { + val presentation = Presentations.Builder() + .setMenuPresentation(remoteViews) + .build() + + filledItems.forEach { filledItem -> + filledItem.applyToDatasetPostTiramisu( + datasetBuilder = datasetBuilder, + presentations = presentation, + ) + } +} + +/** + * Apply this [FilledPartition] to the [datasetBuilder] on devices running OS versions that predate + * Tiramisu. + */ +private fun FilledPartition.buildDatasetPreTiramisu( + datasetBuilder: Dataset.Builder, + remoteViews: RemoteViews, +) { + filledItems.forEach { filledItem -> + filledItem.applyToDatasetPreTiramisu( + datasetBuilder = datasetBuilder, + remoteViews = remoteViews, + ) + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/autofill/BitwardenRemoteViews.kt b/app/src/main/java/com/x8bit/bitwarden/ui/autofill/BitwardenRemoteViews.kt index 3ba728667..8ccea2cb4 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/autofill/BitwardenRemoteViews.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/autofill/BitwardenRemoteViews.kt @@ -1,6 +1,5 @@ package com.x8bit.bitwarden.ui.autofill -import android.content.Context import android.widget.RemoteViews import com.x8bit.bitwarden.R @@ -8,8 +7,8 @@ import com.x8bit.bitwarden.R * Build [RemoteViews] for representing an autofill suggestion. */ fun buildAutofillRemoteViews( - context: Context, packageName: String, + title: String, ): RemoteViews = RemoteViews( packageName, @@ -18,6 +17,6 @@ fun buildAutofillRemoteViews( .apply { setTextViewText( R.id.text, - context.resources.getText(R.string.app_name), + title, ) } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FillResponseBuilderTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FillResponseBuilderTest.kt index 175806067..2e35c9eed 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FillResponseBuilderTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FillResponseBuilderTest.kt @@ -4,21 +4,16 @@ import android.content.Context import android.service.autofill.Dataset import android.service.autofill.FillResponse import android.view.autofill.AutofillId -import android.widget.RemoteViews import com.x8bit.bitwarden.data.autofill.model.AutofillAppInfo import com.x8bit.bitwarden.data.autofill.model.FilledData -import com.x8bit.bitwarden.data.autofill.model.FilledItem -import com.x8bit.bitwarden.data.autofill.util.applyOverlayToDataset +import com.x8bit.bitwarden.data.autofill.model.FilledPartition +import com.x8bit.bitwarden.data.autofill.util.buildDataset import com.x8bit.bitwarden.data.util.mockBuilder -import com.x8bit.bitwarden.ui.autofill.buildAutofillRemoteViews import io.mockk.every -import io.mockk.just import io.mockk.mockk import io.mockk.mockkConstructor import io.mockk.mockkStatic -import io.mockk.runs import io.mockk.unmockkConstructor -import io.mockk.unmockkStatic import io.mockk.verify import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals @@ -29,31 +24,25 @@ import org.junit.jupiter.api.Test class FillResponseBuilderTest { private lateinit var fillResponseBuilder: FillResponseBuilder - private val dataset: Dataset = mockk() private val context: Context = mockk() + private val dataset: Dataset = mockk() private val fillResponse: FillResponse = mockk() - private val remoteViews: RemoteViews = mockk() private val appInfo: AutofillAppInfo = AutofillAppInfo( context = context, packageName = PACKAGE_NAME, sdkInt = 17, ) - private val autofillIdOne: AutofillId = mockk() - private val autofillIdTwo: AutofillId = mockk() - private val filledItemOne: FilledItem = mockk { - every { this@mockk.autofillId } returns autofillIdOne + private val filledPartitionOne: FilledPartition = mockk { + every { this@mockk.filledItems } returns listOf(mockk()) } - private val filledItemTwo: FilledItem = mockk { - every { this@mockk.autofillId } returns autofillIdTwo + private val filledPartitionTwo: FilledPartition = mockk { + every { this@mockk.filledItems } returns emptyList() } @BeforeEach fun setup() { - mockkConstructor(Dataset.Builder::class) mockkConstructor(FillResponse.Builder::class) - mockkStatic(::buildAutofillRemoteViews) - mockkStatic(FilledItem::applyOverlayToDataset) - every { anyConstructed().build() } returns dataset + mockkStatic(FilledPartition::buildDataset) every { anyConstructed().build() } returns fillResponse fillResponseBuilder = FillResponseBuilderImpl() @@ -61,17 +50,15 @@ class FillResponseBuilderTest { @AfterEach fun teardown() { - unmockkConstructor(Dataset.Builder::class) unmockkConstructor(FillResponse.Builder::class) - unmockkStatic(::buildAutofillRemoteViews) - unmockkStatic(FilledItem::applyOverlayToDataset) + mockkStatic(FilledPartition::buildDataset) } @Test - fun `build should return null when filledItems empty`() { + fun `build should return null when filledPartitions is empty`() { // Test val filledData = FilledData( - filledItems = emptyList(), + filledPartitions = emptyList(), ignoreAutofillIds = emptyList(), ) val actual = fillResponseBuilder.build( @@ -84,7 +71,28 @@ class FillResponseBuilderTest { } @Test - fun `build should apply filledItems and ignore ignoreAutofillIds`() { + fun `build should return null when filledPartitions contains no views`() { + // Test + val filledPartitions = FilledPartition( + filledItems = emptyList(), + ) + val filledData = FilledData( + filledPartitions = listOf( + filledPartitions, + ), + ignoreAutofillIds = emptyList(), + ) + val actual = fillResponseBuilder.build( + autofillAppInfo = appInfo, + filledData = filledData, + ) + + // Verify + assertNull(actual) + } + + @Test + fun `build should apply FilledPartitions with filledItems and ignore ignoreAutofillIds`() { // Setup val ignoredAutofillIdOne: AutofillId = mockk() val ignoredAutofillIdTwo: AutofillId = mockk() @@ -92,34 +100,19 @@ class FillResponseBuilderTest { ignoredAutofillIdOne, ignoredAutofillIdTwo, ) - val filledItems = listOf( - filledItemOne, - filledItemTwo, + val filledPartitions = listOf( + filledPartitionOne, + filledPartitionTwo, ) val filledData = FilledData( - filledItems = filledItems, + filledPartitions = filledPartitions, ignoreAutofillIds = ignoreAutofillIds, ) every { - buildAutofillRemoteViews( - context = context, - packageName = PACKAGE_NAME, + filledPartitionOne.buildDataset( + autofillAppInfo = appInfo, ) - } returns remoteViews - every { - filledItemOne.applyOverlayToDataset( - appInfo = appInfo, - datasetBuilder = anyConstructed(), - remoteViews = remoteViews, - ) - } just runs - every { - filledItemTwo.applyOverlayToDataset( - appInfo = appInfo, - datasetBuilder = anyConstructed(), - remoteViews = remoteViews, - ) - } just runs + } returns dataset mockBuilder { it.addDataset(dataset) } mockBuilder { it.setIgnoredIds( @@ -138,15 +131,8 @@ class FillResponseBuilderTest { assertEquals(fillResponse, actual) verify(exactly = 1) { - filledItemOne.applyOverlayToDataset( - appInfo = appInfo, - datasetBuilder = any(), - remoteViews = remoteViews, - ) - filledItemTwo.applyOverlayToDataset( - appInfo = appInfo, - datasetBuilder = any(), - remoteViews = remoteViews, + filledPartitionOne.buildDataset( + autofillAppInfo = appInfo, ) anyConstructed().addDataset(dataset) anyConstructed().setIgnoredIds( diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt index 0647be1b1..b51c8c52f 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt @@ -6,6 +6,7 @@ import com.x8bit.bitwarden.data.autofill.model.AutofillRequest import com.x8bit.bitwarden.data.autofill.model.AutofillView import com.x8bit.bitwarden.data.autofill.model.FilledData import com.x8bit.bitwarden.data.autofill.model.FilledItem +import com.x8bit.bitwarden.data.autofill.model.FilledPartition import io.mockk.mockk import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals @@ -21,7 +22,7 @@ class FilledDataBuilderTest { } @Test - fun `build should return FilledData with FilledItems and ignored AutofillIds`() = runTest { + fun `build should return filled data and ignored AutofillIds when Login`() = runTest { // Setup val autofillId: AutofillId = mockk() val autofillView = AutofillView.Login.Username( @@ -39,10 +40,55 @@ class FilledDataBuilderTest { val filledItem = FilledItem( autofillId = autofillId, ) - val expected = FilledData( + val filledPartition = FilledPartition( filledItems = listOf( filledItem, ), + ) + val expected = FilledData( + filledPartitions = listOf( + filledPartition, + ), + ignoreAutofillIds = ignoreAutofillIds, + ) + + // Test + val actual = filledDataBuilder.build( + autofillRequest = autofillRequest, + ) + + // Verify + assertEquals(expected, actual) + } + + @Test + fun `build should return filled data and ignored AutofillIds when Card`() = runTest { + // Setup + val autofillId: AutofillId = mockk() + val autofillView = AutofillView.Card.Number( + autofillId = autofillId, + isFocused = false, + ) + val autofillPartition = AutofillPartition.Card( + views = listOf(autofillView), + ) + val ignoreAutofillIds: List = mockk() + val autofillRequest = AutofillRequest.Fillable( + ignoreAutofillIds = ignoreAutofillIds, + partition = autofillPartition, + ) + val filledItem = FilledItem( + autofillId = autofillId, + ) + val filledPartition = FilledPartition( + filledItems = listOf( + filledItem, + ), + ) + val expected = FilledData( + filledPartitions = listOf( + filledPartition, + ), ignoreAutofillIds = ignoreAutofillIds, ) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorTest.kt index 9dd02f3be..d8d0bd51b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorTest.kt @@ -11,7 +11,6 @@ import com.x8bit.bitwarden.data.autofill.builder.FilledDataBuilder import com.x8bit.bitwarden.data.autofill.model.AutofillAppInfo import com.x8bit.bitwarden.data.autofill.model.AutofillRequest import com.x8bit.bitwarden.data.autofill.model.FilledData -import com.x8bit.bitwarden.data.autofill.model.FilledItem import com.x8bit.bitwarden.data.autofill.parser.AutofillParser import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import io.mockk.coEvery @@ -138,9 +137,8 @@ class AutofillProcessorTest { val fillRequest: FillRequest = mockk { every { this@mockk.fillContexts } returns fillContextList } - val filledItems: List = listOf(mockk()) val filledData = FilledData( - filledItems = filledItems, + filledPartitions = listOf(mockk()), ignoreAutofillIds = emptyList(), ) val fillResponse: FillResponse = mockk() diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/util/FilledItemExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/util/FilledItemExtensionsTest.kt index b1b3b669c..63759c5bb 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/util/FilledItemExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/util/FilledItemExtensionsTest.kt @@ -1,12 +1,10 @@ package com.x8bit.bitwarden.data.autofill.util -import android.content.Context import android.service.autofill.Dataset import android.service.autofill.Field import android.service.autofill.Presentations import android.view.autofill.AutofillId import android.widget.RemoteViews -import com.x8bit.bitwarden.data.autofill.model.AutofillAppInfo import com.x8bit.bitwarden.data.autofill.model.FilledItem import com.x8bit.bitwarden.data.util.mockBuilder import io.mockk.every @@ -20,7 +18,6 @@ import org.junit.jupiter.api.Test class FilledItemExtensionsTest { private val autofillId: AutofillId = mockk() - private val context: Context = mockk() private val datasetBuilder: Dataset.Builder = mockk() private val field: Field = mockk() private val filledItem = FilledItem( @@ -31,29 +28,19 @@ class FilledItemExtensionsTest { @BeforeEach fun setup() { - mockkConstructor(Dataset.Builder::class) - mockkConstructor(Presentations.Builder::class) mockkConstructor(Field.Builder::class) - every { anyConstructed().build() } returns presentations every { anyConstructed().build() } returns field } @AfterEach fun teardown() { - unmockkConstructor(Dataset.Builder::class) - unmockkConstructor(Presentations.Builder::class) unmockkConstructor(Field.Builder::class) } @Suppress("Deprecation") @Test - fun `applyOverlayToDataset should use setValue to set RemoteViews when before tiramisu`() { + fun `applyToDatasetPreTiramisu should use setValue to set RemoteViews`() { // Setup - val appInfo = AutofillAppInfo( - context = context, - packageName = PACKAGE_NAME, - sdkInt = 1, - ) every { datasetBuilder.setValue( autofillId, @@ -63,8 +50,7 @@ class FilledItemExtensionsTest { } returns datasetBuilder // Test - filledItem.applyOverlayToDataset( - appInfo = appInfo, + filledItem.applyToDatasetPreTiramisu( datasetBuilder = datasetBuilder, remoteViews = remoteViews, ) @@ -80,14 +66,8 @@ class FilledItemExtensionsTest { } @Test - fun `applyOverlayToDataset should use setField to set Presentation on or after Tiramisu`() { + fun `applyToDatasetPostTiramisu should use setField to set presentations`() { // Setup - val appInfo = AutofillAppInfo( - context = context, - packageName = PACKAGE_NAME, - sdkInt = 34, - ) - mockBuilder { it.setMenuPresentation(remoteViews) } mockBuilder { it.setPresentations(presentations) } every { datasetBuilder.setField( @@ -97,15 +77,13 @@ class FilledItemExtensionsTest { } returns datasetBuilder // Test - filledItem.applyOverlayToDataset( - appInfo = appInfo, + filledItem.applyToDatasetPostTiramisu( datasetBuilder = datasetBuilder, - remoteViews = remoteViews, + presentations = presentations, ) // Verify verify(exactly = 1) { - anyConstructed().setMenuPresentation(remoteViews) anyConstructed().setPresentations(presentations) datasetBuilder.setField( autofillId, @@ -113,8 +91,4 @@ class FilledItemExtensionsTest { ) } } - - companion object { - private const val PACKAGE_NAME: String = "com.x8bit.bitwarden" - } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/util/FilledPartitionExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/util/FilledPartitionExtensionsTest.kt new file mode 100644 index 000000000..cc9a7d08a --- /dev/null +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/util/FilledPartitionExtensionsTest.kt @@ -0,0 +1,155 @@ +package com.x8bit.bitwarden.data.autofill.util + +import android.content.Context +import android.content.res.Resources +import android.service.autofill.Dataset +import android.service.autofill.Presentations +import android.widget.RemoteViews +import com.x8bit.bitwarden.R +import com.x8bit.bitwarden.data.autofill.model.AutofillAppInfo +import com.x8bit.bitwarden.data.autofill.model.FilledItem +import com.x8bit.bitwarden.data.autofill.model.FilledPartition +import com.x8bit.bitwarden.data.util.mockBuilder +import com.x8bit.bitwarden.ui.autofill.buildAutofillRemoteViews +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkConstructor +import io.mockk.mockkStatic +import io.mockk.runs +import io.mockk.unmockkConstructor +import io.mockk.unmockkStatic +import io.mockk.verify +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test + +class FilledPartitionExtensionsTest { + private val res: Resources = mockk() + private val context: Context = mockk { + every { this@mockk.resources } returns res + } + private val dataset: Dataset = mockk() + private val filledItem: FilledItem = mockk() + private val filledPartition = FilledPartition( + filledItems = listOf( + filledItem, + ), + ) + private val presentations: Presentations = mockk() + private val remoteViews: RemoteViews = mockk() + + @BeforeEach + fun setup() { + mockkConstructor(Dataset.Builder::class) + mockkConstructor(Presentations.Builder::class) + mockkStatic(::buildAutofillRemoteViews) + mockkStatic(FilledItem::applyToDatasetPostTiramisu) + mockkStatic(FilledItem::applyToDatasetPreTiramisu) + every { anyConstructed().build() } returns dataset + } + + @AfterEach + fun teardown() { + unmockkConstructor(Dataset.Builder::class) + unmockkConstructor(Presentations.Builder::class) + unmockkStatic(::buildAutofillRemoteViews) + unmockkStatic(FilledItem::applyToDatasetPostTiramisu) + unmockkStatic(FilledItem::applyToDatasetPreTiramisu) + } + + @Test + fun `buildDataset should applyToDatasetPostTiramisu when sdkInt is at least 33`() { + // Setup + val autofillAppInfo = AutofillAppInfo( + context = context, + packageName = PACKAGE_NAME, + sdkInt = 34, + ) + val title = "Bitwarden" + every { res.getString(R.string.app_name) } returns title + every { + buildAutofillRemoteViews( + packageName = PACKAGE_NAME, + title = title, + ) + } returns remoteViews + mockBuilder { it.setMenuPresentation(remoteViews) } + every { + filledItem.applyToDatasetPostTiramisu( + datasetBuilder = any(), + presentations = presentations, + ) + } just runs + every { anyConstructed().build() } returns presentations + + // Test + val actual = filledPartition.buildDataset( + autofillAppInfo = autofillAppInfo, + ) + + // Verify + assertEquals(dataset, actual) + verify(exactly = 1) { + buildAutofillRemoteViews( + packageName = PACKAGE_NAME, + title = title, + ) + anyConstructed().setMenuPresentation(remoteViews) + anyConstructed().build() + filledItem.applyToDatasetPostTiramisu( + datasetBuilder = any(), + presentations = presentations, + ) + anyConstructed().build() + } + } + + @Test + fun `buildDataset should applyToDatasetPreTiramisu when sdkInt is less than 33`() { + // Setup + val autofillAppInfo = AutofillAppInfo( + context = context, + packageName = PACKAGE_NAME, + sdkInt = 18, + ) + val title = "Bitwarden" + every { res.getString(R.string.app_name) } returns title + every { + buildAutofillRemoteViews( + packageName = PACKAGE_NAME, + title = title, + ) + } returns remoteViews + every { + filledItem.applyToDatasetPreTiramisu( + datasetBuilder = any(), + remoteViews = remoteViews, + ) + } just runs + + // Test + val actual = filledPartition.buildDataset( + autofillAppInfo = autofillAppInfo, + ) + + // Verify + assertEquals(dataset, actual) + verify(exactly = 1) { + buildAutofillRemoteViews( + packageName = PACKAGE_NAME, + title = title, + ) + filledItem.applyToDatasetPreTiramisu( + datasetBuilder = any(), + remoteViews = remoteViews, + ) + anyConstructed().build() + } + } + + companion object { + private const val PACKAGE_NAME: String = "com.x8bit.bitwarden" + } +} diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/autofill/BitwardenRemoteViewsTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/autofill/BitwardenRemoteViewsTest.kt index 3e5616cc3..6f92e4473 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/autofill/BitwardenRemoteViewsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/autofill/BitwardenRemoteViewsTest.kt @@ -1,6 +1,5 @@ package com.x8bit.bitwarden.ui.autofill -import android.content.Context import android.content.res.Resources import android.widget.RemoteViews import com.x8bit.bitwarden.R @@ -18,9 +17,6 @@ import org.junit.jupiter.api.Test class BitwardenRemoteViewsTest { private val testResources: Resources = mockk() - private val context: Context = mockk { - every { this@mockk.resources } returns testResources - } @BeforeEach fun setup() { @@ -31,7 +27,6 @@ class BitwardenRemoteViewsTest { fun teardown() { unmockkConstructor(RemoteViews::class) confirmVerified( - context, testResources, ) } @@ -39,19 +34,18 @@ class BitwardenRemoteViewsTest { @Test fun `buildAutofillRemoteViews should set text`() { // Setup - val appName = "Bitwarden" - every { testResources.getText(R.string.app_name) } returns appName + val title = "Bitwarden" every { anyConstructed() .setTextViewText( R.id.text, - appName, + title, ) } just runs // Test buildAutofillRemoteViews( - context = context, + title = title, packageName = PACKAGE_NAME, ) @@ -61,12 +55,10 @@ class BitwardenRemoteViewsTest { // Verify verify(exactly = 1) { - context.resources - testResources.getText(R.string.app_name) anyConstructed() .setTextViewText( R.id.text, - "Bitwarden", + title, ) } }