From bfb26e598095a9e3f3abd5352ac6e7ed242c6d58 Mon Sep 17 00:00:00 2001 From: John Blum Date: Tue, 21 Jul 2020 21:05:45 -0700 Subject: [PATCH] Refactor ResourceLoaderResourceResolver to describe the Resource onMissingResource(..) when Resource is not null. Remove 'final' modifier from setResourceLoader(:ResourceLoader). Edit Javadoc. Resolves gh-92. --- .../ResourceLoaderResourceResolver.java | 32 ++++++------ ...sourceLoaderResourceResolverUnitTests.java | 50 +++++++++++++++---- 2 files changed, 59 insertions(+), 23 deletions(-) diff --git a/spring-geode/src/main/java/org/springframework/geode/core/io/support/ResourceLoaderResourceResolver.java b/spring-geode/src/main/java/org/springframework/geode/core/io/support/ResourceLoaderResourceResolver.java index 9e5fc7f7..b74f4a11 100644 --- a/spring-geode/src/main/java/org/springframework/geode/core/io/support/ResourceLoaderResourceResolver.java +++ b/spring-geode/src/main/java/org/springframework/geode/core/io/support/ResourceLoaderResourceResolver.java @@ -52,8 +52,8 @@ public class ResourceLoaderResourceResolver implements ResourceLoaderAware, Reso private final AtomicReference resolvedResourceLoader = new AtomicReference<>(null); /** - * Gets a {@link ClassLoader} used by the {@link ResourceLoader} to resolve and load {@link Resource Resources} - * located on the {@literal classpath}. + * Gets an {@link Optional} {@link ClassLoader} used by the {@link ResourceLoader} to resolve and load + * {@link Resource Resources} located on the {@literal classpath}. * * Returns the {@link ResourceLoader#getClassLoader() ClassLoader} from the configured {@link ResourceLoader}, * if present. Otherwise, returns a {@link ClassLoader} determined by {@link ClassUtils#getDefaultClassLoader()}, @@ -61,6 +61,7 @@ public class ResourceLoaderResourceResolver implements ResourceLoaderAware, Reso * and finally, {@link ClassLoader#getSystemClassLoader()}. * * @return an {@link Optional} {@link ClassLoader} to resolve and load {@link Resource Resources}. + * @see org.springframework.core.io.ResourceLoader#getClassLoader() * @see java.lang.ClassLoader * @see java.util.Optional */ @@ -73,14 +74,14 @@ public class ResourceLoaderResourceResolver implements ResourceLoaderAware, Reso } /** - * Configures the {@link ResourceLoader} used by this {@link ResourceResolver} to resolve - * and load {@link Resource Resources}. + * Configures the {@link ResourceLoader} used by this {@link ResourceResolver} to resolve and load + * {@link Resource Resources}. * * @param resourceLoader {@link ResourceLoader} used to resolve and load {@link Resource Resources}. * @see org.springframework.core.io.ResourceLoader */ @Override - public final void setResourceLoader(@Nullable ResourceLoader resourceLoader) { + public void setResourceLoader(@Nullable ResourceLoader resourceLoader) { this.resolvedResourceLoader.set(resourceLoader); } @@ -136,12 +137,14 @@ public class ResourceLoaderResourceResolver implements ResourceLoaderAware, Reso /** * Determines whether the {@link Resource} is a {@literal qualified} {@link Resource}. * - * Qualifications are determined by the application Use Case (UC) or Requirements at time of resolution. - * For example, it maybe that the {@link Resource} must {@link Resource#exists() exist} to be valid, or that - * the {@link Resource} must be fully-qualified in terms of the protocol, path and name. + * Qualifications are determined by the application Requirements and Use Case (UC) at time of resolution. + * For example, it maybe that the {@link Resource} must {@link Resource#exists() exist} to qualify, or that + * the {@link Resource} must have a valid protocol, path and name. + * + * This default implementation requires the target {@link Resource} to not be {@literal null}. * * @param resource {@link Resource} to qualify. - * @return a boolean value indicating whether the {@link Resource} is qualified (e.g. valid). + * @return a boolean value indicating whether the {@link Resource} is qualified. * @see org.springframework.core.io.Resource */ protected boolean isQualified(@Nullable Resource resource) { @@ -149,18 +152,19 @@ public class ResourceLoaderResourceResolver implements ResourceLoaderAware, Reso } /** - * Action to carry out in the event the {@link Resource} identified at the given {@link String location} is missing, - * or not {@link #isQualified(Resource) qualified}. + * Action to perform when the {@link Resource} identified at the specified {@link String location} is missing, + * or was not {@link #isQualified(Resource) qualified}. * * @param resource missing {@link Resource}. * @param location {@link String} containing the location identifying the missing {@link Resource}. - * @throws ResourceNotFoundException if the {@link Resource} cannot be found at the {@link String location}. + * @throws ResourceNotFoundException if the {@link Resource} cannot be found at the specified {@link String location}. * @return a different {@link Resource}, possibly. Alternatively, this method may throw * a {@link ResourceNotFoundException}. * @see #isQualified(Resource) */ protected @Nullable Resource onMissingResource(@Nullable Resource resource, @NonNull String location) { - throw new ResourceNotFoundException(String.format("Failed to resolve a Resource at location [%s]", location)); + throw new ResourceNotFoundException(String.format("Failed to resolve Resource [%1$s] at location [%2$s]", + ResourceUtils.nullSafeGetDescription(resource), location)); } /** @@ -168,7 +172,7 @@ public class ResourceLoaderResourceResolver implements ResourceLoaderAware, Reso * such as a Spring {@link ApplicationContext}. * * The targeted, identified {@link Resource} can be further {@link #isQualified(Resource) qualified} by subclasses - * based on application Use Case (UC) or Requirements. + * based on application requirements or use case (UC). * * In the event that a {@link Resource} cannot be identified at the given {@link String location}, then applications * have 1 last opportunity to handle the missing {@link Resource} event, and either return a different or default diff --git a/spring-geode/src/test/java/org/springframework/geode/core/io/support/ResourceLoaderResourceResolverUnitTests.java b/spring-geode/src/test/java/org/springframework/geode/core/io/support/ResourceLoaderResourceResolverUnitTests.java index 09e7b51a..dbbc6815 100644 --- a/spring-geode/src/test/java/org/springframework/geode/core/io/support/ResourceLoaderResourceResolverUnitTests.java +++ b/spring-geode/src/test/java/org/springframework/geode/core/io/support/ResourceLoaderResourceResolverUnitTests.java @@ -26,6 +26,7 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; import java.util.Set; @@ -56,7 +57,7 @@ import org.springframework.util.ClassUtils; public class ResourceLoaderResourceResolverUnitTests { @Test - public void getClassLoaderFromResolvedResourceLoader() { + public void getsClassLoaderFromResolvedResourceLoader() { ClassLoader mockClassLoader = mock(ClassLoader.class); @@ -71,6 +72,7 @@ public class ResourceLoaderResourceResolverUnitTests { assertThat(resourceResolver.getClassLoader().orElse(null)).isEqualTo(mockClassLoader); verify(mockResourceLoader, times(1)).getClassLoader(); + verifyNoMoreInteractions(mockResourceLoader); } @Test @@ -96,9 +98,10 @@ public class ResourceLoaderResourceResolverUnitTests { resourceResolver.setResourceLoader(mockResourceLoader); - assertThat(resourceResolver.getResourceLoader()).isEqualTo(mockResourceLoader); + assertThat(resourceResolver.getResourceLoader()).isSameAs(mockResourceLoader); verify(resourceResolver, never()).newResourceLoader(); + verifyNoInteractions(mockResourceLoader); } @Test @@ -115,6 +118,7 @@ public class ResourceLoaderResourceResolverUnitTests { assertThat(resourceLoader).isEqualTo(mockResourceLoader); verify(resourceResolver, times(1)).newResourceLoader(); + verifyNoInteractions(mockResourceLoader); } @Test @@ -133,10 +137,11 @@ public class ResourceLoaderResourceResolverUnitTests { ResourceLoader resourceLoader = resourceResolver.newResourceLoader(); assertThat(resourceLoader).isInstanceOf(DefaultResourceLoader.class); - assertThat(resourceLoader.getClassLoader()).isEqualTo(mockClassLoader); + assertThat(resourceLoader.getClassLoader()).isSameAs(mockClassLoader); verify(resourceResolver, times(1)).getClassLoader(); verify(mockResourceLoader, times(1)).getClassLoader(); + verifyNoMoreInteractions(mockResourceLoader); } @Test @@ -215,13 +220,36 @@ public class ResourceLoaderResourceResolverUnitTests { } catch (ResourceNotFoundException expected) { - assertThat(expected).hasMessage("Failed to resolve a Resource at location [/path/to/resource]"); + assertThat(expected).hasMessage("Failed to resolve Resource [null] at location [/path/to/resource]"); assertThat(expected).hasNoCause(); throw expected; } } + @Test(expected = ResourceNotFoundException.class) + public void onMissingResourceWithResourceThrowsResourceNotFoundException() { + + Resource mockResource = mock(Resource.class); + + doReturn("MOCK").when(mockResource).getDescription(); + + try { + new ResourceLoaderResourceResolver().onMissingResource(mockResource, "/location/of/resource"); + } + catch (ResourceNotFoundException expected) { + + assertThat(expected).hasMessage("Failed to resolve Resource [MOCK] at location [/location/of/resource]"); + assertThat(expected).hasNoCause(); + + throw expected; + } + finally { + verify(mockResource, times(1)).getDescription(); + verifyNoMoreInteractions(mockResource); + } + } + @Test public void resolveReturnsResource() { @@ -262,7 +290,7 @@ public class ResourceLoaderResourceResolverUnitTests { } catch (ResourceNotFoundException expected) { - assertThat(expected).hasMessage("Failed to resolve a Resource at location [%s]", location); + assertThat(expected).hasMessage("Failed to resolve Resource [null] at location [%s]", location); assertThat(expected).hasNoCause(); throw expected; @@ -272,6 +300,7 @@ public class ResourceLoaderResourceResolverUnitTests { verify(resourceResolver, times(1)).isQualified(isNull()); verify(resourceResolver, times(1)).onMissingResource(isNull(), eq(location)); verify(mockResourceLoader, times(1)).getResource(eq(location)); + verifyNoMoreInteractions(mockResourceLoader); } } @@ -287,15 +316,16 @@ public class ResourceLoaderResourceResolverUnitTests { ResourceLoaderResourceResolver resourceResolver = spy(new ResourceLoaderResourceResolver()); doReturn(mockResourceLoader).when(resourceResolver).getResourceLoader(); - doReturn(false).when(resourceResolver).isQualified(eq(mockResource)); + doReturn(false).when(resourceResolver).isQualified(any()); doReturn(mockResource).when(mockResourceLoader).getResource(eq(location)); + doReturn("MOCK").when(mockResource).getDescription(); try { resourceResolver.resolve(location); } catch (ResourceNotFoundException expected) { - assertThat(expected).hasMessage("Failed to resolve a Resource at location [%s]", location); + assertThat(expected).hasMessage("Failed to resolve Resource [MOCK] at location [%s]", location); assertThat(expected).hasNoCause(); throw expected; @@ -305,7 +335,8 @@ public class ResourceLoaderResourceResolverUnitTests { verify(resourceResolver, times(1)).isQualified(eq(mockResource)); verify(resourceResolver, times(1)).onMissingResource(eq(mockResource), eq(location)); verify(mockResourceLoader, times(1)).getResource(eq(location)); - verifyNoInteractions(mockResource); + verify(mockResource, times(1)).getDescription(); + verifyNoMoreInteractions(mockResource, mockResourceLoader); } } @@ -322,7 +353,7 @@ public class ResourceLoaderResourceResolverUnitTests { ResourceLoaderResourceResolver resourceResolver = spy(new ResourceLoaderResourceResolver()); doReturn(mockResourceLoader).when(resourceResolver).getResourceLoader(); - doReturn(false).when(resourceResolver).isQualified(mockResourceOne); + doReturn(false).when(resourceResolver).isQualified(eq(mockResourceOne)); doReturn(mockResourceTwo).when(resourceResolver).onMissingResource(eq(mockResourceOne), eq(location)); doReturn(mockResourceOne).when(mockResourceLoader).getResource(eq(location)); @@ -332,6 +363,7 @@ public class ResourceLoaderResourceResolverUnitTests { verify(resourceResolver, times(1)).isQualified(eq(mockResourceOne)); verify(resourceResolver, times(1)).onMissingResource(eq(mockResourceOne), eq(location)); verify(mockResourceLoader, times(1)).getResource(eq(location)); + verifyNoMoreInteractions(mockResourceLoader); verifyNoInteractions(mockResourceOne, mockResourceTwo); }