Skip to content

Commit 1f9d5c2

Browse files
fsteegfsteeg
authored
Merge pull request #217 from metafacture/130-156-171-insert
Fix issues when inserting into arrays
2 parents d82e1b8 + c557c16 commit 1f9d5c2

File tree

17 files changed

+152
-73
lines changed
  • metafix/src
    • main/java/org/metafacture/metafix
    • test
      • java/org/metafacture/metafix
      • resources/org/metafacture/metafix/integration/record/fromJson/toJson
        • add_fieldAsteriskIntoArrayOfObjects
        • add_fieldIntoArrayOfObjectsWithLastWildcard
        • add_fieldSimpleSubsubfield
        • copy_fieldArrayOfObjectsAndListNewArrayOfObjectsAndMoveSubfield
        • copy_fieldArrayOfObjectsAsRepeatedObjectAndThenArrayOfStringsIntoTheArrayOfObjectsWithLastWildcard
        • copy_fieldArrayOfStringsIntoArrayOfObjectsWithAsteriskWildcard
        • copy_fieldAsteriskIntoArrayOfObjects
        • copy_fieldObjectFromArrayOfObjectsWithFirstWildcard
        • copy_fieldObjectFromArrayOfObjectsWithLastWildcard
        • copy_fieldPrependObjectToArrayOfObjects
        • move_fieldAsteriskIntoArrayOfObjects
        • move_fieldPrependObjectToArrayOfObjects
        • set_arrayIntoArrayOfObjectsWithAsterisk

17 files changed

+152
-73
lines changed

‎metafix/src/main/java/org/metafacture/metafix/FixPath.java

Lines changed: 21 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ else if (isReference(currentSegment)) {
102102

103103
private Value findInValue(final Value value, final String[] p) {
104104
// TODO: move impl into enum elements, here call only value.find
105-
return value == null ? null : value.extractType((m, c) -> m
105+
return p.length == 0 ? value : value == null ? null : value.extractType((m, c) -> m
106106
.ifArray(a -> c.accept(new FixPath(p).findIn(a)))
107107
.ifHash(h -> c.accept(new FixPath(p).findIn(h)))
108108
.orElse(c)
@@ -249,24 +249,15 @@ private void removeNestedFrom(final Value value) {
249249
/*package-private*/ private Value insertInto(final Array array, final InsertMode mode, final Value newValue) {
250250
// basic idea: reuse findIn logic here? setIn(findIn(array), newValue)
251251
final String field = path[0];
252-
if (field.equals(ASTERISK)) {
253-
if (path.length == 1) {
254-
for (int i = 0; i < array.size(); ++i) {
255-
mode.apply(array, String.valueOf(i + 1), newValue);
256-
}
257-
}
258-
else {
259-
array.add(Value.newHash(h -> new FixPath(tail(path)).insertInto(h, mode, newValue)));
260-
}
252+
if (path.length == 1) {
253+
mode.apply(array, field, newValue);
261254
}
262255
else {
263-
if (path.length == 1) {
264-
mode.apply(array, field, newValue);
256+
if (ASTERISK.equals(field)) {
257+
array.forEach(value -> insertInto(value, mode, newValue, field, tail(path)));
265258
}
266-
else {
267-
if (isReference(field)) {
268-
return processRef(getReferencedValue(array, field), mode, newValue, field, tail(path));
269-
}
259+
else if (isReference(field)) {
260+
insertInto(getReferencedValue(array, field), mode, newValue, field, tail(path));
270261
}
271262
}
272263
return new Value(array);
@@ -279,45 +270,34 @@ private void removeNestedFrom(final Value value) {
279270
mode.apply(hash, field, newValue);
280271
}
281272
else {
282-
final String[] tail = tail(path);
283-
if (isReference(field)) {
284-
return processRef(hash.get(field), mode, newValue, field, tail);
285-
}
286273
if (!hash.containsField(field)) {
287274
hash.put(field, Value.newHash());
288275
}
289-
final Value value = hash.get(field);
290-
if (value != null) {
291-
// TODO: move impl into enum elements, here call only value.insert
292-
value.matchType()
293-
.ifArray(a -> new FixPath(tail).insertInto(a, mode, newValue))
294-
.ifHash(h -> new FixPath(tail).insertInto(h, mode, newValue))
295-
.orElseThrow();
296-
}
276+
insertInto(hash.get(field), mode, newValue, field, tail(path));
297277
}
298278

299279
return new Value(hash);
300280
}
301281

302-
private String[] tail(final String[] fields) {
303-
return Arrays.copyOfRange(fields, 1, fields.length);
304-
}
305-
306-
private Value processRef(final Value referencedValue, final InsertMode mode, final Value newValue, final String field,
282+
private Value insertInto(final Value value, final InsertMode mode, final Value newValue, final String field,
307283
final String[] tail) {
308-
if (referencedValue != null) {
284+
if (value != null) {
309285
final FixPath fixPath = new FixPath(tail);
310-
newValue.updatePathAddBase(referencedValue, field);
311-
return referencedValue.extractType((m, c) -> m
312-
.ifArray(a -> c.accept(fixPath.insertInto(referencedValue.asArray(), mode, newValue)))
313-
.ifHash(h -> c.accept(fixPath.insertInto(referencedValue.asHash(), mode, newValue)))
286+
newValue.updatePathAddBase(value, field);
287+
return value.extractType((m, c) -> m
288+
.ifArray(a -> c.accept(fixPath.insertInto(a, mode, newValue)))
289+
.ifHash(h -> c.accept(fixPath.insertInto(h, mode, newValue)))
314290
.orElseThrow());
315291
}
316292
else {
317-
throw new IllegalArgumentException("Using ref, but can't find: " + field + " in: " + referencedValue);
293+
throw new IllegalArgumentException("Using ref, but can't find: " + field + " in: " + value);
318294
}
319295
}
320296

297+
private String[] tail(final String[] fields) {
298+
return Arrays.copyOfRange(fields, 1, fields.length);
299+
}
300+
321301
private enum ReservedField {
322302
$append, $first, $last;
323303

@@ -348,10 +328,10 @@ private Value getReferencedValue(final Array array, final String field) {
348328
if (reservedField != null) {
349329
switch (reservedField) {
350330
case $first:
351-
referencedValue = array.get(0);
331+
referencedValue = getReferencedValue(array, "1");
352332
break;
353333
case $last:
354-
referencedValue = array.get(array.size() - 1);
334+
referencedValue = getReferencedValue(array, String.valueOf(array.size()));
355335
break;
356336
case $append:
357337
referencedValue = Value.newHash(); // TODO: append non-hash?

‎metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1844,10 +1844,34 @@ public void copyFieldToSubfieldOfArrayOfObjectsWithIndexExplicitAppend() {
18441844
}
18451845

18461846
@Test
1847-
@MetafixToDo("See https://github.com/metafacture/metafacture-fix/pull/205")
1848-
public void addFieldIntoArrayOfObjectsWithLastWildcardImplicitSkip() {
1847+
// We currently fail on unresolved references, see MetafixRecordTests#assertThrowsOnEmptyArray
1848+
public void addFieldIntoArrayOfObjectsWithLastWildcardMissingError() {
1849+
MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "Using ref, but can't find: $last in: null", () -> {
1850+
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
1851+
"add_field('animals[].$last.key', 'value')"
1852+
),
1853+
i -> {
1854+
i.startRecord("1");
1855+
i.startEntity("animals[]");
1856+
i.endEntity();
1857+
i.endRecord();
1858+
},
1859+
(o, f) -> {
1860+
o.get().startRecord("1");
1861+
o.get().startEntity("animals[]");
1862+
o.get().endEntity();
1863+
o.get().endRecord();
1864+
}
1865+
);
1866+
});
1867+
}
1868+
1869+
@Test
1870+
public void addFieldIntoArrayOfObjectsWithLastWildcardAllEmpty() {
18491871
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
1850-
"add_field('animals[].$last.key', 'value')"
1872+
"if exists('animals[].$last')",
1873+
" add_field('animals[].$last.key', 'value')",
1874+
"end"
18511875
),
18521876
i -> {
18531877
i.startRecord("1");
@@ -1865,8 +1889,7 @@ public void addFieldIntoArrayOfObjectsWithLastWildcardImplicitSkip() {
18651889
}
18661890

18671891
@Test
1868-
@MetafixToDo("See https://github.com/metafacture/metafacture-fix/pull/205")
1869-
public void addFieldIntoArrayOfObjectsWithLastWildcardExplicitSkip() {
1892+
public void addFieldIntoArrayOfObjectsWithLastWildcardLastEmpty() {
18701893
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
18711894
"if exists('animals[].$last')",
18721895
" add_field('animals[].$last.key', 'value')",
@@ -1875,12 +1898,36 @@ public void addFieldIntoArrayOfObjectsWithLastWildcardExplicitSkip() {
18751898
i -> {
18761899
i.startRecord("1");
18771900
i.startEntity("animals[]");
1901+
i.startEntity("1");
1902+
i.literal("name", "Jake");
1903+
i.literal("type", "dog");
1904+
i.endEntity();
1905+
i.startEntity("2");
1906+
i.literal("name", "Blacky");
1907+
i.literal("type", "bird");
1908+
i.endEntity();
1909+
i.endEntity();
1910+
i.endRecord();
1911+
i.startRecord("2");
1912+
i.startEntity("animals[]");
18781913
i.endEntity();
18791914
i.endRecord();
18801915
},
18811916
(o, f) -> {
18821917
o.get().startRecord("1");
18831918
o.get().startEntity("animals[]");
1919+
o.get().startEntity("1");
1920+
o.get().literal("name", "Jake");
1921+
o.get().literal("type", "dog");
1922+
o.get().endEntity();
1923+
o.get().startEntity("2");
1924+
o.get().literal("name", "Blacky");
1925+
o.get().literal("type", "bird");
1926+
o.get().literal("key", "value");
1927+
f.apply(2).endEntity();
1928+
o.get().endRecord();
1929+
o.get().startRecord("2");
1930+
o.get().startEntity("animals[]");
18841931
o.get().endEntity();
18851932
o.get().endRecord();
18861933
}
@@ -2369,7 +2416,6 @@ public void shouldSortFieldAndRemoveDuplicates() {
23692416
}
23702417

23712418
@Test
2372-
@MetafixToDo("See https://github.com/metafacture/metafacture-fix/issues/121")
23732419
public void shouldSortArrayFieldWithAsterisk() {
23742420
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
23752421
"sort_field('OTHERS[].*.dnimals[]')"
@@ -2558,7 +2604,6 @@ public void shouldSplitHashField() {
25582604
}
25592605

25602606
@Test
2561-
@MetafixToDo("See https://github.com/metafacture/metafacture-fix/issues/100 and https://github.com/metafacture/metafacture-fix/issues/121")
25622607
public void shouldSplitNestedField() {
25632608
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
25642609
"split_field('others[].*.tools', '--')"
@@ -2607,7 +2652,6 @@ public void shouldSumNumbers() {
26072652
}
26082653

26092654
@Test
2610-
@MetafixToDo("java.lang.IllegalStateException: Expected String, got Array; see https://github.com/metafacture/metafacture-fix/issues/121")
26112655
public void shouldSumArrayFieldWithAsterisk() {
26122656
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
26132657
"sum('OTHERS[].*.dumbers[]')"

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

Lines changed: 71 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ public void appendWithAsteriksWildcardAtTheEnd() {
676676
}
677677

678678
@Test
679-
// See https://github.com/metafacture/metafacture-fix/pull/142
679+
@MetafixToDo("See https://github.com/metafacture/metafacture-fix/pull/113")
680680
public void shouldCopyArrayFieldWithoutAsterisk() {
681681
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
682682
"set_array('TEST_TWO[]')",
@@ -691,23 +691,51 @@ public void shouldCopyArrayFieldWithoutAsterisk() {
691691
i.endEntity();
692692
i.endRecord();
693693
},
694-
o -> {
694+
(o, f) -> {
695695
o.get().startRecord("1");
696696
o.get().startEntity("test[]");
697697
o.get().literal("1", "One");
698698
o.get().literal("2", "Two");
699699
o.get().literal("3", "Three");
700700
o.get().endEntity();
701701
o.get().startEntity("TEST_TWO[]");
702+
o.get().startEntity("1");
702703
o.get().literal("1", "One");
703704
o.get().literal("2", "Two");
704705
o.get().literal("3", "Three");
705-
o.get().endEntity();
706+
f.apply(2).endEntity();
706707
o.get().endRecord();
707708
}
708709
);
709710
}
710711

712+
@Test
713+
@MetafixToDo("See https://github.com/metafacture/metafacture-fix/issues/113")
714+
public void copyFieldArrayOfObjectsAndListNewArrayOfObjectsAndMoveSubfield() {
715+
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
716+
"copy_field('author[]','creator[]')",
717+
"do list(path:'creator[]')",
718+
" move_field('name','label')",
719+
"end",
720+
"retain('creator[]')"),
721+
i -> {
722+
i.startRecord("1");
723+
i.startEntity("author[]");
724+
i.startEntity("1");
725+
i.literal("name", "A University");
726+
i.endEntity();
727+
i.endEntity();
728+
i.endRecord();
729+
}, (o, f) -> {
730+
o.get().startRecord("1");
731+
o.get().startEntity("creator[]");
732+
o.get().startEntity("1");
733+
o.get().literal("label", "A University");
734+
f.apply(2).endEntity();
735+
o.get().endRecord();
736+
});
737+
}
738+
711739
@Test
712740
// See https://github.com/metafacture/metafacture-fix/issues/121
713741
public void shouldCopyArrayFieldWithAsterisk() {
@@ -1070,26 +1098,28 @@ public void addFieldToObjectByIndexInIndexedArray() {
10701098

10711099
@Test
10721100
public void addFieldToFirstObjectMissing() {
1073-
assertThrowsOnEmptyRecord("$first");
1101+
assertThrowsOnEmptyArray("$first");
10741102
}
10751103

10761104
@Test
10771105
public void addFieldToLastObjectMissing() {
1078-
assertThrowsOnEmptyRecord("$last");
1106+
assertThrowsOnEmptyArray("$last");
10791107
}
10801108

10811109
@Test
10821110
public void addFieldToObjectByIndexMissing() {
1083-
assertThrowsOnEmptyRecord("2");
1111+
assertThrowsOnEmptyArray("2");
10841112
}
10851113

1086-
private void assertThrowsOnEmptyRecord(final String index) {
1114+
private void assertThrowsOnEmptyArray(final String index) {
10871115
MetafixTestHelpers.assertProcessException(IllegalArgumentException.class, "Using ref, but can't find: " + index + " in: null", () -> {
10881116
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
10891117
"add_field('animals[]." + index + ".kind','nice')"
10901118
),
10911119
i -> {
10921120
i.startRecord("1");
1121+
i.startEntity("animals[]");
1122+
i.endEntity();
10931123
i.endRecord();
10941124
},
10951125
o -> {
@@ -1099,7 +1129,6 @@ private void assertThrowsOnEmptyRecord(final String index) {
10991129
}
11001130

11011131
@Test
1102-
@MetafixToDo("See https://github.com/metafacture/metafacture-fix/issues/121")
11031132
public void shouldAddArraySubFieldWithAsterisk() {
11041133
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
11051134
"add_field('coll[].*.c', 'test')"
@@ -1144,7 +1173,7 @@ public void shouldAddArraySubFieldWithAsterisk() {
11441173
o.get().startEntity("4");
11451174
o.get().literal("a", "Crocodile");
11461175
o.get().literal("c", "test");
1147-
f.apply(3).endEntity();
1176+
f.apply(2).endEntity();
11481177
o.get().endRecord();
11491178
}
11501179
);
@@ -1685,6 +1714,39 @@ public void setArrayReplaceExisting() {
16851714
});
16861715
}
16871716

1717+
@Test
1718+
// See https://github.com/metafacture/metafacture-fix/issues/130
1719+
public void setArrayInArrayWithAsterisk() {
1720+
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
1721+
"set_array('foo[].*.test[]', 'test')"),
1722+
i -> {
1723+
i.startRecord("1");
1724+
i.startEntity("foo[]");
1725+
i.startEntity("1");
1726+
i.literal("id", "A");
1727+
i.endEntity();
1728+
i.startEntity("2");
1729+
i.literal("id", "B");
1730+
i.endEntity();
1731+
i.endEntity();
1732+
i.endRecord();
1733+
}, (o, f) -> {
1734+
o.get().startRecord("1");
1735+
o.get().startEntity("foo[]");
1736+
o.get().startEntity("1");
1737+
o.get().literal("id", "A");
1738+
o.get().startEntity("test[]");
1739+
o.get().literal("1", "test");
1740+
f.apply(2).endEntity();
1741+
o.get().startEntity("2");
1742+
o.get().literal("id", "B");
1743+
o.get().startEntity("test[]");
1744+
o.get().literal("1", "test");
1745+
f.apply(3).endEntity();
1746+
o.get().endRecord();
1747+
});
1748+
}
1749+
16881750
@Test
16891751
public void setHash() {
16901752
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(

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

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎metafix/src/test/resources/org/metafacture/metafix/integration/record/fromJson/toJson/add_fieldIntoArrayOfObjectsWithLastWildcard/test.fix

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
1-
add_field("animals[].$last.key", "value")
1+
if exists("animals[].$last")
2+
add_field("animals[].$last.key", "value")
3+
end

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

Lines changed: 0 additions & 1 deletion
This file was deleted.

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

Lines changed: 0 additions & 1 deletion
This file was deleted.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
{
2626
"author": [
2727
{
28-
"@type": "Person",
29-
"name": "Jürgen Böhner"
28+
"name": "Jürgen Böhner",
29+
"@type": "Person"
3030
}
3131
]
3232
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
See issue #132 may also be related to #113
1+
See issue #132

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

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)