modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/util/CriteriaQueryGenerator.java
| 70 +++++++---
modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/util/CriteriaQueryRunner.java
| 4
2 files changed, 51 insertions(+), 23 deletions(-)
New commits:
commit 078a2da4e488645284359940e3df7a61a0299109
Author: Lukas Krejci <lkrejci(a)redhat.com>
Date: Fri May 24 16:43:50 2013 +0200
[BZ 965833 - Potential bug in join/fetch/avoidance code]
It is now possible to lazily fetch fields defined in super classes.
The avoidance of the join-fetch with limits exposed a missing feature that
was present in the code from the day one. It would be failing if the
criteria query was used to fetch a custom object (i.e. not the primary
persistent class of the criteria object) using an "altered projection" and
a fetch would be set on a field defined in a super class of the persistent
class of the criteria object. Only by chance this has never happened
before, because we don't use the altered projections on the criteria
queries that often.
Now that we lazily fetch fields much more often (to avoid join fetch
with limits) this is a problem even in cases without an altered projection.
The fix is fortunately very simple - we need to also search the
superclasses of the criteria's persistent class when looking for a field
to lazily fetch.
The possibility of fields being present in super-classes was taken into
account on other places in the CriteriaQueryGenerator class, too.
Documentation on the getJoinFetchFields() and alterProjection() was
enhanced to hopefully better explain the conditions underwhich you can
combine fetching and altered projection together.
diff --git
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/util/CriteriaQueryGenerator.java
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/util/CriteriaQueryGenerator.java
index 867ea16..d1d2f73 100644
---
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/util/CriteriaQueryGenerator.java
+++
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/util/CriteriaQueryGenerator.java
@@ -343,7 +343,7 @@ public final class CriteriaQueryGenerator {
} else {
if (this.projection == null) {
/*
- * if not altering the projection, join fetching can be using
+ * if not altering the projection, join fetching can be used
* to retrieve the associated instance in the same SELECT
*
* We further avoid a JOIN FETCH when executing queries with
limits.
@@ -664,14 +664,9 @@ public final class CriteriaQueryGenerator {
}
private boolean isPersistentBag(String fieldName) {
- try {
- Class<?> persistentClass = criteria.getPersistentClass();
- Field field = persistentClass.getDeclaredField(fieldName);
+ Field field = findField(fieldName);
- return isAList(field) &&
!field.isAnnotationPresent(IndexColumn.class);
- } catch (NoSuchFieldException e) {
- return false;
- }
+ return field != null && isAList(field) &&
!field.isAnnotationPresent(IndexColumn.class);
}
private boolean isAList(Field field) {
@@ -690,23 +685,40 @@ public final class CriteriaQueryGenerator {
}
private void addPersistentBag(String fieldName) {
- try {
- Field field = criteria.getPersistentClass().getDeclaredField(fieldName);
- persistentBagFields.add(field);
- } catch (NoSuchFieldException e) {
- LOG.warn("Failed to add persistent bag collection on class [" +
criteria.getPersistentClass().getName() +"]: ", e);
+ Field f = findField(fieldName);
+ if (f == null) {
+ LOG.warn(
+ "Failed to add persistent bag collection [" + fieldName +
"] on class [" + criteria.getPersistentClass().getName() +
+ "]. There doesn't seem to be a field of that name on the
class or any of its superclasses.");
+ } else {
+ persistentBagFields.add(f);
}
}
private void addJoinFetch(String fieldName) {
- try {
- Field field = criteria.getPersistentClass().getDeclaredField(fieldName);
- joinFetchFields.add(field);
- } catch (NoSuchFieldException e) {
- LOG.warn("Failed to add join fetch field on class [" +
criteria.getPersistentClass().getName() + "]: ", e);
+ Field f = findField(fieldName);
+ if (f == null) {
+ LOG.warn(
+ "Failed to add join fetch field [" + fieldName + "] on
class [" + criteria.getPersistentClass().getName() +
+ "]. There doesn't seem to be a field of that name on the
class or any of its superclasses.");
+ } else {
+ joinFetchFields.add(f);
}
}
+ private Field findField(String fieldName) {
+ Class<?> cls = criteria.getPersistentClass();
+ while (cls != null) {
+ try {
+ return cls.getDeclaredField(fieldName);
+ } catch (NoSuchFieldException e) {
+ cls = cls.getSuperclass();
+ }
+ }
+
+ return null;
+ }
+
/**
* <strong>Note:</strong> This method should only be called after {@link
#getQueryString(boolean)}} because it is
* that method where the persistentBagFields property is initialized.
@@ -718,6 +730,21 @@ public final class CriteriaQueryGenerator {
return persistentBagFields;
}
+ /**
+ * <strong>Note:</strong> This method should only be called after {@link
#getQueryString(boolean)}} because it is
+ * that method where the persistentBagFields property is initialized.
+ * <p/>
+ * The elements of the returned list are a (sub)set of the fields that the criteria
object specified to be fetched
+ * (using the fetchXXX() methods). If the {@link CriteriaQueryRunner} is not set to
automatically fetch all the
+ * fields, you need to manually initialize these fields by for example using
+ * {@link CriteriaQueryRunner#initFetchFields(Object)} method on each of the
results.
+ *
+ * @see #alterProjection(String) <code>alterProjection(String)</code> for
special attention you need to make when
+ * mixing fetching fields and altered projection.
+ *
+ * @return Returns a list of fields from the persistent class to which the criteria
class corresponds. The fields in
+ * the list are fields specified by the criteria to be fetched (using the
fetchXXX() methods).
+ */
public List<Field> getJoinFetchFields() {
return joinFetchFields;
}
@@ -728,8 +755,11 @@ public final class CriteriaQueryGenerator {
* only affect the ResultSet for the data query, not the count query.
* <p/>
* If you are projecting a composite object that does not directly extend the entity
your Criteria object
- * represents, then you will need to manually initialize the persistent bags using
the methods exposed on
- * {@link CriteriaQueryRunner}
+ * represents, then you will need to manually initialize the persistent bags and
fetch fields using the
+ * {@link CriteriaQueryRunner#initFetchFields(Object)} method for each object in the
results (for which you need
+ * to instantiate the {@link CriteriaQueryRunner} with automatic fetching switched
OFF). <b>Note</b> that this will
+ * NOT work on the composite object itself. You need to pass an instance of the
entity class to the
+ *{@link CriteriaQueryRunner#initFetchFields(Object)} method.
*/
public void alterProjection(String projection) {
this.projection = projection;
diff --git
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/util/CriteriaQueryRunner.java
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/util/CriteriaQueryRunner.java
index a788fb0..73292be 100644
---
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/util/CriteriaQueryRunner.java
+++
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/util/CriteriaQueryRunner.java
@@ -129,9 +129,7 @@ public class CriteriaQueryRunner<T> {
public void initFetchFields(Object entity) {
initPersistentBags(entity);
- if (queryGenerator.isProjectionAltered()) {
- initJoinFetchFields(entity);
- }
+ initJoinFetchFields(entity);
}
private void initPersistentBags(Object entity) {
Show replies by date