Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using actual argument name as bind parameter on a single collection #1856

Merged
merged 3 commits into from Mar 21, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 37 additions & 2 deletions 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.
Expand All @@ -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;

Expand Down Expand Up @@ -48,6 +51,7 @@ public class ParamNameResolver {
private final SortedMap<Integer, String> names;

private boolean hasParamAnnotation;
private boolean useActualParamName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this variable necessary? No test fails even if I removed it.
If it is necessary for some corner case, there should be a test.

Sorry if I missed something obvious... 😩😪

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx! I will confirm my changes again at tomorrow or after.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is necessary for keeping the backward-compatibility when Configuration#useActualParamName is false. WDYT?

Copy link
Member

@harawata harawata Mar 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Thank you for the update!
It would be simpler to store the value of config.isUseActualParamName() in a final field. i.e.

private final boolean useActualParamName;

public ParamNameResolver(Configuration config, Method method) {
  useActualParamName = config.isUseActualParamName();
  ...

Or, keep the Configuration instance and reference the value directly.

return wrapToMapIfCollection(
  value, config.isUseActualParamName() ? names.get(0) : null);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right! I've applied a your comment via ce14cec.


public ParamNameResolver(Configuration config, Method method) {
final Class<?>[] paramTypes = method.getParameterTypes();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<String, Object> param = new ParamMap<>();
int i = 0;
Expand All @@ -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<Object> 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<Object> map = new ParamMap<>();
map.put("array", object);
Optional.ofNullable(actualParamName).ifPresent(name -> map.put(name, object));
return map;
}
return object;
}

}
@@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -317,21 +318,13 @@ private boolean isCommitOrRollbackRequired(boolean force) {
}

private Object wrapCollection(final Object object) {
if (object instanceof Collection) {
StrictMap<Object> 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<Object> 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<V> extends HashMap<String, V> {

private static final long serialVersionUID = -5741767162221585340L;
Expand Down
@@ -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({
"<script>",
" select count(*) from users u where u.id in",
" <foreach item='item' index='index' collection='ids' open='(' separator=',' close=')'>",
" #{item}",
" </foreach>",
"</script>"
})
Long getUserCountUsingList(List<Integer> ids);

@Select({
"<script>",
" select count(*) from users u where u.id in",
" <foreach item='item' index='index' collection='collection' open='(' separator=',' close=')'>",
" #{item}",
" </foreach>",
"</script>"
})
Long getUserCountUsingListWithAliasIsCollection(List<Integer> ids);

@Select({
"<script>",
" select count(*) from users u where u.id in",
" <foreach item='item' index='index' collection='list' open='(' separator=',' close=')'>",
" #{item}",
" </foreach>",
"</script>"
})
Long getUserCountUsingListWithAliasIsList(List<Integer> ids);

@Select({
"<script>",
" select count(*) from users u where u.id in",
" <foreach item='item' index='index' collection='ids' open='(' separator=',' close=')'>",
" #{item}",
" </foreach>",
"</script>"
})
Long getUserCountUsingArray(Integer... ids);

@Select({
"<script>",
" select count(*) from users u where u.id in",
" <foreach item='item' index='index' collection='array' open='(' separator=',' close=')'>",
" #{item}",
" </foreach>",
"</script>"
})
Long getUserCountUsingArrayWithAliasArray(Integer... ids);
}

}
@@ -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');
@@ -0,0 +1,36 @@
<?xml version="1.0" encoding="UTF-8" ?>
<!--

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.

-->
<!DOCTYPE configuration
PUBLIC "-//mybatis.org//DTD Config 3.0//EN"
"http://mybatis.org/dtd/mybatis-3-config.dtd">

<configuration>

<environments default="development">
<environment id="development">
<transactionManager type="JDBC" />
<dataSource type="UNPOOLED">
<property name="driver" value="org.hsqldb.jdbcDriver"/>
<property name="url" value="jdbc:hsqldb:mem:param_name_resolve"/>
<property name="username" value="sa"/>
</dataSource>
</environment>
</environments>

</configuration>