fix: Make sure we only perform xpath lookup once (#831)

appium · Oct 28, 2022 · adf14ad · adf14ad
1 parent 536c7e7
commit adf14ad
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 22 deletions.
diff --git a/...server/app/src/androidTest/java/io/appium/espressoserver/lib/helpers/ComposeNodeFinder.kt b/...server/app/src/androidTest/java/io/appium/espressoserver/lib/helpers/ComposeNodeFinder.kt
@@ -26,6 +26,7 @@ import io.appium.espressoserver.lib.handlers.exceptions.StaleElementException
 import io.appium.espressoserver.lib.model.Locator
 import io.appium.espressoserver.lib.model.SourceDocument
 import io.appium.espressoserver.lib.model.AttributesEnum
+import io.appium.espressoserver.lib.model.compileXpathExpression
 import io.appium.espressoserver.lib.viewmatcher.fetchIncludedAttributes
 
 /**
@@ -63,9 +64,12 @@ fun semanticsMatcherForLocator(locator: Locator): SemanticsMatcher =
     }
 
 private fun hasXpath(locator: Locator): SemanticsMatcher {
+    val xpath = locator.value!!
+    val expression = compileXpathExpression(xpath)
+    val attributes = fetchIncludedAttributes(xpath)
     val matchingIds = SourceDocument(
-        locator.elementId?.let { getSemanticsNode(it) }, fetchIncludedAttributes(locator.value!!)
-    ).matchingNodeIds(locator.value!!, AttributesEnum.RESOURCE_ID.toString())
+        locator.elementId?.let { getSemanticsNode(it) }, attributes
+    ).findMatchingNodeIds(expression, AttributesEnum.RESOURCE_ID.toString())
 
     return SemanticsMatcher("Matches Xpath ${locator.value}") {
         matchingIds.contains(it.id)

diff --git a/...esso-server/app/src/androidTest/java/io/appium/espressoserver/lib/model/SourceDocument.kt b/...esso-server/app/src/androidTest/java/io/appium/espressoserver/lib/model/SourceDocument.kt
@@ -80,6 +80,12 @@ private fun toXmlNodeName(className: String?): String {
     return fixedName
 }
 
+fun compileXpathExpression(selector: String): XPathExpression =
+    try {
+        XPATH.compile(selector)
+    } catch (xe: XPathExpressionException) {
+        throw XPathLookupException(selector, xe.message!!)
+    }
 
 class SourceDocument constructor(
     private val root: Any? = null,
@@ -324,8 +330,8 @@ class SourceDocument constructor(
         throw AppiumException(lastError!!)
     }
 
-    private fun rootSemanticNodes(): List<SemanticsNode> {
-        return try {
+    private fun rootSemanticNodes(): List<SemanticsNode> =
+        try {
             listOf(EspressoServerRunnerTest.composeTestRule.onRoot(useUnmergedTree = true).fetchSemanticsNode())
         } catch (e: AssertionError) {
 //            Ideally there should be on `root` node but on some cases e.g:overlays screen, there can be more than 1 root.
@@ -337,7 +343,6 @@ class SourceDocument constructor(
             } as SelectionResult
             result.selectedNodes
         }
-    }
 
     private fun performCleanup() {
         tmpXmlName?.let {
@@ -346,8 +351,8 @@ class SourceDocument constructor(
         }
     }
 
-    fun toXMLString(): String {
-        return RESOURCES_GUARD.withPermit({
+    fun toXMLString(): String =
+        RESOURCES_GUARD.withPermit({
             toStream().use { xmlStream ->
                 val sb = StringBuilder()
                 val reader = BufferedReader(InputStreamReader(xmlStream, XML_ENCODING))
@@ -359,24 +364,17 @@ class SourceDocument constructor(
                 sb.toString()
             }
         }, { performCleanup() })
-    }
 
-    fun findViewsByXPath(xpathSelector: String): List<View> =
-        matchingNodeIds(xpathSelector, VIEW_INDEX).map { viewMap.get(it) }
+    fun findViewsByXPath(expression: XPathExpression): List<View> =
+        findMatchingNodeIds(expression, VIEW_INDEX).map { viewMap.get(it) }
 
-    fun matchingNodeIds(xpathSelector: String, attributeName: String): List<Int> {
-        val expr = try {
-            XPATH.compile(xpathSelector)
-        } catch (xe: XPathExpressionException) {
-            throw XPathLookupException(xpathSelector, xe.message!!)
-        }
-        return RESOURCES_GUARD.withPermit({
+    fun findMatchingNodeIds(expression: XPathExpression, attributeName: String): List<Int> =
+        RESOURCES_GUARD.withPermit({
             toStream().use { xmlStream ->
-                val list = expr.evaluate(InputSource(xmlStream), XPathConstants.NODESET) as NodeList
+                val list = expression.evaluate(InputSource(xmlStream), XPathConstants.NODESET) as NodeList
                 (0 until list.length).map { index ->
                     list.item(index).attributes.getNamedItem(attributeName).nodeValue.toInt()
                 }
             }
         }, { performCleanup() })
-    }
 }
diff --git a/...sso-server/app/src/androidTest/java/io/appium/espressoserver/lib/viewmatcher/WithXPath.kt b/...sso-server/app/src/androidTest/java/io/appium/espressoserver/lib/viewmatcher/WithXPath.kt
@@ -16,12 +16,15 @@
 package io.appium.espressoserver.lib.viewmatcher
 
 import android.view.View
+import io.appium.espressoserver.lib.helpers.extensions.withPermit
 import io.appium.espressoserver.lib.model.SourceDocument
 import io.appium.espressoserver.lib.model.AttributesEnum
 import io.appium.espressoserver.lib.model.EspressoAttributes
+import io.appium.espressoserver.lib.model.compileXpathExpression
 import org.hamcrest.Description
 import org.hamcrest.Matcher
 import org.hamcrest.TypeSafeMatcher
+import java.util.concurrent.Semaphore
 
 fun fetchIncludedAttributes(xpath: String): Set<AttributesEnum>? {
     if (xpath.contains("@*")) {
@@ -37,13 +40,24 @@ fun fetchIncludedAttributes(xpath: String): Set<AttributesEnum>? {
 }
 
 fun withXPath(root: View?, xpath: String, index: Int? = null): Matcher<View> {
+    val expression = compileXpathExpression(xpath)
+    val attributes = fetchIncludedAttributes(xpath)
     val matchedXPathViews = mutableListOf<View>()
+    var didLookup = false
+    val lookupGuard = Semaphore(1)
     return object : TypeSafeMatcher<View>() {
         override fun matchesSafely(item: View): Boolean {
+            lookupGuard.withPermit {
+                if (!didLookup) {
+                    matchedXPathViews.addAll(
+                        SourceDocument(root ?: item.rootView, attributes).findViewsByXPath(expression)
+                    )
+                    didLookup = true
+                }
+            }
+
             if (matchedXPathViews.isEmpty()) {
-                matchedXPathViews.addAll(
-                    SourceDocument(root ?: item.rootView, fetchIncludedAttributes(xpath)).findViewsByXPath(xpath)
-                )
+                return false
             }
 
             return if (index != null) {