Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[master] IndexOutOfBoundsException when JPQL has embeddable or relational attributes without the optional entity identification variable - bugfix and unit tests #2278

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/maven.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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() {
}
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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

Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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())
Expand All @@ -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());
Expand All @@ -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())
Expand All @@ -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 " +
Expand All @@ -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)
Expand All @@ -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());
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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)
Expand All @@ -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);
Expand Down Expand Up @@ -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();
}
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {

Expand Down
Loading