Polishing.

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
spring-projects · Oct 1, 2024 · 4e36b16 · 4e36b16
1 parent b1c349a
commit 4e36b16
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 39 deletions.
diff --git a/...jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java b/...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/...jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancerFactory.java b/...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/...est/java/org/springframework/data/jpa/repository/query/QueryEnhancerFactoryUnitTests.java b/...est/java/org/springframework/data/jpa/repository/query/QueryEnhancerFactoryUnitTests.java
@@ -15,14 +15,15 @@
  */
 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;
 
 import org.junit.jupiter.api.Test;
 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<Arguments> 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<Arguments> 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<Arguments> 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
@@ -307,7 +307,13 @@ public interface UserRepository extends JpaRepository<User, Long> {
 
 [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.