From fdc8166ce571e2f47e2b5c70bdf7a793f7dc61b1 Mon Sep 17 00:00:00 2001 From: Jens Wille Date: Wed, 1 Dec 2021 13:17:19 +0100 Subject: [PATCH] Emit all underscore fields. (#63) Don't mark internal fields with underscore prefix; instead, make `_id` a virtual field. NOTE: Inlined `Value.merge()` in `Value.Hash.add()` in order to prevent subtle bugs with virtual fields in the future. --- .../java/org/metafacture/metafix/Metafix.java | 8 +-- .../java/org/metafacture/metafix/Record.java | 18 ++++++ .../java/org/metafacture/metafix/Value.java | 17 +++--- .../metafix/MetafixRecordTest.java | 61 +++++++++++++++++-- 4 files changed, 86 insertions(+), 18 deletions(-) diff --git a/metafix/src/main/java/org/metafacture/metafix/Metafix.java b/metafix/src/main/java/org/metafacture/metafix/Metafix.java index e3daed4d..315ce701 100644 --- a/metafix/src/main/java/org/metafacture/metafix/Metafix.java +++ b/metafix/src/main/java/org/metafacture/metafix/Metafix.java @@ -113,6 +113,7 @@ private void buildPipeline(final Reader fixDef, final Map theVar @Override public void startRecord(final String identifier) { currentRecord = new Record(); + currentRecord.putVirtualField(StandardEventNames.ID, new Value(identifier)); LOG.debug("Start record: {}", identifier); flattener.startRecord(identifier); entityCountStack.clear(); @@ -120,7 +121,6 @@ public void startRecord(final String identifier) { entityCountStack.add(Integer.valueOf(entityCount)); recordIdentifier = identifier; entities = new ArrayList<>(); - literal(StandardEventNames.ID, identifier); } @Override @@ -136,11 +136,7 @@ public void endRecord() { if (!currentRecord.getReject()) { outputStreamReceiver.startRecord(recordIdentifier); LOG.debug("Sending results to {}", outputStreamReceiver); - currentRecord.forEach((f, v) -> { - if (!f.startsWith("_")) { - emit(f, v); - } - }); + currentRecord.forEach(this::emit); outputStreamReceiver.endRecord(); } } diff --git a/metafix/src/main/java/org/metafacture/metafix/Record.java b/metafix/src/main/java/org/metafacture/metafix/Record.java index ce597d6a..b0805a18 100644 --- a/metafix/src/main/java/org/metafacture/metafix/Record.java +++ b/metafix/src/main/java/org/metafacture/metafix/Record.java @@ -115,6 +115,24 @@ public Value get(final String field) { return containsField(field) ? super.get(field) : virtualFields.get(field); } + /** + * {@link #put(String, Value) Adds} a field/value pair to this record. Turns + * virtual fields into regular metadata fields if they're not already + * {@link #containsField(String) present}. + * + * @param field the field name + * @param newValue the new metadata value + */ + @Override + public void add(final String field, final Value newValue) { + if (containsField(field)) { + super.add(field, newValue); + } + else { + put(field, newValue); + } + } + /** * Retains only the given field/value pairs in this record. Turns * virtual fields into regular metadata fields if they're not already diff --git a/metafix/src/main/java/org/metafacture/metafix/Value.java b/metafix/src/main/java/org/metafacture/metafix/Value.java index 831b54d0..e1d44859 100644 --- a/metafix/src/main/java/org/metafacture/metafix/Value.java +++ b/metafix/src/main/java/org/metafacture/metafix/Value.java @@ -206,10 +206,6 @@ public Value asList(final Consumer consumer) { } } - public Value merge(final Value value) { - return asList(a1 -> value.asList(a2 -> a2.forEach(a1::add))); - } - @Override public String toString() { final String result; @@ -266,9 +262,7 @@ void apply(final Hash hash, final String field, final String value) { APPEND { @Override void apply(final Hash hash, final String field, final String value) { - final Value oldValue = hash.get(field); - final Value newValue = new Value(value); - hash.put(field, oldValue == null ? newValue : oldValue.merge(newValue)); + hash.add(field, new Value(value)); } }; @@ -561,9 +555,16 @@ public void addAll(final Hash hash) { hash.forEach(this::add); } + /** + * {@link #put(String, Value) Adds} a field/value pair to this hash, + * potentially merging with an existing value. + * + * @param field the field name + * @param newValue the new metadata value + */ public void add(final String field, final Value newValue) { final Value oldValue = get(field); - put(field, oldValue == null ? newValue : oldValue.merge(newValue)); + put(field, oldValue == null ? newValue : oldValue.asList(a1 -> newValue.asList(a2 -> a2.forEach(a1::add)))); } public Value insert(final InsertMode mode, final String fieldPath, final String newValue) { diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java b/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java index 8cf6c446..2d277f4b 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixRecordTest.java @@ -70,17 +70,70 @@ public void entitiesPassThrough() { } @Test - public void internalIdUsage() { + public void shouldNotEmitVirtualFieldsByDefault() { MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( - "copy_field('_id', id)"), + "vacuum()" + ), i -> { i.startRecord("1"); i.endRecord(); - }, o -> { + }, + o -> { + o.get().startRecord("1"); + o.get().endRecord(); + } + ); + } + + @Test + public void shouldEmitVirtualFieldsWhenRetained() { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "retain('_id')" + ), + i -> { + i.startRecord("1"); + i.endRecord(); + }, + o -> { + o.get().startRecord("1"); + o.get().literal("_id", "1"); + o.get().endRecord(); + } + ); + } + + @Test + public void shouldEmitVirtualFieldsWhenCopied() { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "copy_field('_id', id)" + ), + i -> { + i.startRecord("1"); + i.endRecord(); + }, + o -> { o.get().startRecord("1"); o.get().literal("id", "1"); o.get().endRecord(); - }); + } + ); + } + + @Test + public void shouldEmitVirtualFieldsWhenAdded() { + MetafixTestHelpers.assertFix(streamReceiver, Arrays.asList( + "add_field('_id', 'id')" + ), + i -> { + i.startRecord("1"); + i.endRecord(); + }, + o -> { + o.get().startRecord("1"); + o.get().literal("_id", "id"); + o.get().endRecord(); + } + ); } @Test