Merge pull request #434 from wttech/refactor-empty-item-handling

refactor empty list and empty structure handling

Author: dprzybyl

app/aem/core/src/main/java/com/cognifide/apm/core/grammar/ApmType.java

@@ -34,10 +34,6 @@ public Object getArgument(DecryptionService decryptionService) {
     return null;
   }
 
-  protected String toPrettyString(int depth, int prefixDepth) {
-    return null;
-  }
-
   public Integer getInteger() {
     return null;
   }
@@ -54,6 +50,14 @@ public Map<String, ApmType> getMap() {
     return null;
   }
 
+  public boolean isEmpty() {
+    return false;
+  }
+
+  protected String toPrettyString(int depth, int prefixDepth) {
+    return null;
+  }
+
   @Override
   public String toString() {
     return toPrettyString(0, 0);
@@ -149,6 +153,11 @@ public List<ApmType> getList() {
       return values;
     }
 
+    @Override
+    public boolean isEmpty() {
+      return values.isEmpty();
+    }
+
     @Override
     protected String toPrettyString(int depth, int prefixDepth) {
       boolean simpleList = values.stream()
@@ -196,6 +205,11 @@ public Map<String, ApmType> getMap() {
       return values;
     }
 
+    @Override
+    public boolean isEmpty() {
+      return values.isEmpty();
+    }
+
     @Override
     protected String toPrettyString(int depth, int prefixDepth) {
       ApmType firstEntry = values.values()
@@ -254,6 +268,11 @@ public ApmType getValue() {
       return value;
     }
 
+    @Override
+    public boolean isEmpty() {
+      return value.isEmpty();
+    }
+
     @Override
     protected String toPrettyString(int depth, int prefixDepth) {
       return StringUtils.repeat('\t', depth) + key + ": " + value.toPrettyString(depth, 0);
@@ -262,5 +281,9 @@ protected String toPrettyString(int depth, int prefixDepth) {
 
   public static class ApmEmpty extends ApmType {
 
+    @Override
+    public boolean isEmpty() {
+      return true;
+    }
   }
 }

app/aem/core/src/main/java/com/cognifide/apm/core/grammar/ScriptRunner.java

@@ -23,9 +23,8 @@
 import com.cognifide.apm.api.scripts.Script;
 import com.cognifide.apm.api.services.ScriptFinder;
 import com.cognifide.apm.api.status.Status;
-import com.cognifide.apm.core.grammar.ApmType.ApmEmpty;
 import com.cognifide.apm.core.grammar.ApmType.ApmList;
-import com.cognifide.apm.core.grammar.ApmType.ApmMap;
+import com.cognifide.apm.core.grammar.ApmType.ApmPair;
 import com.cognifide.apm.core.grammar.ApmType.ApmString;
 import com.cognifide.apm.core.grammar.antlr.ApmLangBaseVisitor;
 import com.cognifide.apm.core.grammar.antlr.ApmLangParser.AllowDenyCommandContext;
@@ -49,7 +48,6 @@
 import com.cognifide.apm.core.logger.Position;
 import com.cognifide.apm.core.logger.Progress;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -148,25 +146,25 @@ public Status visitRequireVariable(RequireVariableContext ctx) {
 
     @Override
     public Status visitForEach(ForEachContext ctx) {
-      List<Map<String, ApmType>> values = readValues(ctx);
-      ListIterator<Map<String, ApmType>> iterator = values.listIterator();
+      List<ApmPair> values = readValues(ctx);
+      ListIterator<ApmPair> iterator = values.listIterator();
       if (!iterator.hasNext() && shouldVisitNextChild()) {
         String key = ctx.IDENTIFIER().toString();
         progress(ctx, Status.SKIPPED, "for-each", String.format("%s is always empty", key));
       }
       while (iterator.hasNext() && shouldVisitNextChild()) {
         try {
           int index = iterator.nextIndex();
-          Map<String, ApmType> value = iterator.next();
-          executionContext.createLocalContext();
-          String valueStr = value.entrySet()
-              .stream()
-              .map(entry -> String.format("%s=%s", entry.getKey(), entry.getValue()))
-              .collect(Collectors.joining("\n"));
-          progress(ctx, Status.SUCCESS, "for-each", String.format("%d. Begin: %s", index, valueStr));
-          value.forEach(executionContext::setVariable);
-          visit(ctx.body());
-          progress(ctx, Status.SUCCESS, "for-each", String.format("%d. End", index));
+          ApmPair value = iterator.next();
+          if (value.isEmpty()) {
+            progress(ctx, Status.SKIPPED, "for-each", String.format("%d. %s is empty", index, value.getKey()));
+          } else {
+            executionContext.createLocalContext();
+            progress(ctx, Status.SUCCESS, "for-each", String.format("%d. Begin: %s=%s", index, value.getKey(), value.getValue()));
+            executionContext.setVariable(value.getKey(), value.getValue());
+            visit(ctx.body());
+            progress(ctx, Status.SUCCESS, "for-each", String.format("%d. End", index));
+          }
         } finally {
           executionContext.removeLocalContext();
         }
@@ -278,21 +276,19 @@ public Status visitImportScript(ImportScriptContext ctx) {
       return Status.SUCCESS;
     }
 
-    private List<Map<String, ApmType>> readValues(ForEachContext ctx) {
+    private List<ApmPair> readValues(ForEachContext ctx) {
       String key = ctx.IDENTIFIER().toString();
       ApmType variableValue = executionContext.resolveArgument(ctx.argument());
       List<ApmType> values;
-      if (variableValue instanceof ApmList) {
-        values = variableValue.getList();
-      } else if (variableValue instanceof ApmEmpty) {
-        values = Collections.emptyList();
-      } else if (variableValue instanceof ApmMap && variableValue.getMap().isEmpty()) {
+      if (variableValue.isEmpty()) {
         values = Collections.emptyList();
+      } else if (variableValue instanceof ApmList) {
+        values = variableValue.getList();
       } else {
         values = ImmutableList.of(variableValue);
       }
       return values.stream()
-          .map(value -> ImmutableMap.of(key, value))
+          .map(value -> new ApmPair(key, value))
           .collect(Collectors.toList());
     }