Separate Fix expression interpretation from record data transformation.

Everything that is data-independent should be done only once at initialization time. IOW, perform as little work as possible per record.

Author: blackwinter

metafix/src/main/java/org/metafacture/metafix/RecordTransformer.java

@@ -39,9 +39,9 @@
 import org.slf4j.LoggerFactory;
 
 import java.util.LinkedHashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -57,126 +57,138 @@ public class RecordTransformer { // checkstyle-disable-line ClassFanOutComplexit
 
     private static final Logger LOG = LoggerFactory.getLogger(RecordTransformer.class);
 
-    private final List<Expression> expressions;
+    private final List<Consumer<Record>> consumers = new LinkedList<>();
     private final Map<String, String> vars;
     private final Metafix metafix;
 
-    private Record record;
+    private Supplier<String> currentMessageSupplier;
 
     /*package-private*/ RecordTransformer(final Metafix metafix, final Fix fix) {
         this(metafix, fix.getElements());
     }
 
     private RecordTransformer(final Metafix metafix, final List<Expression> expressions) {
-        this.expressions = expressions;
         this.metafix = metafix;
         vars = metafix.getVars();
-    }
-
-    public void transform(final Record currentRecord) {
-        this.record = currentRecord;
-        process(expressions);
-    }
-
-    private void process(final List<Expression> currentExpressions) {
-        currentExpressions.forEach(e -> {
-            final List<String> params = resolveParams(e.getParams());
 
+        expressions.forEach(e -> {
             if (e instanceof Do) {
-                processDo((Do) e, params);
+                processDo((Do) e);
             }
             else if (e instanceof If) {
-                processIf((If) e, params);
+                processIf((If) e);
             }
             else if (e instanceof Unless) {
-                processUnless((Unless) e, params);
+                processUnless((Unless) e);
             }
             else if (e instanceof MethodCall) {
-                processFunction((MethodCall) e, params);
+                processFunction((MethodCall) e);
             }
             else {
                 throw new FixProcessException(executionExceptionMessage(e));
             }
         });
     }
 
-    private void processDo(final Do expression, final List<String> params) {
-        processExpression(expression, name -> {
-            final FixContext context = getInstance(name, FixContext.class, FixBind::valueOf);
-            context.execute(metafix, record, params, options(expression.getOptions()), new RecordTransformer(metafix, expression.getElements()));
-        });
+    public void transform(final Record record) {
+        consumers.forEach(consumer -> {
+            final FixExecutionException exception = tryRun(() -> consumer.accept(record));
 
-        // TODO, possibly: use morph collectors here
-        // final CollectFactory collectFactory = new CollectFactory();
-        // final Map<String, String> attributes = resolvedAttributeMap(params, expression.getOptions());
-        // final Collect collect = collectFactory.newInstance(expression.getName(), attributes);
+            if (exception != null) {
+                metafix.getStrictness().handle(exception, record);
+            }
+        });
     }
 
-    private void processIf(final If expression, final List<String> params) {
-        final ElsIf elseIfExpression = expression.getElseIf();
-        final Else elseExpression = expression.getElse();
+    private void processDo(final Do expression) {
+        processFix(() -> executionExceptionMessage(expression), () -> {
+            final FixContext context = getInstance(expression.getName(), FixContext.class, FixBind::valueOf);
+            final RecordTransformer recordTransformer = new RecordTransformer(metafix, expression.getElements());
 
-        if (testConditional(expression, expression.eResource(), expression.getName(), params)) {
-            process(expression.getElements());
-        }
-        else if (elseIfExpression != null && testConditional(elseIfExpression,
-                    elseIfExpression.eResource(), elseIfExpression.getName(), resolveParams(elseIfExpression.getParams()))) {
-            process(elseIfExpression.getElements());
-        }
-        else if (elseExpression != null) {
-            process(elseExpression.getElements());
-        }
+            return record -> context.execute(metafix, record, params(expression.getParams()), options(expression.getOptions()), recordTransformer);
+        });
     }
 
-    private void processUnless(final Unless expression, final List<String> params) {
-        if (!testConditional(expression, expression.eResource(), expression.getName(), params)) {
-            process(expression.getElements());
-        }
+    private void processIf(final If ifExpression) {
+        final ElsIf elseIfExpression = ifExpression.getElseIf();
+        final Else elseExpression = ifExpression.getElse();
+
+        final Supplier<String> elseIfMessageSupplier = () -> executionExceptionMessage(elseIfExpression, elseIfExpression.eResource());
+        final Supplier<String> elseMessageSupplier = () -> executionExceptionMessage(elseExpression, elseExpression.eResource());
+
+        processFix(() -> executionExceptionMessage(ifExpression, ifExpression.eResource()), () -> {
+            final FixPredicate ifPredicate = getInstance(ifExpression.getName(), FixPredicate.class, FixConditional::valueOf);
+            final FixPredicate elseIfPredicate = elseIfExpression != null ? getInstance(elseIfExpression.getName(), FixPredicate.class, FixConditional::valueOf) : null;
+
+            final RecordTransformer ifTransformer = new RecordTransformer(metafix, ifExpression.getElements());
+            final RecordTransformer elseIfTransformer = elseIfExpression != null ? new RecordTransformer(metafix, elseIfExpression.getElements()) : null;
+            final RecordTransformer elseTransformer = elseExpression != null ? new RecordTransformer(metafix, elseExpression.getElements()) : null;
+
+            return record -> {
+                if (ifPredicate.test(metafix, record, params(ifExpression.getParams()), options(null))) { // TODO: options
+                    ifTransformer.transform(record);
+                }
+                else {
+                    if (elseIfExpression != null) {
+                        currentMessageSupplier = elseIfMessageSupplier;
+
+                        if (elseIfPredicate.test(metafix, record, params(elseIfExpression.getParams()), options(null))) { // TODO: options
+                            elseIfTransformer.transform(record);
+                            return;
+                        }
+                    }
+
+                    if (elseExpression != null) {
+                        currentMessageSupplier = elseMessageSupplier;
+                        elseTransformer.transform(record);
+                    }
+                }
+            };
+        });
     }
 
-    private boolean testConditional(final EObject object, final Resource resource, final String conditional, final List<String> params) {
-        LOG.debug("<IF>: {} parameters: {}", conditional, params);
-
-        final AtomicBoolean bool = new AtomicBoolean();
+    private void processUnless(final Unless expression) {
+        processFix(() -> executionExceptionMessage(expression, expression.eResource()), () -> {
+            final FixPredicate predicate = getInstance(expression.getName(), FixPredicate.class, FixConditional::valueOf);
+            final RecordTransformer recordTransformer = new RecordTransformer(metafix, expression.getElements());
 
-        processFix(() -> executionExceptionMessage(object, resource), () -> {
-            final FixPredicate predicate = getInstance(conditional, FixPredicate.class, FixConditional::valueOf);
-            bool.set(predicate.test(metafix, record, params, options(null))); // TODO: options
+            return record -> {
+                if (!predicate.test(metafix, record, params(expression.getParams()), options(null))) { // TODO: options
+                    recordTransformer.transform(record);
+                }
+            };
         });
-
-        return bool.get();
-
-        // TODO, possibly: use morph functions here (& in processFunction):
-        // final FunctionFactory functionFactory = new FunctionFactory();
-        // functionFactory.registerClass("not_equals", NotEquals.class);
-        // functionFactory.registerClass("replace_all", Replace.class);
-        // final Function function = functionFactory.newInstance(conditional,
-        // resolvedAttributeMap(params, theIf.getOptions()));
     }
 
-    private void processFunction(final MethodCall expression, final List<String> params) {
-        processExpression(expression, name -> {
-            final FixFunction function = getInstance(name, FixFunction.class, FixMethod::valueOf);
-            function.apply(metafix, record, params, options(expression.getOptions()));
+    private void processFunction(final MethodCall expression) {
+        processFix(() -> executionExceptionMessage(expression), () -> {
+            final FixFunction function = getInstance(expression.getName(), FixFunction.class, FixMethod::valueOf);
+            return record -> function.apply(metafix, record, params(expression.getParams()), options(expression.getOptions()));
         });
     }
 
     private <T> T getInstance(final String name, final Class<T> baseType, final Function<String, ? extends T> enumFunction) {
         return name.contains(".") ? ReflectionUtil.loadClass(name, baseType).newInstance() : enumFunction.apply(name);
     }
 
-    private void processExpression(final Expression expression, final Consumer<String> consumer) {
-        processFix(() -> executionExceptionMessage(expression), () -> consumer.accept(expression.getName()));
-    }
+    private void processFix(final Supplier<String> messageSupplier, final Supplier<Consumer<Record>> consumerSupplier) {
+        currentMessageSupplier = messageSupplier;
+
+        final FixExecutionException exception = tryRun(() -> {
+            final Consumer<Record> consumer = consumerSupplier.get();
+
+            consumers.add(record -> {
+                currentMessageSupplier = messageSupplier;
+                consumer.accept(record);
+            });
+        });
 
-    private void processFix(final Supplier<String> messageSupplier, final Runnable runnable) {
-        final FixExecutionException exception = tryRun(messageSupplier, runnable);
         if (exception != null) {
-            metafix.getStrictness().handle(exception, record);
+            throw exception;
         }
     }
 
-    private FixExecutionException tryRun(final Supplier<String> messageSupplier, final Runnable runnable) { // checkstyle-disable-line ReturnCount
+    private FixExecutionException tryRun(final Runnable runnable) { // checkstyle-disable-line ReturnCount
         try {
             runnable.run();
         }
@@ -187,11 +199,12 @@ private FixExecutionException tryRun(final Supplier<String> messageSupplier, fin
             return e; // TODO: Add nesting information?
         }
         catch (final IllegalStateException | NumberFormatException e) {
-            return new FixExecutionException(messageSupplier.get(), e);
+            return new FixExecutionException(currentMessageSupplier.get(), e);
         }
         catch (final RuntimeException e) { // checkstyle-disable-line IllegalCatch
-            throw new FixProcessException(messageSupplier.get(), e);
+            throw new FixProcessException(currentMessageSupplier.get(), e);
         }
+
         return null;
     }
 
@@ -206,14 +219,14 @@ private String executionExceptionMessage(final EObject object, final Resource re
                 resource.getURI(), node.getStartLine(), NodeModelUtils.getTokenText(node));
     }
 
-    private List<String> resolveParams(final List<String> params) {
-        return params.stream().map(this::resolveVars).collect(Collectors.toList());
-    }
-
     private String resolveVars(final String value) {
         return value == null ? null : StringUtil.format(value, Metafix.VAR_START, Metafix.VAR_END, false, vars);
     }
 
+    private List<String> params(final List<String> params) {
+        return params.stream().map(this::resolveVars).collect(Collectors.toList());
+    }
+
     private Map<String, String> options(final Options options) {
         final Map<String, String> map = new LinkedHashMap<>();
 

metafix/src/test/java/org/metafacture/metafix/MetafixIfTest.java

@@ -1012,4 +1012,61 @@ public void shouldApplyCustomJavaPredicate() {
         );
     }
 
+    @Test
+    public void shouldIncludeLocationAndTextInProcessExceptionInConditional() {
+        final String text1 = "elsif exists()";
+        final String text2 = "nothing()";
+        final String message = "Error while executing Fix expression (at FILE, line 3): " + text1 + " " + text2;
+
+        MetafixTestHelpers.assertThrows(FixProcessException.class, s -> s.replaceAll("file:/.+?\\.fix", "FILE"), message, () ->
+            MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
+                    "if exists('animal')",
+                    "nothing()",
+                    text1,
+                    text2,
+                    "end"
+                ),
+                i -> {
+                    i.startRecord("1");
+                    i.startEntity("animals");
+                    i.literal("1", "dog");
+                    i.literal("2", "cat");
+                    i.literal("3", "zebra");
+                    i.endEntity();
+                    i.endRecord();
+                },
+                o -> {
+                }
+            )
+        );
+    }
+
+    @Test
+    public void shouldIncludeLocationAndTextInProcessExceptionInBody() {
+        final String text = "add_field()";
+        final String message = "Error while executing Fix expression (at FILE, line 4): " + text;
+
+        MetafixTestHelpers.assertThrows(FixProcessException.class, s -> s.replaceAll("file:/.+?\\.fix", "FILE"), message, () ->
+            MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
+                    "if exists('animal')",
+                    "nothing()",
+                    "elsif exists('animals')",
+                    text,
+                    "end"
+                ),
+                i -> {
+                    i.startRecord("1");
+                    i.startEntity("animals");
+                    i.literal("1", "dog");
+                    i.literal("2", "cat");
+                    i.literal("3", "zebra");
+                    i.endEntity();
+                    i.endRecord();
+                },
+                o -> {
+                }
+            )
+        );
+    }
+
 }

metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/expected.err

@@ -1,2 +1,3 @@
-^Exception in thread "main" org\.metafacture\.metafix\.FixProcessException: Error while executing Fix expression \(at file:.*/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/test\.fix, line 2\): foo\(\)$
+^Exception in thread "main" org\.metafacture\.commons\.reflection\.ReflectionException: class could not be instantiated: class org\.metafacture\.metafix\.Metafix$
+^Caused by: org\.metafacture\.metafix\.FixProcessException: Error while executing Fix expression \(at file:.*/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/test\.fix, line 2\): foo\(\)$
 ^Caused by: java\.lang\.IllegalArgumentException: No enum constant org\.metafacture\.metafix\.FixMethod.foo$