From 0cd771b2365643474e5a41ebea9636a529cb14ba Mon Sep 17 00:00:00 2001 From: John Blum Date: Tue, 21 Jul 2020 21:27:10 -0700 Subject: [PATCH] Move filter on isImportProfilesActive(:Environment) before Region Predicate test in importInto(:Region). Move the isImportProfilesActive(:Environment) method before the importInto(:Region) method. Annotate toSet(..) class method signature with Spring Framework @NonNull and @Nullable annotations. Edit Javadoc. Resolves gh-92. --- .../AbstractCacheDataImporterExporter.java | 71 ++++++++++--------- ...actCacheDataImporterExporterUnitTests.java | 31 ++++---- 2 files changed, 52 insertions(+), 50 deletions(-) diff --git a/spring-geode/src/main/java/org/springframework/geode/data/AbstractCacheDataImporterExporter.java b/spring-geode/src/main/java/org/springframework/geode/data/AbstractCacheDataImporterExporter.java index 1b422ec6..1e0e8109 100644 --- a/spring-geode/src/main/java/org/springframework/geode/data/AbstractCacheDataImporterExporter.java +++ b/spring-geode/src/main/java/org/springframework/geode/data/AbstractCacheDataImporterExporter.java @@ -225,6 +225,7 @@ public abstract class AbstractCacheDataImporterExporter * @see org.apache.geode.cache.Region * @see #isExportEnabled(Environment) * @see #getRegionPredicate() + * @see #doExportFrom(Region) */ @NonNull @Override public Region exportFrom(@NonNull Region region) { @@ -245,6 +246,7 @@ public abstract class AbstractCacheDataImporterExporter * @param region {@link Region} to export data from. * @return the given {@link Region}. * @see org.apache.geode.cache.Region + * @see #exportFrom(Region) */ protected abstract @NonNull Region doExportFrom(@NonNull Region region); @@ -263,38 +265,6 @@ public abstract class AbstractCacheDataImporterExporter DEFAULT_CACHE_DATA_IMPORT_ENABLED)); } - /** - * Imports data into the given {@link Region}. - * - * @param region {@link Region} to import data into. - * @return the given {@link Region}. - * @see org.apache.geode.cache.Region - * @see #isImportEnabled(Environment) - * @see #getRegionPredicate() - */ - @NonNull @Override - public Region importInto(@NonNull Region region) { - - Assert.notNull(region, "Region must not be null"); - - boolean importEnabled = getEnvironment() - .filter(this::isImportEnabled) - .filter(environment -> getRegionPredicate().test(region)) - .filter(this::isImportProfilesActive) - .isPresent(); - - return importEnabled ? doImportInto(region) : region; - } - - /** - * Imports data into the given {@link Region}. - * - * @param region {@link Region} to import data into. - * @return the given {@link Region}. - * @see org.apache.geode.cache.Region - */ - protected abstract @NonNull Region doImportInto(@NonNull Region region); - /** * Determines whether the Cache Data Import data access operation is enabled based on the configured, active/default * {@literal Profiles} as declared in the Spring {@link Environment}. @@ -331,6 +301,41 @@ public abstract class AbstractCacheDataImporterExporter return importProfilesActive; } + /** + * Imports data into the given {@link Region}. + * + * @param region {@link Region} to import data into. + * @return the given {@link Region}. + * @see org.apache.geode.cache.Region + * @see #isImportEnabled(Environment) + * @see #isImportProfilesActive(Environment) + * @see #getRegionPredicate() + * @see #doImportInto(Region) + */ + @NonNull @Override + public Region importInto(@NonNull Region region) { + + Assert.notNull(region, "Region must not be null"); + + boolean importEnabled = getEnvironment() + .filter(this::isImportEnabled) + .filter(this::isImportProfilesActive) + .filter(environment -> getRegionPredicate().test(region)) + .isPresent(); + + return importEnabled ? doImportInto(region) : region; + } + + /** + * Imports data into the given {@link Region}. + * + * @param region {@link Region} to import data into. + * @return the given {@link Region}. + * @see org.apache.geode.cache.Region + * @see #importInto(Region) + */ + protected abstract @NonNull Region doImportInto(@NonNull Region region); + @NonNull Set commaDelimitedStringToSet(@Nullable String commaDelimitedString) { return StringUtils.hasText(commaDelimitedString) @@ -379,7 +384,7 @@ public abstract class AbstractCacheDataImporterExporter && !Collections.singleton(RESERVED_DEFAULT_PROFILE_NAME).containsAll(profiles); } - private static Set toSet(T[] array, Class type) { + private static @NonNull Set toSet(@Nullable T[] array, @NonNull Class type) { return CollectionUtils.asSet(ArrayUtils.nullSafeArray(array, type)); } } diff --git a/spring-geode/src/test/java/org/springframework/geode/data/AbstractCacheDataImporterExporterUnitTests.java b/spring-geode/src/test/java/org/springframework/geode/data/AbstractCacheDataImporterExporterUnitTests.java index 93462895..42bb1288 100644 --- a/spring-geode/src/test/java/org/springframework/geode/data/AbstractCacheDataImporterExporterUnitTests.java +++ b/spring-geode/src/test/java/org/springframework/geode/data/AbstractCacheDataImporterExporterUnitTests.java @@ -568,6 +568,7 @@ public class AbstractCacheDataImporterExporterUnitTests { doReturn(Optional.of(mockEnvironment)).when(importer).getEnvironment(); doReturn(true).when(importer).isImportEnabled(eq(mockEnvironment)); + doReturn(true).when(importer).isImportProfilesActive(eq(mockEnvironment)); doReturn(mockPredicate).when(importer).getRegionPredicate(); doReturn(false).when(mockPredicate).test(any()); @@ -575,8 +576,8 @@ public class AbstractCacheDataImporterExporterUnitTests { verify(importer, times(1)).getEnvironment(); verify(importer, times(1)).isImportEnabled(eq(mockEnvironment)); + verify(importer, times(1)).isImportProfilesActive(eq(mockEnvironment)); verify(importer, times(1)).getRegionPredicate(); - verify(importer, never()).isImportProfilesActive(any()); verify(importer, never()).doImportInto(any(Region.class)); verify(mockPredicate, times(1)).test(eq(mockRegion)); verifyNoMoreInteractions(mockPredicate); @@ -604,8 +605,8 @@ public class AbstractCacheDataImporterExporterUnitTests { verify(importer, times(1)).getEnvironment(); verify(importer, times(1)).isImportEnabled(eq(mockEnvironment)); - verify(importer, times(1)).getRegionPredicate(); verify(importer, times(1)).isImportProfilesActive(eq(mockEnvironment)); + verify(importer, never()).getRegionPredicate(); verify(importer, never()).doImportInto(eq(mockRegion)); verify(mockEnvironment, times(1)).getActiveProfiles(); verify(mockEnvironment, times(1)).getDefaultProfiles(); @@ -615,9 +616,8 @@ public class AbstractCacheDataImporterExporterUnitTests { verify(mockEnvironment, times(1)) .getProperty(eq(AbstractCacheDataImporterExporter.CACHE_DATA_IMPORT_ACTIVE_PROFILES_PROPERTY_NAME), eq(AbstractCacheDataImporterExporter.DEFAULT_CACHE_DATA_IMPORT_ACTIVE_PROFILES)); - verify(mockPredicate, times(1)).test(eq(mockRegion)); verifyNoMoreInteractions(mockEnvironment); - verifyNoInteractions(mockRegion); + verifyNoInteractions(mockPredicate, mockRegion); } @Test @@ -644,8 +644,8 @@ public class AbstractCacheDataImporterExporterUnitTests { order.verify(importer, times(1)).getEnvironment(); order.verify(importer, times(1)).isImportEnabled(eq(mockEnvironment)); - order.verify(importer, times(1)).getRegionPredicate(); order.verify(importer, times(1)).isImportProfilesActive(eq(mockEnvironment)); + order.verify(importer, times(1)).getRegionPredicate(); order. verify(importer, times(1)).doImportInto(eq(mockRegion)); verify(mockEnvironment, never()).getActiveProfiles(); @@ -687,8 +687,8 @@ public class AbstractCacheDataImporterExporterUnitTests { verify(importer, times(2)).getEnvironment(); verify(importer, times(2)).isImportEnabled(eq(mockEnvironment)); - verify(importer, times(2)).getRegionPredicate(); verify(importer, times(2)).isImportProfilesActive(eq(mockEnvironment)); + verify(importer, never()).getRegionPredicate(); verify(importer, never()).doImportInto(eq(mockRegion)); verify(mockEnvironment, times(2)).getActiveProfiles(); verify(mockEnvironment, times(1)).getDefaultProfiles(); @@ -698,9 +698,8 @@ public class AbstractCacheDataImporterExporterUnitTests { verify(mockEnvironment, times(2)) .getProperty(eq(AbstractCacheDataImporterExporter.CACHE_DATA_IMPORT_ACTIVE_PROFILES_PROPERTY_NAME), eq(AbstractCacheDataImporterExporter.DEFAULT_CACHE_DATA_IMPORT_ACTIVE_PROFILES)); - verify(mockPredicate, times(2)).test(eq(mockRegion)); - verifyNoMoreInteractions(mockEnvironment, mockPredicate); - verifyNoInteractions(mockRegion); + verifyNoMoreInteractions(mockEnvironment); + verifyNoInteractions(mockPredicate, mockRegion); } @Test @@ -726,8 +725,8 @@ public class AbstractCacheDataImporterExporterUnitTests { verify(importer, times(2)).getEnvironment(); verify(importer, times(2)).isImportEnabled(eq(mockEnvironment)); - verify(importer, times(2)).getRegionPredicate(); verify(importer, times(2)).isImportProfilesActive(eq(mockEnvironment)); + verify(importer, never()).getRegionPredicate(); verify(importer, never()).doImportInto(any(Region.class)); verify(mockEnvironment, times(2)).getActiveProfiles(); verify(mockEnvironment, times(2)).getDefaultProfiles(); @@ -737,9 +736,8 @@ public class AbstractCacheDataImporterExporterUnitTests { verify(mockEnvironment, times(2)) .getProperty(eq(AbstractCacheDataImporterExporter.CACHE_DATA_IMPORT_ACTIVE_PROFILES_PROPERTY_NAME), eq(AbstractCacheDataImporterExporter.DEFAULT_CACHE_DATA_IMPORT_ACTIVE_PROFILES)); - verify(mockPredicate, times(2)).test(eq(mockRegion)); - verifyNoMoreInteractions(mockEnvironment, mockPredicate); - verifyNoInteractions(mockRegion); + verifyNoMoreInteractions(mockEnvironment); + verifyNoInteractions(mockPredicate, mockRegion); } @Test @@ -764,8 +762,8 @@ public class AbstractCacheDataImporterExporterUnitTests { verify(importer, times(1)).getEnvironment(); verify(importer, times(1)).isImportEnabled(eq(mockEnvironment)); - verify(importer, times(1)).getRegionPredicate(); verify(importer, times(1)).isImportProfilesActive(eq(mockEnvironment)); + verify(importer, never()).getRegionPredicate(); verify(importer, never()).doImportInto(any(Region.class)); verify(mockEnvironment, times(1)).getActiveProfiles(); verify(mockEnvironment, never()).getDefaultProfiles(); @@ -775,9 +773,8 @@ public class AbstractCacheDataImporterExporterUnitTests { verify(mockEnvironment, times(1)) .getProperty(eq(AbstractCacheDataImporterExporter.CACHE_DATA_IMPORT_ACTIVE_PROFILES_PROPERTY_NAME), eq(AbstractCacheDataImporterExporter.DEFAULT_CACHE_DATA_IMPORT_ACTIVE_PROFILES)); - verify(mockPredicate, times(1)).test(eq(mockRegion)); - verifyNoMoreInteractions(mockEnvironment, mockPredicate); - verifyNoInteractions(mockRegion); + verifyNoMoreInteractions(mockEnvironment); + verifyNoInteractions(mockPredicate, mockRegion); } @Test(expected = IllegalArgumentException.class)