From 2a5fdc3b0de1110c80fe3f12d01501846b91ffa7 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 1 Oct 2024 15:22:40 +0200 Subject: [PATCH] Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use ObjectUtils instead of Enum.valueOf(…), move class presence check into field. Allow force-selection of JSQLParser. Add more tests. See #2989 Original pull request: #3623 --- .../query/DefaultQueryEnhancer.java | 2 +- .../query/QueryEnhancerFactory.java | 63 ++++++++++++------- .../query/QueryEnhancerFactoryUnitTests.java | 50 ++++++++++----- .../modules/ROOT/pages/jpa/query-methods.adoc | 8 ++- 4 files changed, 84 insertions(+), 39 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java index 3053bb95f5..3aff35357b 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java @@ -21,7 +21,7 @@ import org.springframework.lang.Nullable; /** - * The implementation of {@link QueryEnhancer} using {@link QueryUtils}. + * The implementation of the Regex-based {@link QueryEnhancer} using {@link QueryUtils}. * * @author Diego Krupitza * @since 2.7.0 diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancerFactory.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancerFactory.java index 0ffcc6b660..c40ab11f66 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancerFactory.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancerFactory.java @@ -19,8 +19,8 @@ import org.apache.commons.logging.LogFactory; import org.springframework.core.SpringProperties; import org.springframework.data.jpa.provider.PersistenceProvider; -import org.springframework.lang.Nullable; import org.springframework.util.ClassUtils; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** @@ -39,7 +39,7 @@ public final class QueryEnhancerFactory { static { - NATIVE_QUERY_ENHANCER = NativeQueryEnhancer.select(QueryEnhancerFactory.class.getClassLoader()); + NATIVE_QUERY_ENHANCER = NativeQueryEnhancer.select(); if (PersistenceProvider.ECLIPSELINK.isPresent()) { LOG.info("EclipseLink is in classpath; If applicable, EQL parser will be used."); @@ -81,47 +81,66 @@ public static QueryEnhancer forQuery(DeclaredQuery query) { */ private static QueryEnhancer getNativeQueryEnhancer(DeclaredQuery query) { - if (NATIVE_QUERY_ENHANCER.equals(NativeQueryEnhancer.JSQL)) { + if (NATIVE_QUERY_ENHANCER.equals(NativeQueryEnhancer.JSQLPARSER)) { return new JSqlParserQueryEnhancer(query); } + return new DefaultQueryEnhancer(query); } /** - * Possible choices for the {@link #NATIVE_PARSER_PROPERTY}. Read current selection via {@link #select(ClassLoader)}. + * Possible choices for the {@link #NATIVE_PARSER_PROPERTY}. Resolve the parser through {@link #select()}. + * + * @since 3.3.5 */ enum NativeQueryEnhancer { - AUTO, DEFAULT, JSQL; + AUTO, REGEX, JSQLPARSER; static final String NATIVE_PARSER_PROPERTY = "spring.data.jpa.query.native.parser"; - private static NativeQueryEnhancer from(@Nullable String name) { - if (!StringUtils.hasText(name)) { - return AUTO; - } - return NativeQueryEnhancer.valueOf(name.toUpperCase()); - } + static final boolean JSQLPARSER_PRESENT = ClassUtils.isPresent("net.sf.jsqlparser.parser.JSqlParser", null); /** - * @param classLoader ClassLoader to look up available libraries. - * @return the current selection considering classpath avialability and user selection via + * @return the current selection considering classpath availability and user selection via * {@link #NATIVE_PARSER_PROPERTY}. */ - static NativeQueryEnhancer select(ClassLoader classLoader) { + static NativeQueryEnhancer select() { - if (!ClassUtils.isPresent("net.sf.jsqlparser.parser.JSqlParser", classLoader)) { - return NativeQueryEnhancer.DEFAULT; + NativeQueryEnhancer selected = resolve(); + + if (selected.equals(NativeQueryEnhancer.JSQLPARSER)) { + LOG.info("User choice: Using JSqlParser"); + return NativeQueryEnhancer.JSQLPARSER; + } + + if (selected.equals(NativeQueryEnhancer.REGEX)) { + LOG.info("Using Regex QueryEnhancer"); + return NativeQueryEnhancer.REGEX; } - NativeQueryEnhancer selected = NativeQueryEnhancer.from(SpringProperties.getProperty(NATIVE_PARSER_PROPERTY)); - if (selected.equals(NativeQueryEnhancer.AUTO) || selected.equals(NativeQueryEnhancer.JSQL)) { - LOG.info("JSqlParser is in classpath; If applicable, JSqlParser will be used."); - return NativeQueryEnhancer.JSQL; + if (!JSQLPARSER_PRESENT) { + return NativeQueryEnhancer.REGEX; + } + + LOG.info("JSqlParser is in classpath; If applicable, JSqlParser will be used."); + return NativeQueryEnhancer.JSQLPARSER; + } + + /** + * Resolve {@link NativeQueryEnhancer} from {@link SpringProperties}. + * + * @return the {@link NativeQueryEnhancer} constant. + */ + private static NativeQueryEnhancer resolve() { + + String name = SpringProperties.getProperty(NATIVE_PARSER_PROPERTY); + + if (StringUtils.hasText(name)) { + return ObjectUtils.caseInsensitiveValueOf(NativeQueryEnhancer.values(), name); } - LOG.info("JSqlParser is in classpath but won't be used due to user choice."); - return selected; + return AUTO; } } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerFactoryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerFactoryUnitTests.java index d650277bd6..568ea9b53d 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerFactoryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerFactoryUnitTests.java @@ -15,7 +15,7 @@ */ package org.springframework.data.jpa.repository.query; -import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.*; import java.util.stream.Stream; @@ -23,6 +23,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; + import org.springframework.data.jpa.repository.query.QueryEnhancerFactory.NativeQueryEnhancer; import org.springframework.data.jpa.util.ClassPathExclusions; import org.springframework.lang.Nullable; @@ -33,6 +34,7 @@ * @author Diego Krupitza * @author Greg Turnquist * @author Christoph Strobl + * @author Mark Paluch */ class QueryEnhancerFactoryUnitTests { @@ -66,26 +68,48 @@ void createsJSqlImplementationForNativeQuery() { @MethodSource("nativeEnhancerSelectionArgs") void createsNativeImplementationAccordingToUserChoice(@Nullable String selection, NativeQueryEnhancer enhancer) { + assertThat(NativeQueryEnhancer.JSQLPARSER_PRESENT).isTrue(); + withSystemProperty(NativeQueryEnhancer.NATIVE_PARSER_PROPERTY, selection, () -> { - assertThat(NativeQueryEnhancer.select(this.getClass().getClassLoader())).isEqualTo(enhancer); + assertThat(NativeQueryEnhancer.select()).isEqualTo(enhancer); }); } - @Test // GH-2989 + static Stream nativeEnhancerSelectionArgs() { + return Stream.of(Arguments.of(null, NativeQueryEnhancer.JSQLPARSER), // + Arguments.of("", NativeQueryEnhancer.JSQLPARSER), // + Arguments.of("auto", NativeQueryEnhancer.JSQLPARSER), // + Arguments.of("regex", NativeQueryEnhancer.REGEX), // + Arguments.of("jsqlparser", NativeQueryEnhancer.JSQLPARSER)); + } + + @ParameterizedTest // GH-2989 + @MethodSource("nativeEnhancerExclusionSelectionArgs") @ClassPathExclusions(packages = { "net.sf.jsqlparser.parser" }) - void selectedDefaultImplementationIfJsqlNotAvailable() { + void createsNativeImplementationAccordingWithoutJsqlParserToUserChoice(@Nullable String selection, + NativeQueryEnhancer enhancer) { + + assertThat(NativeQueryEnhancer.JSQLPARSER_PRESENT).isFalse(); - assertThat(assertThat(NativeQueryEnhancer.select(this.getClass().getClassLoader())) - .isEqualTo(NativeQueryEnhancer.DEFAULT)); + withSystemProperty(NativeQueryEnhancer.NATIVE_PARSER_PROPERTY, selection, () -> { + assertThat(NativeQueryEnhancer.select()).isEqualTo(enhancer); + }); + } + + static Stream nativeEnhancerExclusionSelectionArgs() { + return Stream.of(Arguments.of(null, NativeQueryEnhancer.REGEX), // + Arguments.of("", NativeQueryEnhancer.REGEX), // + Arguments.of("auto", NativeQueryEnhancer.REGEX), // + Arguments.of("regex", NativeQueryEnhancer.REGEX), // + Arguments.of("jsqlparser", NativeQueryEnhancer.JSQLPARSER)); } @Test // GH-2989 @ClassPathExclusions(packages = { "net.sf.jsqlparser.parser" }) - void selectedDefaultImplementationIfJsqlNotAvailableEvenIfExplicitlyStated/* or should we raise an error? */() { + void selectedDefaultImplementationIfJsqlNotAvailable() { - withSystemProperty(NativeQueryEnhancer.NATIVE_PARSER_PROPERTY, "jsql", () -> { - assertThat(NativeQueryEnhancer.select(this.getClass().getClassLoader())).isEqualTo(NativeQueryEnhancer.DEFAULT); - }); + assertThat(NativeQueryEnhancer.JSQLPARSER_PRESENT).isFalse(); + assertThat(NativeQueryEnhancer.select()).isEqualTo(NativeQueryEnhancer.REGEX); } void withSystemProperty(String property, @Nullable String value, Runnable exeution) { @@ -108,9 +132,5 @@ void withSystemProperty(String property, @Nullable String value, Runnable exeuti } - static Stream nativeEnhancerSelectionArgs() { - return Stream.of(Arguments.of(null, NativeQueryEnhancer.JSQL), Arguments.of("", NativeQueryEnhancer.JSQL), - Arguments.of("auto", NativeQueryEnhancer.JSQL), Arguments.of("default", NativeQueryEnhancer.DEFAULT), - Arguments.of("jsql", NativeQueryEnhancer.JSQL)); - } + } diff --git a/src/main/antora/modules/ROOT/pages/jpa/query-methods.adoc b/src/main/antora/modules/ROOT/pages/jpa/query-methods.adoc index 1681d2b7d7..980adac923 100644 --- a/src/main/antora/modules/ROOT/pages/jpa/query-methods.adoc +++ b/src/main/antora/modules/ROOT/pages/jpa/query-methods.adoc @@ -306,7 +306,13 @@ public interface UserRepository extends JpaRepository { [TIP] ==== -It is possible to disable usage of `JSqlParser` for parsing natvie queries although it is available in classpath by setting `spring.data.jpa.query.native.parser=default` via the `spring.properties` file or a system property. +It is possible to disable usage of `JSqlParser` for parsing native queries although it is available on the classpath by setting `spring.data.jpa.query.native.parser=regex` via the `spring.properties` file or a system property. + +Valid values are (case-insensitive): + +* `auto` (default, automatic selection) +* `regex` (Use the builtin regex-based Query Enhancer) +* `jsqlparser` (Use JSqlParser) ==== A similar approach also works with named native queries, by adding the `.count` suffix to a copy of your query. You probably need to register a result set mapping for your count query, though.