Fix issues when inserting into arrays

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,24 +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)) {
-            if (path.length == 1) {
-                for (int i = 0; i < array.size(); ++i) {
-                    mode.apply(array, String.valueOf(i + 1), newValue);
-                }
-            }
-            else {
-                array.add(Value.newHash(h -> new FixPath(tail(path)).insertInto(h, mode, newValue)));
-            }
+        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);
@@ -279,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;
 
@@ -348,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?

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();
             }
@@ -2369,7 +2416,6 @@ public void shouldSortFieldAndRemoveDuplicates() {
     }
 
     @Test
-    @MetafixToDo("See https://github.com/metafacture/metafacture-fix/issues/121")
     public void shouldSortArrayFieldWithAsterisk() {
         MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
                 "sort_field('OTHERS[].*.dnimals[]')"
@@ -2558,7 +2604,6 @@ public void shouldSplitHashField() {
     }
 
     @Test
-    @MetafixToDo("See https://github.com/metafacture/metafacture-fix/issues/100 and https://github.com/metafacture/metafacture-fix/issues/121")
     public void shouldSplitNestedField() {
         MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
                 "split_field('others[].*.tools', '--')"
@@ -2607,7 +2652,6 @@ public void shouldSumNumbers() {
     }
 
     @Test
-    @MetafixToDo("java.lang.IllegalStateException: Expected String, got Array; see https://github.com/metafacture/metafacture-fix/issues/121")
     public void shouldSumArrayFieldWithAsterisk() {
         MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
                 "sum('OTHERS[].*.dumbers[]')"

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

@@ -676,7 +676,7 @@ public void appendWithAsteriksWildcardAtTheEnd() {
     }
 
     @Test
-    // See https://github.com/metafacture/metafacture-fix/pull/142
+    @MetafixToDo("See https://github.com/metafacture/metafacture-fix/pull/113")
     public void shouldCopyArrayFieldWithoutAsterisk() {
         MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
                 "set_array('TEST_TWO[]')",
@@ -691,23 +691,51 @@ public void shouldCopyArrayFieldWithoutAsterisk() {
                 i.endEntity();
                 i.endRecord();
             },
-            o -> {
+            (o, f) -> {
                 o.get().startRecord("1");
                 o.get().startEntity("test[]");
                 o.get().literal("1", "One");
                 o.get().literal("2", "Two");
                 o.get().literal("3", "Three");
                 o.get().endEntity();
                 o.get().startEntity("TEST_TWO[]");
+                o.get().startEntity("1");
                 o.get().literal("1", "One");
                 o.get().literal("2", "Two");
                 o.get().literal("3", "Three");
-                o.get().endEntity();
+                f.apply(2).endEntity();
                 o.get().endRecord();
             }
         );
     }
 
+    @Test
+    @MetafixToDo("See https://github.com/metafacture/metafacture-fix/issues/113")
+    public void copyFieldArrayOfObjectsAndListNewArrayOfObjectsAndMoveSubfield() {
+        MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
+                "copy_field('author[]','creator[]')",
+                "do list(path:'creator[]')",
+                "  move_field('name','label')",
+                "end",
+                "retain('creator[]')"),
+            i -> {
+                i.startRecord("1");
+                i.startEntity("author[]");
+                i.startEntity("1");
+                i.literal("name", "A University");
+                i.endEntity();
+                i.endEntity();
+                i.endRecord();
+            }, (o, f) -> {
+                o.get().startRecord("1");
+                o.get().startEntity("creator[]");
+                o.get().startEntity("1");
+                o.get().literal("label", "A University");
+                f.apply(2).endEntity();
+                o.get().endRecord();
+            });
+    }
+
     @Test
     // See https://github.com/metafacture/metafacture-fix/issues/121
     public void shouldCopyArrayFieldWithAsterisk() {
@@ -1070,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 -> {
@@ -1099,7 +1129,6 @@ private void assertThrowsOnEmptyRecord(final String index) {
     }
 
     @Test
-    @MetafixToDo("See https://github.com/metafacture/metafacture-fix/issues/121")
     public void shouldAddArraySubFieldWithAsterisk() {
         MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
                 "add_field('coll[].*.c', 'test')"
@@ -1144,7 +1173,7 @@ public void shouldAddArraySubFieldWithAsterisk() {
                 o.get().startEntity("4");
                 o.get().literal("a", "Crocodile");
                 o.get().literal("c", "test");
-                f.apply(3).endEntity();
+                f.apply(2).endEntity();
                 o.get().endRecord();
             }
         );
@@ -1685,6 +1714,39 @@ public void setArrayReplaceExisting() {
             });
     }
 
+    @Test
+    // See https://github.com/metafacture/metafacture-fix/issues/130
+    public void setArrayInArrayWithAsterisk() {
+        MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
+                "set_array('foo[].*.test[]', 'test')"),
+            i -> {
+                i.startRecord("1");
+                i.startEntity("foo[]");
+                i.startEntity("1");
+                i.literal("id", "A");
+                i.endEntity();
+                i.startEntity("2");
+                i.literal("id", "B");
+                i.endEntity();
+                i.endEntity();
+                i.endRecord();
+            }, (o, f) -> {
+                o.get().startRecord("1");
+                o.get().startEntity("foo[]");
+                o.get().startEntity("1");
+                o.get().literal("id", "A");
+                o.get().startEntity("test[]");
+                o.get().literal("1", "test");
+                f.apply(2).endEntity();
+                o.get().startEntity("2");
+                o.get().literal("id", "B");
+                o.get().startEntity("test[]");
+                o.get().literal("1", "test");
+                f.apply(3).endEntity();
+                o.get().endRecord();
+            });
+    }
+
     @Test
     public void setHash() {
         MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(

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

metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldArrayOfObjectsAndListNewArrayOfObjectsAndMoveSubfield/input.json

@@ -25,8 +25,8 @@
 {
   "author": [
     {
-      "@type": "Person",
-      "name": "Jürgen Böhner"
+      "name": "Jürgen Böhner",
+      "@type": "Person"
     }
   ]
 }

metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/copy_fieldArrayOfObjectsAsRepeatedObjectAndThenArrayOfStringsIntoTheArrayOfObjectsWithLastWildcard/todo.txt

@@ -1 +1 @@
-See issue #132 may also be related to #113
+See issue #132