From 5459304a4b5e8dc6ce08ff8d9e09228ab7d72659 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 10 Oct 2023 21:54:58 +0200 Subject: [PATCH 1/3] Re-introduce support for annotation declarations with self references Closes gh-31400 --- .../annotation/AnnotationTypeMapping.java | 9 +- .../KotlinMergedAnnotationsTests.kt | 86 +++++++++++++++++++ .../core/annotation/PersonWithAlias.kt | 31 +++++++ .../core/annotation/PersonWithoutAlias.kt | 29 +++++++ 4 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 spring-core/src/test/kotlin/org/springframework/core/annotation/KotlinMergedAnnotationsTests.kt create mode 100644 spring-core/src/test/kotlin/org/springframework/core/annotation/PersonWithAlias.kt create mode 100644 spring-core/src/test/kotlin/org/springframework/core/annotation/PersonWithoutAlias.kt diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java index 05039aa8d3f..794aa33feb4 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java @@ -45,6 +45,7 @@ import org.springframework.util.StringUtils; * * @author Phillip Webb * @author Sam Brannen + * @author Juergen Hoeller * @since 5.2 * @see AnnotationTypeMappings */ @@ -402,9 +403,11 @@ final class AnnotationTypeMapping { if (type.isAnnotation() || (type.isArray() && type.getComponentType().isAnnotation())) { Class annotationType = (Class) (type.isAnnotation() ? type : type.getComponentType()); - AnnotationTypeMapping mapping = AnnotationTypeMappings.forAnnotationType(annotationType).get(0); - if (mapping.isSynthesizable()) { - return true; + if (annotationType != this.annotationType) { + AnnotationTypeMapping mapping = AnnotationTypeMappings.forAnnotationType(annotationType).get(0); + if (mapping.isSynthesizable()) { + return true; + } } } } diff --git a/spring-core/src/test/kotlin/org/springframework/core/annotation/KotlinMergedAnnotationsTests.kt b/spring-core/src/test/kotlin/org/springframework/core/annotation/KotlinMergedAnnotationsTests.kt new file mode 100644 index 00000000000..2b3cc397c67 --- /dev/null +++ b/spring-core/src/test/kotlin/org/springframework/core/annotation/KotlinMergedAnnotationsTests.kt @@ -0,0 +1,86 @@ +/* + * Copyright 2002-2023 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.core.annotation + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +/** + * Tests for {@link MergedAnnotations} and {@link MergedAnnotation} in Kotlin. + * + * @author Sam Brannen + * @author Juergen Hoeller + * @since 5.3.16 + */ +class KotlinMergedAnnotationsTests { + + @Test // gh-28012 + fun recursiveAnnotationWithAlias() { + val method = javaClass.getMethod("personWithAliasMethod") + + // MergedAnnotations + val mergedAnnotations = MergedAnnotations.from(method) + assertThat(mergedAnnotations.isPresent(PersonWithAlias::class.java)).isTrue(); + + // MergedAnnotation + val mergedAnnotation = MergedAnnotation.from(method.getAnnotation(PersonWithAlias::class.java)) + assertThat(mergedAnnotation).isNotNull(); + + // Synthesized Annotations + val jane = mergedAnnotation.synthesize() + assertThat(jane.value).isEqualTo("jane") + assertThat(jane.name).isEqualTo("jane") + val synthesizedFriends = jane.friends + assertThat(synthesizedFriends).hasSize(2) + + val john = synthesizedFriends[0] + assertThat(john.value).isEqualTo("john") + assertThat(john.name).isEqualTo("john") + + val sally = synthesizedFriends[1] + assertThat(sally.value).isEqualTo("sally") + assertThat(sally.name).isEqualTo("sally") + } + + @Test // gh-31400 + fun recursiveAnnotationWithoutAlias() { + val method = javaClass.getMethod("personWithoutAliasMethod") + + // MergedAnnotations + val mergedAnnotations = MergedAnnotations.from(method) + assertThat(mergedAnnotations.isPresent(PersonWithoutAlias::class.java)).isTrue(); + + // MergedAnnotation + val mergedAnnotation = MergedAnnotation.from(method.getAnnotation(PersonWithoutAlias::class.java)) + assertThat(mergedAnnotation).isNotNull(); + + // Synthesized Annotations + val jane = mergedAnnotation.synthesize() + val synthesizedFriends = jane.friends + assertThat(synthesizedFriends).hasSize(2) + } + + + @PersonWithAlias("jane", friends = [PersonWithAlias("john"), PersonWithAlias("sally")]) + fun personWithAliasMethod() { + } + + @PersonWithoutAlias("jane", friends = [PersonWithoutAlias("john"), PersonWithoutAlias("sally")]) + fun personWithoutAliasMethod() { + } + +} diff --git a/spring-core/src/test/kotlin/org/springframework/core/annotation/PersonWithAlias.kt b/spring-core/src/test/kotlin/org/springframework/core/annotation/PersonWithAlias.kt new file mode 100644 index 00000000000..249a2b83c14 --- /dev/null +++ b/spring-core/src/test/kotlin/org/springframework/core/annotation/PersonWithAlias.kt @@ -0,0 +1,31 @@ +/* + * Copyright 2002-2023 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.core.annotation + +@Target(AnnotationTarget.FUNCTION) +@Retention(AnnotationRetention.RUNTIME) +annotation class PersonWithAlias( + + @get:AliasFor("name") + val value: String = "", + + @get:AliasFor("value") + val name: String = "", + + vararg val friends: PersonWithAlias = [] + +) diff --git a/spring-core/src/test/kotlin/org/springframework/core/annotation/PersonWithoutAlias.kt b/spring-core/src/test/kotlin/org/springframework/core/annotation/PersonWithoutAlias.kt new file mode 100644 index 00000000000..75a918f14f7 --- /dev/null +++ b/spring-core/src/test/kotlin/org/springframework/core/annotation/PersonWithoutAlias.kt @@ -0,0 +1,29 @@ +/* + * Copyright 2002-2023 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.core.annotation + +@Target(AnnotationTarget.FUNCTION) +@Retention(AnnotationRetention.RUNTIME) +annotation class PersonWithoutAlias( + + val value: String = "", + + val name: String = "", + + vararg val friends: PersonWithoutAlias = [] + +) From 8b5d993e614ed2f07b80251fa76023f269dd451c Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 10 Oct 2023 21:55:12 +0200 Subject: [PATCH 2/3] Throw IllegalArgumentException for null SQL String Closes gh-31391 --- .../jdbc/core/namedparam/NamedParameterJdbcTemplate.java | 1 + 1 file changed, 1 insertion(+) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplate.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplate.java index d3ccb11fe2d..8a36d209235 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplate.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplate.java @@ -436,6 +436,7 @@ public class NamedParameterJdbcTemplate implements NamedParameterJdbcOperations * @return a representation of the parsed SQL statement */ protected ParsedSql getParsedSql(String sql) { + Assert.notNull(sql, "SQL must not be null"); return this.parsedSqlCache.get(sql); } From 387a16bd4e353caabf6c8521f29e0969c879fdef Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 10 Oct 2023 21:56:14 +0200 Subject: [PATCH 3/3] Revise transaction annotation recommendations Closes gh-23538 --- .../transaction/declarative/annotations.adoc | 49 +++++++++---------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/framework-docs/modules/ROOT/pages/data-access/transaction/declarative/annotations.adoc b/framework-docs/modules/ROOT/pages/data-access/transaction/declarative/annotations.adoc index 52306d54a3e..b5522a6a47e 100644 --- a/framework-docs/modules/ROOT/pages/data-access/transaction/declarative/annotations.adoc +++ b/framework-docs/modules/ROOT/pages/data-access/transaction/declarative/annotations.adoc @@ -229,30 +229,27 @@ in the testing chapter for examples. ==== You can apply the `@Transactional` annotation to an interface definition, a method -on an interface, a class definition, or a method on a class. However, the -mere presence of the `@Transactional` annotation is not enough to activate the -transactional behavior. The `@Transactional` annotation is merely metadata that can -be consumed by some runtime infrastructure that is `@Transactional`-aware and that -can use the metadata to configure the appropriate beans with transactional behavior. -In the preceding example, the `` element switches on the -transactional behavior. +on an interface, a class definition, or a method on a class. However, the mere presence +of the `@Transactional` annotation is not enough to activate the transactional behavior. +The `@Transactional` annotation is merely metadata that can be consumed by corresponding +runtime infrastructure which uses that metadata to configure the appropriate beans with +transactional behavior. In the preceding example, the `` element +switches on actual transaction management at runtime. -TIP: The Spring team recommends that you annotate only concrete classes (and methods of -concrete classes) with the `@Transactional` annotation, as opposed to annotating interfaces. -You certainly can place the `@Transactional` annotation on an interface (or an interface -method), but this works only as you would expect it to if you use interface-based -proxies. The fact that Java annotations are not inherited from interfaces means that, -if you use class-based proxies (`proxy-target-class="true"`) or the weaving-based -aspect (`mode="aspectj"`), the transaction settings are not recognized by the proxying -and weaving infrastructure, and the object is not wrapped in a transactional proxy. +TIP: The Spring team recommends that you annotate methods of concrete classes with the +`@Transactional` annotation, rather than relying on annotated methods in interfaces, +even if the latter does work for interface-based and target-class proxies as of 5.0. +Since Java annotations are not inherited from interfaces, interface-declared annotations +are still not recognized by the weaving infrastructure when using AspectJ mode, so the +aspect does not get applied. As a consequence, your transaction annotations may be +silently ignored: Your code might appear to "work" until you test a rollback scenario. NOTE: In proxy mode (which is the default), only external method calls coming in through the proxy are intercepted. This means that self-invocation (in effect, a method within the target object calling another method of the target object) does not lead to an actual transaction at runtime even if the invoked method is marked with `@Transactional`. Also, the proxy must be fully initialized to provide the expected behavior, so you should not -rely on this feature in your initialization code -- for example, in a `@PostConstruct` -method. +rely on this feature in your initialization code -- e.g. in a `@PostConstruct` method. Consider using AspectJ mode (see the `mode` attribute in the following table) if you expect self-invocations to be wrapped with transactions as well. In this case, there is @@ -277,20 +274,20 @@ is modified) to support `@Transactional` runtime behavior on any kind of method. framework (following proxy semantics, as discussed earlier, applying to method calls coming in through the proxy only). The alternative mode (`aspectj`) instead weaves the affected classes with Spring's AspectJ transaction aspect, modifying the target class - byte code to apply to any kind of method call. AspectJ weaving requires - `spring-aspects.jar` in the classpath as well as having load-time weaving (or compile-time - weaving) enabled. (See xref:core/aop/using-aspectj.adoc#aop-aj-ltw-spring[Spring configuration] - for details on how to set up load-time weaving.) + byte code to apply to any kind of method call. AspectJ weaving requires `spring-aspects.jar` + in the classpath as well as having load-time weaving (or compile-time weaving) enabled. + (See xref:core/aop/using-aspectj.adoc#aop-aj-ltw-spring[Spring configuration] for details + on how to set up load-time weaving.) | `proxy-target-class` | `proxyTargetClass` | `false` | Applies to `proxy` mode only. Controls what type of transactional proxies are created - for classes annotated with the `@Transactional` annotation. If the - `proxy-target-class` attribute is set to `true`, class-based proxies are created. - If `proxy-target-class` is `false` or if the attribute is omitted, then standard JDK - interface-based proxies are created. (See xref:core/aop/proxying.adoc[Proxying Mechanisms] - for a detailed examination of the different proxy types.) + for classes annotated with the `@Transactional` annotation. If the `proxy-target-class` + attribute is set to `true`, class-based proxies are created. If `proxy-target-class` is + `false` or if the attribute is omitted, then standard JDK interface-based proxies are + created. (See xref:core/aop/proxying.adoc[Proxying Mechanisms] for a detailed examination + of the different proxy types.) | `order` | `order`