CARDS-2509: Reference question should support conditions based on sou…

…rce form metadata

- Rebase ontop of latest dev to pull in other reference question changes
- Code cleanup:
  - Refactor null checks to remove double negatives
  - Move condition utils to it's own module
  - Fix test form naming issues
  - Clean up constant string naming and reuse utility strings when available

distribution/src/main/features/core/cards.json

@@ -96,6 +96,10 @@
         {
             "id":"io.uhndata.cards:cards-google-apis:${project.version}",
             "start-order":"25"
+        },
+        {
+            "id":"io.uhndata.cards:cards-conditions:${project.version}",
+            "start-order":"25"
         }
     ]
 }

modules/conditions/pom.xml

@@ -0,0 +1,54 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <parent>
+    <groupId>io.uhndata.cards</groupId>
+    <artifactId>cards-modules</artifactId>
+    <version>0.9.26-SNAPSHOT</version>
+  </parent>
+
+  <artifactId>cards-conditions</artifactId>
+  <packaging>bundle</packaging>
+  <name>CARDS - Conditions</name>
+
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.felix</groupId>
+        <artifactId>maven-bundle-plugin</artifactId>
+        <extensions>true</extensions>
+      </plugin>
+    </plugins>
+  </build>
+
+  <dependencies>
+    <dependency>
+      <groupId>javax.jcr</groupId>
+      <artifactId>jcr</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.jackrabbit</groupId>
+      <artifactId>oak-api</artifactId>
+      <version>${oak.version}</version>
+    </dependency>
+  </dependencies>
+</project>

modules/utils/src/main/java/io/uhndata/cards/utils/ConditionalUtils.java → modules/conditions/src/main/java/io/uhndata/cards/conditions/ConditionalUtils.java

@@ -17,7 +17,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package io.uhndata.cards.utils;
+package io.uhndata.cards.conditions;
 
 import java.util.ArrayList;
 import java.util.List;

modules/data-model/forms/api/src/main/java/io/uhndata/cards/forms/api/FormUtils.java

@@ -51,7 +51,7 @@ public interface FormUtils
     String RELATED_SUBJECTS_PROPERTY = "relatedSubjects";
 
     /** The name of a property on an Answer, Section or Form node that holds the status flags for that node.*/
-    String STATUS_FLAGS = "statusFlags";
+    String STATUS_FLAGS_PROPERTY = "statusFlags";
 
     /**
      * The primary node type for an Answer Section, a group of related answers and subsections in a Form, corresponding

modules/data-model/forms/impl/pom.xml

@@ -135,6 +135,11 @@
       <artifactId>cards-export</artifactId>
       <version>${project.version}</version>
     </dependency>
+    <dependency>
+      <groupId>${project.groupId}</groupId>
+      <artifactId>cards-conditions</artifactId>
+      <version>${project.version}</version>
+    </dependency>
     <dependency>
       <groupId>javax.servlet</groupId>
       <artifactId>javax.servlet-api</artifactId>

modules/data-model/forms/impl/src/main/java/io/uhndata/cards/forms/internal/ReferenceAnswersChangedListener.java

@@ -60,9 +60,6 @@
 })
 public class ReferenceAnswersChangedListener implements ResourceChangeListener
 {
-    /** Property on an answer node that stores the a reference to the question. */
-    public static final String QUESTION = "question";
-
     private static final Logger LOGGER = LoggerFactory.getLogger(ReferenceAnswersChangedListener.class);
 
     /** Provides access to resources. */
@@ -125,11 +122,10 @@ private void handleEvent(final ResourceChange event)
 
     /**
      * This method reads through a NodeIterator of changed Nodes. If a given changed Node is a cards/Answer node all
-     * other cards/Answer nodes that make reference to it are updated so that the value property of the referenced Node
-     * matches the value property of the changed node.
+     * other cards/Answer nodes that make reference to it are updated if appropriate so that the value property of the
+     * referenced Node matches the value property of the changed node.
      *
      * @param nodeIterator an iterator of nodes of which have changed due to an update made to a Form
-     * @param serviceResolver a ResourceResolver that can be used for querying the JCR
      * @param session a service session providing access to the repository
      */
     private void checkAndUpdateAnswersValues(final NodeIterator nodeIterator, final Session session)
@@ -145,6 +141,17 @@ private void checkAndUpdateAnswersValues(final NodeIterator nodeIterator, final
             }
         }
     }
+
+    /**
+     * This method reads through a NodeIterator of changed Nodes. If a given changed Node is a cards/Answer node all
+     * other cards/Answer nodes that make reference to it are updated if appropriate so that the value property of the
+     * referenced Node matches the value property of the changed node.
+     * @param nodeIterator an iterator of nodes of which have changed due to an update made to a Form
+     * @param session a service session providing access to the repository
+     * @param checkoutPaths the list of forms that were checked out which will need to be checked back in
+     * @param versionManager the version manager that should be used to checkout any needed forms
+     * @throws RepositoryException if the node could not be processed
+     */
     private void checkAndUpdateAnswersValues(final NodeIterator nodeIterator, final Session session,
         Set<String> checkoutPaths, VersionManager versionManager)
         throws RepositoryException
@@ -159,15 +166,27 @@ private void checkAndUpdateAnswersValues(final NodeIterator nodeIterator, final
         }
     }
 
+    /**
+     * Process a changed cards/Answer node and check if there are any other cards/Answer nodes that reference the
+     * changed node or should reference the changed node. If there are any referencing cards/Answer nodes, update
+     * their values to the newly changed value if appropriate.
+     * @param versionManager the version manager that should be used to checkout any needed forms
+     * @param session a service session providing access to the repository
+     * @param checkoutPaths the list of forms that were checked out which will need to be checked back in
+     * @param answerNode the cards/Answer node to check for any (potential) referencing nodes
+     * @throws RepositoryException if the node could not be processed
+     */
     private void processAnswer(final VersionManager versionManager, final Session session,
         final Set<String> checkoutPaths, final Node answerNode)
         throws RepositoryException
     {
-
         final String answerNodeType = answerNode.getPrimaryNodeType().getName();
         final String subject = this.formUtils.getSubject(this.formUtils.getForm(answerNode)).getIdentifier();
-        // TODO: is this query needed with the refactor in CARDS-2509/2571?
-        // May be possible to replace it with a loop on node.getReferences()
+        // Query for two different types of answers:
+        // 1. Reference answers that are referencing the current changed answer
+        //    (Reference answers already copying from the current answer)
+        // 2. Reference answers that have no value and who's question references the current answer's question
+        //    (Unanswered reference answers that could potentially be copied from the current answer)
         final NodeIterator resourceIteratorReferencingAnswers = session
             .getWorkspace().getQueryManager().createQuery(
                 // Answers that were explicitly copied from this answer
@@ -183,7 +202,7 @@ private void processAnswer(final VersionManager versionManager, final Session se
                     + "    a.value is null"
                     // The answer's question references this question
                     + "    AND q.question = '"
-                    + escape(answerNode.getProperty(QUESTION).getNode().getPath()) + "'"
+                    + escape(answerNode.getProperty(FormUtils.QUESTION_PROPERTY).getNode().getPath()) + "'"
                     // The answer belongs to the same subject or one of its descendants
                     + "    AND f.relatedSubjects = '" + subject + "'"
                     // Use the fast index for the query
@@ -194,7 +213,7 @@ private void processAnswer(final VersionManager versionManager, final Session se
             !answerNode.hasProperty(FormUtils.VALUE_PROPERTY) ? null : answerNode.getProperty(FormUtils.VALUE_PROPERTY);
         while (resourceIteratorReferencingAnswers.hasNext()) {
             final Node referenceAnswer = resourceIteratorReferencingAnswers.nextNode();
-            final Node referenceQuestion = referenceAnswer.getProperty(QUESTION).getNode();
+            final Node referenceQuestion = referenceAnswer.getProperty(FormUtils.QUESTION_PROPERTY).getNode();
             if (updatePolicyApplies(sourceAnswerValue, referenceAnswer)) {
                 if (ReferenceConditionUtils.referenceHasCondition(referenceQuestion)
                     && !ReferenceConditionUtils.isReferenceConditionSatisfied(
@@ -208,6 +227,13 @@ private void processAnswer(final VersionManager versionManager, final Session se
         }
     }
 
+    /**
+     * Check if the reference question's update policy allows for updating the reference answer.
+     * @param source The source answer that may be copied from
+     * @param reference The reference answer that may be updated
+     * @return True if the policy allows for updating, false otherwise
+     * @throws RepositoryException if an unexpected error occurs determining if the policy allows for changes
+     */
     private boolean updatePolicyApplies(final Property source, final Node reference) throws RepositoryException
     {
         String updateMode = "";
@@ -232,7 +258,7 @@ private boolean updatePolicyApplies(final Property source, final Node reference)
     }
 
     /**
-     * Fill out a refernce answer with a value copied from the referenced question.
+     * Fill out a reference answer with a value copied from the referenced question.
      * @param versionManager A version manager to be used to checkout forms if needed
      * @param checkoutPaths The list of forms that have been checkout out and need to be checked back in
      * @param sourceAnswerValue The source answer value to copy the answer from
@@ -273,7 +299,7 @@ private void updateValueWithFallback(final VersionManager versionManager, final
         throws RepositoryException
     {
         Object values = ReferenceConditionUtils.getFallbackValue(session,
-            referenceAnswer.getProperty(QUESTION).getNode());
+            referenceAnswer.getProperty(FormUtils.QUESTION_PROPERTY).getNode());
         Property referenceAnswerProperty = referenceAnswer.hasProperty(FormUtils.VALUE_PROPERTY)
             ? referenceAnswer.getProperty(FormUtils.VALUE_PROPERTY)
             : null;
@@ -298,13 +324,14 @@ private void addInvalidSourceStatusFlag(final VersionManager versionManager, Nod
         final Set<String> checkoutPaths)
         throws RepositoryException
     {
-        if (answer.hasProperty(FormUtils.STATUS_FLAGS)) {
-            List<String> statusValues = Arrays.stream(answer.getProperty(FormUtils.STATUS_FLAGS).getValues())
+        if (answer.hasProperty(FormUtils.STATUS_FLAGS_PROPERTY)) {
+            List<String> statusValues = Arrays.stream(answer.getProperty(FormUtils.STATUS_FLAGS_PROPERTY).getValues())
                 .map(v -> v.toString()).collect(Collectors.toList());
             if (!statusValues.contains(ReferenceConditionUtils.INVALID_SOURCE_FLAG)) {
                 checkoutFormIfNeeded(versionManager, answer, checkoutPaths);
                 statusValues.add(ReferenceConditionUtils.INVALID_SOURCE_FLAG);
-                answer.setProperty(FormUtils.STATUS_FLAGS, statusValues.toArray(new String[statusValues.size()]));
+                answer.setProperty(FormUtils.STATUS_FLAGS_PROPERTY,
+                    statusValues.toArray(new String[statusValues.size()]));
             }
         }
     }
@@ -313,13 +340,13 @@ private void removeInvalidSourceStatusFlag(final VersionManager versionManager,
         final Set<String> checkoutPaths)
         throws RepositoryException
     {
-        if (answer.hasProperty(FormUtils.STATUS_FLAGS)) {
-            Value[] statusValues = answer.getProperty(FormUtils.STATUS_FLAGS).getValues();
+        if (answer.hasProperty(FormUtils.STATUS_FLAGS_PROPERTY)) {
+            Value[] statusValues = answer.getProperty(FormUtils.STATUS_FLAGS_PROPERTY).getValues();
             Value[] filteredValues = Arrays.stream(statusValues)
                 .filter(v -> !ReferenceConditionUtils.INVALID_SOURCE_FLAG.equals(v.toString())).toArray(Value[]::new);
             if (statusValues.length != filteredValues.length) {
                 checkoutFormIfNeeded(versionManager, answer, checkoutPaths);
-                answer.setProperty(FormUtils.STATUS_FLAGS, filteredValues);
+                answer.setProperty(FormUtils.STATUS_FLAGS_PROPERTY, filteredValues);
             }
         }
     }
@@ -356,14 +383,15 @@ private boolean isSame(final Property property, final Node answerNode) throws Re
         final Property nodeValue =
             !answerNode.hasProperty(FormUtils.VALUE_PROPERTY) ? null : answerNode.getProperty(FormUtils.VALUE_PROPERTY);
 
-        if (isNullStatusSame(property, nodeValue)) {
-            return true;
-        } else if (property == null || nodeValue == null) {
-            return false;
+        if (property != null && nodeValue != null) {
+            // Both values are not null: check if the values are the same
+            Set<String> propertyValues = propertyToStrings(property);
+            Set<String> nodeValues = propertyToStrings(nodeValue);
+            return isSame(propertyValues, nodeValues);
+        } else {
+            // Same if both are null, otherwise not the same
+            return property == null && nodeValue == null;
         }
-        Set<String> propertyValues = propertyToStrings(property);
-        Set<String> nodeValues = propertyToStrings(nodeValue);
-        return isSame(propertyValues, nodeValues);
     }
 
     /**
@@ -377,16 +405,15 @@ private boolean isSame(final Property property, final Node answerNode) throws Re
     private boolean isSame(final Property property, final Value value)
         throws RepositoryException
     {
-        if (isNullStatusSame(property, value)) {
-            return true;
-        } else if (property == null || value == null) {
-            return false;
+        if (property != null && value != null) {
+            Set<String> propertyStrings = propertyToStrings(property);
+            Set<String> valueStrings = new HashSet<>();
+            valueStrings.add(value.getString());
+            return isSame(propertyStrings, valueStrings);
+        } else {
+            return property == null && value == null;
         }
 
-        Set<String> propertyStrings = propertyToStrings(property);
-        Set<String> valueStrings = new HashSet<>();
-        valueStrings.add(value.getString());
-        return isSame(propertyStrings, valueStrings);
     }
 
     /**
@@ -400,18 +427,17 @@ private boolean isSame(final Property property, final Value value)
     private boolean isSame(final Property property, final Value[] values)
         throws RepositoryException
     {
-        if (isNullStatusSame(property, values)) {
-            return true;
-        } else if (property == null || values == null) {
-            return false;
+        if (property != null && values != null) {
+            Set<String> propertyStrings = propertyToStrings(property);
+            Set<String> valueStrings = new HashSet<>();
+            for (Value v : values) {
+                valueStrings.add(v.getString());
+            }
+            return isSame(propertyStrings, valueStrings);
+        } else {
+            return property == null && values == null;
         }
 
-        Set<String> propertyStrings = propertyToStrings(property);
-        Set<String> valueStrings = new HashSet<>();
-        for (Value v : values) {
-            valueStrings.add(v.getString());
-        }
-        return isSame(propertyStrings, valueStrings);
     }
 
     /**
@@ -425,17 +451,6 @@ private boolean isSame(Set<String> left, Set<String> right)
         return left.equals(right);
     }
 
-    /**
-     * Check if two objects are either both null or both not null.
-     * @param left An object to check
-     * @param right An object to check
-     * @return True if both object are null or both are not null
-     */
-    private boolean isNullStatusSame(Object left, Object right)
-    {
-        return (left == null && right == null) || (left != null && right != null);
-    }
-
     /**
      * Etract the values of  property into a set of strings.
      * @param property The values to extract the values from

modules/data-model/forms/impl/src/main/java/io/uhndata/cards/forms/internal/ReferenceAnswersEditor.java

@@ -178,8 +178,8 @@ private void setAnswer(NodeBuilder answer, ReferenceAnswer result, Type<?> resul
 
     private void setInvalidSourceStatusFlag(NodeBuilder answer)
     {
-        if (answer.hasProperty(FormUtils.STATUS_FLAGS)) {
-            Iterable<String> statusFlags = answer.getProperty(FormUtils.STATUS_FLAGS).getValue(Type.STRINGS);
+        if (answer.hasProperty(FormUtils.STATUS_FLAGS_PROPERTY)) {
+            Iterable<String> statusFlags = answer.getProperty(FormUtils.STATUS_FLAGS_PROPERTY).getValue(Type.STRINGS);
             final boolean[] containsInvalidSourceFlag = {false};
             statusFlags.forEach(s -> {
                 if (ReferenceConditionUtils.INVALID_SOURCE_FLAG.equals(s)) {
@@ -191,10 +191,10 @@ private void setInvalidSourceStatusFlag(NodeBuilder answer)
                 ArrayList<String> newFlags = new ArrayList<>();
                 statusFlags.forEach(s -> newFlags.add(s));
                 newFlags.add(ReferenceConditionUtils.INVALID_SOURCE_FLAG);
-                answer.setProperty(FormUtils.STATUS_FLAGS, newFlags, Type.STRINGS);
+                answer.setProperty(FormUtils.STATUS_FLAGS_PROPERTY, newFlags, Type.STRINGS);
             }
         } else {
-            answer.setProperty(FormUtils.STATUS_FLAGS,
+            answer.setProperty(FormUtils.STATUS_FLAGS_PROPERTY,
                 new String[]{ReferenceConditionUtils.INVALID_SOURCE_FLAG});
         }
     }

modules/data-model/forms/impl/src/main/java/io/uhndata/cards/forms/internal/ReferenceConditionUtils.java

@@ -31,8 +31,8 @@
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 
+import io.uhndata.cards.conditions.ConditionalUtils;
 import io.uhndata.cards.forms.api.FormUtils;
-import io.uhndata.cards.utils.ConditionalUtils;
 
 /**
  * A set of utility functions to help evaluate and apply conditional reference questions.

modules/form-completion-status/pom.xml

@@ -81,8 +81,8 @@
       <version>${project.version}</version>
     </dependency>
     <dependency>
-      <groupId>io.uhndata.cards</groupId>
-      <artifactId>cards-utils</artifactId>
+      <groupId>${project.groupId}</groupId>
+      <artifactId>cards-conditions</artifactId>
       <version>${project.version}</version>
     </dependency>
   </dependencies>