IndexOutOfBoundsException when JPQL has embeddable or relational attr…

…ibutes without the optional entity identification variable - bugfix and unit tests (#2278) * IndexOutOfBoundsException when JPQL has embeddable or relational attributes without the optional entity identification variable - bugfix and unit tests Signed-off-by: Radek Felcman <[email protected]>
eclipse-ee4j · Nov 13, 2024 · f80c0f3 · f80c0f3
1 parent 8292f96
commit f80c0f3
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 27 deletions.
diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml
@@ -50,7 +50,7 @@ jobs:
           java-version: ${{ matrix.java_version }}
           cache: maven
       - name: Execute Tests on JDK ${{ matrix.java_version }}
-        run: mvn -B -V -U -C -Pstaging,oss-release,test-lrg,mysql clean verify -Dgpg.skip=true -Dwarn.limit=15 -Dcomp.xlint=-Xlint:none
+        run: mvn -B -V -U -C -Pstaging,oss-release,test-lrg,mysql clean verify -Dgpg.skip=true -Dwarn.limit=15 -Dcomp.xlint=-Xlint:none -e
 
   common_validations_job:
     name: Common Validations

diff --git a/...test.advanced/src/main/java/org/eclipse/persistence/testing/models/jpa/advanced/Door.java b/...test.advanced/src/main/java/org/eclipse/persistence/testing/models/jpa/advanced/Door.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2022 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2024 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -45,6 +45,17 @@ public class Door implements Serializable, Cloneable {
     @ManyToOne(cascade=CascadeType.ALL, fetch=FetchType.LAZY)
     private Room room;
 
+    public Door() {
+    }
+
+    public Door(int id, int width, int height, Room room, Date warrantyDate) {
+        this.id = id;
+        this.width = width;
+        this.height = height;
+        this.room = room;
+        WarrantyDate = warrantyDate;
+    }
+
     public int getId() {
         return id;
     }

diff --git a/...test.advanced/src/main/java/org/eclipse/persistence/testing/models/jpa/advanced/Room.java b/...test.advanced/src/main/java/org/eclipse/persistence/testing/models/jpa/advanced/Room.java
@@ -28,7 +28,7 @@
 import java.io.Serializable;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
-import java.util.Collection;
+import java.util.List;
 
 @Entity
 @Table(name="CMP3_ROOM")
@@ -56,7 +56,7 @@ public enum Status {
     private Status status;
 
     @OneToMany(mappedBy="room", cascade=CascadeType.ALL, fetch=FetchType.LAZY)
-    private Collection<Door> doors;
+    private List<Door> doors;
 
     public Room() {
     }
@@ -110,11 +110,11 @@ public void setStatus(Status status) {
         this.status = status;
     }
 
-    public Collection<Door> getDoors() {
+    public List<Door> getDoors() {
         return doors;
     }
 
-    public void setDoors(Collection<Door> doors) {
+    public void setDoors(List<Door> doors) {
         this.doors = doors;
     }
 

diff --git a/.../eclipse/persistence/testing/tests/jpa/jpql/advanced/JUnitJPQLJakartaDataNoAliasTest.java b/.../eclipse/persistence/testing/tests/jpa/jpql/advanced/JUnitJPQLJakartaDataNoAliasTest.java
@@ -29,10 +29,13 @@
 
 import java.math.BigDecimal;
 import java.math.BigInteger;
+import java.sql.Date;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Stream;
 import org.eclipse.persistence.sessions.server.ServerSession;
 import org.eclipse.persistence.testing.models.jpa.advanced.AdvancedTableCreator;
+import org.eclipse.persistence.testing.models.jpa.advanced.Door;
 import org.eclipse.persistence.testing.models.jpa.advanced.Room;
 
 /**
@@ -55,10 +58,10 @@ public class JUnitJPQLJakartaDataNoAliasTest extends JUnitTestCase {
     private static final String STRING_DATA_LIKE_EXPRESSION = "A%"; // should match STRING_DATA
     private static final Room[] ROOMS = new Room[]{
         null, // Skip array index 0
-        aRoom(1, 1, 1, 1, Room.Status.FREE),
-        aRoom(2, 1, 1, 1, Room.Status.FREE),
-        aRoom(3, 1, 1, 1, Room.Status.OCCUPIED),
-        aRoom(4, 1, 1, 1, Room.Status.OCCUPIED)
+        aRoom(1, 1, 1, 1, Room.Status.FREE, 1001, 100, 100, new Date(1L)),
+        aRoom(2, 1, 1, 1, Room.Status.FREE, 2001, 200, 200, new Date(2L)),
+        aRoom(3, 1, 1, 1, Room.Status.OCCUPIED, 3001, 300, 300, new Date(3L)),
+        aRoom(4, 1, 1, 1, Room.Status.OCCUPIED, 4001, 400, 400, new Date(4L)),
     };
     private static final long ROOMS_COUNT = ROOMS.length - 1; // we ignore the first one with index 0
 
@@ -111,6 +114,7 @@ public static Test suite() {
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testSelectQueryImplicitThisVariableInAggregateFunctionPath"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testSelectQueryImplicitThisVariableInArithmeticExpression"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testSelectQueryImplicitThisVariableAndEnum"));
+        suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testSelectQueryImplicitThisVariableAndRelationalAttributes"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testUpdateImplicitVariableInArithmeticExpression"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testDeleteQueryLengthInExpressionOnLeft"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testDeleteQueryLengthInExpressionOnRight"));
@@ -318,7 +322,7 @@ public void testGeneratedSelectNoAliasFromWhere() {
     }
 
     public void testUpdateQueryLengthInAssignmentAndExpression() {
-        resetRooms();
+        resetRooms(false);
         long numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                 "UPDATE Room SET length = length + 1").executeUpdate());
         assertTrue("All rooms should be updated", numberOfChanges == ROOMS_COUNT);
@@ -330,7 +334,7 @@ public void testUpdateQueryLengthInAssignmentAndExpression() {
     }
 
     public void testSelectQueryLengthInAssignmentAndExpression() {
-        resetRooms();
+        resetRooms(false);
         List<Room> roomsWithIdOne = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                 "SELECT this FROM Room WHERE id + length = length + 1", Room.class).getResultList());
         assertTrue("Number of rooms with ID = 1", roomsWithIdOne.size() == 1);
@@ -339,7 +343,7 @@ public void testSelectQueryLengthInAssignmentAndExpression() {
 
     // Covers https://github.com/eclipse-ee4j/eclipselink/issues/2182
     public void testSelectQueryImplicitThisVariableInPath() {
-        resetRooms();
+        resetRooms(false);
         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())
@@ -351,7 +355,7 @@ public void testSelectQueryImplicitThisVariableInPath() {
 
     // Covers https://github.com/eclipse-ee4j/eclipselink/issues/2192
     public void testSelectQueryImplicitThisVariableInAggregateFunctionPath() {
-        resetRooms();
+        resetRooms(false);
         Integer roomsMaxWidth = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                         "SELECT MAX(width) FROM Room", Integer.class)
                 .getSingleResult());
@@ -360,7 +364,7 @@ public void testSelectQueryImplicitThisVariableInAggregateFunctionPath() {
 
     // Covers https://github.com/eclipse-ee4j/eclipselink/issues/2247
     public void testSelectQueryImplicitThisVariableInArithmeticExpression() {
-        resetRooms();
+        resetRooms(false);
         int roomCapacity = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                         "SELECT length * width * height  FROM Room WHERE id = :idParam", Integer.class)
                 .setParameter("idParam", ROOMS[1].getId())
@@ -370,7 +374,7 @@ public void testSelectQueryImplicitThisVariableInArithmeticExpression() {
 
     // Covers https://github.com/eclipse-ee4j/eclipselink/issues/2185
     public void testSelectQueryImplicitThisVariableAndEnum() {
-        resetRooms();
+        resetRooms(false);
         List<Room> rooms = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                         "SELECT NEW org.eclipse.persistence.testing.models.jpa.advanced.Room(id, width, length, height, status) " +
                                 "FROM Room " +
@@ -382,8 +386,21 @@ public void testSelectQueryImplicitThisVariableAndEnum() {
         assertEquals(ROOMS[1], rooms.get(0));
     }
 
+    // Covers https://github.com/eclipse-ee4j/eclipselink/issues/2188
+    public void testSelectQueryImplicitThisVariableAndRelationalAttributes() {
+        resetRooms(false);
+        List<Door> doors = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
+                                "FROM Door " +
+                                "WHERE room.id = :idParam " +
+                                "ORDER BY width",
+                        Door.class)
+                .setParameter("idParam", ROOMS[1].getId())
+                .getResultList());
+        assertEquals(ROOMS[1].getDoors().get(0).getId(), doors.get(0).getId());
+    }
+
     public void testUpdateImplicitVariableInArithmeticExpression() {
-        resetRooms();
+        resetRooms(false);
         int numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                 "UPDATE Room SET width = width * :widthMultiplicator WHERE id = :id")
                 .setParameter("widthMultiplicator", 5)
@@ -395,7 +412,7 @@ public void testUpdateImplicitVariableInArithmeticExpression() {
     }
 
     public void testDeleteQueryLengthInExpressionOnLeft() {
-        resetRooms();
+        resetRooms(true);
         assertTrue("Number of remaining rooms", getAllRooms().count() == ROOMS_COUNT);
         int numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                 "DELETE FROM Room WHERE length = id - 1").executeUpdate());
@@ -405,15 +422,15 @@ public void testDeleteQueryLengthInExpressionOnLeft() {
     }
 
     public void testDeleteQueryLengthInExpressionOnRight() {
-        resetRooms();
+        resetRooms(true);
         int numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                 "DELETE FROM Room WHERE id = length + 1").executeUpdate());
         assertTrue("Number of rooms with ID = 1 deleted", numberOfChanges == 1);
         assertTrue("Number of remaining rooms", getAllRooms().count() == ROOMS_COUNT - 1);
     }
 
     public void tesUpdateQueryWithThisVariable() {
-        resetRooms();
+        resetRooms(false);
         long numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                 "UPDATE Room SET length = this.length + 1").executeUpdate());
         assertTrue("All rooms should be updated", numberOfChanges == ROOMS_COUNT);
@@ -427,7 +444,7 @@ public void tesUpdateQueryWithThisVariable() {
 
     // Covers https://github.com/eclipse-ee4j/eclipselink/issues/2197
     public void testThisVariableInPathExpressionUpdate() {
-        resetRooms();
+        resetRooms(false);
         int numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                 "UPDATE Room SET this.length = 10 WHERE this.id = 1").executeUpdate());
         assertTrue("Number of rooms with ID = 1 modified is " + numberOfChanges, numberOfChanges == 1);
@@ -437,7 +454,7 @@ public void testThisVariableInPathExpressionUpdate() {
 
     // Covers https://github.com/eclipse-ee4j/eclipselink/issues/2184
     public void testThisVariableInPathAndIdFunctionExpressionUpdate() {
-        resetRooms();
+        resetRooms(false);
         int numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                 "UPDATE Room SET length = 10 WHERE ID(this) = :idParam")
                 .setParameter("idParam", 1)
@@ -449,7 +466,7 @@ public void testThisVariableInPathAndIdFunctionExpressionUpdate() {
 
     // Covers https://github.com/eclipse-ee4j/eclipselink/issues/2198
     public void testThisVariableInPathExpressionDelete() {
-        resetRooms();
+        resetRooms(true);
         int numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
                 "DELETE FROM Room WHERE this.length < 10").executeUpdate());
         assertTrue("Number of rooms deleted is " + numberOfChanges, numberOfChanges == ROOMS_COUNT);
@@ -481,22 +498,29 @@ private Room findRoomById(int i) {
         });
     }
 
-    private static Room aRoom(int id, int width, int length, int height, Room.Status status) {
+    private static Room aRoom(int id, int width, int length, int height, Room.Status status, int doorId, int doorWidth, int doorHeight, Date doorWarrantyDate) {
         Room room = new Room();
         room.setId(id);
         room.setWidth(width);
         room.setLength(length);
         room.setHeight(height);
         room.setStatus(status);
+        List<Door> doors = new ArrayList<>();
+        doors.add(new Door(doorId, doorWidth, doorHeight, room, doorWarrantyDate));
+        room.setDoors(doors);
         return room;
     }
 
-    private void resetRooms() {
+    private void resetRooms(boolean noDoors) {
         getEntityManagerFactory().runInTransaction(em -> {
+            em.createQuery("DELETE FROM Door ").executeUpdate();
             em.createQuery("DELETE FROM Room").executeUpdate();
             for (int i = 1; i < ROOMS.length; i++) {
                 em.persist(ROOMS[i]);
             }
+            if (noDoors) {
+                em.createQuery("DELETE FROM Door ").executeUpdate();
+            }
         });
     }
 

diff --git a/...ce.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/AbstractSemanticValidator.java b/...ce.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/AbstractSemanticValidator.java
@@ -2421,7 +2421,18 @@ else if ((pathType == PathType.ASSOCIATION_FIELD_ONLY) &&
                         // Third path extension did not validate the path expression, continue validating it
                         else {
                             type = helper.getType(expression);
-
+                            if (type == null && expression.getParentExpression().isGenerateImplicitThisAlias()) {
+                                //Seems path is not available. Try again with new virtual "this" IdentificationVariable.
+                                // Remove the used identification variable since it is not
+                                // Entity identification variable as generated/implicit "this" is expected.
+                                usedIdentificationVariables.remove(expression.getIdentificationVariable());
+                                //Add generated/implicit "this" alias.
+                                expression.setVirtualIdentificationVariable(Expression.THIS);
+                                type = helper.getType(expression);
+                                if (helper.isTypeResolvable(type)) {
+                                    return valid;
+                                }
+                            }
                             // Does not resolve to a valid path
                             if (!helper.isTypeResolvable(type)) {
                                 addProblem(expression, StateFieldPathExpression_NotResolvable, expression.toActualText());

diff --git a/...pa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/AbstractPathExpression.java b/...pa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/AbstractPathExpression.java
@@ -323,7 +323,7 @@ public final int pathSize() {
      *
      * @param variableName The identification variable that was generated to identify the "root" object
      */
-    protected final void setVirtualIdentificationVariable(String variableName) {
+    public final void setVirtualIdentificationVariable(String variableName) {
 
         identificationVariable = new IdentificationVariable(this, variableName, true);
 

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
@@ -28,6 +28,7 @@
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.new_;
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.numeric;
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.orderBy;
+import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.orderByItem;
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.orderByItemAsc;
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.orderByItemDesc;
 import static org.eclipse.persistence.jpa.tests.jpql.parser.JPQLParserTester.path;
@@ -274,6 +275,25 @@ public void testSelectWithImplicitThisAliasAndEnum() {
         testJakartaDataQuery(inputJPQLQuery, selectStatement);
     }
 
+    // Covers https://github.com/eclipse-ee4j/eclipselink/issues/2188
+    // This is just basic syntax tests. Semantic validation from org.eclipse.persistence.core Maven module is required for full test
+    // to resolve that path "location.address.city" is correct.
+    // Full test is in org.eclipse.persistence.jpa.testapps.jpql Maven module
+    @Test
+    public void testSelectWithImplicitThisAliasAndRelationalAttributes() {
+
+        String inputJPQLQuery = "FROM Business WHERE location.address.city=:cityParam ORDER BY name";
+
+        SelectStatementTester selectStatement = selectStatement(
+                select(variable("this")),
+                from("Business", "{this}"),
+                where(equal(path("location.address.city"), inputParameter(":cityParam"))),
+                orderBy(orderByItem(virtualVariable("this", "name")))
+        );
+
+        testJakartaDataQuery(inputJPQLQuery, selectStatement);
+    }
+
     @Test
     public void testUpdateFunctionNameAsImplicitStateFieldInNumericExpression() {