From b6c0204b1d7d487fe0abec3f60ebf2443f0c768a Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 25 Aug 2023 11:39:24 +0100 Subject: [PATCH 1/2] Add failing test for adding custom functions --- .../javarosa/core/model/test/FormDefTest.java | 55 +++++++++++++++++++ .../java/org/javarosa/core/test/Scenario.java | 24 +++++++- 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/javarosa/core/model/test/FormDefTest.java b/src/test/java/org/javarosa/core/model/test/FormDefTest.java index 69930e0e4..8b63288af 100644 --- a/src/test/java/org/javarosa/core/model/test/FormDefTest.java +++ b/src/test/java/org/javarosa/core/model/test/FormDefTest.java @@ -38,9 +38,14 @@ import static org.junit.Assert.assertThat; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; import org.javarosa.core.model.FormDef; +import org.javarosa.core.model.condition.EvaluationContext; +import org.javarosa.core.model.condition.IFunctionHandler; import org.javarosa.core.test.Scenario; import org.javarosa.core.util.XFormsElement; import org.javarosa.form.api.FormEntryCaption; @@ -371,4 +376,54 @@ public void fillTemplateString_resolvesRelativeReferences_inItext() throws IOExc caption = new FormEntryCaption(scenario.getFormDef(), scenario.getCurrentIndex()); MatcherAssert.assertThat(caption.getQuestionText(), is("Position: 2")); } + + @Test + public void canAddFunctionHandlersBeforeInitialize() throws Exception { + FormDef formDef = Scenario.createFormDef("custom-func-form", html( + head( + title("custom-func-form"), + model( + mainInstance(t("data", + t("calculate"), + t("input") + )), + bind("/data/calculate").type("string").calculate("custom-func()") + ) + ), + body( + input("/data/input", + label("/data/calculate") + ) + ) + )); + + formDef.getEvaluationContext().addFunctionHandler(new IFunctionHandler() { + @Override + public String getName() { + return "custom-func"; + } + + @Override + public List getPrototypes() { + return new ArrayList(); + } + + @Override + public boolean rawArgs() { + return true; + } + + @Override + public boolean realTime() { + return false; + } + + @Override + public Object eval(Object[] args, EvaluationContext ec) { + return "blah"; + } + }); + + Scenario.init(formDef); + } } diff --git a/src/test/java/org/javarosa/core/test/Scenario.java b/src/test/java/org/javarosa/core/test/Scenario.java index 754d61237..0a2ac10be 100644 --- a/src/test/java/org/javarosa/core/test/Scenario.java +++ b/src/test/java/org/javarosa/core/test/Scenario.java @@ -57,6 +57,7 @@ import org.javarosa.xpath.expr.XPathNumericLiteral; import org.javarosa.xpath.expr.XPathPathExpr; import org.javarosa.xpath.parser.XPathSyntaxException; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -438,6 +439,14 @@ public static Scenario init(String formName, XFormsElement form) throws IOExcept return Scenario.init(formFile); } + public static FormDef createFormDef(String formName, XFormsElement form) throws IOException, XFormParser.ParseException { + Path formFile = createTempDirectory("javarosa").resolve(formName + ".xml"); + String xml = form.asXml(); + System.out.println(xml); + write(formFile, xml.getBytes(UTF_8), CREATE); + return Scenario.createFormDef(formFile); + } + /** * Initializes the Scenario with provided form filename. *

@@ -451,13 +460,24 @@ public static Scenario init(String formFileName) throws XFormParser.ParseExcepti * Initializes the Scenario with the form at the provided path */ public static Scenario init(Path formFile) throws XFormParser.ParseException { + FormDef formDef = createFormDef(formFile); + formDef.initialize(true, new InstanceInitializationFactory()); + return Scenario.from(formDef); + } + + public static Scenario init(FormDef formDef) throws XFormParser.ParseException { + formDef.initialize(true, new InstanceInitializationFactory()); + return Scenario.from(formDef); + } + + @NotNull + public static FormDef createFormDef(Path formFile) throws XFormParser.ParseException { // TODO explain why this sequence of calls StorageManager.setStorageFactory((name, type) -> new DummyIndexedStorageUtility<>()); new XFormsModule().registerModule(); FormParseInit fpi = new FormParseInit(formFile); FormDef formDef = fpi.getFormDef(); - formDef.initialize(true, new InstanceInitializationFactory()); - return Scenario.from(formDef); + return formDef; } // endregion From 28e371892d42b4361b6d4e0aa73336eb06c1f185 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 25 Aug 2023 13:51:02 +0100 Subject: [PATCH 2/2] Reinstate memoized EvaluationContext --- .../java/org/javarosa/core/model/FormDef.java | 309 +++++++++--------- 1 file changed, 160 insertions(+), 149 deletions(-) diff --git a/src/main/java/org/javarosa/core/model/FormDef.java b/src/main/java/org/javarosa/core/model/FormDef.java index 995f515d5..531e2be2e 100644 --- a/src/main/java/org/javarosa/core/model/FormDef.java +++ b/src/main/java/org/javarosa/core/model/FormDef.java @@ -103,6 +103,8 @@ public class FormDef implements IFormElement, Localizable, Persistable, IMetaDat private static EventNotifier defaultEventNotifier = new EventNotifierSilent(); private ExternalizableExtras extras = new ExternalizableExtras(); + private EvaluationContext evaluationContext; + /** * Takes a (possibly relative) reference, and makes it absolute based on its parent. * Moved from the parser to this class so it can be used more cleanly by ItemsetBinding. @@ -236,6 +238,7 @@ public void setEventNotifier(EventNotifier eventNotifier) { * Getters and setters for the lists */ public void addNonMainInstance(DataInstance instance) { + resetEvaluationContext(); formInstances.put(instance.getName(), instance); } @@ -263,6 +266,8 @@ public Enumeration getNonMainInstances() { } public void setInstance(FormInstance fi) { + resetEvaluationContext(); + mainInstance = fi; fi.setFormId(getID()); @@ -748,177 +753,179 @@ public boolean evaluateConstraint(TreeReference ref, IAnswerData data) { } public EvaluationContext getEvaluationContext() { - EvaluationContext ec = new EvaluationContext(mainInstance, getFormInstances(), new EvaluationContext(null)); - - if (!ec.getFunctionHandlers().containsKey("jr:itext")) { - final FormDef f = this; - ec.addFunctionHandler(new IFunctionHandler() { - @Override - public String getName() { - return "jr:itext"; - } + if (evaluationContext == null) { + evaluationContext = new EvaluationContext(mainInstance, getFormInstances(), new EvaluationContext(null)); + + if (!evaluationContext.getFunctionHandlers().containsKey("jr:itext")) { + final FormDef f = this; + evaluationContext.addFunctionHandler(new IFunctionHandler() { + @Override + public String getName() { + return "jr:itext"; + } - @Override - public Object eval(Object[] args, EvaluationContext ec) { - String textID = (String) args[0]; - try { - // SUUUUPER HACKY - String form = ec.getOutputTextForm(); - if (form != null) { - textID = textID + ";" + form; - String result = f.getLocalizer().getRawText(f.getLocalizer().getLocale(), - textID); - return result == null ? "" : result; - } else { - String text = f.getLocalizer().getText(textID); - return text == null ? "[itext:" + textID + "]" : text; + @Override + public Object eval(Object[] args, EvaluationContext ec) { + String textID = (String) args[0]; + try { + // SUUUUPER HACKY + String form = ec.getOutputTextForm(); + if (form != null) { + textID = textID + ";" + form; + String result = f.getLocalizer().getRawText(f.getLocalizer().getLocale(), + textID); + return result == null ? "" : result; + } else { + String text = f.getLocalizer().getText(textID); + return text == null ? "[itext:" + textID + "]" : text; + } + } catch (NoSuchElementException nsee) { + return "[nolocale]"; } - } catch (NoSuchElementException nsee) { - return "[nolocale]"; } - } - @Override - public List getPrototypes() { - Class[] proto = {String.class}; - List v = new ArrayList<>(1); - v.add(proto); - return v; - } + @Override + public List getPrototypes() { + Class[] proto = {String.class}; + List v = new ArrayList<>(1); + v.add(proto); + return v; + } - @Override - public boolean rawArgs() { - return false; - } + @Override + public boolean rawArgs() { + return false; + } - @Override - public boolean realTime() { - return false; - } - }); - } + @Override + public boolean realTime() { + return false; + } + }); + } - /* - * Given a select value, looks the label up in the choice list defined by the given question and returns it in - * the currently-set language. - * - * arg 1: select value - * arg 2: string xpath referring to question that defines the select - * - * this won't work at all if the original label needed to be - * processed/calculated in some way (s, etc.) (is this even - * allowed?) likely won't work with multi-media labels _might_ work for - * itemsets, but probably not very well or at all; could potentially work - * better if we had some context info - * - * it's mainly intended for the simple case of reversing a question with - * compile-time-static fields, for use inside an - */ - if (!ec.getFunctionHandlers().containsKey("jr:choice-name")) { - final FormDef f = this; - ec.addFunctionHandler(new IFunctionHandler() { - @Override - public String getName() { - return "jr:choice-name"; - } + /* + * Given a select value, looks the label up in the choice list defined by the given question and returns it in + * the currently-set language. + * + * arg 1: select value + * arg 2: string xpath referring to question that defines the select + * + * this won't work at all if the original label needed to be + * processed/calculated in some way (s, etc.) (is this even + * allowed?) likely won't work with multi-media labels _might_ work for + * itemsets, but probably not very well or at all; could potentially work + * better if we had some context info + * + * it's mainly intended for the simple case of reversing a question with + * compile-time-static fields, for use inside an + */ + if (!evaluationContext.getFunctionHandlers().containsKey("jr:choice-name")) { + final FormDef f = this; + evaluationContext.addFunctionHandler(new IFunctionHandler() { + @Override + public String getName() { + return "jr:choice-name"; + } - @Override - public Object eval(Object[] args, EvaluationContext ec) { - try { - String value = (String) args[0]; - String questionXpath = (String) args[1]; - TreeReference ref = RestoreUtils.xfFact.ref(questionXpath); - ref = ref.anchor(ec.getContextRef()); - - QuestionDef q = findQuestionByRef(ref, f); - if (q == null - || (q.getControlType() != Constants.CONTROL_SELECT_ONE - && q.getControlType() != Constants.CONTROL_SELECT_MULTI - && q.getControlType() != Constants.CONTROL_RANK)) { - return ""; - } + @Override + public Object eval(Object[] args, EvaluationContext ec) { + try { + String value = (String) args[0]; + String questionXpath = (String) args[1]; + TreeReference ref = RestoreUtils.xfFact.ref(questionXpath); + ref = ref.anchor(ec.getContextRef()); + + QuestionDef q = findQuestionByRef(ref, f); + if (q == null + || (q.getControlType() != Constants.CONTROL_SELECT_ONE + && q.getControlType() != Constants.CONTROL_SELECT_MULTI + && q.getControlType() != Constants.CONTROL_RANK)) { + return ""; + } - List choices; - - ItemsetBinding itemset = q.getDynamicChoices(); - if (itemset != null) { - // 2019-HM: See ChoiceNameTest for test and more explanation - - // NOTE: We have no context against which to evaluate a dynamic selection list. This will - // generally cause that evaluation to break if any filtering is done, or, worst case, give - // unexpected results. - // - // We should hook into the existing code (FormEntryPrompt) for pulling display text for select - // choices. however, it's hard, because we don't really have any context to work with, and all - // the situations where that context would be used don't make sense for trying to reverse a - // select value back to a label in an unrelated expression - if (ref.isAmbiguous()) { - // SurveyCTO: We need a absolute "ref" to populate the dynamic choices, - // like we do when we populate those at FormEntryPrompt (line 251). - // The "ref" here is ambiguous, so we need to make it concrete first. - ref = ref.contextualize(ec.getContextRef()); + List choices; + + ItemsetBinding itemset = q.getDynamicChoices(); + if (itemset != null) { + // 2019-HM: See ChoiceNameTest for test and more explanation + + // NOTE: We have no context against which to evaluate a dynamic selection list. This will + // generally cause that evaluation to break if any filtering is done, or, worst case, give + // unexpected results. + // + // We should hook into the existing code (FormEntryPrompt) for pulling display text for select + // choices. however, it's hard, because we don't really have any context to work with, and all + // the situations where that context would be used don't make sense for trying to reverse a + // select value back to a label in an unrelated expression + if (ref.isAmbiguous()) { + // SurveyCTO: We need a absolute "ref" to populate the dynamic choices, + // like we do when we populate those at FormEntryPrompt (line 251). + // The "ref" here is ambiguous, so we need to make it concrete first. + ref = ref.contextualize(ec.getContextRef()); + } + choices = itemset.getChoices(f, ref); + } else { // static choices + choices = q.getChoices(); } - choices = itemset.getChoices(f, ref); - } else { // static choices - choices = q.getChoices(); - } - if (choices != null) { - for (SelectChoice ch : choices) { - if (ch.getValue().equals(value)) { - // this is really not ideal. we should hook into the existing code (FormEntryPrompt) - // for pulling display text for select choices. however, it's hard, because we don't - // really have any context to work with, and all the situations where that context - // would be used don't make sense for trying to reverse a select value back to a - // label in an unrelated expression - - String textID = ch.getTextID(); - String templateStr; - if (textID != null) { - templateStr = f.getLocalizer().getText(textID); - } else { - templateStr = ch.getLabelInnerText(); + if (choices != null) { + for (SelectChoice ch : choices) { + if (ch.getValue().equals(value)) { + // this is really not ideal. we should hook into the existing code (FormEntryPrompt) + // for pulling display text for select choices. however, it's hard, because we don't + // really have any context to work with, and all the situations where that context + // would be used don't make sense for trying to reverse a select value back to a + // label in an unrelated expression + + String textID = ch.getTextID(); + String templateStr; + if (textID != null) { + templateStr = f.getLocalizer().getText(textID); + } else { + templateStr = ch.getLabelInnerText(); + } + return fillTemplateString(templateStr, ref); } - return fillTemplateString(templateStr, ref); } } + return ""; + } catch (Exception e) { + throw new WrappedException("error in evaluation of xpath function [choice-name]", + e); } - return ""; - } catch (Exception e) { - throw new WrappedException("error in evaluation of xpath function [choice-name]", - e); } - } - @Override - public List getPrototypes() { - Class[] proto = {String.class, String.class}; - List v = new ArrayList<>(1); - v.add(proto); - return v; - } + @Override + public List getPrototypes() { + Class[] proto = {String.class, String.class}; + List v = new ArrayList<>(1); + v.add(proto); + return v; + } - @Override - public boolean rawArgs() { - return false; - } + @Override + public boolean rawArgs() { + return false; + } - @Override - public boolean realTime() { - return false; - } - }); - } + @Override + public boolean realTime() { + return false; + } + }); + } - if (predicateCaching) { - List filters = Stream.concat( - customFilterStrategies.stream(), - Stream.of(equalityExpressionIndexFilterStrategy, comparisonExpressionCacheFilterStrategy) - ).collect(Collectors.toList()); + if (predicateCaching) { + List filters = Stream.concat( + customFilterStrategies.stream(), + Stream.of(equalityExpressionIndexFilterStrategy, comparisonExpressionCacheFilterStrategy) + ).collect(Collectors.toList()); - ec = new EvaluationContext(ec, filters); + evaluationContext = new EvaluationContext(evaluationContext, filters); + } } - return ec; + return evaluationContext; } public String fillTemplateString(String template, TreeReference contextRef) { @@ -1713,4 +1720,8 @@ public void disablePredicateCaching() { public void addFilterStrategy(FilterStrategy filterStrategy) { customFilterStrategies.add(filterStrategy); } + + private void resetEvaluationContext() { + evaluationContext = null; + } }