OPENJPA-111: Validate native SQL parameters by the numbers of parameters in StoreQuery level and bypass validation at facade layer

git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@686349 13f79535-47bb-0310-9956-ffa450edef68

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java

@@ -28,8 +28,10 @@ import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Set;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.openjpa.jdbc.meta.ClassMapping;
@@ -118,13 +120,17 @@ public class SQLStoreQuery
             }
         }
 
+        int distinctParams = countDistinct(paramOrder);
+        if (params.size() < distinctParams)
+        	throw new UserException(_loc.get("sqlquery-fewer-params", 
+        		new Object[] {sql, distinctParams, params.size(), params}));
         // now go through the paramOrder list and re-order the params array
         List translated = new ArrayList();
         for (Iterator i = paramOrder.iterator(); i.hasNext();) {
             int index = ((Number) i.next()).intValue() - 1;
             if (index >= params.size())
                 throw new UserException(_loc.get("sqlquery-missing-params",
-                    sql, String.valueOf(index), params));
+                    sql, String.valueOf(index+1), params));
             translated.add(params.get(index));
         }
 
@@ -133,6 +139,18 @@ public class SQLStoreQuery
         params.addAll(translated);
         return buf.toString();
     }
+    
+    static int countDistinct(List list) {
+    	if (list == null || list.isEmpty())
+    		return 0;
+    	int distinct = 0;
+    	Set set = new HashSet();
+    	for (Object o : list) {
+    		if (set.add(o))
+    			distinct++;
+    	}
+    	return distinct;
+    }
 
     public boolean supportsParameterDeclarations() {
         return false;

openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties

@@ -22,6 +22,8 @@ mult-mapping-aggregate: Cannot perform an aggregate query on a hierarchy with \
 	unjoined subclasses: {0}
 sqlquery-missing-params: SQL query "{0}" declares a parameter index "{1}" for \
 	which no value was given.  The given parameters were: {2}
+sqlquery-fewer-params: SQL query "{0}" declares {1} distinct parameter(s), \
+	but only {2} parameters are given. Given parameter values are "{3}".  
 no-sql: You have not specified a SQL filter to execute in your SQL query.
 del-ins-cycle: An unresolvable constraint cycle was detected.  This typically \
 	means that you are persisting a new object with the same primary key value \

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java

@@ -908,7 +908,11 @@ public class QueryImpl
             else if (key instanceof Number) {
                 if (base == -1)
                     base = positionalParameterBase(params.keySet());
-                arr[((Number) key).intValue() - base] = entry.getValue();
+                int arrayIndex = ((Number) key).intValue() - base;
+                if (arrayIndex >= arr.length)
+                	throw new UserException(_loc.get("gap-query-param", 
+                		new Object[]{_query, key, params.size(), params}));
+                arr[arrayIndex] = entry.getValue();
             } else
                 throw new UserException(_loc.get("bad-param-name", key));
         }

openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties

@@ -400,3 +400,6 @@ cant-serialize-connected-broker: Serialization not allowed for brokers with \
     an active connection to the database.
 no-interface-metadata: No metadata was found for managed interface {0}.
 fetch-configuration-stack-empty: Fetch configuration stack is empty.
+gap-query-param: Parameter {1} for query "{0}" exceeds the number of {2} \
+	bound parameters with following values "{3}". This can happen if you have \
+	decalred but missed to bind values for one or more parameters.

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java

@@ -203,8 +203,8 @@ public class TestQueryParameterBinding extends SingleEMFTestCase {
 	}
 	
 	public void testPositionalParameterWithWrongType() {
-		String JPQL_NAMED  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3";
+		String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3";
-		Query q = em.createQuery(JPQL_NAMED);
+		Query q = em.createQuery(JPQL_POSITIONAL);
 		q.setParameter(1,  INT_VALUE);
 		q.setParameter(2,  DBL_VALUE);
 		q.setParameter(3,  STR_VALUE);
@@ -213,8 +213,8 @@ public class TestQueryParameterBinding extends SingleEMFTestCase {
 	}
 	
 	public void testNamedParameterWithNullValue() {
-		String JPQL_NAMED  = JPQL + "WHERE p.p1=:p1 AND p.p2=:p2 AND p.p3=:p3";
+		String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=:p1 AND p.p2=:p2 AND p.p3=:p3";
-		Query q = em.createQuery(JPQL_NAMED);
+		Query q = em.createQuery(JPQL_POSITIONAL);
 		q.setParameter("p1",  INT_VALUE);
 		q.setParameter("p2",  null);
 		q.setParameter("p3",  null);
@@ -223,8 +223,8 @@ public class TestQueryParameterBinding extends SingleEMFTestCase {
 	}
 	
 	public void testPositionalParameterWithNullValue() {
-		String JPQL_NAMED  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3";
+		String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3";
-		Query q = em.createQuery(JPQL_NAMED);
+		Query q = em.createQuery(JPQL_POSITIONAL);
 		q.setParameter(1,  INT_VALUE);
 		q.setParameter(2,  null);
 		q.setParameter(3,  null);
@@ -232,10 +232,55 @@ public class TestQueryParameterBinding extends SingleEMFTestCase {
 		fail(q);
 	}
 	
+	public void testPositionalParameterWithSingleResult() {
+		Query q = em.createNamedQuery("JPQL_POSITIONAL");
+		// "SELECT p FROM Binder p WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3"
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(2,  null);
+		q.setParameter(3,  null);
+		
+		fail(q, true);
+	}
+	
+	public void testPositionalParameterWithNativeQuery() {
+		Query q = em.createNamedQuery("SQL_POSITIONAL");
+		// "SELECT p.id FROM Binder WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3"
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(2,  STR_VALUE);
+		q.setParameter(3,  DBL_VALUE);
+		
+		assertEquals(1,q.getResultList().size());
+	}
+	
+	public void testPositionalParameterWithNativeQueryFails() {
+		Query q = em.createNamedQuery("SQL_POSITIONAL");
+		// "SELECT p.id FROM Binder WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3"
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(2,  STR_VALUE);
+		
+		fail(q);
+	}
+	
+	public void testPositionalParameterWithNativeQueryFailsWithGap() {
+		Query q = em.createNamedQuery("SQL_POSITIONAL");
+		// "SELECT p.id FROM Binder WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3"
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(3,  DBL_VALUE);
+		
+		fail(q);
+	}
+	
 	
 	void fail(Query q) {
+		fail(q, false);
+	}
+	
+	void fail(Query q, boolean single) {
 		try {
-			q.getResultList();
+			if (single) 
+				q.getSingleResult();
+			else 
+				q.getResultList();
 			fail("Expeceted " + ArgumentException.class.getName());
 		} catch (IllegalArgumentException ex) {
 		// good

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java

@@ -18,16 +18,17 @@
  */
 package org.apache.openjpa.persistence.jdbc.query.domain;
 
-import java.sql.Time;
-import java.sql.Timestamp;
-import java.util.Date;
-
-import javax.persistence.Column;
 import javax.persistence.Entity;
 import javax.persistence.GeneratedValue;
 import javax.persistence.Id;
+import javax.persistence.NamedNativeQuery;
+import javax.persistence.NamedQuery;
 
 @Entity
+@NamedQuery(name="JPQL_POSITIONAL", 
+		query="SELECT p FROM Binder p WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3")
+@NamedNativeQuery(name="SQL_POSITIONAL",
+		query="SELECT id, p1 FROM Binder WHERE p1=?1 AND p2=?2 AND p3=?3")
 public class Binder {
 	@Id
 	@GeneratedValue

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java

@@ -262,6 +262,8 @@ public class QueryImpl implements OpenJPAQuerySPI, Serializable {
 	 * Validate that the types of the parameters are correct.
 	 * The idea is to catch as many validation error as possible at the facade
 	 * layer itself.
+	 * For native SQL queries, however, parameter validation is bypassed as
+	 * we do not parse SQL.
 	 * 
 	 * The expected parameters are parsed from the query and in a LinkedMap 
 	 *	key   : name of the parameter as declared in query
@@ -294,6 +296,10 @@ public class QueryImpl implements OpenJPAQuerySPI, Serializable {
 	 *    f) parameter is primitive type but bound to null value
 	 */
 	private void validateParameters() {
+		if (isNative()) {
+			removeGaps(_positional);
+			return;
+		}
 		String query = getQueryString();
 		if (_positional != null) {
 			LinkedMap expected = _query.getParameterTypes();
@@ -400,6 +406,19 @@ public class QueryImpl implements OpenJPAQuerySPI, Serializable {
 			}
 		}
 	}
+	
+	Map<Integer, Object> removeGaps(Map<Integer, Object> map) {
+		if (map == null || !map.containsValue(GAP_FILLER))
+			return map;
+		List<Integer> gaps = new ArrayList<Integer>();
+		for (Integer key : map.keySet())
+			if (map.get(key) == GAP_FILLER)
+				gaps.add(key);
+		for (Integer gap : gaps) {
+			map.remove(gap);
+		}
+		return map;
+	}
 
 	void newValidationException(String msgKey, Object...args) {
 		throw new ArgumentException(_loc.get(msgKey, args), null, null, false);

openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties

@@ -158,6 +158,6 @@ param-extra: Parameter "{0}" is bound to "{1}" but is missing from the \
 	declared parameters "{2}".
 param-type-mismatch: Parameter "{0}" declared in "{1}" is set to value of \
 	"{2}" of type "{3}", but this parameter is bound to a field of type "{4}".
-param-type-mismatch: Parameter "{0}" declared in "{1}" is set to null, \
+param-type-null: Parameter "{0}" declared in "{1}" is set to null, \
 	but this parameter is bound to a field of primitive type "{2}".