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.
metafacture · Dec 1, 2021 · fdc8166 · fdc8166
1 parent bfafecc
commit fdc8166
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 18 deletions.
diff --git a/metafix/src/main/java/org/metafacture/metafix/Metafix.java b/metafix/src/main/java/org/metafacture/metafix/Metafix.java
@@ -113,14 +113,14 @@ private void buildPipeline(final Reader fixDef, final Map<String, String> 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();
         entityCount = 0;
         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
@@ -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
+     * <i>virtual</i> 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
      * <i>virtual</i> 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
@@ -206,10 +206,6 @@ public Value asList(final Consumer<Array> 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
@@ -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