Merge #560 from branch '543-fixBatchResetCreatesEmptyRecords' of gith…

…ub.com:metafacture/metafacture-core
metafacture · Oct 17, 2024 · bdb93ad · bdb93ad
2 parents f02d757 + d3a47ee
commit bdb93ad
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 18 deletions.
diff --git a/metafacture-biblio/src/main/java/org/metafacture/biblio/marc21/MarcXmlEncoder.java b/metafacture-biblio/src/main/java/org/metafacture/biblio/marc21/MarcXmlEncoder.java
@@ -212,7 +212,7 @@ public void literal(final String name, final String value) {
 
     @Override
     protected void onResetStream() {
-        pipe.resetStream();
+        encoder.onResetStream();
     }
 
     @Override
@@ -372,7 +372,9 @@ protected void onResetStream() {
 
         @Override
         protected void onCloseStream() {
-            writeFooter();
+            if (!atStreamStart) {
+                writeFooter();
+            }
             sendAndClearData();
         }
 

diff --git a/metafacture-biblio/src/test/java/org/metafacture/biblio/marc21/MarcXmlEncoderTest.java b/metafacture-biblio/src/test/java/org/metafacture/biblio/marc21/MarcXmlEncoderTest.java
@@ -51,6 +51,7 @@ public class MarcXmlEncoderTest {
     private static final String RECORD_ID = "92005291";
 
     private static StringBuilder resultCollector;
+    private static int resultCollectorsResetStreamCount;
     private static MarcXmlEncoder encoder;
 
     @Before
@@ -62,6 +63,11 @@ public void setUp() {
             public void process(final String obj) {
                 resultCollector.append(obj);
             }
+            @Override
+            public void resetStream() {
+                ++resultCollectorsResetStreamCount;
+            }
+
         });
         resultCollector = new StringBuilder();
     }
@@ -389,4 +395,26 @@ public void shouldNotEncodeNestedTypeLiteralAsAttribute() {
         assertEquals(expected, actual);
     }
 
+    @Test
+    public void issue543_shouldNotWriteFooterWhenRecordIsEmpty() {
+        encoder.closeStream();
+        String actual = resultCollector.toString();
+        assertTrue(actual.isEmpty());
+    }
+
+    @Test
+    public void issue543_shouldOnlyResetStreamOnce() {
+        resultCollectorsResetStreamCount = 0;
+        encoder.resetStream();
+        assertEquals(resultCollectorsResetStreamCount, 1);
+    }
+
+    @Test
+    public void issue543_shouldOnlyResetStreamOnceUsingWrapper() {
+        resultCollectorsResetStreamCount = 0;
+        encoder.setEnsureCorrectMarc21Xml(true);
+        encoder.resetStream();
+        assertEquals(resultCollectorsResetStreamCount, 1);
+    }
+
 }
diff --git a/metafacture-io/src/main/java/org/metafacture/io/ObjectFileWriter.java b/metafacture-io/src/main/java/org/metafacture/io/ObjectFileWriter.java
@@ -90,18 +90,21 @@ public void setCompression(final String compression) {
     @Override
     public void process(final T obj) {
         assert !closed;
-        try {
-            if (firstObject) {
-                getWriter().write(getHeader());
-                firstObject = false;
+        final String objStr = obj.toString();
+        if (!objStr.isEmpty()) {
+            try {
+                if (firstObject) {
+                    getWriter().write(getHeader());
+                    firstObject = false;
+                }
+                else {
+                    getWriter().write(getSeparator());
+                }
+                getWriter().write(objStr);
             }
-            else {
-                getWriter().write(getSeparator());
+            catch (final IOException e) {
+                throw new MetafactureException(e);
             }
-            getWriter().write(obj.toString());
-        }
-        catch (final IOException e) {
-            throw new MetafactureException(e);
         }
     }
 

diff --git a/metafacture-io/src/test/java/org/metafacture/io/ObjectFileWriterTest.java b/metafacture-io/src/test/java/org/metafacture/io/ObjectFileWriterTest.java
@@ -20,6 +20,12 @@
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeFalse;
 
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.metafacture.commons.ResourceUtil;
+
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
@@ -28,12 +34,6 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-import org.metafacture.commons.ResourceUtil;
-
 /**
  * Tests for class {@link ObjectFileWriter}.
  *
@@ -105,6 +105,14 @@ public void shouldIncrementCountOnResetBeforeStartingNewFile() throws IOExceptio
         assertTrue(new File(tempFolder.getRoot(), "test-1").exists());
     }
 
+    @Test
+    public void issue543_shouldResultEmptyWhenNothingIsProcessed() throws IOException {
+        writer.process("");
+        writer.closeStream();
+
+        assertOutput("");
+    }
+
     @Override
     protected ConfigurableObjectWriter<String> getWriter() {
         return writer;