Spring/spring-data-geode
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Review and refactor the AbstractFactoryBeanSupport class.

Browse Source 
* Fix bug in the filter of the is<LogLevel>LoggingEnabled() methods.
* Rename the getLog() method to getLogger().
* Rename the getOptionalLog() method to getOptionalLogger().
* Edit Javadoc.
* Annotate the API with Spring's @NonNull and @Nullable annotations.
This commit is contained in:
John Blum
2022-03-30 15:15:12 -07:00
parent a6131d8e35
commit fd3d8ca429
3 changed files with 118 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files

96
spring-data-geode/src/main/java/org/springframework/data/gemfire/support/AbstractFactoryBeanSupport.java
View File

@@ -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<T>
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<T>
}
/**
* 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<Logger> getOptionalLog() {
return Optional.ofNullable(getLog());
protected Optional<Logger> getOptionalLogger() {
return Optional.ofNullable(getLogger());
}
/**
@@ -177,10 +181,10 @@ public abstract class AbstractFactoryBeanSupport<T>
*
* @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<T>
*
* @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<T>
*
* @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<T>
* @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<String> message) {
getOptionalLog()
getOptionalLogger()
.filter(Logger::isDebugEnabled)
.ifPresent(log -> log.debug(message.get()));
}
@@ -269,10 +273,10 @@ public abstract class AbstractFactoryBeanSupport<T>
* @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<String> message) {
getOptionalLog()
getOptionalLogger()
.filter(Logger::isInfoEnabled)
.ifPresent(log -> log.info(message.get()));
}
@@ -294,10 +298,10 @@ public abstract class AbstractFactoryBeanSupport<T>
* @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<String> message) {
getOptionalLog()
getOptionalLogger()
.filter(Logger::isWarnEnabled)
.ifPresent(log -> log.warn(message.get()));
}
@@ -319,10 +323,10 @@ public abstract class AbstractFactoryBeanSupport<T>
* @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<String> message) {
getOptionalLog()
getOptionalLogger()
.filter(Logger::isErrorEnabled)
.ifPresent(log -> log.error(message.get()));
}

4
spring-data-geode/src/test/java/org/springframework/data/gemfire/IndexFactoryBeanUnitTests.java
View File

@@ -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;
}
});

122
spring-data-geode/src/test/java/org/springframework/data/gemfire/support/AbstractFactoryBeanSupportUnitTests.java
View File

@@ -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<T> extends AbstractFactoryBeanSupport<T> {