Merge pull request #1 from opensrp/bugfixes_enhancements

Bug Fixes and Enhancements - Version 2.1.0 release
onaio · Apr 26, 2023 · c46912b · c46912b
2 parents 82e4547 + ff52bc6
commit c46912b
Show file tree

Hide file tree

Showing 7 changed files with 79 additions and 32 deletions.
diff --git a/README.md b/README.md
@@ -6,11 +6,11 @@ A command line utility to support FHIRCore content authoring. This tool supports
 
 Download the latest release from https://github.com/opensrp/fhircore-tooling/releases
 
-To run it as a java jar by using the command `java -jar efsity-2.0.0.jar -h` . This is the help command and will list the available options.
+To run it as a java jar by using the command `java -jar efsity-2.1.0.jar -h` . This is the help command and will list the available options.
 
 If you are using a linux environment e.g. bash you can choose to create an _alias_ for this as shown below. _(Remember to reload the terminal)_
 
-`alias fct='java -jar ~/Downloads/efsity-2.0.0.jar'`
+`alias fct='java -jar ~/Downloads/efsity-2.1.0.jar'`
 
 To run the previous help command you can then run `fct -h` in your terminal.
 

diff --git a/pom.xml b/pom.xml
@@ -4,7 +4,7 @@
 
   <groupId>org.smartregister</groupId>
   <artifactId>fhircore-tooling</artifactId>
-  <version>2.0.0</version>
+  <version>2.1.0</version>
 
   <properties>
     <code.name>efsity</code.name>

diff --git a/src/main/java/org/smartregister/Main.java b/src/main/java/org/smartregister/Main.java
@@ -11,15 +11,15 @@
 @Command(
     name = "fct",
     description = "FHIRCore tooling to make content authoring easier.",
-    version = "2.0.0",
+    version = "2.1.0",
     mixinStandardHelpOptions = true,
     subcommands = {
       ConvertCommand.class,
       StructureMapExtractResourcesCommand.class,
       ValidateCommand.class
     })
 public class Main implements Runnable {
-  public static final String VERSION = "2.0.0";
+  public static final String VERSION = "2.1.0";
 
   @CommandLine.Option(
       names = {"-v"},

diff --git a/src/main/java/org/smartregister/util/FCTUtils.java b/src/main/java/org/smartregister/util/FCTUtils.java
@@ -14,9 +14,11 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.*;
 import java.nio.file.attribute.BasicFileAttributes;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
+import org.apache.commons.io.FilenameUtils;
 import org.smartregister.domain.FCTFile;
 
 public class FCTUtils {
@@ -124,16 +126,19 @@ public static void printCompletedInDuration(long startTime) {
             FCTUtils.getHumanDuration(System.currentTimeMillis() - startTime)));
   }
 
-  public static Map<String, Map<String, String>> indexConfigurationFiles(String inputDirectoryPath)
-      throws IOException {
+  public static Map<String, Map<String, String>> indexConfigurationFiles(
+      String inputDirectoryPath, String... fileExtensions) throws IOException {
     Map<String, Map<String, String>> filesMap = new HashMap<>();
     Path rootDir = Paths.get(inputDirectoryPath);
     Files.walkFileTree(
         rootDir,
         new SimpleFileVisitor<>() {
           @Override
           public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
-            if (!Files.isDirectory(file)) {
+            if (!Files.isDirectory(file)
+                && (fileExtensions.length == 1 && fileExtensions[0] == "*"
+                    || Arrays.asList(fileExtensions)
+                        .contains(FilenameUtils.getExtension(file.getFileName().toString())))) {
 
               String parentDirKey =
                   file.getParent().equals(rootDir)

diff --git a/src/main/java/org/smartregister/util/FCTValidationEngine.java b/src/main/java/org/smartregister/util/FCTValidationEngine.java
@@ -25,12 +25,12 @@ public class FCTValidationEngine {
 
   private JSONObject currentParamDataWorkflowJSONObject;
   private JSONObject lastWorkflowJSONObject;
-  private JSONObject previousParentJSONObject;
   private JSONObject parentJSONObject;
   private String parentJSONObjectKey;
   private String currentFile;
   private int configurationFilesCount;
-  private Set<String> factMapKeys = new HashSet<>();
+
+  private Map<String, Set<String>> factMapKeysByFile = new HashMap<>();
   private Map<String, String> fileConfigTypeIdentifierToFilenameMap = new HashMap<>();
   private Map<String, Set<String>> questionnairesToLinkIds;
   private Map<String, Set<String>> structureMapToLinkIds;
@@ -47,7 +47,6 @@ private void handleValue(String key, Object value, boolean isComposition) {
     } else if (value instanceof JSONObject) {
 
       parentJSONObjectKey = key;
-      previousParentJSONObject = parentJSONObject;
       parentJSONObject = (JSONObject) value;
 
       if (parentJSONObject != null && parentJSONObject.has(Constants.workflow))
@@ -121,22 +120,24 @@ private void handleValue(String key, Object value, boolean isComposition) {
 
           if (value.toString().contains("data.put(")) {
             String ruleFactKey = StringUtils.substringBetween(value.toString(), "'", "'");
-            factMapKeys.add(ruleFactKey);
+            Set<String> factsByFile = factMapKeysByFile.getOrDefault(currentFile, new HashSet<>());
+            factsByFile.add(ruleFactKey);
+            factMapKeysByFile.put(currentFile, factsByFile);
             if (errorsMap.getOrDefault(currentFile, new HashMap<>()).containsKey(Constants.Rules))
               errorsMap.get(currentFile).get(Constants.Rules).remove(ruleFactKey);
           } else if (value.toString().contains("@{")) {
             String ruleFactKey = StringUtils.substringBetween(value.toString(), "@{", "}");
-            if (!factMapKeys.contains(ruleFactKey)) {
+            if (!factMapKeysByFile
+                .getOrDefault(currentFile, new HashSet<>())
+                .contains(ruleFactKey)) {
               addToErrorMap(Constants.Rules, ruleFactKey);
             }
           }
 
-          if (Constants.PARAMDATA.equals(value)) {
-
-            if (previousParentJSONObject != null
-                && previousParentJSONObject.has(Constants.workflow))
-              currentParamDataWorkflowJSONObject = previousParentJSONObject;
+          if (Constants.workflow.equals(key))
+            currentParamDataWorkflowJSONObject = lastWorkflowJSONObject;
 
+          if (Constants.PARAMDATA.equals(value)) {
             String configFileIdentifier =
                 currentParamDataWorkflowJSONObject.getString(Constants.ID);
 
@@ -150,8 +151,13 @@ private void handleValue(String key, Object value, boolean isComposition) {
 
             // Add to fact-map for this file
             if (currentFile.equals(
-                fileConfigTypeIdentifierToFilenameMap.getOrDefault(configFileIdentifier, null)))
-              factMapKeys.add(parentJSONObject.getString(Constants.KEY));
+                fileConfigTypeIdentifierToFilenameMap.getOrDefault(configFileIdentifier, null))) {
+
+              Set<String> factsByFile =
+                  factMapKeysByFile.getOrDefault(currentFile, new HashSet<>());
+              factsByFile.add(parentJSONObject.getString(Constants.KEY));
+              factMapKeysByFile.put(currentFile, factsByFile);
+            }
           }
 
           //
@@ -190,7 +196,8 @@ private void handleValue(String key, Object value, boolean isComposition) {
                       questionnaireId));
             }
 
-            if (structureMapToLinkIds.containsKey(structureMapId)
+            if (structureMapId != null
+                && structureMapToLinkIds.containsKey(structureMapId)
                 && !structureMapToLinkIds.get(structureMapId).contains(fieldLinkId)) {
               addToErrorMap(
                   "Prepopulate",
@@ -281,7 +288,7 @@ public void process(
 
     long startTime = System.currentTimeMillis();
     Map<String, Map<String, String>> configDirIndexMap =
-        FCTUtils.indexConfigurationFiles(directoryPath);
+        FCTUtils.indexConfigurationFiles(directoryPath, "*");
 
     if (questionnairesFolderPath != null) {
 
@@ -296,7 +303,7 @@ public void process(
       structureMapToLinkIds = new StructureMapProcessor(structureMapsFolderPath).process();
 
       FCTUtils.printNewLine();
-      FCTUtils.printInfo("\u001b[36mPRE PARSING VALIDATION\u001b[0m");
+      FCTUtils.printInfo("\u001b[36mPreparsing validation\u001b[0m");
 
       // Validate all Structure Map Link ids should be in questionnaire
       for (var entry : structureMapToLinkIds.entrySet()) {
@@ -323,11 +330,31 @@ public void process(
         } else {
           FCTUtils.printError(
               String.format(
-                  "No Questionnaire found for Structure Map with id \u001b[36m%s\u001b[0m",
+                  "No Questionnaire resource was found for the Structure Map with id \u001b[36m%s\u001b[0m",
                   structureMapId));
         }
       }
+
+      // Validate all Structure Map IDs referenced in Questionnaire extensions exist as Structure
+      // Maps(files)
+
+      for (var entry : questionnaireToStructureMapId.entrySet()) {
+
+        String questionnaireId = entry.getKey();
+        String structureMapId = entry.getValue().stream().findFirst().orElse("");
+
+        if (!structureMapId.isBlank() && !structureMapToLinkIds.containsKey(structureMapId)) {
+
+          FCTUtils.printError(
+              String.format(
+                  "No Structure Map with id \u001b[36m%s\u001b[0m was found. Defining Questionnaire has id \u001b[36m%s\u001b[0m",
+                  structureMapId, questionnaireId));
+        }
+      }
     }
+
+    FCTUtils.printInfo("\u001b[36mPreparsing validation complete\u001b[0m");
+
     // Load Composition
     currentFile = compositionPath;
     FCTFile compositionFile = FCTUtils.readFile(compositionPath);
@@ -365,9 +392,15 @@ public void process(
         }
       }
     }
-
-    FCTUtils.printToConsole(String.format("%d translation files found", translationsMap.size()));
-    FCTUtils.printToConsole(String.format("%d configuration files found", configurationFilesCount));
+    FCTUtils.printNewLine();
+    FCTUtils.printToConsole(
+        String.format(
+            "%d translation file" + (translationsMap.size() > 1 ? "s" : "") + " found",
+            translationsMap.size()));
+    FCTUtils.printToConsole(
+        String.format(
+            "%d configuration file" + (configurationFilesCount > 1 ? "s" : "") + " found",
+            configurationFilesCount));
     printValidationResults();
 
     FCTUtils.printNewLine();
@@ -387,7 +420,6 @@ private String getQuestionnaireIdByStructureMapId(String structureMapId) {
 
   private void resetStatePerFile() {
     currentFile = null;
-    factMapKeys.clear();
   }
 
   private void printValidationResults() {

diff --git a/src/main/java/org/smartregister/util/QuestionnaireProcessor.java b/src/main/java/org/smartregister/util/QuestionnaireProcessor.java
@@ -31,7 +31,7 @@ public Map<String, Map<String, Set<String>>> process() {
     try {
 
       Map<String, Map<String, String>> folderTofilesIndexMap =
-          FCTUtils.indexConfigurationFiles(directoryPath);
+          FCTUtils.indexConfigurationFiles(directoryPath, "json");
 
       // Process other configurations
       for (var entry : folderTofilesIndexMap.entrySet()) {
@@ -69,7 +69,7 @@ public Map<String, Map<String, Set<String>>> process() {
           } catch (JSONException jsonException) {
 
             FCTUtils.printError(String.format("Error processing file %s", currentFile));
-            FCTUtils.printError(String.format("Error message %s", jsonException.getMessage()));
+            printJsonExceptionMessages(jsonException.getMessage());
           }
         }
       }
@@ -82,6 +82,15 @@ public Map<String, Map<String, Set<String>>> process() {
     return resultsMap;
   }
 
+  private void printJsonExceptionMessages(String message) {
+    if (message.contains("JSONObject[\"id\"] not found")) {
+      FCTUtils.printWarning(
+          "Questionnaire DOES NOT have an id field. Are we expecting it to be generated on the Server?");
+    } else {
+      FCTUtils.printError(String.format("%s", message));
+    }
+  }
+
   private String getStructureMapId(JSONArray extensionJSONArray) {
 
     for (int i = 0; i < extensionJSONArray.length(); i++) {

diff --git a/src/main/java/org/smartregister/util/StructureMapProcessor.java b/src/main/java/org/smartregister/util/StructureMapProcessor.java
@@ -29,7 +29,7 @@ public Map<String, Set<String>> process() {
     try {
 
       Map<String, Map<String, String>> folderTofilesIndexMap =
-          FCTUtils.indexConfigurationFiles(directoryPath);
+          FCTUtils.indexConfigurationFiles(directoryPath, "map", "txt");
 
       // Process other configurations
       for (var entry : folderTofilesIndexMap.entrySet()) {
@@ -82,7 +82,8 @@ public Map<String, Set<String>> process() {
             }
           }
 
-          structureMapToLinkIds.put(getStructureMapId(firstLine), linkIds);
+          if (!firstLine.isBlank())
+            structureMapToLinkIds.put(getStructureMapId(firstLine), linkIds);
         }
       }