From d38442af2a3ddded578b55e79ced30e73211c319 Mon Sep 17 00:00:00 2001 From: John Blum Date: Mon, 20 Jul 2020 18:17:59 -0700 Subject: [PATCH] Refactor AbstractResourceWriter to throw a ResourcewWriteException on an IOException during doWrite(..). Additionally, add a preProcess(:WritableResource) to process the WritableResource before data for the target Resource is written. Resolves gh-92. --- .../geode/core/io/AbstractResourceWriter.java | 17 ++++- .../io/AbstractResourceWriterUnitTests.java | 75 +++++++++++++------ 2 files changed, 64 insertions(+), 28 deletions(-) diff --git a/spring-geode/src/main/java/org/springframework/geode/core/io/AbstractResourceWriter.java b/spring-geode/src/main/java/org/springframework/geode/core/io/AbstractResourceWriter.java index 89dfd94a..112c6fc8 100644 --- a/spring-geode/src/main/java/org/springframework/geode/core/io/AbstractResourceWriter.java +++ b/spring-geode/src/main/java/org/springframework/geode/core/io/AbstractResourceWriter.java @@ -17,11 +17,9 @@ package org.springframework.geode.core.io; import java.io.IOException; import java.io.OutputStream; -import java.util.Optional; import org.springframework.core.io.Resource; import org.springframework.core.io.WritableResource; -import org.springframework.dao.DataAccessResourceFailureException; import org.springframework.geode.core.io.support.ResourceUtils; import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; @@ -44,15 +42,16 @@ public abstract class AbstractResourceWriter implements ResourceWriter { @Override public void write(@NonNull Resource resource, byte[] data) { - Optional.ofNullable(ResourceUtils.getWritableResource(resource)) + ResourceUtils.asWritableResource(resource) .filter(this::isAbleToHandle) + .map(this::preProcess) .map(it -> { try (OutputStream out = it.getOutputStream()) { doWrite(out, data); return true; } catch (IOException cause) { - throw new DataAccessResourceFailureException(String.format("Failed to write to Resource [%s]", + throw new ResourceWriteException(String.format("Failed to write to Resource [%s]", it.getDescription()), cause); } }) @@ -91,4 +90,14 @@ public abstract class AbstractResourceWriter implements ResourceWriter { */ protected abstract void doWrite(OutputStream resourceOutputStream, byte[] data) throws IOException; + /** + * Pre-processes the target {@link WritableResource} before writing to the {@link WritableResource}. + * + * @param resource {@link WritableResource} to pre-process; never {@literal null}. + * @return the given, target {@link WritableResource}. + * @see org.springframework.core.io.WritableResource + */ + protected @NonNull WritableResource preProcess(@NonNull WritableResource resource) { + return resource; + } } diff --git a/spring-geode/src/test/java/org/springframework/geode/core/io/AbstractResourceWriterUnitTests.java b/spring-geode/src/test/java/org/springframework/geode/core/io/AbstractResourceWriterUnitTests.java index dece0b45..f38c84a9 100644 --- a/spring-geode/src/test/java/org/springframework/geode/core/io/AbstractResourceWriterUnitTests.java +++ b/spring-geode/src/test/java/org/springframework/geode/core/io/AbstractResourceWriterUnitTests.java @@ -18,9 +18,12 @@ package org.springframework.geode.core.io; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doCallRealMethod; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -32,10 +35,10 @@ import java.io.IOException; import java.io.OutputStream; import org.junit.Test; +import org.mockito.InOrder; import org.springframework.core.io.Resource; import org.springframework.core.io.WritableResource; -import org.springframework.dao.DataAccessResourceFailureException; /** * Unit Tests for {@link AbstractResourceWriter}. @@ -51,7 +54,7 @@ import org.springframework.dao.DataAccessResourceFailureException; public class AbstractResourceWriterUnitTests { @Test - public void writeCallsDoWrite() throws IOException { + public void writeToWritableResourceCallsDoWrite() throws IOException { byte[] array = { (byte) 0xCA, (byte) 0xFE, (byte) 0xBA, (byte) 0xBE }; @@ -62,22 +65,26 @@ public class AbstractResourceWriterUnitTests { WritableResource mockResource = mock(WritableResource.class); doCallRealMethod().when(mockResourceWriter).write(eq(mockResource), eq(array)); + doAnswer(invocation -> invocation.getArgument(0)).when(mockResourceWriter).preProcess(any()); doReturn(true).when(mockResourceWriter).isAbleToHandle(eq(mockResource)); doReturn(mockOutputStream).when(mockResource).getOutputStream(); doReturn(true).when(mockResource).isWritable(); mockResourceWriter.write(mockResource, array); - verify(mockResourceWriter, times(1)).isAbleToHandle(eq(mockResource)); - verify(mockResourceWriter, times(1)).doWrite(eq(mockOutputStream), eq(array)); - verify(mockResource, times(1)).isWritable(); + InOrder order = inOrder(mockResourceWriter); + + order.verify(mockResourceWriter, times(1)).isAbleToHandle(eq(mockResource)); + order.verify(mockResourceWriter, times(1)).preProcess(eq(mockResource)); + order.verify(mockResourceWriter, times(1)).doWrite(eq(mockOutputStream), eq(array)); + verify(mockResource, times(1)).getOutputStream(); verify(mockOutputStream, times(1)).close(); verifyNoMoreInteractions(mockOutputStream, mockResource); } - @Test(expected = IllegalStateException.class) - public void writeNonWritableResourceThrowsIllegalStateException() throws IOException { + @Test(expected = UnhandledResourceException.class) + public void writeToNonWritableResourceThrowsUnhandledResourceException() throws IOException { byte[] array = { (byte) 0x01 }; @@ -91,23 +98,24 @@ public class AbstractResourceWriterUnitTests { try { mockResourceWriter.write(mockResource, array); } - catch (IllegalStateException expected) { + catch (UnhandledResourceException expected) { - assertThat(expected).hasMessage("Resource [MOCK] is not writable"); + assertThat(expected).hasMessage("Unable to handle Resource [MOCK]"); assertThat(expected).hasNoCause(); throw expected; } finally { verify(mockResourceWriter, never()).isAbleToHandle(any()); + verify(mockResourceWriter, never()).preProcess(any()); verify(mockResourceWriter, never()).doWrite(any(), any()); verify(mockResource, times(1)).getDescription(); verifyNoMoreInteractions(mockResource); } } - @Test(expected = IllegalStateException.class) - public void writeNullResourceThrowsIllegalStateException() throws IOException { + @Test(expected = UnhandledResourceException.class) + public void writeToNullResourceThrowsUnhandledResourceException() throws IOException { byte[] array = { (byte) 0x02 }; @@ -118,21 +126,22 @@ public class AbstractResourceWriterUnitTests { try { mockResourceWriter.write(null, array); } - catch (IllegalStateException expected) { + catch (UnhandledResourceException expected) { - assertThat(expected).hasMessage("Resource [null] is not writable"); + assertThat(expected).hasMessage("Unable to handle Resource [null]"); assertThat(expected).hasNoCause(); throw expected; } finally { verify(mockResourceWriter, never()).isAbleToHandle(any()); + verify(mockResourceWriter, never()).preProcess(any()); verify(mockResourceWriter, never()).doWrite(any(), any()); } } @Test(expected = UnhandledResourceException.class) - public void writeUnhandledResourceThrowsUnhandledResourceException() throws IOException { + public void writeToUnhandledResourceThrowsUnhandledResourceException() throws IOException { byte[] array = { (byte) 0x04 }; @@ -141,9 +150,9 @@ public class AbstractResourceWriterUnitTests { WritableResource mockResource = mock(WritableResource.class); doCallRealMethod().when(mockResourceWriter).write(any(), any(byte[].class)); + doReturn(false).when(mockResourceWriter).isAbleToHandle(eq(mockResource)); doReturn(true).when(mockResource).isWritable(); doReturn("MOCK").when(mockResource).getDescription(); - doReturn(false).when(mockResourceWriter).isAbleToHandle(eq(mockResource)); try { mockResourceWriter.write(mockResource, array); @@ -156,15 +165,16 @@ public class AbstractResourceWriterUnitTests { throw expected; } finally { - verify(mockResource, times(1)).isWritable(); - verify(mockResource, times(1)).getDescription(); + verify(mockResourceWriter, times(1)).isAbleToHandle(eq(mockResource)); + verify(mockResourceWriter, never()).preProcess(eq(mockResource)); verify(mockResourceWriter, never()).doWrite(any(), any()); + verify(mockResource, times(1)).getDescription(); verifyNoMoreInteractions(mockResource); } } - @Test(expected = DataAccessResourceFailureException.class) - public void writeThrowsDataAccessResourceFailureExceptionForIoException() throws IOException { + @Test(expected = ResourceWriteException.class) + public void writeThrowsResourceWriteExceptionForIoException() throws IOException { byte[] array = { (byte) 0x08 }; @@ -175,16 +185,16 @@ public class AbstractResourceWriterUnitTests { WritableResource mockResource = mock(WritableResource.class); doCallRealMethod().when(mockResourceWriter).write(any(), any(byte[].class)); - doReturn(true).when(mockResource).isWritable(); + doReturn(true).when(mockResourceWriter).isAbleToHandle(eq(mockResource)); + doAnswer(invocation -> invocation.getArgument(0)).when(mockResourceWriter).preProcess(any()); + doThrow(new IOException("TEST")).when(mockResourceWriter).doWrite(eq(mockOutputStream), eq(array)); doReturn("MOCK").when(mockResource).getDescription(); doReturn(mockOutputStream).when(mockResource).getOutputStream(); - doReturn(true).when(mockResourceWriter).isAbleToHandle(eq(mockResource)); - doThrow(new IOException("TEST")).when(mockResourceWriter).doWrite(eq(mockOutputStream), eq(array)); try { mockResourceWriter.write(mockResource, array); } - catch (DataAccessResourceFailureException expected) { + catch (ResourceWriteException expected) { assertThat(expected).hasMessageStartingWith("Failed to write to Resource [MOCK]"); assertThat(expected).hasCauseInstanceOf(IOException.class); @@ -195,8 +205,8 @@ public class AbstractResourceWriterUnitTests { } finally { verify(mockResourceWriter, times(1)).isAbleToHandle(eq(mockResource)); + verify(mockResourceWriter, times(1)).preProcess(eq(mockResource)); verify(mockResourceWriter, times(1)).doWrite(eq(mockOutputStream), eq(array)); - verify(mockResource, times(1)).isWritable(); verify(mockResource, times(1)).getDescription(); verify(mockResource, times(1)).getOutputStream(); verify(mockOutputStream, times(1)).close(); @@ -227,4 +237,21 @@ public class AbstractResourceWriterUnitTests { assertThat(mockResourceWriter.isAbleToHandle(null)).isFalse(); } + + @Test + public void preProcessWritableResourceReturnsWritableResource() { + + WritableResource mockResource = mock(WritableResource.class); + + AbstractResourceWriter mockResourceWriter = mock(AbstractResourceWriter.class); + + doCallRealMethod().when(mockResourceWriter).preProcess(any()); + + assertThat(mockResourceWriter.preProcess(mockResource)).isSameAs(mockResource); + assertThat(mockResourceWriter.preProcess(null)).isNull(); + + verify(mockResourceWriter, times(1)).preProcess(eq(mockResource)); + verify(mockResourceWriter, times(1)).preProcess(isNull()); + verifyNoInteractions(mockResource); + } }