Fix for bug 430042 (2.5 branch checkin)
- Allow an cache only pk query to succeed when a query is performed on related objects
- Resolves an expression issue when the query string is constructed in a certain order
- Reviewed: cdelahunt
diff --git a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/expressions/LogicalExpression.java b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/expressions/LogicalExpression.java
index e9edf1a..c717398 100644
--- a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/expressions/LogicalExpression.java
+++ b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/expressions/LogicalExpression.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 1998, 2013 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2014 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 v1.0 and Eclipse Distribution License v. 1.0
* which accompanies this distribution.
@@ -73,6 +73,14 @@
return false;
}
}
+
+ // Ensure that both sides of the expression tree have a session, for correct descriptor, value resolution
+ if (this.secondChild.getSession() == null && this.firstChild.getSession() != null) {
+ this.secondChild.getBuilder().setSession(this.firstChild.getSession());
+ } else if (this.firstChild.getSession() == null && this.secondChild.getSession() != null) {
+ this.firstChild.getBuilder().setSession(this.secondChild.getSession());
+ }
+
boolean validExpression = this.firstChild.extractValues(primaryKeyOnly, requireExactMatch, descriptor, primaryKeyRow, translationRow);
if (requireExactMatch && (!validExpression)) {
return false;
@@ -94,6 +102,14 @@
return false;
}
}
+
+ // Ensure that both sides of the expression tree have a session, for correct descriptor, field resolution
+ if (this.secondChild.getSession() == null && this.firstChild.getSession() != null) {
+ this.secondChild.getBuilder().setSession(this.firstChild.getSession());
+ } else if (this.firstChild.getSession() == null && this.secondChild.getSession() != null) {
+ this.firstChild.getBuilder().setSession(this.secondChild.getSession());
+ }
+
boolean validExpression = this.firstChild.extractFields(requireExactMatch, primaryKey, descriptor, searchFields, foundFields);
if (requireExactMatch && (!validExpression)) {
return false;
diff --git a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/expressions/RelationExpression.java b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/expressions/RelationExpression.java
index 28259f4..365005e 100644
--- a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/expressions/RelationExpression.java
+++ b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/expressions/RelationExpression.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 1998, 2013 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2014 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 v1.0 and Eclipse Distribution License v. 1.0
* which accompanies this distribution.
@@ -233,6 +233,9 @@
return false;
}
+ // Descriptor to use for child query key
+ ClassDescriptor descriptorForChild = null;
+
// Ensure that the primary key is being queried on.
if (this.firstChild.isFieldExpression()) {
FieldExpression child = (FieldExpression)this.firstChild;
@@ -247,7 +250,12 @@
if (!child.getBaseExpression().isExpressionBuilder()) {
return false;
}
- DatabaseMapping mapping = descriptor.getObjectBuilder().getMappingForAttributeName(child.getName());
+ descriptorForChild = ((ExpressionBuilder)child.getBaseExpression()).getDescriptor();
+ if (descriptorForChild == null) {
+ descriptorForChild = descriptor;
+ }
+ DatabaseMapping mapping = descriptorForChild.getObjectBuilder().getMappingForAttributeName(child.getName());
+
if (mapping != null) {
if (primaryKeyOnly && !mapping.isPrimaryKeyMapping()) {
return false;
@@ -264,7 +272,7 @@
field = ((AbstractColumnMapping)mapping).getField();
} else {
// Only get field for the source object.
- field = descriptor.getObjectBuilder().getFieldForQueryKeyName(child.getName());
+ field = descriptorForChild.getObjectBuilder().getFieldForQueryKeyName(child.getName());
}
} else if (this.secondChild.isFieldExpression()) {
FieldExpression child = (FieldExpression)this.secondChild;
@@ -279,7 +287,12 @@
if (!child.getBaseExpression().isExpressionBuilder()) {
return false;
}
- DatabaseMapping mapping = descriptor.getObjectBuilder().getMappingForAttributeName(child.getName());
+ descriptorForChild = ((ExpressionBuilder)child.getBaseExpression()).getDescriptor();
+ if (descriptorForChild == null) {
+ descriptorForChild = descriptor;
+ }
+ DatabaseMapping mapping = descriptorForChild.getObjectBuilder().getMappingForAttributeName(child.getName());
+
// Only support referencing limited number of relationship types.
if (mapping != null) {
if (primaryKeyOnly && !mapping.isPrimaryKeyMapping()) {
@@ -294,12 +307,26 @@
}
field = ((AbstractColumnMapping)mapping).getField();
} else {
- field = descriptor.getObjectBuilder().getFieldForQueryKeyName(child.getName());
+ field = descriptorForChild.getObjectBuilder().getFieldForQueryKeyName(child.getName());
}
} else {
return false;
}
- if ((field == null) || (primaryKeyOnly && !descriptor.getPrimaryKeyFields().contains(field))) {
+ if (field == null) {
+ return false;
+ }
+ // Check child descriptor's primary key fields if the passed descriptor does not contain the field
+ if (primaryKeyOnly && !descriptor.getPrimaryKeyFields().contains(field)) {
+ if (descriptorForChild != null && descriptorForChild != descriptor && descriptorForChild.getPrimaryKeyFields().contains(field)) {
+ // Child descriptor's pk fields contains the field, return true.
+ // Do not add the field from the query key's descriptor to the primaryKeyRow
+ return true;
+ } else {
+ return false;
+ }
+ }
+ // Do not replace the field in the row with the same field
+ if (primaryKeyRow.get(field) != null) {
return false;
}
primaryKeyRow.put(field, value);
diff --git a/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/JUnitJPQLParameterTestSuite.java b/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/JUnitJPQLParameterTestSuite.java
index 94291eb..d5d8e53 100644
--- a/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/JUnitJPQLParameterTestSuite.java
+++ b/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/JUnitJPQLParameterTestSuite.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 1998, 2013 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2014 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 v1.0 and Eclipse Distribution License v. 1.0
* which accompanies this distribution.
@@ -13,24 +13,31 @@
package org.eclipse.persistence.testing.tests.jpa.jpql;
-
+import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.Vector;
-import junit.framework.Assert;
import junit.framework.Test;
import junit.framework.TestSuite;
import javax.persistence.Query;
import javax.persistence.EntityManager;
+import org.eclipse.persistence.testing.models.jpa.advanced.Address;
+import org.eclipse.persistence.testing.models.jpa.advanced.Department;
import org.eclipse.persistence.testing.models.jpa.advanced.Employee;
+import org.eclipse.persistence.testing.models.jpa.advanced.Jigsaw;
+import org.eclipse.persistence.testing.models.jpa.advanced.JigsawPiece;
+import org.eclipse.persistence.testing.models.jpa.advanced.PhoneNumber;
import org.eclipse.persistence.testing.models.jpa.advanced.EmployeePopulator;
import org.eclipse.persistence.testing.framework.junit.JUnitTestCase;
import org.eclipse.persistence.sessions.DatabaseSession;
+import org.eclipse.persistence.config.CacheUsage;
+import org.eclipse.persistence.config.QueryHints;
import org.eclipse.persistence.internal.sessions.AbstractSession;
import org.eclipse.persistence.testing.models.jpa.advanced.AdvancedTableCreator;
+import org.eclipse.persistence.testing.models.relationshipmaintenance.Dept;
/**
* <p>
* <b>Purpose</b>: Test EJBQL parameter functionality.
@@ -80,6 +87,10 @@
suite.addTest(new JUnitJPQLParameterTestSuite("testSetup"));
suite.addTest(new JUnitJPQLParameterTestSuite("multipleParameterTest"));
suite.addTest(new JUnitJPQLParameterTestSuite("updateEnumParameter"));
+ suite.addTest(new JUnitJPQLParameterTestSuite("testQueryParametersCheckCacheTest"));
+ suite.addTest(new JUnitJPQLParameterTestSuite("testQueryParametersDontCheckCacheTest"));
+ suite.addTest(new JUnitJPQLParameterTestSuite("testQueryParametersReversedCheckCacheTest"));
+ suite.addTest(new JUnitJPQLParameterTestSuite("testQueryParametersReversedDontCheckCacheTest"));
if (! JUnitTestCase.isJPA10()) {
suite.addTest(new JUnitJPQLParameterTestSuite("emptyParametersForNonParameterizedNamedQueryTest"));
}
@@ -126,7 +137,7 @@
query.setParameter(3, employee.getId());
List result = query.getResultList();
- Assert.assertTrue("Multiple Parameter Test Case Failed", comparer.compareObjects(result, expectedResult));
+ assertTrue("Multiple Parameter Test Case Failed", comparer.compareObjects(result, expectedResult));
}
// Test for GF#1123 - UPDATE with JPQL does not handle enums correctly.
@@ -184,4 +195,67 @@
assertEquals("Parameters size should be 0", 0, parameters.size());
}
+ public void testQueryParametersCheckCacheTest() {
+ testQueryParametersWithQueryAndCheckCache("SELECT p FROM JigsawPiece p, Jigsaw j WHERE j.id=:jigsawId AND p.id=:jigsawPieceId", true);
+ }
+
+ public void testQueryParametersDontCheckCacheTest() {
+ testQueryParametersWithQueryAndCheckCache("SELECT p FROM JigsawPiece p, Jigsaw j WHERE j.id=:jigsawId AND p.id=:jigsawPieceId", false);
+ }
+
+ public void testQueryParametersReversedCheckCacheTest() {
+ testQueryParametersWithQueryAndCheckCache("SELECT p FROM JigsawPiece p, Jigsaw j WHERE p.id=:jigsawPieceId AND j.id=:jigsawId", true);
+ }
+
+ public void testQueryParametersReversedDontCheckCacheTest() {
+ testQueryParametersWithQueryAndCheckCache("SELECT p FROM JigsawPiece p, Jigsaw j WHERE p.id=:jigsawPieceId AND j.id=:jigsawId", false);
+ }
+
+ // EL bug 430042 - pk query parameter binding incorrect
+ private void testQueryParametersWithQueryAndCheckCache(String jpqlQueryString, boolean checkCacheOnly) {
+ EntityManager em = createEntityManager();
+ Jigsaw jigsawTestData = null;
+ JigsawPiece jigsawPieceTestData = null;
+
+ try {
+ beginTransaction(em);
+
+ jigsawTestData = new Jigsaw();
+ jigsawPieceTestData = new JigsawPiece();
+ jigsawTestData.addPiece(jigsawPieceTestData);
+ em.persist(jigsawTestData);
+
+ commitTransaction(em);
+ closeEntityManager(em);
+
+ em = createEntityManager();
+
+ Query query = em.createQuery(jpqlQueryString);
+ query.setParameter("jigsawId", jigsawTestData.getId());
+ query.setParameter("jigsawPieceId", jigsawPieceTestData.getId());
+
+ if (checkCacheOnly) {
+ query.setHint(QueryHints.CACHE_USAGE, CacheUsage.CheckCacheOnly);
+ }
+
+ JigsawPiece resultPiece = (JigsawPiece) query.getSingleResult();
+ assertNotNull("Query Entity should not be null", resultPiece);
+ Jigsaw resultJigsaw = resultPiece.getJigsaw();
+ assertNotNull("Queried Entity's parent reference should not be null", resultJigsaw);
+ assertEquals("Queried Entity should have the same id", jigsawPieceTestData.getId(), resultPiece.getId());
+ assertEquals("Queried Entity should reference the same parent Entity", jigsawTestData.getId(), resultJigsaw.getId());
+ } finally {
+ if (jigsawTestData != null) {
+ beginTransaction(em);
+ Jigsaw jigsawToRemove = em.find(Jigsaw.class, jigsawTestData.getId());
+ em.remove(jigsawToRemove);
+ for (JigsawPiece piece : jigsawToRemove.getPieces()) {
+ em.remove(piece);
+ }
+ commitTransaction(em);
+ }
+ closeEntityManager(em);
+ }
+ }
+
}
