From cd12bd5693a90137f0fac80f235ba8df370cf702 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 30 Jan 2025 14:16:27 +0100 Subject: [PATCH] Polishing. Refine HQL rendering for duration expressions. Retain whitespace for error message reconstruction, refine error handling. See #3757 --- .../data/jpa/repository/query/Eql.g4 | 2 +- .../data/jpa/repository/query/Hql.g4 | 2 +- .../data/jpa/repository/query/Jpql.g4 | 2 +- .../query/BadJpqlGrammarErrorListener.java | 50 +++++++++++++++- .../repository/query/HqlQueryRenderer.java | 2 +- .../repository/query/JpaQueryEnhancer.java | 10 +++- .../BadJpqlGrammarExceptionUnitTests.java | 59 +++++++++++++++++++ .../query/HqlQueryRendererTests.java | 1 + 8 files changed, 122 insertions(+), 6 deletions(-) create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/BadJpqlGrammarExceptionUnitTests.java diff --git a/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Eql.g4 b/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Eql.g4 index 3ed025efb..9db07f631 100644 --- a/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Eql.g4 +++ b/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Eql.g4 @@ -837,7 +837,7 @@ reserved_word */ -WS : [ \t\r\n] -> skip ; +WS : [ \t\r\n] -> channel(HIDDEN) ; // Build up case-insentive tokens diff --git a/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Hql.g4 b/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Hql.g4 index 2c79fc040..da0518d08 100644 --- a/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Hql.g4 +++ b/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Hql.g4 @@ -1560,7 +1560,7 @@ identifier */ -WS : [ \t\r\n] -> skip ; +WS : [ \t\r\n] -> channel(HIDDEN); // Build up case-insentive tokens diff --git a/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Jpql.g4 b/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Jpql.g4 index e87d523b1..9ddbcadb4 100644 --- a/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Jpql.g4 +++ b/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Jpql.g4 @@ -817,7 +817,7 @@ reserved_word */ -WS : [ \t\r\n] -> skip ; +WS : [ \t\r\n] -> channel(HIDDEN) ; // Build up case-insentive tokens diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/BadJpqlGrammarErrorListener.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/BadJpqlGrammarErrorListener.java index a2edc2d91..dd3eab641 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/BadJpqlGrammarErrorListener.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/BadJpqlGrammarErrorListener.java @@ -15,10 +15,17 @@ */ package org.springframework.data.jpa.repository.query; +import java.util.List; + import org.antlr.v4.runtime.BaseErrorListener; +import org.antlr.v4.runtime.CommonToken; +import org.antlr.v4.runtime.InputMismatchException; +import org.antlr.v4.runtime.NoViableAltException; import org.antlr.v4.runtime.RecognitionException; import org.antlr.v4.runtime.Recognizer; +import org.springframework.util.ObjectUtils; + /** * A {@link BaseErrorListener} that will throw a {@link BadJpqlGrammarException} if the query is invalid. * @@ -43,7 +50,48 @@ class BadJpqlGrammarErrorListener extends BaseErrorListener { @Override public void syntaxError(Recognizer recognizer, Object offendingSymbol, int line, int charPositionInLine, String msg, RecognitionException e) { - throw new BadJpqlGrammarException("Line " + line + ":" + charPositionInLine + " " + msg, grammar, query, null); + throw new BadJpqlGrammarException(formatMessage(offendingSymbol, line, charPositionInLine, msg, e, query), grammar, + query, null); + } + + /** + * Rewrite the error message. + */ + private static String formatMessage(Object offendingSymbol, int line, int charPositionInLine, String message, + RecognitionException e, String query) { + + String errorText = "At " + line + ":" + charPositionInLine; + + if (offendingSymbol instanceof CommonToken ct) { + + String token = ct.getText(); + if (!ObjectUtils.isEmpty(token)) { + errorText += " and token '" + token + "'"; + } + } + errorText += ", "; + + if (e instanceof NoViableAltException) { + + errorText += message.substring(0, message.indexOf('\'')); + if (query.isEmpty()) { + errorText += "'*' (empty query string)"; + } else { + + List list = query.lines().toList(); + String lineText = list.get(line - 1); + String text = lineText.substring(0, charPositionInLine) + "*" + lineText.substring(charPositionInLine); + errorText += "'" + text + "'"; + } + + } else if (e instanceof InputMismatchException) { + errorText += message.substring(0, message.length() - 1).replace(" expecting {", + ", expecting one of the following tokens: "); + } else { + errorText += message; + } + + return errorText; } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryRenderer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryRenderer.java index 26a4b775c..36f8a2f62 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryRenderer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryRenderer.java @@ -1752,7 +1752,7 @@ class HqlQueryRenderer extends HqlBaseVisitor { QueryRendererBuilder builder = QueryRenderer.builder(); - builder.append(visit(ctx.expression())); + builder.appendExpression(visit(ctx.expression())); builder.append(QueryTokens.expression(ctx.BY())); builder.appendExpression(visit(ctx.datetimeField())); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java index ffd4bd6d7..d6d4cbeda 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java @@ -27,6 +27,7 @@ import org.antlr.v4.runtime.CommonTokenStream; import org.antlr.v4.runtime.Lexer; import org.antlr.v4.runtime.Parser; import org.antlr.v4.runtime.ParserRuleContext; +import org.antlr.v4.runtime.RecognitionException; import org.antlr.v4.runtime.TokenStream; import org.antlr.v4.runtime.atn.PredictionMode; import org.antlr.v4.runtime.misc.ParseCancellationException; @@ -82,7 +83,14 @@ class JpaQueryEnhancer implements QueryEnhancer { P parser = getParser(query, lexerFactoryFunction, parserFactoryFunction); parser.getInterpreter().setPredictionMode(PredictionMode.SLL); - parser.setErrorHandler(new BailErrorStrategy()); + parser.setErrorHandler(new BailErrorStrategy() { + @Override + public void reportError(Parser recognizer, RecognitionException e) { + + // avoid BadJpqlGrammarException creation in the first pass. + // recover(…) is going to handle cancellation. + } + }); try { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/BadJpqlGrammarExceptionUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/BadJpqlGrammarExceptionUnitTests.java new file mode 100644 index 000000000..16766673f --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/BadJpqlGrammarExceptionUnitTests.java @@ -0,0 +1,59 @@ +/* + * Copyright 2025 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.repository.query; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link BadJpqlGrammarException}. + * + * @author Mark Paluch + */ +class BadJpqlGrammarExceptionUnitTests { + + @Test // GH-3757 + void shouldContainOriginalText() { + + assertThatExceptionOfType(BadJpqlGrammarException.class) + .isThrownBy(() -> JpaQueryEnhancer.HqlQueryParser + .parseQuery("SELECT e FROM Employee e WHERE FOO(x).bar RESPECTING NULLS")) + .withMessageContaining("no viable alternative") + .withMessageContaining("SELECT e FROM Employee e WHERE FOO(x).bar *RESPECTING NULLS") + .withMessageContaining("Bad HQL grammar [SELECT e FROM Employee e WHERE FOO(x).bar RESPECTING NULLS]"); + } + + @Test // GH-3757 + void shouldReportExtraneousInput() { + + assertThatExceptionOfType(BadJpqlGrammarException.class) + .isThrownBy(() -> JpaQueryEnhancer.HqlQueryParser.parseQuery("select * from User group by name")) + .withMessageContaining("extraneous input '*'") + .withMessageContaining("Bad HQL grammar [select * from User group by name]"); + } + + @Test // GH-3757 + void shouldReportMismatchedInput() { + + assertThatExceptionOfType(BadJpqlGrammarException.class) + .isThrownBy(() -> JpaQueryEnhancer.HqlQueryParser.parseQuery("SELECT AVG(m.price) AS m.avg FROM Magazine m")) + .withMessageContaining("mismatched input '.'").withMessageContaining("expecting one of the following tokens:") + .withMessageContaining("EXCEPT") + .withMessageContaining("Bad HQL grammar [SELECT AVG(m.price) AS m.avg FROM Magazine m]"); + } + +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryRendererTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryRendererTests.java index 4de4c2b9d..922c20bc2 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryRendererTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryRendererTests.java @@ -1891,6 +1891,7 @@ class HqlQueryRendererTests { assertQuery("SELECT a FROM foo a WHERE (cast(a.createdAt as date) - CURRENT_DATE()) BY day - 2 = 0"); assertQuery("SELECT a FROM foo a WHERE (cast(a.createdAt as date)) BY day - 2 = 0"); + assertQuery("SELECT f.start BY DAY - 2 FROM foo f"); assertQuery("SELECT f.start - 1 minute FROM foo f"); assertQuery("SELECT f FROM foo f WHERE (cast(f.start as date) - CURRENT_DATE()) BY day - 2 = 0");