From 04525d938ea509e124f64b2e1eca783d71bd6317 Mon Sep 17 00:00:00 2001 From: Pascal Christoph Date: Fri, 27 Sep 2024 14:48:08 +0200 Subject: [PATCH 1/3] Don't write footer when record is empty (#543) --- .../java/org/metafacture/biblio/marc21/MarcXmlEncoder.java | 4 +++- .../org/metafacture/biblio/marc21/MarcXmlEncoderTest.java | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) 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 index feecd64e..e754bb70 100644 --- a/metafacture-biblio/src/main/java/org/metafacture/biblio/marc21/MarcXmlEncoder.java +++ b/metafacture-biblio/src/main/java/org/metafacture/biblio/marc21/MarcXmlEncoder.java @@ -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 index 8caac8d9..5b979e5a 100644 --- a/metafacture-biblio/src/test/java/org/metafacture/biblio/marc21/MarcXmlEncoderTest.java +++ b/metafacture-biblio/src/test/java/org/metafacture/biblio/marc21/MarcXmlEncoderTest.java @@ -389,4 +389,11 @@ public void shouldNotEncodeNestedTypeLiteralAsAttribute() { assertEquals(expected, actual); } + @Test + public void issue543_shouldNotWriteFooterWhenRecordIsEmpty() { + encoder.closeStream(); + String actual = resultCollector.toString(); + assertTrue(actual.isEmpty()); + } + } From 07e7f0c56cf5e193852e309f6631e430f13165f8 Mon Sep 17 00:00:00 2001 From: Pascal Christoph Date: Fri, 27 Sep 2024 15:03:19 +0200 Subject: [PATCH 2/3] Reset once, not twice, when resetted once (#543) By not calling the pipe (aka wrapper) but the receiver directly the stream is only once resetted when called once. (In conjunction with ObjectFileWriter and StreamBatchResetter this bug had resulted in as many empty files as non-empty ones.) --- .../biblio/marc21/MarcXmlEncoder.java | 2 +- .../biblio/marc21/MarcXmlEncoderTest.java | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) 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 index e754bb70..3113a324 100644 --- 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 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 index 5b979e5a..2e1c8df2 100644 --- 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(); } @@ -396,4 +402,19 @@ public void issue543_shouldNotWriteFooterWhenRecordIsEmpty() { 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); + } + } From d3a47eeb70166d1ffe66ffd1cf7f3926dbc89c4d Mon Sep 17 00:00:00 2001 From: Pascal Christoph Date: Fri, 27 Sep 2024 15:56:40 +0200 Subject: [PATCH 3/3] Don't insert a line break when nothing is processed (#543) --- .../org/metafacture/io/ObjectFileWriter.java | 23 +++++++++++-------- .../metafacture/io/ObjectFileWriterTest.java | 20 +++++++++++----- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/metafacture-io/src/main/java/org/metafacture/io/ObjectFileWriter.java b/metafacture-io/src/main/java/org/metafacture/io/ObjectFileWriter.java index f7462982..36d45426 100644 --- 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 index 64d877fb..77d9ad61 100644 --- 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 getWriter() { return writer;