Author: ppoddar
Date: Fri Aug 15 12:38:10 2008
New Revision: 686349

URL: http://svn.apache.org/viewvc?rev=686349&view=rev
Log:
OPENJPA-111: Validate native SQL parameters by the numbers of parameters in 
StoreQuery level and bypass validation at facade layer

Modified:
    
openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java
    
openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties
    
openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
    
openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
    
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java
    
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java
    
openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java
    
openjpa/trunk/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties

Modified: 
openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java
URL: 
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java?rev=686349&r1=686348&r2=686349&view=diff
==============================================================================
--- 
openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java
 (original)
+++ 
openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java
 Fri Aug 15 12:38:10 2008
@@ -28,8 +28,10 @@
 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 @@
             }
         }
 
+        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 @@
         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;

Modified: 
openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties
URL: 
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties?rev=686349&r1=686348&r2=686349&view=diff
==============================================================================
--- 
openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties
 (original)
+++ 
openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties
 Fri Aug 15 12:38:10 2008
@@ -22,6 +22,8 @@
        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 \

Modified: 
openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
URL: 
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java?rev=686349&r1=686348&r2=686349&view=diff
==============================================================================
--- 
openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
 (original)
+++ 
openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
 Fri Aug 15 12:38:10 2008
@@ -908,7 +908,11 @@
             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));
         }

Modified: 
openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
URL: 
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties?rev=686349&r1=686348&r2=686349&view=diff
==============================================================================
--- 
openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
 (original)
+++ 
openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
 Fri Aug 15 12:38:10 2008
@@ -400,3 +400,6 @@
     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.

Modified: 
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java
URL: 
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java?rev=686349&r1=686348&r2=686349&view=diff
==============================================================================
--- 
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java
 (original)
+++ 
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java
 Fri Aug 15 12:38:10 2008
@@ -203,8 +203,8 @@
        }
        
        public void testPositionalParameterWithWrongType() {
-               String JPQL_NAMED  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND 
p.p3=?3";
-               Query q = em.createQuery(JPQL_NAMED);
+               String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND 
p.p3=?3";
+               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 void testNamedParameterWithNullValue() {
-               String JPQL_NAMED  = JPQL + "WHERE p.p1=:p1 AND p.p2=:p2 AND 
p.p3=:p3";
-               Query q = em.createQuery(JPQL_NAMED);
+               String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=:p1 AND p.p2=:p2 
AND p.p3=:p3";
+               Query q = em.createQuery(JPQL_POSITIONAL);
                q.setParameter("p1",  INT_VALUE);
                q.setParameter("p2",  null);
                q.setParameter("p3",  null);
@@ -223,8 +223,8 @@
        }
        
        public void testPositionalParameterWithNullValue() {
-               String JPQL_NAMED  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND 
p.p3=?3";
-               Query q = em.createQuery(JPQL_NAMED);
+               String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND 
p.p3=?3";
+               Query q = em.createQuery(JPQL_POSITIONAL);
                q.setParameter(1,  INT_VALUE);
                q.setParameter(2,  null);
                q.setParameter(3,  null);
@@ -232,10 +232,55 @@
                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

Modified: 
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java
URL: 
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java?rev=686349&r1=686348&r2=686349&view=diff
==============================================================================
--- 
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java
 (original)
+++ 
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java
 Fri Aug 15 12:38:10 2008
@@ -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
[EMAIL PROTECTED](name="JPQL_POSITIONAL", 
+               query="SELECT p FROM Binder p WHERE p.p1=?1 AND p.p2=?2 AND 
p.p3=?3")
[EMAIL PROTECTED](name="SQL_POSITIONAL",
+               query="SELECT id, p1 FROM Binder WHERE p1=?1 AND p2=?2 AND 
p3=?3")
 public class Binder {
        @Id
        @GeneratedValue

Modified: 
openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java
URL: 
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java?rev=686349&r1=686348&r2=686349&view=diff
==============================================================================
--- 
openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java
 (original)
+++ 
openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java
 Fri Aug 15 12:38:10 2008
@@ -262,6 +262,8 @@
         * 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 @@
         *    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 @@
                        }
                }
        }
+       
+       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);

Modified: 
openjpa/trunk/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties
URL: 
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties?rev=686349&r1=686348&r2=686349&view=diff
==============================================================================
--- 
openjpa/trunk/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties
 (original)
+++ 
openjpa/trunk/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties
 Fri Aug 15 12:38:10 2008
@@ -158,6 +158,6 @@
        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}".
        
\ No newline at end of file


Reply via email to