Merge pull request #49 from nedap/fix_bug_empty_form

Rule Evaluation: construct only needed structure when fixing assertions

src/main/java/com/nedap/archie/query/APathToXPathConverter.java

@@ -31,9 +31,11 @@ public class APathToXPathConverter {
 
     public static String convertQueryToXPath(String query, String firstNodeName) {
         String convertedQuery = convertWithAntlr(query);
-        if(query.startsWith("//")) {
+        if(convertedQuery.equals("/")) {
+            return "/" + firstNodeName;
+        } else if(query.startsWith("//")) {
             return convertedQuery;
-        } if(query.startsWith("/")) {
+        } else if(query.startsWith("/")) {
             return "/" + firstNodeName + convertedQuery;
         } else {
             return convertedQuery;

src/main/java/com/nedap/archie/query/RMQueryContext.java

@@ -37,6 +37,7 @@ public class RMQueryContext {
     private final XPathFactory xPathFactory;
     private Binder<Node> binder;
     private Document domForQueries;
+    private Object rootNode;
 
     /**
      * TODO: for now we will add /firstXPathNode, because otherwise there will be something like '/composition' missing
@@ -60,6 +61,7 @@ public RMQueryContext(Object rootNode) {
      */
     public RMQueryContext(Object rootNode, JAXBContext jaxbContext) {
         try {
+            this.rootNode = rootNode;
             this.binder = jaxbContext.createBinder();
             domForQueries = createBlankDOMDocument(true);
 
@@ -96,17 +98,24 @@ public Document createBlankDOMDocument(boolean namespaceAware) {
     }
 
     public <T> List<T> findList(String query) throws XPathExpressionException {
-        String convertedQuery = APathToXPathConverter.convertQueryToXPath(query, firstXPathNode);
-        XPath xpath = xPathFactory.newXPath();
+        List<T> result = new ArrayList<T>();
 
+        if (query.equals("/")) {
+            result.add((T) rootNode);
+        } else {
 
-        xpath.setNamespaceContext( new ArchieNamespaceResolver(domForQueries));
-        NodeList foundNodes = (NodeList)xpath.evaluate(convertedQuery, domForQueries, XPathConstants.NODESET);
-        List<T> result = new ArrayList<T>();
-        //Perform decoration
-        for(int i=0; i<foundNodes.getLength(); i++){
-            Node node = foundNodes.item(i);
-            result.add(getJAXBNode(node));
+            String convertedQuery = APathToXPathConverter.convertQueryToXPath(query, firstXPathNode);
+            XPath xpath = xPathFactory.newXPath();
+
+
+            xpath.setNamespaceContext(new ArchieNamespaceResolver(domForQueries));
+            NodeList foundNodes = (NodeList) xpath.evaluate(convertedQuery, domForQueries, XPathConstants.NODESET);
+
+            //Perform decoration
+            for (int i = 0; i < foundNodes.getLength(); i++) {
+                Node node = foundNodes.item(i);
+                result.add(getJAXBNode(node));
+            }
         }
 
         return result;

src/main/java/com/nedap/archie/rules/evaluation/AssertionsFixer.java

@@ -60,7 +60,8 @@ public void fixAssertions(Archetype archetype, AssertionResult assertionResult)
 
                 while(parents.isEmpty()) {
                     //there's object missing in the RMObject. Construct it here.
-                    parents = constructMissingStructure(archetype, pathOfParent, lastPathSegment, parents);
+                    constructMissingStructure(archetype, pathOfParent, lastPathSegment, parents);
+                    parents = ruleEvaluation.getQueryContext().findList(pathOfParent);
                 }
 
                 for(Object parent:parents) {
@@ -188,7 +189,7 @@ private void setTerminologyFromArchetype(Archetype archetype, DvCodedText codedT
         }
     }
 
-    private List<Object> constructMissingStructure(Archetype archetype, String pathOfParent, String lastPathSegment, List<Object> parents) throws XPathExpressionException {
+    private void constructMissingStructure(Archetype archetype, String pathOfParent, String lastPathSegment, List<Object> parents) throws XPathExpressionException {
         //TODO: this is great but not enough. Fix it by hardcoding support for DV_CODED_TEXT and DV_ORDINAL, here or in the FixableAssertionsChecker.
         String newPathOfParent = pathOfParent;
         String newLastPathSegment = lastPathSegment;
@@ -198,7 +199,13 @@ private List<Object> constructMissingStructure(Archetype archetype, String pathO
             newPathOfParent = stripLastPathSegment(newPathOfParent);
             parents = ruleEvaluation.getQueryContext().findList(newPathOfParent);
         }
-        List<ArchetypeModelObject> constraints = archetype.itemsAtPath(newPathOfParent + "/" + newLastPathSegment);
+        List<ArchetypeModelObject> constraints;
+        if (newPathOfParent.equals("/")) {
+            constraints = archetype.itemsAtPath("/" + newLastPathSegment);
+        } else {
+            constraints = archetype.itemsAtPath(newPathOfParent + "/" + newLastPathSegment);
+        }
+
         if (constraintsHasNoComplexObjects(constraints)) {
             Object object = parents.get(0);
 
@@ -207,7 +214,6 @@ private List<Object> constructMissingStructure(Archetype archetype, String pathO
 
             creator.addElementToListOrSetSingleValues(object, newLastPathSegment, Lists.newArrayList(newEmptyObject));
             ruleEvaluation.refreshQueryContext();
-            parents = ruleEvaluation.getQueryContext().findList(pathOfParent);
         } else {
             CObject constraint = getCObjectFromResult(constraints);
             if (constraint != null) {
@@ -229,10 +235,8 @@ private List<Object> constructMissingStructure(Archetype archetype, String pathO
                 creator.addElementToListOrSetSingleValues(object, attributeName, Lists.newArrayList(newEmptyObject));
 
                 ruleEvaluation.refreshQueryContext();
-                parents = ruleEvaluation.getQueryContext().findList(pathOfParent);
             }
         }
-        return parents;
     }
 
     private CObject getCObjectFromResult(List<? extends ArchetypeModelObject> objects) {

src/main/java/com/nedap/archie/rules/evaluation/EmptyRMObjectConstructor.java

@@ -28,24 +28,6 @@ class EmptyRMObjectConstructor {
      * @return
      */
     RMObject constructEmptyRMObject(CObject object) {
-        RMObject result = creator.create(object);
-        for(CAttribute attribute: object.getAttributes()) {
-            List<Object> children = new ArrayList<>();
-            for(CObject childConstraint:attribute.getChildren()) {
-                if(childConstraint instanceof CComplexObject) {
-                    RMObject childObject = constructEmptyRMObject(childConstraint);
-                    children.add(childObject);
-                }
-            }
-            if(!children.isEmpty()) {
-                if(attribute.isMultiple()) {
-                    creator.set(result, attribute.getRmAttributeName(), children);
-                } else if(!children.isEmpty()){
-                    //set the first possible result in case of multiple children for a single valued value
-                    creator.set(result, attribute.getRmAttributeName(), Lists.newArrayList(children.get(0)));
-                }
-            }
-        }
-        return result;
+        return creator.create(object);
     }
 }

src/test/java/com/nedap/archie/rules/evaluation/FixableAssertionsCheckerTest.java

@@ -19,10 +19,12 @@ public class FixableAssertionsCheckerTest {
     private Archetype archetype;
 
     private TestUtil testUtil;
+    private EmptyRMObjectConstructor emptyRMObjectConstructor;
 
     @Before
     public void setup() {
         testUtil = new TestUtil();
+        emptyRMObjectConstructor = new EmptyRMObjectConstructor();
         parser = ADLParser.withRMConstraintsImposer();
     }
 
@@ -88,4 +90,28 @@ public void andExpression() throws Exception {
 
     }
 
+    @Test
+    public void constructOnlyNecessaryStructure() throws Exception {
+        archetype = parser.parse(ParsedRulesEvaluationTest.class.getResourceAsStream("construct_only_necessary_structure.adls"));
+        RuleEvaluation ruleEvaluation = new RuleEvaluation(archetype);
+
+        Pathable root = (Pathable) emptyRMObjectConstructor.constructEmptyRMObject(archetype.getDefinition());
+        EvaluationResult evaluate = ruleEvaluation.evaluate(root, archetype.getRules().getRules());
+        assertEquals("there must be three values that must be set", 1, evaluate.getSetPathValues().size());
+
+        //assert that paths must be set to specific values
+        assertEquals("test string", evaluate.getSetPathValues().get("/data[id2]/events[id3]/data[id4]/items[id5]/value/value").getValue());
+        assertEquals(null, evaluate.getSetPathValues().get("d/ata[id2]/events[id3]/data[id4]/items[id6]/value"));
+
+        //now assert that the RM Object cloned by rule evaluation has been modified with the new values for further evaluation
+        assertEquals("test string", ruleEvaluation.getRMRoot().itemAtPath("/data[id2]/events[id3]/data[id4]/items[id5]/value/value"));
+        assertEquals(null, ruleEvaluation.getRMRoot().itemAtPath("/data[id2]/events[id3]/data[id4]/items[id6]/value"));
+
+        evaluate = ruleEvaluation.evaluate(ruleEvaluation.getRMRoot(), archetype.getRules().getRules());
+        for(AssertionResult result:evaluate.getAssertionResults()) {
+            assertTrue(result.getResult());
+        }
+
+    }
+
 }

src/test/resources/com/nedap/archie/rules/evaluation/construct_only_necessary_structure.adls

@@ -0,0 +1,82 @@
+archetype (adl_version=2.0.5; rm_release=1.0.2; generated)
+	openEHR-EHR-OBSERVATION.matches.v1.0.0
+
+language
+	original_language = <[ISO_639-1::en]>
+
+description
+	original_author = <
+		["name"] = <"Pieter Bos">
+	>
+	details = <
+		["en"] = <
+			language = <[ISO_639-1::en]>
+			purpose = <"Test for rules, simple constant arithmetics">
+			keywords = <"ADL", "test">
+		>
+	>
+	lifecycle_state = <"published">
+	other_details = <
+		["regression"] = <"PASS">
+	>
+	copyright = <"copyright © 2004 openEHR Foundation <http://www.openEHR.org>">
+
+definition
+	OBSERVATION[id1] matches {	-- Body mass index
+		data matches {
+			HISTORY[id2] matches {
+				events cardinality matches {1..*; unordered} matches {
+					EVENT[id3] occurrences matches {1..*} matches {	-- Any event
+						data matches {
+							ITEM_TREE[id4] matches {
+								items cardinality matches {1..*; unordered} matches {
+									ELEMENT[id5] matches {
+									    value matches {
+                                            DV_TEXT[id64]
+                                        }
+									}
+									ELEMENT[id6] matches {
+                                        value matches {
+                                            DV_ORDINAL[id7] matches {
+                                                [value, symbol] matches {
+                                                    [{2}, {[at1]}],
+                                                    [{1}, {[at2]}],
+                                                    [{0}, {[at3]}]
+                                                }
+                                            }
+                                        }
+									}
+								}
+							}
+						}
+					}
+				}
+			}
+		}
+	}
+
+rules
+	/data[id2]/events[id3]/data[id4]/items[id5]/value/value matches {"test string"}
+
+terminology
+	term_definitions = <
+		["en"] = <
+			["id1"] = <
+				text = <"Blood Pressure">
+				description = <"The local measurement of arterial blood pressure which is a surrogate for arterial. pressure in the systemic circulation.  Most commonly, use of the term 'blood pressure' refers to measurement of brachial artery pressure in the upper arm.">
+			>
+            ["at1"] = <
+                text = <"De patiënt kan dit zelf">
+                description = <"De patiënt kan dit zelf">
+            >
+            ["at2"] = <
+                text = <"De patiënt heeft hulp van 1 persoon nodig">
+                description = <"De patiënt heeft hulp van 1 persoon nodig">
+            >
+            ["at3"] = <
+                text = <"De patiënt heeft hulp van 2 of meer personen nodig">
+                description = <"De patiënt heeft hulp van 2 of meer personen nodig">
+            >
+		>
+    >
+