diff --git a/metafix/src/main/java/org/metafacture/metafix/FixPath.java b/metafix/src/main/java/org/metafacture/metafix/FixPath.java index 4e2ba0e3..8bb956c7 100644 --- a/metafix/src/main/java/org/metafacture/metafix/FixPath.java +++ b/metafix/src/main/java/org/metafacture/metafix/FixPath.java @@ -45,16 +45,20 @@ private FixPath(final String[] path) { } /*package-private*/ Value findIn(final Hash hash) { + return findIn(hash, false); + } + + /*package-private*/ Value findIn(final Hash hash, final boolean enforceStringValue) { final String currentSegment = path[0]; final FixPath remainingPath = new FixPath(tail(path)); if (currentSegment.equals(ASTERISK)) { // TODO: search in all elements of value.asHash()? - return remainingPath.findIn(hash); + return remainingPath.findIn(hash, enforceStringValue); } - final Value value = hash.get(currentSegment); + final Value value = hash.get(currentSegment, enforceStringValue && path.length == 1); return value == null || path.length == 1 ? value : value.extractType((m, c) -> m .ifArray(a -> c.accept(remainingPath.findIn(a))) - .ifHash(h -> c.accept(remainingPath.findIn(h))) + .ifHash(h -> c.accept(remainingPath.findIn(h, enforceStringValue))) .orElseThrow() ); } @@ -118,17 +122,6 @@ public String toString() { // try to replace this with consistent usage of Value#getPath // (e.g. take care of handling repeated fields and their paths) - /*package-private*/ void throwIfNonString(final Value value) { - final boolean isNonString = value != null && - // basic idea: path is only set on literals/strings - value.getPath() == null && - // but with wildcards, we might still point to literals/strings - !hasWildcard(); - if (isNonString) { - value.asString(); - } - } - /*package-private*/ FixPath to(final Value value, final int i) { final FixPath result; // One *, no matching path: replace with index of current result diff --git a/metafix/src/main/java/org/metafacture/metafix/Record.java b/metafix/src/main/java/org/metafacture/metafix/Record.java index 021da543..275c9964 100644 --- a/metafix/src/main/java/org/metafacture/metafix/Record.java +++ b/metafix/src/main/java/org/metafacture/metafix/Record.java @@ -200,8 +200,7 @@ public void retainFields(final Collection fields) { */ public void transform(final String field, final UnaryOperator operator) { final FixPath findPath = new FixPath(field); - final Value found = findPath.findIn(this); - findPath.throwIfNonString(found); + final Value found = findPath.findIn(this, true); Value.asList(found, results -> { final Deque toDelete = new LinkedList<>(); for (int i = 0; i < results.size(); ++i) { diff --git a/metafix/src/main/java/org/metafacture/metafix/Value.java b/metafix/src/main/java/org/metafacture/metafix/Value.java index 51cc0409..57682d1c 100644 --- a/metafix/src/main/java/org/metafacture/metafix/Value.java +++ b/metafix/src/main/java/org/metafacture/metafix/Value.java @@ -555,8 +555,18 @@ public void replace(final String field, final Value value) { * @return the metadata value */ public Value get(final String field) { + return get(field, false); + } + + /*package-private*/ Value get(final String field, final boolean enforceStringValue) { // TODO use Type.String etc.? // TODO: special treatment (only) for exact matches? - final List list = findFields(field).map(this::getField).collect(Collectors.toList()); + final List list = findFields(field).map(actualField -> { + final Value value = getField(actualField); + if (enforceStringValue) { + value.asString(); + } + return value; + }).collect(Collectors.toList()); return list.isEmpty() ? null : list.size() == 1 ? list.get(0) : newArray(a -> list.forEach(v -> v.matchType() .ifArray(b -> b.forEach(a::add)) .orElse(a::add))); diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java b/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java index 019d94a9..c111f82b 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixMethodTest.java @@ -305,10 +305,32 @@ public void shouldNotTrimRepeatedField() { ); } + @Test + public void shouldNotTrimIndexedArray() { + MetafixTestHelpers.assertExecutionException(IllegalStateException.class, "Expected String, got Hash", () -> + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "trim('data.title')" + ), + i -> { + i.startRecord("1"); + i.startEntity("data"); + i.startEntity("title"); + i.literal("1", " marc "); + i.literal("2", " json "); + i.endEntity(); + i.endEntity(); + i.endRecord(); + }, + o -> { + } + ) + ); + } + @Test // See https://github.com/metafacture/metafacture-fix/pull/133 public void shouldNotTrimStringInImplicitArrayOfHashes() { - MetafixTestHelpers.assertExecutionException(IllegalStateException.class, "Expected String, got Array", () -> + MetafixTestHelpers.assertExecutionException(NumberFormatException.class, "For input string: \"title\"", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( "trim('data.title')" ), @@ -550,6 +572,39 @@ public void wildcard() { }); } + @Test + public void wildcardNested() { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "trim('work.title-?')"), + i -> { + i.startRecord("1"); + i.endRecord(); + + i.startRecord("2"); + i.startEntity("work"); + i.literal("title-1", " marc "); + i.literal("title-2", " json "); + i.endEntity(); + i.endRecord(); + + i.startRecord("3"); + i.endRecord(); + }, o -> { + o.get().startRecord("1"); + o.get().endRecord(); + + o.get().startRecord("2"); + o.get().startEntity("work"); + o.get().literal("title-1", "marc"); + o.get().literal("title-2", "json"); + o.get().endEntity(); + o.get().endRecord(); + + o.get().startRecord("3"); + o.get().endRecord(); + }); + } + @Test public void characterClass() { MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( @@ -1171,7 +1226,6 @@ public void shouldNotPrependValueToArray() { } @Test - @MetafixToDo("See https://github.com/metafacture/metafacture-fix/pull/170") public void shouldNotPrependValueToArrayWithWildcard() { MetafixTestHelpers.assertExecutionException(IllegalStateException.class, "Expected String, got Array", () -> MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( @@ -1192,6 +1246,29 @@ public void shouldNotPrependValueToArrayWithWildcard() { ); } + @Test + public void shouldNotPrependValueToArrayWithWildcardNested() { + MetafixTestHelpers.assertExecutionException(IllegalStateException.class, "Expected String, got Array", () -> + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "prepend('some.animal?[]', 'the first one')" + ), + i -> { + i.startRecord("1"); + i.startEntity("some"); + i.startEntity("animals[]"); + i.literal("1", "dog"); + i.literal("2", "cat"); + i.literal("3", "zebra"); + i.endEntity(); + i.endEntity(); + i.endRecord(); + }, + o -> { + } + ) + ); + } + @Test public void shouldReplaceAllRegexes() { MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList(