From 2da7ae016d14fc534ff457e5cc6a0ecea7ebef22 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Thu, 21 Nov 2019 12:11:30 -0500 Subject: [PATCH] Add NPE test for ExprEvalSqlParameterSourceFactory Related to https://github.com/spring-projects/spring-integration/issues/3113 Also clean up code style in the `ExpressionEvaluatingSqlParameterSourceFactory` --- ...onEvaluatingSqlParameterSourceFactory.java | 20 +-- ...luatingSqlParameterSourceFactoryTests.java | 119 ++++++++++-------- 2 files changed, 78 insertions(+), 61 deletions(-) diff --git a/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/ExpressionEvaluatingSqlParameterSourceFactory.java b/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/ExpressionEvaluatingSqlParameterSourceFactory.java index cf4f3e6aef..ca04187da4 100644 --- a/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/ExpressionEvaluatingSqlParameterSourceFactory.java +++ b/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/ExpressionEvaluatingSqlParameterSourceFactory.java @@ -22,11 +22,10 @@ import java.util.Map; import org.springframework.expression.Expression; import org.springframework.expression.ExpressionException; -import org.springframework.expression.ExpressionParser; -import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.integration.util.AbstractExpressionEvaluator; import org.springframework.jdbc.core.namedparam.AbstractSqlParameterSource; import org.springframework.jdbc.core.namedparam.SqlParameterSource; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; /** @@ -45,8 +44,6 @@ import org.springframework.util.Assert; public class ExpressionEvaluatingSqlParameterSourceFactory extends AbstractExpressionEvaluator implements SqlParameterSourceFactory { - private static final ExpressionParser PARSER = new SpelExpressionParser(); - private static final Object ERROR = new Object(); private final Map staticParameters = new HashMap<>(); @@ -117,8 +114,8 @@ public class ExpressionEvaluatingSqlParameterSourceFactory extends AbstractExpre String key = entry.getKey(); String expression = entry.getValue(); Expression[] expressions = new Expression[] { - PARSER.parseExpression(expression), - PARSER.parseExpression("#root.![" + expression + "]") + EXPRESSION_PARSER.parseExpression(expression), + EXPRESSION_PARSER.parseExpression("#root.![" + expression + "]") }; paramExpressions.put(key, expressions); } @@ -172,6 +169,7 @@ public class ExpressionEvaluatingSqlParameterSourceFactory extends AbstractExpre ExpressionEvaluatingSqlParameterSource(Object input, Map staticParameters, Map parameterExpressions, boolean cache) { + this.input = input; this.parameterExpressions = parameterExpressions; this.values.putAll(staticParameters); @@ -182,10 +180,12 @@ public class ExpressionEvaluatingSqlParameterSourceFactory extends AbstractExpre } @Override + @Nullable public Object getValue(String paramName) throws IllegalArgumentException { - return this.doGetValue(paramName, false); + return doGetValue(paramName, false); } + @Nullable public Object doGetValue(String paramName, boolean calledFromHasValue) throws IllegalArgumentException { if (this.values.containsKey(paramName)) { Object cachedByHasValue = this.values.get(paramName); @@ -197,14 +197,14 @@ public class ExpressionEvaluatingSqlParameterSourceFactory extends AbstractExpre if (!this.parameterExpressions.containsKey(paramName)) { Expression[] expressions = new Expression[] { - PARSER.parseExpression(paramName), - PARSER.parseExpression("#root.![" + paramName + "]") + EXPRESSION_PARSER.parseExpression(paramName), + EXPRESSION_PARSER.parseExpression("#root.![" + paramName + "]") }; ExpressionEvaluatingSqlParameterSourceFactory.this.parameterExpressions.put(paramName, expressions); this.parameterExpressions.put(paramName, expressions); } - Expression expression = null; + Expression expression; if (this.input instanceof Collection) { expression = this.parameterExpressions.get(paramName)[1]; diff --git a/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/ExpressionEvaluatingSqlParameterSourceFactoryTests.java b/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/ExpressionEvaluatingSqlParameterSourceFactoryTests.java index d284fbb0f9..4f9eb4b9c9 100644 --- a/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/ExpressionEvaluatingSqlParameterSourceFactoryTests.java +++ b/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/ExpressionEvaluatingSqlParameterSourceFactoryTests.java @@ -23,7 +23,7 @@ import java.sql.Types; import java.util.Arrays; import java.util.Collections; -import org.junit.Test; +import org.junit.jupiter.api.Test; import org.springframework.beans.factory.BeanFactory; import org.springframework.jdbc.core.namedparam.SqlParameterSource; @@ -32,39 +32,41 @@ import org.springframework.jdbc.support.JdbcUtils; /** * @author Dave Syer * @author Meherzad Lahewala + * @author Artem Bilan */ -public class ExpressionEvaluatingSqlParameterSourceFactoryTests { +class ExpressionEvaluatingSqlParameterSourceFactoryTests { private final ExpressionEvaluatingSqlParameterSourceFactory factory = new ExpressionEvaluatingSqlParameterSourceFactory(); @Test - public void testSetStaticParameters() throws Exception { - factory.setStaticParameters(Collections.singletonMap("foo", "bar")); - factory.setBeanFactory(mock(BeanFactory.class)); - factory.afterPropertiesSet(); - SqlParameterSource source = factory.createParameterSource(null); + void testSetStaticParameters() { + this.factory.setStaticParameters(Collections.singletonMap("foo", "bar")); + this.factory.setBeanFactory(mock(BeanFactory.class)); + this.factory.afterPropertiesSet(); + SqlParameterSource source = this.factory.createParameterSource(null); assertThat(source.hasValue("foo")).isTrue(); assertThat(source.getValue("foo")).isEqualTo("bar"); assertThat(source.getSqlType("foo")).isEqualTo(JdbcUtils.TYPE_UNKNOWN); } @Test - public void testMapInput() throws Exception { - factory.setBeanFactory(mock(BeanFactory.class)); - factory.afterPropertiesSet(); - SqlParameterSource source = factory.createParameterSource(Collections.singletonMap("foo", "bar")); + void testMapInput() { + this.factory.setBeanFactory(mock(BeanFactory.class)); + this.factory.afterPropertiesSet(); + SqlParameterSource source = this.factory.createParameterSource(Collections.singletonMap("foo", "bar")); assertThat(source.hasValue("foo")).isTrue(); assertThat(source.getValue("foo")).isEqualTo("bar"); assertThat(source.getSqlType("foo")).isEqualTo(JdbcUtils.TYPE_UNKNOWN); } @Test - public void testListOfMapsInput() throws Exception { - factory.setBeanFactory(mock(BeanFactory.class)); - factory.afterPropertiesSet(); - SqlParameterSource source = factory.createParameterSource(Arrays.asList(Collections.singletonMap("foo", "bar"), - Collections.singletonMap("foo", "bucket"))); + void testListOfMapsInput() { + this.factory.setBeanFactory(mock(BeanFactory.class)); + this.factory.afterPropertiesSet(); + SqlParameterSource source = + this.factory.createParameterSource(Arrays.asList(Collections.singletonMap("foo", "bar"), + Collections.singletonMap("foo", "bucket"))); String expression = "foo"; assertThat(source.hasValue(expression)).isTrue(); assertThat(source.getValue(expression).toString()).isEqualTo("[bar, bucket]"); @@ -72,10 +74,10 @@ public class ExpressionEvaluatingSqlParameterSourceFactoryTests { } @Test - public void testMapInputWithExpression() throws Exception { - factory.setBeanFactory(mock(BeanFactory.class)); - factory.afterPropertiesSet(); - SqlParameterSource source = factory.createParameterSource(Collections.singletonMap("foo", "bar")); + void testMapInputWithExpression() { + this.factory.setBeanFactory(mock(BeanFactory.class)); + this.factory.afterPropertiesSet(); + SqlParameterSource source = this.factory.createParameterSource(Collections.singletonMap("foo", "bar")); // This is an illegal parameter name in Spring JDBC so we'd never get this as input assertThat(source.hasValue("foo.toUpperCase()")).isTrue(); assertThat(source.getValue("foo.toUpperCase()")).isEqualTo("BAR"); @@ -83,35 +85,37 @@ public class ExpressionEvaluatingSqlParameterSourceFactoryTests { } @Test - public void testMapInputWithMappedExpression() throws Exception { - factory.setParameterExpressions(Collections.singletonMap("spam", "foo.toUpperCase()")); - factory.setBeanFactory(mock(BeanFactory.class)); - factory.afterPropertiesSet(); - SqlParameterSource source = factory.createParameterSource(Collections.singletonMap("foo", "bar")); + void testMapInputWithMappedExpression() { + this.factory.setParameterExpressions(Collections.singletonMap("spam", "foo.toUpperCase()")); + this.factory.setBeanFactory(mock(BeanFactory.class)); + this.factory.afterPropertiesSet(); + SqlParameterSource source = this.factory.createParameterSource(Collections.singletonMap("foo", "bar")); assertThat(source.hasValue("spam")).isTrue(); assertThat(source.getValue("spam")).isEqualTo("BAR"); assertThat(source.getSqlType("spam")).isEqualTo(JdbcUtils.TYPE_UNKNOWN); } @Test - public void testMapInputWithMappedExpressionResolveStatic() throws Exception { - factory.setParameterExpressions(Collections.singletonMap("spam", "#staticParameters['foo'].toUpperCase()")); - factory.setStaticParameters(Collections.singletonMap("foo", "bar")); - factory.setBeanFactory(mock(BeanFactory.class)); - factory.afterPropertiesSet(); - SqlParameterSource source = factory.createParameterSource(Collections.singletonMap("crap", "bucket")); + void testMapInputWithMappedExpressionResolveStatic() { + this.factory.setParameterExpressions( + Collections.singletonMap("spam", "#staticParameters['foo'].toUpperCase()")); + this.factory.setStaticParameters(Collections.singletonMap("foo", "bar")); + this.factory.setBeanFactory(mock(BeanFactory.class)); + this.factory.afterPropertiesSet(); + SqlParameterSource source = this.factory.createParameterSource(Collections.singletonMap("crap", "bucket")); assertThat(source.hasValue("spam")).isTrue(); assertThat(source.getValue("spam")).isEqualTo("BAR"); assertThat(source.getSqlType("spam")).isEqualTo(JdbcUtils.TYPE_UNKNOWN); } @Test - public void testListOfMapsInputWithExpression() throws Exception { - factory.setParameterExpressions(Collections.singletonMap("spam", "foo.toUpperCase()")); - factory.setBeanFactory(mock(BeanFactory.class)); - factory.afterPropertiesSet(); - SqlParameterSource source = factory.createParameterSource(Arrays.asList(Collections.singletonMap("foo", "bar"), - Collections.singletonMap("foo", "bucket"))); + void testListOfMapsInputWithExpression() { + this.factory.setParameterExpressions(Collections.singletonMap("spam", "foo.toUpperCase()")); + this.factory.setBeanFactory(mock(BeanFactory.class)); + this.factory.afterPropertiesSet(); + SqlParameterSource source = + this.factory.createParameterSource(Arrays.asList(Collections.singletonMap("foo", "bar"), + Collections.singletonMap("foo", "bucket"))); String expression = "spam"; assertThat(source.hasValue(expression)).isTrue(); assertThat(source.getValue(expression).toString()).isEqualTo("[BAR, BUCKET]"); @@ -119,13 +123,14 @@ public class ExpressionEvaluatingSqlParameterSourceFactoryTests { } @Test - public void testListOfMapsInputWithExpressionAndTypes() throws Exception { - factory.setParameterExpressions(Collections.singletonMap("spam", "foo.toUpperCase()")); - factory.setBeanFactory(mock(BeanFactory.class)); - factory.setSqlParameterTypes(Collections.singletonMap("spam", Types.SQLXML)); - factory.afterPropertiesSet(); - SqlParameterSource source = factory.createParameterSource(Arrays.asList(Collections.singletonMap("foo", "bar"), - Collections.singletonMap("foo", "bucket"))); + void testListOfMapsInputWithExpressionAndTypes() { + this.factory.setParameterExpressions(Collections.singletonMap("spam", "foo.toUpperCase()")); + this.factory.setBeanFactory(mock(BeanFactory.class)); + this.factory.setSqlParameterTypes(Collections.singletonMap("spam", Types.SQLXML)); + this.factory.afterPropertiesSet(); + SqlParameterSource source = + this.factory.createParameterSource(Arrays.asList(Collections.singletonMap("foo", "bar"), + Collections.singletonMap("foo", "bucket"))); String expression = "spam"; assertThat(source.hasValue(expression)).isTrue(); assertThat(source.getValue(expression).toString()).isEqualTo("[BAR, BUCKET]"); @@ -133,17 +138,29 @@ public class ExpressionEvaluatingSqlParameterSourceFactoryTests { } @Test - public void testListOfMapsInputWithExpressionAndEmptyTypes() throws Exception { - factory.setParameterExpressions(Collections.singletonMap("spam", "foo.toUpperCase()")); - factory.setBeanFactory(mock(BeanFactory.class)); - factory.setSqlParameterTypes(Collections.emptyMap()); - factory.afterPropertiesSet(); - SqlParameterSource source = factory.createParameterSource(Arrays.asList(Collections.singletonMap("foo", "bar"), - Collections.singletonMap("foo", "bucket"))); + void testListOfMapsInputWithExpressionAndEmptyTypes() { + this.factory.setParameterExpressions(Collections.singletonMap("spam", "foo.toUpperCase()")); + this.factory.setBeanFactory(mock(BeanFactory.class)); + this.factory.setSqlParameterTypes(Collections.emptyMap()); + this.factory.afterPropertiesSet(); + SqlParameterSource source = + this.factory.createParameterSource(Arrays.asList(Collections.singletonMap("foo", "bar"), + Collections.singletonMap("foo", "bucket"))); String expression = "spam"; assertThat(source.hasValue(expression)).isTrue(); assertThat(source.getValue(expression).toString()).isEqualTo("[BAR, BUCKET]"); assertThat(source.getSqlType("spam")).isEqualTo(JdbcUtils.TYPE_UNKNOWN); } + @Test + void testNullValue() { + this.factory.setStaticParameters(Collections.singletonMap("foo", null)); + this.factory.setBeanFactory(mock(BeanFactory.class)); + this.factory.afterPropertiesSet(); + SqlParameterSource source = this.factory.createParameterSource(null); + assertThat(source.hasValue("foo")).isTrue(); + assertThat(source.getValue("foo")).isNull(); + assertThat(source.getSqlType("foo")).isEqualTo(JdbcUtils.TYPE_UNKNOWN); + } + }