Skip to content

Commit

Permalink
(eclipse-ee4j#2217) Test for length field in an expression
Browse files Browse the repository at this point in the history
  • Loading branch information
@OndroMih
OndroMih committed Jul 23, 2024
1 parent e7f1fba commit 5d1620a
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 105 deletions.
34 changes: 2 additions & 32 deletions ...nce.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/AbstractGrammarValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1791,35 +1791,6 @@ protected final boolean isNewerThanOrEqual(JPAVersion version) {
return getJPAVersion().isNewerThanOrEqual(version);
}

/**
* Determines whether the given sequence of characters is a numeric literal or not. There are
* two types of numeric literal that is supported:
* <ul>
* <li>Decimal literal</li>
* <li>Hexadecimal literal</li>
* </ul>
*
* @param text The sequence of characters to validate
* @return <code>true</code> if the given sequence of characters is a valid numeric literal;
* <code>false</code> otherwise
*/
protected boolean isNumericLiteral(String text) {

// The ending 'l' or 'L' for a long number has to be removed, Java will not parse it
if (text.endsWith("l") || text.endsWith("L")) {
text = text.substring(0, text.length() - 1);
}

// Simply try to parse it as a double number (integer and hexadecimal are handled as well)
try {
Double.parseDouble(text);
return true;
}
catch (Exception e2) {
return false;
}
}

/**
* Determines whether the JPA version for which the JPQL grammar was defined represents a version
* that is older than the given version.
Expand Down Expand Up @@ -4394,15 +4365,14 @@ public void visit(NullIfExpression expression) {
@Override
public void visit(NumericLiteral expression) {

String text = expression.getText();

// - Exact numeric literals support the use of Java integer literal syntax as well as SQL
// exact numeric literal syntax
// - Approximate literals support the use Java floating point literal syntax as well as SQL
// approximate numeric literal syntax
// - Appropriate suffixes can be used to indicate the specific type of a numeric literal in
// accordance with the Java Language Specification
if (!isNumericLiteral(text)) {
if (!expression.hasValidValue()) {
String text = expression.getText();
int startPosition = position(expression);
int endPosition = startPosition + text.length();
addProblem(expression, startPosition, endPosition, NumericLiteral_Invalid, text);
Expand Down
10 changes: 0 additions & 10 deletions ...ce.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/AbstractExpression.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -769,8 +769,6 @@ tolerant && isParsingComplete(wordParser, word, expression)) {
if (factory != null) {
child = factory.buildExpression(this, wordParser, word, queryBNF, expression, tolerant);

child = revertExpressionIfInvalid(child, wordParser, word);

if (child != null) {

// The new expression is a child of the previous expression,
Expand Down Expand Up @@ -1154,14 +1152,6 @@ public final String toString() {
return toParsedText();
}

public static AbstractExpression revertExpressionIfInvalid(AbstractExpression child, WordParser wordParser, String word) {
if (child != null && child.shouldBeReverted()) {
wordParser.moveBackward(word);
return null;
}
return child;
}

/**
* Rather than creating three lists when performing a single parsing operation ({@link
* AbstractExpression#parse(WordParser, String, boolean) parse(WordParser, String, boolean)}),
Expand Down
77 changes: 42 additions & 35 deletions ...c/main/java/org/eclipse/persistence/jpa/jpql/parser/AbstractLiteralExpressionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
//
package org.eclipse.persistence.jpa.jpql.parser;



import org.eclipse.persistence.jpa.jpql.WordParser;

/**
Expand Down Expand Up @@ -72,7 +74,12 @@ protected AbstractExpression buildExpression(AbstractExpression parent,
case NUMERIC_LITERAL: {
expression = new NumericLiteral(parent, word);
expression.parse(wordParser, tolerant);
return expression;
expression = revertExpressionIfInvalid(expression, wordParser, word);
if (expression != null) {
return expression;
}
// if not valid numeric literal, continue further parsing
break;
}

case STRING_LITERAL: {
Expand All @@ -86,47 +93,47 @@ protected AbstractExpression buildExpression(AbstractExpression parent,
expression.parse(wordParser, tolerant);
return expression;
}
}

// Path expression
if (word.indexOf(AbstractExpression.DOT) > -1) {
char character = word.charAt(0);

if ((expression != null) && (character == AbstractExpression.DOT)) {
if (isCollection()) {
expression = new CollectionValuedPathExpression(parent, expression, word);
}
else {
expression = new StateFieldPathExpression(parent, expression, word);
default: {
// Path expression
if (word.indexOf(AbstractExpression.DOT) > -1) {
char character = word.charAt(0);

if ((expression != null) && (character == AbstractExpression.DOT)) {
if (isCollection()) {
expression = new CollectionValuedPathExpression(parent, expression, word);
} else {
expression = new StateFieldPathExpression(parent, expression, word);
}
} else {
if (isCollection()) {
expression = new CollectionValuedPathExpression(parent, word);
} else {
expression = new StateFieldPathExpression(parent, word);
}
}

expression.parse(wordParser, tolerant);
return expression;
}
}
else {
if (isCollection()) {
expression = new CollectionValuedPathExpression(parent, word);
}
else {
expression = new StateFieldPathExpression(parent, word);
}
}

expression.parse(wordParser, tolerant);
return expression;
}

// Checks for invalid JPQL queries
ExpressionRegistry registry = getExpressionRegistry();
// Checks for invalid JPQL queries
ExpressionRegistry registry = getExpressionRegistry();

if (tolerant && registry.isIdentifier(word)) {
ExpressionFactory factory = registry.expressionFactoryForIdentifier(word);
// TODO: Before creating the expression, check to make sure it's not a function: 'identifier('
if (factory != null) {
expression = factory.buildExpression(parent, wordParser, word, queryBNF, expression, tolerant);
if (tolerant && registry.isIdentifier(word)) {
ExpressionFactory factory = registry.expressionFactoryForIdentifier(word);
// TODO: Before creating the expression, check to make sure it's not a function: 'identifier('
if (factory != null) {
expression = factory.buildExpression(parent, wordParser, word, queryBNF, expression, tolerant);

expression = AbstractExpression.revertExpressionIfInvalid(expression, wordParser, word);
expression = revertExpressionIfInvalid(expression, wordParser, word);

if (expression != null) {
return new BadExpression(parent, expression);
if (expression != null) {
return new BadExpression(parent, expression);
}
}
}

}
}

Expand Down
8 changes: 8 additions & 0 deletions ...nce.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/ExpressionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,14 @@ public boolean isIdentifier(WordParser wordParser, String word) {
return result;
}

public static AbstractExpression revertExpressionIfInvalid(AbstractExpression expression, WordParser wordParser, String word) {
if (expression != null && expression.shouldBeReverted()) {
wordParser.moveBackward(word);
return null;
}
return expression;
}

@Override
public final String toString() {
StringBuilder sb = new StringBuilder();
Expand Down
1 change: 1 addition & 0 deletions ...a.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/LengthExpressionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ protected AbstractExpression buildExpression(AbstractExpression parent,

expression = new LengthExpression(parent);
expression.parse(wordParser, tolerant);
expression = revertExpressionIfInvalid(expression, wordParser, word);
return expression;
}
}
36 changes: 36 additions & 0 deletions ...stence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/NumericLiteral.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,40 @@ public String toParsedText() {
protected void toParsedText(StringBuilder writer, boolean actual) {
writer.append(getText());
}

/**
* Determines whether the text of this literal is a valid numeric value or not. There are
* two types of numeric values that are supported:
* <ul>
* <li>Decimal literal</li>
* <li>Hexadecimal literal</li>
* </ul>
*
* @return <code>true</code> if the text in this literal is a valid numeric value;
* <code>false</code> otherwise
*/
public boolean hasValidValue() {
String text = getText();

// The ending 'l' or 'L' for a long number has to be removed, Java will not parse it
if (text.endsWith("l") || text.endsWith("L")) {
text = text.substring(0, text.length() - 1);
}

// Simply try to parse it as a double number (integer and hexadecimal are handled as well)
try {
Double.parseDouble(text);
return true;
}
catch (Exception e) {
return false;
}
}

@Override
protected boolean shouldBeReverted() {
return !hasValidValue();
}


}
20 changes: 10 additions & 10 deletions ....jpql/src/test/java/org/eclipse/persistence/jpa/tests/jpql/parser/AllJPQLParserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,16 @@
* @author Pascal Filion
*/
@SuiteClasses({
AllJPQLParserTests1_0.class,
AllJPQLParserTests2_0.class,
AllJPQLParserTests2_1.class,
AllJPQLParserTests3_1.class,
AllJPQLParserTests3_2.class,
AllEclipseLinkJPQLParserTests.class,
AllEclipseLinkJPQLParserTests2_1.class,
AllEclipseLinkJPQLParserTests2_4.class,
AllEclipseLinkJPQLParserTests2_5.class,
AllJPQLParserConcurrentTests.class,
// AllJPQLParserTests1_0.class,
// AllJPQLParserTests2_0.class,
// AllJPQLParserTests2_1.class,
// AllJPQLParserTests3_1.class,
// AllJPQLParserTests3_2.class,
// AllEclipseLinkJPQLParserTests.class,
// AllEclipseLinkJPQLParserTests2_1.class,
// AllEclipseLinkJPQLParserTests2_4.class,
// AllEclipseLinkJPQLParserTests2_5.class,
// AllJPQLParserConcurrentTests.class,
JPQLExpressionTestJakartaData.class
})
@RunWith(JPQLTestRunner.class)
Expand Down
66 changes: 49 additions & 17 deletions ...est/java/org/eclipse/persistence/jpa/tests/jpql/parser/JPQLExpressionTestJakartaData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,20 @@
*
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
*/

// Contributors:
// Oracle - initial API and implementation
//
package org.eclipse.persistence.jpa.tests.jpql.parser;

import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.abstractSchemaName;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.equal;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.from;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.identificationVariableDeclaration;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.numeric;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.rangeVariableDeclaration;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.path;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.select;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.selectStatement;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.set;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.update;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.updateStatement;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.variable;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.virtualVariable;
import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.where;
Expand Down Expand Up @@ -120,40 +120,72 @@ public void testGeneratedSelect() {
}

@Test
public void testFunctionNameAsStateFieldWithImplicitVariable() {
public void testFunctionNameAsImplicitStateField() {

String inputJPQLQuery = "SELECT this FROM Order WHERE length = 1";
// String inputJPQLQuery = "UPDATE Box SET length = length + 1";

SelectStatementTester selectStatement = selectStatement(
select(variable("this")),
from(identificationVariableDeclaration(
rangeVariableDeclaration(abstractSchemaName("Order"), virtualVariable("this")))),
from("Order", "{this}"),
where(equal(virtualVariable("this", "length"), numeric(1)))
);

testJakartaDataQuery(inputJPQLQuery, selectStatement);
}

@Test
public void testFunctionNameAsImplicitStateFieldInNumericExpression() {

String inputJPQLQuery = "SELECT this FROM Order WHERE id = length + 1";

SelectStatementTester selectStatement = selectStatement(
select(variable("this")),
from("Order", "{this}"),
where(equal(
virtualVariable("this", "id"),
virtualVariable("this", "length").add(numeric(1))
))
);

testJakartaDataQuery(inputJPQLQuery, selectStatement);
}

@Test
public void testUpdateFunctionNameAsImplicitStateFieldInNumericExpression() {

String inputJPQLQuery = "UPDATE Order SET length = length + 1";

UpdateStatementTester selectStatement = updateStatement(
update(
"Order",
set(path("{order}.length"),
virtualVariable("order", "length")
.add(numeric(1))
))
);

testJakartaDataQuery(inputJPQLQuery, selectStatement);
}

private JPQLExpression checkAliasFrom(String actualQuery, String expectedAlias) {
JPQLExpression jpqlExpression = JPQLQueryBuilder.buildQuery(actualQuery, JPQLGrammar3_2.instance(), JPQLStatementBNF.ID, true, true);
SelectStatement selectStatement = (SelectStatement)jpqlExpression.getQueryStatement();
FromClause fromClause = (FromClause)selectStatement.getFromClause();
IdentificationVariableDeclaration identificationVariableDeclaration = (IdentificationVariableDeclaration)fromClause.getDeclaration();
RangeVariableDeclaration rangeVariableDeclaration = (RangeVariableDeclaration)identificationVariableDeclaration.getRangeVariableDeclaration();
IdentificationVariable identificationVariable = (IdentificationVariable)rangeVariableDeclaration.getIdentificationVariable();
SelectStatement selectStatement = (SelectStatement) jpqlExpression.getQueryStatement();
FromClause fromClause = (FromClause) selectStatement.getFromClause();
IdentificationVariableDeclaration identificationVariableDeclaration = (IdentificationVariableDeclaration) fromClause.getDeclaration();
RangeVariableDeclaration rangeVariableDeclaration = (RangeVariableDeclaration) identificationVariableDeclaration.getRangeVariableDeclaration();
IdentificationVariable identificationVariable = (IdentificationVariable) rangeVariableDeclaration.getIdentificationVariable();
String alias = identificationVariable.getText();
assertEquals(expectedAlias, alias);
return jpqlExpression;
}

private void checkAliasesWhere(JPQLExpression jpqlExpression, String expectedAlias) {
SelectStatement selectStatement = (SelectStatement)jpqlExpression.getQueryStatement();
WhereClause whereClause = (WhereClause)selectStatement.getWhereClause();
SelectStatement selectStatement = (SelectStatement) jpqlExpression.getQueryStatement();
WhereClause whereClause = (WhereClause) selectStatement.getWhereClause();
ListIterable<Expression> expressions = whereClause.children();
List<IdentificationVariable> identificationVariableList = new ArrayList<>();
collectIdentificationVariables(expressions, identificationVariableList);
for (IdentificationVariable identificationVariable: identificationVariableList) {
for (IdentificationVariable identificationVariable : identificationVariableList) {
if (expectedAlias.equals(identificationVariable.getText())) {
assertEquals(expectedAlias, identificationVariable.getText());
} else {
Expand All @@ -164,7 +196,7 @@ private void checkAliasesWhere(JPQLExpression jpqlExpression, String expectedAli
}

private void collectIdentificationVariables(ListIterable<Expression> expressions, List<IdentificationVariable> identificationVariableList) {
for (Expression expression: expressions) {
for (Expression expression : expressions) {
if (expression.children() != null) {
collectIdentificationVariables(expression.children(), identificationVariableList);
}
Expand Down
Loading

0 comments on commit 5d1620a

Please sign in to comment.