Refactor autofill flow to partition fill data by cipher (#571)

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<Dataset.Builder>().build() } returns dataset
+        mockkStatic(FilledPartition::buildDataset)
         every { anyConstructed<FillResponse.Builder>().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<FillResponse.Builder> { it.addDataset(dataset) }
         mockBuilder<FillResponse.Builder> {
             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<FillResponse.Builder>().addDataset(dataset)
             anyConstructed<FillResponse.Builder>().setIgnoredIds(

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<AutofillId> = 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,
         )
 

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<FilledItem> = listOf(mockk())
             val filledData = FilledData(
-                filledItems = filledItems,
+                filledPartitions = listOf(mockk()),
                 ignoreAutofillIds = emptyList(),
             )
             val fillResponse: FillResponse = mockk()

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<Presentations.Builder>().build() } returns presentations
         every { anyConstructed<Field.Builder>().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<Presentations.Builder> { it.setMenuPresentation(remoteViews) }
         mockBuilder<Field.Builder> { 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<Presentations.Builder>().setMenuPresentation(remoteViews)
             anyConstructed<Field.Builder>().setPresentations(presentations)
             datasetBuilder.setField(
                 autofillId,
@@ -113,8 +91,4 @@ class FilledItemExtensionsTest {
             )
         }
     }
-
-    companion object {
-        private const val PACKAGE_NAME: String = "com.x8bit.bitwarden"
-    }
 }

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<Dataset.Builder>().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<Presentations.Builder> { it.setMenuPresentation(remoteViews) }
+        every {
+            filledItem.applyToDatasetPostTiramisu(
+                datasetBuilder = any(),
+                presentations = presentations,
+            )
+        } just runs
+        every { anyConstructed<Presentations.Builder>().build() } returns presentations
+
+        // Test
+        val actual = filledPartition.buildDataset(
+            autofillAppInfo = autofillAppInfo,
+        )
+
+        // Verify
+        assertEquals(dataset, actual)
+        verify(exactly = 1) {
+            buildAutofillRemoteViews(
+                packageName = PACKAGE_NAME,
+                title = title,
+            )
+            anyConstructed<Presentations.Builder>().setMenuPresentation(remoteViews)
+            anyConstructed<Presentations.Builder>().build()
+            filledItem.applyToDatasetPostTiramisu(
+                datasetBuilder = any(),
+                presentations = presentations,
+            )
+            anyConstructed<Dataset.Builder>().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<Dataset.Builder>().build()
+        }
+    }
+
+    companion object {
+        private const val PACKAGE_NAME: String = "com.x8bit.bitwarden"
+    }
+}

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<RemoteViews>()
                 .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<RemoteViews>()
                 .setTextViewText(
                     R.id.text,
-                    "Bitwarden",
+                    title,
                 )
         }
     }