From 3ad78aba4fa82ae49107922ab83c91c112a9f6d1 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 22 Oct 2024 10:25:49 +0200 Subject: [PATCH] Polishing. Refactor DisabledOnHibernate61/62 annotation to DisabledOnHibernate with a version string. --- .../data/jpa/AntlrVersionTests.java | 5 +- ...stgresStoredProcedureIntegrationTests.java | 6 +- ...ProcedureNullHandlingIntegrationTests.java | 4 +- ...odelEntityInformationIntegrationTests.java | 9 +- .../jpa/util/BooleanExecutionCondition.java | 58 ++++++++++ ...ernate61.java => DisabledOnHibernate.java} | 25 ++++- .../data/jpa/util/DisabledOnHibernate62.java | 36 ------- .../util/DisabledOnHibernateCondition.java | 100 ++++++++++++++++++ .../DisabledOnHibernateConditionTests.java | 56 ++++++++++ .../data/jpa/util/HibernateSupport.java | 62 ----------- 10 files changed, 249 insertions(+), 112 deletions(-) create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/util/BooleanExecutionCondition.java rename spring-data-jpa/src/test/java/org/springframework/data/jpa/util/{DisabledOnHibernate61.java => DisabledOnHibernate.java} (50%) delete mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernate62.java create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernateCondition.java create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernateConditionTests.java delete mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/util/HibernateSupport.java diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/AntlrVersionTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/AntlrVersionTests.java index bc3c34dd3..c69458716 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/AntlrVersionTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/AntlrVersionTests.java @@ -23,11 +23,12 @@ import java.util.regex.Pattern; import org.antlr.v4.runtime.RuntimeMetaData; import org.hibernate.grammars.hql.HqlParser; import org.junit.jupiter.api.Test; + import org.springframework.asm.ClassReader; import org.springframework.asm.ClassVisitor; import org.springframework.asm.MethodVisitor; import org.springframework.asm.Opcodes; -import org.springframework.data.jpa.util.DisabledOnHibernate62; +import org.springframework.data.jpa.util.DisabledOnHibernate; import org.springframework.lang.Nullable; /** @@ -41,7 +42,7 @@ import org.springframework.lang.Nullable; class AntlrVersionTests { @Test - @DisabledOnHibernate62 + @DisabledOnHibernate("6.2") void antlrVersionConvergence() throws Exception { ClassReader reader = new ClassReader(HqlParser.class.getName()); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java index eaf489fdc..b3c76d25b 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java @@ -42,7 +42,7 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.config.EnableJpaRepositories; import org.springframework.data.jpa.repository.query.Procedure; -import org.springframework.data.jpa.util.DisabledOnHibernate62; +import org.springframework.data.jpa.util.DisabledOnHibernate; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.transaction.annotation.EnableTransactionManagement; @@ -106,7 +106,7 @@ class PostgresStoredProcedureIntegrationTests { new Employee(4, "Gabriel")); } - @DisabledOnHibernate62 + @DisabledOnHibernate("6") @Test // 2256 void testSingleEntityFromResultSet() { @@ -160,6 +160,8 @@ class PostgresStoredProcedureIntegrationTests { } @Test // GH-3081 + @DisabledOnHibernate(value = "6.2", + disabledReason = "Hibernate 6.2 does not support stored procedures with array types") void supportsArrayTypes() { String result = repository.accept_array(new String[] { "one", "two" }); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureNullHandlingIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureNullHandlingIntegrationTests.java index d6c7a1dab..98783a9c2 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureNullHandlingIntegrationTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureNullHandlingIntegrationTests.java @@ -36,7 +36,7 @@ import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Temporal; import org.springframework.data.jpa.repository.config.EnableJpaRepositories; import org.springframework.data.jpa.repository.query.Procedure; -import org.springframework.data.jpa.util.DisabledOnHibernate61; +import org.springframework.data.jpa.util.DisabledOnHibernate; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.transaction.annotation.EnableTransactionManagement; @@ -49,7 +49,7 @@ import org.testcontainers.containers.PostgreSQLContainer; * * @author Greg Turnquist */ -@DisabledOnHibernate61 // GH-2903 +@DisabledOnHibernate("6.1") // GH-2903 @Transactional @ExtendWith(SpringExtension.class) @ContextConfiguration(classes = PostgresStoredProcedureNullHandlingIntegrationTests.Config.class) diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/HibernateJpaMetamodelEntityInformationIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/HibernateJpaMetamodelEntityInformationIntegrationTests.java index c305e9b4b..82824c084 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/HibernateJpaMetamodelEntityInformationIntegrationTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/HibernateJpaMetamodelEntityInformationIntegrationTests.java @@ -17,7 +17,8 @@ package org.springframework.data.jpa.repository.support; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.springframework.data.jpa.util.DisabledOnHibernate61; + +import org.springframework.data.jpa.util.DisabledOnHibernate; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; @@ -35,21 +36,21 @@ class HibernateJpaMetamodelEntityInformationIntegrationTests extends JpaMetamode return "metadata-id-handling"; } - @DisabledOnHibernate61 + @DisabledOnHibernate("6.1") @Test @Override void correctlyDeterminesIdValueForNestedIdClassesWithNonPrimitiveNonManagedType() { super.correctlyDeterminesIdValueForNestedIdClassesWithNonPrimitiveNonManagedType(); } - @DisabledOnHibernate61 + @DisabledOnHibernate("6.1") @Test @Override void prefersPrivateGetterOverFieldAccess() { super.prefersPrivateGetterOverFieldAccess(); } - @DisabledOnHibernate61 + @DisabledOnHibernate("6.1") @Test @Override void findsIdClassOnMappedSuperclass() { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/BooleanExecutionCondition.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/BooleanExecutionCondition.java new file mode 100644 index 000000000..bd617cb55 --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/BooleanExecutionCondition.java @@ -0,0 +1,58 @@ +/* + * Copyright 2024 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.data.jpa.util; + +import static org.junit.jupiter.api.extension.ConditionEvaluationResult.*; +import static org.junit.platform.commons.util.AnnotationUtils.*; + +import java.lang.annotation.Annotation; +import java.util.function.Function; + +import org.junit.jupiter.api.extension.ConditionEvaluationResult; +import org.junit.jupiter.api.extension.ExecutionCondition; +import org.junit.jupiter.api.extension.ExtensionContext; + +abstract class BooleanExecutionCondition implements ExecutionCondition { + + private final Class annotationType; + private final String enabledReason; + private final String disabledReason; + private final Function customDisabledReason; + + BooleanExecutionCondition(Class annotationType, String enabledReason, String disabledReason, + Function customDisabledReason) { + this.annotationType = annotationType; + this.enabledReason = enabledReason; + this.disabledReason = disabledReason; + this.customDisabledReason = customDisabledReason; + } + + abstract boolean isEnabled(A annotation); + + @Override + public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext context) { + return findAnnotation(context.getElement(), annotationType) // + .map(annotation -> isEnabled(annotation) ? enabled(enabledReason) + : disabled(disabledReason, customDisabledReason.apply(annotation))) // + .orElseGet(this::enabledByDefault); + } + + private ConditionEvaluationResult enabledByDefault() { + String reason = String.format("@%s is not present", annotationType.getSimpleName()); + return enabled(reason); + } + +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernate61.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernate.java similarity index 50% rename from spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernate61.java rename to spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernate.java index 451eb8866..0ee4e2598 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernate61.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernate.java @@ -23,14 +23,31 @@ import java.lang.annotation.Target; import org.junit.jupiter.api.extension.ExtendWith; /** - * Annotation to flag JUnit 5 test cases to ONLY activate when Hibernate 6.2 is on the classpath. + * {@code @DisabledOnHibernate} is used to signal that the annotated test class or test method is only disabled + * if the given Hibernate {@linkplain #value version} is being used. * * @author Greg Turnquist - * @since 3.1 + * @author Mark Paluch + * @since 3.2 */ @Target({ ElementType.TYPE, ElementType.METHOD, ElementType.ANNOTATION_TYPE }) @Retention(RetentionPolicy.RUNTIME) -@ExtendWith(HibernateSupport.DisabledWhenHibernate61OnClasspath.class) -public @interface DisabledOnHibernate61 { +@ExtendWith(DisabledOnHibernateCondition.class) +public @interface DisabledOnHibernate { + /** + * The version of Hibernate to disable the test or container case on. The version specifier can hold individual + * version components matching effectively the version in a prefix-manner. The more specific you want to match, the + * more version components you can specify, such as {@code 6.2.1} to match a specific service release or {@code 6} to + * match a full major version. + */ + String value(); + + /** + * Custom reason to provide if the test or container is disabled. + *

+ * If a custom reason is supplied, it will be combined with the default reason for this annotation. If a custom reason + * is not supplied, the default reason will be used. + */ + String disabledReason() default ""; } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernate62.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernate62.java deleted file mode 100644 index b39f7000d..000000000 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernate62.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright 2015-2024 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.data.jpa.util; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -import org.junit.jupiter.api.extension.ExtendWith; - -/** - * Annotation to flag JUnit 5 test cases to ONLY activate when Hibernate 6.1 is on the classpath. - * - * @author Greg Turnquist - * @since 3.1 - */ -@Target({ ElementType.TYPE, ElementType.METHOD, ElementType.ANNOTATION_TYPE }) -@Retention(RetentionPolicy.RUNTIME) -@ExtendWith(HibernateSupport.DisabledWhenHibernate62OnClasspath.class) -public @interface DisabledOnHibernate62 { - -} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernateCondition.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernateCondition.java new file mode 100644 index 000000000..804309544 --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernateCondition.java @@ -0,0 +1,100 @@ +/* + * Copyright 2024 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.data.jpa.util; + +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.junit.jupiter.api.extension.ExecutionCondition; + +/** + * {@link ExecutionCondition} for {@link DisabledOnHibernate @DisabledOnHibernate}. + * + * @see DisabledOnHibernate + */ +class DisabledOnHibernateCondition extends BooleanExecutionCondition { + + static final String ENABLED_ON_CURRENT_HIBERNATE = // + "Enabled on Hibernate version: " + org.hibernate.Version.getVersionString(); + + static final String DISABLED_ON_CURRENT_HIBERNATE = // + "Disabled on Hibernate version: " + org.hibernate.Version.getVersionString(); + + DisabledOnHibernateCondition() { + super(DisabledOnHibernate.class, ENABLED_ON_CURRENT_HIBERNATE, DISABLED_ON_CURRENT_HIBERNATE, + DisabledOnHibernate::disabledReason); + } + + @Override + boolean isEnabled(DisabledOnHibernate annotation) { + + VersionMatcher disabled = VersionMatcher.parse(annotation.value()); + VersionMatcher hibernate = VersionMatcher.parse(org.hibernate.Version.getVersionString()); + + return !disabled.matches(hibernate); + } + + static class VersionMatcher { + + private static final Pattern PATTERN = Pattern.compile("(\\d+)+"); + private final int[] components; + + private VersionMatcher(int[] components) { + this.components = components; + } + + /** + * Parse the given version string into a {@link VersionMatcher}. + * + * @param version + * @return + */ + public static VersionMatcher parse(String version) { + + Matcher matcher = PATTERN.matcher(version); + List ints = new ArrayList<>(); + while (matcher.find()) { + ints.add(Integer.parseInt(matcher.group())); + } + + return new VersionMatcher(ints.stream().mapToInt(value -> value).toArray()); + } + + /** + * Match the given version against another VersionMatcher. This matcher's version spec controls the expected length. + * If the other version is shorter, then the match returns {@code false}. + * + * @param version + * @return + */ + public boolean matches(VersionMatcher version) { + + for (int i = 0; i < components.length; i++) { + if (version.components.length <= i) { + return false; + } + if (components[i] != version.components[i]) { + return false; + } + } + + return true; + } + } + +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernateConditionTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernateConditionTests.java new file mode 100644 index 000000000..15a977cb5 --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/DisabledOnHibernateConditionTests.java @@ -0,0 +1,56 @@ +/* + * Copyright 2024 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.data.jpa.util; + +import static org.assertj.core.api.Assertions.*; +import static org.springframework.data.jpa.util.DisabledOnHibernateCondition.*; + +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link DisabledOnHibernate}. + * + * @author Mark Paluch + */ +class DisabledOnHibernateConditionTests { + + @Test // GH-3081 + void shouldMatchVersions() { + + VersionMatcher spec = VersionMatcher.parse("1.2"); + VersionMatcher lib = VersionMatcher.parse("v1.2.3.4.Final"); + + assertThat(spec.matches(lib)).isTrue(); + } + + @Test // GH-3081 + void shouldNotMatchVersions() { + + VersionMatcher spec = VersionMatcher.parse("1.2"); + VersionMatcher lib = VersionMatcher.parse("2.2.3.4"); + + assertThat(spec.matches(lib)).isFalse(); + } + + @Test // GH-3081 + void shouldNotMatchVersionsWithExceedingLength() { + + VersionMatcher spec = VersionMatcher.parse("1.2.3.4"); + VersionMatcher lib = VersionMatcher.parse("1.2"); + + assertThat(spec.matches(lib)).isFalse(); + } +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/HibernateSupport.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/HibernateSupport.java deleted file mode 100644 index d2b325354..000000000 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/util/HibernateSupport.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright 2015-2024 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.data.jpa.util; - -import org.junit.jupiter.api.extension.ConditionEvaluationResult; -import org.junit.jupiter.api.extension.ExecutionCondition; -import org.junit.jupiter.api.extension.ExtensionContext; -import org.springframework.util.ClassUtils; - -/** - * JUnit 5 utilities to support conditional test cases based upon Hibernate classpath settings. - * - * @author Greg Turnquist - * @since 3.1 - */ -abstract class HibernateSupport { - - /** - * {@literal org.hibernate.dialect.PostgreSQL91Dialect} is deprecated in Hibernate 6.1 and fully removed in Hibernate - * 6.2, making it a perfect detector between the two. - */ - private static final boolean HIBERNATE_61_ON_CLASSPATH = ClassUtils - .isPresent("org.hibernate.dialect.PostgreSQL91Dialect", HibernateSupport.class.getClassLoader()); - - private static final boolean HIBERNATE_62_ON_CLASSPATH = !HIBERNATE_61_ON_CLASSPATH; - - static class DisabledWhenHibernate61OnClasspath implements ExecutionCondition { - - @Override - public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext extensionContext) { - - return HibernateSupport.HIBERNATE_61_ON_CLASSPATH - ? ConditionEvaluationResult.disabled("Disabled because Hibernate 6.1 is on the classpath") - : ConditionEvaluationResult.enabled("NOT disabled because Hibernate 6.2 is on the classpath"); - } - } - - static class DisabledWhenHibernate62OnClasspath implements ExecutionCondition { - - @Override - public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext extensionContext) { - - return HibernateSupport.HIBERNATE_62_ON_CLASSPATH - ? ConditionEvaluationResult.disabled("Disabled because Hibernate 6.2 is on the classpath") - : ConditionEvaluationResult.enabled("NOT disabled because Hibernate 6.1 is on the classpath"); - } - - } -}