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 <aadamc...@apache.org> Authored: Thu Jan 22 10:37:55 2015 +0300 Committer: aadamchik <aadamc...@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) "
