From d1346e3543efec49aa063c54c81bf559678966f3 Mon Sep 17 00:00:00 2001 From: Kazuki Shimizu Date: Sun, 15 Mar 2020 02:34:23 +0900 Subject: [PATCH 1/3] Allow using actual argument name as bind parameter on a single collection Fixes gh-1237 --- .../ibatis/reflection/ParamNameResolver.java | 39 ++++- .../session/defaults/DefaultSqlSession.java | 21 +-- .../ActualParamNameTest.java | 148 ++++++++++++++++++ .../submitted/param_name_resolve/CreateDB.sql | 24 +++ .../param_name_resolve/mybatis-config.xml | 36 +++++ 5 files changed, 252 insertions(+), 16 deletions(-) create mode 100644 src/test/java/org/apache/ibatis/submitted/param_name_resolve/ActualParamNameTest.java create mode 100644 src/test/java/org/apache/ibatis/submitted/param_name_resolve/CreateDB.sql create mode 100644 src/test/java/org/apache/ibatis/submitted/param_name_resolve/mybatis-config.xml diff --git a/src/main/java/org/apache/ibatis/reflection/ParamNameResolver.java b/src/main/java/org/apache/ibatis/reflection/ParamNameResolver.java index 809ed6ff0dc..942a7351844 100644 --- a/src/main/java/org/apache/ibatis/reflection/ParamNameResolver.java +++ b/src/main/java/org/apache/ibatis/reflection/ParamNameResolver.java @@ -1,5 +1,5 @@ /** - * Copyright 2009-2019 the original author or authors. + * Copyright 2009-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,8 +17,11 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Method; +import java.util.Collection; import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.SortedMap; import java.util.TreeMap; @@ -48,6 +51,7 @@ public class ParamNameResolver { private final SortedMap names; private boolean hasParamAnnotation; + private boolean useActualParamName; public ParamNameResolver(Configuration config, Method method) { final Class[] paramTypes = method.getParameterTypes(); @@ -77,6 +81,8 @@ public ParamNameResolver(Configuration config, Method method) { // use the parameter index as the name ("0", "1", ...) // gcode issue #71 name = String.valueOf(map.size()); + } else { + useActualParamName = true; } } map.put(paramIndex, name); @@ -112,7 +118,8 @@ public Object getNamedParams(Object[] args) { if (args == null || paramCount == 0) { return null; } else if (!hasParamAnnotation && paramCount == 1) { - return args[names.firstKey()]; + Object value = args[names.firstKey()]; + return wrapToMapIfCollection(value, useActualParamName ? names.get(0) : null); } else { final Map param = new ParamMap<>(); int i = 0; @@ -129,4 +136,32 @@ public Object getNamedParams(Object[] args) { return param; } } + + /** + * Wrap to a {@link ParamMap} if object is {@link Collection} or array. + * + * @param object a parameter object + * @param actualParamName an actual parameter name + * (If specify a name, set an object to {@link ParamMap} with specified name) + * @return a {@link ParamMap} + * @since 3.5.5 + */ + public static Object wrapToMapIfCollection(Object object, String actualParamName) { + if (object instanceof Collection) { + ParamMap map = new ParamMap<>(); + map.put("collection", object); + if (object instanceof List) { + map.put("list", object); + } + Optional.ofNullable(actualParamName).ifPresent(name -> map.put(name, object)); + return map; + } else if (object != null && object.getClass().isArray()) { + ParamMap map = new ParamMap<>(); + map.put("array", object); + Optional.ofNullable(actualParamName).ifPresent(name -> map.put(name, object)); + return map; + } + return object; + } + } diff --git a/src/main/java/org/apache/ibatis/session/defaults/DefaultSqlSession.java b/src/main/java/org/apache/ibatis/session/defaults/DefaultSqlSession.java index 9cb9d4405fe..a1e30ac0043 100644 --- a/src/main/java/org/apache/ibatis/session/defaults/DefaultSqlSession.java +++ b/src/main/java/org/apache/ibatis/session/defaults/DefaultSqlSession.java @@ -1,5 +1,5 @@ /** - * Copyright 2009-2019 the original author or authors. + * Copyright 2009-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -34,6 +34,7 @@ import org.apache.ibatis.executor.result.DefaultMapResultHandler; import org.apache.ibatis.executor.result.DefaultResultContext; import org.apache.ibatis.mapping.MappedStatement; +import org.apache.ibatis.reflection.ParamNameResolver; import org.apache.ibatis.session.Configuration; import org.apache.ibatis.session.ResultHandler; import org.apache.ibatis.session.RowBounds; @@ -317,21 +318,13 @@ private boolean isCommitOrRollbackRequired(boolean force) { } private Object wrapCollection(final Object object) { - if (object instanceof Collection) { - StrictMap map = new StrictMap<>(); - map.put("collection", object); - if (object instanceof List) { - map.put("list", object); - } - return map; - } else if (object != null && object.getClass().isArray()) { - StrictMap map = new StrictMap<>(); - map.put("array", object); - return map; - } - return object; + return ParamNameResolver.wrapToMapIfCollection(object, null); } + /** + * @deprecated Since 3.5.5 + */ + @Deprecated public static class StrictMap extends HashMap { private static final long serialVersionUID = -5741767162221585340L; diff --git a/src/test/java/org/apache/ibatis/submitted/param_name_resolve/ActualParamNameTest.java b/src/test/java/org/apache/ibatis/submitted/param_name_resolve/ActualParamNameTest.java new file mode 100644 index 00000000000..2d0af704a4d --- /dev/null +++ b/src/test/java/org/apache/ibatis/submitted/param_name_resolve/ActualParamNameTest.java @@ -0,0 +1,148 @@ +/** + * Copyright 2009-2020 the original author or authors. + * + * Licensed 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.ibatis.submitted.param_name_resolve; + +import org.apache.ibatis.annotations.Select; +import org.apache.ibatis.io.Resources; +import org.apache.ibatis.jdbc.ScriptRunner; +import org.apache.ibatis.session.SqlSession; +import org.apache.ibatis.session.SqlSessionFactory; +import org.apache.ibatis.session.SqlSessionFactoryBuilder; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.io.Reader; +import java.sql.Connection; +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.assertEquals; + +class ActualParamNameTest { + + private static SqlSessionFactory sqlSessionFactory; + + @BeforeAll + static void setUp() throws Exception { + // create an SqlSessionFactory + try (Reader reader = Resources + .getResourceAsReader("org/apache/ibatis/submitted/param_name_resolve/mybatis-config.xml")) { + sqlSessionFactory = new SqlSessionFactoryBuilder().build(reader); + sqlSessionFactory.getConfiguration().addMapper(Mapper.class); + } + + // populate in-memory database + try (Connection conn = sqlSessionFactory.getConfiguration().getEnvironment().getDataSource().getConnection(); + Reader reader = Resources + .getResourceAsReader("org/apache/ibatis/submitted/param_name_resolve/CreateDB.sql")) { + ScriptRunner runner = new ScriptRunner(conn); + runner.setLogWriter(null); + runner.runScript(reader); + } + } + + @Test + void testSingleListParameterWhenUseActualParamNameIsTrue() { + try (SqlSession sqlSession = sqlSessionFactory.openSession()) { + Mapper mapper = sqlSession.getMapper(Mapper.class); + // use actual name + { + long count = mapper.getUserCountUsingList(Arrays.asList(1, 2)); + assertEquals(2, count); + } + // use 'collection' as alias + { + long count = mapper.getUserCountUsingListWithAliasIsCollection(Arrays.asList(1, 2)); + assertEquals(2, count); + } + // use 'list' as alias + { + long count = mapper.getUserCountUsingListWithAliasIsList(Arrays.asList(1, 2)); + assertEquals(2, count); + } + } + } + + @Test + void testSingleArrayParameterWhenUseActualParamNameIsTrue() { + try (SqlSession sqlSession = sqlSessionFactory.openSession()) { + Mapper mapper = sqlSession.getMapper(Mapper.class); + // use actual name + { + long count = mapper.getUserCountUsingArray(1, 2); + assertEquals(2, count); + } + // use 'array' as alias + { + long count = mapper.getUserCountUsingArrayWithAliasArray(1, 2); + assertEquals(2, count); + } + } + } + + interface Mapper { + @Select({ + "" + }) + Long getUserCountUsingList(List ids); + + @Select({ + "" + }) + Long getUserCountUsingListWithAliasIsCollection(List ids); + + @Select({ + "" + }) + Long getUserCountUsingListWithAliasIsList(List ids); + + @Select({ + "" + }) + Long getUserCountUsingArray(Integer... ids); + + @Select({ + "" + }) + Long getUserCountUsingArrayWithAliasArray(Integer... ids); + } + +} diff --git a/src/test/java/org/apache/ibatis/submitted/param_name_resolve/CreateDB.sql b/src/test/java/org/apache/ibatis/submitted/param_name_resolve/CreateDB.sql new file mode 100644 index 00000000000..abff5cf3b5a --- /dev/null +++ b/src/test/java/org/apache/ibatis/submitted/param_name_resolve/CreateDB.sql @@ -0,0 +1,24 @@ +-- +-- Copyright 2009-2020 the original author or authors. +-- +-- Licensed 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. +-- + +drop table users if exists; + +create table users ( + id int, + name varchar(20) +); + +insert into users (id, name) values (1, 'User1'), (2, 'User2'), (3, 'User3'); diff --git a/src/test/java/org/apache/ibatis/submitted/param_name_resolve/mybatis-config.xml b/src/test/java/org/apache/ibatis/submitted/param_name_resolve/mybatis-config.xml new file mode 100644 index 00000000000..107f2b9e370 --- /dev/null +++ b/src/test/java/org/apache/ibatis/submitted/param_name_resolve/mybatis-config.xml @@ -0,0 +1,36 @@ + + + + + + + + + + + + + + + + + + From 7d1191b992ed844550cfe9b1d98649ad9693083c Mon Sep 17 00:00:00 2001 From: Kazuki Shimizu Date: Fri, 20 Mar 2020 01:29:58 +0900 Subject: [PATCH 2/3] Add test for Configuration#useActualParamName is false Related with gh-1237 --- .../NoActualParamNameTest.java | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 src/test/java/org/apache/ibatis/submitted/param_name_resolve/NoActualParamNameTest.java diff --git a/src/test/java/org/apache/ibatis/submitted/param_name_resolve/NoActualParamNameTest.java b/src/test/java/org/apache/ibatis/submitted/param_name_resolve/NoActualParamNameTest.java new file mode 100644 index 00000000000..d9a3246ee32 --- /dev/null +++ b/src/test/java/org/apache/ibatis/submitted/param_name_resolve/NoActualParamNameTest.java @@ -0,0 +1,119 @@ +/** + * Copyright 2009-2020 the original author or authors. + * + * Licensed 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.ibatis.submitted.param_name_resolve; + +import org.apache.ibatis.annotations.Select; +import org.apache.ibatis.exceptions.PersistenceException; +import org.apache.ibatis.io.Resources; +import org.apache.ibatis.jdbc.ScriptRunner; +import org.apache.ibatis.session.SqlSession; +import org.apache.ibatis.session.SqlSessionFactory; +import org.apache.ibatis.session.SqlSessionFactoryBuilder; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.io.Reader; +import java.sql.Connection; +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +class NoActualParamNameTest { + + private static SqlSessionFactory sqlSessionFactory; + + @BeforeAll + static void setUp() throws Exception { + // create an SqlSessionFactory + try (Reader reader = Resources + .getResourceAsReader("org/apache/ibatis/submitted/param_name_resolve/mybatis-config.xml")) { + sqlSessionFactory = new SqlSessionFactoryBuilder().build(reader); + sqlSessionFactory.getConfiguration().addMapper(Mapper.class); + sqlSessionFactory.getConfiguration().setUseActualParamName(false); + } + + // populate in-memory database + try (Connection conn = sqlSessionFactory.getConfiguration().getEnvironment().getDataSource().getConnection(); + Reader reader = Resources + .getResourceAsReader("org/apache/ibatis/submitted/param_name_resolve/CreateDB.sql")) { + ScriptRunner runner = new ScriptRunner(conn); + runner.setLogWriter(null); + runner.runScript(reader); + } + } + + @Test + void testSingleListParameterWhenUseActualParamNameIsFalse() { + try (SqlSession sqlSession = sqlSessionFactory.openSession()) { + Mapper mapper = sqlSession.getMapper(Mapper.class); + // use actual name -> no available and index parameter("0") is not available too + { + try { + mapper.getUserCountUsingList(Arrays.asList(1, 2)); + fail(); + } catch (PersistenceException e) { + assertEquals("Parameter 'ids' not found. Available parameters are [collection, list]", e.getCause().getMessage()); + } + } + // use 'collection' as alias + { + long count = mapper.getUserCountUsingListWithAliasIsCollection(Arrays.asList(1, 2)); + assertEquals(2, count); + } + // use 'list' as alias + { + long count = mapper.getUserCountUsingListWithAliasIsList(Arrays.asList(1, 2)); + assertEquals(2, count); + } + } + } + + interface Mapper { + @Select({ + "" + }) + Long getUserCountUsingList(List ids); + + @Select({ + "" + }) + Long getUserCountUsingListWithAliasIsCollection(List ids); + + @Select({ + "" + }) + Long getUserCountUsingListWithAliasIsList(List ids); + + } + +} From ce14cec86659e7ea31e846aeb87d3733841a7450 Mon Sep 17 00:00:00 2001 From: Kazuki Shimizu Date: Sat, 21 Mar 2020 10:04:28 +0900 Subject: [PATCH 3/3] Polishing how to refers the useActualParamName on ParamNameResolver Related with gh-1237 --- .../org/apache/ibatis/reflection/ParamNameResolver.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/ibatis/reflection/ParamNameResolver.java b/src/main/java/org/apache/ibatis/reflection/ParamNameResolver.java index 942a7351844..87f5583ce44 100644 --- a/src/main/java/org/apache/ibatis/reflection/ParamNameResolver.java +++ b/src/main/java/org/apache/ibatis/reflection/ParamNameResolver.java @@ -35,6 +35,8 @@ public class ParamNameResolver { public static final String GENERIC_NAME_PREFIX = "param"; + private final boolean useActualParamName; + /** *

* The key is the index and the value is the name of the parameter.
@@ -51,9 +53,9 @@ public class ParamNameResolver { private final SortedMap names; private boolean hasParamAnnotation; - private boolean useActualParamName; public ParamNameResolver(Configuration config, Method method) { + this.useActualParamName = config.isUseActualParamName(); final Class[] paramTypes = method.getParameterTypes(); final Annotation[][] paramAnnotations = method.getParameterAnnotations(); final SortedMap map = new TreeMap<>(); @@ -74,15 +76,13 @@ public ParamNameResolver(Configuration config, Method method) { } if (name == null) { // @Param was not specified. - if (config.isUseActualParamName()) { + if (useActualParamName) { name = getActualParamName(method, paramIndex); } if (name == null) { // use the parameter index as the name ("0", "1", ...) // gcode issue #71 name = String.valueOf(map.size()); - } else { - useActualParamName = true; } } map.put(paramIndex, name);