Skip to content

Commit c062322

Browse files
blackwinterblackwinter
authored
Merge pull request #704 from metafacture/631-changeSetFunctionsToNotCreateIntermediateStructures
Change `set_*` functions to not create intermediate structures.
2 parents d835ff4 + d433295 commit c062322

File tree

32 files changed

+646
-54
lines changed

32 files changed

+646
-54
lines changed

‎README.md

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -642,21 +642,41 @@ retain("<sourceField_1>"[, ...])
642642
643643
##### `set_array`
644644
645-
_Currently alias for [`add_array`](#add_array)._
645+
Creates a new array (with optional values), provided that the intermediate structures (i.e. parent fields) exist. To create any missing intermediate structures, use [`add_array`](#add_array) instead.
646646
647-
We advise you to use [`add_array`](#add_array) instead of `set_array` due to changing behaviour in an upcoming release. For more information see: [#309](https://github.com/metafacture/metafacture-fix/issues/309)
647+
```perl
648+
set_array("<targetFieldName>")
649+
set_array("<targetFieldName>", "<value_1>"[, ...])
650+
```
651+
652+
[Example in Playground](https://metafacture.org/playground/?example=set_array)
653+
654+
[Java Code](https://github.com/search?type=code&q=repo:metafacture/metafacture-core+path:FixMethod.java+"+set_array+{")
648655
649656
##### `set_field`
650657
651-
_Currently alias for [`add_field`](#add_field)._
658+
Creates a field with a defined value, provided that the intermediate structures (i.e. parent fields) exist. To create any missing intermediate structures, use [`add_field`](#add_field) instead.
659+
660+
```perl
661+
set_field("<targetFieldName>", "<fieldValue>")
662+
```
663+
664+
[Example in Playground](https://metafacture.org/playground/?example=set_field)
652665
653-
We advise you to use [`add_field`](#add_field) instead of `set_field` due to changing behaviour in an upcoming release. For more information see: [#309](https://github.com/metafacture/metafacture-fix/issues/309)
666+
[Java Code](https://github.com/search?type=code&q=repo:metafacture/metafacture-core+path:FixMethod.java+"+set_field+{")
654667
655668
##### `set_hash`
656669
657-
_Currently alias for [`add_hash`](#add_hash)._
670+
Creates a new hash (with optional values), provided that the intermediate structures (i.e. parent fields) exist. To create any missing intermediate structures, use [`add_hash`](#add_hash) instead.
671+
672+
```perl
673+
set_hash("<targetFieldName>")
674+
set_hash("<targetFieldName>", "subfieldName": "<subfieldValue>"[, ...])
675+
```
676+
677+
[Example in Playground](https://metafacture.org/playground/?example=set_hash)
658678
659-
We advise you to use [`add_hash`](#add_hash) instead of `set_hash` due to changing behaviour in an upcoming release. For more information see: [#309](https://github.com/metafacture/metafacture-fix/issues/309)
679+
[Java Code](https://github.com/search?type=code&q=repo:metafacture/metafacture-core+path:FixMethod.java+"+set_hash+{")
660680
661681
##### `timestamp`
662682

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ public void apply(final Metafix metafix, final Record record, final List<String>
170170
final String field = params.get(0);
171171
final Value newValue = newArray(params.subList(1, params.size()).stream().map(Value::new));
172172
record.set(field, newValue);
173-
newValue.asArray().forEach(value -> value.withPathSet(newValue.getPath() + "." + value.getPath()));
173+
newValue.asArray().forEach(value -> value.withPathSet(newValue.getPath() + Value.FIELD_PATH_SEPARATOR + value.getPath()));
174174
}
175175
},
176176
add_field {
@@ -408,19 +408,25 @@ public void apply(final Metafix metafix, final Record record, final List<String>
408408
set_array {
409409
@Override
410410
public void apply(final Metafix metafix, final Record record, final List<String> params, final Map<String, String> options) {
411-
add_array.apply(metafix, record, params, options);
411+
if (parentFieldExists(record, params.get(0))) {
412+
add_array.apply(metafix, record, params, options);
413+
}
412414
}
413415
},
414416
set_field {
415417
@Override
416418
public void apply(final Metafix metafix, final Record record, final List<String> params, final Map<String, String> options) {
417-
add_field.apply(metafix, record, params, options);
419+
if (parentFieldExists(record, params.get(0))) {
420+
add_field.apply(metafix, record, params, options);
421+
}
418422
}
419423
},
420424
set_hash {
421425
@Override
422426
public void apply(final Metafix metafix, final Record record, final List<String> params, final Map<String, String> options) {
423-
add_hash.apply(metafix, record, params, options);
427+
if (parentFieldExists(record, params.get(0))) {
428+
add_hash.apply(metafix, record, params, options);
429+
}
424430
}
425431
},
426432
timestamp {

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

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,22 @@
2929
* With all get/set/update/create/delete logic collected here.
3030
*
3131
* @author Fabian Steeg (fsteeg)
32-
*
3332
*/
34-
/*package-private*/ class FixPath {
33+
public class FixPath {
3534

3635
private static final String ASTERISK = "*";
36+
37+
private static final String INDEX_SUBPATH_PATTERN = Value.FIELD_PATH_SEPARATOR_PATTERN + "\\d" + Value.FIELD_PATH_SEPARATOR_PATTERN;
38+
private static final String ASTERISK_SUBPATH = Value.FIELD_PATH_SEPARATOR + ASTERISK + Value.FIELD_PATH_SEPARATOR;
39+
3740
private String[] path;
3841

39-
/*package-private*/ FixPath(final String path) {
42+
/**
43+
* Creates an instance of {@link FixPath} based on the given path string.
44+
*
45+
* @param path the path string
46+
*/
47+
public FixPath(final String path) {
4048
this(Value.split(path));
4149
}
4250

@@ -111,7 +119,7 @@ private Value findInValue(final Value value, final String[] p) {
111119

112120
@Override
113121
public String toString() {
114-
return String.join(".", path);
122+
return String.join(Value.FIELD_PATH_SEPARATOR, path);
115123
}
116124

117125
/*package-private*/ int size() {
@@ -141,7 +149,9 @@ else if (value.getPath() != null && hasWildcard()) {
141149
}
142150

143151
private boolean matches(final String thatPath) {
144-
return thatPath != null && thatPath.replaceAll("\\.\\d+\\.", ".*.").equals(String.join(".", this.path));
152+
return thatPath != null && thatPath
153+
.replaceAll(INDEX_SUBPATH_PATTERN, ASTERISK_SUBPATH)
154+
.equals(String.join(Value.FIELD_PATH_SEPARATOR, this.path));
145155
}
146156

147157
private String[] replaceInPath(final String find, final int i) {
@@ -299,9 +309,29 @@ private Value insertInto(final Value value, final InsertMode mode, final Value n
299309
}
300310
}
301311

312+
/**
313+
* Checks whether the given field is adding to an array.
314+
*
315+
* @param field the field
316+
*
317+
* @return true if the given field is adding to an array
318+
*/
319+
public static boolean isAddingToArray(final String field) {
320+
return field.equals(ReservedField.$prepend.name()) || field.equals(ReservedField.$append.name());
321+
}
322+
323+
/**
324+
* Checks whether any of the path segments are adding to an array.
325+
*
326+
* @return true if any of the path segments are adding to an array
327+
*/
328+
public boolean isAddingToArray() {
329+
return Arrays.stream(path).anyMatch(FixPath::isAddingToArray);
330+
}
331+
302332
private Value getContainerValue(final Hash hash, final String field, final String newPath, final String nextField) {
303333
Value result = hash.get(field);
304-
final boolean isAddingToArray = nextField.equals(ReservedField.$prepend.name()) || nextField.equals(ReservedField.$append.name());
334+
final boolean isAddingToArray = isAddingToArray(nextField);
305335
if (result == null) {
306336
result = (isAddingToArray ? Value.newArray() : Value.newHash()).withPathSet(newPath);
307337
hash.put(field, result);
@@ -320,6 +350,15 @@ private String[] tail(final String[] fields) {
320350
return Arrays.copyOfRange(fields, 1, fields.length);
321351
}
322352

353+
/**
354+
* Creates an instance of {@link FixPath} based on the parent path.
355+
*
356+
* @return an instance of {@link FixPath}, or {@code null} if already at the top level
357+
*/
358+
public FixPath getParentPath() {
359+
return path.length > 1 ? new FixPath(Arrays.copyOfRange(path, 0, path.length - 1)) : null;
360+
}
361+
323362
private enum ReservedField {
324363
$prepend, $append, $first, $last;
325364

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@
5252
*/
5353
public class Value implements JsonValue { // checkstyle-disable-line ClassDataAbstractionCoupling
5454

55-
private static final String FIELD_PATH_SEPARATOR = "\\.";
55+
public static final String FIELD_PATH_SEPARATOR = ".";
56+
/*package-private*/ static final String FIELD_PATH_SEPARATOR_PATTERN = Pattern.quote(FIELD_PATH_SEPARATOR);
5657

5758
private final Array array;
5859
private final Hash hash;
@@ -409,7 +410,7 @@ public void toJson(final JsonGenerator jsonGenerator) {
409410
}
410411

411412
/*package-private*/ static String[] split(final String fieldPath) {
412-
return fieldPath.split(FIELD_PATH_SEPARATOR);
413+
return fieldPath.split(FIELD_PATH_SEPARATOR_PATTERN);
413414
}
414415

415416
/**
@@ -431,7 +432,7 @@ private Value withPathAppend(final int i) {
431432
}
432433

433434
private Value withPathAppend(final String field) {
434-
return withPathSet(path == null || path.isEmpty() ? field : path + "." + field);
435+
return withPathSet(path == null || path.isEmpty() ? field : path + FIELD_PATH_SEPARATOR + field);
435436
}
436437

437438
/*package-private*/ Value copy() {
@@ -574,7 +575,7 @@ protected <T> Map<T, Collection<String>> retainFields(final Collection<String> f
574575
final Map<T, Collection<String>> retainFields = new HashMap<>();
575576

576577
fields.forEach(p -> {
577-
final String[] parts = p.split(FIELD_PATH_SEPARATOR, 2);
578+
final String[] parts = p.split(FIELD_PATH_SEPARATOR_PATTERN, 2);
578579

579580
function.apply(parts[0]).forEach(f -> {
580581
final Collection<String> retainNested = retainFields.computeIfAbsent(f, k -> new HashSet<>());

‎metafix/src/main/java/org/metafacture/metafix/api/FixFunction.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import org.metafacture.framework.StandardEventNames;
2020
import org.metafacture.io.ObjectWriter;
21+
import org.metafacture.metafix.FixPath;
2122
import org.metafacture.metafix.Metafix;
2223
import org.metafacture.metafix.Record;
2324
import org.metafacture.metafix.Value;
@@ -194,4 +195,17 @@ default Stream<Value> flatten(final Stream<Value> stream) {
194195
));
195196
}
196197

198+
/**
199+
* Checks whether the given field's parent field exists in the record.
200+
*
201+
* @param record the record
202+
* @param field the field
203+
*
204+
* @return true if the given field's parent field exists in the record
205+
*/
206+
default boolean parentFieldExists(final Record record, final String field) {
207+
final FixPath parentPath = new FixPath(field).getParentPath();
208+
return parentPath == null || !parentPath.isAddingToArray() && record.containsPath(parentPath.toString());
209+
}
210+
197211
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,7 @@ public void shouldDoListAsWithMultipleListsOfDifferentSizes() {
810810
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
811811
"set_array('sourceOrga[]')",
812812
"do list_as(orgId: 'ccm:university[]', orgName: 'ccm:university_DISPLAYNAME[]', orgLoc: 'ccm:university_LOCATION[]')",
813-
" set_hash('sourceOrga[].$append')",
813+
" add_hash('sourceOrga[].$append')",
814814
" copy_field(orgId, 'sourceOrga[].$last.id')",
815815
" copy_field(orgName, 'sourceOrga[].$last.name')",
816816
" copy_field(orgLoc, 'sourceOrga[].$last.location')",

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -994,9 +994,9 @@ public void shouldLookupInCopiedNestedArraysCreatedWithPrepend() {
994994
private void shouldLookupInCopiedNestedArraysCreatedWith(final String reservedField) {
995995
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
996996
"put_map('rswk-indicator', s: 'SubjectHeading')",
997-
"set_array('subject[]')",
998-
"set_array('subject[]." + reservedField + ".componentList[]')",
999-
"set_array('subject[].$last.componentList[]." + reservedField + ".type[]')",
997+
"add_array('subject[]')",
998+
"add_array('subject[]." + reservedField + ".componentList[]')",
999+
"add_array('subject[].$last.componentList[]." + reservedField + ".type[]')",
10001000
"do list(path: 'D', 'var': '$i')",
10011001
" copy_field('$i', 'subject[].$last.componentList[].$last.type[]." + reservedField + "')",
10021002
"end",

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2818,11 +2818,9 @@ public void addFieldWithReplaceAllArray() {
28182818
private void addFieldWithReplaceAllArray(final boolean replaceAll) {
28192819
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
28202820
"do list(path: 'contribution[]', 'var': '$i')",
2821-
" set_array('$i.agent.altLabel[]')",
28222821
" add_field('$i.agent.altLabel[].$append', 'contribution')",
28232822
"end",
28242823
"do list(path: 'subject[]', 'var': '$i')",
2825-
" set_array('$i.altLabel[]')",
28262824
" add_field('$i.altLabel[].$append', 'subject')",
28272825
"end",
28282826
replaceAll ? "replace_all('contribution[].*.agent.altLabel[].*', 't', '')" : "",
@@ -2875,10 +2873,10 @@ public void setArrayWithReplaceAll() {
28752873
private void setArrayWithReplaceAll(final boolean replaceAll) {
28762874
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
28772875
"do list(path: 'contribution[]', 'var': '$i')",
2878-
" set_array('$i.agent.altLabel[]', 'contribution')",
2876+
" add_array('$i.agent.altLabel[]', 'contribution')",
28792877
"end",
28802878
"do list(path: 'subject[]', 'var': '$i')",
2881-
" set_array('$i.altLabel[]', 'subject')",
2879+
" add_array('$i.altLabel[]', 'subject')",
28822880
"end",
28832881
replaceAll ? "replace_all('contribution[].*.agent.altLabel[].*', 't', '')" : "",
28842882
replaceAll ? "replace_all('subject[].*.altLabel[].*', 't', '')" : ""
@@ -2930,11 +2928,9 @@ public void copyFieldWithReplaceAllArray() {
29302928
private void copyFieldWithReplaceAllArray(final boolean replaceAll) {
29312929
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
29322930
"do list(path: 'contribution[]', 'var': '$i')",
2933-
" set_array('$i.agent.altLabel[]')",
29342931
" copy_field('$i.label', '$i.agent.altLabel[].$append')",
29352932
"end",
29362933
"do list(path: 'subject[]', 'var': '$i')",
2937-
" set_array('$i.altLabel[]')",
29382934
" copy_field('$i.label', '$i.altLabel[].$append')",
29392935
"end",
29402936
replaceAll ? "replace_all('contribution[].*.agent.altLabel[].*', 't', '')" : "",
@@ -2987,11 +2983,9 @@ public void pasteWithReplaceAll() {
29872983
private void pasteWithReplaceAll(final boolean replaceAll) {
29882984
MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(
29892985
"do list(path: 'contribution[]', 'var': '$i')",
2990-
" set_array('$i.agent.altLabel[]')",
29912986
" paste('$i.agent.altLabel[].$append', '$i.label', '~!')",
29922987
"end",
29932988
"do list(path: 'subject[]', 'var': '$i')",
2994-
" set_array('$i.altLabel[]')",
29952989
" paste('$i.altLabel[].$append', '$i.label', '~!')",
29962990
"end",
29972991
replaceAll ? "replace_all('contribution[].*.agent.altLabel[].*', ' !', '')" : "",

0 commit comments

Comments
 (0)