Bigfix + unit tests for #2182 and #2192 issues

JPQL SELECT not allowing entity attribute without optional entity identification variable #2182 - bug fix (main + tests) Unable to omit optional entity identification variable from arguments to built-in aggregate functions #2192 - bug fix (main + tests) Signed-off-by: Radek Felcman <[email protected]>
eclipse-ee4j · Sep 9, 2024 · c732dbe · c732dbe
1 parent c149d8a
commit c732dbe
Show file tree

Hide file tree

Showing 9 changed files with 102 additions and 15 deletions.
diff --git a/.../eclipse/persistence/testing/tests/jpa/jpql/advanced/JUnitJPQLJakartaDataNoAliasTest.java b/.../eclipse/persistence/testing/tests/jpa/jpql/advanced/JUnitJPQLJakartaDataNoAliasTest.java
@@ -107,6 +107,8 @@ public static Test suite() {
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testGeneratedSelect"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testUpdateQueryLengthInAssignmentAndExpression"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testSelectQueryLengthInAssignmentAndExpression"));
+        suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testSelectQueryImplicitThisVariableInPath"));
+        suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testSelectQueryImplicitThisVariableInAggregateFunctionPath"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testUpdateImplicitVariableInArithmeticExpression"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testDeleteQueryLengthInExpressionOnLeft"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testDeleteQueryLengthInExpressionOnRight"));
@@ -332,6 +334,28 @@ public void testSelectQueryLengthInAssignmentAndExpression() {
         assertTrue("Number of rooms with ID = 1", roomsWithIdOne.size() == 1);
     }
 
+
+    // Covers https://github.com/eclipse-ee4j/eclipselink/issues/2182
+    public void testSelectQueryImplicitThisVariableInPath() {
+        resetRooms();
+        List<Integer> roomsLengthWithIdOne = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
+                "SELECT length FROM Room WHERE length IS NOT NULL AND id = :idParam", Integer.class)
+                .setParameter("idParam", ROOMS[1].getId())
+                .getResultList());
+        assertTrue("Number of rooms with ID = 1", roomsLengthWithIdOne.size() == 1);
+        assertTrue("Room ID = " + ROOMS[1].getId() + " has length of ", roomsLengthWithIdOne.get(0) == ROOMS[1].getLength());
+    }
+
+
+    // Covers https://github.com/eclipse-ee4j/eclipselink/issues/2192
+    public void testSelectQueryImplicitThisVariableInAggregateFunctionPath() {
+        resetRooms();
+        Integer roomsMaxWidth = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
+                        "SELECT MAX(width) FROM Room", Integer.class)
+                .getSingleResult());
+        assertTrue("Some Room with max width exist ", roomsMaxWidth != null);
+    }
+
     public void testUpdateImplicitVariableInArithmeticExpression() {
         resetRooms();
         int numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(

diff --git a/...ersistence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/AbstractValidator.java b/...ersistence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/AbstractValidator.java
@@ -973,7 +973,7 @@ public void visit(NullExpression expression) {
         public void visit(StateFieldPathExpression expression) {
             JPQLQueryBNF originQueryBNF = queryBNF;
             if (Expression.THIS.equalsIgnoreCase(expression.toString()) &&
-                    expression.getParentExpression().isGenerateThisPrefix() &&
+                    expression.getParentExpression().isGenerateImplicitThisAlias() &&
                     expression.getIdentificationVariable() != null &&
                     ((IdentificationVariable)(expression.getIdentificationVariable())).isVirtual()) {
                 queryBNF = expression.getQueryBNF();

diff --git a/...ce.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/AbstractFromClause.java b/...ce.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/AbstractFromClause.java
@@ -344,7 +344,7 @@ else if (hierarchicalQueryClause == null) {
             identificationVariableDeclaration.hasRangeVariableDeclaration() && identificationVariableDeclaration.getRangeVariableDeclaration() instanceof RangeVariableDeclaration rangeVariableDeclaration &&
             rangeVariableDeclaration.hasIdentificationVariable() && rangeVariableDeclaration.getIdentificationVariable() instanceof IdentificationVariable identificationVariable &&
             Expression.THIS.equals(identificationVariable.getText())) {
-            this.getParentExpression().setGenerateThisPrefix(true);
+            this.getParentExpression().setGenerateImplicitThisAlias(true);
         }
     }
 

diff --git a/...pa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/IdentificationVariable.java b/...pa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/IdentificationVariable.java
@@ -96,7 +96,7 @@ public final class IdentificationVariable extends AbstractExpression {
     public IdentificationVariable(AbstractExpression parent, String identificationVariable) {
         super(parent, identificationVariable);
         //In subqueries "this" generation is not allowed. There are expected qualified IdentificationVariable from query string
-        if (!Expression.THIS.equalsIgnoreCase(identificationVariable) && getParentExpression().isGenerateThisPrefix() && !isInsideSubquery()) {
+        if (!Expression.THIS.equalsIgnoreCase(identificationVariable) && getParentExpression().isGenerateImplicitThisAlias() && !isInsideSubquery()) {
             this.setVirtualIdentificationVariable(Expression.THIS);
         }
     }
@@ -133,7 +133,7 @@ public IdentificationVariable(AbstractExpression parent, String identificationVa
     public void accept(ExpressionVisitor visitor) {
         //In "this" (Jakarta Data) generation mode pass for a validation stateFieldPathExpression
         //generated by this.setVirtualIdentificationVariable(Expression.THIS); in constructor above
-        if (getParentExpression().isGenerateThisPrefix() && isVirtual() && stateFieldPathExpression != null) {
+        if (getParentExpression().isGenerateImplicitThisAlias() && isVirtual() && stateFieldPathExpression != null) {
             visitor.visit(getStateFieldPathExpression());
         } else {
             visitor.visit(this);

diff --git a/...stence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/JPQLExpression.java b/...stence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/JPQLExpression.java
@@ -16,8 +16,12 @@
 //
 package org.eclipse.persistence.jpa.jpql.parser;
 
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
+
 import org.eclipse.persistence.jpa.jpql.ExpressionTools;
 import org.eclipse.persistence.jpa.jpql.JPAVersion;
 import org.eclipse.persistence.jpa.jpql.WordParser;
@@ -89,7 +93,7 @@ public final class JPQLExpression extends AbstractExpression implements ParentEx
      */
     private boolean jakartaData = false;
 
-    private boolean generateThisPrefix = false;
+    private boolean generateImplicitThisAlias = false;
 
     /**
      * Creates a new <code>JPQLExpression</code>, which is the root of the JPQL parsed tree.
@@ -161,6 +165,7 @@ public JPQLExpression(CharSequence jpqlFragment,
         if (jakartaData) {
             jpqlFragment = preParse(jpqlFragment);
         }
+        generateImplicitThisAlias = generateImplicitThisAliasDetection((String) jpqlFragment);
         parse(new WordParser(jpqlFragment), tolerant);
     }
 
@@ -257,13 +262,13 @@ public JPQLQueryBNF getQueryBNF() {
     }
 
     @Override
-    public boolean isGenerateThisPrefix() {
-        return generateThisPrefix;
+    public boolean isGenerateImplicitThisAlias() {
+        return generateImplicitThisAlias;
     }
 
     @Override
-    public void setGenerateThisPrefix(boolean generateThisPrefix) {
-        this.generateThisPrefix = generateThisPrefix;
+    public void setGenerateImplicitThisAlias(boolean generateImplicitThisAlias) {
+        this.generateImplicitThisAlias = generateImplicitThisAlias;
     }
 
     @Override
@@ -431,4 +436,26 @@ private CharSequence preParse(CharSequence jpqlFragment) {
         }
         return jpqlFragment;
     }
+
+    //Quick pre-check if 'this' entity alias should be generated automatically
+    //There are some additional checks later during parsing query
+    private boolean generateImplicitThisAliasDetection(String jpqlFragment) {
+        String jpqlFragmentLowerCase = jpqlFragment.toLowerCase(Locale.ROOT);
+        String fromLowerCase = Expression.FROM.toLowerCase(Locale.ROOT);
+        int count = 0;
+        int fromIndex = jpqlFragmentLowerCase.indexOf(fromLowerCase);
+        int formOccurrences = Collections.frequency(Arrays.asList(jpqlFragmentLowerCase.split(" ")), fromLowerCase);
+        if (fromIndex >= 1 && formOccurrences == 1) {
+            String jpqlFragmentFrom = jpqlFragment.substring(fromIndex);
+            WordParser wordParser = new WordParser(jpqlFragmentFrom);
+            wordParser.moveForwardIgnoreWhitespace(Expression.FROM);
+            wordParser.skipLeadingWhitespace();
+            while (!wordParser.word().equalsIgnoreCase(Expression.WHERE.toLowerCase(Locale.ROOT)) && !wordParser.word().isEmpty()) {
+                count++;
+                wordParser.moveForwardIgnoreWhitespace(wordParser.word());
+                wordParser.skipLeadingWhitespace();
+            }
+        }
+        return count == 1;
+    }
 }
diff --git a/...ence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/ParentExpression.java b/...ence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/ParentExpression.java
@@ -19,9 +19,9 @@ public interface ParentExpression extends Expression {
     /**
      * Whether should automatically add missing "this" prefix into where field variables if it doesn't exist.
      */
-    boolean isGenerateThisPrefix();
+    boolean isGenerateImplicitThisAlias();
 
-    void setGenerateThisPrefix(boolean generateThisPrefix);
+    void setGenerateImplicitThisAlias(boolean generateImplicitThisAlias);
 
     boolean isParentExpression();
 }
diff --git a/....jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/RangeVariableDeclaration.java b/....jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/RangeVariableDeclaration.java
@@ -367,7 +367,7 @@ private void addMissingAlias(String aliasName) {
                 || isMissingAliasInUpdateClause()
                 || isMissingAliasInDeleteFromClause()) {
             this.setVirtualIdentificationVariable(aliasName);
-            this.getParentExpression().setGenerateThisPrefix(true);
+            this.getParentExpression().setGenerateImplicitThisAlias(true);
         }
     }
 

diff --git a/...istence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/SubExpression.java b/...istence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/SubExpression.java
@@ -48,13 +48,13 @@ public SubExpression(AbstractExpression parent, JPQLQueryBNF queryBNF) {
     }
 
     @Override
-    public boolean isGenerateThisPrefix() {
+    public boolean isGenerateImplicitThisAlias() {
         return generateThisPrefix;
     }
 
     @Override
-    public void setGenerateThisPrefix(boolean generateThisPrefix) {
-        this.generateThisPrefix = generateThisPrefix;
+    public void setGenerateImplicitThisAlias(boolean generateImplicitThisAlias) {
+        this.generateThisPrefix = generateImplicitThisAlias;
     }
 
     @Override

diff --git a/...est/java/org/eclipse/persistence/jpa/tests/jpql/parser/JPQLExpressionTestJakartaData.java b/...est/java/org/eclipse/persistence/jpa/tests/jpql/parser/JPQLExpressionTestJakartaData.java
@@ -16,9 +16,14 @@
 //
 package org.eclipse.persistence.jpa.tests.jpql.parser;
 
+import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.and;
+import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.avg;
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.equal;
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.id;
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.from;
+import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.inputParameter;
+import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.isNotNull;
+import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.max;
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.numeric;
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.path;
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.select;
@@ -171,6 +176,37 @@ public void testFunctionNameAsImplicitStateFieldInNumericExpression() {
         testJakartaDataQuery(inputJPQLQuery, selectStatement);
     }
 
+    @Test
+    public void testFunctionNameAsImplicitStateFieldInSelectWhereExpression() {
+
+        String inputJPQLQuery = "SELECT name FROM Order WHERE name IS NOT NULL AND id = :idParam";
+
+        SelectStatementTester selectStatement = selectStatement(
+                select(virtualVariable("this", "name")),
+                from("Order", "{this}"),
+                where(and(isNotNull(virtualVariable("this", "name")),
+                        equal(virtualVariable("this", "id"),
+                                inputParameter(":idParam"))
+                ))
+        );
+
+        testJakartaDataQuery(inputJPQLQuery, selectStatement);
+    }
+
+    @Test
+    public void testFunctionNameAsImplicitStateFieldInSelectAggregateMaxExpression() {
+
+        String inputJPQLQuery = "SELECT MAX(price) FROM Item WHERE AVG(price) = 100";
+
+        SelectStatementTester selectStatement = selectStatement(
+                select(max(virtualVariable("this", "price"))),
+                from("Item", "{this}"),
+                where(equal(avg(virtualVariable("this", "price")), numeric(100)))
+        );
+
+        testJakartaDataQuery(inputJPQLQuery, selectStatement);
+    }
+
     @Test
     public void testUpdateFunctionNameAsImplicitStateFieldInNumericExpression() {