You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cayenne.apache.org by aa...@apache.org on 2015/01/22 09:38:34 UTC

cayenne git commit: CAY-1966 SQLTemplate/SQLSelect positional parameter binding

Repository: cayenne
Updated Branches:
  refs/heads/master 8434e5f2d -> 3b63842ae


CAY-1966 SQLTemplate/SQLSelect positional parameter binding

* positional params break when wrapped in conditionals,
  like #chunk, etc. So changing the policy to treat only
  the *first* occasion of each named parameter as a position match


Project: http://git-wip-us.apache.org/repos/asf/cayenne/repo
Commit: http://git-wip-us.apache.org/repos/asf/cayenne/commit/3b63842a
Tree: http://git-wip-us.apache.org/repos/asf/cayenne/tree/3b63842a
Diff: http://git-wip-us.apache.org/repos/asf/cayenne/diff/3b63842a

Branch: refs/heads/master
Commit: 3b63842aee6e636828a7e917c0e1b1d19a9374bf
Parents: 8434e5f
Author: aadamchik <aa...@apache.org>
Authored: Thu Jan 22 10:37:55 2015 +0300
Committer: aadamchik <aa...@apache.org>
Committed: Thu Jan 22 11:38:25 2015 +0300

----------------------------------------------------------------------
 .../org/apache/cayenne/query/SQLSelect.java     | 26 ++++++++
 .../org/apache/cayenne/query/SQLTemplate.java   | 22 +++++--
 .../apache/cayenne/velocity/BindDirective.java  |  9 +--
 .../apache/cayenne/velocity/ChainDirective.java |  2 +-
 .../cayenne/velocity/VelocityParamSequence.java | 68 --------------------
 .../velocity/VelocitySQLTemplateProcessor.java  | 20 +++---
 .../org/apache/cayenne/query/SQLSelectIT.java   | 62 ++++++++++++++----
 .../org/apache/cayenne/query/SQLTemplateIT.java | 27 ++++----
 8 files changed, 115 insertions(+), 121 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cayenne/blob/3b63842a/cayenne-server/src/main/java/org/apache/cayenne/query/SQLSelect.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/query/SQLSelect.java b/cayenne-server/src/main/java/org/apache/cayenne/query/SQLSelect.java
index 754c4b6..aa157ba 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/query/SQLSelect.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/query/SQLSelect.java
@@ -194,10 +194,36 @@ public class SQLSelect<T> extends IndirectQuery implements Select<T> {
 		return this;
 	}
 
+	/**
+	 * Initializes positional parameters of the query. Parameters are bound in
+	 * the order they are found in the SQL template. If a given parameter name
+	 * is used more than once, only the first occurrence is treated as
+	 * "position", subsequent occurrences are bound with the same value as the
+	 * first one. If template parameters count is different from the array
+	 * parameter count, an exception will be thrown.
+	 * <p>
+	 * Note that calling this method will reset any previously set *named*
+	 * parameters.
+	 * 
+	 * @since 4.0
+	 */
 	public SQLSelect<T> paramsArray(Object... params) {
 		return paramsList(params != null ? Arrays.asList(params) : null);
 	}
 
+	/**
+	 * Initializes positional parameters of the query. Parameters are bound in
+	 * the order they are found in the SQL template. If a given parameter name
+	 * is used more than once, only the first occurrence is treated as
+	 * "position", subsequent occurrences are bound with the same value as the
+	 * first one. If template parameters count is different from the list
+	 * parameter count, an exception will be thrown.
+	 * <p>
+	 * Note that calling this method will reset any previously set *named*
+	 * parameters.
+	 * 
+	 * @since 4.0
+	 */
 	public SQLSelect<T> paramsList(List<Object> params) {
 		// since named parameters are specified, resetting positional
 		// parameters

http://git-wip-us.apache.org/repos/asf/cayenne/blob/3b63842a/cayenne-server/src/main/java/org/apache/cayenne/query/SQLTemplate.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/query/SQLTemplate.java b/cayenne-server/src/main/java/org/apache/cayenne/query/SQLTemplate.java
index 0241975..e569578 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/query/SQLTemplate.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/query/SQLTemplate.java
@@ -345,12 +345,12 @@ public class SQLTemplate extends AbstractQuery implements ParameterizedQuery, XM
 	}
 
 	/**
-	 * Initializes positional parameters of the query. This is a positional
-	 * style of binding. Names of variables in the expression are ignored and
-	 * parameters are bound in order they are found in the expression. E.g. if
-	 * the same name is mentioned twice, it can be bound to two different
-	 * values. If declared and provided parameters counts are mismatched, an
-	 * exception will be thrown.
+	 * Initializes positional parameters of the query. Parameters are bound in
+	 * the order they are found in the SQL template. If a given parameter name
+	 * is used more than once, only the first occurrence is treated as
+	 * "position", subsequent occurrences are bound with the same value as the
+	 * first one. If template parameters count is different from the array
+	 * parameter count, an exception will be thrown.
 	 * <p>
 	 * Note that calling this method will reset any previously set *named*
 	 * parameters.
@@ -362,6 +362,16 @@ public class SQLTemplate extends AbstractQuery implements ParameterizedQuery, XM
 	}
 
 	/**
+	 * Initializes positional parameters of the query. Parameters are bound in
+	 * the order they are found in the SQL template. If a given parameter name
+	 * is used more than once, only the first occurrence is treated as
+	 * "position", subsequent occurrences are bound with the same value as the
+	 * first one. If template parameters count is different from the list
+	 * parameter count, an exception will be thrown.
+	 * <p>
+	 * Note that calling this method will reset any previously set *named*
+	 * parameters.
+	 * 
 	 * @since 4.0
 	 */
 	public void setParamsList(List<Object> params) {

http://git-wip-us.apache.org/repos/asf/cayenne/blob/3b63842a/cayenne-server/src/main/java/org/apache/cayenne/velocity/BindDirective.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/velocity/BindDirective.java b/cayenne-server/src/main/java/org/apache/cayenne/velocity/BindDirective.java
index a2b50a2..43e807a 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/velocity/BindDirective.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/velocity/BindDirective.java
@@ -140,14 +140,7 @@ public class BindDirective extends Directive {
 	}
 
 	protected Object getChild(InternalContextAdapter context, Node node, int i) throws MethodInvocationException {
-		Object child = (i >= 0 && i < node.jjtGetNumChildren()) ? node.jjtGetChild(i).value(context) : null;
-
-		// unwrap postional parameters
-		if (child instanceof VelocityParamSequence) {
-			child = ((VelocityParamSequence) child).next();
-		}
-
-		return child;
+		return (i >= 0 && i < node.jjtGetNumChildren()) ? node.jjtGetChild(i).value(context) : null;
 	}
 
 	/**

http://git-wip-us.apache.org/repos/asf/cayenne/blob/3b63842a/cayenne-server/src/main/java/org/apache/cayenne/velocity/ChainDirective.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/velocity/ChainDirective.java b/cayenne-server/src/main/java/org/apache/cayenne/velocity/ChainDirective.java
index b0be445..acd0f8c 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/velocity/ChainDirective.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/velocity/ChainDirective.java
@@ -75,7 +75,7 @@ public class ChainDirective extends Directive {
         String join = (size > 1) ? (String) node.jjtGetChild(0).value(context) : "";
         String prefix = (size > 2) ? (String) node.jjtGetChild(1).value(context) : "";
 
-        // if there is a conditional prefix, use a separate buffer ofr children
+        // if there is a conditional prefix, use a separate buffer for children
         StringWriter childWriter = new StringWriter(30);
 
         int len = block.jjtGetNumChildren();

http://git-wip-us.apache.org/repos/asf/cayenne/blob/3b63842a/cayenne-server/src/main/java/org/apache/cayenne/velocity/VelocityParamSequence.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/velocity/VelocityParamSequence.java b/cayenne-server/src/main/java/org/apache/cayenne/velocity/VelocityParamSequence.java
deleted file mode 100644
index 089e7f4..0000000
--- a/cayenne-server/src/main/java/org/apache/cayenne/velocity/VelocityParamSequence.java
+++ /dev/null
@@ -1,68 +0,0 @@
-/*****************************************************************
- *   Licensed to the Apache Software Foundation (ASF) under one
- *  or more contributor license agreements.  See the NOTICE file
- *  distributed with this work for additional information
- *  regarding copyright ownership.  The ASF licenses this file
- *  to you under the Apache License, Version 2.0 (the
- *  "License"); you may not use this file except in compliance
- *  with the License.  You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- *  Unless required by applicable law or agreed to in writing,
- *  software distributed under the License is distributed on an
- *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- *  KIND, either express or implied.  See the License for the
- *  specific language governing permissions and limitations
- *  under the License.
- ****************************************************************/
-package org.apache.cayenne.velocity;
-
-import java.io.IOException;
-import java.io.Writer;
-import java.util.ArrayList;
-import java.util.List;
-
-import org.apache.velocity.context.InternalContextAdapter;
-import org.apache.velocity.exception.MethodInvocationException;
-import org.apache.velocity.exception.ParseErrorException;
-import org.apache.velocity.exception.ResourceNotFoundException;
-import org.apache.velocity.runtime.Renderable;
-
-/**
- * A parameter value container that helps to may a single velocity variable to a
- * sequence of positional parameters.
- * 
- * @since 4.0
- */
-class VelocityParamSequence implements Renderable {
-
-	private List<Object> parameters;
-	private int index;
-
-	VelocityParamSequence() {
-		this.parameters = new ArrayList<Object>();
-	}
-
-	void add(Object parameter) {
-		parameters.add(parameter);
-	}
-
-	Object next() {
-		return parameters.get(index++);
-	}
-
-	@Override
-	public boolean render(InternalContextAdapter context, Writer writer) throws IOException, MethodInvocationException,
-			ParseErrorException, ResourceNotFoundException {
-
-		// rewind the list regardless of whether we produce any output
-		Object next = next();
-
-		if (context.getAllowRendering()) {
-			writer.write(String.valueOf(next));
-		}
-		return true;
-	}
-
-}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/cayenne/blob/3b63842a/cayenne-server/src/main/java/org/apache/cayenne/velocity/VelocitySQLTemplateProcessor.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/velocity/VelocitySQLTemplateProcessor.java b/cayenne-server/src/main/java/org/apache/cayenne/velocity/VelocitySQLTemplateProcessor.java
index be5641d..6246821 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/velocity/VelocitySQLTemplateProcessor.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/velocity/VelocitySQLTemplateProcessor.java
@@ -64,19 +64,19 @@ public class VelocitySQLTemplateProcessor implements SQLTemplateProcessor {
 		@Override
 		public Object visit(ASTReference node, Object data) {
 
-			if (i >= positionalParams.size()) {
-				throw new ExpressionException("Too few parameters to bind template: " + positionalParams.size());
-			}
-
 			// strip off leading "$"
 			String paramName = node.getFirstToken().image.substring(1);
-			VelocityParamSequence sequence = (VelocityParamSequence) params.get(paramName);
-			if (sequence == null) {
-				sequence = new VelocityParamSequence();
-				params.put(paramName, sequence);
-			}
 
-			sequence.add(positionalParams.get(i++));
+			// only consider the first instance of each named parameter
+			if (!params.containsKey(paramName)) {
+
+				if (i >= positionalParams.size()) {
+					throw new ExpressionException("Too few parameters to bind template: " + positionalParams.size());
+				}
+
+				params.put(paramName, positionalParams.get(i));
+				i++;
+			}
 
 			return data;
 		}

http://git-wip-us.apache.org/repos/asf/cayenne/blob/3b63842a/cayenne-server/src/test/java/org/apache/cayenne/query/SQLSelectIT.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/test/java/org/apache/cayenne/query/SQLSelectIT.java b/cayenne-server/src/test/java/org/apache/cayenne/query/SQLSelectIT.java
index f0c3760..0ff7d34 100644
--- a/cayenne-server/src/test/java/org/apache/cayenne/query/SQLSelectIT.java
+++ b/cayenne-server/src/test/java/org/apache/cayenne/query/SQLSelectIT.java
@@ -18,6 +18,15 @@
  ****************************************************************/
 package org.apache.cayenne.query;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.sql.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 import org.apache.cayenne.DataRow;
 import org.apache.cayenne.access.DataContext;
 import org.apache.cayenne.di.Inject;
@@ -27,14 +36,9 @@ import org.apache.cayenne.testdo.testmap.Artist;
 import org.apache.cayenne.unit.di.server.CayenneProjects;
 import org.apache.cayenne.unit.di.server.ServerCase;
 import org.apache.cayenne.unit.di.server.UseServerRuntime;
+import org.junit.Before;
 import org.junit.Test;
 
-import java.util.List;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
 @UseServerRuntime(CayenneProjects.TESTMAP_PROJECT)
 public class SQLSelectIT extends ServerCase {
 
@@ -44,9 +48,14 @@ public class SQLSelectIT extends ServerCase {
 	@Inject
 	private DBHelper dbHelper;
 
+	private TableHelper tArtist;
+
+	@Before
+	public void before() {
+		tArtist = new TableHelper(dbHelper, "ARTIST").setColumns("ARTIST_ID", "ARTIST_NAME", "DATE_OF_BIRTH");
+	}
+
 	protected void createArtistsDataSet() throws Exception {
-		TableHelper tArtist = new TableHelper(dbHelper, "ARTIST");
-		tArtist.setColumns("ARTIST_ID", "ARTIST_NAME", "DATE_OF_BIRTH");
 
 		long dateBase = System.currentTimeMillis();
 
@@ -250,15 +259,44 @@ public class SQLSelectIT extends ServerCase {
 	@Test
 	public void test_ParamsArray_Multiple_OptionalChunks() throws Exception {
 
-		createArtistsDataSet();
+		Date dob = new java.sql.Date(System.currentTimeMillis());
+
+		tArtist.insert(1, "artist1", dob);
+		tArtist.insert(2, "artist2", null);
 
 		List<Long> ids = SQLSelect
 				.scalarQuery(
 						Long.class,
-						"SELECT ARTIST_ID FROM ARTIST #chain('OR' 'WHERE') #chunk($a) ARTIST_NAME = #bind($a) #end #chunk($b) ARTIST_NAME = #bind($b) #end #end ORDER BY ARTIST_ID")
-				.paramsArray(null, null, "artist2", "artist2").select(context);
+						"SELECT ARTIST_ID FROM ARTIST #chain('OR' 'WHERE') "
+								+ "#chunk($a) DATE_OF_BIRTH #bindEqual($a) #end "
+								+ "#chunk($b) ARTIST_NAME #bindEqual($b) #end #end ORDER BY ARTIST_ID")
+				.paramsArray(null, "artist1").select(context);
 
 		assertEquals(1, ids.size());
-		assertEquals(2l, ids.get(0).longValue());
+		assertEquals(1l, ids.get(0).longValue());
+	}
+
+	@Test
+	public void test_Params_Multiple_OptionalChunks() throws Exception {
+
+		Date dob = new java.sql.Date(System.currentTimeMillis());
+
+		tArtist.insert(1, "artist1", dob);
+		tArtist.insert(2, "artist2", null);
+
+		Map<String, Object> params = new HashMap<String, Object>();
+		params.put("a", null);
+		params.put("b", "artist1");
+
+		List<Long> ids = SQLSelect
+				.scalarQuery(
+						Long.class,
+						"SELECT ARTIST_ID FROM ARTIST #chain('OR' 'WHERE') "
+								+ "#chunk($a) DATE_OF_BIRTH #bindEqual($a) #end "
+								+ "#chunk($b) ARTIST_NAME #bindEqual($b) #end #end ORDER BY ARTIST_ID").params(params)
+				.select(context);
+
+		assertEquals(1, ids.size());
+		assertEquals(1l, ids.get(0).longValue());
 	}
 }

http://git-wip-us.apache.org/repos/asf/cayenne/blob/3b63842a/cayenne-server/src/test/java/org/apache/cayenne/query/SQLTemplateIT.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/test/java/org/apache/cayenne/query/SQLTemplateIT.java b/cayenne-server/src/test/java/org/apache/cayenne/query/SQLTemplateIT.java
index d152765..902ff04 100644
--- a/cayenne-server/src/test/java/org/apache/cayenne/query/SQLTemplateIT.java
+++ b/cayenne-server/src/test/java/org/apache/cayenne/query/SQLTemplateIT.java
@@ -59,7 +59,7 @@ public class SQLTemplateIT extends ServerCase {
 				Types.INTEGER, Types.BIGINT, Types.VARCHAR, Types.DECIMAL);
 	}
 
-    @Test
+	@Test
 	public void testSQLTemplateForDataMap() {
 		DataMap testDataMap = context.getEntityResolver().getDataMap("testmap");
 		SQLTemplate q1 = new SQLTemplate(testDataMap, "SELECT * FROM ARTIST", true);
@@ -67,7 +67,7 @@ public class SQLTemplateIT extends ServerCase {
 		assertEquals(0, result.size());
 	}
 
-    @Test
+	@Test
 	public void testSQLTemplateForDataMapWithInsert() {
 		DataMap testDataMap = context.getEntityResolver().getDataMap("testmap");
 		String sql = "INSERT INTO ARTIST VALUES (15, 'Surikov', null)";
@@ -79,7 +79,7 @@ public class SQLTemplateIT extends ServerCase {
 		assertEquals(1, result.size());
 	}
 
-    @Test
+	@Test
 	public void testSQLTemplateForDataMapWithInsertException() {
 		DataMap testDataMap = context.getEntityResolver().getDataMap("testmap");
 		String sql = "INSERT INTO ARTIST VALUES (15, 'Surikov', null)";
@@ -97,7 +97,7 @@ public class SQLTemplateIT extends ServerCase {
 				gotRuntimeException);
 	}
 
-    @Test
+	@Test
 	public void testSQLTemplate_PositionalParams() throws SQLException {
 
 		String sql = "INSERT INTO PAINTING (PAINTING_ID, PAINTING_TITLE, ESTIMATED_PRICE) "
@@ -112,39 +112,34 @@ public class SQLTemplateIT extends ServerCase {
 		assertEquals(10005.d, tPainting.getDouble("ESTIMATED_PRICE"), 0.001);
 	}
 
-    @Test
+	@Test
 	public void testSQLTemplate_PositionalParams_RepeatingVars() throws SQLException {
 
 		String sql = "INSERT INTO PAINTING (PAINTING_ID, PAINTING_TITLE, ESTIMATED_PRICE) "
 				+ "VALUES ($b, '$n', #bind($b 'INTEGER'))";
 
 		SQLTemplate q1 = new SQLTemplate(Painting.class, sql);
-		q1.setParamsArray(11, "The Fiddler", 4567);
+		q1.setParamsArray(11, "The Fiddler");
 		context.performNonSelectingQuery(q1);
 
 		assertEquals("The Fiddler", tPainting.getString("PAINTING_TITLE"));
 		assertEquals(11, tPainting.getInt("PAINTING_ID"));
-		assertEquals(4567.d, tPainting.getDouble("ESTIMATED_PRICE"), 0.001);
+		assertEquals(11.d, tPainting.getDouble("ESTIMATED_PRICE"), 0.001);
 	}
 
-    @Test
+	@Test(expected = CayenneRuntimeException.class)
 	public void testSQLTemplate_PositionalParams_ToFewParams() throws SQLException {
 
 		String sql = "INSERT INTO PAINTING (PAINTING_ID, PAINTING_TITLE, ESTIMATED_PRICE) "
-				+ "VALUES ($b, '$n', #bind($b 'INTEGER'))";
+				+ "VALUES ($b, '$n', #bind($c 'INTEGER'))";
 
 		SQLTemplate q1 = new SQLTemplate(Painting.class, sql);
 		q1.setParamsArray(11, "The Fiddler");
 
-		try {
-			context.performNonSelectingQuery(q1);
-			fail("Exception not thrown on parameter length mismatch");
-		} catch (CayenneRuntimeException e) {
-			// expected
-		}
+		context.performNonSelectingQuery(q1);
 	}
 
-    @Test
+	@Test
 	public void testSQLTemplate_PositionalParams_ToManyParams() throws SQLException {
 
 		String sql = "INSERT INTO PAINTING (PAINTING_ID, PAINTING_TITLE, ESTIMATED_PRICE) "