diff --git a/src/main/java/org/javarosa/core/model/CompareChildToAbsoluteExpression.java b/src/main/java/org/javarosa/core/model/CompareChildToAbsoluteExpression.java new file mode 100644 index 000000000..b1cde3afe --- /dev/null +++ b/src/main/java/org/javarosa/core/model/CompareChildToAbsoluteExpression.java @@ -0,0 +1,99 @@ +package org.javarosa.core.model; + +import kotlin.Pair; +import org.javarosa.core.model.condition.EvaluationContext; +import org.javarosa.core.model.instance.DataInstance; +import org.javarosa.core.model.instance.TreeReference; +import org.javarosa.xpath.expr.XPathCmpExpr; +import org.javarosa.xpath.expr.XPathEqExpr; +import org.javarosa.xpath.expr.XPathExpression; +import org.javarosa.xpath.expr.XPathNumericLiteral; +import org.javarosa.xpath.expr.XPathPathExpr; +import org.javarosa.xpath.expr.XPathStringLiteral; +import org.jetbrains.annotations.Nullable; + +import java.util.Arrays; +import java.util.LinkedList; +import java.util.Queue; + +public class CompareChildToAbsoluteExpression { + + private final XPathPathExpr relativeSide; + private final XPathExpression absoluteSide; + private final XPathExpression original; + + public CompareChildToAbsoluteExpression(XPathPathExpr relativeSide, XPathExpression absoluteSide, XPathExpression original) { + this.relativeSide = relativeSide; + this.absoluteSide = absoluteSide; + this.original = original; + } + + public Object evalRelative(DataInstance sourceInstance, EvaluationContext evaluationContext, TreeReference child, int childIndex) { + EvaluationContext rescopedContext = evaluationContext.rescope(child, childIndex); + return getRelativeSide().eval(sourceInstance, rescopedContext).unpack(); + } + + public Object evalAbsolute(DataInstance sourceInstance, EvaluationContext evaluationContext) { + if (absoluteSide instanceof XPathPathExpr) { + return ((XPathPathExpr) getAbsoluteSide()).eval(sourceInstance, evaluationContext).unpack(); + } else { + return absoluteSide.eval(sourceInstance, evaluationContext); + } + } + + public XPathPathExpr getRelativeSide() { + return relativeSide; + } + + public XPathExpression getAbsoluteSide() { + return absoluteSide; + } + + public XPathExpression getOriginal() { + return original; + } + + @Nullable + public static CompareChildToAbsoluteExpression parse(XPathExpression expression) { + XPathExpression a = null; + XPathExpression b = null; + + if (expression instanceof XPathCmpExpr) { + a = ((XPathCmpExpr) expression).a; + b = ((XPathCmpExpr) expression).b; + } else if (expression instanceof XPathEqExpr) { + a = ((XPathEqExpr) expression).a; + b = ((XPathEqExpr) expression).b; + } + + Pair relativeAndAbsolute = getRelativeAndAbsolute(a, b); + if (relativeAndAbsolute != null) { + return new CompareChildToAbsoluteExpression(relativeAndAbsolute.getFirst(), relativeAndAbsolute.getSecond(), expression); + } else { + return null; + } + } + + private static Pair getRelativeAndAbsolute(XPathExpression a, XPathExpression b) { + XPathPathExpr relative = null; + XPathExpression absolute = null; + + Queue subExpressions = new LinkedList<>(Arrays.asList(a, b)); + while (!subExpressions.isEmpty()) { + XPathExpression subExpression = subExpressions.poll(); + if (subExpression instanceof XPathPathExpr && ((XPathPathExpr) subExpression).init_context == XPathPathExpr.INIT_CONTEXT_RELATIVE) + relative = (XPathPathExpr) subExpression; + else if (subExpression instanceof XPathPathExpr && ((XPathPathExpr) subExpression).init_context == XPathPathExpr.INIT_CONTEXT_ROOT) { + absolute = subExpression; + } else if (subExpression instanceof XPathNumericLiteral || subExpression instanceof XPathStringLiteral) { + absolute = subExpression; + } + } + + if (relative != null && absolute != null) { + return new Pair<>(relative, absolute); + } else { + return null; + } + } +} diff --git a/src/main/java/org/javarosa/core/model/CompareChildToAbsoluteExpressionFilter.java b/src/main/java/org/javarosa/core/model/CompareChildToAbsoluteExpressionFilter.java new file mode 100644 index 000000000..733f3fc88 --- /dev/null +++ b/src/main/java/org/javarosa/core/model/CompareChildToAbsoluteExpressionFilter.java @@ -0,0 +1,49 @@ +package org.javarosa.core.model; + +import org.javarosa.core.model.condition.EvaluationContext; +import org.javarosa.core.model.condition.PredicateFilter; +import org.javarosa.core.model.instance.DataInstance; +import org.javarosa.core.model.instance.TreeReference; +import org.javarosa.xpath.expr.XPathCmpExpr; +import org.javarosa.xpath.expr.XPathEqExpr; +import org.javarosa.xpath.expr.XPathExpression; +import org.jetbrains.annotations.NotNull; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Supplier; + +/** + * Caches down stream evaluations (in the {@link PredicateFilter} chain) for supported expressions - currently just + * {@link XPathCmpExpr} and {@link XPathEqExpr}. Repeated evaluations are fetched in O(1) time. + */ +public class CompareChildToAbsoluteExpressionFilter implements PredicateFilter { + + private final Map> cachedEvaluations = new HashMap<>(); + + @NotNull + @Override + public List filter(@NotNull DataInstance sourceInstance, @NotNull TreeReference nodeSet, @NotNull XPathExpression predicate, @NotNull List children, @NotNull EvaluationContext evaluationContext, @NotNull Supplier> next) { + if (sourceInstance.getInstanceId() == null) { + return next.get(); + } + + CompareChildToAbsoluteExpression candidate = CompareChildToAbsoluteExpression.parse(predicate); + if (candidate != null) { + Object absoluteValue = candidate.evalAbsolute(sourceInstance, evaluationContext); + String key = nodeSet.toString() + predicate + candidate.getRelativeSide() + absoluteValue.toString(); + + if (cachedEvaluations.containsKey(key)) { + return cachedEvaluations.get(key); + } else { + List filtered = next.get(); + cachedEvaluations.put(key, filtered); + return filtered; + } + } else { + return next.get(); + } + } + +} diff --git a/src/main/java/org/javarosa/core/model/FormDef.java b/src/main/java/org/javarosa/core/model/FormDef.java index ba257c9ed..343ba8310 100644 --- a/src/main/java/org/javarosa/core/model/FormDef.java +++ b/src/main/java/org/javarosa/core/model/FormDef.java @@ -24,6 +24,7 @@ import org.javarosa.core.model.condition.EvaluationContext; import org.javarosa.core.model.condition.IConditionExpr; import org.javarosa.core.model.condition.IFunctionHandler; +import org.javarosa.core.model.condition.PredicateFilter; import org.javarosa.core.model.condition.Triggerable; import org.javarosa.core.model.data.IAnswerData; import org.javarosa.core.model.data.MultipleItemsData; @@ -1707,4 +1708,8 @@ public Extras getExtras() { public void disablePredicateCaching() { dagImpl.disablePredicateCaching(); } + + public void addPredicateFilter(PredicateFilter predicateFilter) { + dagImpl.addPredicateFilter(predicateFilter); + } } diff --git a/src/main/java/org/javarosa/core/model/IdempotentInMemPredicateCache.java b/src/main/java/org/javarosa/core/model/IdempotentInMemPredicateCache.java deleted file mode 100644 index d0a243577..000000000 --- a/src/main/java/org/javarosa/core/model/IdempotentInMemPredicateCache.java +++ /dev/null @@ -1,45 +0,0 @@ -package org.javarosa.core.model; - -import org.javarosa.core.model.condition.PredicateCache; -import org.javarosa.core.model.instance.TreeReference; -import org.javarosa.xpath.expr.XPathExpression; -import org.jetbrains.annotations.NotNull; - -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.function.Supplier; - -/** - * In memory implementation of a {@link PredicateCache}. Cannot cache predicate evaluations that contain a - * non-idempotent function. - */ -public class IdempotentInMemPredicateCache implements PredicateCache { - - public Map> cachedEvaluations = new HashMap<>(); - - @Override - @NotNull - public List get(TreeReference nodeSet, XPathExpression predicate, Supplier> onMiss) { - String key = getKey(nodeSet, predicate); - - if (cachedEvaluations.containsKey(key)) { - return cachedEvaluations.get(key); - } else { - List references = onMiss.get(); - if (isCacheable(predicate)) { - cachedEvaluations.put(key, references); - } - - return references; - } - } - - private String getKey(TreeReference nodeSet, XPathExpression predicate) { - return nodeSet.toString() + predicate.toString(); - } - - private boolean isCacheable(XPathExpression predicate) { - return predicate.isIdempotent(); - } -} diff --git a/src/main/java/org/javarosa/core/model/IdempotentPredicateCache.java b/src/main/java/org/javarosa/core/model/IdempotentPredicateCache.java new file mode 100644 index 000000000..06042657e --- /dev/null +++ b/src/main/java/org/javarosa/core/model/IdempotentPredicateCache.java @@ -0,0 +1,48 @@ +package org.javarosa.core.model; + +import org.javarosa.core.model.condition.EvaluationContext; +import org.javarosa.core.model.condition.PredicateFilter; +import org.javarosa.core.model.instance.DataInstance; +import org.javarosa.core.model.instance.TreeReference; +import org.javarosa.xpath.expr.XPathExpression; +import org.jetbrains.annotations.NotNull; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Supplier; + +/** + * Caches down stream evaluations (in the {@link PredicateFilter} chain) for "idempotent" (with respect to current form + * state) predicates. Can only be used for static instances or in cases where form state won't change - will cause + * clashes otherwise. Repeated evaluations are fetched in O(1) time. + */ +public class IdempotentPredicateCache implements PredicateFilter { + + private final Map> cachedEvaluations = new HashMap<>(); + + @NotNull + @Override + public List filter(@NotNull DataInstance sourceInstance, @NotNull TreeReference nodeSet, @NotNull XPathExpression predicate, @NotNull List children, @NotNull EvaluationContext evaluationContext, @NotNull Supplier> next) { + String key = getKey(nodeSet, predicate); + + if (cachedEvaluations.containsKey(key)) { + return cachedEvaluations.get(key); + } else { + List filtered = next.get(); + if (isCacheable(predicate)) { + cachedEvaluations.put(key, filtered); + } + + return filtered; + } + } + + private String getKey(TreeReference nodeSet, XPathExpression predicate) { + return nodeSet.toString() + predicate.toString(); + } + + private boolean isCacheable(XPathExpression predicate) { + return predicate.isIdempotent(); + } +} diff --git a/src/main/java/org/javarosa/core/model/IndexPredicateFilter.java b/src/main/java/org/javarosa/core/model/IndexPredicateFilter.java new file mode 100644 index 000000000..c46fea708 --- /dev/null +++ b/src/main/java/org/javarosa/core/model/IndexPredicateFilter.java @@ -0,0 +1,94 @@ +package org.javarosa.core.model; + +import org.javarosa.core.model.condition.EvaluationContext; +import org.javarosa.core.model.condition.PredicateFilter; +import org.javarosa.core.model.instance.DataInstance; +import org.javarosa.core.model.instance.TreeReference; +import org.javarosa.measure.Measure; +import org.javarosa.xpath.expr.XPathEqExpr; +import org.javarosa.xpath.expr.XPathExpression; +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Supplier; + +import static java.util.Collections.emptyList; + +/** + * Uses a (lazily constructed) index to evaluate a predicate for supported expressions - currently just + * {@link XPathEqExpr} where one side is relative to the instance child being filtered. Evaluations are fetched in + * O(1) time with O(n) expression evaluations only being required the first time a relative side is evaluated. + */ +public class IndexPredicateFilter implements PredicateFilter { + + private final InMemTreeReferenceIndex index = new InMemTreeReferenceIndex(); + + @NotNull + @Override + public List filter(@NotNull DataInstance sourceInstance, @NotNull TreeReference nodeSet, @NotNull XPathExpression predicate, @NotNull List children, @NotNull EvaluationContext evaluationContext, @NotNull Supplier> next) { + if (sourceInstance.getInstanceId() == null || !(predicate instanceof XPathEqExpr)) { + return next.get(); + } + + CompareChildToAbsoluteExpression candidate = CompareChildToAbsoluteExpression.parse(predicate); + if (candidate != null) { + XPathEqExpr original = (XPathEqExpr) candidate.getOriginal(); + if (original.isEqual()) { + String section = sourceInstance.getInstanceId() + nodeSet + candidate.getRelativeSide().toString(); + if (!index.contains(section)) { + buildIndex(sourceInstance, candidate, children, evaluationContext, section); + } + + Object absoluteValue = candidate.evalAbsolute(sourceInstance, evaluationContext); + return index.lookup(section, absoluteValue.toString()); + } else { + return next.get(); + } + } else { + return next.get(); + } + } + + private void buildIndex(DataInstance sourceInstance, CompareChildToAbsoluteExpression predicate, List children, EvaluationContext evaluationContext, String section) { + for (int i = 0; i < children.size(); i++) { + TreeReference child = children.get(i); + + Measure.log("IndexEvaluation"); + String relativeValue = predicate.evalRelative(sourceInstance, evaluationContext, child, i).toString(); + index.add(section, relativeValue, child); + } + } + + private static class InMemTreeReferenceIndex { + + private final Map>> map = new HashMap<>(); + + public boolean contains(String section) { + return map.containsKey(section); + } + + public void add(String section, String key, TreeReference reference) { + if (!map.containsKey(section)) { + map.put(section, new HashMap<>()); + } + + Map> sectionMap = map.get(section); + if (!sectionMap.containsKey(key)) { + sectionMap.put(key, new ArrayList<>()); + } + + sectionMap.get(key).add(reference); + } + + public List lookup(String section, String key) { + if (map.containsKey(section) && map.get(section).containsKey(key)) { + return map.get(section).get(key); + } else { + return emptyList(); + } + } + } +} diff --git a/src/main/java/org/javarosa/core/model/TriggerableDag.java b/src/main/java/org/javarosa/core/model/TriggerableDag.java index a0cbe37a5..270f6fd12 100644 --- a/src/main/java/org/javarosa/core/model/TriggerableDag.java +++ b/src/main/java/org/javarosa/core/model/TriggerableDag.java @@ -18,6 +18,7 @@ import org.javarosa.core.model.condition.Condition; import org.javarosa.core.model.condition.EvaluationContext; +import org.javarosa.core.model.condition.PredicateFilter; import org.javarosa.core.model.condition.Recalculate; import org.javarosa.core.model.condition.Triggerable; import org.javarosa.core.model.instance.AbstractTreeElement; @@ -46,7 +47,10 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Queue; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; import static java.util.Collections.emptySet; @@ -97,6 +101,9 @@ public interface EventNotifierAccessor { private Map relevancePerRepeat = new HashMap<>(); private boolean predicateCaching = true; + private final PredicateFilter cachingPredicateFilter = new CompareChildToAbsoluteExpressionFilter(); + private final PredicateFilter indexPredicateFilter = new IndexPredicateFilter(); + private final Queue customPredicateFilters = new LinkedList<>(); TriggerableDag(EventNotifierAccessor accessor) { this.accessor = accessor; @@ -516,9 +523,15 @@ private Set getAllToTrigger(Set cascadeRoots private Set doEvaluateTriggerables(FormInstance mainInstance, EvaluationContext evalContext, Set toTrigger, TreeReference changedRef, Set affectAllRepeatInstances, Set alreadyEvaluated) { Set evaluated = new HashSet<>(); + EvaluationContext context; if (predicateCaching) { - context = new EvaluationContext(evalContext, new IdempotentInMemPredicateCache()); + List filters = Stream.concat( + customPredicateFilters.stream(), + Stream.of(indexPredicateFilter, cachingPredicateFilter, new IdempotentPredicateCache()) + ).collect(Collectors.toList()); + + context = new EvaluationContext(evalContext, filters); } else { context = evalContext; } @@ -737,4 +750,8 @@ private List getRecalculates() { public void disablePredicateCaching() { this.predicateCaching = false; } + + public void addPredicateFilter(PredicateFilter predicateFilter) { + customPredicateFilters.add(predicateFilter); + } } diff --git a/src/main/java/org/javarosa/core/model/condition/EvaluationContext.java b/src/main/java/org/javarosa/core/model/condition/EvaluationContext.java index 74bc7d066..79772eb8f 100644 --- a/src/main/java/org/javarosa/core/model/condition/EvaluationContext.java +++ b/src/main/java/org/javarosa/core/model/condition/EvaluationContext.java @@ -21,14 +21,18 @@ import org.javarosa.core.model.instance.DataInstance; import org.javarosa.core.model.instance.TreeElement; import org.javarosa.core.model.instance.TreeReference; -import org.javarosa.measure.Measure; import org.javarosa.xpath.IExprDataType; import org.javarosa.xpath.expr.XPathExpression; +import org.jetbrains.annotations.NotNull; import java.util.ArrayList; import java.util.Date; import java.util.HashMap; import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static java.util.Collections.singletonList; /** * A collection of objects that affect the evaluation of an expression, like @@ -63,8 +67,8 @@ public class EvaluationContext { private DataInstance instance; private int[] predicateEvaluationProgress; - private PredicateCache predicateCache = ((reference, predicate, onMiss) -> onMiss.get()); - + private static final List DEFAULT_PREDICATE_FILTER_CHAIN = singletonList(new XPathEvalPredicateFilter()); + private List predicateFilterChain = DEFAULT_PREDICATE_FILTER_CHAIN; /** * Copy Constructor @@ -91,12 +95,15 @@ private EvaluationContext(EvaluationContext base) { //invalidate this currentContextPosition = base.currentContextPosition; - predicateCache = base.predicateCache; + predicateFilterChain = base.predicateFilterChain; } - public EvaluationContext(EvaluationContext base, PredicateCache predicateCache) { + public EvaluationContext(EvaluationContext base, List aroundPredicateFilterChain) { this(base); - this.predicateCache = predicateCache; + this.predicateFilterChain = Stream.concat( + aroundPredicateFilterChain.stream(), + predicateFilterChain.stream() + ).collect(Collectors.toList()); } public EvaluationContext(EvaluationContext base, TreeReference context) { @@ -325,37 +332,24 @@ private void expandReferenceAccumulator(TreeReference sourceRef, DataInstance so TreeReference nodeSetRef = workingRef.clone(); nodeSetRef.add(name, -1); - boolean firstTime = true; - List passed = new ArrayList(treeReferences.size()); - for (XPathExpression xpe : predicates) { - boolean firstTimeCapture = firstTime; - passed.addAll(predicateCache.get(nodeSetRef, xpe, () -> { - List predicatePassed = new ArrayList<>(treeReferences.size()); - for (int i = 0; i < treeReferences.size(); ++i) { - //if there are predicates then we need to see if e.nextElement meets the standard of the predicate - TreeReference treeRef = treeReferences.get(i); - - //test the predicate on the treeElement - EvaluationContext evalContext = rescope(treeRef, (firstTimeCapture ? treeRef.getMultLast() : i)); - - Measure.log("PredicateEvaluations"); - Object o = xpe.eval(sourceInstance, evalContext); - - if (o instanceof Boolean) { - boolean testOutcome = (Boolean) o; - if (testOutcome) { - predicatePassed.add(treeRef); - } - } - } + for (int i = 0; i < predicates.size(); i++) { + List filterChain; + if (i == 0 && !isNested(nodeSetRef)) { + filterChain = predicateFilterChain; + } else { + filterChain = DEFAULT_PREDICATE_FILTER_CHAIN; + } - return predicatePassed; - })); + List passed = filterWithPredicate( + sourceInstance, + nodeSetRef, + predicates.get(i), + treeReferences, + filterChain + ); - firstTime = false; treeReferences.clear(); treeReferences.addAll(passed); - passed.clear(); if (predicateEvaluationProgress != null) { predicateEvaluationProgress[0]++; @@ -368,7 +362,29 @@ private void expandReferenceAccumulator(TreeReference sourceRef, DataInstance so } } - private EvaluationContext rescope(TreeReference treeRef, int currentContextPosition) { + private static boolean isNested(TreeReference nodeSet) { + for (int i = 1; i < nodeSet.size(); i++) { + if (nodeSet.getMultiplicity(i) > -1) { + return true; + } + } + + return false; + } + + @NotNull + private List filterWithPredicate(DataInstance sourceInstance, TreeReference treeReference, XPathExpression predicate, List children, List filterChain) { + return filterWithPredicate(sourceInstance, treeReference, predicate, children, 0, filterChain); + } + + @NotNull + private List filterWithPredicate(DataInstance sourceInstance, TreeReference treeReference, XPathExpression predicate, List children, int i, List filterChain) { + return filterChain.get(i).filter(sourceInstance, treeReference, predicate, children, this, () -> { + return filterWithPredicate(sourceInstance, treeReference, predicate, children, i + 1, filterChain); + }); + } + + public EvaluationContext rescope(TreeReference treeRef, int currentContextPosition) { EvaluationContext ec = new EvaluationContext(this, treeRef); // broken: ec.currentContextPosition = currentContextPosition; diff --git a/src/main/java/org/javarosa/core/model/condition/PredicateCache.java b/src/main/java/org/javarosa/core/model/condition/PredicateCache.java deleted file mode 100644 index a074a0d36..000000000 --- a/src/main/java/org/javarosa/core/model/condition/PredicateCache.java +++ /dev/null @@ -1,19 +0,0 @@ -package org.javarosa.core.model.condition; - -import org.javarosa.core.model.instance.TreeReference; -import org.javarosa.xpath.expr.XPathExpression; -import org.jetbrains.annotations.NotNull; - -import java.util.List; -import java.util.function.Supplier; - -/** - * Allows the result of predicate evaluations (references for matching nodes) to be cached. The cache doesn't know - * anything about the values that might be referenced in a predicate (the "triggerables"), so can only be used in cases - * where the predicates are "static". - */ -public interface PredicateCache { - - @NotNull - List get(TreeReference nodeSet, XPathExpression predicate, Supplier> onMiss); -} diff --git a/src/main/java/org/javarosa/core/model/condition/PredicateFilter.java b/src/main/java/org/javarosa/core/model/condition/PredicateFilter.java new file mode 100644 index 000000000..ec754fa3f --- /dev/null +++ b/src/main/java/org/javarosa/core/model/condition/PredicateFilter.java @@ -0,0 +1,20 @@ +package org.javarosa.core.model.condition; + +import org.javarosa.core.model.instance.DataInstance; +import org.javarosa.core.model.instance.TreeReference; +import org.javarosa.xpath.expr.XPathExpression; +import org.jetbrains.annotations.NotNull; + +import java.util.List; +import java.util.function.Supplier; + +public interface PredicateFilter { + + @NotNull + List filter(@NotNull DataInstance sourceInstance, + @NotNull TreeReference nodeSet, + @NotNull XPathExpression predicate, + @NotNull List children, + @NotNull EvaluationContext evaluationContext, + @NotNull Supplier> next); +} diff --git a/src/main/java/org/javarosa/core/model/condition/XPathEvalPredicateFilter.java b/src/main/java/org/javarosa/core/model/condition/XPathEvalPredicateFilter.java new file mode 100644 index 000000000..092d2acb5 --- /dev/null +++ b/src/main/java/org/javarosa/core/model/condition/XPathEvalPredicateFilter.java @@ -0,0 +1,44 @@ +package org.javarosa.core.model.condition; + +import org.javarosa.core.model.instance.DataInstance; +import org.javarosa.core.model.instance.TreeReference; +import org.javarosa.measure.Measure; +import org.javarosa.xpath.expr.XPathExpression; +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.List; +import java.util.function.Supplier; + +class XPathEvalPredicateFilter implements PredicateFilter { + + @NotNull + @Override + public List filter(@NotNull DataInstance sourceInstance, + @NotNull TreeReference nodeSet, + @NotNull XPathExpression predicate, + @NotNull List children, + @NotNull EvaluationContext evaluationContext, + @NotNull Supplier> nextFilter) { + List predicatePassed = new ArrayList<>(children.size()); + for (int i = 0; i < children.size(); ++i) { + //if there are predicates then we need to see if e.nextElement meets the standard of the predicate + TreeReference treeRef = children.get(i); + + //test the predicate on the treeElement + EvaluationContext evalContext = evaluationContext.rescope(treeRef, i); + + Measure.log("PredicateEvaluation"); + Object o = predicate.eval(sourceInstance, evalContext); + + if (o instanceof Boolean) { + boolean testOutcome = (Boolean) o; + if (testOutcome) { + predicatePassed.add(treeRef); + } + } + } + + return predicatePassed; + } +} diff --git a/src/main/java/org/javarosa/form/api/FormEntryController.java b/src/main/java/org/javarosa/form/api/FormEntryController.java index a3d788c01..c66c0dda4 100644 --- a/src/main/java/org/javarosa/form/api/FormEntryController.java +++ b/src/main/java/org/javarosa/form/api/FormEntryController.java @@ -21,6 +21,7 @@ import org.javarosa.core.model.GroupDef; import org.javarosa.core.model.IFormElement; import org.javarosa.core.model.QuestionDef; +import org.javarosa.core.model.condition.PredicateFilter; import org.javarosa.core.model.data.IAnswerData; import org.javarosa.core.model.instance.InvalidReferenceException; import org.javarosa.core.model.instance.TreeElement; @@ -349,4 +350,8 @@ private static FormIndex getRepeatGroupIndex(FormIndex index, FormDef formDef) { public void disablePredicateCaching() { model.getForm().disablePredicateCaching(); } + + public void addPredicateFilter(PredicateFilter predicateFilter) { + model.getForm().addPredicateFilter(predicateFilter); + } } diff --git a/src/main/java/org/javarosa/measure/Measure.java b/src/main/java/org/javarosa/measure/Measure.java index c225a5423..3b6ff87c4 100644 --- a/src/main/java/org/javarosa/measure/Measure.java +++ b/src/main/java/org/javarosa/measure/Measure.java @@ -1,6 +1,7 @@ package org.javarosa.measure; import java.util.HashMap; +import java.util.List; import java.util.Map; public class Measure { @@ -12,11 +13,16 @@ private Measure() { } - public static int withMeasure(String event, Runnable work) { + public static int withMeasure(List events, Runnable work) { start(); work.run(); - int count = getCount(event); - start(); + + int count = 0; + for (String event : events) { + count += getCount(event); + } + + stop(); return count; } @@ -42,6 +48,10 @@ private static void stop() { } private static int getCount(String event) { - return counts.get(event); + if (counts.containsKey(event)) { + return counts.get(event); + } else { + return 0; + } } } diff --git a/src/main/java/org/javarosa/xpath/expr/XPathEqExpr.java b/src/main/java/org/javarosa/xpath/expr/XPathEqExpr.java index c52a1f336..16c45c60f 100644 --- a/src/main/java/org/javarosa/xpath/expr/XPathEqExpr.java +++ b/src/main/java/org/javarosa/xpath/expr/XPathEqExpr.java @@ -100,6 +100,10 @@ public void writeExternal(DataOutputStream out) throws IOException { super.writeExternal(out); } + public boolean isEqual() { + return equal; + } + private String id() { return "@" + Integer.toHexString(System.identityHashCode(this)); } diff --git a/src/test/java/org/javarosa/core/model/CompareChildToAbsoluteExpressionTest.java b/src/test/java/org/javarosa/core/model/CompareChildToAbsoluteExpressionTest.java new file mode 100644 index 000000000..ce35d1476 --- /dev/null +++ b/src/test/java/org/javarosa/core/model/CompareChildToAbsoluteExpressionTest.java @@ -0,0 +1,72 @@ +package org.javarosa.core.model; + +import org.javarosa.xpath.expr.XPathEqExpr; +import org.javarosa.xpath.expr.XPathNumericLiteral; +import org.javarosa.xpath.expr.XPathPathExpr; +import org.javarosa.xpath.expr.XPathQName; +import org.javarosa.xpath.expr.XPathStep; +import org.javarosa.xpath.expr.XPathStringLiteral; +import org.junit.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; + +public class CompareChildToAbsoluteExpressionTest { + + @Test + public void parse_parsesStringLiteralAsAbsolute() { + XPathEqExpr expression = new XPathEqExpr( + true, + new XPathPathExpr(XPathPathExpr.INIT_CONTEXT_RELATIVE, new XPathStep[]{ + new XPathStep(XPathStep.AXIS_CHILD, new XPathQName("name")) } + ), + new XPathStringLiteral("string") + ); + + CompareChildToAbsoluteExpression parsed = CompareChildToAbsoluteExpression.parse(expression); + assertThat(parsed, not(nullValue())); + assertThat(parsed.getRelativeSide(), equalTo(expression.a)); + assertThat(parsed.getAbsoluteSide(), equalTo(expression.b)); + } + + @Test + public void parse_parsesNumericLiteralAsAbsolute() { + XPathEqExpr expression = new XPathEqExpr( + true, + new XPathPathExpr(XPathPathExpr.INIT_CONTEXT_RELATIVE, new XPathStep[]{ + new XPathStep(XPathStep.AXIS_CHILD, new XPathQName("name")) } + ), + new XPathNumericLiteral(45.0) + ); + + CompareChildToAbsoluteExpression parsed = CompareChildToAbsoluteExpression.parse(expression); + assertThat(parsed, not(nullValue())); + assertThat(parsed.getRelativeSide(), equalTo(expression.a)); + assertThat(parsed.getAbsoluteSide(), equalTo(expression.b)); + } + + @Test + public void parse_parsesRelativeAndAbsoluteRegardlessOfSide() { + XPathPathExpr relative = new XPathPathExpr(XPathPathExpr.INIT_CONTEXT_RELATIVE, new XPathStep[]{ + new XPathStep(XPathStep.AXIS_CHILD, new XPathQName("name"))} + ); + XPathPathExpr absolute = new XPathPathExpr(XPathPathExpr.INIT_CONTEXT_ROOT, new XPathStep[]{ + new XPathStep(XPathStep.AXIS_CHILD, new XPathQName("something"))} + ); + + XPathEqExpr ltr = new XPathEqExpr(true, relative, absolute); + XPathEqExpr rtl = new XPathEqExpr(true, absolute, relative); + CompareChildToAbsoluteExpression ltrParsed = CompareChildToAbsoluteExpression.parse(ltr); + CompareChildToAbsoluteExpression rtlParsed = CompareChildToAbsoluteExpression.parse(rtl); + + assertThat(ltrParsed, not(nullValue())); + assertThat(ltrParsed.getRelativeSide(), equalTo(relative)); + assertThat(ltrParsed.getAbsoluteSide(), equalTo(absolute)); + + assertThat(rtlParsed, not(nullValue())); + assertThat(rtlParsed.getRelativeSide(), equalTo(relative)); + assertThat(rtlParsed.getAbsoluteSide(), equalTo(absolute)); + } +} \ No newline at end of file diff --git a/src/test/java/org/javarosa/core/model/test/PredicateCachingTest.java b/src/test/java/org/javarosa/core/model/test/PredicateCachingTest.java index fda054c88..daa19c9e9 100644 --- a/src/test/java/org/javarosa/core/model/test/PredicateCachingTest.java +++ b/src/test/java/org/javarosa/core/model/test/PredicateCachingTest.java @@ -4,37 +4,257 @@ import org.javarosa.measure.Measure; import org.junit.Test; +import static java.util.Arrays.asList; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.not; import static org.javarosa.core.util.BindBuilderXFormsElement.bind; import static org.javarosa.core.util.XFormsElement.body; +import static org.javarosa.core.util.XFormsElement.group; import static org.javarosa.core.util.XFormsElement.head; import static org.javarosa.core.util.XFormsElement.html; import static org.javarosa.core.util.XFormsElement.input; import static org.javarosa.core.util.XFormsElement.instance; +import static org.javarosa.core.util.XFormsElement.item; import static org.javarosa.core.util.XFormsElement.mainInstance; import static org.javarosa.core.util.XFormsElement.model; +import static org.javarosa.core.util.XFormsElement.repeat; import static org.javarosa.core.util.XFormsElement.t; import static org.javarosa.core.util.XFormsElement.title; public class PredicateCachingTest { @Test - public void repeatedPredicatesAreOnlyEvaluatedOnceWhileAnswering() throws Exception { - Scenario scenario = Scenario.init("secondary-instance-filter.xml"); + public void repeatedEqPredicatesAreOnlyEvaluatedOnceWhileAnswering() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("choice"), + t("calculate1"), + t("calculate2") + )), + instance("instance", + item("a", "A"), + item("b", "B") + ), + bind("/data/choice").type("string"), + bind("/data/calculate1").type("string") + .calculate("instance('instance')/root/item[value = /data/choice]/label"), + bind("/data/calculate2").type("string") + .calculate("instance('instance')/root/item[value = /data/choice]/value") + ) + ), + body( + input("/data/choice") + ) + )); - int evaluations = Measure.withMeasure("PredicateEvaluations", new Runnable() { - @Override - public void run() { - scenario.answer("/data/choice","a"); - } + int evaluations = Measure.withMeasure(asList("PredicateEvaluation", "IndexEvaluation"), () -> { + scenario.answer("/data/choice", "a"); + }); + + // Check that we do less than (size of secondary instance) * (number of calculates with a filter) + assertThat(evaluations, lessThan(4)); + } + + @Test + public void repeatedCompPredicatesAreOnlyEvaluatedOnceWhileAnswering() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("choice"), + t("calculate1"), + t("calculate2") + )), + instance("instance", + item("1", "A"), + item("2", "B") + ), + bind("/data/choice").type("string"), + bind("/data/calculate1").type("string") + .calculate("instance('instance')/root/item[value < /data/choice]/label"), + bind("/data/calculate2").type("string") + .calculate("instance('instance')/root/item[value < /data/choice]/value") + ) + ), + body( + input("/data/choice") + ) + )); + + int evaluations = Measure.withMeasure(asList("PredicateEvaluation", "IndexEvaluation"), () -> { + scenario.answer("/data/choice", "2"); }); // Check that we do less than (size of secondary instance) * (number of calculates with a filter) assertThat(evaluations, lessThan(4)); } + @Test + public void repeatedIdempotentFuncPredicatesAreOnlyEvaluatedOnceWhileAnswering() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("choice"), + t("calculate1"), + t("calculate2") + )), + instance("instance", + item("1", "A"), + item("2", "B") + ), + bind("/data/choice").type("string"), + bind("/data/calculate1").type("string") + .calculate("instance('instance')/root/item[regex(value, /data/choice)]/label"), + bind("/data/calculate2").type("string") + .calculate("instance('instance')/root/item[regex(value, /data/choice)]/value") + ) + ), + body( + input("/data/choice") + ) + )); + + int evaluations = Measure.withMeasure(asList("PredicateEvaluation", "IndexEvaluation"), () -> { + scenario.answer("/data/choice", "1"); + }); + + // Check that we do less than (size of secondary instance) * (number of calculates with a filter) + assertThat(evaluations, lessThan(4)); + } + + @Test + public void repeatedEqPredicatesAreOnlyEvaluatedOnce() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("choice"), + t("calculate1"), + t("calculate2") + )), + instance("instance", + item("a", "A"), + item("b", "B") + ), + bind("/data/choice").type("string"), + bind("/data/calculate1").type("string") + .calculate("instance('instance')/root/item[value = /data/choice]/label"), + bind("/data/calculate2").type("string") + .calculate("instance('instance')/root/item[value = /data/choice]/value") + ) + ), + body( + input("/data/choice") + ) + )); + + int evaluations = Measure.withMeasure(asList("PredicateEvaluation", "IndexEvaluation"), () -> { + scenario.answer("/data/choice", "a"); + scenario.answer("/data/choice", "b"); + }); + + // Check that we do less than size of secondary instance * number of times we answer + assertThat(evaluations, lessThan(4)); + } + + @Test + public void firstPredicateInMultipleEqPredicatesAreOnlyEvaluatedOnce() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("calc"), + t("input1"), + t("input2") + )), + instance("instance", + t("item", + t("value", "A"), + t("count", "2"), + t("id", "A2") + ), + t("item", + t("value", "A"), + t("count", "3"), + t("id", "A3") + ), + t("item", + t("value", "B"), + t("count", "2"), + t("id", "B2") + ) + ), + bind("/data/calc").type("string") + .calculate("instance('instance')/root/item[value = /data/input1][count = /data/input2]/id"), + bind("/data/input1").type("string"), + bind("/data/input2").type("string") + ) + ), + body( + input("/data/input1"), + input("/data/input2") + ) + )); + + int evaluations = Measure.withMeasure(asList("PredicateEvaluation", "IndexEvaluation"), () -> { + scenario.answer("/data/input1", "A"); + scenario.answer("/data/input2", "3"); + + scenario.answer("/data/input1", "A"); + scenario.answer("/data/input2", "2"); + }); + + // Check that we do less than size of (secondary instance + filtered secondary instance) * number of times we answer + assertThat(evaluations, lessThan(20)); + } + + @Test + public void repeatedCompPredicatesWithSameAnswerAreOnlyEvaluatedOnce() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("choice"), + t("calculate1"), + t("calculate2") + )), + instance("instance", + item("1", "A"), + item("2", "B") + ), + bind("/data/choice").type("string"), + bind("/data/calculate1").type("string") + .calculate("instance('instance')/root/item[value < /data/choice]/label"), + bind("/data/calculate2").type("string") + .calculate("instance('instance')/root/item[value < /data/choice]/value") + ) + ), + body( + input("/data/choice") + ) + )); + + int evaluations = Measure.withMeasure(asList("PredicateEvaluation", "IndexEvaluation"), () -> { + scenario.answer("/data/choice", "2"); + scenario.answer("/data/choice", "2"); + }); + + // Check that we do less than size of secondary instance * number of times we answer + assertThat(evaluations, lessThan(4)); + } + /** * A form with multiple secondary instances can have expressions with "equivalent" predicates that filter on * different sets of children. It's pretty possible to write a bug where these predicates are treated as the same @@ -87,16 +307,302 @@ public void predicatesOnDifferentChildNamesDoNotGetConfused() throws Exception { ) ), bind("/data/cat").type("string") - .calculate("instance('instance')/root/cat[name = 'Vinnie']/age"), + .calculate("instance('instance')/root/cat[name = /data/input]/age"), bind("/data/dog").type("string") - .calculate("instance('instance')/root/dog[name = 'Vinnie']/age"), + .calculate("instance('instance')/root/dog[name = /data/input]/age"), bind("/data/input").type("string") ) ), body(input("/data/input")) )); + scenario.answer("/data/input", "Vinnie"); + assertThat(scenario.answerOf("/data/cat").getValue(), equalTo("12")); assertThat(scenario.answerOf("/data/dog").getValue(), equalTo("9")); } + + @Test + public void eqExpressionsWorkIfEitherSideIsRelative() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("calcltr"), + t("calcrtl"), + t("input") + )), + instance("instance", + t("item", + t("value", "A") + ), + t("item", + t("value", "B") + ) + ), + bind("/data/calcltr").type("string") + .calculate("instance('instance')/root/item[value = /data/input]/value"), + bind("/data/calcrtl").type("string") + .calculate("instance('instance')/root/item[/data/input = value]/value"), + bind("/data/input").type("string") + ) + ), + body(input("/data/input")) + )); + + scenario.answer("/data/input", "A"); + assertThat(scenario.answerOf("/data/calcltr").getValue(), equalTo("A")); + assertThat(scenario.answerOf("/data/calcrtl").getValue(), equalTo("A")); + + scenario.answer("/data/input", "B"); + assertThat(scenario.answerOf("/data/calcltr").getValue(), equalTo("B")); + assertThat(scenario.answerOf("/data/calcrtl").getValue(), equalTo("B")); + } + + @Test + public void eqExpressionsWorkIfBothSidesAreRelative() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("calc"), + t("input") + )), + instance("instance", + t("item", + t("value", "A") + ) + ), + bind("/data/calc").type("string") + .calculate("instance('instance')/root/item[value = value]/value"), + bind("/data/input").type("string") + ) + ), + body(input("/data/input")) + )); + + assertThat(scenario.answerOf("/data/calc").getValue(), equalTo("A")); + } + + @Test + public void nestedPredicatesDoNotGetConfused() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("calc"), + t("calc2"), + t("input1"), + t("input2") + )), + instance("instance", + t("item", + t("value", "A"), + t("count", "2"), + t("id", "A2") + ), + t("item", + t("value", "A"), + t("count", "3"), + t("id", "A3") + ), + t("item", + t("value", "B"), + t("count", "2"), + t("id", "B2") + ) + ), + bind("/data/calc").type("string") + .calculate("instance('instance')/root/item[value = /data/input1][count = '3']/id"), + bind("/data/calc2").type("string") + .calculate("instance('instance')/root/item[value = /data/input2][count = '3']/id"), + bind("/data/input1").type("string"), + bind("/data/input2").type("string") + ) + ), + body( + input("/data/input1"), + input("/data/input2") + ) + )); + + scenario.answer("/data/input1", "A"); + scenario.answer("/data/input2", "B"); + + assertThat(scenario.answerOf("/data/calc").getValue(), equalTo("A3")); + assertThat(scenario.answerOf("/data/calc2"), equalTo(null)); + } + + @Test + public void similarCmpAndEqExpressionsDoNotGetConfused() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("input"), + t("calculate1"), + t("calculate2") + )), + instance("instance", + item("1", "A"), + item("2", "B") + ), + bind("/data/input").type("string"), + bind("/data/calculate1").type("string") + .calculate("instance('instance')/root/item[value < /data/input]/label"), + bind("/data/calculate2").type("string") + .calculate("instance('instance')/root/item[value = /data/input]/label") + ) + ), + body( + input("/data/input") + ) + )); + + scenario.answer("/data/input", "2"); + assertThat(scenario.answerOf("/data/calculate1").getValue(), equalTo("A")); + assertThat(scenario.answerOf("/data/calculate2").getValue(), equalTo("B")); + } + + @Test + public void differentEqExpressionsAreNotConfused() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("calc1"), + t("calc2"), + t("input1"), + t("input2") + )), + instance("instance", + item("a", "A"), + item("b", "B") + ), + bind("/data/calc1").type("string") + .calculate("instance('instance')/root/item[value = /data/input1]/label"), + bind("/data/calc2").type("string") + .calculate("instance('instance')/root/item[label = /data/input2]/label"), + bind("/data/input").type("string") + ) + ), + body( + input("/data/input1"), + input("/data/input2") + ) + )); + + scenario.answer("/data/input1", "a"); + scenario.answer("/data/input2", "B"); + + assertThat(scenario.answerOf("/data/calc1").getValue(), equalTo("A")); + assertThat(scenario.answerOf("/data/calc2").getValue(), equalTo("B")); + } + + @Test + public void differentKindsOfEqExpressionsAreNotConfused() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("calc1"), + t("calc2"), + t("input") + )), + instance("instance", + item("a", "A"), + item("b", "B") + ), + bind("/data/calc1").type("string") + .calculate("instance('instance')/root/item[value = 'a']/label"), + bind("/data/calc2").type("string") + .calculate("instance('instance')/root/item[value != 'a']/label"), + bind("/data/input").type("string") + ) + ), + body( + input("/data/input") + ) + )); + + assertThat(scenario.answerOf("/data/calc1").getValue(), equalTo("A")); + assertThat(scenario.answerOf("/data/calc2").getValue(), equalTo("B")); + } + + @Test + public void repeatsUsedInCalculatesStayUpToDate() throws Exception { + Scenario scenario = Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("repeat", + t("name"), + t("age") + ), + t("result") + )), + bind("/data/repeat/input").type("string"), + bind("/data/result").type("string").calculate("/data/repeat[name = 'John Bell']/age") + ) + ), + body( + group("/data/repeat", + repeat("/data/repeat", + input("/data/repeat/name"), + input("/data/repeat/age") + ) + ) + ) + )); + + assertThat(scenario.answerOf("/data/result"), equalTo(null)); + + scenario.createNewRepeat("/data/repeat"); + scenario.answer("/data/repeat[0]/name", "John Bell"); + scenario.answer("/data/repeat[0]/age", "70"); + + assertThat(scenario.answerOf("/data/result").getValue(), equalTo("70")); + } + + @Test + public void eqPredicatesDoNotIncreaseLoadTime() { + int evaluations = Measure.withMeasure(asList("PredicateEvaluation", "IndexEvaluation"), () -> { + try { + Scenario.init("Some form", html( + head( + title("Some form"), + model( + mainInstance(t("data id=\"some-form\"", + t("choice"), + t("calculate1"), + t("calculate2") + )), + instance("instance", + item("a", "A"), + item("b", "B") + ), + bind("/data/choice").type("string"), + bind("/data/calculate1").type("string") + .calculate("instance('instance')/root/item[value = /data/choice]/label") + ) + ), + body( + input("/data/choice") + ) + )); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + ); + + assertThat(evaluations, not(greaterThan(2))); + } } diff --git a/src/test/resources/secondary-instance-filter.xml b/src/test/resources/secondary-instance-filter.xml deleted file mode 100644 index f541b5190..000000000 --- a/src/test/resources/secondary-instance-filter.xml +++ /dev/null @@ -1,41 +0,0 @@ - - - - Secondary instance filter - - - - - - - - - - - - a - - - - b - - - - - - - - - - - - - - - - -