From b3e09723465219556fda0f829e6c28945130912b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= <ln@getodk.org>
Date: Wed, 7 Feb 2024 15:49:53 -0800
Subject: [PATCH] Prevent instance name conflicts with search and pulldata

---
 pyxform/survey.py                | 15 ++++-----------
 pyxform/utils.py                 | 18 ++++++++++++++++--
 pyxform/xls2json.py              |  9 +++++++--
 tests/test_external_instances.py | 17 +++++++++++++++++
 4 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/pyxform/survey.py b/pyxform/survey.py
index a45659b1..91e31306 100644
--- a/pyxform/survey.py
+++ b/pyxform/survey.py
@@ -30,6 +30,7 @@
     PatchedText,
     get_languages_with_bad_tags,
     has_dynamic_label,
+    has_search_appearance_function,
     node,
 )
 from pyxform.validators import enketo_validate, odk_validate
@@ -42,7 +43,8 @@
     r"(instance\(.*\)\/root\/item\[.*?(\$\{.*\})\]\/.*?)\s"
 )
 RE_PULLDATA = re.compile(r"(pulldata\s*\(\s*)(.*?),")
-SEARCH_APPEARANCE_REGEX = re.compile(r"search\(.*?\)")
+RE_XML_OUTPUT = re.compile(r"\n.*(<output.*>)\n(\s\s)*")
+RE_XML_TEXT = re.compile(r"(>)\n\s*(\s[^<>\s].*?)\n\s*(\s</)", re.DOTALL)
 
 
 class InstanceInfo:
@@ -686,16 +688,7 @@ def _redirect_is_search_itext(element: SurveyElement) -> None:
         :param element: A select type question.
         :return: None, the question/children are modified in-place.
         """
-        try:
-            is_search = bool(
-                SEARCH_APPEARANCE_REGEX.search(
-                    element[constants.CONTROL][constants.APPEARANCE]
-                )
-            )
-        except (KeyError, TypeError):
-            is_search = False
-        if is_search:
-            element[constants.ITEMSET] = ""
+        if has_search_appearance_function(element):
             for i, opt in enumerate(element.get(constants.CHILDREN, [])):
                 opt["_choice_itext_id"] = f"{element['list_name']}-{i}"
 
diff --git a/pyxform/utils.py b/pyxform/utils.py
index 70bb194d..2809ffda 100644
--- a/pyxform/utils.py
+++ b/pyxform/utils.py
@@ -10,12 +10,13 @@
 import re
 from collections import namedtuple
 from json.decoder import JSONDecodeError
-from typing import Dict, List, Tuple
+from typing import Any, Dict, List, Tuple
 from xml.dom import Node
 from xml.dom.minidom import Element, Text, _write_data, parseString
 
 import openpyxl
 import xlrd
+from pyxform import constants
 
 from pyxform.xls2json_backends import is_empty, xls_value_to_unicode, xlsx_value_to_str
 
@@ -29,7 +30,6 @@
 PYXFORM_REFERENCE_REGEX = re.compile(r"\$\{(.*?)\}")
 NODE_TYPE_TEXT = (Node.TEXT_NODE, Node.CDATA_SECTION_NODE)
 
-
 NSMAP = {
     "xmlns": "http://www.w3.org/2002/xforms",
     "xmlns:h": "http://www.w3.org/1999/xhtml",
@@ -327,6 +327,20 @@ def has_dynamic_label(choice_list: "List[Dict[str, str]]") -> bool:
     return False
 
 
+def has_search_appearance_function(question: Dict[str, Any]) -> bool:
+    """
+    The search() appearance can be applied to selects to use a Collect-only database-backed select implementation.
+    """
+    try:
+        return bool(
+            re.compile(r"search\(.*?\)").search(
+                question[constants.CONTROL][constants.APPEARANCE]
+            )
+        )
+    except (KeyError, TypeError):
+        return False
+
+
 def levenshtein_distance(a: str, b: str) -> int:
     """
     Calculate Levenshtein distance between two strings.
diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py
index 02fe8aa2..25799052 100644
--- a/pyxform/xls2json.py
+++ b/pyxform/xls2json.py
@@ -23,7 +23,11 @@
 )
 from pyxform.errors import PyXFormError
 from pyxform.parsing.expression import is_single_token_expression
-from pyxform.utils import PYXFORM_REFERENCE_REGEX, default_is_dynamic
+from pyxform.utils import (
+    PYXFORM_REFERENCE_REGEX,
+    default_is_dynamic,
+    has_search_appearance_function,
+)
 from pyxform.validators.pyxform import parameters_generic, select_from_file
 from pyxform.validators.pyxform.android_package_name import validate_android_package_name
 from pyxform.validators.pyxform.translations_checks import SheetTranslations
@@ -362,7 +366,8 @@ def add_choices_info_to_question(
     if file_extension is None:
         file_extension = ""
 
-    question[constants.ITEMSET] = list_name
+    if not has_search_appearance_function(question):
+        question[constants.ITEMSET] = list_name
 
     if choice_filter:
         # External selects e.g. type = "select_one_external city".
diff --git a/tests/test_external_instances.py b/tests/test_external_instances.py
index 385cb832..094bf56f 100644
--- a/tests/test_external_instances.py
+++ b/tests/test_external_instances.py
@@ -610,3 +610,20 @@ def test_mixed_quotes_and_functions_in_pulldata(self):
                 '<instance id="instance4" src="jr://file-csv/instance4.csv"/>',
             ],
         )
+
+    def test_search_and_pulldata(self):
+        """Should not generate secondary instance for choice list used to configure search()"""
+        md = """
+        | survey  |                   |         |             |                    |             |
+        |         | type              | name    | label       | appearance         | calculation |
+        |         | select_one fruits | fruit   | Question 1  | search('fruits')   |             |
+        |         | calculate         | calc    |             |                    | pulldata('fruits', 'this', 'that', ${fruit}) |
+        | choices |               |       |        |
+        |         | list_name     | name  | label  |
+        |         | fruits        | na    | la     |
+        """
+        self.assertPyxformXform(
+            md=md,
+            xml__xpath_match=["/h:html/h:body/x:select1/x:item[./x:value/text()='na']"],
+            xml__excludes=['<instance id="fruits">']
+        )