From e9dd4ab71b687e7cc9d9a570f4c5fe541762a69e Mon Sep 17 00:00:00 2001 From: Fabian Steeg Date: Fri, 4 Mar 2022 14:53:48 +0100 Subject: [PATCH] Tweak `lookup` behavior, partial `delete` support (#102, #149) --- .../org/metafacture/metafix/FixMethod.java | 5 +- .../java/org/metafacture/metafix/Metafix.java | 2 +- .../java/org/metafacture/metafix/Value.java | 3 +- .../metafix/MetafixLookupTest.java | 196 +++++++++++++++--- .../todo.txt | 1 + .../todo.txt | 1 + 6 files changed, 173 insertions(+), 35 deletions(-) create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/lookup/fromJson/toJson/lookupSimpleWithInternalMapDeleteNotMatchingLiteral/todo.txt create mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/move_fieldAppendObjectToArrayOfObjects/todo.txt diff --git a/metafix/src/main/java/org/metafacture/metafix/FixMethod.java b/metafix/src/main/java/org/metafacture/metafix/FixMethod.java index cb35f068..44af6413 100644 --- a/metafix/src/main/java/org/metafacture/metafix/FixMethod.java +++ b/metafix/src/main/java/org/metafacture/metafix/FixMethod.java @@ -383,7 +383,10 @@ public void apply(final Metafix metafix, final Record record, final List } final String defaultValue = map.get(Maps.DEFAULT_MAP_KEY); // TODO: Catmandu uses 'default' - record.transform(params.get(0), k -> map.getOrDefault(k, defaultValue)); + record.transform(params.get(0), oldValue -> { + final String newValue = map.getOrDefault(oldValue, defaultValue); + return newValue != null ? newValue : Boolean.valueOf(options.getOrDefault("delete", "false")) ? null : oldValue; + }); } }, prepend { diff --git a/metafix/src/main/java/org/metafacture/metafix/Metafix.java b/metafix/src/main/java/org/metafacture/metafix/Metafix.java index 757be561..eda42bb7 100644 --- a/metafix/src/main/java/org/metafacture/metafix/Metafix.java +++ b/metafix/src/main/java/org/metafacture/metafix/Metafix.java @@ -201,7 +201,7 @@ private void emit(final String field, final Value value) { h.forEach(this::emit); outputStreamReceiver.endEntity(); }) - .orElse(v -> outputStreamReceiver.literal(fieldName, v.asString())); + .ifString(s -> outputStreamReceiver.literal(fieldName, s)); } if (isMulti) { diff --git a/metafix/src/main/java/org/metafacture/metafix/Value.java b/metafix/src/main/java/org/metafacture/metafix/Value.java index 99bbf67d..c0b1c93f 100644 --- a/metafix/src/main/java/org/metafacture/metafix/Value.java +++ b/metafix/src/main/java/org/metafacture/metafix/Value.java @@ -313,7 +313,8 @@ private TypeMatcher match(final Type type, final Consumer consumer, final private abstract static class AbstractValueType { - protected static final Predicate REMOVE_EMPTY_VALUES = v -> v.extractType((m, c) -> m + protected static final Predicate REMOVE_EMPTY_VALUES = v -> + v.isNull() ? true : v.extractType((m, c) -> m .ifArray(a -> { a.removeEmptyValues(); c.accept(a.isEmpty()); diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixLookupTest.java b/metafix/src/test/java/org/metafacture/metafix/MetafixLookupTest.java index 0581813a..69266f7a 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixLookupTest.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixLookupTest.java @@ -240,22 +240,156 @@ public void shouldIgnoreOptionsOnLookupInSeparateExternalFileMap() { @Test public void shouldNotLookupInExternalFileMapWithWrongOptions() { - MetafixTestHelpers.assertThrows(IllegalStateException.class, "Expected String, got null", () -> - MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( - LOOKUP + " '" + CSV_MAP + "', sep_char: '\t')" - ), - i -> { - i.startRecord("1"); - i.literal("title", "Aloha"); - i.literal("title", "Moin"); - i.literal("title", "Hey"); - i.endRecord(); - }, - (o, f) -> { - o.get().startRecord("1"); - o.get().endRecord(); - } - ) + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + LOOKUP + " '" + CSV_MAP + "', sep_char: '\t', delete: 'true')" + ), + i -> { + i.startRecord("1"); + i.literal("title", "Aloha"); + i.literal("title", "Moin"); + i.literal("title", "Hey"); + i.endRecord(); + }, + o -> { + o.get().startRecord("1"); + o.get().endRecord(); + } + ); + } + + @Test + // See https://github.com/metafacture/metafacture-fix/issues/149 + public void shouldKeepOriginalValueIfNotFoundAndNoDefault() { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "lookup('title.*', Aloha: 'Alohaeha', 'Moin': 'Moin zäme')" + ), + i -> { + i.startRecord("1"); + i.literal("title", "Aloha"); + i.literal("title", "Moin"); + i.literal("title", "Yo"); + i.endRecord(); + }, + o -> { + o.get().startRecord("1"); + o.get().literal("title", "Alohaeha"); + o.get().literal("title", "Moin zäme"); + o.get().literal("title", "Yo"); + o.get().endRecord(); + } + ); + } + + @Test + public void shouldUseDefaultValueIfNotFound() { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "lookup('title.*', Aloha: 'Alohaeha', 'Moin': 'Moin zäme', __default: Tach)" + ), + i -> { + i.startRecord("1"); + i.literal("title", "Aloha"); + i.literal("title", "Moin"); + i.literal("title", "Yo"); + i.endRecord(); + }, + o -> { + o.get().startRecord("1"); + o.get().literal("title", "Alohaeha"); + o.get().literal("title", "Moin zäme"); + o.get().literal("title", "Tach"); + o.get().endRecord(); + } + ); + } + + @Test + // See https://github.com/metafacture/metafacture-fix/issues/149 + public void shouldDeleteNonFoundLookupOnDemand() { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "lookup('title.*', Aloha: Alohaeha, 'Moin': 'Moin zäme', delete: 'true')" + ), + i -> { + i.startRecord("1"); + i.literal("title", "Aloha"); + i.literal("title", "Moin"); + i.literal("title", "Yo"); + i.endRecord(); + }, + o -> { + o.get().startRecord("1"); + o.get().literal("title", "Alohaeha"); + o.get().literal("title", "Moin zäme"); + o.get().endRecord(); + } + ); + } + + @Test + // See https://github.com/metafacture/metafacture-fix/issues/149 + public void shouldNotDeleteNonFoundLookupExplicitly() { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "lookup('title.*', Aloha: Alohaeha, 'Moin': 'Moin zäme', delete: 'false')" + ), + i -> { + i.startRecord("1"); + i.literal("title", "Aloha"); + i.literal("title", "Moin"); + i.literal("title", "Yo"); + i.endRecord(); + }, + o -> { + o.get().startRecord("1"); + o.get().literal("title", "Alohaeha"); + o.get().literal("title", "Moin zäme"); + o.get().literal("title", "Yo"); + o.get().endRecord(); + } + ); + } + + @Test + public void shouldDeleteNonFoundLookupOnDemandAndVacuum() { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "put_map('testMap', Aloha: Alohaeha, 'Moin': 'Moin zäme')", + "lookup('title.*', testMap, delete: 'true')", + "vacuum()" + ), + i -> { + i.startRecord("1"); + i.literal("title", "Aloha"); + i.literal("title", "Moin"); + i.literal("title", "Yo"); + i.endRecord(); + }, + o -> { + o.get().startRecord("1"); + o.get().literal("title", "Alohaeha"); + o.get().literal("title", "Moin zäme"); + o.get().endRecord(); + } + ); + } + + @Test + public void shouldDeleteNonFoundLookupOnDemandAndMove() { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "put_map('testMap', Aloha: Alohaeha, 'Moin': 'Moin zäme')", + "lookup('title.*', testMap, delete: 'true')", + "move_field('title','t')" + ), + i -> { + i.startRecord("1"); + i.literal("title", "Aloha"); + i.literal("title", "Moin"); + i.literal("title", "Yo"); + i.endRecord(); + }, + o -> { + o.get().startRecord("1"); + o.get().literal("t", "Alohaeha"); + o.get().literal("t", "Moin zäme"); + o.get().endRecord(); + } ); } @@ -282,22 +416,20 @@ public void shouldIgnoreOptionsOnSubsequentLookupInExternalFileMap() { @Test public void shouldNotLookupInUnknownInternalMap() { - MetafixTestHelpers.assertThrows(IllegalStateException.class, "Expected String, got null", () -> - MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( - LOOKUP + " 'testMap')" - ), - i -> { - i.startRecord("1"); - i.literal("title", "Aloha"); - i.literal("title", "Moin"); - i.literal("title", "Hey"); - i.endRecord(); - }, - (o, f) -> { - o.get().startRecord("1"); - o.get().endRecord(); - } - ) + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + LOOKUP + " 'testMap', delete: 'true')" + ), + i -> { + i.startRecord("1"); + i.literal("title", "Aloha"); + i.literal("title", "Moin"); + i.literal("title", "Hey"); + i.endRecord(); + }, + o -> { + o.get().startRecord("1"); + o.get().endRecord(); + } ); } diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/lookup/fromJson/toJson/lookupSimpleWithInternalMapDeleteNotMatchingLiteral/todo.txt b/metafix/src/test/resources/org/metafacture/metafix/integration/lookup/fromJson/toJson/lookupSimpleWithInternalMapDeleteNotMatchingLiteral/todo.txt new file mode 100644 index 00000000..969e64d5 --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/lookup/fromJson/toJson/lookupSimpleWithInternalMapDeleteNotMatchingLiteral/todo.txt @@ -0,0 +1 @@ +See https://github.com/metafacture/metafacture-fix/issues/149 diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/move_fieldAppendObjectToArrayOfObjects/todo.txt b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/move_fieldAppendObjectToArrayOfObjects/todo.txt new file mode 100644 index 00000000..1381aad2 --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/move_fieldAppendObjectToArrayOfObjects/todo.txt @@ -0,0 +1 @@ +See https://github.com/metafacture/metafacture-fix/issues/92