diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/support/AbstractFactoryBeanSupport.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/support/AbstractFactoryBeanSupport.java index b393891c..c32c4c72 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/support/AbstractFactoryBeanSupport.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/support/AbstractFactoryBeanSupport.java @@ -24,18 +24,19 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.BeanNameAware; import org.springframework.beans.factory.FactoryBean; +import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * The {@link AbstractFactoryBeanSupport} class is an abstract Spring {@link FactoryBean} base class implementation - * encapsulating operations common to SDG's {@link FactoryBean} implementations. + * An abstract Spring {@link FactoryBean} base class implementation encapsulating operations common to all + * Spring Data for Apache Geode (SDG) {@link FactoryBean} implementations. * * @author John Blum - * @see org.apache.commons.logging.Log - * @see org.apache.commons.logging.LogFactory + * @see org.slf4j.Logger + * @see org.slf4j.LoggerFactory * @see org.springframework.beans.factory.BeanClassLoaderAware * @see org.springframework.beans.factory.BeanFactory * @see org.springframework.beans.factory.BeanFactoryAware @@ -53,27 +54,28 @@ public abstract class AbstractFactoryBeanSupport private BeanFactory beanFactory; - private final Logger log; + private final Logger logger; private String beanName; /** - * Constructs a new instance of {@link AbstractFactoryBeanSupport} initializing an object instance {@link Logger}. + * Constructs a new instance of {@link AbstractFactoryBeanSupport} initializing a {@link Logger} to log operations + * performed by {@literal this} {@link FactoryBean}. * - * @see #newLog() + * @see #newLogger() */ protected AbstractFactoryBeanSupport() { - this.log = newLog(); + this.logger = newLogger(); } /** - * Constructs a new instance of {@link Logger} to log statements printed by Spring Data GemFire/Geode. + * Constructs a new instance of {@link Logger} to log statements printed by Spring Data for Apache Geode. * - * @return a new instance of {@link Logger}. + * @return a new instance of SLF4J {@link Logger}. * @see org.apache.commons.logging.LogFactory#getLog(Class) * @see org.apache.commons.logging.Log */ - protected Logger newLog() { + protected @NonNull Logger newLogger() { return LoggerFactory.getLogger(getClass()); } @@ -149,27 +151,29 @@ public abstract class AbstractFactoryBeanSupport } /** - * Returns a reference to the {@link Logger} used by this {@link FactoryBean} to log {@link String messages}. + * Returns a reference to the {@link Logger} used by {@literal this} {@link FactoryBean} + * to log {@link String messages}. * - * @return a reference to the {@link Logger} used by this {@link FactoryBean} to log {@link String messages}. + * @return a reference to the {@link Logger} used by {@literal this} {@link FactoryBean} + * to log {@link String messages}. * @see org.apache.commons.logging.Log */ - protected @Nullable Logger getLog() { - return this.log; + protected @NonNull Logger getLogger() { + return this.logger; } /** - * Returns an {@link Optional} reference to the {@link Logger} used by this {@link FactoryBean} + * Returns an {@link Optional} reference to the {@link Logger} used by {@literal this} {@link FactoryBean} * to log {@link String messages}. * - * @return an {@link Optional} reference to the {@link Logger} used by this {@link FactoryBean} + * @return an {@link Optional} reference to the {@link Logger} used by {@literal this} {@link FactoryBean} * to log {@link String messages}. * @see java.util.Optional * @see org.slf4j.Logger - * @see #getLog() + * @see #getLogger() */ - protected Optional getOptionalLog() { - return Optional.ofNullable(getLog()); + protected Optional getOptionalLogger() { + return Optional.ofNullable(getLogger()); } /** @@ -177,10 +181,10 @@ public abstract class AbstractFactoryBeanSupport * * @return a boolean value indicating whether {@literal DEBUG} logging is enabled. * @see org.slf4j.Logger#isDebugEnabled() - * @see #getOptionalLog() + * @see #getOptionalLogger() */ public boolean isDebugLoggingEnabled() { - return getOptionalLog().filter(Logger::isInfoEnabled).isPresent(); + return getOptionalLogger().filter(Logger::isDebugEnabled).isPresent(); } /** @@ -188,21 +192,10 @@ public abstract class AbstractFactoryBeanSupport * * @return a boolean value indicating whether {@literal INFO} logging is enabled. * @see org.slf4j.Logger#isInfoEnabled() - * @see #getOptionalLog() + * @see #getOptionalLogger() */ public boolean isInfoLoggingEnabled() { - return getOptionalLog().filter(Logger::isInfoEnabled).isPresent(); - } - - /** - * Determines whether {@literal ERROR} logging is enabled. - * - * @return a boolean value indicating whether {@literal ERROR} logging is enabled. - * @see org.slf4j.Logger#isErrorEnabled() - * @see #getOptionalLog() - */ - public boolean isErrorLoggingEnabled() { - return getOptionalLog().filter(Logger::isInfoEnabled).isPresent(); + return getOptionalLogger().filter(Logger::isInfoEnabled).isPresent(); } /** @@ -210,14 +203,25 @@ public abstract class AbstractFactoryBeanSupport * * @return a boolean value indicating whether {@literal WARN} logging is enabled. * @see org.slf4j.Logger#isWarnEnabled() - * @see #getOptionalLog() + * @see #getOptionalLogger() */ public boolean isWarnLoggingEnabled() { - return getOptionalLog().filter(Logger::isInfoEnabled).isPresent(); + return getOptionalLogger().filter(Logger::isWarnEnabled).isPresent(); } /** - * Indicates that this {@link FactoryBean} produces a single bean instance. + * Determines whether {@literal ERROR} logging is enabled. + * + * @return a boolean value indicating whether {@literal ERROR} logging is enabled. + * @see org.slf4j.Logger#isErrorEnabled() + * @see #getOptionalLogger() + */ + public boolean isErrorLoggingEnabled() { + return getOptionalLogger().filter(Logger::isErrorEnabled).isPresent(); + } + + /** + * Indicates that {@literal this} {@link FactoryBean} produces a single bean instance. * * @return {@literal true} by default. * @see org.springframework.beans.factory.FactoryBean#isSingleton() @@ -244,10 +248,10 @@ public abstract class AbstractFactoryBeanSupport * @param message {@link Supplier} containing the {@link String message} and arguments to log. * @see org.apache.commons.logging.Log#isDebugEnabled() * @see org.apache.commons.logging.Log#debug(Object) - * @see #getLog() + * @see #getLogger() */ protected void logDebug(Supplier message) { - getOptionalLog() + getOptionalLogger() .filter(Logger::isDebugEnabled) .ifPresent(log -> log.debug(message.get())); } @@ -269,10 +273,10 @@ public abstract class AbstractFactoryBeanSupport * @param message {@link Supplier} containing the {@link String message} and arguments to log. * @see org.apache.commons.logging.Log#isInfoEnabled() * @see org.apache.commons.logging.Log#info(Object) - * @see #getLog() + * @see #getLogger() */ protected void logInfo(Supplier message) { - getOptionalLog() + getOptionalLogger() .filter(Logger::isInfoEnabled) .ifPresent(log -> log.info(message.get())); } @@ -294,10 +298,10 @@ public abstract class AbstractFactoryBeanSupport * @param message {@link Supplier} containing the {@link String message} and arguments to log. * @see org.apache.commons.logging.Log#isWarnEnabled() * @see org.apache.commons.logging.Log#warn(Object) - * @see #getLog() + * @see #getLogger() */ protected void logWarning(Supplier message) { - getOptionalLog() + getOptionalLogger() .filter(Logger::isWarnEnabled) .ifPresent(log -> log.warn(message.get())); } @@ -319,10 +323,10 @@ public abstract class AbstractFactoryBeanSupport * @param message {@link Supplier} containing the {@link String message} and arguments to log. * @see org.apache.commons.logging.Log#isErrorEnabled() * @see org.apache.commons.logging.Log#error(Object) - * @see #getLog() + * @see #getLogger() */ protected void logError(Supplier message) { - getOptionalLog() + getOptionalLogger() .filter(Logger::isErrorEnabled) .ifPresent(log -> log.error(message.get())); } diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/IndexFactoryBeanUnitTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/IndexFactoryBeanUnitTests.java index c8652231..706abb3c 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/IndexFactoryBeanUnitTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/IndexFactoryBeanUnitTests.java @@ -138,8 +138,8 @@ public class IndexFactoryBeanUnitTests { IndexFactoryBean indexFactoryBean = spy(new IndexFactoryBean() { @Override - protected Logger newLog() { - return mockLogger; + protected Logger newLogger() { + return IndexFactoryBeanUnitTests.this.mockLogger; } }); diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/support/AbstractFactoryBeanSupportUnitTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/support/AbstractFactoryBeanSupportUnitTests.java index 1978cba7..fe7b8c02 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/support/AbstractFactoryBeanSupportUnitTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/support/AbstractFactoryBeanSupportUnitTests.java @@ -13,16 +13,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.support; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import org.junit.Before; @@ -37,13 +38,14 @@ import org.springframework.beans.factory.BeanFactory; import org.slf4j.Logger; /** - * Unit tests for {@link AbstractFactoryBeanSupport}. + * Unit Tests for {@link AbstractFactoryBeanSupport}. * * @author John Blum * @see org.junit.Test * @see org.mockito.Mock * @see org.mockito.Mockito * @see org.mockito.Spy + * @see org.mockito.junit.MockitoJUnitRunner * @see org.springframework.data.gemfire.support.AbstractFactoryBeanSupport * @since 1.0.0 */ @@ -51,159 +53,167 @@ import org.slf4j.Logger; public class AbstractFactoryBeanSupportUnitTests { @Mock - private Logger mockLog; + private Logger mockLogger; @Spy private TestFactoryBeanSupport factoryBeanSupport; @Before public void setup() { - when(factoryBeanSupport.getLog()).thenReturn(mockLog); + doReturn(this.mockLogger).when(this.factoryBeanSupport).getLogger(); } @Test public void setAndGetBeanClassLoader() { - assertThat(factoryBeanSupport.getBeanClassLoader()).isNull(); + assertThat(this.factoryBeanSupport.getBeanClassLoader()).isNull(); ClassLoader mockClassLoader = mock(ClassLoader.class); - factoryBeanSupport.setBeanClassLoader(mockClassLoader); + this.factoryBeanSupport.setBeanClassLoader(mockClassLoader); - assertThat(factoryBeanSupport.getBeanClassLoader()).isSameAs(mockClassLoader); + assertThat(this.factoryBeanSupport.getBeanClassLoader()).isSameAs(mockClassLoader); ClassLoader systemClassLoader = ClassLoader.getSystemClassLoader(); - factoryBeanSupport.setBeanClassLoader(systemClassLoader); + this.factoryBeanSupport.setBeanClassLoader(systemClassLoader); - assertThat(factoryBeanSupport.getBeanClassLoader()).isSameAs(systemClassLoader); + assertThat(this.factoryBeanSupport.getBeanClassLoader()).isSameAs(systemClassLoader); - factoryBeanSupport.setBeanClassLoader(null); + this.factoryBeanSupport.setBeanClassLoader(null); - assertThat(factoryBeanSupport.getBeanClassLoader()).isNull(); + assertThat(this.factoryBeanSupport.getBeanClassLoader()).isNull(); } @Test public void setAndGetBeanFactory() { - assertThat(factoryBeanSupport.getBeanFactory()).isNull(); + assertThat(this.factoryBeanSupport.getBeanFactory()).isNull(); BeanFactory mockBeanFactory = mock(BeanFactory.class); - factoryBeanSupport.setBeanFactory(mockBeanFactory); + this.factoryBeanSupport.setBeanFactory(mockBeanFactory); - assertThat(factoryBeanSupport.getBeanFactory()).isSameAs(mockBeanFactory); + assertThat(this.factoryBeanSupport.getBeanFactory()).isSameAs(mockBeanFactory); - factoryBeanSupport.setBeanFactory(null); + this.factoryBeanSupport.setBeanFactory(null); - assertThat(factoryBeanSupport.getBeanFactory()).isNull(); + assertThat(this.factoryBeanSupport.getBeanFactory()).isNull(); } @Test public void setAndGetBeanName() { - assertThat(factoryBeanSupport.getBeanName()).isNullOrEmpty(); + assertThat(this.factoryBeanSupport.getBeanName()).isNullOrEmpty(); - factoryBeanSupport.setBeanName("test"); + this.factoryBeanSupport.setBeanName("test"); - assertThat(factoryBeanSupport.getBeanName()).isEqualTo("test"); + assertThat(this.factoryBeanSupport.getBeanName()).isEqualTo("test"); - factoryBeanSupport.setBeanName(null); + this.factoryBeanSupport.setBeanName(null); - assertThat(factoryBeanSupport.getBeanName()).isNullOrEmpty(); + assertThat(this.factoryBeanSupport.getBeanName()).isNullOrEmpty(); } @Test public void isSingletonDefaultsToTrue() { - assertThat(factoryBeanSupport.isSingleton()).isTrue(); + assertThat(this.factoryBeanSupport.isSingleton()).isTrue(); } @Test public void logsDebugWhenDebugIsEnabled() { - when(mockLog.isDebugEnabled()).thenReturn(true); + when(this.mockLogger.isDebugEnabled()).thenReturn(true); - factoryBeanSupport.logDebug("%s log test", "debug"); + this.factoryBeanSupport.logDebug("%s log test", "debug"); - verify(mockLog, times(1)).isDebugEnabled(); - verify(mockLog, times(1)).debug(eq("debug log test")); + verify(this.mockLogger, times(1)).isDebugEnabled(); + verify(this.mockLogger, times(1)).debug(eq("debug log test")); + verifyNoMoreInteractions(this.mockLogger); } @Test public void logsInfoWhenInfoIsEnabled() { - when(mockLog.isInfoEnabled()).thenReturn(true); + when(this.mockLogger.isInfoEnabled()).thenReturn(true); - factoryBeanSupport.logInfo("%s log test", "info"); + this.factoryBeanSupport.logInfo("%s log test", "info"); - verify(mockLog, times(1)).isInfoEnabled(); - verify(mockLog, times(1)).info(eq("info log test")); + verify(this.mockLogger, times(1)).isInfoEnabled(); + verify(this.mockLogger, times(1)).info(eq("info log test")); + verifyNoMoreInteractions(this.mockLogger); } @Test public void logsWarningWhenWarnIsEnabled() { - when(mockLog.isWarnEnabled()).thenReturn(true); + when(this.mockLogger.isWarnEnabled()).thenReturn(true); - factoryBeanSupport.logWarning("%s log test", "warn"); + this.factoryBeanSupport.logWarning("%s log test", "warn"); - verify(mockLog, times(1)).isWarnEnabled(); - verify(mockLog, times(1)).warn(eq("warn log test")); + verify(this.mockLogger, times(1)).isWarnEnabled(); + verify(this.mockLogger, times(1)).warn(eq("warn log test")); + verifyNoMoreInteractions(this.mockLogger); } @Test - public void logsWarningWhenErrorIsEnabled() { + public void logsErrorWhenErrorIsEnabled() { - when(mockLog.isErrorEnabled()).thenReturn(true); + when(this.mockLogger.isErrorEnabled()).thenReturn(true); - factoryBeanSupport.logError("%s log test", "error"); + this.factoryBeanSupport.logError("%s log test", "error"); - verify(mockLog, times(1)).isErrorEnabled(); - verify(mockLog, times(1)).error(eq("error log test")); + verify(this.mockLogger, times(1)).isErrorEnabled(); + verify(this.mockLogger, times(1)).error(eq("error log test")); + verifyNoMoreInteractions(this.mockLogger); } @Test public void suppressesDebugLoggingWhenDebugIsDisabled() { - when(mockLog.isDebugEnabled()).thenReturn(false); + when(this.mockLogger.isDebugEnabled()).thenReturn(false); - factoryBeanSupport.logDebug(() -> "test"); + this.factoryBeanSupport.logDebug(() -> "test"); - verify(mockLog, times(1)).isDebugEnabled(); - verify(mockLog, never()).debug(any()); + verify(this.mockLogger, times(1)).isDebugEnabled(); + verify(this.mockLogger, never()).debug(any()); + verifyNoMoreInteractions(this.mockLogger); } @Test public void suppressesInfoLoggingWhenInfoIsDisabled() { - when(mockLog.isInfoEnabled()).thenReturn(false); + when(this.mockLogger.isInfoEnabled()).thenReturn(false); - factoryBeanSupport.logInfo(() -> "test"); + this.factoryBeanSupport.logInfo(() -> "test"); - verify(mockLog, times(1)).isInfoEnabled(); - verify(mockLog, never()).info(any()); + verify(this.mockLogger, times(1)).isInfoEnabled(); + verify(this.mockLogger, never()).info(any()); + verifyNoMoreInteractions(this.mockLogger); } @Test public void suppressesWarnLoggingWhenWarnIsDisabled() { - when(mockLog.isWarnEnabled()).thenReturn(false); + when(this.mockLogger.isWarnEnabled()).thenReturn(false); - factoryBeanSupport.logWarning(() -> "test"); + this.factoryBeanSupport.logWarning(() -> "test"); - verify(mockLog, times(1)).isWarnEnabled(); - verify(mockLog, never()).warn(any()); + verify(this.mockLogger, times(1)).isWarnEnabled(); + verify(this.mockLogger, never()).warn(any()); + verifyNoMoreInteractions(this.mockLogger); } @Test - public void suppressesErrorLoggingWhenInfoIsDisabled() { + public void suppressesErrorLoggingWhenErrorIsDisabled() { - when(mockLog.isErrorEnabled()).thenReturn(false); + when(this.mockLogger.isErrorEnabled()).thenReturn(false); - factoryBeanSupport.logError(() -> "test"); + this.factoryBeanSupport.logError(() -> "test"); - verify(mockLog, times(1)).isErrorEnabled(); - verify(mockLog, never()).error(any()); + verify(this.mockLogger, times(1)).isErrorEnabled(); + verify(this.mockLogger, never()).error(any()); + verifyNoMoreInteractions(this.mockLogger); } private static class TestFactoryBeanSupport extends AbstractFactoryBeanSupport {