From a9d569b60355c9835721de03e34e62d6504a59c6 Mon Sep 17 00:00:00 2001 From: Jens Wille Date: Thu, 11 Jan 2024 14:53:25 +0100 Subject: [PATCH 1/3] Extract helpers from FileOpenerTest. (#514) --- .../io/FileOpenerCompressionTest.java | 2 +- .../org/metafacture/io/FileOpenerTest.java | 23 +------ .../java/org/metafacture/io/TestHelpers.java | 63 +++++++++++++++++++ 3 files changed, 66 insertions(+), 22 deletions(-) create mode 100644 metafacture-io/src/test/java/org/metafacture/io/TestHelpers.java diff --git a/metafacture-io/src/test/java/org/metafacture/io/FileOpenerCompressionTest.java b/metafacture-io/src/test/java/org/metafacture/io/FileOpenerCompressionTest.java index 83489531a..8751b7194 100644 --- a/metafacture-io/src/test/java/org/metafacture/io/FileOpenerCompressionTest.java +++ b/metafacture-io/src/test/java/org/metafacture/io/FileOpenerCompressionTest.java @@ -93,7 +93,7 @@ public void testOpenCompressedFiles() throws IOException { Files.copy(in, file.toPath(), StandardCopyOption.REPLACE_EXISTING); } - FileOpenerTest.assertData(receiver, DATA, file, o -> o.setCompression(compression)); + TestHelpers.assertFile(receiver, DATA, file, o -> o.setCompression(compression)); } } diff --git a/metafacture-io/src/test/java/org/metafacture/io/FileOpenerTest.java b/metafacture-io/src/test/java/org/metafacture/io/FileOpenerTest.java index a59ba5db0..763ca58e7 100644 --- a/metafacture-io/src/test/java/org/metafacture/io/FileOpenerTest.java +++ b/metafacture-io/src/test/java/org/metafacture/io/FileOpenerTest.java @@ -25,7 +25,6 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -40,7 +39,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.StandardCopyOption; -import java.util.function.Consumer; /** * Tests for class {@link FileOpener}. @@ -66,7 +64,7 @@ public void testUtf8IsDefaultEncoding() throws IOException { Assume.assumeFalse("Default encoding is UTF-8: It is not possible to test whether FileOpener sets " + "the encoding to UTF-8 correctly.", StandardCharsets.UTF_8.equals(Charset.defaultCharset())); - assertData(receiver, DATA, createTestFile(), null); + TestHelpers.assertFile(receiver, DATA, createTestFile(), null); } @Test @@ -93,27 +91,10 @@ private void testDecompressConcatenated(final boolean decompressConcatenated) th final String data = sb.toString(); Assert.assertTrue(data.length() + " > " + maxBytes, data.length() > maxBytes); - assertData(receiver, decompressConcatenated ? data : data.substring(0, maxBytes), + TestHelpers.assertFile(receiver, decompressConcatenated ? data : data.substring(0, maxBytes), copyResourceToTempFile("compressed-large.txt.bgzf"), o -> o.setDecompressConcatenated(decompressConcatenated)); } - /*package-private*/ static void assertData(final ObjectReceiver receiver, final String expected, final File file, final Consumer consumer) { - final StringBuilder sb = new StringBuilder(); - Mockito.doAnswer(i -> sb.append(ResourceUtil.readAll(i.getArgument(0)))).when(receiver).process(Mockito.any(Reader.class)); - - final FileOpener opener = new FileOpener(); - if (consumer != null) { - consumer.accept(opener); - } - - opener.setReceiver(receiver); - opener.process(file.getAbsolutePath()); - opener.closeStream(); - - Mockito.verify(receiver).process(Mockito.any(Reader.class)); - Assert.assertEquals(expected, sb.toString()); - } - private File createTestFile() throws IOException { final File file = tempFolder.newFile(); try (OutputStream stream = new FileOutputStream(file)) { diff --git a/metafacture-io/src/test/java/org/metafacture/io/TestHelpers.java b/metafacture-io/src/test/java/org/metafacture/io/TestHelpers.java new file mode 100644 index 000000000..bb4c7725e --- /dev/null +++ b/metafacture-io/src/test/java/org/metafacture/io/TestHelpers.java @@ -0,0 +1,63 @@ +/* + * Copyright 2024 hbz NRW + * + * Licensed under the Apache License, Version 2.0 the "License"; + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.metafacture.io; + +import org.metafacture.commons.ResourceUtil; +import org.metafacture.framework.ObjectReceiver; + +import org.junit.Assert; +import org.mockito.Mockito; + +import java.io.File; +import java.io.Reader; +import java.util.function.Consumer; +import java.util.function.IntSupplier; + +public final class TestHelpers { + + public static void assertFile(final ObjectReceiver receiver, final String expected, final File file, final Consumer consumer) { + assertReader(receiver, expected, () -> { + final FileOpener opener = new FileOpener(); + if (consumer != null) { + consumer.accept(opener); + } + + opener.setReceiver(receiver); + opener.process(file.getAbsolutePath()); + opener.closeStream(); + + return 1; + }); + } + + public static void assertReader(final ObjectReceiver receiver, final String expected, final IntSupplier supplier) { + final StringBuilder sb = new StringBuilder(); + + Mockito.doAnswer(i -> { + sb.delete(0, sb.length()); + sb.append(ResourceUtil.readAll(i.getArgument(0))); + + return null; + }).when(receiver).process(Mockito.any(Reader.class)); + + final int times = supplier.getAsInt(); + + Mockito.verify(receiver, Mockito.times(times)).process(Mockito.any(Reader.class)); + Assert.assertEquals(expected, sb.toString()); + } + +} From 83e5d9e351206e91905d621c98dd6f00fe309ea2 Mon Sep 17 00:00:00 2001 From: Jens Wille Date: Thu, 11 Jan 2024 14:54:26 +0100 Subject: [PATCH 2/3] HttpOpener: Close input stream resources. (#514) --- .../java/org/metafacture/io/HttpOpener.java | 10 ++++--- .../org/metafacture/io/HttpOpenerTest.java | 26 ++++++------------- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/metafacture-io/src/main/java/org/metafacture/io/HttpOpener.java b/metafacture-io/src/main/java/org/metafacture/io/HttpOpener.java index adf5de307..24960160c 100644 --- a/metafacture-io/src/main/java/org/metafacture/io/HttpOpener.java +++ b/metafacture-io/src/main/java/org/metafacture/io/HttpOpener.java @@ -295,12 +295,16 @@ public void process(final String input) { final HttpURLConnection connection = requestBody != null ? doOutput(urlToOpen, requestBody) : doRedirects(urlToOpen); - final InputStream inputStream = getInputStream(connection); final String charset = getContentCharset(connection); - getReceiver().process(new InputStreamReader( + try ( + InputStream inputStream = getInputStream(connection); + Reader reader = new InputStreamReader( "gzip".equalsIgnoreCase(connection.getContentEncoding()) ? - new GZIPInputStream(inputStream) : inputStream, charset)); + new GZIPInputStream(inputStream) : inputStream, charset) + ) { + getReceiver().process(reader); + } } catch (final IOException e) { throw new MetafactureException(e); diff --git a/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java b/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java index 245ba4d11..87dcdda39 100644 --- a/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java +++ b/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java @@ -16,15 +16,12 @@ package org.metafacture.io; -import org.metafacture.commons.ResourceUtil; import org.metafacture.framework.ObjectReceiver; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder; import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.core.WireMockConfiguration; -import com.github.tomakehurst.wiremock.http.HttpHeader; -import com.github.tomakehurst.wiremock.http.HttpHeaders; import com.github.tomakehurst.wiremock.http.RequestMethod; import com.github.tomakehurst.wiremock.junit.WireMockRule; import com.github.tomakehurst.wiremock.matching.RequestPatternBuilder; @@ -34,10 +31,7 @@ import org.junit.ComparisonFailure; import org.junit.Rule; import org.junit.Test; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -49,8 +43,6 @@ import java.util.function.Consumer; import java.util.zip.GZIPOutputStream; -import static org.mockito.Mockito.times; - /** * Tests for class {@link HttpOpener}. * @@ -92,9 +84,6 @@ public final class HttpOpenerTest { @Mock private ObjectReceiver receiver; - @Captor - private ArgumentCaptor processedObject; - @Test public void shouldPerformGetRequestWithInputAsUrlByDefault() throws IOException { shouldPerformRequest(TEST_URL, HttpOpener.Method.GET, (o, u) -> {}); @@ -341,17 +330,18 @@ private void shouldPerformRequest(final String input, final HttpOpener.Method me WireMock.stubFor(stub); - opener.process(String.format(input, baseUrl)); + TestHelpers.assertReader(receiver, responseBody, () -> { + opener.process(String.format(input, baseUrl)); - // use the opener a second time in a workflow: - opener.process(String.format(input, baseUrl)); + // use the opener a second time in a workflow: + opener.process(String.format(input, baseUrl)); - opener.closeStream(); + opener.closeStream(); - WireMock.verify(request); + return 2; + }); - Mockito.verify(receiver, times(2)).process(processedObject.capture()); - Assert.assertEquals(responseBody, ResourceUtil.readAll(processedObject.getValue())); + WireMock.verify(request); } } From 3beb8c47601569a5a48dded1a5692c8f9bfe973e Mon Sep 17 00:00:00 2001 From: Jens Wille Date: Fri, 12 Jan 2024 15:21:35 +0100 Subject: [PATCH 3/3] Verify each FileOpener/HttpOpener test "process" individually. (#517) Instead of only the last one in the case of HttpOpener. --- .../org/metafacture/io/HttpOpenerTest.java | 6 ++-- .../java/org/metafacture/io/TestHelpers.java | 29 ++++++++----------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java b/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java index 87dcdda39..dd3bdbb1a 100644 --- a/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java +++ b/metafacture-io/src/test/java/org/metafacture/io/HttpOpenerTest.java @@ -330,16 +330,14 @@ private void shouldPerformRequest(final String input, final HttpOpener.Method me WireMock.stubFor(stub); - TestHelpers.assertReader(receiver, responseBody, () -> { + TestHelpers.assertReader(receiver, () -> { opener.process(String.format(input, baseUrl)); // use the opener a second time in a workflow: opener.process(String.format(input, baseUrl)); opener.closeStream(); - - return 2; - }); + }, responseBody, responseBody); WireMock.verify(request); } diff --git a/metafacture-io/src/test/java/org/metafacture/io/TestHelpers.java b/metafacture-io/src/test/java/org/metafacture/io/TestHelpers.java index bb4c7725e..722cefeb0 100644 --- a/metafacture-io/src/test/java/org/metafacture/io/TestHelpers.java +++ b/metafacture-io/src/test/java/org/metafacture/io/TestHelpers.java @@ -24,13 +24,15 @@ import java.io.File; import java.io.Reader; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.function.Consumer; -import java.util.function.IntSupplier; public final class TestHelpers { public static void assertFile(final ObjectReceiver receiver, final String expected, final File file, final Consumer consumer) { - assertReader(receiver, expected, () -> { + assertReader(receiver, () -> { final FileOpener opener = new FileOpener(); if (consumer != null) { consumer.accept(opener); @@ -39,25 +41,18 @@ public static void assertFile(final ObjectReceiver receiver, final Strin opener.setReceiver(receiver); opener.process(file.getAbsolutePath()); opener.closeStream(); - - return 1; - }); + }, expected); } - public static void assertReader(final ObjectReceiver receiver, final String expected, final IntSupplier supplier) { - final StringBuilder sb = new StringBuilder(); - - Mockito.doAnswer(i -> { - sb.delete(0, sb.length()); - sb.append(ResourceUtil.readAll(i.getArgument(0))); - - return null; - }).when(receiver).process(Mockito.any(Reader.class)); + public static void assertReader(final ObjectReceiver receiver, final Runnable runnable, final String... expected) { + final List actual = new ArrayList<>(); + Mockito.doAnswer(i -> actual.add(ResourceUtil.readAll(i.getArgument(0)))).when(receiver).process(Mockito.any(Reader.class)); - final int times = supplier.getAsInt(); + runnable.run(); - Mockito.verify(receiver, Mockito.times(times)).process(Mockito.any(Reader.class)); - Assert.assertEquals(expected, sb.toString()); + Mockito.verify(receiver, Mockito.times(expected.length)).process(Mockito.any(Reader.class)); + Arrays.stream(expected).forEach(i -> Assert.assertEquals(i, actual.remove(0))); + Assert.assertEquals(0, actual.size()); } }