Merge branch 'master' into ctsims/fixture_performance_improvement

src/cli/java/org/commcare/util/engine/CommCareConfigEngine.java

@@ -74,7 +74,7 @@ public CommCareConfigEngine(PrototypeFactory prototypeFactory) {
 
     public CommCareConfigEngine(OutputStream output, PrototypeFactory prototypeFactory) {
         this.print = new PrintStream(output);
-        this.platform = new CommCarePlatform(2, 37);
+        this.platform = new CommCarePlatform(2, 38);
         this.liveFactory = prototypeFactory;
 
         if (storageFactory == null) {

src/main/java/org/commcare/cases/entity/NodeEntityFactory.java

@@ -67,11 +67,17 @@ public Entity<TreeReference> getEntity(TreeReference data) {
                     sortData[count] = sortText.evaluate(nodeContext);
                 }
                 relevancyData[count] = f.isRelevant(nodeContext);
-            } catch (XPathSyntaxException e) {
-                storeErrorDetails(e, count, fieldData, relevancyData);
-            } catch (XPathException xpe) {
-                //XPathErrorLogger.INSTANCE.logErrorToCurrentApp(xpe);
-                storeErrorDetails(xpe, count, fieldData, relevancyData);
+            } catch (XPathSyntaxException | XPathException e) {
+                /**
+                 * TODO: 25/06/17 remove catch blocks from here
+                 * We are wrapping the original exception in a new XPathException to avoid
+                 * refactoring large number of functions caused by throwing XPathSyntaxException here.
+                 */
+                XPathException xe = new XPathException(e.getMessage(), e);
+                if (e instanceof XPathException) {
+                    xe.setSource(((XPathException)e).getSource());
+                }
+                throw xe;
             }
             count++;
         }
@@ -97,15 +103,6 @@ protected String loadCalloutDataMapKey(EvaluationContext entityContext) {
         return null;
     }
 
-    private static void storeErrorDetails(Exception e, int index,
-                                          Object[] details,
-                                          boolean[] relevancyDetails) {
-        e.printStackTrace();
-        details[index] = "<invalid xpath: " + e.getMessage() + ">";
-        // assume that if there's an error, user should see it
-        relevancyDetails[index] = true;
-    }
-
     public List<TreeReference> expandReferenceList(TreeReference treeReference) {
         EvaluationContext tracableContext = new EvaluationContext(ec, ec.getOriginalContext());
         if (inDebugMode) {

src/main/java/org/commcare/cases/instance/FixtureIndexSchema.java

@@ -57,7 +57,7 @@ public Set<String> getColumnIndices() {
         return columnIndices;
     }
 
-    private static String escapeIndex(String index) {
+    public static String escapeIndex(String index) {
         if (index.contains(",")) {
             StringBuilder compoundIndex = new StringBuilder();
             String prefix = "";

src/main/java/org/commcare/cases/model/StorageIndexedTreeElementModel.java

@@ -1,5 +1,6 @@
 package org.commcare.cases.model;
 
+import org.commcare.cases.instance.FixtureIndexSchema;
 import org.javarosa.core.model.data.IAnswerData;
 import org.javarosa.core.model.instance.TreeElement;
 import org.javarosa.core.services.storage.IMetaData;
@@ -13,6 +14,7 @@
 import java.io.DataOutputStream;
 import java.io.IOException;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.Vector;
@@ -110,6 +112,14 @@ public TreeElement getRoot() {
         return root;
     }
 
+    public Set<String> getIndexColumnNames() {
+        Set<String> indexColumnNames = new HashSet<>();
+        for (String index : this.indices) {
+            indexColumnNames.add(FixtureIndexSchema.escapeIndex(index));
+        }
+        return indexColumnNames;
+    }
+
     @Override
     public void readExternal(DataInputStream in, PrototypeFactory pf)
             throws IOException, DeserializationException {

src/main/java/org/commcare/modern/database/DatabaseIndexingUtils.java

@@ -33,7 +33,7 @@ public static String[] getIndexStatements(String tableName, Set<String> indices)
     }
 
     private static String makeIndexingStatement(String tableName, String index) {
-        String indexName = index + "_index";
+        String indexName = "fixture_" + tableName + "_" + index + "_index";
         if (index.contains(",")) {
             indexName = index.replaceAll(",", "_") + "_index";
         }

src/main/java/org/commcare/suite/model/Detail.java

@@ -145,7 +145,7 @@ public Detail(String id, DisplayUnit title, String nodeset, Vector<Detail> detai
             numEntitiesToDisplayPerRow = 1;
         }
 
-        if (relevancy != null) {
+        if (relevancy != null && !"".equals(relevancy)) {
             try {
                 this.parsedRelevancyExpression = XPathParseTool.parseXPath(relevancy);
             } catch (XPathSyntaxException e) {

src/main/java/org/commcare/xml/IndexedFixtureXmlParser.java

@@ -39,11 +39,11 @@ public class IndexedFixtureXmlParser extends TransactionParser<StorageIndexedTre
     private IStorageUtilityIndexed<StorageIndexedTreeElementModel> indexedFixtureStorage;
     private IStorageUtilityIndexed<FormInstance> normalFixtureStorage;
 
-    public IndexedFixtureXmlParser(KXmlParser parser, String fixtureName,
+    public IndexedFixtureXmlParser(KXmlParser parser, String fixtureId,
                                    FixtureIndexSchema schema, UserSandbox sandbox) {
         super(parser);
         this.sandbox = sandbox;
-        this.fixtureName = fixtureName;
+        this.fixtureName = fixtureId;
 
         if (schema == null) {
             // don't create any table indices if there was no fixture index schema
@@ -60,32 +60,31 @@ public StorageIndexedTreeElementModel parse() throws InvalidStructureException,
             XmlPullParserException, UnfullfilledRequirementsException {
         checkNode("fixture");
 
-        String fixtureId = parser.getAttributeValue(null, "id");
-        if (fixtureId == null) {
+        if (fixtureName == null) {
             throw new InvalidStructureException("fixture is lacking id attribute", parser);
         }
 
         if (nextTagInBlock("fixture")) {
             // only commit fixtures with bodies to storage
-            TreeElement root = new TreeElementParser(parser, 0, fixtureId).parse();
-            processRoot(root, fixtureId);
+            TreeElement root = new TreeElementParser(parser, 0, fixtureName).parse();
+            processRoot(root);
 
             // commit whole instance to normal fixture storage to allow for
             // migrations going forward, if ever needed
             String userId = parser.getAttributeValue(null, "user_id");
             Pair<FormInstance, Boolean> instanceAndCommitStatus =
                     FixtureXmlParser.setupInstance(getNormalFixtureStorage(),
-                            root, fixtureId, userId, true);
+                            root, fixtureName, userId, true);
             commitToNormalStorage(instanceAndCommitStatus.first);
         }
 
         return null;
     }
 
-    private void processRoot(TreeElement root, String fixtureId) throws IOException {
+    private void processRoot(TreeElement root) throws IOException {
         if (root.hasChildren()) {
             String entryName = root.getChildAt(0).getName();
-            writeFixtureIndex(fixtureId, root.getName(), entryName);
+            writeFixtureIndex(root.getName(), entryName);
 
             for (TreeElement entry : root.getChildrenWithName(entryName)) {
                 processEntry(entry, indices);
@@ -95,7 +94,6 @@ private void processRoot(TreeElement root, String fixtureId) throws IOException
 
     private void processEntry(TreeElement child, Set<String> indices) throws IOException {
         StorageIndexedTreeElementModel model = new StorageIndexedTreeElementModel(indices, child);
-
         commit(model);
     }
 
@@ -128,9 +126,9 @@ private IStorageUtilityIndexed<FormInstance> getNormalFixtureStorage() {
 
     /**
      * Store base and child node names associated with a fixture.
-     * Used for reconstructiong fixture instance
+     * Used for reconstructing fixture instance
      */
-    private void writeFixtureIndex(String fixtureName, String baseName, String childName) {
+    private void writeFixtureIndex(String baseName, String childName) {
         sandbox.setIndexedFixturePathBases(fixtureName, baseName, childName);
     }
 }

src/main/java/org/javarosa/core/model/FormDef.java

@@ -1310,17 +1310,14 @@ public String fillTemplateString(String template, TreeReference contextRef, Hash
      *                used to determine the values to be chosen from.
      */
     public void populateDynamicChoices(ItemsetBinding itemset, TreeReference curQRef) {
-        Vector<SelectChoice> choices = new Vector<>();
-
-        DataInstance fi;
-        if (itemset.nodesetRef.getInstanceName() != null) //We're not dealing with the default instance
-        {
-            fi = getNonMainInstance(itemset.nodesetRef.getInstanceName());
-            if (fi == null) {
+        DataInstance formInstance;
+        if (itemset.nodesetRef.getInstanceName() != null) {
+            formInstance = getNonMainInstance(itemset.nodesetRef.getInstanceName());
+            if (formInstance == null) {
                 throw new XPathException("Instance " + itemset.nodesetRef.getInstanceName() + " not found");
             }
         } else {
-            fi = getMainInstance();
+            formInstance = getMainInstance();
         }
 
         EvaluationContext ec =
@@ -1332,7 +1329,7 @@ public void populateDynamicChoices(ItemsetBinding itemset, TreeReference curQRef
             ec.setDebugModeOn(reporter);
         }
 
-        Vector<TreeReference> matches = itemset.nodesetExpr.evalNodeset(fi,ec);
+        Vector<TreeReference> matches = itemset.nodesetExpr.evalNodeset(formInstance,ec);
 
         if(reporter != null) {
             InstrumentationUtils.printAndClearTraces(reporter, "itemset expansion");
@@ -1350,34 +1347,48 @@ public void populateDynamicChoices(ItemsetBinding itemset, TreeReference curQRef
             }
         }
 
+        Vector<SelectChoice> choices = new Vector<>();
         //Escalate the new context if our result set is substantial, this will prevent reverting
         //from a bulk read mode to a scanned read mode
         QueryContext newContext = ec.getCurrentQueryContext()
                 .checkForDerivativeContextAndReturn(matches.size());
         ec.setQueryContext(newContext);
 
         for (int i = 0; i < matches.size(); i++) {
-            TreeReference item = matches.elementAt(i);
+            choices.addElement(buildSelectChoice(matches.elementAt(i), itemset, formInstance, ec, i));
+        }
+        itemset.setChoices(choices);
+    }
 
-            String label = itemset.labelExpr.evalReadable(fi, new EvaluationContext(ec, item));
-            String value = null;
-            TreeElement copyNode = null;
+    private SelectChoice buildSelectChoice(TreeReference choiceRef, ItemsetBinding itemset,
+                                           DataInstance formInstance, EvaluationContext ec, int index) {
+        String label = itemset.labelExpr.evalReadable(formInstance,
+                new EvaluationContext(ec, choiceRef));
+        String value = null;
+        TreeElement copyNode = null;
+        if (itemset.copyMode) {
+            copyNode = this.getMainInstance().resolveReference(itemset.copyRef.contextualize(choiceRef));
+        }
+        if (itemset.valueRef != null) {
+            value = itemset.valueExpr.evalReadable(formInstance,
+                    new EvaluationContext(ec, choiceRef));
+        }
 
-            if (itemset.copyMode) {
-                copyNode = this.getMainInstance().resolveReference(itemset.copyRef.contextualize(item));
-            }
-            if (itemset.valueRef != null) {
-                value = itemset.valueExpr.evalReadable(fi, new EvaluationContext(ec, item));
-            }
-            SelectChoice choice = new SelectChoice(label, value != null ? value : "dynamic:" + i, itemset.labelIsItext);
-            choice.setIndex(i);
-            if (itemset.copyMode)
-                choice.copyNode = copyNode;
+        SelectChoice choice = new SelectChoice(label, value != null ? value : "dynamic:" + index,
+                itemset.labelIsItext);
+        choice.setIndex(index);
+
+        if (itemset.copyMode) {
+            choice.copyNode = copyNode;
+        }
 
-            choices.addElement(choice);
+        if (itemset.sortRef != null) {
+            String evaluatedSortProperty = itemset.sortExpr.evalReadable(formInstance,
+                    new EvaluationContext(ec, choiceRef));
+            choice.setSortProperty(evaluatedSortProperty);
         }
 
-        itemset.setChoices(choices, this.getLocalizer());
+        return choice;
 
         if(reporter != null) {
             InstrumentationUtils.printAndClearTraces(reporter, "itemset population");

src/main/java/org/javarosa/core/model/ItemsetBinding.java

@@ -4,7 +4,6 @@
 import org.javarosa.core.model.instance.FormInstance;
 import org.javarosa.core.model.instance.TreeReference;
 import org.javarosa.core.model.util.restorable.RestoreUtils;
-import org.javarosa.core.services.locale.Localizer;
 import org.javarosa.core.util.externalizable.DeserializationException;
 import org.javarosa.core.util.externalizable.ExtUtil;
 import org.javarosa.core.util.externalizable.ExtWrapNullable;
@@ -15,6 +14,8 @@
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
 import java.io.IOException;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.Vector;
 
 public class ItemsetBinding implements Externalizable {
@@ -37,23 +38,47 @@ public class ItemsetBinding implements Externalizable {
 
     public boolean copyMode;           //true = copy subtree; false = copy string value
     public TreeReference copyRef;      //absolute ref to copy
+
     public TreeReference valueRef;     //absolute ref to value
     public IConditionExpr valueExpr;   //path expression for value; may be relative, no predicates (must be relative if copy mode)
 
+    public TreeReference sortRef;     //absolute ref to sort
+    public IConditionExpr sortExpr;   //path expression for sort; may be relative, no predicates (must be relative if copy mode)
+
     private TreeReference destRef; //ref that identifies the repeated nodes resulting from this itemset
-    //not serialized -- set by QuestionDef.setDynamicChoices()
-    private Vector<SelectChoice> choices; //dynamic choices -- not serialized, obviously
+
+    // dynamic choices, not serialized
+    private Vector<SelectChoice> choices;
 
     public Vector<SelectChoice> getChoices() {
         return choices;
     }
 
-    public void setChoices(Vector<SelectChoice> choices, Localizer localizer) {
+    public void setChoices(Vector<SelectChoice> choices) {
         if (this.choices != null) {
             System.out.println("warning: previous choices not cleared out");
             clearChoices();
         }
         this.choices = choices;
+        sortChoices();
+    }
+
+    private void sortChoices() {
+        if (this.sortRef != null) {
+
+            // Perform sort
+            Collections.sort(choices, new Comparator<SelectChoice>() {
+                @Override
+                public int compare(SelectChoice choice1, SelectChoice choice2) {
+                    return choice1.evaluatedSortProperty.compareTo(choice2.evaluatedSortProperty);
+                }
+            });
+
+            // Re-set indices after sorting
+            for (int i = 0; i < choices.size(); i++) {
+                choices.get(i).setIndex(i);
+            }
+        }
     }
 
     public void clearChoices() {

src/main/java/org/javarosa/core/model/SelectChoice.java

@@ -18,6 +18,7 @@ public class SelectChoice implements Externalizable {
     private String textID;
     private boolean isLocalizable;
     private String value;
+    public String evaluatedSortProperty;
     private int index = -1;
 
     public TreeElement copyNode; //if this choice represents part of an <itemset>, and the itemset uses 'copy'
@@ -56,6 +57,10 @@ public SelectChoice(String labelOrID, String Value, boolean isLocalizable) {
                 Value, isLocalizable);
     }
 
+    public void setSortProperty(String s) {
+        this.evaluatedSortProperty = s;
+    }
+
     public void setIndex(int index) {
         this.index = index;
     }

src/main/java/org/javarosa/core/model/utils/DateUtils.java

@@ -336,7 +336,9 @@ public static String format(DateFields f, String format) {
                 } else if (c == 'a') {    //Three letter short text day
                     String[] dayNames = new String[]{"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"};
                     sb.append(dayNames[f.dow - 1]);
-                } else if (Arrays.asList('c', 'C', 'D', 'F', 'g', 'G', 'I', 'j', 'k', 'l', 'p', 'P', 'r', 'R', 's', 't', 'T', 'u', 'U', 'V', 'w', 'W', 'x', 'X', 'z', 'Z').contains(c)) {
+                } else if (c == 'w') {    //Day of the week (0 through 6), Sunday being 0.
+                    sb.append(f.dow - 1);
+                } else if (Arrays.asList('c', 'C', 'D', 'F', 'g', 'G', 'I', 'j', 'k', 'l', 'p', 'P', 'r', 'R', 's', 't', 'T', 'u', 'U', 'V', 'W', 'x', 'X', 'z', 'Z').contains(c)) {
                     // Format specifiers supported by libc's strftime: https://www.gnu.org/software/libc/manual/html_node/Formatting-Calendar-Time.html
                     throw new RuntimeException("unsupported escape in date format string [%" + c + "]");
                 } else {

src/main/java/org/javarosa/core/util/DataUtil.java

@@ -1,5 +1,6 @@
 package org.javarosa.core.util;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
@@ -47,6 +48,18 @@ public static <T> List<T> intersection(Collection<T> a, Collection<T> b) {
         return new Vector<>(setA);
     }
 
+    public static String listToString(List<String> list) {
+        StringBuilder sb = new StringBuilder();
+        for (String s : list) {
+            sb.append(s + " ");
+        }
+        return sb.toString().substring(0, sb.length()-1);
+    }
+
+    public static List<String> stringToList(String s) {
+        return Arrays.asList(splitOnSpaces(s));
+    }
+
     public static String[] splitOnSpaces(String s) {
         if ("".equals(s)) {
             return new String[0];