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") +;