From 1c164e8be5c834dcae0154b5da6de3bc63893703 Mon Sep 17 00:00:00 2001 From: Jens Wille Date: Fri, 25 Mar 2022 15:22:56 +0100 Subject: [PATCH 1/3] Implement "strict" mode. (#192) Levels of strictness: - `PROCESS`: Abort process by throwing an exception. - `RECORD`: Ignore (skip) record and log an error. - `EXPRESSION`: Ignore (skip) expression and log a warning. Introduces `FixProcessException` to differentiate from `FixExecutionException`: The latter indicating potentially data-dependent issues that should be subject to strictness handling, while the former should only refer to static issues with the usage of Fix expressions. --- .../metafix/FixExecutionException.java | 6 ++ .../metafix/FixProcessException.java | 37 ++++++++ .../java/org/metafacture/metafix/Metafix.java | 58 ++++++++++++ .../metafix/RecordTransformer.java | 17 +++- .../metafix/MetafixLookupTest.java | 4 +- .../metafix/MetafixMethodTest.java | 14 +-- .../metafix/MetafixRecordTest.java | 4 +- .../metafix/MetafixScriptTest.java | 94 ++++++++++++++++++- .../metafix/MetafixTestHelpers.java | 6 +- .../expected.json | 14 +++ .../input.json | 3 + .../test.fix | 3 + .../test.flux | 8 ++ .../expected.json | 10 ++ .../input.json | 3 + .../test.fix | 3 + .../test.flux | 8 ++ 17 files changed, 274 insertions(+), 18 deletions(-) create mode 100644 metafix/src/main/java/org/metafacture/metafix/FixProcessException.java create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/expected.json create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/input.json create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/test.fix create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/test.flux create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/expected.json create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/input.json create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/test.fix create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/test.flux diff --git a/metafix/src/main/java/org/metafacture/metafix/FixExecutionException.java b/metafix/src/main/java/org/metafacture/metafix/FixExecutionException.java index 95d218e0..bb20f522 100644 --- a/metafix/src/main/java/org/metafacture/metafix/FixExecutionException.java +++ b/metafix/src/main/java/org/metafacture/metafix/FixExecutionException.java @@ -18,6 +18,12 @@ import org.metafacture.framework.MetafactureException; +/** + * Indicates dynamic (i.e., data-dependent) issues during Fix execution that + * should be subject to {@link Metafix.Strictness strictness} handling. + * + * @see FixProcessException + */ public class FixExecutionException extends MetafactureException { public FixExecutionException(final String message) { diff --git a/metafix/src/main/java/org/metafacture/metafix/FixProcessException.java b/metafix/src/main/java/org/metafacture/metafix/FixProcessException.java new file mode 100644 index 00000000..849f445c --- /dev/null +++ b/metafix/src/main/java/org/metafacture/metafix/FixProcessException.java @@ -0,0 +1,37 @@ +/* + * Copyright 2022 hbz NRW + * + * Licensed 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. + */ + +package org.metafacture.metafix; + +import org.metafacture.framework.MetafactureException; + +/** + * Indicates static (i.e., data-independent) issues with the usage of Fix + * expressions. + * + * @see FixExecutionException + */ +public class FixProcessException extends MetafactureException { + + public FixProcessException(final String message) { + super(message); + } + + public FixProcessException(final String message, final Throwable cause) { + super(message, cause); + } + +} diff --git a/metafix/src/main/java/org/metafacture/metafix/Metafix.java b/metafix/src/main/java/org/metafacture/metafix/Metafix.java index 6491522c..8dcda9da 100644 --- a/metafix/src/main/java/org/metafacture/metafix/Metafix.java +++ b/metafix/src/main/java/org/metafacture/metafix/Metafix.java @@ -45,6 +45,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.function.BiConsumer; /** * Transforms a data stream sent via the {@link StreamReceiver} interface. Use @@ -61,6 +62,8 @@ public class Metafix implements StreamPipe, Maps { // checkstyle public static final String VAR_END = "]"; public static final String VAR_START = "$["; + public static final Strictness DEFAULT_STRICTNESS = Strictness.PROCESS; + public static final Map NO_VARS = Collections.emptyMap(); private static final Logger LOG = LoggerFactory.getLogger(Metafix.class); @@ -79,6 +82,7 @@ public class Metafix implements StreamPipe, Maps { // checkstyle private List entities = new ArrayList<>(); private Record currentRecord = new Record(); private StreamReceiver outputStreamReceiver; + private Strictness strictness = DEFAULT_STRICTNESS; private String fixFile; private String recordIdentifier; private int entityCount; @@ -325,4 +329,58 @@ public String putValue(final String mapName, final String key, final String valu return maps.computeIfAbsent(mapName, k -> new HashMap<>()).put(key, value); } + public void setStrictness(final Strictness strictness) { + this.strictness = strictness != null ? strictness : DEFAULT_STRICTNESS; + } + + public Strictness getStrictness() { + return strictness; + } + + public enum Strictness { + + /** + * Aborts process by throwing an exception. + */ + PROCESS { + @Override + protected void handleInternal(final FixExecutionException exception, final Record record) { + throw exception; + } + }, + + /** + * Ignores (skips) record and logs an error. + */ + RECORD { + @Override + protected void handleInternal(final FixExecutionException exception, final Record record) { + log(exception, LOG::error); + record.setReject(true); // TODO: Skip remaining expressions? + } + }, + + /** + * Ignores (skips) expression and logs a warning. + */ + EXPRESSION { + @Override + protected void handleInternal(final FixExecutionException exception, final Record record) { + log(exception, LOG::warn); + } + }; + + public void handle(final FixExecutionException exception, final Record record) { + LOG.debug("Current record: {}", record); + handleInternal(exception, record); + } + + protected abstract void handleInternal(FixExecutionException exception, Record record); + + protected void log(final FixExecutionException exception, final BiConsumer logger) { + logger.accept(exception.getMessage(), exception.getCause()); + } + + } + } diff --git a/metafix/src/main/java/org/metafacture/metafix/RecordTransformer.java b/metafix/src/main/java/org/metafacture/metafix/RecordTransformer.java index 4909bc71..c9e613ba 100644 --- a/metafix/src/main/java/org/metafacture/metafix/RecordTransformer.java +++ b/metafix/src/main/java/org/metafacture/metafix/RecordTransformer.java @@ -98,7 +98,7 @@ else if (e instanceof MethodCall) { processFunction((MethodCall) e, params); } else { - throw new FixExecutionException(executionExceptionMessage(e)); + throw new FixProcessException(executionExceptionMessage(e)); } }); } @@ -173,15 +173,26 @@ private void processExpression(final Expression expression, final Consumer messageSupplier, final Runnable runnable) { + final FixExecutionException exception; + try { runnable.run(); + return; } - catch (final FixExecutionException e) { + catch (final FixProcessException e) { throw e; // TODO: Add nesting information? } + catch (final FixExecutionException e) { + exception = e; // TODO: Add nesting information? + } + catch (final IllegalStateException | NumberFormatException e) { + exception = new FixExecutionException(messageSupplier.get(), e); + } catch (final RuntimeException e) { // checkstyle-disable-line IllegalCatch - throw new FixExecutionException(messageSupplier.get(), e); + throw new FixProcessException(messageSupplier.get(), e); } + + metafix.getStrictness().handle(exception, record); } private String executionExceptionMessage(final Expression expression) { diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixLookupTest.java b/metafix/src/test/java/org/metafacture/metafix/MetafixLookupTest.java index c50c37f7..de1dc1ff 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixLookupTest.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixLookupTest.java @@ -159,7 +159,7 @@ public void shouldLookupInSeparateExternalFileMap() { public void shouldNotLookupInRelativeExternalFileMapFromInlineScript() { final String mapFile = "../maps/test.csv"; - MetafixTestHelpers.assertThrowsCause(IllegalArgumentException.class, "Cannot resolve relative path: " + mapFile, () -> + MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "Cannot resolve relative path: " + mapFile, () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( LOOKUP + " '" + mapFile + "')" ), @@ -299,7 +299,7 @@ public void shouldNotLookupInUnknownInternalMap() { @Test public void shouldFailLookupInUnknownExternalMap() { - MetafixTestHelpers.assertThrowsCause(MorphExecutionException.class, "File not found: testMap.csv", () -> + MetafixTestHelpers.assertProcessException(MorphExecutionException.class, "File not found: testMap.csv", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( LOOKUP + " 'testMap.csv')" ), diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java b/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java index b7af1676..6174b4c4 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java @@ -160,7 +160,7 @@ public void shouldCapitalizeStringsInArray() { @Test public void shouldNotCapitalizeArray() { - MetafixTestHelpers.assertThrowsCause(IllegalStateException.class, "Expected String, got Array", () -> + MetafixTestHelpers.assertExecutionException(IllegalStateException.class, "Expected String, got Array", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "capitalize('title')" ), @@ -431,7 +431,7 @@ public void parseTextMixedGroups() { @Test public void parseTextEscapedGroups() { - MetafixTestHelpers.assertThrowsCause(IllegalArgumentException.class, "No group with name ", () -> + MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "No group with name ", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "parse_text(data, '(?.)(.)\\\\(?.\\\\)')" ), @@ -448,7 +448,7 @@ public void parseTextEscapedGroups() { @Test public void parseTextQuotedGroups() { - MetafixTestHelpers.assertThrowsCause(IllegalArgumentException.class, "No group with name ", () -> + MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "No group with name ", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "parse_text(data, '(?.)(.)\\\\Q(?.)\\\\E')" ), @@ -669,7 +669,7 @@ public void shouldAppendValueInEntireArray() { @Test // See https://github.com/metafacture/metafacture-fix/issues/100 public void shouldNotAppendValueToArray() { - MetafixTestHelpers.assertThrowsCause(IllegalStateException.class, "Expected String, got Array", () -> + MetafixTestHelpers.assertExecutionException(IllegalStateException.class, "Expected String, got Array", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "append('animals[]', ' is cool')" ), @@ -691,7 +691,7 @@ public void shouldNotAppendValueToArray() { @Test // See https://github.com/metafacture/metafacture-fix/issues/100 public void shouldNotAppendValueToHash() { - MetafixTestHelpers.assertThrowsCause(IllegalStateException.class, "Expected String, got Hash", () -> + MetafixTestHelpers.assertExecutionException(IllegalStateException.class, "Expected String, got Hash", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "append('animals', ' is cool')" ), @@ -1126,7 +1126,7 @@ public void shouldPrependValueInArraySubField() { @Test // See https://github.com/metafacture/metafacture-fix/issues/100 public void shouldNotPrependValueToArray() { - MetafixTestHelpers.assertThrowsCause(IllegalStateException.class, "Expected String, got Array", () -> + MetafixTestHelpers.assertExecutionException(IllegalStateException.class, "Expected String, got Array", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "prepend('animals[]', 'cool ')" ), @@ -1592,7 +1592,7 @@ public void shouldSortFieldNumerically() { @Test public void shouldFailToSortNumericallyWithInvalidNumber() { - MetafixTestHelpers.assertThrowsCause(NumberFormatException.class, "For input string: \"x\"", () -> + MetafixTestHelpers.assertExecutionException(NumberFormatException.class, "For input string: \"x\"", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "sort_field(numbers, numeric: 'true')" ), diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java b/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java index 7a313f6d..46f4ed0a 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java @@ -1083,7 +1083,7 @@ public void addFieldToObjectByIndexMissing() { } private void assertThrowsOnEmptyRecord(final String index) { - MetafixTestHelpers.assertThrowsCause(IllegalArgumentException.class, "Using ref, but can't find: " + index + " in: {}", () -> { + MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "Using ref, but can't find: " + index + " in: {}", () -> { MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "add_field('animals[]." + index + ".kind','nice')" ), @@ -2150,7 +2150,7 @@ public void accessArrayByIndex() { @Test public void shouldNotAccessArrayImplicitly() { - MetafixTestHelpers.assertThrowsCause(IllegalStateException.class, "Expected String, got Array", () -> + MetafixTestHelpers.assertExecutionException(IllegalStateException.class, "Expected String, got Array", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "upcase('name')" ), diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixScriptTest.java b/metafix/src/test/java/org/metafacture/metafix/MetafixScriptTest.java index c1ac1bcb..c257568b 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixScriptTest.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixScriptTest.java @@ -30,6 +30,7 @@ import java.util.Arrays; import java.util.Map; import java.util.function.Consumer; +import java.util.function.Supplier; import java.util.regex.Pattern; /** @@ -107,7 +108,7 @@ public void shouldResolveVariablesInOptionsFromPreviousMap() { @Test public void shouldNotResolveVariablesInOptionsFromCurrentMap() { - MetafixTestHelpers.assertThrowsCause(IllegalArgumentException.class, "Variable 'varName' was not assigned!\nAssigned variables:\n{var=1}", () -> + MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "Variable 'varName' was not assigned!\nAssigned variables:\n{var=1}", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "put_vars(varName: 'value$[var]', '$[varName]Var': 'value2')" ), @@ -149,7 +150,7 @@ public void shouldPutExternalFileMap() { public void shouldNotPutRelativeExternalFileMapFromInlineScript() { final String mapFile = "../maps/test.csv"; - MetafixTestHelpers.assertThrowsCause(IllegalArgumentException.class, "Cannot resolve relative path: " + mapFile, () -> + MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "Cannot resolve relative path: " + mapFile, () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "put_filemap('" + mapFile + "')" ), @@ -285,7 +286,7 @@ public void shouldIncludeFixFileInBind() { @Test public void shouldNotIncludeRelativeFixFileFromInlineScript() { - MetafixTestHelpers.assertThrowsCause(IllegalArgumentException.class, "Cannot resolve relative path: ./base.fix", () -> + MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "Cannot resolve relative path: ./base.fix", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "include('src/test/resources/org/metafacture/metafix/fixes/include.fix')" ), @@ -389,6 +390,93 @@ public void shouldIncludeLocationAndTextInExecutionException() { ); } + private void assertStrictness(final Metafix.Strictness strictness, final String fixDef, final Consumer> out) { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "add_field('before', '')", + fixDef, + "add_field('after', '')" + ), + i -> { + i.setStrictness(strictness); + + i.startRecord("1"); + i.literal("data", "foo"); + i.endRecord(); + + i.startRecord("2"); + i.literal("data", "foo"); + i.literal("data", "bar"); + i.endRecord(); + + i.startRecord("3"); + i.literal("data", "bar"); + i.endRecord(); + }, + out + ); + + // TODO: Test logging statements + } + + private void assertStrictness(final Metafix.Strictness strictness, final Consumer> out) { + assertStrictness(strictness, "upcase('data')", out); + } + + @Test + public void shouldSkipExpressionOnExecutionException() { + assertStrictness(Metafix.Strictness.EXPRESSION, o -> { + o.get().startRecord("1"); + o.get().literal("before", ""); + o.get().literal("data", "FOO"); + o.get().literal("after", ""); + o.get().endRecord(); + + o.get().startRecord("2"); + o.get().literal("before", ""); + o.get().literal("after", ""); + o.get().endRecord(); + + o.get().startRecord("3"); + o.get().literal("before", ""); + o.get().literal("data", "BAR"); + o.get().literal("after", ""); + o.get().endRecord(); + }); + } + + @Test + public void shouldSkipRecordOnExecutionException() { + assertStrictness(Metafix.Strictness.RECORD, o -> { + o.get().startRecord("1"); + o.get().literal("before", ""); + o.get().literal("data", "FOO"); + o.get().literal("after", ""); + o.get().endRecord(); + + o.get().startRecord("3"); + o.get().literal("before", ""); + o.get().literal("data", "BAR"); + o.get().literal("after", ""); + o.get().endRecord(); + }); + } + + @Test + public void shouldAbortProcessOnExecutionException() { + MetafixTestHelpers.assertExecutionException(IllegalStateException.class, "Expected String, got Array", () -> + assertStrictness(Metafix.Strictness.PROCESS, o -> { + }) + ); + } + + @Test + public void shouldAbortProcessOnProcessException() { + MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "No enum constant org.metafacture.metafix.FixMethod.foo", () -> + assertStrictness(Metafix.Strictness.EXPRESSION, "foo()", o -> { + }) + ); + } + private void assertVar(final String fixDef, final Map vars, final Map result) { assertFix(fixDef, vars, f -> result.forEach((k, v) -> Assertions.assertEquals(v, f.getVars().get(k)))); } diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixTestHelpers.java b/metafix/src/test/java/org/metafacture/metafix/MetafixTestHelpers.java index 20e4fd3a..773df96c 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixTestHelpers.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixTestHelpers.java @@ -45,10 +45,14 @@ public final class MetafixTestHelpers { private MetafixTestHelpers() { } - public static void assertThrowsCause(final Class expectedClass, final String expectedMessage, final Executable executable) { + public static void assertExecutionException(final Class expectedClass, final String expectedMessage, final Executable executable) { assertThrows(FixExecutionException.class, expectedClass, expectedMessage, executable, UnaryOperator.identity()); } + public static void assertProcessException(final Class expectedClass, final String expectedMessage, final Executable executable) { + assertThrows(FixProcessException.class, expectedClass, expectedMessage, executable, UnaryOperator.identity()); + } + public static void assertThrows(final Class expectedClass, final String expectedMessage, final Executable executable) { assertThrows(expectedClass, null, expectedMessage, executable, UnaryOperator.identity()); } diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/expected.json b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/expected.json new file mode 100644 index 00000000..31a21c97 --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/expected.json @@ -0,0 +1,14 @@ +{ + "before" : "", + "data" : "FOO", + "after" : "" +} +{ + "before" : "", + "after" : "" +} +{ + "before" : "", + "data" : "BAR", + "after" : "" +} diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/input.json b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/input.json new file mode 100644 index 00000000..ebe2c4eb --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/input.json @@ -0,0 +1,3 @@ +{"data":"foo"} +{"data":"foo","data":"bar"} +{"data":"bar"} diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/test.fix b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/test.fix new file mode 100644 index 00000000..39647f37 --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/test.fix @@ -0,0 +1,3 @@ +add_field("before", "") +upcase("data") +add_field("after", "") diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/test.flux b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/test.flux new file mode 100644 index 00000000..f12a5ac7 --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipExpressionOnExecutionException/test.flux @@ -0,0 +1,8 @@ +FLUX_DIR + "input.json" +|open-file +|as-records +|decode-json +|fix(FLUX_DIR + "test.fix", strictness="expression") +|encode-json(prettyPrinting="true") +|write(FLUX_DIR + "output-metafix.json") +; diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/expected.json b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/expected.json new file mode 100644 index 00000000..cca60a9b --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/expected.json @@ -0,0 +1,10 @@ +{ + "before" : "", + "data" : "FOO", + "after" : "" +} +{ + "before" : "", + "data" : "BAR", + "after" : "" +} diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/input.json b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/input.json new file mode 100644 index 00000000..ebe2c4eb --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/input.json @@ -0,0 +1,3 @@ +{"data":"foo"} +{"data":"foo","data":"bar"} +{"data":"bar"} diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/test.fix b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/test.fix new file mode 100644 index 00000000..39647f37 --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/test.fix @@ -0,0 +1,3 @@ +add_field("before", "") +upcase("data") +add_field("after", "") diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/test.flux b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/test.flux new file mode 100644 index 00000000..a8ed8f74 --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessSkipRecordOnExecutionException/test.flux @@ -0,0 +1,8 @@ +FLUX_DIR + "input.json" +|open-file +|as-records +|decode-json +|fix(FLUX_DIR + "test.fix", strictness="record") +|encode-json(prettyPrinting="true") +|write(FLUX_DIR + "output-metafix.json") +; From 6291ed5a70511bc71d4462ae2ea16cefd7119cda Mon Sep 17 00:00:00 2001 From: Jens Wille Date: Wed, 30 Mar 2022 13:08:45 +0200 Subject: [PATCH 2/3] Suppress log messages when testing "strict" mode. (#192) Requires "inline mock making"; see https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#mockito-inline. --- metafix/build.gradle | 2 +- .../metafix/MetafixScriptTest.java | 22 ++++++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/metafix/build.gradle b/metafix/build.gradle index 4c9e6919..4445d5b6 100644 --- a/metafix/build.gradle +++ b/metafix/build.gradle @@ -31,7 +31,7 @@ dependencies { implementation "org.metafacture:metamorph:${versions.metafacture}" testImplementation "nl.jqno.equalsverifier:equalsverifier:${versions.equalsverifier}" - testImplementation "org.mockito:mockito-core:${versions.mockito}" + testImplementation "org.mockito:mockito-inline:${versions.mockito}" testImplementation "org.mockito:mockito-junit-jupiter:${versions.mockito}" } diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixScriptTest.java b/metafix/src/test/java/org/metafacture/metafix/MetafixScriptTest.java index c257568b..3b00e3ff 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixScriptTest.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixScriptTest.java @@ -24,6 +24,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import java.io.FileNotFoundException; @@ -390,14 +391,19 @@ public void shouldIncludeLocationAndTextInExecutionException() { ); } - private void assertStrictness(final Metafix.Strictness strictness, final String fixDef, final Consumer> out) { + private void assertStrictness(final Metafix.Strictness strictness, final String fixDef, final boolean stubLogging, final Consumer> out) { MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "add_field('before', '')", fixDef, "add_field('after', '')" ), i -> { - i.setStrictness(strictness); + final Metafix.Strictness strictnessSpy = Mockito.spy(strictness); + i.setStrictness(strictnessSpy); + + if (stubLogging) { + Mockito.doNothing().when(strictnessSpy).log(Mockito.any(), Mockito.any()); + } i.startRecord("1"); i.literal("data", "foo"); @@ -418,13 +424,13 @@ private void assertStrictness(final Metafix.Strictness strictness, final String // TODO: Test logging statements } - private void assertStrictness(final Metafix.Strictness strictness, final Consumer> out) { - assertStrictness(strictness, "upcase('data')", out); + private void assertStrictness(final Metafix.Strictness strictness, final boolean stubLogging, final Consumer> out) { + assertStrictness(strictness, "upcase('data')", stubLogging, out); } @Test public void shouldSkipExpressionOnExecutionException() { - assertStrictness(Metafix.Strictness.EXPRESSION, o -> { + assertStrictness(Metafix.Strictness.EXPRESSION, true, o -> { o.get().startRecord("1"); o.get().literal("before", ""); o.get().literal("data", "FOO"); @@ -446,7 +452,7 @@ public void shouldSkipExpressionOnExecutionException() { @Test public void shouldSkipRecordOnExecutionException() { - assertStrictness(Metafix.Strictness.RECORD, o -> { + assertStrictness(Metafix.Strictness.RECORD, true, o -> { o.get().startRecord("1"); o.get().literal("before", ""); o.get().literal("data", "FOO"); @@ -464,7 +470,7 @@ public void shouldSkipRecordOnExecutionException() { @Test public void shouldAbortProcessOnExecutionException() { MetafixTestHelpers.assertExecutionException(IllegalStateException.class, "Expected String, got Array", () -> - assertStrictness(Metafix.Strictness.PROCESS, o -> { + assertStrictness(Metafix.Strictness.PROCESS, false, o -> { }) ); } @@ -472,7 +478,7 @@ public void shouldAbortProcessOnExecutionException() { @Test public void shouldAbortProcessOnProcessException() { MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "No enum constant org.metafacture.metafix.FixMethod.foo", () -> - assertStrictness(Metafix.Strictness.EXPRESSION, "foo()", o -> { + assertStrictness(Metafix.Strictness.EXPRESSION, "foo()", false, o -> { }) ); } From e167bbe9585cbe4136579f76712058165bb848b7 Mon Sep 17 00:00:00 2001 From: Jens Wille Date: Wed, 30 Mar 2022 18:18:56 +0200 Subject: [PATCH 3/3] Add integration tests with expected exceptions for "strict" mode. (#210, #140) --- .gitignore | 1 + metafix/integrationTest.sh | 96 ++++++++++++------- .../expected.err | 2 + .../input.json | 3 + .../test.fix | 3 + .../test.flux | 8 ++ .../expected.err | 2 + .../input.json | 3 + .../test.fix | 3 + .../test.flux | 8 ++ 10 files changed, 94 insertions(+), 35 deletions(-) create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/expected.err create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/input.json create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/test.fix create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/test.flux create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/expected.err create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/input.json create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/test.fix create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/test.flux diff --git a/.gitignore b/.gitignore index 15c00716..12ec8d9a 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ package-lock.json /metafix/src/test/resources/org/metafacture/metafix/integration/**/*.err /metafix/src/test/resources/org/metafacture/metafix/integration/**/*.out /metafix/src/test/resources/org/metafacture/metafix/integration/**/output-* +!/metafix/src/test/resources/org/metafacture/metafix/integration/**/expected.err diff --git a/metafix/integrationTest.sh b/metafix/integrationTest.sh index 36e9154a..25520658 100755 --- a/metafix/integrationTest.sh +++ b/metafix/integrationTest.sh @@ -12,6 +12,7 @@ todo_file=todo.txt input_glob=input.* expected_glob=expected.* +expected_errors_extension=err metafix_output_glob=output-metafix.* catmandu_output_glob=output-catmandu.* @@ -102,6 +103,8 @@ function get_file() { reason="Ambiguous $type files: $*" elif [ ! -r "$1" ]; then reason="No $type file: $1" + elif [ "$type" != "output" ] && [ ! -s "$1" ]; then + reason="Empty $type file: $1" else current_file=$1 return 0 @@ -138,6 +141,40 @@ function skip_test() { fi } +function test_passed() { + if [ -r "$2" ]; then + log "$color_test$1$color_reset: ${color_failed}FAILED$color_reset (Marked as \"to do\", but passed.)" + + ((failed++)) || true + else + if parse_boolean "$METAFIX_LOG_PASSED"; then + log "$color_test$1$color_reset: ${color_passed}PASSED$color_reset$3" + fi + + ((passed++)) || true + fi +} + +function test_failed() { + if ! skip_test "$1" "$2"; then + log "$color_test$1$color_reset: $color_failed$4$color_reset$3" + + if [ $# -ge 13 ]; then + log " Fix: $9" + log " Input: ${10}" + log " Expected: ${11}" + log " Output: ${12}" + log " Diff: ${13}" + + [ -s "${13}" ] && $colordiff <"${13}" || rm -f "${13}" + fi + + command_info "$5" "$6" "$7" "$8" + + ((failed++)) || true + fi +} + function run_tests() { local test matched=1\ test_directory test_fix test_input test_expected test_todo\ @@ -165,62 +202,51 @@ function run_tests() { test_todo="$test_directory/$todo_file" if [ -z "$disable_todo" ] || ! skip_test "$test" "$test_todo"; then + # TODO: catmandu (optional) + metafix_command_output="$test_directory/metafix.out" metafix_command_error="$test_directory/metafix.err" metafix_start_time=$(current_time) - # TODO: catmandu (optional) + run_metafix "$test_directory/$metafix_file" >"$metafix_command_output" 2>"$metafix_command_error" + metafix_exit_status=$? - if run_metafix "$test_directory/$metafix_file" >"$metafix_command_output" 2>"$metafix_command_error"; then - metafix_exit_status=$? + metafix_elapsed_time=$(elapsed_time "$metafix_start_time") + if [ "$metafix_exit_status" -eq 0 ]; then if get_file "$test" output "$test_directory"/$metafix_output_glob; then metafix_output=$current_file metafix_diff="$test_directory/metafix.diff" - metafix_elapsed_time=$(elapsed_time "$metafix_start_time") - if diff -u "$test_expected" "$metafix_output" >"$metafix_diff"; then - if [ -r "$test_todo" ]; then - log "$color_test$test$color_reset: ${color_failed}FAILED$color_reset (Marked as \"to do\", but passed.)" - - ((failed++)) || true - else - if parse_boolean "$METAFIX_LOG_PASSED"; then - log "$color_test$test$color_reset: ${color_passed}PASSED$color_reset$metafix_elapsed_time" - fi - - ((passed++)) || true - fi + test_passed "$test" "$test_todo" "$metafix_elapsed_time" rm -f "$metafix_diff" "$metafix_command_output" "$metafix_command_error" - elif ! skip_test "$test" "$test_todo"; then - log "$color_test$test$color_reset: ${color_failed}FAILED$color_reset$metafix_elapsed_time" - - log " Fix: $test_fix" - log " Input: $test_input" - log " Expected: $test_expected" - log " Output: $metafix_output" - log " Diff: $metafix_diff" - - [ -s "$metafix_diff" ] && $colordiff <"$metafix_diff" || rm -f "$metafix_diff" - - command_info metafix "$metafix_exit_status" "$metafix_command_output" "$metafix_command_error" - - ((failed++)) || true + else + test_failed "$test" "$test_todo" "$metafix_elapsed_time" FAILED\ + metafix "$metafix_exit_status" "$metafix_command_output" "$metafix_command_error"\ + "$test_fix" "$test_input" "$test_expected" "$metafix_output" "$metafix_diff" fi else command_info metafix "$metafix_exit_status" "$metafix_command_output" "$metafix_command_error" fi - elif ! skip_test "$test" "$test_todo"; then - metafix_exit_status=$? + elif [ "${test_expected##*.}" == "$expected_errors_extension" ]; then + get_file "$test" error "$metafix_command_error" || { log; continue; } - log "$color_test$test$color_reset: ${color_error}ERROR$color_reset" + while read -r pattern; do + if ! grep -qE "$pattern" "$metafix_command_error"; then + test_failed "$test" "$test_todo" " (Pattern not found: $pattern)" FAILED\ + metafix "$metafix_exit_status" "$metafix_command_output" "$metafix_command_error" - command_info metafix "$metafix_exit_status" "$metafix_command_output" "$metafix_command_error" + continue 2 + fi + done <"$test_expected" - ((failed++)) || true + test_passed "$test" "$test_todo" "$metafix_elapsed_time" + else + test_failed "$test" "$test_todo" "$metafix_elapsed_time" ERROR\ + metafix "$metafix_exit_status" "$metafix_command_output" "$metafix_command_error" fi fi done diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/expected.err b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/expected.err new file mode 100644 index 00000000..9e1924a5 --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/expected.err @@ -0,0 +1,2 @@ +^Exception in thread "main" org\.metafacture\.metafix\.FixExecutionException: Error while executing Fix expression \(at file:.*/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/test\.fix, line 2\): upcase\("data"\)$ +^Caused by: java\.lang\.IllegalStateException: Expected String, got Array$ diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/input.json b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/input.json new file mode 100644 index 00000000..ebe2c4eb --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/input.json @@ -0,0 +1,3 @@ +{"data":"foo"} +{"data":"foo","data":"bar"} +{"data":"bar"} diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/test.fix b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/test.fix new file mode 100644 index 00000000..39647f37 --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/test.fix @@ -0,0 +1,3 @@ +add_field("before", "") +upcase("data") +add_field("after", "") diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/test.flux b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/test.flux new file mode 100644 index 00000000..d7e8568c --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnExecutionException/test.flux @@ -0,0 +1,8 @@ +FLUX_DIR + "input.json" +|open-file +|as-records +|decode-json +|fix(FLUX_DIR + "test.fix", strictness="process") +|encode-json +|write(FLUX_DIR + "output-metafix.json") +; diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/expected.err b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/expected.err new file mode 100644 index 00000000..14126ede --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/expected.err @@ -0,0 +1,2 @@ +^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\(\)$ +^Caused by: java\.lang\.IllegalArgumentException: No enum constant org\.metafacture\.metafix\.FixMethod.foo$ diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/input.json b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/input.json new file mode 100644 index 00000000..ebe2c4eb --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/input.json @@ -0,0 +1,3 @@ +{"data":"foo"} +{"data":"foo","data":"bar"} +{"data":"bar"} diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/test.fix b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/test.fix new file mode 100644 index 00000000..ea5169c6 --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/test.fix @@ -0,0 +1,3 @@ +add_field("before", "") +foo() +add_field("after", "") diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/test.flux b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/test.flux new file mode 100644 index 00000000..1264c7ff --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/script/fromJson/toJson/strictnessAbortProcessOnProcessException/test.flux @@ -0,0 +1,8 @@ +FLUX_DIR + "input.json" +|open-file +|as-records +|decode-json +|fix(FLUX_DIR + "test.fix", strictness="expression") +|encode-json +|write(FLUX_DIR + "output-metafix.json") +;