From d76c646acce071248efeeb3426013e31f89d0e14 Mon Sep 17 00:00:00 2001 From: Pinaki Poddar Date: Fri, 15 Aug 2008 19:38:10 +0000 Subject: [PATCH] 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/kernel/SQLStoreQuery.java | 20 ++++++- .../openjpa/jdbc/kernel/localizer.properties | 2 + .../org/apache/openjpa/kernel/QueryImpl.java | 6 +- .../openjpa/kernel/localizer.properties | 3 + .../jdbc/query/TestQueryParameterBinding.java | 59 ++++++++++++++++--- .../persistence/jdbc/query/domain/Binder.java | 11 ++-- .../apache/openjpa/persistence/QueryImpl.java | 19 ++++++ .../openjpa/persistence/localizer.properties | 2 +- 8 files changed, 107 insertions(+), 15 deletions(-) diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java index 81b2cb2e5..5356a01a8 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java +++ b/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; diff --git a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties index 5c918cd85..a35789a98 100644 --- a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties +++ b/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 \ diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java index b23f5b90b..3464d1fe4 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java +++ b/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)); } diff --git a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties index de70bf03d..4b0d508ae 100644 --- a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties +++ b/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. diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java index 97441334b..1f2dd72ad 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java +++ b/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"; - 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 class TestQueryParameterBinding extends SingleEMFTestCase { } 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 class TestQueryParameterBinding extends SingleEMFTestCase { } 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 @@ 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 diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java index 52b275aed..37fdb656d 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java +++ b/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 diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java index d49c1d656..a5fefe81b 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java +++ b/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 removeGaps(Map map) { + if (map == null || !map.containsValue(GAP_FILLER)) + return map; + List gaps = new ArrayList(); + 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); diff --git a/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties b/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties index 9ab0ffb5e..9427d7245 100644 --- a/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties +++ b/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}". \ No newline at end of file