From 22ed0a1f10b2947ff2fbf632465bb4cfad35f548 Mon Sep 17 00:00:00 2001 From: John Blum Date: Wed, 8 Apr 2020 20:11:29 -0700 Subject: [PATCH] DATAGEODE-326 - Fix possible NullPointerException. Additionally, added the nullSafeType(:Class) and nullSafeType(:Class, defaultType:Class) methods. Added the getOrder(:Object) method. Applied the Spring @NonNull and @Nullable annotations appropriately. --- .../data/gemfire/util/SpringUtils.java | 47 ++++++++++++--- .../gemfire/util/SpringUtilsUnitTests.java | 59 +++++++++++++++++-- 2 files changed, 92 insertions(+), 14 deletions(-) diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/SpringUtils.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/SpringUtils.java index 833e5731..0bbfb625 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/SpringUtils.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/SpringUtils.java @@ -56,9 +56,15 @@ import org.springframework.util.StringUtils; * @author John Blum * @see java.lang.Class * @see java.lang.Object + * @see java.util.function.Function + * @see java.util.stream.Stream * @see org.springframework.beans.factory.BeanFactory + * @see org.springframework.beans.factory.FactoryBean * @see org.springframework.beans.factory.config.BeanDefinition * @see org.springframework.beans.factory.config.RuntimeBeanReference + * @see org.springframework.core.Ordered + * @see org.springframework.core.annotation.AnnotationAwareOrderComparator + * @see org.springframework.core.annotation.Order * @since 1.8.0 */ @SuppressWarnings("unused") @@ -77,7 +83,7 @@ public abstract class SpringUtils { * @see java.lang.Class * @see java.lang.String */ - public static boolean isMatchingBean(BeanFactory beanFactory, String beanName, Class beanType) { + public static boolean isMatchingBean(@NonNull BeanFactory beanFactory, String beanName, Class beanType) { return beanFactory.containsBean(beanName) && beanFactory.isTypeMatch(beanName, beanType); } @@ -90,11 +96,12 @@ public abstract class SpringUtils { * @return the given {@link BeanDefinition}. * @see org.springframework.beans.factory.config.BeanDefinition */ + @NonNull public static BeanDefinition addDependsOn(@NonNull BeanDefinition beanDefinition, @Nullable String... beanNames) { List dependsOnList = new ArrayList<>(); - Collections.addAll(dependsOnList, nullSafeArray(beanDefinition.getDependsOn(), String.class)); + Collections.addAll(dependsOnList, ArrayUtils.nullSafeArray(beanDefinition.getDependsOn(), String.class)); dependsOnList.addAll(Arrays.asList(nullSafeArray(beanNames, String.class))); beanDefinition.setDependsOn(dependsOnList.toArray(new String[0])); @@ -169,6 +176,7 @@ public abstract class SpringUtils { } // No Javadoc + @NonNull private static List> orderUnorderedBeans(@NonNull ConfigurableListableBeanFactory beanFactory, @NonNull Map beansOfType, @NonNull Set unorderedBeanNames) { @@ -200,18 +208,30 @@ public abstract class SpringUtils { T bean = beanEntry.getValue(); - Integer order = bean instanceof Ordered - ? ((Ordered) bean).getOrder() - : Optional.ofNullable(bean) - .map(Object::getClass) - .map(OrderUtils::getOrder) - .orElse(null); + Integer order = getOrder(bean); + + if (order == null) { + order = bean != null ? OrderUtils.getOrder(bean.getClass()) : null; + } return order != null ? DefaultOrderedBeanWrapper.from(beanEntry.getKey(), bean, order) : null; } + /** + * Null-safe operation to return the {@link Integer order} of the given {@link Object} if it is {@link Ordered} + * or {@literal null} if the given {@link Object} is not {@link Ordered}. + * + * @param target {@link Object} to evaluate; may be {@literal null}. + * @return the {@link Integer order} of the given {@link Object} if {@link Ordered}, + * otherwise return {@literal null}. + * @see org.springframework.core.Ordered + */ + public static @Nullable Integer getOrder(@Nullable Object target) { + return target instanceof Ordered ? ((Ordered) target).getOrder() : null; + } + /** * Returns bean of the given {@link Class type} in an ordered {@link Stream}. * @@ -223,7 +243,8 @@ public abstract class SpringUtils { * @see java.util.stream.Stream * @see java.lang.Class */ - public static Stream getOrderedStreamOfBeansByType(BeanFactory beanFactory, Class beanType) { + public static Stream getOrderedStreamOfBeansByType(@NonNull BeanFactory beanFactory, + @NonNull Class beanType) { Assert.notNull(beanFactory, "BeanFactory must not be null"); Assert.notNull(beanType,"Bean type must not be null"); @@ -295,6 +316,14 @@ public abstract class SpringUtils { return type != null ? type.getSimpleName() : null; } + public static Class nullSafeType(Object target) { + return nullSafeType(target, null); + } + + public static Class nullSafeType(Object target, Class defaultType) { + return target != null ? target.getClass() : defaultType; + } + public static boolean safeDoOperation(VoidReturningThrowableOperation operation) { try { diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/SpringUtilsUnitTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/SpringUtilsUnitTests.java index ea77bf1e..9819a11a 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/SpringUtilsUnitTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/SpringUtilsUnitTests.java @@ -31,6 +31,7 @@ import static org.springframework.data.gemfire.util.ArrayUtils.asArray; import static org.springframework.data.gemfire.util.RuntimeExceptionFactory.newIllegalStateException; import static org.springframework.data.gemfire.util.RuntimeExceptionFactory.newRuntimeException; +import java.sql.Time; import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -50,6 +51,7 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.data.gemfire.test.model.Person; import org.springframework.data.gemfire.util.SpringUtils.ValueReturningThrowableOperation; /** @@ -216,7 +218,6 @@ public class SpringUtilsUnitTests { } @Test - @SuppressWarnings("all") public void setBeanDefinitionPropertyReference() { MutablePropertyValues mutablePropertyValues = new MutablePropertyValues(); @@ -236,7 +237,6 @@ public class SpringUtilsUnitTests { } @Test - @SuppressWarnings("all") public void setBeanDefinitionPropertyValue() { MutablePropertyValues mutablePropertyValues = new MutablePropertyValues(); @@ -336,7 +336,6 @@ public class SpringUtilsUnitTests { } @Test - @SuppressWarnings("all") public void equalsIgnoreNullIsFalse() { assertThat(SpringUtils.equalsIgnoreNull(null, "null")).isFalse(); @@ -358,7 +357,6 @@ public class SpringUtilsUnitTests { } @Test - @SuppressWarnings("all") public void nullOrEqualsWithNullIsTrue() { assertThat(SpringUtils.nullOrEquals(null, "test")).isTrue(); } @@ -374,7 +372,6 @@ public class SpringUtilsUnitTests { } @Test - @SuppressWarnings("all") public void nullSafeEqualsWithNullObjectsIsFalse() { assertThat(SpringUtils.nullSafeEquals(null, "test")).isFalse(); assertThat(SpringUtils.nullSafeEquals("test", null)).isFalse(); @@ -385,6 +382,58 @@ public class SpringUtilsUnitTests { assertThat(SpringUtils.nullSafeEquals("test", "mock")).isFalse(); } + @Test + public void nullSafeNameWithType() { + + assertThat(SpringUtils.nullSafeName(Boolean.class)).isEqualTo(Boolean.class.getName()); + assertThat(SpringUtils.nullSafeName(Integer.class)).isEqualTo(Integer.class.getName()); + assertThat(SpringUtils.nullSafeName(Double.class)).isEqualTo(Double.class.getName()); + assertThat(SpringUtils.nullSafeName(String.class)).isEqualTo(String.class.getName()); + assertThat(SpringUtils.nullSafeName(Time.class)).isEqualTo(Time.class.getName()); + assertThat(SpringUtils.nullSafeName(Person.class)).isEqualTo(Person.class.getName()); + } + + @Test + public void nullSafeNameWithNull() { + assertThat(SpringUtils.nullSafeName(null)).isNull(); + } + + @Test + public void nullSafeSimpleNameWithType() { + + assertThat(SpringUtils.nullSafeSimpleName(Boolean.class)).isEqualTo(Boolean.class.getSimpleName()); + assertThat(SpringUtils.nullSafeSimpleName(Integer.class)).isEqualTo(Integer.class.getSimpleName()); + assertThat(SpringUtils.nullSafeSimpleName(Double.class)).isEqualTo(Double.class.getSimpleName()); + assertThat(SpringUtils.nullSafeSimpleName(String.class)).isEqualTo(String.class.getSimpleName()); + assertThat(SpringUtils.nullSafeSimpleName(Time.class)).isEqualTo(Time.class.getSimpleName()); + assertThat(SpringUtils.nullSafeSimpleName(Person.class)).isEqualTo(Person.class.getSimpleName()); + } + + @Test + public void nullSafeSimpleNameWithNull() { + assertThat(SpringUtils.nullSafeSimpleName(null)).isNull(); + } + + @Test + public void nullSafeTypeWithObject() { + assertThat(SpringUtils.nullSafeType(new Object())).isEqualTo(Object.class); + } + + @Test + public void nullSafeTypeWithObjectAndDefaultType() { + assertThat(SpringUtils.nullSafeType("test", Person.class)).isEqualTo(String.class); + } + + @Test + public void nullSafeTypeWithNull() { + assertThat(SpringUtils.nullSafeType(null)).isNull(); + } + + @Test + public void nullSafeTypeWithNullAndDefaultType() { + assertThat(SpringUtils.nullSafeType(null, Person.class)).isEqualTo(Person.class); + } + @Test public void safeDoOperationWithNonThrowingOperation() {