diff --git a/pyxform/builder.py b/pyxform/builder.py index 1d3efd063..c0f2b336c 100644 --- a/pyxform/builder.py +++ b/pyxform/builder.py @@ -118,15 +118,15 @@ def create_survey_element_from_dict( def _save_trigger(self, d: dict) -> None: if "trigger" in d: - triggering_ref = d["trigger"].strip() - value = "" - if const.BIND in d and "calculate" in d[const.BIND]: - value = d[const.BIND]["calculate"] - question_ref = (d[const.NAME], value) - if d[const.TYPE] == "background-geopoint": - self.setgeopoint_by_triggering_ref[triggering_ref].append(question_ref) - else: - self.setvalues_by_triggering_ref[triggering_ref].append(question_ref) + for trigger in d.get("trigger"): + value = "" + if const.BIND in d and "calculate" in d[const.BIND]: + value = d[const.BIND]["calculate"] + question_ref = (d[const.NAME], value) + if d[const.TYPE] == "background-geopoint": + self.setgeopoint_by_triggering_ref[trigger].append(question_ref) + else: + self.setvalues_by_triggering_ref[trigger].append(question_ref) @staticmethod def _create_question_from_dict( diff --git a/pyxform/errors.py b/pyxform/errors.py index 89ca6ff96..586f105ec 100644 --- a/pyxform/errors.py +++ b/pyxform/errors.py @@ -2,10 +2,93 @@ Common base classes for pyxform exceptions. """ +from enum import Enum +from string import Formatter +from typing import Any + + +class _ErrorFormatter(Formatter): + """Allows specifying a default for missing format keys.""" + + def __init__(self, default_value: str = "unknown"): + self.default_value: str = default_value + + def get_value(self, key, args, kwargs): + if isinstance(key, str): + return kwargs.get(key, self.default_value) + else: + return super().get_value(key, args, kwargs) + + +_ERROR_FORMATTER = _ErrorFormatter() + + +class _Detail: + """ErrorCode details.""" + + __slots__ = ("msg", "name") + + def __init__(self, name: str, msg: str) -> None: + self.name: str = name + self.msg: str = msg + + def format(self, **kwargs): + return _ERROR_FORMATTER.format(self.msg, **kwargs) + + +class ErrorCode(Enum): + PYREF_001: _Detail = _Detail( + name="PyXForm Reference Parsing Failed", + msg=( + "[row : {row}] On the '{sheet}' sheet, the '{column}' value is invalid. " + "Reference variables must start with '${{', then a question name, and end with '}}'." + ), + ) + PYREF_002: _Detail = _Detail( + name="PyXForm Reference Parsing Limit Reached", + msg=( + "[row : {row}] On the '{sheet}' sheet, the '{column}' value is invalid. " + "Reference variable lists must have a comma between each variable." + ), + ) + PYREF_003: _Detail = _Detail( + name="PyXForm Reference Question Not Found", + msg=( + "[row : {row}] On the '{sheet}' sheet, the '{column}' value is invalid. " + "Reference variables must refer to a question name. Could not find '{q}'." + ), + ) + class PyXFormError(Exception): """Common base class for pyxform exceptions.""" + def __init__( + self, *args, code: ErrorCode | None = None, context: dict[str, Any] | None = None + ) -> None: + """ + :param args: Args for the base exception, such as a pre-formatted error message. + :param code: If provided, used for an error message template. + :param context: If provided, used to format the error message template. + """ + super().__init__(*args) + self.code: ErrorCode | None = code + self.context: dict = context if context else {} + + def __str__(self): + return self.__repr__() + + def __repr__(self): + if self.code: + if self.context: + return self.code.value.format(**self.context) + else: + return self.code.value.name + elif self.args[0]: + return self.args[0] + else: + return super().__repr__() + class ValidationError(PyXFormError): """Common base class for pyxform validation exceptions.""" diff --git a/pyxform/parsing/expression.py b/pyxform/parsing/expression.py index 972cff33a..6f0edbf23 100644 --- a/pyxform/parsing/expression.py +++ b/pyxform/parsing/expression.py @@ -1,72 +1,81 @@ import re from functools import lru_cache - - -def get_lexer_rules(): - # ncname regex adapted from eulxml https://github.com/emory-libraries/eulxml/blob/2e1a9f71ffd1fd455bd8326ec82125e333b352e0/eulxml/xpath/lexrules.py - # (C) 2010,2011 Emory University Libraries [Apache v2.0 License] - # They in turn adapted it from https://www.w3.org/TR/REC-xml/#NT-NameStartChar - # and https://www.w3.org/TR/REC-xml-names/#NT-NCName - namestartchar = ( - r"([A-Z]|_|[a-z]|\xc0-\xd6]|[\xd8-\xf6]|[\xf8-\u02ff]|" - + r"[\u0370-\u037d]|[\u037f-\u1fff]|[\u200c-\u200d]|[\u2070-\u218f]|" - + r"[\u2c00-\u2fef]|[\u3001-\uD7FF]|[\uF900-\uFDCF]|[\uFDF0-\uFFFD]" - + r"|[\U00010000-\U000EFFFF])" - ) - # additional characters allowed in NCNames after the first character - namechar_extra = r"[-.0-9\xb7\u0300-\u036f\u203f-\u2040]" - ncname_regex = ( - r"(" + namestartchar + r")(" + namestartchar + r"|" + namechar_extra + r")*" - ) - ncname_regex = ncname_regex + r"(:" + ncname_regex + r")?" - - date_regex = r"-?\d{4}-\d{2}-\d{2}" - time_regex = r"\d{2}:\d{2}:\d{2}(\.\s+)?(((\+|\-)\d{2}:\d{2})|Z)?" - date_time_regex = date_regex + "T" + time_regex - - # Rule order is significant - match priority runs top to bottom. - return { - # https://www.w3.org/TR/xmlschema-2/#dateTime - "DATETIME": date_time_regex, - "DATE": date_regex, - "TIME": time_regex, - "NUMBER": r"-?\d+\.\d*|-?\.\d+|-?\d+", - # https://www.w3.org/TR/1999/REC-xpath-19991116/#exprlex - "OPS_MATH": r"[\*\+\-]| mod | div ", - "OPS_COMP": r"\=|\!\=|\<|\>|\<=|>=", - "OPS_BOOL": r" and | or ", - "OPS_UNION": r"\|", - "OPEN_PAREN": r"\(", - "CLOSE_PAREN": r"\)", - "BRACKET": r"\[\]\{\}", - "PARENT_REF": r"\.\.", - "SELF_REF": r"\.", - "PATH_SEP": r"\/", # javarosa.xpath says "//" is an "unsupported construct". - "SYSTEM_LITERAL": r""""[^"]*"|'[^']*'""", - "COMMA": r",", - "WHITESPACE": r"\s+", - "PYXFORM_REF": r"\$\{(last-saved#)?" + ncname_regex + r"\}", - "FUNC_CALL": ncname_regex + r"\(", - "XPATH_PRED_START": ncname_regex + r"\[", - "XPATH_PRED_END": r"\]", - "URI_SCHEME": ncname_regex + r"://", - "NAME": ncname_regex, # Must be after rules containing ncname_regex. - "PYXFORM_REF_START": r"\$\{", - "PYXFORM_REF_END": r"\}", - "OTHER": r".+?", # Catch any other character so that parsing doesn't stop. - } - - -LEXER_RULES = get_lexer_rules() -RE_ONLY_NCNAME = re.compile(rf"""^{LEXER_RULES["NAME"]}$""") -RE_ONLY_PYXFORM_REF = re.compile(rf"""^{LEXER_RULES["PYXFORM_REF"]}$""") -RE_ANY_PYXFORM_REF = re.compile(LEXER_RULES["PYXFORM_REF"]) +from typing import Any + +# ncname regex adapted from eulxml https://github.com/emory-libraries/eulxml/blob/2e1a9f71ffd1fd455bd8326ec82125e333b352e0/eulxml/xpath/lexrules.py +# (C) 2010,2011 Emory University Libraries [Apache v2.0 License] +# They in turn adapted it from https://www.w3.org/TR/REC-xml/#NT-NameStartChar +# and https://www.w3.org/TR/REC-xml-names/#NT-NCName +namestartchar = ( + r"(?:[A-Z]|_|[a-z]|\xc0-\xd6]|[\xd8-\xf6]|[\xf8-\u02ff]|" + + r"[\u0370-\u037d]|[\u037f-\u1fff]|[\u200c-\u200d]|[\u2070-\u218f]|" + + r"[\u2c00-\u2fef]|[\u3001-\uD7FF]|[\uF900-\uFDCF]|[\uFDF0-\uFFFD]" + + r"|[\U00010000-\U000EFFFF])" +) +# additional characters allowed in NCNames after the first character +namechar_extra = r"[-.0-9\xb7\u0300-\u036f\u203f-\u2040]" +ncname_regex = rf"{namestartchar}(?:{namestartchar}|{namechar_extra})*" +ncname_regex_named = rf"(?P{ncname_regex})" +# namespaced ncname +ncname_regex_ns = rf"{ncname_regex}(?:\:{ncname_regex})?" +ncname_regex_ns_named = rf"(?P{ncname_regex_ns})" + +date_regex = r"-?\d{4}-\d{2}-\d{2}" +time_regex = r"\d{2}:\d{2}:\d{2}(\.\s+)?(((\+|\-)\d{2}:\d{2})|Z)?" +date_time_regex = date_regex + "T" + time_regex + +# pyxform_ref_outer picks up possible refs, and matches unterminated refs to exclude them. +pyxform_ref_outer = r"\$\{(?P[^}]+)\}|\$\{[^}]*$" +pyxform_ref_inner = rf"(?Plast-saved#)?{ncname_regex_named}" +pyxform_ref_inner_last_saved_required = ( + rf"(?Plast-saved#){ncname_regex_named}" +) +pyxform_ref = rf"(?P\$\{{{pyxform_ref_inner}\}})" + +# Rule order is significant - match priority runs top to bottom. +LEXER_RULES = { + # https://www.w3.org/TR/xmlschema-2/#dateTime + "DATETIME": date_time_regex, + "DATE": date_regex, + "TIME": time_regex, + "NUMBER": r"-?\d+\.\d*|-?\.\d+|-?\d+", + # https://www.w3.org/TR/1999/REC-xpath-19991116/#exprlex + "OPS_MATH": r"[\*\+\-]| mod | div ", + "OPS_COMP": r"\=|\!\=|\<|\>|\<=|>=", + "OPS_BOOL": r" and | or ", + "OPS_UNION": r"\|", + "OPEN_PAREN": r"\(", + "CLOSE_PAREN": r"\)", + "BRACKET": r"\[\]\{\}", + "PARENT_REF": r"\.\.", + "SELF_REF": r"\.", + "PATH_SEP": r"\/", # javarosa.xpath says "//" is an "unsupported construct". + "SYSTEM_LITERAL": r""""[^"]*"|'[^']*'""", + "COMMA": r",", + "WHITESPACE": r"\s+", + "PYXFORM_REF": pyxform_ref, + "FUNC_CALL": ncname_regex_ns_named + r"\(", + "XPATH_PRED_START": ncname_regex_ns_named + r"\[", + "XPATH_PRED_END": r"\]", + "URI_SCHEME": ncname_regex_named + r"://", + "NAME": ncname_regex_named, # Must be after rules containing ncname_regex. + "PYXFORM_REF_START": r"\$\{", + "PYXFORM_REF_END": r"\}", + "OTHER": r".+?", # Catch any other character so that parsing doesn't stop. +} + + +RE_NCNAME_NAMESPACED = re.compile(ncname_regex_ns_named) +RE_PYXFORM_REF = re.compile(pyxform_ref) +RE_PYXFORM_REF_OUTER = re.compile(pyxform_ref_outer) +RE_PYXFORM_REF_INNER = re.compile(pyxform_ref_inner) def get_expression_lexer() -> re.Scanner: def get_tokenizer(name): def tokenizer(scan, value) -> ExpLexerToken | str: - return ExpLexerToken(name, value, scan.match.start(), scan.match.end()) + match = scan.match + return ExpLexerToken(name, value, match.start(), match.end()) return tokenizer @@ -104,29 +113,22 @@ def parse_expression(text: str) -> tuple[list[ExpLexerToken], str]: return tokens, remainder -def is_pyxform_reference(value: str) -> bool: - """ - Does the input string contain only a valid Pyxform reference? e.g. ${my_question} - """ - # Needs 3 characters for "${}", plus a name inside. - return value and len(value) > 3 and bool(RE_ONLY_PYXFORM_REF.match(value)) - - def is_xml_tag(value: str) -> bool: """ Does the input string contain only a valid XML tag / element name? """ - return value and bool(RE_ONLY_NCNAME.match(value)) + return value and bool(RE_NCNAME_NAMESPACED.fullmatch(value)) -def has_last_saved(value: str) -> bool: +def maybe_strip(value: Any) -> Any: """ - Does the input string contain a valid '#last-saved' Pyxform reference? e.g. ${last-saved#my_question} + If the value is a string and looks like it has whitespace at either end, strip it. + + If a string was "interned" (cached) by Python, string.strip() should generally return + the existing string if no leading/trailing whitespace was found. But strings may or + may not be interned by Python, and there may be a large cache for many unique values + (which is likely for XLSForms), so this function tries to avoid calling strip(). """ - # Needs 14 characters for "${last-saved#}", plus a name inside. - return ( - value - and len(value) > 14 - and "${last-saved#" in value - and RE_ANY_PYXFORM_REF.search(value) - ) + if isinstance(value, str) and value and (value[0].isspace() or value[-1].isspace()): + return value.strip() + return value diff --git a/pyxform/parsing/instance_expression.py b/pyxform/parsing/instance_expression.py index 3c43d53ed..2090c2602 100644 --- a/pyxform/parsing/instance_expression.py +++ b/pyxform/parsing/instance_expression.py @@ -1,7 +1,7 @@ from typing import TYPE_CHECKING -from pyxform.parsing.expression import parse_expression -from pyxform.utils import BRACKETED_TAG_REGEX, node +from pyxform.parsing.expression import RE_PYXFORM_REF, parse_expression +from pyxform.utils import node if TYPE_CHECKING: from pyxform.survey import Survey @@ -99,7 +99,7 @@ def replace_with_output(xml_text: str, context: "SurveyElement", survey: "Survey :return: The possibly modified string. """ # 9 = len("instance(") - if 9 >= len(xml_text): + if len(xml_text) <= 9 or "instance(" not in xml_text: return xml_text boundaries = find_boundaries(xml_text=xml_text) if boundaries: @@ -108,7 +108,7 @@ def replace_with_output(xml_text: str, context: "SurveyElement", survey: "Survey old_str = xml_text[start:end] # Pass the new string through the pyxform reference replacer. # noinspection PyProtectedMember - new_str = BRACKETED_TAG_REGEX.sub( + new_str = RE_PYXFORM_REF.sub( lambda m: survey._var_repl_function(m, context), old_str, ) diff --git a/pyxform/question.py b/pyxform/question.py index bf4850804..9ab09ba50 100644 --- a/pyxform/question.py +++ b/pyxform/question.py @@ -3,7 +3,6 @@ """ import os.path -import re from collections.abc import Callable, Generator, Iterable from itertools import chain from typing import TYPE_CHECKING @@ -17,16 +16,16 @@ EXTERNAL_INSTANCE_EXTENSIONS, ) from pyxform.errors import PyXFormError -from pyxform.parsing.expression import RE_ANY_PYXFORM_REF +from pyxform.parsing.expression import maybe_strip from pyxform.question_type_dictionary import QUESTION_TYPE_DICT from pyxform.survey_element import SURVEY_ELEMENT_FIELDS, SurveyElement from pyxform.utils import ( - PYXFORM_REFERENCE_REGEX, DetachableElement, combine_lists, default_is_dynamic, node, ) +from pyxform.validators.pyxform.pyxform_reference import has_pyxform_reference if TYPE_CHECKING: from pyxform.survey import Survey @@ -197,7 +196,7 @@ def xml_action(self) -> DetachableElement | None: def nest_set_nodes(self, survey, xml_node, tag, nested_items): for item in nested_items: node_attrs = { - "ref": survey.insert_xpaths(f"${{{item[0]}}}", survey).strip(), + "ref": survey.get_element_by_name(item[0]).get_xpath(), "event": "xforms-value-changed", } if item[1]: @@ -239,6 +238,23 @@ def to_json_dict(self, delete_keys: Iterable[str] | None = None) -> dict: result[k] = v return result + def get_setvalue_node_for_dynamic_default( + self, survey: "Survey", in_repeat=False + ) -> DetachableElement | None: + if not self.default or not default_is_dynamic(self.default, self.type): + return None + + triggering_events = "odk-instance-first-load" + if in_repeat: + triggering_events = f"{triggering_events} odk-new-repeat" + + return node( + "setvalue", + ref=self.get_xpath(), + value=survey.insert_xpaths(text=self.default, context=self), + event=triggering_events, + ) + class InputQuestion(Question): """ @@ -331,10 +347,7 @@ def get_options(self, choices: Iterable[dict]) -> Generator[Option, None, None]: if label_is_dict: requires_itext = True # Dynamic label: string contains a pyxform reference. - elif ( - choice_label - and re.search(RE_ANY_PYXFORM_REF, choice_label) is not None - ): + elif choice_label and has_pyxform_reference(choice_label): requires_itext = True yield option self.requires_itext = requires_itext @@ -391,7 +404,7 @@ def build_xml(self, survey: "Survey"): itemset_value_ref = self.parameters.get("value", itemset_value_ref) itemset_label_ref = self.parameters.get("label", itemset_label_ref) - is_previous_question = bool(PYXFORM_REFERENCE_REGEX.search(self.itemset)) + is_previous_question = has_pyxform_reference(self.itemset) if file_extension in EXTERNAL_INSTANCE_EXTENSIONS: pass @@ -437,11 +450,10 @@ def build_xml(self, survey: "Survey"): nodeset = f"randomize({nodeset}" if "seed" in params: - if params["seed"].startswith("${"): - seed = survey.insert_xpaths(params["seed"], self).strip() - nodeset = f"{nodeset}, {seed}" - else: - nodeset = f"""{nodeset}, {params["seed"]}""" + seed = maybe_strip( + survey.insert_xpaths(text=params["seed"], context=self) + ) + nodeset = f"{nodeset}, {seed}" nodeset += ")" diff --git a/pyxform/survey.py b/pyxform/survey.py index 07d5e6321..970c58dc3 100644 --- a/pyxform/survey.py +++ b/pyxform/survey.py @@ -18,13 +18,12 @@ from pyxform.errors import PyXFormError, ValidationError from pyxform.external_instance import ExternalInstance from pyxform.instance import SurveyInstance -from pyxform.parsing.expression import has_last_saved +from pyxform.parsing.expression import RE_PYXFORM_REF from pyxform.parsing.instance_expression import replace_with_output from pyxform.question import Itemset, MultipleChoiceQuestion, Option, Question, Tag from pyxform.section import SECTION_EXTRA_FIELDS, Section -from pyxform.survey_element import SURVEY_ELEMENT_FIELDS, SurveyElement +from pyxform.survey_element import _GET_SENTINEL, SURVEY_ELEMENT_FIELDS, SurveyElement from pyxform.utils import ( - BRACKETED_TAG_REGEX, LAST_SAVED_INSTANCE_NAME, DetachableElement, escape_text_for_xml, @@ -32,6 +31,10 @@ ) from pyxform.validators import enketo_validate, odk_validate from pyxform.validators.pyxform.iana_subtags.validation import get_languages_with_bad_tags +from pyxform.validators.pyxform.pyxform_reference import ( + has_pyxform_reference_with_last_saved, + is_pyxform_reference_candidate, +) RE_BRACKET = re.compile(r"\[([^]]+)\]") RE_FUNCTION_ARGS = re.compile(r"\b[^()]+\((.*)\)$") @@ -342,15 +345,6 @@ def xml(self): self.validate() self._setup_xpath_dictionary() - for triggering_reference in self.setvalues_by_triggering_ref: - if not re.search(BRACKETED_TAG_REGEX, triggering_reference): - raise PyXFormError( - "Only references to other fields are allowed in the 'trigger' column." - ) - - # try to resolve reference and fail if can't - self.insert_xpaths(triggering_reference, self) - body_kwargs = {} if self.style: body_kwargs["class"] = self.style @@ -365,9 +359,9 @@ def xml(self): def get_trigger_values_for_question_name(self, question_name: str, trigger_type: str): if trigger_type == "setvalue": - return self.setvalues_by_triggering_ref.get(f"${{{question_name}}}") + return self.setvalues_by_triggering_ref.get(question_name) elif trigger_type == "setgeopoint": - return self.setgeopoint_by_triggering_ref.get(f"${{{question_name}}}") + return self.setgeopoint_by_triggering_ref.get(question_name) def _generate_static_instances( self, list_name: str, itemset: Itemset @@ -531,16 +525,23 @@ def _generate_last_saved_instance(element: Question) -> bool: """ True if a last-saved instance should be generated, false otherwise. """ - if has_last_saved(element.default): + if element.default and has_pyxform_reference_with_last_saved(element.default): return True - if has_last_saved(element.choice_filter): + if element.choice_filter and has_pyxform_reference_with_last_saved( + element.choice_filter + ): return True if element.bind: # Assuming average len(bind) < 10 and len(EXTERNAL_INSTANCES) = 5 and the # current has_last_saved implementation, iterating bind keys is fastest. for k, v in element.bind.items(): - if k in constants.EXTERNAL_INSTANCES and has_last_saved(v): + if ( + k in constants.EXTERNAL_INSTANCES + and v + and has_pyxform_reference_with_last_saved(v) + ): return True + return False @staticmethod def _get_last_saved_instance() -> InstanceInfo: @@ -1084,6 +1085,24 @@ def _setup_xpath_dictionary(self): xpaths[element_name] = element self._xpath = xpaths + def get_element_by_name( + self, name: str, error_prefix: str | None = None + ) -> SurveyElement: + element = self._xpath.get(name, _GET_SENTINEL) + + prefix = "" + if error_prefix: + prefix = f"{error_prefix} " + + if element is _GET_SENTINEL: + raise PyXFormError(f"{prefix}There is no survey element named '{name}'.") + elif element is None: + raise PyXFormError( + f"{prefix}There are multiple survey elements named '{name}'." + ) + + return element + def _var_repl_function( self, matchobj, context, use_current=False, reference_parent=False ): @@ -1092,8 +1111,8 @@ def _var_repl_function( replace ${varname} with the xpath to varname. """ - name = matchobj.group(2) - last_saved = matchobj.group(1) is not None + name = matchobj.group("ncname") + last_saved = matchobj.group("last_saved") is not None is_indexed_repeat = matchobj.string.find("indexed-repeat(") > -1 def _in_secondary_instance_predicate() -> bool: @@ -1114,15 +1133,16 @@ def _in_secondary_instance_predicate() -> bool: return False return False - def _relative_path(ref_name: str, _use_current: bool) -> str | None: + def _relative_path( + ref_name: str, _use_current: bool, _target_xpath: str + ) -> str | None: """Given name in ${name}, return relative xpath to ${name}.""" return_path = None - xpath = self._xpath[ref_name].get_xpath() context_xpath = context.get_xpath() # share same root i.e repeat_a from /data/repeat_a/... if ( len(context_xpath.split("/")) > 2 - and xpath.split("/")[2] == context_xpath.split("/")[2] + and _target_xpath.split("/")[2] == context_xpath.split("/")[2] ): # if context xpath and target xpath fall under the same # repeat use relative xpath referencing. @@ -1131,7 +1151,7 @@ def _relative_path(ref_name: str, _use_current: bool) -> str | None: return return_path else: steps, ref_path = share_same_repeat_parent( - self, xpath, context_xpath, reference_parent + self, _target_xpath, context_xpath, reference_parent ) if steps: ref_path = ref_path if ref_path.endswith(ref_name) else f"/{name}" @@ -1187,27 +1207,24 @@ def _is_return_relative_path() -> bool: return False intro = ( - f"There has been a problem trying to replace {matchobj.group(0)} with the " - f"XPath to the survey element named '{name}'." + f"""There has been a problem trying to replace {matchobj.group("pyxform_ref")} """ + f"""with the XPath to the survey element named '{name}'.""" ) - if name not in self._xpath: - raise PyXFormError(intro + " There is no survey element with this name.") - if self._xpath[name] is None: - raise PyXFormError( - intro + " There are multiple survey elements with this name." - ) + target_xpath = self.get_element_by_name(name=name, error_prefix=intro).get_xpath() if _is_return_relative_path(): if not use_current: use_current = _in_secondary_instance_predicate() - relative_path = _relative_path(ref_name=name, _use_current=use_current) + relative_path = _relative_path( + ref_name=name, _use_current=use_current, _target_xpath=target_xpath + ) if relative_path: return relative_path last_saved_prefix = ( f"instance('{LAST_SAVED_INSTANCE_NAME}')" if last_saved else "" ) - return f" {last_saved_prefix}{self._xpath[name].get_xpath()} " + return f" {last_saved_prefix}{target_xpath} " def insert_xpaths( self, @@ -1218,15 +1235,25 @@ def insert_xpaths( ): """ Replace all instances of ${var} with the xpath to var. + + :param text: The string to perform dereferencing on. + :param context: The context to use for relative references (if any). + :param use_current: If True, use 'current()' in the relative reference (if any). + :param reference_parent: Reference the lowest common ancestor repeat parent, + rather than using the shortest possible relative path. """ + # "text" may actually be a dict, e.g. for custom attributes. + value = str(text) + + if not is_pyxform_reference_candidate(value): + return value def _var_repl_function(matchobj): return self._var_repl_function( matchobj, context, use_current, reference_parent ) - # "text" may actually be a dict, e.g. for custom attributes. - return re.sub(BRACKETED_TAG_REGEX, _var_repl_function, str(text)) + return re.sub(RE_PYXFORM_REF, _var_repl_function, value) def _var_repl_output_function(self, matchobj, context): """ @@ -1265,12 +1292,12 @@ def _var_repl_output_function(matchobj): # need to make sure we have reason to replace # since at this point < is <, # the net effect < gets translated again to &lt; - xml_text = replace_with_output(original_xml, context, self) - if "{" in xml_text: - xml_text = re.sub(BRACKETED_TAG_REGEX, _var_repl_output_function, xml_text) - changed = xml_text != original_xml + value = replace_with_output(original_xml, context, self) + if is_pyxform_reference_candidate(value): + value = re.sub(RE_PYXFORM_REF, _var_repl_output_function, value) + changed = value != original_xml if changed: - return xml_text, True + return value, True else: return text, False diff --git a/pyxform/survey_element.py b/pyxform/survey_element.py index 4b1e6c455..d52fc1854 100644 --- a/pyxform/survey_element.py +++ b/pyxform/survey_element.py @@ -13,12 +13,9 @@ from pyxform import constants as const from pyxform.errors import PyXFormError from pyxform.parsing.expression import is_xml_tag -from pyxform.utils import ( - BRACKETED_TAG_REGEX, - INVALID_XFORM_TAG_REGEXP, - DetachableElement, - default_is_dynamic, - node, +from pyxform.utils import INVALID_XFORM_TAG_REGEXP, DetachableElement, node +from pyxform.validators.pyxform.pyxform_reference import ( + has_pyxform_reference, ) from pyxform.xls2json import print_pyobj_to_json @@ -377,7 +374,7 @@ def get_translations(self, default_language): "text": text, "output_context": self, } - elif constraint_msg and re.search(BRACKETED_TAG_REGEX, constraint_msg): + elif constraint_msg and has_pyxform_reference(constraint_msg): yield { "path": self._translation_path("jr:constraintMsg"), "lang": default_language, @@ -394,7 +391,7 @@ def get_translations(self, default_language): "text": text, "output_context": self, } - elif required_msg and re.search(BRACKETED_TAG_REGEX, required_msg): + elif required_msg and has_pyxform_reference(required_msg): yield { "path": self._translation_path("jr:requiredMsg"), "lang": default_language, @@ -463,26 +460,10 @@ def needs_itext_ref(self): hasattr(self, const.MEDIA) and isinstance(self.media, dict) and self.media ) - def get_setvalue_node_for_dynamic_default(self, survey: "Survey", in_repeat=False): - if ( - not hasattr(self, "default") - or not self.default - or not default_is_dynamic(self.default, self.type) - ): - return None - - default_with_xpath_paths = survey.insert_xpaths(self.default, self) - - triggering_events = "odk-instance-first-load" - if in_repeat: - triggering_events += " odk-new-repeat" - - return node( - "setvalue", - ref=self.get_xpath(), - value=default_with_xpath_paths, - event=triggering_events, - ) + def get_setvalue_node_for_dynamic_default( + self, survey: "Survey", in_repeat=False + ) -> DetachableElement | None: + return None # XML generating functions, these probably need to be moved around. def xml_label(self, survey: "Survey"): @@ -563,17 +544,17 @@ def xml_bindings( # I think all the binding conversions should be happening on # the xls2json side. if ( - hashable(v) + k in const.CONVERTIBLE_BIND_ATTRIBUTES + and hashable(v) and v in alias.BINDING_CONVERSIONS - and k in const.CONVERTIBLE_BIND_ATTRIBUTES ): v = alias.BINDING_CONVERSIONS[v] elif k == "jr:constraintMsg" and ( - isinstance(v, dict) or re.search(BRACKETED_TAG_REGEX, v) + isinstance(v, dict) or has_pyxform_reference(v) ): v = f"""jr:itext('{self._translation_path("jr:constraintMsg")}')""" elif k == "jr:requiredMsg" and ( - isinstance(v, dict) or re.search(BRACKETED_TAG_REGEX, v) + isinstance(v, dict) or has_pyxform_reference(v) ): v = f"""jr:itext('{self._translation_path("jr:requiredMsg")}')""" elif k == "jr:noAppErrorString" and isinstance(v, dict): diff --git a/pyxform/utils.py b/pyxform/utils.py index bc5faeae4..7f1c3f830 100644 --- a/pyxform/utils.py +++ b/pyxform/utils.py @@ -21,11 +21,9 @@ from pyxform.parsing.expression import parse_expression from pyxform.xls2json_backends import DefinitionData -BRACKETED_TAG_REGEX = re.compile(r"\${(last-saved#)?(.*?)}") INVALID_XFORM_TAG_REGEXP = re.compile(r"[^a-zA-Z:_][^a-zA-Z:_0-9\-.]*") LAST_SAVED_INSTANCE_NAME = "__last-saved" NODE_TYPE_TEXT = {Node.TEXT_NODE, Node.CDATA_SECTION_NODE} -PYXFORM_REFERENCE_REGEX = re.compile(r"\$\{(.*?)\}") SPACE_TRANS_TABLE = str.maketrans({" ": "_"}) XML_TEXT_SUBS = {"&": "&", "<": "<", ">": ">"} XML_TEXT_TABLE = str.maketrans(XML_TEXT_SUBS) diff --git a/pyxform/validators/pyxform/iana_subtags/validation.py b/pyxform/validators/pyxform/iana_subtags/validation.py index 4cd368426..a65481a1b 100644 --- a/pyxform/validators/pyxform/iana_subtags/validation.py +++ b/pyxform/validators/pyxform/iana_subtags/validation.py @@ -2,6 +2,8 @@ from functools import lru_cache from pathlib import Path +from pyxform.parsing.expression import maybe_strip + LANG_CODE_REGEX = re.compile(r"\((.*)\)$") HERE = Path(__file__).parent @@ -10,7 +12,7 @@ def read_tags(file_name: str) -> set[str]: path = HERE / file_name with open(path, encoding="utf-8") as f: - return {line.strip() for line in f} + return {maybe_strip(line) for line in f} def get_languages_with_bad_tags(languages): diff --git a/pyxform/validators/pyxform/parameters_generic.py b/pyxform/validators/pyxform/parameters_generic.py index a0524d9aa..ee5769a69 100644 --- a/pyxform/validators/pyxform/parameters_generic.py +++ b/pyxform/validators/pyxform/parameters_generic.py @@ -1,12 +1,13 @@ -from collections.abc import Sequence +from collections.abc import Collection from typing import Any from pyxform.errors import PyXFormError +from pyxform.parsing.expression import maybe_strip PARAMETERS_TYPE = dict[str, Any] # Label and value are used to match against user-specified files so case should be preserved. -CASE_SENSITIVE_VALUES = ["label", "value"] +CASE_SENSITIVE_VALUES = {"label", "value"} def parse(raw_parameters: str) -> PARAMETERS_TYPE: @@ -24,15 +25,15 @@ def parse(raw_parameters: str) -> PARAMETERS_TYPE: "'parameter1=value parameter2=value'." ) k, v = param.split("=")[:2] - key = k.lower().strip() - params[key] = v.strip() if key in CASE_SENSITIVE_VALUES else v.lower().strip() + key = maybe_strip(k.lower()) + params[key] = v if key in CASE_SENSITIVE_VALUES else maybe_strip(v.lower()) return params def validate( parameters: PARAMETERS_TYPE, - allowed: Sequence[str], + allowed: Collection[str], ) -> dict[str, str]: """ Raise an error if 'parameters' includes any keys not named in 'allowed'. diff --git a/pyxform/validators/pyxform/pyxform_reference.py b/pyxform/validators/pyxform/pyxform_reference.py index 45c6c60a5..e50510575 100644 --- a/pyxform/validators/pyxform/pyxform_reference.py +++ b/pyxform/validators/pyxform/pyxform_reference.py @@ -1,56 +1,170 @@ -from pyxform import constants as co -from pyxform.errors import PyXFormError -from pyxform.parsing.expression import parse_expression +""" +Helpers for parsing and validating pyxform reference variables from strings. + +The relatively small LRU cache sizes used here attempt to balance: +a) how expensive is the function? Regex and/or Scanner is worse than len and membership, + and there is also a cost to hash/lookup from the cache. +b) how much memory is it reasonable to spend on getting a high cache hit ratio vs. + lower ratio with extra time re-parsing; and the memory for the key and return value? +c) how likely is it that a large variety of unique strings are present in a XLSForm, and + how likely is it that similar strings are close to each other vs. randomly dispersed? +""" + +from collections.abc import Generator +from functools import lru_cache -PYXFORM_REFERENCE_INVALID = ( - "[row : {row_number}] On the '{sheet}' sheet, the '{column}' value is invalid. " - "Reference expressions must only include question names, and end with '}}'." +from pyxform import constants as co +from pyxform.errors import ErrorCode, PyXFormError +from pyxform.parsing.expression import ( + RE_PYXFORM_REF_INNER, + RE_PYXFORM_REF_OUTER, ) +def is_pyxform_reference_candidate(value: str) -> bool: + """ + Does the string look like a pyxform reference? + + Needs 2 characters for "${", plus at least 1 more for a name inside. Does not look + for closing brace because full parsing will try to detect malformed references. This + pre-check can help avoid more expensive full parsing. + + :param value: The string to inspect. + """ + return len(value) > 2 and "${" in value + + +def _parse( + value: str, + match_limit: int | None = None, + match_full: bool = False, +) -> Generator[str, None, None]: + """ + Parse the string and return reference target(s) e.g. `name` from `${name}`. + + It is an error if the reference token contains anything other than a valid ncname + (a question name), optionally with the `last-saved#` prefix. + + Does not otherwise treat `last-saved#` differently since https://docs.getodk.org/form-logic/ + says: "References to the last saved record could be used as part of any expression + wherever expressions are allowed". + + :param value: The string to inspect. + :param match_limit: If provided, parse only this many references in the string, and if + more references than the limit are found, then raise an error. + :param match_full: If True, require that the string contains a reference and nothing + else (no other characters or references). + """ + if not is_pyxform_reference_candidate(value): + return None + + if match_full: + outer_matches = (RE_PYXFORM_REF_OUTER.fullmatch(value),) + else: + outer_matches = RE_PYXFORM_REF_OUTER.finditer(value) + + count = 0 + # Look for any possible matches, then check each one for valid reference syntax. + for match in outer_matches: + ref_candidate = match.group("pyxform_ref") + # Although it's an "any" match pattern, fullmatch is used to require "only". + # Return the ref_candidate since it has original string start/end positions. + if ref_candidate and RE_PYXFORM_REF_INNER.fullmatch(ref_candidate): + if match_limit is not None and count >= match_limit: + raise PyXFormError(code=ErrorCode.PYREF_002) + + yield ref_candidate + count += 1 + else: + raise PyXFormError(code=ErrorCode.PYREF_001) + + +@lru_cache(maxsize=128) +def is_pyxform_reference(value: str) -> bool: + """ + Does the input string contain only a valid Pyxform reference? e.g. `${my_question}` + + :param value: The string to inspect. + """ + try: + return next(_parse(value=value, match_full=True), None) is not None + except (StopIteration, PyXFormError): + return False + + +@lru_cache(maxsize=128) +def has_pyxform_reference(value: str) -> bool: + """ + Does the input string contain a valid Pyxform reference? e.g. `hi ${name}` + + :param value: The string to inspect. + """ + try: + return next(_parse(value=value), None) is not None + except (StopIteration, PyXFormError): + return False + + +@lru_cache(maxsize=128) +def has_pyxform_reference_with_last_saved(value: str) -> bool: + """ + Does the input string contain a valid '#last-saved' reference? e.g. `${last-saved#my_question}` + + Needs 14 characters for "${last-saved#}", plus a name inside. This pre-check can help + avoid more expensive full parsing. + + :param value: The string to inspect. + """ + try: + return len(value) > 14 and any( + i.startswith("last-saved#") for i in _parse(value=value) + ) + except (StopIteration, PyXFormError): + return False + + +@lru_cache(maxsize=128) +def parse_pyxform_references( + value: str, + match_limit: int | None = None, +) -> tuple[str, ...]: + """ + Parse all pyxform references in a string. + + :param value: The string to inspect. + :param match_limit: If provided, parse only this many references in the string, and if + more references than the limit are found, then raise an error. + """ + return tuple(_parse(value=value, match_limit=match_limit)) + + def validate_pyxform_reference_syntax( - value: str, sheet_name: str, row_number: int, key: str -) -> None: - # Needs 3 characters for "${}" plus a name inside, but need to catch ${ for warning. - if not value or len(value) <= 2 or "${" not in value: - return + value: str, sheet_name: str, row_number: int, column: str +) -> tuple[str, ...] | None: + """ + Parse all pyxform references in a string, and raise an error if any are malformed. + + Generally the same as `parse_pyxform_references` but adds the XLSForm context to the + error message, if any. + + :param value: The string to inspect. + :param sheet_name: The XLSForm sheet the value is from. + :param row_number: The XLSForm row the value is from. + :param column: The XLSForm column the value is from. + """ # Skip columns in potentially large sheets where references are not allowed. - elif sheet_name == co.SURVEY: - if key in {co.TYPE, co.NAME}: - return + if sheet_name == co.SURVEY: + if column in {co.TYPE, co.NAME}: + return None elif sheet_name == co.CHOICES: - if key in {co.LIST_NAME_S, co.LIST_NAME_U, co.NAME}: - return + if column in {co.LIST_NAME_S, co.LIST_NAME_U, co.NAME}: + return None elif sheet_name == co.ENTITIES: - if key in {co.LIST_NAME_S, co.LIST_NAME_U}: - return - - tokens, _ = parse_expression(value) - start_token = None - - for t in tokens: - # The start of an expression. - if t is not None and t.name == "PYXFORM_REF_START" and start_token is None: - start_token = t - # Tokens that are part of an expression. - elif start_token is not None: - if t.name == "NAME": - continue - elif t.name == "PYXFORM_REF_END": - start_token = None - elif t.name in {"PYXFORM_REF_START", "PYXFORM_REF"}: - msg = PYXFORM_REFERENCE_INVALID.format( - sheet=sheet_name, row_number=row_number, column=key - ) - raise PyXFormError(msg) - else: - msg = PYXFORM_REFERENCE_INVALID.format( - sheet=sheet_name, row_number=row_number, column=key - ) - raise PyXFormError(msg) - - if start_token is not None: - msg = PYXFORM_REFERENCE_INVALID.format( - sheet=sheet_name, row_number=row_number, column=key - ) - raise PyXFormError(msg) + if column in {co.LIST_NAME_S, co.LIST_NAME_U}: + return None + + try: + return parse_pyxform_references(value=value) + except PyXFormError as e: + e.context.update(sheet=sheet_name, column=column, row=row_number) + raise diff --git a/pyxform/validators/pyxform/question_types.py b/pyxform/validators/pyxform/question_types.py index 3c62673f9..2fef51c9c 100644 --- a/pyxform/validators/pyxform/question_types.py +++ b/pyxform/validators/pyxform/question_types.py @@ -2,14 +2,21 @@ Validations for question types. """ -from pyxform.errors import PyXFormError -from pyxform.parsing.expression import is_pyxform_reference -from pyxform.utils import PYXFORM_REFERENCE_REGEX - -BACKGROUND_GEOPOINT_CALCULATION = "[row : {r}] For 'background-geopoint' questions, the 'calculation' column must be empty." -TRIGGER_INVALID = ( - "[row : {r}] For '{t}' questions, the 'trigger' column must be a reference to another " - "question that exists, in the format ${{question_name_here}}." +from collections.abc import Collection, Iterable + +from pyxform.errors import ErrorCode, PyXFormError +from pyxform.validators.pyxform.pyxform_reference import ( + is_pyxform_reference_candidate, + parse_pyxform_references, +) + +BACKGROUND_GEOPOINT_CALCULATION = ( + "[row : {r}] For 'background-geopoint' questions, " + "the 'calculation' column must be empty." +) +BACKGROUND_GEOPOINT_TRIGGER = ( + "[row : {r}] For 'background-geopoint' questions, " + "the 'trigger' column must not be empty." ) @@ -23,19 +30,38 @@ def validate_background_geopoint_calculation(row: dict, row_num: int) -> bool: raise PyXFormError(BACKGROUND_GEOPOINT_CALCULATION.format(r=row_num)) -def validate_background_geopoint_trigger(row: dict, row_num: int) -> bool: +def validate_background_geopoint_trigger(trigger: str | None, row_num: int) -> bool: """A background-geopoint must have a trigger.""" - if not row.get("trigger", False) or not is_pyxform_reference(value=row["trigger"]): - raise PyXFormError(TRIGGER_INVALID.format(r=row_num, t=row["type"])) + if not trigger: + raise PyXFormError(BACKGROUND_GEOPOINT_TRIGGER.format(r=row_num)) return True -def validate_references(referrers: list[tuple[dict, int]], questions: set[str]) -> bool: - """Triggers must refer to a question that exists.""" - for row, row_num in referrers: - matches = PYXFORM_REFERENCE_REGEX.match(row["trigger"]) - if matches is not None: - trigger = matches.groups()[0] - if trigger not in questions: - raise PyXFormError(TRIGGER_INVALID.format(r=row_num, t=row["type"])) +def validate_references( + referrers: Iterable[Iterable[str | None, int]], questions: Collection[str] +) -> bool: + """Pyxform references must refer to a question that exists.""" + for target, row_num in referrers: + if target not in questions: + raise PyXFormError( + code=ErrorCode.PYREF_003, context={"q": target, "row": row_num} + ) return True + + +def parse_trigger(trigger: str | None) -> tuple[str, ...] | None: + """A trigger may contain one pyxform reference, or multiple comma-separated references.""" + if not trigger: + return None + + if is_pyxform_reference_candidate(trigger): + trigger_values = trigger.split(",") + trigger_refs = tuple( + r + for t in trigger_values + for r in parse_pyxform_references(value=t, match_limit=1) + ) + if trigger_refs: + return trigger_refs + else: + raise PyXFormError(code=ErrorCode.PYREF_001, context={"q": trigger}) diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 335ad049a..bfa55d461 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -21,18 +21,21 @@ validate_entity_saveto, ) from pyxform.errors import PyXFormError -from pyxform.parsing.expression import is_pyxform_reference, is_xml_tag -from pyxform.parsing.sheet_headers import dealias_and_group_headers -from pyxform.utils import ( - PYXFORM_REFERENCE_REGEX, - coalesce, - default_is_dynamic, +from pyxform.parsing.expression import ( + is_xml_tag, + maybe_strip, ) +from pyxform.parsing.sheet_headers import dealias_and_group_headers +from pyxform.utils import coalesce, default_is_dynamic from pyxform.validators.pyxform import parameters_generic, select_from_file from pyxform.validators.pyxform import question_types as qt from pyxform.validators.pyxform.android_package_name import validate_android_package_name from pyxform.validators.pyxform.choices import validate_and_clean_choices -from pyxform.validators.pyxform.pyxform_reference import validate_pyxform_reference_syntax +from pyxform.validators.pyxform.pyxform_reference import ( + has_pyxform_reference, + is_pyxform_reference, + validate_pyxform_reference_syntax, +) from pyxform.validators.pyxform.sheet_misspellings import find_sheet_misspellings from pyxform.validators.pyxform.translations_checks import SheetTranslations from pyxform.xls2json_backends import ( @@ -101,12 +104,12 @@ def clean_text_values( if isinstance(value, str) and value: # Remove extraneous whitespace characters. if strip_whitespace: - value = RE_WHITESPACE.sub(" ", value.strip()) + value = RE_WHITESPACE.sub(" ", maybe_strip(value)) # Replace "smart" quotes with regular quotes. row[key] = RE_SMART_QUOTES.sub(lambda m: SMART_QUOTES[m.group(0)], value) # Check cross reference syntax. validate_pyxform_reference_syntax( - value=value, sheet_name=sheet_name, row_number=row_number, key=key + value=value, sheet_name=sheet_name, row_number=row_number, column=key ) if add_row_number: row["__row"] = row_number @@ -262,7 +265,7 @@ def add_choices_info_to_question( # Select from file e.g. type = "select_one_from_file cities.xml". or file_extension in EXTERNAL_INSTANCE_EXTENSIONS # Select from previous answers e.g. type = "select_one ${q1}". - or bool(PYXFORM_REFERENCE_REGEX.search(list_name)) + or has_pyxform_reference(list_name) ): question[constants.LIST_NAME_U] = list_name question[constants.CHOICES] = choices[list_name] @@ -999,7 +1002,7 @@ def workbook_to_json( list_name not in choices and select_type != constants.SELECT_ONE_EXTERNAL and file_extension not in EXTERNAL_INSTANCE_EXTENSIONS - and not PYXFORM_REFERENCE_REGEX.search(list_name) + and not has_pyxform_reference(list_name) ): if not choices: k = constants.CHOICES @@ -1115,7 +1118,7 @@ def workbook_to_json( ) if "seed" in parameters: - if not parameters["seed"].startswith("${"): + if not is_pyxform_reference(parameters["seed"]): try: float(parameters["seed"]) except ValueError as seed_err: @@ -1368,17 +1371,30 @@ def workbook_to_json( continue # TODO: Consider adding some question_type validation here. - # Ensure that + trigger = row.get("trigger") + if trigger: + try: + row["trigger"] = qt.parse_trigger(trigger=trigger) + except PyXFormError as e: + e.context.update(sheet="survey", column="trigger", row=row_number) + raise + trigger_references.extend((t, row_number) for t in row["trigger"]) + if question_type == "background-geopoint": - qt.validate_background_geopoint_trigger(row=row, row_num=row_number) + qt.validate_background_geopoint_trigger( + trigger=row.get("trigger"), row_num=row_number + ) qt.validate_background_geopoint_calculation(row=row, row_num=row_number) - trigger_references.append((row, row_number)) # Put the row in the json dict as is: parent_children_array.append(row) sheet_translations.or_other_check(warnings=warnings) - qt.validate_references(referrers=trigger_references, questions=question_names) + try: + qt.validate_references(referrers=trigger_references, questions=question_names) + except PyXFormError as e: + e.context.update(sheet="survey", column="trigger") + raise if len(stack) != 1: raise PyXFormError( diff --git a/tests/test_background_geopoint.py b/tests/test_background_geopoint.py index a7705facf..cdb32c18b 100644 --- a/tests/test_background_geopoint.py +++ b/tests/test_background_geopoint.py @@ -1,3 +1,4 @@ +from pyxform.errors import ErrorCode from pyxform.validators.pyxform import question_types as qt from tests.pyxform_test_case import PyxformTestCase @@ -18,7 +19,7 @@ def test_error__missing_trigger(self): name="data", md=md, errored=True, - error__contains=[qt.TRIGGER_INVALID.format(r=3, t="background-geopoint")], + error__contains=[qt.BACKGROUND_GEOPOINT_TRIGGER.format(r=3)], ) def test_error__invalid_trigger(self): @@ -30,10 +31,13 @@ def test_error__invalid_trigger(self): | | background-geopoint | temp_geo | | ${invalid} | """ self.assertPyxformXform( - name="data", md=md, errored=True, - error__contains=[qt.TRIGGER_INVALID.format(r=3, t="background-geopoint")], + error__contains=[ + ErrorCode.PYREF_003.value.format( + sheet="survey", column="trigger", row="3", q="invalid" + ) + ], ) def test_error__calculation_exists(self): @@ -45,7 +49,6 @@ def test_error__calculation_exists(self): | | background-geopoint | temp_geo | | ${temp} | 5 * temp | """ self.assertPyxformXform( - name="data", md=md, errored=True, error__contains=[qt.BACKGROUND_GEOPOINT_CALCULATION.format(r=3)], diff --git a/tests/test_dynamic_default.py b/tests/test_dynamic_default.py index fcda82b4c..d2c31b228 100644 --- a/tests/test_dynamic_default.py +++ b/tests/test_dynamic_default.py @@ -776,14 +776,14 @@ def test_dynamic_default_performance__time(self): """ Should find the dynamic default check costs little extra relative time large forms. - Results with Python 3.10.14 on VM with 2vCPU (i7-7700HQ) 1GB RAM, x questions + Results with Python 3.12.11 on VM with 2vCPU (i7-7700HQ) 2GB RAM, x questions each, average of 10 runs (seconds), with and without the check, per question: | num | with | without | peak RSS MB | - | 500 | 0.2203 | 0.1610 | 59 | - | 1000 | 0.2851 | 0.2580 | 63 | - | 2000 | 0.5001 | 0.5330 | 71 | - | 5000 | 1.2762 | 1.2931 | 92 | - | 10000 | 2.6226 | 2.6001 | 132 | + | 500 | 0.1693 | 0.1161 | 58 | + | 1000 | 0.2477 | 0.2233 | 63 | + | 2000 | 0.5128 | 0.4946 | 69 | + | 5000 | 1.2482 | 1.1611 | 88 | + | 10000 | 2.4909 | 2.5672 | 127 | """ survey_header = """ | survey | | | | | diff --git a/tests/test_last_saved.py b/tests/test_last_saved.py index 094efde0a..b41186e2d 100644 --- a/tests/test_last_saved.py +++ b/tests/test_last_saved.py @@ -192,7 +192,7 @@ def test_last_saved_errors_when_field_does_not_exist(self): """, errored=True, error__contains=[ - "There has been a problem trying to replace ${last-saved#foo} with the XPath to the survey element named 'foo'. There is no survey element with this name." + "There has been a problem trying to replace ${last-saved#foo} with the XPath to the survey element named 'foo'. There is no survey element named 'foo'." ], ) diff --git a/tests/test_translations.py b/tests/test_translations.py index 1699924ed..879ee5827 100644 --- a/tests/test_translations.py +++ b/tests/test_translations.py @@ -380,15 +380,15 @@ def test_missing_translations_check_performance(self): """ Should find the translations check costs a fraction of a second for large forms. - Results with Python 3.10.14 on VM with 2vCPU (i7-7700HQ) 1GB RAM, x questions + Results with Python 3.12.11 on VM with 2vCPU (i7-7700HQ) 2GB RAM, x questions with 2 choices each, average of 10 runs (seconds), with and without the check, per question: | num | with | without | peak RSS MB | - | 500 | 0.6467 | 0.5648 | 77 | - | 1000 | 1.1448 | 1.2868 | 94 | - | 2000 | 2.3626 | 2.1485 | 129 | - | 5000 | 5.9631 | 5.7911 | 247 | - | 10000 | 11.404 | 11.399 | 423 | + | 500 | 0.4812 | 0.4412 | 73 | + | 1000 | 1.0376 | 1.0013 | 88 | + | 2000 | 2.0885 | 2.1231 | 136 | + | 5000 | 5.7501 | 5.8106 | 235 | + | 10000 | 11.151 | 10.724 | 415 | """ survey_header = """ | survey | | | | | diff --git a/tests/test_trigger.py b/tests/test_trigger.py index 4a248fb2d..c0dab78cd 100644 --- a/tests/test_trigger.py +++ b/tests/test_trigger.py @@ -2,6 +2,8 @@ Test handling setvalue of 'trigger' column in forms """ +from pyxform.errors import ErrorCode + from tests.pyxform_test_case import PyxformTestCase @@ -16,8 +18,9 @@ def test_trigger_reference_to_nonexistent_node_gives_error(self): """, errored=True, error__contains=[ - "There has been a problem trying to replace ${a} with the XPath to the " - + "survey element named 'a'. There is no survey element with this name." + ErrorCode.PYREF_003.value.format( + sheet="survey", column="trigger", row=2, q="a" + ) ], ) @@ -31,7 +34,9 @@ def test_trigger_with_something_other_than_node_ref_gives_error(self): """, errored=True, error__contains=[ - "Only references to other fields are allowed in the 'trigger' column." + ErrorCode.PYREF_001.value.format( + sheet="survey", column="trigger", row=2, q="6" + ) ], ) @@ -319,3 +324,68 @@ def test_trigger_column_in_repeat_should_have_expanded_xpath_in_value(self): + " and @value='string-length( ../one )']", ], ) + + def test_multiple_triggers__with_comma_delimiter_ok(self): + """Should find setvalue elements added for each trigger reference question.""" + md = """ + | survey | | | | | | + | | type | name | label | calculation | trigger | + | | text | a | Enter text | | | + | | text | b | Enter text | | | + | | dateTime | c | | now() | {case} | + """ + cases = ( + "${a},${b}", # single comma + "${a},,${b}", # double comma + "${a},${b},", # trailing comma + "${a},${b},,", # trailing double comma + ) + for case in cases: + with self.subTest(msg=case): + self.assertPyxformXform( + md=md.format(case=case), + xml__xpath_match=[ + """ + /h:html/h:body/x:input[@ref='/test_name/a']/x:setvalue[ + @ref='/test_name/c' + and @event='xforms-value-changed' + and @value='now()' + ] + """, + """ + /h:html/h:body/x:input[@ref='/test_name/b']/x:setvalue[ + @ref='/test_name/c' + and @event='xforms-value-changed' + and @value='now()' + ] + """, + ], + ) + + def test_multiple_triggers__with_bad_comma_delimiter(self): + """Should find an error is raised for incorrectly specified multiple references.""" + md = """ + | survey | | | | | | + | | type | name | label | calculation | trigger | + | | text | a | Enter text | | | + | | text | b | Enter text | | | + | | dateTime | c | | now() | {case} | + """ + cases = ( + "${a}${b}", # no comma + ",${a}${b}", # multiple after comma first pos + "${a},${b}${c}", # multiple after comma second pos + "${a}${b},", # multiple before comma first pos + "${a},${b}${c}", # multiple before comma second pos + ) + for case in cases: + with self.subTest(msg=case): + self.assertPyxformXform( + md=md.format(case=case), + errored=True, + error__contains=[ + ErrorCode.PYREF_002.value.format( + sheet="survey", column="trigger", row="4" + ) + ], + ) diff --git a/tests/validators/pyxform/test_pyxform_reference.py b/tests/validators/pyxform/test_pyxform_reference.py index 6a8d28a06..461e14de4 100644 --- a/tests/validators/pyxform/test_pyxform_reference.py +++ b/tests/validators/pyxform/test_pyxform_reference.py @@ -1,6 +1,6 @@ from itertools import chain, product -from pyxform.errors import PyXFormError +from pyxform.errors import ErrorCode, PyXFormError from pyxform.validators.pyxform import pyxform_reference as pr from tests.pyxform_test_case import PyxformTestCase @@ -51,10 +51,8 @@ def test_single_reference__error(self): case = context.format(token) pr.validate_pyxform_reference_syntax(case, "test", 1, "test") self.assertEqual( - err.exception.args[0], - pr.PYXFORM_REFERENCE_INVALID.format( - sheet="test", row_number=1, column="test" - ), + str(err.exception), + ErrorCode.PYREF_001.value.format(sheet="test", column="test", row=1), msg=case, ) @@ -99,9 +97,7 @@ def test_multiple_references__error(self): case = context.format(token1, token2) pr.validate_pyxform_reference_syntax(case, "test", 1, "test") self.assertEqual( - err.exception.args[0], - pr.PYXFORM_REFERENCE_INVALID.format( - sheet="test", row_number=1, column="test" - ), + str(err.exception), + ErrorCode.PYREF_001.value.format(sheet="test", column="test", row=1), msg=case, )