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