From c557c16d0abd85c0f9452165fbeeb6e939e0cd8f Mon Sep 17 00:00:00 2001 From: Fabian Steeg Date: Thu, 5 May 2022 17:41:25 +0200 Subject: [PATCH] Reuse former `processRef` in `insertInto` methods (#92, #171, #205) Fix issue with testing non-existing refs, update tests --- .../java/org/metafacture/metafix/FixPath.java | 58 +++++++------------ .../metafix/MetafixMethodTest.java | 57 ++++++++++++++++-- .../metafix/MetafixRecordTest.java | 10 ++-- .../test.fix | 4 +- .../todo.txt | 1 - .../add_fieldSimpleSubsubfield/todo.txt | 1 - .../todo.txt | 1 - .../todo.txt | 1 - .../test.fix | 2 +- .../test.fix | 2 +- 10 files changed, 84 insertions(+), 53 deletions(-) delete mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldIntoArrayOfObjectsWithLastWildcard/todo.txt delete mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldSimpleSubsubfield/todo.txt delete mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldObjectFromArrayOfObjectsWithFirstWildcard/todo.txt delete mode 100644 metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldObjectFromArrayOfObjectsWithLastWildcard/todo.txt diff --git a/metafix/src/main/java/org/metafacture/metafix/FixPath.java b/metafix/src/main/java/org/metafacture/metafix/FixPath.java index 5dee05f5..60c6a90b 100644 --- a/metafix/src/main/java/org/metafacture/metafix/FixPath.java +++ b/metafix/src/main/java/org/metafacture/metafix/FixPath.java @@ -102,7 +102,7 @@ else if (isReference(currentSegment)) { private Value findInValue(final Value value, final String[] p) { // TODO: move impl into enum elements, here call only value.find - return value == null ? null : value.extractType((m, c) -> m + return p.length == 0 ? value : value == null ? null : value.extractType((m, c) -> m .ifArray(a -> c.accept(new FixPath(p).findIn(a))) .ifHash(h -> c.accept(new FixPath(p).findIn(h))) .orElse(c) @@ -249,20 +249,15 @@ private void removeNestedFrom(final Value value) { /*package-private*/ private Value insertInto(final Array array, final InsertMode mode, final Value newValue) { // basic idea: reuse findIn logic here? setIn(findIn(array), newValue) final String field = path[0]; - if (field.equals(ASTERISK)) { - array.forEach(value -> value.matchType() - .ifArray(a -> new FixPath(tail(path)).insertInto(a, mode, newValue)) - .ifHash(h -> new FixPath(tail(path)).insertInto(h, mode, newValue)) - .orElseThrow()); + if (path.length == 1) { + mode.apply(array, field, newValue); } else { - if (path.length == 1) { - mode.apply(array, field, newValue); + if (ASTERISK.equals(field)) { + array.forEach(value -> insertInto(value, mode, newValue, field, tail(path))); } - else { - if (isReference(field)) { - return processRef(getReferencedValue(array, field), mode, newValue, field, tail(path)); - } + else if (isReference(field)) { + insertInto(getReferencedValue(array, field), mode, newValue, field, tail(path)); } } return new Value(array); @@ -275,45 +270,34 @@ private void removeNestedFrom(final Value value) { mode.apply(hash, field, newValue); } else { - final String[] tail = tail(path); - if (isReference(field)) { - return processRef(hash.get(field), mode, newValue, field, tail); - } if (!hash.containsField(field)) { hash.put(field, Value.newHash()); } - final Value value = hash.get(field); - if (value != null) { - // TODO: move impl into enum elements, here call only value.insert - value.matchType() - .ifArray(a -> new FixPath(tail).insertInto(a, mode, newValue)) - .ifHash(h -> new FixPath(tail).insertInto(h, mode, newValue)) - .orElseThrow(); - } + insertInto(hash.get(field), mode, newValue, field, tail(path)); } return new Value(hash); } - private String[] tail(final String[] fields) { - return Arrays.copyOfRange(fields, 1, fields.length); - } - - private Value processRef(final Value referencedValue, final InsertMode mode, final Value newValue, final String field, + private Value insertInto(final Value value, final InsertMode mode, final Value newValue, final String field, final String[] tail) { - if (referencedValue != null) { + if (value != null) { final FixPath fixPath = new FixPath(tail); - newValue.updatePathAddBase(referencedValue, field); - return referencedValue.extractType((m, c) -> m - .ifArray(a -> c.accept(fixPath.insertInto(referencedValue.asArray(), mode, newValue))) - .ifHash(h -> c.accept(fixPath.insertInto(referencedValue.asHash(), mode, newValue))) + newValue.updatePathAddBase(value, field); + return value.extractType((m, c) -> m + .ifArray(a -> c.accept(fixPath.insertInto(a, mode, newValue))) + .ifHash(h -> c.accept(fixPath.insertInto(h, mode, newValue))) .orElseThrow()); } else { - throw new IllegalArgumentException("Using ref, but can't find: " + field + " in: " + referencedValue); + throw new IllegalArgumentException("Using ref, but can't find: " + field + " in: " + value); } } + private String[] tail(final String[] fields) { + return Arrays.copyOfRange(fields, 1, fields.length); + } + private enum ReservedField { $append, $first, $last; @@ -344,10 +328,10 @@ private Value getReferencedValue(final Array array, final String field) { if (reservedField != null) { switch (reservedField) { case $first: - referencedValue = array.get(0); + referencedValue = getReferencedValue(array, "1"); break; case $last: - referencedValue = array.get(array.size() - 1); + referencedValue = getReferencedValue(array, String.valueOf(array.size())); break; case $append: referencedValue = Value.newHash(); // TODO: append non-hash? diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java b/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java index 01404d0c..0b7036e0 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java @@ -1844,10 +1844,34 @@ public void copyFieldToSubfieldOfArrayOfObjectsWithIndexExplicitAppend() { } @Test - @MetafixToDo("See https://github.com/metafacture/metafacture-fix/pull/205") - public void addFieldIntoArrayOfObjectsWithLastWildcardImplicitSkip() { + // We currently fail on unresolved references, see MetafixRecordTests#assertThrowsOnEmptyArray + public void addFieldIntoArrayOfObjectsWithLastWildcardMissingError() { + MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "Using ref, but can't find: $last in: null", () -> { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "add_field('animals[].$last.key', 'value')" + ), + i -> { + i.startRecord("1"); + i.startEntity("animals[]"); + i.endEntity(); + i.endRecord(); + }, + (o, f) -> { + o.get().startRecord("1"); + o.get().startEntity("animals[]"); + o.get().endEntity(); + o.get().endRecord(); + } + ); + }); + } + + @Test + public void addFieldIntoArrayOfObjectsWithLastWildcardAllEmpty() { MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( - "add_field('animals[].$last.key', 'value')" + "if exists('animals[].$last')", + " add_field('animals[].$last.key', 'value')", + "end" ), i -> { i.startRecord("1"); @@ -1865,8 +1889,7 @@ public void addFieldIntoArrayOfObjectsWithLastWildcardImplicitSkip() { } @Test - @MetafixToDo("See https://github.com/metafacture/metafacture-fix/pull/205") - public void addFieldIntoArrayOfObjectsWithLastWildcardExplicitSkip() { + public void addFieldIntoArrayOfObjectsWithLastWildcardLastEmpty() { MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "if exists('animals[].$last')", " add_field('animals[].$last.key', 'value')", @@ -1875,12 +1898,36 @@ public void addFieldIntoArrayOfObjectsWithLastWildcardExplicitSkip() { i -> { i.startRecord("1"); i.startEntity("animals[]"); + i.startEntity("1"); + i.literal("name", "Jake"); + i.literal("type", "dog"); + i.endEntity(); + i.startEntity("2"); + i.literal("name", "Blacky"); + i.literal("type", "bird"); + i.endEntity(); + i.endEntity(); + i.endRecord(); + i.startRecord("2"); + i.startEntity("animals[]"); i.endEntity(); i.endRecord(); }, (o, f) -> { o.get().startRecord("1"); o.get().startEntity("animals[]"); + o.get().startEntity("1"); + o.get().literal("name", "Jake"); + o.get().literal("type", "dog"); + o.get().endEntity(); + o.get().startEntity("2"); + o.get().literal("name", "Blacky"); + o.get().literal("type", "bird"); + o.get().literal("key", "value"); + f.apply(2).endEntity(); + o.get().endRecord(); + o.get().startRecord("2"); + o.get().startEntity("animals[]"); o.get().endEntity(); o.get().endRecord(); } diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java b/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java index 7c168c9b..3e78836b 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java @@ -1098,26 +1098,28 @@ public void addFieldToObjectByIndexInIndexedArray() { @Test public void addFieldToFirstObjectMissing() { - assertThrowsOnEmptyRecord("$first"); + assertThrowsOnEmptyArray("$first"); } @Test public void addFieldToLastObjectMissing() { - assertThrowsOnEmptyRecord("$last"); + assertThrowsOnEmptyArray("$last"); } @Test public void addFieldToObjectByIndexMissing() { - assertThrowsOnEmptyRecord("2"); + assertThrowsOnEmptyArray("2"); } - private void assertThrowsOnEmptyRecord(final String index) { + private void assertThrowsOnEmptyArray(final String index) { MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "Using ref, but can't find: " + index + " in: null", () -> { MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "add_field('animals[]." + index + ".kind','nice')" ), i -> { i.startRecord("1"); + i.startEntity("animals[]"); + i.endEntity(); i.endRecord(); }, o -> { diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldIntoArrayOfObjectsWithLastWildcard/test.fix b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldIntoArrayOfObjectsWithLastWildcard/test.fix index ca7943f9..44cfd14c 100644 --- a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldIntoArrayOfObjectsWithLastWildcard/test.fix +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldIntoArrayOfObjectsWithLastWildcard/test.fix @@ -1 +1,3 @@ -add_field("animals[].$last.key", "value") +if exists("animals[].$last") + add_field("animals[].$last.key", "value") +end diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldIntoArrayOfObjectsWithLastWildcard/todo.txt b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldIntoArrayOfObjectsWithLastWildcard/todo.txt deleted file mode 100644 index 0d3f4274..00000000 --- a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldIntoArrayOfObjectsWithLastWildcard/todo.txt +++ /dev/null @@ -1 +0,0 @@ -See PR #205. diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldSimpleSubsubfield/todo.txt b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldSimpleSubsubfield/todo.txt deleted file mode 100644 index 289b8cb3..00000000 --- a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldSimpleSubsubfield/todo.txt +++ /dev/null @@ -1 +0,0 @@ -See issue #171 diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldObjectFromArrayOfObjectsWithFirstWildcard/todo.txt b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldObjectFromArrayOfObjectsWithFirstWildcard/todo.txt deleted file mode 100644 index 2f4b8670..00000000 --- a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldObjectFromArrayOfObjectsWithFirstWildcard/todo.txt +++ /dev/null @@ -1 +0,0 @@ -See issue #92 diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldObjectFromArrayOfObjectsWithLastWildcard/todo.txt b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldObjectFromArrayOfObjectsWithLastWildcard/todo.txt deleted file mode 100644 index 2f4b8670..00000000 --- a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldObjectFromArrayOfObjectsWithLastWildcard/todo.txt +++ /dev/null @@ -1 +0,0 @@ -See issue #92 diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldPrependObjectToArrayOfObjects/test.fix b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldPrependObjectToArrayOfObjects/test.fix index dc7b29ca..cf2c5df0 100644 --- a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldPrependObjectToArrayOfObjects/test.fix +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldPrependObjectToArrayOfObjects/test.fix @@ -1 +1 @@ -copy_field("test_2", "test[].$append") +copy_field("test_2", "test[].$prepend") diff --git a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/move_fieldPrependObjectToArrayOfObjects/test.fix b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/move_fieldPrependObjectToArrayOfObjects/test.fix index dc7b29ca..cf2c5df0 100644 --- a/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/move_fieldPrependObjectToArrayOfObjects/test.fix +++ b/metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/move_fieldPrependObjectToArrayOfObjects/test.fix @@ -1 +1 @@ -copy_field("test_2", "test[].$append") +copy_field("test_2", "test[].$prepend")