Merge pull request #845 from lindsay-stevens/pyxform-812

812: Leading and trailing spaces in parameters lead to form error

Author: lognaturel

pyxform/constants.py

@@ -50,7 +50,6 @@
 CONTROL = "control"
 APPEARANCE = "appearance"
 ITEMSET = "itemset"
-RANDOMIZE = "randomize"
 CHOICE_FILTER = "choice_filter"
 PARAMETERS = "parameters"
 
@@ -102,13 +101,6 @@
 XLSX_EXTENSIONS = {".xlsx", ".xlsm"}
 SUPPORTED_FILE_EXTENSIONS = {*XLS_EXTENSIONS, *XLSX_EXTENSIONS}
 
-LOCATION_PRIORITY = "location-priority"
-LOCATION_MIN_INTERVAL = "location-min-interval"
-LOCATION_MAX_AGE = "location-max-age"
-TRACK_CHANGES = "track-changes"
-IDENTIFY_USER = "identify-user"
-TRACK_CHANGES_REASONS = "track-changes-reasons"
-
 # supported bind keywords for which external instances will be created for pulldata function
 EXTERNAL_INSTANCES = {"calculate", "constraint", "readonly", "required", "relevant"}
 
@@ -177,3 +169,64 @@ class EntityColumns(StrEnum):
 SUPPORTED_MEDIA_TYPES = {"image", "big-image", "audio", "video"}
 OR_OTHER_CHOICE = {NAME: "other", LABEL: "Other"}
 RESERVED_NAMES_SURVEY_SHEET = {META}
+
+
+##########################################################################################
+# Parameters
+##########################################################################################
+# Name enums by question type, or if shared then use a sensible question type prefix.
+# For aliased question types, use the primary documented name e.g. "image" not "photo".
+# The module question_type_dictionary.py handles default parameter values, if any.
+# Add new enums or keys alphabetical order.
+class ParametersAudio(StrEnum):
+    QUALITY = "quality"
+
+
+class ParametersAudit(StrEnum):
+    IDENTIFY_USER = "identify-user"
+    LOCATION_MAX_AGE = "location-max-age"
+    LOCATION_MIN_INTERVAL = "location-min-interval"
+    LOCATION_PRIORITY = "location-priority"
+    TRACK_CHANGES = "track-changes"
+    TRACK_CHANGES_REASONS = "track-changes-reasons"
+
+
+class ParametersGeo(StrEnum):
+    ALLOW_MOCK_ACCURACY = "allow-mock-accuracy"
+    INCREMENTAL = "incremental"
+
+
+class ParametersGeoPoint(StrEnum):
+    ALLOW_MOCK_ACCURACY = "allow-mock-accuracy"
+    CAPTURE_ACCURACY = "capture-accuracy"
+    WARNING_ACCURACY = "warning-accuracy"
+
+
+class ParametersImage(StrEnum):
+    APP = "app"
+    MAX_PIXELS = "max-pixels"
+
+
+class ParametersRange(StrEnum):
+    END = "end"
+    PLACEHOLDER = "placeholder"
+    START = "start"
+    STEP = "step"
+    TICK_INTERVAL = "tick_interval"
+    TICK_LABELSET = "tick_labelset"
+
+
+class ParametersSelect(StrEnum):
+    RANDOMIZE = "randomize"
+    SEED = "seed"
+
+
+class ParametersSelectFromFile(StrEnum):
+    LABEL = "label"
+    RANDOMIZE = "randomize"
+    SEED = "seed"
+    VALUE = "value"
+
+
+class ParametersText(StrEnum):
+    ROWS = "rows"

pyxform/errors.py

@@ -506,6 +506,21 @@ class ErrorCode(Enum):
             "be 'true' or not included."
         ),
     )
+    SURVEY_004: Detail = Detail(
+        name="Survey sheet - parameters parsing failed",
+        msg=(
+            "[row : {row}] On the 'survey' sheet, the 'parameters' value is invalid. "
+            "Parameters must be in the form of 'key1=value1 key2=value2."
+        ),
+    )
+    SURVEY_005: Detail = Detail(
+        name="Survey sheet - parameters unknown key",
+        msg=(
+            "[row : {row}] On the 'survey' sheet, the 'parameters' value is invalid. "
+            "The accepted parameter keys for this question type are '{accepted}'. "
+            "The following are invalid parameter key(s): '{rejected}'."
+        ),
+    )
 
 
 class PyXFormError(Exception):

pyxform/parsing/parameters.py

@@ -0,0 +1,55 @@
+from lark import Lark, Token, Transformer
+from lark.exceptions import LarkError
+
+from pyxform.errors import ErrorCode, PyXFormError
+from pyxform.parsing.expression import maybe_strip
+
+# Label and value are used to match against user-specified files so case should be preserved.
+CASE_SENSITIVE_VALUES = {"label", "value"}
+
+PARAMETER_GRAMMAR = r"""
+    start: pair*
+    pair: TOKEN "=" TOKEN
+    // Anything that's not a delimiter.
+    TOKEN: /[^\s=,;]+/
+    // Delimiters between key-value pairs.
+    %ignore /[\s,;]+/
+"""
+
+_PARAMETER_PARSER = Lark(PARAMETER_GRAMMAR, parser="lalr", start="start")
+
+
+class ParameterTransformer(Transformer):
+    @staticmethod
+    def start(pairs: list[tuple[str, str]]) -> dict[str, str]:
+        """Combine (key, value) tuples into a dict"""
+        return dict(pairs)
+
+    @staticmethod
+    def pair(items: list[Token, Token]) -> tuple[str, str]:
+        """Normalise matched (key, value) tokens."""
+        raw_key, raw_value = items
+        key = maybe_strip(str(raw_key).lower())
+        value = maybe_strip(str(raw_value))
+
+        if key not in CASE_SENSITIVE_VALUES:
+            value = value.lower()
+
+        return key, value
+
+
+# No token-specific (ALL_CAPS) methods, so visit_tokens=False.
+_PARAMETER_TRANSFORMER = ParameterTransformer(visit_tokens=False)
+
+
+def parse(
+    raw_parameters: str,
+    row_number: int,
+) -> dict[str, str]:
+    if not raw_parameters or not raw_parameters.strip():
+        return {}
+
+    try:
+        return _PARAMETER_TRANSFORMER.transform(_PARAMETER_PARSER.parse(raw_parameters))
+    except LarkError as e:
+        raise PyXFormError(code=ErrorCode.SURVEY_004, context={"row": row_number}) from e

pyxform/question.py

@@ -401,8 +401,12 @@ def build_xml(self, survey: "Survey"):
                 itemset_value_ref = DEFAULT_ITEMSET_VALUE_REF
                 itemset_label_ref = DEFAULT_ITEMSET_LABEL_REF
             if self.parameters is not None:
-                itemset_value_ref = self.parameters.get("value", itemset_value_ref)
-                itemset_label_ref = self.parameters.get("label", itemset_label_ref)
+                itemset_value_ref = self.parameters.get(
+                    constants.ParametersSelectFromFile.VALUE, itemset_value_ref
+                )
+                itemset_label_ref = self.parameters.get(
+                    constants.ParametersSelectFromFile.LABEL, itemset_label_ref
+                )
 
             is_previous_question = has_pyxform_reference(self.itemset)
 
@@ -451,12 +455,17 @@ def build_xml(self, survey: "Survey"):
             if self.parameters:
                 params = self.parameters
 
-                if "randomize" in params and params["randomize"] == "true":
+                if (
+                    constants.ParametersSelect.RANDOMIZE in params
+                    and params[constants.ParametersSelect.RANDOMIZE] == "true"
+                ):
                     nodeset = f"randomize({nodeset}"
 
-                    if "seed" in params:
+                    if constants.ParametersSelect.SEED in params:
                         seed = maybe_strip(
-                            survey.insert_xpaths(text=params["seed"], context=self)
+                            survey.insert_xpaths(
+                                text=params[constants.ParametersSelect.SEED], context=self
+                            )
                         )
                         nodeset = f"{nodeset}, {seed}"
 

pyxform/question_type_dictionary.py

@@ -6,7 +6,7 @@
 from types import MappingProxyType
 from typing import Any
 
-from pyxform import constants as const
+from pyxform import constants as co
 
 _QUESTION_TYPE_DICT = {
     "q picture": {
@@ -359,7 +359,11 @@
     "range": {
         "control": {"tag": "range"},
         "bind": {"type": "int"},
-        "parameters": {"start": "1", "end": "10", "step": "1"},
+        "parameters": {
+            co.ParametersRange.START.value: "1",
+            co.ParametersRange.END.value: "10",
+            co.ParametersRange.STEP.value: "1",
+        },
     },
     "audit": {"bind": {"type": "binary"}},
     "xml-external": {
@@ -390,8 +394,8 @@ def get_meta_group(children: Sequence[dict[str, Any]]) -> dict[str, Any]:
     if children is None:
         children = []
     return {
-        const.NAME: "meta",
-        const.TYPE: const.GROUP,
-        const.CONTROL: {"bodyless": True},
-        const.CHILDREN: children,
+        co.NAME: "meta",
+        co.TYPE: co.GROUP,
+        co.CONTROL: {"bodyless": True},
+        co.CHILDREN: children,
     }

pyxform/util/enum.py

@@ -28,5 +28,13 @@ def __new__(cls, *values):
         return member
 
     @classmethod
-    def value_list(cls):
+    def value_list(cls) -> list:
         return list(cls.__members__.values())
+
+    @classmethod
+    def value_set(cls) -> set:
+        return set(cls.__members__.values())
+
+    @classmethod
+    def value_str_sorted(cls) -> str:
+        return ", ".join(sorted(cls.__members__.values()))

pyxform/validators/pyxform/parameters.py

@@ -0,0 +1,26 @@
+from typing import Any
+
+from pyxform.errors import ErrorCode, PyXFormError
+from pyxform.util.enum import StrEnum
+
+PARAMETERS_TYPE = dict[str, Any]
+
+
+def validate(
+    parameters: PARAMETERS_TYPE,
+    accepted: type[StrEnum],
+    row_number: int,
+) -> dict[str, str]:
+    """
+    Raise an error if 'parameters' includes any keys not named in 'accepted'.
+    """
+    extras = set(parameters) - accepted.value_set()
+    if 0 < len(extras):
+        raise PyXFormError(
+            ErrorCode.SURVEY_005.value.format(
+                row=row_number,
+                accepted=accepted.value_str_sorted(),
+                rejected=", ".join(sorted(extras)),
+            )
+        )
+    return parameters