From c368ce82234fd47ae66c4c4598a66d0a8d4ce8d1 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sat, 5 Sep 2020 12:59:12 +0200 Subject: [PATCH 1/3] Fix SpEL generated code for default method invocation Closes gh-25706 --- .../expression/spel/ast/MethodReference.java | 7 +- .../spel/SpelCompilationCoverageTests.java | 2 +- .../spel/standard/SpelCompilerTests.java | 66 ++++++++++++++++++- 3 files changed, 70 insertions(+), 5 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java index 970060cdf31..cbaf1c8d1ad 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -347,8 +347,9 @@ public class MethodReference extends SpelNodeImpl { } generateCodeForArguments(mv, cf, method, this.children); - mv.visitMethodInsn((isStaticMethod ? INVOKESTATIC : INVOKEVIRTUAL), classDesc, method.getName(), - CodeFlow.createSignatureDescriptor(method), method.getDeclaringClass().isInterface()); + mv.visitMethodInsn((isStaticMethod ? INVOKESTATIC : (method.isDefault() ? INVOKEINTERFACE : INVOKEVIRTUAL)), + classDesc, method.getName(), CodeFlow.createSignatureDescriptor(method), + method.getDeclaringClass().isInterface()); cf.pushDescriptor(this.exitTypeDescriptor); if (this.originalPrimitiveExitTypeDescriptor != null) { diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index 2f5ad40e2a8..b53e8dbb0ba 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java @@ -5159,7 +5159,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertThatExceptionOfType(Exception.class).isThrownBy(expression::getValue); } - private void assertIsCompiled(Expression expression) { + public static void assertIsCompiled(Expression expression) { try { Field field = SpelExpression.class.getDeclaredField("compiledAst"); field.setAccessible(true); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelCompilerTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelCompilerTests.java index dc80b9c90de..c230151d562 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelCompilerTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelCompilerTests.java @@ -22,8 +22,10 @@ import org.junit.jupiter.api.Test; import org.springframework.core.Ordered; import org.springframework.expression.Expression; +import org.springframework.expression.spel.SpelCompilationCoverageTests; import org.springframework.expression.spel.SpelCompilerMode; import org.springframework.expression.spel.SpelParserConfiguration; +import org.springframework.expression.spel.support.StandardEvaluationContext; import static org.assertj.core.api.Assertions.assertThat; @@ -31,11 +33,12 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for the {@link SpelCompiler}. * * @author Sam Brannen + * @author Andy Clement * @since 5.1.14 */ class SpelCompilerTests { - @Test // gh-24357 + @Test // gh-24357 void expressionCompilesWhenMethodComesFromPublicInterface() { SpelParserConfiguration config = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null); SpelExpressionParser parser = new SpelExpressionParser(config); @@ -47,6 +50,31 @@ class SpelCompilerTests { IntStream.rangeClosed(1, 5).forEach(i -> assertThat(expression.getValue(component)).isEqualTo(42)); } + @Test // gh-25706 + void defaultMethodInvocation() { + SpelParserConfiguration config = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null); + SpelExpressionParser parser = new SpelExpressionParser(config); + + StandardEvaluationContext context = new StandardEvaluationContext(); + Item item = new Item(); + context.setRootObject(item); + + Expression expression = parser.parseExpression("#root.isEditable2()"); + assertThat(SpelCompiler.compile(expression)).isFalse(); + assertThat(expression.getValue(context)).isEqualTo(false); + assertThat(SpelCompiler.compile(expression)).isTrue(); + SpelCompilationCoverageTests.assertIsCompiled(expression); + assertThat(expression.getValue(context)).isEqualTo(false); + + context.setVariable("user", new User()); + expression = parser.parseExpression("#root.isEditable(#user)"); + assertThat(SpelCompiler.compile(expression)).isFalse(); + assertThat(expression.getValue(context)).isEqualTo(true); + assertThat(SpelCompiler.compile(expression)).isTrue(); + SpelCompilationCoverageTests.assertIsCompiled(expression); + assertThat(expression.getValue(context)).isEqualTo(true); + } + static class OrderedComponent implements Ordered { @@ -56,4 +84,40 @@ class SpelCompilerTests { } } + + public static class User { + + boolean isAdmin() { + return true; + } + } + + + public static class Item implements Editable { + + // some fields + private String someField = ""; + + // some getters and setters + + @Override + public boolean hasSomeProperty() { + return someField != null; + } + } + + + public interface Editable { + + default boolean isEditable(User user) { + return user.isAdmin() && hasSomeProperty(); + } + + default boolean isEditable2() { + return false; + } + + boolean hasSomeProperty(); + } + } From 40bf83c9e585bcca99dc321f071f69f4e7cdffa9 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sat, 5 Sep 2020 12:59:34 +0200 Subject: [PATCH 2/3] Restore original 4.x behavior for initialization of function return name Closes gh-25707 --- .../jdbc/core/metadata/CallMetaDataContext.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/CallMetaDataContext.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/CallMetaDataContext.java index b707d6d3150..c8295860ac5 100755 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/CallMetaDataContext.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/CallMetaDataContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -355,7 +355,7 @@ public class CallMetaDataContext { logger.debug("Using declared out parameter '" + paramName + "' for function return value"); } - setFunctionReturnName(paramName); + this.actualFunctionReturnName = paramName; returnDeclared = true; } } @@ -393,8 +393,8 @@ public class CallMetaDataContext { "Unable to locate declared parameter for function return value - " + " add an SqlOutParameter with name '" + getFunctionReturnName() + "'"); } - else if (paramName != null) { - setFunctionReturnName(paramName); + else { + this.actualFunctionReturnName = param.getName(); } } else { @@ -422,7 +422,7 @@ public class CallMetaDataContext { (StringUtils.hasLength(paramNameToUse) ? paramNameToUse : getFunctionReturnName()); workParams.add(provider.createDefaultOutParameter(returnNameToUse, meta)); if (isFunction()) { - setFunctionReturnName(returnNameToUse); + this.actualFunctionReturnName = returnNameToUse; outParamNames.add(returnNameToUse); } if (logger.isDebugEnabled()) { From 939c76c4a52455349cbbc21c1a8639629f8fed80 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sat, 5 Sep 2020 13:00:06 +0200 Subject: [PATCH 3/3] Revise documentation notes on getParameterType performance issues See gh-25679 --- .../jdbc/core/StatementCreatorUtils.java | 13 ++++++------- src/docs/asciidoc/data-access.adoc | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/StatementCreatorUtils.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/StatementCreatorUtils.java index 4d9da27bb55..5c6c1d859d2 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/StatementCreatorUtils.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/StatementCreatorUtils.java @@ -66,12 +66,11 @@ public abstract class StatementCreatorUtils { * completely, i.e. to never even attempt to retrieve {@link PreparedStatement#getParameterMetaData()} * for {@link StatementCreatorUtils#setNull} calls. *

The default is "false", trying {@code getParameterType} calls first and falling back to - * {@link PreparedStatement#setNull} / {@link PreparedStatement#setObject} calls based on well-known - * behavior of common databases. Spring records JDBC drivers with non-working {@code getParameterType} - * implementations and won't attempt to call that method for that driver again, always falling back. - *

Consider switching this flag to "true" if you experience misbehavior at runtime, e.g. with - * a connection pool setting back the {@link PreparedStatement} instance in case of an exception - * thrown from {@code getParameterType} (as reported on JBoss AS 7). + * {@link PreparedStatement#setNull} / {@link PreparedStatement#setObject} calls based on + * well-known behavior of common databases. + *

Consider switching this flag to "true" if you experience misbehavior at runtime, + * e.g. with connection pool issues in case of an exception thrown from {@code getParameterType} + * (as reported on JBoss AS 7) or in case of performance problems (as reported on PostgreSQL). */ public static final String IGNORE_GETPARAMETERTYPE_PROPERTY_NAME = "spring.jdbc.getParameterType.ignore"; @@ -266,7 +265,7 @@ public abstract class StatementCreatorUtils { } else if (databaseProductName.startsWith("DB2") || jdbcDriverName.startsWith("jConnect") || - jdbcDriverName.startsWith("SQLServer")|| + jdbcDriverName.startsWith("SQLServer") || jdbcDriverName.startsWith("Apache Derby")) { sqlTypeToUse = Types.VARCHAR; } diff --git a/src/docs/asciidoc/data-access.adoc b/src/docs/asciidoc/data-access.adoc index f5312a7b0ac..62e41acb32d 100644 --- a/src/docs/asciidoc/data-access.adoc +++ b/src/docs/asciidoc/data-access.adoc @@ -4449,7 +4449,7 @@ While this usually works well, there is a potential for issues (for example, wit case, which can be expensive with your JDBC driver. You should use a recent driver version and consider setting the `spring.jdbc.getParameterType.ignore` property to `true` (as a JVM system property or in a `spring.properties` file in the root of your classpath) -if you encounter a performance issue -- for example, as reported on Oracle 12c (SPR-16139). +if you encounter a performance issue (as reported on Oracle 12c, JBoss and PostgreSQL). Alternatively, you might consider specifying the corresponding JDBC types explicitly, either through a 'BatchPreparedStatementSetter' (as shown earlier), through an explicit type