HHH-5424 HHH-5425 : Put ResultTransformer in QueryKey only if data is transformed; PropertyAccessException when caching 1 result per row

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@20078 1b8cb986-b30d-0410-93ca-fae66ebed9b2

core/src/main/java/org/hibernate/cache/QueryKey.java

@@ -73,6 +73,8 @@ public class QueryKey implements Serializable {
 	 * @param queryParameters The query parameters
 	 * @param filterKeys The keys of any enabled filters.
 	 * @param session The current session.
+	 * @param customTransformer The result transformer; should be
+	 *            null if data is not transformed before being cached.
 	 *
 	 * @return The generate query cache key.
 	 */
@@ -80,7 +82,8 @@ public class QueryKey implements Serializable {
 			String queryString,
 			QueryParameters queryParameters,
 			Set filterKeys,
-			SessionImplementor session) {
+			SessionImplementor session,
+			ResultTransformer customTransformer) {
 		// disassemble positional parameters
 		final int positionalParameterCount = queryParameters.getPositionalParameterTypes().length;
 		final Type[] types = new Type[positionalParameterCount];
@@ -134,7 +137,7 @@ public class QueryKey implements Serializable {
 				maxRows,
 				filterKeys,
 				session.getEntityMode(),
-				queryParameters.getResultTransformer()
+				customTransformer
 		);
 	}
 

core/src/main/java/org/hibernate/hql/HolderInstantiator.java

@@ -41,11 +41,14 @@ public final class HolderInstantiator {
 	private final String[] queryReturnAliases;
 	
 	public static HolderInstantiator getHolderInstantiator(ResultTransformer selectNewTransformer, ResultTransformer customTransformer, String[] queryReturnAliases) {
-		if(selectNewTransformer!=null) {
+		return new HolderInstantiator(
-			return new HolderInstantiator(selectNewTransformer, queryReturnAliases);
+				resolveResultTransformer( selectNewTransformer, customTransformer ),
-		} else {
+				queryReturnAliases
-			return new HolderInstantiator(customTransformer, queryReturnAliases);
+		);
 	}
+
+	public static ResultTransformer resolveResultTransformer(ResultTransformer selectNewTransformer, ResultTransformer customTransformer) {
+		return selectNewTransformer != null ? selectNewTransformer : customTransformer;
 	}	
 
 	public static ResultTransformer createSelectNewTransformer(Constructor constructor, boolean returnMaps, boolean returnLists) {
@@ -65,12 +68,13 @@ public final class HolderInstantiator {
 	
 	static public HolderInstantiator createClassicHolderInstantiator(Constructor constructor, 
 			ResultTransformer transformer) {
-		if ( constructor != null ) {
+		return new HolderInstantiator( resolveClassicResultTransformer( constructor, transformer ), null );
-			return new HolderInstantiator(new AliasToBeanConstructorResultTransformer(constructor), null);
-		}
-		else {
-			return new HolderInstantiator(transformer, null);
 	}
+
+	static public ResultTransformer resolveClassicResultTransformer(
+			Constructor constructor,
+			ResultTransformer transformer) {
+		return constructor != null ? new AliasToBeanConstructorResultTransformer( constructor ) : transformer;
 	}	
 
 	public HolderInstantiator( 

core/src/main/java/org/hibernate/hql/classic/QueryTranslatorImpl.java

@@ -983,6 +983,13 @@ public class QueryTranslatorImpl extends BasicLoader implements FilterTranslator
 		throw new UnsupportedOperationException( "Not supported!  Use the AST translator...");
 	}
 
+	protected ResultTransformer resolveResultTransformer(ResultTransformer resultTransformer) {
+		return HolderInstantiator.resolveClassicResultTransformer(
+				holderConstructor,
+				resultTransformer
+		);
+	}
+
 	protected Object getResultColumnOrRow(Object[] row, ResultTransformer transformer, ResultSet rs, SessionImplementor session)
 			throws SQLException, HibernateException {
 		row = toResultRow( row );

core/src/main/java/org/hibernate/loader/Loader.java

@@ -1008,10 +1008,30 @@ public abstract class Loader {
 				.endLoadingCollections( collectionPersister );
 	}
 
+	/**
+	 * Determine the actual ResultTransformer that will be used to
+	 * transform query results.
+	 *
+	 * @param resultTransformer the specified result transformer
+	 * @return the actual result transformer
+	 */
+	protected ResultTransformer resolveResultTransformer(ResultTransformer resultTransformer) {
+		return resultTransformer;
+	}
+	
 	protected List getResultList(List results, ResultTransformer resultTransformer) throws QueryException {
 		return results;
 	}
 
+	/**
+	 * Are rows transformed immediately after being read from the ResultSet?
+	 * @param transformer, the specified transformer
+	 * @return true, if getResultColumnOrRow() transforms the results; false, otherwise
+	 */
+	protected boolean areResultSetRowsTransformedImmediately(ResultTransformer transformer) {
+		return false;
+	}
+
 	/**
 	 * Get the actual object that is returned in the user-visible result list.
 	 * This empty implementation merely returns its first argument. This is
@@ -2272,7 +2292,11 @@ public abstract class Loader {
 				getSQLString(), 
 				queryParameters, 
 				filterKeys, 
-				session
+				session,
+				( areResultSetRowsTransformedImmediately( queryParameters.getResultTransformer() ) ?
+						queryParameters.getResultTransformer() :
+						null
+				)
 		);
 		
 		if ( querySpaces == null || querySpaces.size() == 0 ) {
@@ -2304,6 +2328,7 @@ public abstract class Loader {
 			);
 		}
 
+
 		return getResultList( result, queryParameters.getResultTransformer() );
 	}
 
@@ -2344,6 +2369,22 @@ public abstract class Loader {
 				persistenceContext.setDefaultReadOnly( defaultReadOnlyOrig );
 			}
 
+			// If there is a result transformer, but the loader is not expecting the data to be
+			// transformed yet, then the loader expects result elements that are Object[].
+			// The problem is that StandardQueryCache.get(...) does not return a tuple when
+			// resultTypes.length == 1. The following changes the data returned from the cache
+			// to be a tuple.
+			// TODO: this really doesn't belong here, but only Loader has the information
+			// to be able to do this.
+			if ( result != null &&
+					resultTypes.length == 1 &&
+					key.getResultTransformer() == null &&
+					resolveResultTransformer( queryParameters.getResultTransformer() ) != null ) {
+				for ( int i = 0 ; i < result.size() ; i++ ) {
+					result.set( i, new Object[] { result.get( i ) } );
+				}
+			}
+
 			if ( factory.getStatistics().isStatisticsEnabled() ) {
 				if ( result == null ) {
 					factory.getStatisticsImplementor()
@@ -2374,7 +2415,23 @@ public abstract class Loader {
 						result
 				);
 			}
-			boolean put = queryCache.put( key, resultTypes, result, queryParameters.isNaturalKeyLookup(), session );
+			// If there is a result transformer, but the data has not been transformed yet,
+			// then result elements are Object[]. The problem is that StandardQueryCache.put(...)
+			// does not expect a tuple when resultTypes.length == 1. The following changes the
+			// data being cached to what StandardQueryCache.put(...) expects.
+			// TODO: this really doesn't belong here, but only Loader has the information
+			// to be able to do this.
+			List cachedResult = result;
+			if ( resultTypes.length == 1 &&
+					key.getResultTransformer() == null &&
+					resolveResultTransformer( queryParameters.getResultTransformer() ) != null ) {
+				cachedResult = new ArrayList( result.size() );
+				for ( int i = 0 ; i < result.size() ; i++ ) {
+					cachedResult.add( ( ( Object[] ) result.get( i ) )[ 0 ] );
+				}
+			}
+
+			boolean put = queryCache.put( key, resultTypes, cachedResult, queryParameters.isNaturalKeyLookup(), session );
 			if ( put && factory.getStatistics().isStatisticsEnabled() ) {
 				factory.getStatisticsImplementor()
 						.queryCachePut( getQueryIdentifier(), queryCache.getRegion().getName() );
@@ -2382,7 +2439,6 @@ public abstract class Loader {
 		}
 	}
 
-
 	private void logCachedResultDetails(ResultTransformer resultTransformer, Type[] returnTypes, List result) {
 		if ( ! log.isTraceEnabled() ) {
 			return;

core/src/main/java/org/hibernate/loader/criteria/CriteriaLoader.java

@@ -120,6 +120,16 @@ public class CriteriaLoader extends OuterJoinLoader {
 
 	}
 
+	protected ResultTransformer resolveResultTransformer(ResultTransformer resultTransformer) {
+		return translator.getRootCriteria().getResultTransformer();
+	}
+
+	protected boolean areResultSetRowsTransformedImmediately( ResultTransformer transformer ) {
+		// comparing to null just in case there is no transformer
+		// (there should always be a result transformer; 
+		return resolveResultTransformer( transformer ) != null;
+	}
+
 	protected Object getResultColumnOrRow(Object[] row, ResultTransformer transformer, ResultSet rs, SessionImplementor session)
 	throws SQLException, HibernateException {
 		final Object[] result;
@@ -145,8 +155,7 @@ public class CriteriaLoader extends OuterJoinLoader {
 			result = row;
 			aliases = userAliases;
 		}
-		return translator.getRootCriteria().getResultTransformer()
+		return resolveResultTransformer( transformer ).transformTuple(result, aliases);
-				.transformTuple(result, aliases);
 	}
 
 	public Set getQuerySpaces() {
@@ -198,7 +207,7 @@ public class CriteriaLoader extends OuterJoinLoader {
 	}
 
 	protected List getResultList(List results, ResultTransformer resultTransformer) {
-		return translator.getRootCriteria().getResultTransformer().transformList( results );
+		return resolveResultTransformer( resultTransformer ).transformList( results );
 	}
 
 }

core/src/main/java/org/hibernate/loader/custom/CustomLoader.java

@@ -336,6 +336,10 @@ public class CustomLoader extends Loader {
 		}
 	}
 
+	protected ResultTransformer resolveResultTransformer(ResultTransformer resultTransformer) {
+		return HolderInstantiator.resolveResultTransformer( null, resultTransformer );
+	}
+	
 	protected Object getResultColumnOrRow(
 			Object[] row,
 	        ResultTransformer transformer,

core/src/main/java/org/hibernate/loader/hql/QueryLoader.java

@@ -382,6 +382,10 @@ public class QueryLoader extends BasicLoader {
 		return implicitResultTransformer != null;
 	}
 
+	protected ResultTransformer resolveResultTransformer(ResultTransformer resultTransformer) {
+		return HolderInstantiator.resolveResultTransformer( implicitResultTransformer, resultTransformer );
+	}
+	
 	protected Object getResultColumnOrRow(Object[] row, ResultTransformer transformer, ResultSet rs, SessionImplementor session)
 			throws SQLException, HibernateException {
 

testsuite/src/test/java/org/hibernate/test/querycache/HqlQueryCacheNormalResultTransformerTest.java

@@ -45,29 +45,4 @@ public class HqlQueryCacheNormalResultTransformerTest extends HqlQueryCachePutRe
 	protected CacheMode getQueryCacheMode() {
 		return CacheMode.NORMAL;
 	}
-
-	public void testAliasToBeanDtoMultiArgList() {
-		reportSkip( "Results from queries using Transformers.aliasToBean cannot be found in the cache due to bug in hashCode",
-				"Query using Transformers.aliasToBean with cache"
-		);
-	}
-	public void testAliasToBeanDtoMultiArgListFailureExpected() throws Exception {
-		super.testAliasToBeanDtoMultiArgList();
-	}
-
-	public void testAliasToBeanDtoLiteralArgList() {
-		reportSkip( "Results from queries using Transformers.aliasToBean cannot be found in the cache due to bug in hashCode",
-				"Query using Transformers.aliasToBean with cache" );
-	}
-	public void testAliasToBeanDtoLiteralArgListFailureExpected() throws Exception {
-		super.testAliasToBeanDtoLiteralArgList();
-	}
-
-	public void testAliasToBeanDtoWithNullAliasList() {
-		reportSkip( "Results from queries using Transformers.aliasToBean cannot be found in the cache due to bug in hashCode",
-				"Query using Transformers.aliasToBean with cache" );
-	}
-	public void testAliasToBeanDtoWithNullAliasListFailureExpected() throws Exception {
-		super.testAliasToBeanDtoWithNullAliasList();
-	}
 }

testsuite/src/test/java/org/hibernate/test/querycache/HqlQueryCachePutResultTransformerTest.java

@@ -50,29 +50,4 @@ public class HqlQueryCachePutResultTransformerTest extends HqlQueryCacheIgnoreRe
 	protected boolean areDynamicNonLazyAssociationsChecked() {
 		return false;
 	}
-
-	public void testAliasToEntityMapOneProjectionList() {
-		reportSkip( "HQL queries using a ResultTransformer are known to fail when caching a row with a single value",
-				"HQL queries using a ResultTransformer has row with a single value");
-	}
-	public void testAliasToEntityMapOneProjectionListFailureExpected() throws Exception {
-		super.testAliasToEntityMapOneProjectionList();
-	}
-
-	public void testAliasToBeanDtoOneArgList() {
-		reportSkip( "HQL queries using a ResultTransformer are known to fail when caching a row with a single value",
-				"HQL queries using a ResultTransformer has row with a single value");
-	}
-
-	public void testAliasToBeanDtoOneArgListFailureExpected() throws Exception {
-		super.testAliasToBeanDtoOneArgList();
-	}
-
-	public void testOneSelectNewList() {
-		reportSkip( "HQL queries using a ResultTransformer are known to fail when caching a row with a single value",
-				"HQL queries using a ResultTransformer has row with a single value");
-	}
-	public void testOneSelectNewListFailureExpected() throws Exception {
-		super.testOneSelectNewList();
-	}
 }

testsuite/src/test/java/org/hibernate/test/querycache/QueryCacheTest.java

@@ -224,7 +224,7 @@ public class QueryCacheTest extends FunctionalTestCase {
 		getSessions().evictQueries();
         getSessions().getStatistics().clear();
 
-		final String queryString = "select i.description from Item i where i.name='widget'";
+		final String queryString = "select i.description as desc from Item i where i.name='widget'";
 
 		Session s = openSession();
 		Transaction t = s.beginTransaction();
@@ -239,25 +239,35 @@ public class QueryCacheTest extends FunctionalTestCase {
         QueryStatistics qs = s.getSessionFactory().getStatistics().getQueryStatistics( queryString );
 		EntityStatistics es = s.getSessionFactory().getStatistics().getEntityStatistics( Item.class.getName() );
 
+		assertEquals( qs.getCacheHitCount(), 0 );
+		assertEquals( qs.getCacheMissCount(), 1 );
+		assertEquals( qs.getCachePutCount(), 1 );
+
 		Thread.sleep(200);
 
 		s = openSession();
 		t = s.beginTransaction();
 		List result = s.createQuery( queryString ).setCacheable(true).list();
 		assertEquals( result.size(), 1 );
+		assertEquals( i.getDescription(), ( ( String ) result.get( 0 ) ) );
 		t.commit();
 		s.close();
 
 		assertEquals( qs.getCacheHitCount(), 0 );
+		assertEquals( qs.getCacheMissCount(), 2 );
+		assertEquals( qs.getCachePutCount(), 2 );
 
 		s = openSession();
 		t = s.beginTransaction();
 		result = s.createQuery( queryString ).setCacheable(true).list();
 		assertEquals( result.size(), 1 );
+		assertEquals( i.getDescription(), result.get( 0 ) );
 		t.commit();
 		s.close();
 
 		assertEquals( qs.getCacheHitCount(), 1 );
+		assertEquals( qs.getCacheMissCount(), 2 );
+		assertEquals( qs.getCachePutCount(), 2 );
 
 		s = openSession();
 		t = s.beginTransaction();
@@ -265,10 +275,13 @@ public class QueryCacheTest extends FunctionalTestCase {
 		assertEquals( result.size(), 1 );
 		Map m = (Map) result.get(0);
 		assertEquals( 1, m.size() );
+		assertEquals( i.getDescription(), m.get( "desc" ) );
 		t.commit();
 		s.close();
 
-		assertEquals( "hit count should not go up since we are adding a resulttransformer", qs.getCacheHitCount(), 1 );
+		assertEquals( "hit count should go up since data is not transformed until after it is cached", qs.getCacheHitCount(), 2 );
+		assertEquals( qs.getCacheMissCount(), 2 );
+		assertEquals( qs.getCachePutCount(), 2 );
 
 		s = openSession();
 		t = s.beginTransaction();
@@ -276,10 +289,13 @@ public class QueryCacheTest extends FunctionalTestCase {
 		assertEquals( result.size(), 1 );
 		m = (Map) result.get(0);
 		assertEquals(1, m.size());
+		assertEquals( i.getDescription(), m.get( "desc" ) );
 		t.commit();
 		s.close();
 
-		assertEquals( "hit count should go up since we are using the same resulttransformer", qs.getCacheHitCount(), 2 );
+		assertEquals( "hit count should go up since data is not transformed until after it is cachedr", qs.getCacheHitCount(), 3 );
+		assertEquals( qs.getCacheMissCount(), 2 );
+		assertEquals( qs.getCachePutCount(), 2 );
 
 		s = openSession();
 		t = s.beginTransaction();
@@ -292,8 +308,9 @@ public class QueryCacheTest extends FunctionalTestCase {
 		t.commit();
 		s.close();
 
-		assertEquals( qs.getCacheHitCount(), 3 );
+		assertEquals( qs.getCacheHitCount(), 4 );
-		assertEquals( qs.getCacheMissCount(), 3 );
+		assertEquals( qs.getCacheMissCount(), 2 );
+		assertEquals( qs.getCachePutCount(), 2 );
 
 		Thread.sleep(200);
 
@@ -304,14 +321,18 @@ public class QueryCacheTest extends FunctionalTestCase {
 		i = (Item) s.get( Item.class, new Long(i.getId()) );
 		assertEquals( (String) result.get(0), "A middle-quality widget." );
 
+		assertEquals( qs.getCacheHitCount(), 4 );
+		assertEquals( qs.getCacheMissCount(), 3 );
+		assertEquals( qs.getCachePutCount(), 3 );
+
 		s.delete(i);
 		t.commit();
 		s.close();
 
-		assertEquals( qs.getCacheHitCount(), 3 );
+		assertEquals( qs.getCacheHitCount(), 4 );
-		assertEquals( qs.getCacheMissCount(), 4 );
+		assertEquals( qs.getCacheMissCount(), 3 );
-		assertEquals( qs.getCachePutCount(), 4 );
+		assertEquals( qs.getCachePutCount(), 3 );
-		assertEquals( qs.getExecutionCount(), 4 );
+		assertEquals( qs.getExecutionCount(), 3 );
 		assertEquals( es.getFetchCount(), 0 ); //check that it was being cached
 
 	}