From e758e273becdc076b01d141eb8dc798bc0540e2d Mon Sep 17 00:00:00 2001 From: Lucas Kivi <125697099+lucas-livefront@users.noreply.github.com> Date: Sun, 14 Jan 2024 21:21:22 -0600 Subject: [PATCH] Refactor autofill parser to take a FillRequest (#614) --- .../data/autofill/parser/AutofillParser.kt | 6 +-- .../autofill/parser/AutofillParserImpl.kt | 21 +++++++- .../processor/AutofillProcessorImpl.kt | 47 +++++++--------- .../autofill/parser/AutofillParserTests.kt | 31 +++++++++-- .../processor/AutofillProcessorTest.kt | 53 +++---------------- 5 files changed, 75 insertions(+), 83 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParser.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParser.kt index f0ae5a65e0..1f1fb75629 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParser.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParser.kt @@ -1,6 +1,6 @@ package com.x8bit.bitwarden.data.autofill.parser -import android.app.assist.AssistStructure +import android.service.autofill.FillRequest import com.x8bit.bitwarden.data.autofill.model.AutofillRequest /** @@ -9,7 +9,7 @@ import com.x8bit.bitwarden.data.autofill.model.AutofillRequest interface AutofillParser { /** - * Parse the useful information from [assistStructure] into an [AutofillRequest]. + * Parse the useful information from [fillRequest] into an [AutofillRequest]. */ - fun parse(assistStructure: AssistStructure): AutofillRequest + fun parse(fillRequest: FillRequest): AutofillRequest } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt index ca090b1646..159d1c6f55 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt @@ -1,6 +1,7 @@ package com.x8bit.bitwarden.data.autofill.parser import android.app.assist.AssistStructure +import android.service.autofill.FillRequest import android.view.autofill.AutofillId import com.x8bit.bitwarden.data.autofill.model.AutofillPartition import com.x8bit.bitwarden.data.autofill.model.AutofillRequest @@ -14,7 +15,25 @@ import com.x8bit.bitwarden.data.autofill.util.toAutofillView * from the OS into domain models. */ class AutofillParserImpl : AutofillParser { - override fun parse(assistStructure: AssistStructure): AutofillRequest { + override fun parse(fillRequest: FillRequest): AutofillRequest = + // Attempt to get the most recent autofill context. + fillRequest + .fillContexts + .lastOrNull() + ?.structure + ?.let { assistStructure -> + parseInternal( + assistStructure = assistStructure, + ) + } + ?: AutofillRequest.Unfillable + + /** + * Parse the [AssistStructure] into an [AutofillRequest]. + */ + private fun parseInternal( + assistStructure: AssistStructure, + ): AutofillRequest { // Parse the `assistStructure` into internal models. val traversalDataList = assistStructure.traverse() // Flatten the autofill views for processing. diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorImpl.kt index 048a3899f9..5fa62c732e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorImpl.kt @@ -1,11 +1,10 @@ package com.x8bit.bitwarden.data.autofill.processor -import android.app.assist.AssistStructure import android.os.CancellationSignal import android.service.autofill.FillCallback import android.service.autofill.FillRequest -import com.x8bit.bitwarden.data.autofill.builder.FilledDataBuilder import com.x8bit.bitwarden.data.autofill.builder.FillResponseBuilder +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.parser.AutofillParser @@ -36,57 +35,49 @@ class AutofillProcessorImpl( fillCallback: FillCallback, request: FillRequest, ) { - // Attempt to get the most recent autofill context. - val assistStructure = request - .fillContexts - .lastOrNull() - ?.structure - ?: run { - // There is no data for us to process. - fillCallback.onSuccess(null) - return - } - // Set the listener so that any long running work is cancelled when it is no longer needed. cancellationSignal.setOnCancelListener { scope.cancel() } // Process the OS data and handle invoking the callback with the result. - assistStructure.process( + process( autofillAppInfo = autofillAppInfo, fillCallback = fillCallback, + fillRequest = request, ) } /** - * Process the [AssistStructure] and invoke the [FillCallback] with the response. + * Process the [fillRequest] and invoke the [FillCallback] with the response. */ - private fun AssistStructure.process( + private fun process( autofillAppInfo: AutofillAppInfo, fillCallback: FillCallback, + fillRequest: FillRequest, ) { - scope.launch { - // Parse the OS data into an [AutofillRequest] for easier processing. - val fillResponse = when (val autofillRequest = parser.parse(this@process)) { - is AutofillRequest.Fillable -> { + // Parse the OS data into an [AutofillRequest] for easier processing. + when (val autofillRequest = parser.parse(fillRequest)) { + is AutofillRequest.Fillable -> { + scope.launch { // Fulfill the [autofillRequest]. val filledData = filledDataBuilder.build( autofillRequest = autofillRequest, ) // Load the [filledData] into a [FillResponse]. - fillResponseBuilder.build( + val response = fillResponseBuilder.build( autofillAppInfo = autofillAppInfo, filledData = filledData, ) - } - AutofillRequest.Unfillable -> { - // If we are unable to fulfill the request, we should invoke the callback - // with null. This effectively disables autofill for this view set and - // allows the [AutofillService] to be unbound. - null + fillCallback.onSuccess(response) } } - fillCallback.onSuccess(fillResponse) + + AutofillRequest.Unfillable -> { + // If we are unable to fulfill the request, we should invoke the callback + // with null. This effectively disables autofill for this view set and + // allows the [AutofillService] to be unbound. + fillCallback.onSuccess(null) + } } } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt index d0f1b6dacd..94be6dc486 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt @@ -1,6 +1,8 @@ package com.x8bit.bitwarden.data.autofill.parser import android.app.assist.AssistStructure +import android.service.autofill.FillContext +import android.service.autofill.FillRequest import android.view.View import android.view.autofill.AutofillId import com.x8bit.bitwarden.data.autofill.model.AutofillPartition @@ -50,6 +52,12 @@ class AutofillParserTests { private val loginWindowNode: AssistStructure.WindowNode = mockk { every { this@mockk.rootViewNode } returns loginViewNode } + private val fillContext: FillContext = mockk { + every { this@mockk.structure } returns assistStructure + } + private val fillRequest: FillRequest = mockk { + every { this@mockk.fillContexts } returns listOf(fillContext) + } @BeforeEach fun setup() { @@ -65,6 +73,19 @@ class AutofillParserTests { unmockkStatic(List::buildUriOrNull) } + @Test + fun `parse should return Unfillable when no contexts`() { + // Setup + val expected = AutofillRequest.Unfillable + every { fillRequest.fillContexts } returns emptyList() + + // Test + val actual = parser.parse(fillRequest) + + // Verify + assertEquals(expected, actual) + } + @Test fun `parse should return Unfillable when windowNodeCount is 0`() { // Setup @@ -72,7 +93,7 @@ class AutofillParserTests { every { assistStructure.windowNodeCount } returns 0 // Test - val actual = parser.parse(assistStructure) + val actual = parser.parse(fillRequest) // Verify assertEquals(expected, actual) @@ -120,7 +141,7 @@ class AutofillParserTests { every { assistStructure.getWindowNodeAt(0) } returns windowNode // Test - val actual = parser.parse(assistStructure) + val actual = parser.parse(fillRequest) // Verify assertEquals(expected, actual) @@ -157,7 +178,7 @@ class AutofillParserTests { every { loginViewNode.toAutofillView() } returns loginAutofillView // Test - val actual = parser.parse(assistStructure) + val actual = parser.parse(fillRequest) // Verify assertEquals(expected, actual) @@ -194,7 +215,7 @@ class AutofillParserTests { every { loginViewNode.toAutofillView() } returns loginAutofillView // Test - val actual = parser.parse(assistStructure) + val actual = parser.parse(fillRequest) // Verify assertEquals(expected, actual) @@ -231,7 +252,7 @@ class AutofillParserTests { every { loginViewNode.toAutofillView() } returns loginAutofillView // Test - val actual = parser.parse(assistStructure) + val actual = parser.parse(fillRequest) // Verify assertEquals(expected, actual) 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 d8d0bd51b9..b6f0871d1d 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 @@ -1,9 +1,7 @@ package com.x8bit.bitwarden.data.autofill.processor -import android.app.assist.AssistStructure import android.os.CancellationSignal import android.service.autofill.FillCallback -import android.service.autofill.FillContext import android.service.autofill.FillRequest import android.service.autofill.FillResponse import com.x8bit.bitwarden.data.autofill.builder.FillResponseBuilder @@ -44,7 +42,6 @@ class AutofillProcessorTest { packageName = "com.x8bit.bitwarden", sdkInt = 42, ) - private val assistStructure: AssistStructure = mockk() private val fillCallback: FillCallback = mockk() @BeforeEach @@ -73,41 +70,13 @@ class AutofillProcessorTest { ) } - @Test - fun `processFillRequest should invoke callback with null when no fillContexts`() { - // Setup - val fillRequest: FillRequest = mockk { - every { this@mockk.fillContexts } returns emptyList() - } - every { fillCallback.onSuccess(null) } just runs - - // Test - autofillProcessor.processFillRequest( - autofillAppInfo = appInfo, - cancellationSignal = cancellationSignal, - fillCallback = fillCallback, - request = fillRequest, - ) - - // Verify - verify(exactly = 1) { - fillCallback.onSuccess(null) - } - } - @Test fun `processFillRequest should invoke callback with null when parse returns Unfillable`() { // Setup val autofillRequest = AutofillRequest.Unfillable - val lastFillContext: FillContext = mockk { - every { this@mockk.structure } returns assistStructure - } - val fillContexts: List = listOf(mockk(), lastFillContext) - val fillRequest: FillRequest = mockk { - every { this@mockk.fillContexts } returns fillContexts - } + val fillRequest: FillRequest = mockk() every { cancellationSignal.setOnCancelListener(any()) } just runs - every { parser.parse(assistStructure) } returns autofillRequest + every { parser.parse(fillRequest) } returns autofillRequest every { fillCallback.onSuccess(null) } just runs // Test @@ -121,7 +90,7 @@ class AutofillProcessorTest { // Verify verify(exactly = 1) { cancellationSignal.setOnCancelListener(any()) - parser.parse(assistStructure) + parser.parse(fillRequest) fillCallback.onSuccess(null) } } @@ -130,28 +99,20 @@ class AutofillProcessorTest { fun `processFillRequest should invoke callback with filled response when has filledItems`() = runTest { // Setup - val lastFillContext: FillContext = mockk { - every { this@mockk.structure } returns assistStructure - } - val fillContextList: List = listOf(mockk(), lastFillContext) - val fillRequest: FillRequest = mockk { - every { this@mockk.fillContexts } returns fillContextList - } + val fillRequest: FillRequest = mockk() val filledData = FilledData( filledPartitions = listOf(mockk()), ignoreAutofillIds = emptyList(), ) val fillResponse: FillResponse = mockk() - val autofillRequest: AutofillRequest.Fillable = mockk { - every { this@mockk.ignoreAutofillIds } returns emptyList() - } + val autofillRequest: AutofillRequest.Fillable = mockk() coEvery { filledDataBuilder.build( autofillRequest = autofillRequest, ) } returns filledData every { cancellationSignal.setOnCancelListener(any()) } just runs - every { parser.parse(assistStructure) } returns autofillRequest + every { parser.parse(fillRequest) } returns autofillRequest every { fillResponseBuilder.build( autofillAppInfo = appInfo, @@ -176,7 +137,7 @@ class AutofillProcessorTest { } verify(exactly = 1) { cancellationSignal.setOnCancelListener(any()) - parser.parse(assistStructure) + parser.parse(fillRequest) fillResponseBuilder.build( autofillAppInfo = appInfo, filledData = filledData,