From 584990a347367c0d63c5b831cceee579c8568911 Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sat, 17 Dec 2022 12:07:35 +0900 Subject: [PATCH] When processing DynamicSqlSource, evaluate param values in scripting phase - Evaluated param values are stored in `ParameterMapping` and later used in DefaultParameterHandler - There is no change when processing RawSqlSource - Removed unused `injectionFilter` from TextSqlNode (gh-117) This should fix #2754 . This might also fix #206 and #575 , but with this patch, it still is not possible to invoke a method with parameter inside a parameter reference like `#{_parameter.mymethod(_parameter.value)}`. It might be possible to accept OGNL expression in a param reference (e.g. `#{${_parameter.mymethod(_parameter.value)}}`), but I'm not sure if that's a good idea. --- .../builder/ParameterMappingTokenHandler.java | 154 ++++++++++++++++++ .../ibatis/builder/SqlSourceBuilder.java | 118 +------------- .../apache/ibatis/executor/BaseExecutor.java | 4 +- .../ibatis/mapping/ParameterMapping.java | 17 ++ .../defaults/DefaultParameterHandler.java | 30 ++-- .../scripting/defaults/RawSqlSource.java | 26 +-- .../scripting/xmltags/DynamicContext.java | 63 ++++++- .../scripting/xmltags/DynamicSqlSource.java | 7 +- .../scripting/xmltags/ForEachSqlNode.java | 60 +++---- .../scripting/xmltags/StaticTextSqlNode.java | 2 +- .../ibatis/scripting/xmltags/TextSqlNode.java | 28 +--- .../ibatis/scripting/xmltags/TrimSqlNode.java | 20 +-- .../ibatis/builder/SqlSourceBuilderTest.java | 29 +--- .../xml/dynamic/DynamicSqlSourceTest.java | 33 +++- .../raw_sql_source/RawSqlSourceTest.java | 43 +++++ 15 files changed, 383 insertions(+), 251 deletions(-) create mode 100644 src/main/java/org/apache/ibatis/builder/ParameterMappingTokenHandler.java diff --git a/src/main/java/org/apache/ibatis/builder/ParameterMappingTokenHandler.java b/src/main/java/org/apache/ibatis/builder/ParameterMappingTokenHandler.java new file mode 100644 index 00000000000..08724361b7b --- /dev/null +++ b/src/main/java/org/apache/ibatis/builder/ParameterMappingTokenHandler.java @@ -0,0 +1,154 @@ +/* + * Copyright 2009-2022 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 + * + * https://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.builder; + +import java.util.List; +import java.util.Map; + +import org.apache.ibatis.mapping.ParameterMapping; +import org.apache.ibatis.mapping.ParameterMode; +import org.apache.ibatis.parsing.TokenHandler; +import org.apache.ibatis.reflection.MetaClass; +import org.apache.ibatis.reflection.MetaObject; +import org.apache.ibatis.reflection.property.PropertyTokenizer; +import org.apache.ibatis.session.Configuration; +import org.apache.ibatis.type.JdbcType; + +public class ParameterMappingTokenHandler extends BaseBuilder implements TokenHandler { + + private static final String PARAMETER_PROPERTIES = "javaType,jdbcType,mode,numericScale,resultMap,typeHandler,jdbcTypeName"; + private final List parameterMappings; + private final Class parameterType; + private final MetaObject metaParameters; + private final Object parameterObject; + private final boolean paramExists; + + public ParameterMappingTokenHandler(List parameterMappings, Configuration configuration, + Object parameterObject, Class parameterType, Map additionalParameters, boolean paramExists) { + super(configuration); + this.parameterType = parameterObject == null + ? (parameterType == null ? Object.class : parameterType) + : parameterObject.getClass(); + this.metaParameters = configuration.newMetaObject(additionalParameters); + this.parameterObject = parameterObject; + this.paramExists = paramExists; + this.parameterMappings = parameterMappings; + } + + public ParameterMappingTokenHandler(List parameterMappings, Configuration configuration, + Class parameterType, + Map additionalParameters) { + super(configuration); + this.parameterType = parameterType; + this.metaParameters = configuration.newMetaObject(additionalParameters); + this.parameterObject = null; + this.paramExists = false; + this.parameterMappings = parameterMappings; + } + + public List getParameterMappings() { + return parameterMappings; + } + + @Override + public String handleToken(String content) { + parameterMappings.add(buildParameterMapping(content)); + return "?"; + } + + private ParameterMapping buildParameterMapping(String content) { + Map propertiesMap = parseParameterMapping(content); + String property = propertiesMap.get("property"); + PropertyTokenizer propertyTokenizer = new PropertyTokenizer(property); + Class propertyType; + if (metaParameters.hasGetter(propertyTokenizer.getName())) { // issue #448 get type from additional params + propertyType = metaParameters.getGetterType(property); + } else if (typeHandlerRegistry.hasTypeHandler(parameterType)) { + propertyType = parameterType; + } else if (JdbcType.CURSOR.name().equals(propertiesMap.get("jdbcType"))) { + propertyType = java.sql.ResultSet.class; + } else if (property == null || Map.class.isAssignableFrom(parameterType)) { + propertyType = Object.class; + } else { + MetaClass metaClass = MetaClass.forClass(parameterType, configuration.getReflectorFactory()); + if (metaClass.hasGetter(property)) { + propertyType = metaClass.getGetterType(property); + } else { + propertyType = Object.class; + } + } + ParameterMapping.Builder builder = new ParameterMapping.Builder(configuration, property, propertyType); + Class javaType = propertyType; + String typeHandlerAlias = null; + ParameterMode mode = null; + for (Map.Entry entry : propertiesMap.entrySet()) { + String name = entry.getKey(); + String value = entry.getValue(); + if ("javaType".equals(name)) { + javaType = resolveClass(value); + builder.javaType(javaType); + } else if ("jdbcType".equals(name)) { + builder.jdbcType(resolveJdbcType(value)); + } else if ("mode".equals(name)) { + mode = resolveParameterMode(value); + builder.mode(mode); + } else if ("numericScale".equals(name)) { + builder.numericScale(Integer.valueOf(value)); + } else if ("resultMap".equals(name)) { + builder.resultMapId(value); + } else if ("typeHandler".equals(name)) { + typeHandlerAlias = value; + } else if ("jdbcTypeName".equals(name)) { + builder.jdbcTypeName(value); + } else if ("property".equals(name)) { + // Do Nothing + } else if ("expression".equals(name)) { + throw new BuilderException("Expression based parameters are not supported yet"); + } else { + throw new BuilderException("An invalid property '" + name + "' was found in mapping #{" + content + + "}. Valid properties are " + PARAMETER_PROPERTIES); + } + } + if (typeHandlerAlias != null) { + builder.typeHandler(resolveTypeHandler(javaType, typeHandlerAlias)); + } + if (!ParameterMode.OUT.equals(mode) && paramExists) { + if (metaParameters.hasGetter(propertyTokenizer.getName())) { + builder.value(metaParameters.getValue(property)); + } else if (parameterObject == null) { + builder.value(null); + } else if (typeHandlerRegistry.hasTypeHandler(parameterObject.getClass())) { + builder.value(parameterObject); + } else { + MetaObject metaObject = configuration.newMetaObject(parameterObject); + builder.value(metaObject.getValue(property)); + } + } + return builder.build(); + } + + private Map parseParameterMapping(String content) { + try { + return new ParameterExpression(content); + } catch (BuilderException ex) { + throw ex; + } catch (Exception ex) { + throw new BuilderException("Parsing error was found in mapping #{" + content + + "}. Check syntax #{property|(expression), var1=value1, var2=value2, ...} ", ex); + } + } +} diff --git a/src/main/java/org/apache/ibatis/builder/SqlSourceBuilder.java b/src/main/java/org/apache/ibatis/builder/SqlSourceBuilder.java index 9c6b3fb2765..280447d6a5a 100644 --- a/src/main/java/org/apache/ibatis/builder/SqlSourceBuilder.java +++ b/src/main/java/org/apache/ibatis/builder/SqlSourceBuilder.java @@ -15,41 +15,27 @@ */ package org.apache.ibatis.builder; -import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.StringTokenizer; import org.apache.ibatis.mapping.ParameterMapping; import org.apache.ibatis.mapping.SqlSource; -import org.apache.ibatis.parsing.GenericTokenParser; -import org.apache.ibatis.parsing.TokenHandler; -import org.apache.ibatis.reflection.MetaClass; -import org.apache.ibatis.reflection.MetaObject; import org.apache.ibatis.session.Configuration; -import org.apache.ibatis.type.JdbcType; /** * @author Clinton Begin */ -public class SqlSourceBuilder extends BaseBuilder { +public class SqlSourceBuilder { - private static final String PARAMETER_PROPERTIES = "javaType,jdbcType,mode,numericScale,resultMap,typeHandler,jdbcTypeName"; - - public SqlSourceBuilder(Configuration configuration) { - super(configuration); + private SqlSourceBuilder() { + super(); } - public SqlSource parse(String originalSql, Class parameterType, Map additionalParameters) { - ParameterMappingTokenHandler handler = new ParameterMappingTokenHandler(configuration, parameterType, additionalParameters); - GenericTokenParser parser = new GenericTokenParser("#{", "}", handler); - String sql; - if (configuration.isShrinkWhitespacesInSql()) { - sql = parser.parse(removeExtraWhitespaces(originalSql)); - } else { - sql = parser.parse(originalSql); - } - return new StaticSqlSource(configuration, sql, handler.getParameterMappings()); + public static SqlSource buildSqlSource(Configuration configuration, String sql, + List parameterMappings) { + return new StaticSqlSource(configuration, + configuration.isShrinkWhitespacesInSql() ? SqlSourceBuilder.removeExtraWhitespaces(sql) : sql, + parameterMappings); } public static String removeExtraWhitespaces(String original) { @@ -66,92 +52,4 @@ public static String removeExtraWhitespaces(String original) { return builder.toString(); } - private static class ParameterMappingTokenHandler extends BaseBuilder implements TokenHandler { - - private final List parameterMappings = new ArrayList<>(); - private final Class parameterType; - private final MetaObject metaParameters; - - public ParameterMappingTokenHandler(Configuration configuration, Class parameterType, Map additionalParameters) { - super(configuration); - this.parameterType = parameterType; - this.metaParameters = configuration.newMetaObject(additionalParameters); - } - - public List getParameterMappings() { - return parameterMappings; - } - - @Override - public String handleToken(String content) { - parameterMappings.add(buildParameterMapping(content)); - return "?"; - } - - private ParameterMapping buildParameterMapping(String content) { - Map propertiesMap = parseParameterMapping(content); - String property = propertiesMap.get("property"); - Class propertyType; - if (metaParameters.hasGetter(property)) { // issue #448 get type from additional params - propertyType = metaParameters.getGetterType(property); - } else if (typeHandlerRegistry.hasTypeHandler(parameterType)) { - propertyType = parameterType; - } else if (JdbcType.CURSOR.name().equals(propertiesMap.get("jdbcType"))) { - propertyType = java.sql.ResultSet.class; - } else if (property == null || Map.class.isAssignableFrom(parameterType)) { - propertyType = Object.class; - } else { - MetaClass metaClass = MetaClass.forClass(parameterType, configuration.getReflectorFactory()); - if (metaClass.hasGetter(property)) { - propertyType = metaClass.getGetterType(property); - } else { - propertyType = Object.class; - } - } - ParameterMapping.Builder builder = new ParameterMapping.Builder(configuration, property, propertyType); - Class javaType = propertyType; - String typeHandlerAlias = null; - for (Map.Entry entry : propertiesMap.entrySet()) { - String name = entry.getKey(); - String value = entry.getValue(); - if ("javaType".equals(name)) { - javaType = resolveClass(value); - builder.javaType(javaType); - } else if ("jdbcType".equals(name)) { - builder.jdbcType(resolveJdbcType(value)); - } else if ("mode".equals(name)) { - builder.mode(resolveParameterMode(value)); - } else if ("numericScale".equals(name)) { - builder.numericScale(Integer.valueOf(value)); - } else if ("resultMap".equals(name)) { - builder.resultMapId(value); - } else if ("typeHandler".equals(name)) { - typeHandlerAlias = value; - } else if ("jdbcTypeName".equals(name)) { - builder.jdbcTypeName(value); - } else if ("property".equals(name)) { - // Do Nothing - } else if ("expression".equals(name)) { - throw new BuilderException("Expression based parameters are not supported yet"); - } else { - throw new BuilderException("An invalid property '" + name + "' was found in mapping #{" + content + "}. Valid properties are " + PARAMETER_PROPERTIES); - } - } - if (typeHandlerAlias != null) { - builder.typeHandler(resolveTypeHandler(javaType, typeHandlerAlias)); - } - return builder.build(); - } - - private Map parseParameterMapping(String content) { - try { - return new ParameterExpression(content); - } catch (BuilderException ex) { - throw ex; - } catch (Exception ex) { - throw new BuilderException("Parsing error was found in mapping #{" + content + "}. Check syntax #{property|(expression), var1=value1, var2=value2, ...} ", ex); - } - } - } - } diff --git a/src/main/java/org/apache/ibatis/executor/BaseExecutor.java b/src/main/java/org/apache/ibatis/executor/BaseExecutor.java index 834690abfd9..d2ae95c38f5 100644 --- a/src/main/java/org/apache/ibatis/executor/BaseExecutor.java +++ b/src/main/java/org/apache/ibatis/executor/BaseExecutor.java @@ -208,7 +208,9 @@ public CacheKey createCacheKey(MappedStatement ms, Object parameterObject, RowBo if (parameterMapping.getMode() != ParameterMode.OUT) { Object value; String propertyName = parameterMapping.getProperty(); - if (boundSql.hasAdditionalParameter(propertyName)) { + if (parameterMapping.hasValue()) { + value = parameterMapping.getValue(); + } else if (boundSql.hasAdditionalParameter(propertyName)) { value = boundSql.getAdditionalParameter(propertyName); } else if (parameterObject == null) { value = null; diff --git a/src/main/java/org/apache/ibatis/mapping/ParameterMapping.java b/src/main/java/org/apache/ibatis/mapping/ParameterMapping.java index 94abab3bbb4..612cbdc4286 100644 --- a/src/main/java/org/apache/ibatis/mapping/ParameterMapping.java +++ b/src/main/java/org/apache/ibatis/mapping/ParameterMapping.java @@ -38,6 +38,8 @@ public class ParameterMapping { private String resultMapId; private String jdbcTypeName; private String expression; + private Object value; + private boolean hasValue; private ParameterMapping() { } @@ -99,6 +101,12 @@ public Builder expression(String expression) { return this; } + public Builder value(Object value) { + parameterMapping.value = value; + parameterMapping.hasValue = true; + return this; + } + public ParameterMapping build() { resolveTypeHandler(); validate(); @@ -207,6 +215,14 @@ public String getExpression() { return expression; } + public Object getValue() { + return value; + } + + public boolean hasValue() { + return hasValue; + } + @Override public String toString() { final StringBuilder sb = new StringBuilder("ParameterMapping{"); @@ -220,6 +236,7 @@ public String toString() { sb.append(", resultMapId='").append(resultMapId).append('\''); sb.append(", jdbcTypeName='").append(jdbcTypeName).append('\''); sb.append(", expression='").append(expression).append('\''); + sb.append(", value='").append(value).append('\''); sb.append('}'); return sb.toString(); } diff --git a/src/main/java/org/apache/ibatis/scripting/defaults/DefaultParameterHandler.java b/src/main/java/org/apache/ibatis/scripting/defaults/DefaultParameterHandler.java index a9f5a91a0b8..f2256201422 100644 --- a/src/main/java/org/apache/ibatis/scripting/defaults/DefaultParameterHandler.java +++ b/src/main/java/org/apache/ibatis/scripting/defaults/DefaultParameterHandler.java @@ -66,18 +66,7 @@ public void setParameters(PreparedStatement ps) { for (int i = 0; i < parameterMappings.size(); i++) { ParameterMapping parameterMapping = parameterMappings.get(i); if (parameterMapping.getMode() != ParameterMode.OUT) { - Object value; - String propertyName = parameterMapping.getProperty(); - if (boundSql.hasAdditionalParameter(propertyName)) { // issue #448 ask first for additional params - value = boundSql.getAdditionalParameter(propertyName); - } else if (parameterObject == null) { - value = null; - } else if (typeHandlerRegistry.hasTypeHandler(parameterObject.getClass())) { - value = parameterObject; - } else { - MetaObject metaObject = configuration.newMetaObject(parameterObject); - value = metaObject.getValue(propertyName); - } + Object value = getValue(parameterMapping); TypeHandler typeHandler = parameterMapping.getTypeHandler(); JdbcType jdbcType = parameterMapping.getJdbcType(); if (value == null && jdbcType == null) { @@ -93,4 +82,21 @@ public void setParameters(PreparedStatement ps) { } } + private Object getValue(ParameterMapping parameterMapping) { + if (parameterMapping.hasValue()) { + return parameterMapping.getValue(); + } + String propertyName = parameterMapping.getProperty(); + if (boundSql.hasAdditionalParameter(propertyName)) { // issue #448 ask first for additional params + return boundSql.getAdditionalParameter(propertyName); + } else if (parameterObject == null) { + return null; + } else if (typeHandlerRegistry.hasTypeHandler(parameterObject.getClass())) { + return parameterObject; + } else { + MetaObject metaObject = configuration.newMetaObject(parameterObject); + return metaObject.getValue(propertyName); + } + } + } diff --git a/src/main/java/org/apache/ibatis/scripting/defaults/RawSqlSource.java b/src/main/java/org/apache/ibatis/scripting/defaults/RawSqlSource.java index 556712e5501..ce21535c2df 100644 --- a/src/main/java/org/apache/ibatis/scripting/defaults/RawSqlSource.java +++ b/src/main/java/org/apache/ibatis/scripting/defaults/RawSqlSource.java @@ -15,19 +15,23 @@ */ package org.apache.ibatis.scripting.defaults; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; +import org.apache.ibatis.builder.ParameterMappingTokenHandler; import org.apache.ibatis.builder.SqlSourceBuilder; import org.apache.ibatis.mapping.BoundSql; +import org.apache.ibatis.mapping.ParameterMapping; import org.apache.ibatis.mapping.SqlSource; +import org.apache.ibatis.parsing.GenericTokenParser; import org.apache.ibatis.scripting.xmltags.DynamicContext; import org.apache.ibatis.scripting.xmltags.DynamicSqlSource; import org.apache.ibatis.scripting.xmltags.SqlNode; import org.apache.ibatis.session.Configuration; /** - * Static SqlSource. It is faster than {@link DynamicSqlSource} because mappings are - * calculated during startup. + * Static SqlSource. It is faster than {@link DynamicSqlSource} because mappings are calculated during startup. * * @since 3.2.0 * @author Eduardo Macarron @@ -37,19 +41,19 @@ public class RawSqlSource implements SqlSource { private final SqlSource sqlSource; public RawSqlSource(Configuration configuration, SqlNode rootSqlNode, Class parameterType) { - this(configuration, getSql(configuration, rootSqlNode), parameterType); + DynamicContext context = new DynamicContext(configuration, parameterType); + rootSqlNode.apply(context); + String sql = context.getSql(); + sqlSource = SqlSourceBuilder.buildSqlSource(configuration, sql, context.getParameterMappings()); } public RawSqlSource(Configuration configuration, String sql, Class parameterType) { - SqlSourceBuilder sqlSourceParser = new SqlSourceBuilder(configuration); Class clazz = parameterType == null ? Object.class : parameterType; - sqlSource = sqlSourceParser.parse(sql, clazz, new HashMap<>()); - } - - private static String getSql(Configuration configuration, SqlNode rootSqlNode) { - DynamicContext context = new DynamicContext(configuration, null); - rootSqlNode.apply(context); - return context.getSql(); + List parameterMappings = new ArrayList<>(); + ParameterMappingTokenHandler tokenHandler = new ParameterMappingTokenHandler(parameterMappings, configuration, + clazz, new HashMap<>()); + GenericTokenParser parser = new GenericTokenParser("#{", "}", tokenHandler); + sqlSource = SqlSourceBuilder.buildSqlSource(configuration, parser.parse(sql), parameterMappings); } @Override diff --git a/src/main/java/org/apache/ibatis/scripting/xmltags/DynamicContext.java b/src/main/java/org/apache/ibatis/scripting/xmltags/DynamicContext.java index 43d84d96729..74cda88830f 100644 --- a/src/main/java/org/apache/ibatis/scripting/xmltags/DynamicContext.java +++ b/src/main/java/org/apache/ibatis/scripting/xmltags/DynamicContext.java @@ -15,17 +15,22 @@ */ package org.apache.ibatis.scripting.xmltags; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.StringJoiner; +import org.apache.ibatis.builder.ParameterMappingTokenHandler; +import org.apache.ibatis.mapping.ParameterMapping; +import org.apache.ibatis.parsing.GenericTokenParser; +import org.apache.ibatis.reflection.MetaObject; +import org.apache.ibatis.session.Configuration; + import ognl.OgnlContext; import ognl.OgnlRuntime; import ognl.PropertyAccessor; -import org.apache.ibatis.reflection.MetaObject; -import org.apache.ibatis.session.Configuration; - /** * @author Clinton Begin */ @@ -38,20 +43,36 @@ public class DynamicContext { OgnlRuntime.setPropertyAccessor(ContextMap.class, new ContextAccessor()); } - private final ContextMap bindings; + protected final ContextMap bindings; private final StringJoiner sqlBuilder = new StringJoiner(" "); private int uniqueNumber = 0; - public DynamicContext(Configuration configuration, Object parameterObject) { - if (parameterObject != null && !(parameterObject instanceof Map)) { + private final Configuration configuration; + private final Object parameterObject; + private final Class parameterType; + private final boolean paramExists; + + private GenericTokenParser tokenParser; + private ParameterMappingTokenHandler tokenHandler; + + public DynamicContext(Configuration configuration, Class parameterType) { + this(configuration, null, parameterType, false); + } + + public DynamicContext(Configuration configuration, Object parameterObject, Class parameterType, boolean paramExists) { + if (parameterObject == null || parameterObject instanceof Map) { + bindings = new ContextMap(null, false); + } else { MetaObject metaObject = configuration.newMetaObject(parameterObject); boolean existsTypeHandler = configuration.getTypeHandlerRegistry().hasTypeHandler(parameterObject.getClass()); bindings = new ContextMap(metaObject, existsTypeHandler); - } else { - bindings = new ContextMap(null, false); } bindings.put(PARAMETER_OBJECT_KEY, parameterObject); bindings.put(DATABASE_ID_KEY, configuration.getDatabaseId()); + this.configuration = configuration; + this.parameterObject = parameterObject; + this.paramExists = paramExists; + this.parameterType = parameterType; } public Map getBindings() { @@ -74,6 +95,30 @@ public int getUniqueNumber() { return uniqueNumber++; } + public List getParameterMappings() { + return tokenHandler == null ? new ArrayList<>() : tokenHandler.getParameterMappings(); + } + + protected String parseParam(String sql) { + if (tokenParser == null) { + tokenHandler = new ParameterMappingTokenHandler(getParameterMappings(), configuration, parameterObject, parameterType, bindings, paramExists); + tokenParser = new GenericTokenParser("#{", "}", tokenHandler); + } + return tokenParser.parse(sql); + } + + protected Object getParameterObject() { + return parameterObject; + } + + protected Class getParameterType() { + return parameterType; + } + + protected boolean isParamExists() { + return paramExists; + } + static class ContextMap extends HashMap { private static final long serialVersionUID = 2977601501966151582L; private final MetaObject parameterMetaObject; @@ -117,7 +162,7 @@ public Object getProperty(Map context, Object target, Object name) { Object parameterObject = map.get(PARAMETER_OBJECT_KEY); if (parameterObject instanceof Map) { - return ((Map)parameterObject).get(name); + return ((Map) parameterObject).get(name); } return null; diff --git a/src/main/java/org/apache/ibatis/scripting/xmltags/DynamicSqlSource.java b/src/main/java/org/apache/ibatis/scripting/xmltags/DynamicSqlSource.java index fce0bfa5396..03ab703b2b7 100644 --- a/src/main/java/org/apache/ibatis/scripting/xmltags/DynamicSqlSource.java +++ b/src/main/java/org/apache/ibatis/scripting/xmltags/DynamicSqlSource.java @@ -35,11 +35,10 @@ public DynamicSqlSource(Configuration configuration, SqlNode rootSqlNode) { @Override public BoundSql getBoundSql(Object parameterObject) { - DynamicContext context = new DynamicContext(configuration, parameterObject); + DynamicContext context = new DynamicContext(configuration, parameterObject, null, true); rootSqlNode.apply(context); - SqlSourceBuilder sqlSourceParser = new SqlSourceBuilder(configuration); - Class parameterType = parameterObject == null ? Object.class : parameterObject.getClass(); - SqlSource sqlSource = sqlSourceParser.parse(context.getSql(), parameterType, context.getBindings()); + String sql = context.getSql(); + SqlSource sqlSource = SqlSourceBuilder.buildSqlSource(configuration, sql, context.getParameterMappings()); BoundSql boundSql = sqlSource.getBoundSql(parameterObject); context.getBindings().forEach(boundSql::setAdditionalParameter); return boundSql; diff --git a/src/main/java/org/apache/ibatis/scripting/xmltags/ForEachSqlNode.java b/src/main/java/org/apache/ibatis/scripting/xmltags/ForEachSqlNode.java index 1c7ebbb5f1b..0bf0959d996 100644 --- a/src/main/java/org/apache/ibatis/scripting/xmltags/ForEachSqlNode.java +++ b/src/main/java/org/apache/ibatis/scripting/xmltags/ForEachSqlNode.java @@ -15,9 +15,11 @@ */ package org.apache.ibatis.scripting.xmltags; +import java.util.List; import java.util.Map; import java.util.Optional; +import org.apache.ibatis.mapping.ParameterMapping; import org.apache.ibatis.parsing.GenericTokenParser; import org.apache.ibatis.session.Configuration; @@ -74,33 +76,30 @@ public boolean apply(DynamicContext context) { applyOpen(context); int i = 0; for (Object o : iterable) { - DynamicContext oldContext = context; + DynamicContext scopedContext; if (first || separator == null) { - context = new PrefixedContext(context, ""); + scopedContext = new PrefixedContext(context, ""); } else { - context = new PrefixedContext(context, separator); + scopedContext = new PrefixedContext(context, separator); } - int uniqueNumber = context.getUniqueNumber(); + int uniqueNumber = scopedContext.getUniqueNumber(); // Issue #709 if (o instanceof Map.Entry) { @SuppressWarnings("unchecked") Map.Entry mapEntry = (Map.Entry) o; - applyIndex(context, mapEntry.getKey(), uniqueNumber); - applyItem(context, mapEntry.getValue(), uniqueNumber); + applyIndex(scopedContext, mapEntry.getKey(), uniqueNumber); + applyItem(scopedContext, mapEntry.getValue(), uniqueNumber); } else { - applyIndex(context, i, uniqueNumber); - applyItem(context, o, uniqueNumber); + applyIndex(scopedContext, i, uniqueNumber); + applyItem(scopedContext, o, uniqueNumber); } - contents.apply(new FilteredDynamicContext(configuration, context, index, item, uniqueNumber)); + contents.apply(new FilteredDynamicContext(configuration, scopedContext, index, item, uniqueNumber)); if (first) { - first = !((PrefixedContext) context).isPrefixApplied(); + first = !((PrefixedContext) scopedContext).isPrefixApplied(); } - context = oldContext; i++; } applyClose(context); - context.getBindings().remove(item); - context.getBindings().remove(index); return true; } @@ -141,21 +140,12 @@ private static class FilteredDynamicContext extends DynamicContext { private final String item; public FilteredDynamicContext(Configuration configuration,DynamicContext delegate, String itemIndex, String item, int i) { - super(configuration, null); + super(configuration, delegate.getParameterObject(), delegate.getParameterType(), delegate.isParamExists()); this.delegate = delegate; this.index = i; this.itemIndex = itemIndex; this.item = item; - } - - @Override - public Map getBindings() { - return delegate.getBindings(); - } - - @Override - public void bind(String name, Object value) { - delegate.bind(name, value); + this.bindings.putAll(delegate.getBindings()); } @Override @@ -181,6 +171,10 @@ public int getUniqueNumber() { return delegate.getUniqueNumber(); } + @Override + public List getParameterMappings() { + return delegate.getParameterMappings(); + } } @@ -190,26 +184,17 @@ private class PrefixedContext extends DynamicContext { private boolean prefixApplied; public PrefixedContext(DynamicContext delegate, String prefix) { - super(configuration, null); + super(configuration, delegate.getParameterObject(), delegate.getParameterType(), delegate.isParamExists()); this.delegate = delegate; this.prefix = prefix; this.prefixApplied = false; + this.bindings.putAll(delegate.getBindings()); } public boolean isPrefixApplied() { return prefixApplied; } - @Override - public Map getBindings() { - return delegate.getBindings(); - } - - @Override - public void bind(String name, Object value) { - delegate.bind(name, value); - } - @Override public void appendSql(String sql) { if (!prefixApplied && sql != null && sql.trim().length() > 0) { @@ -228,6 +213,11 @@ public String getSql() { public int getUniqueNumber() { return delegate.getUniqueNumber(); } + + @Override + public List getParameterMappings() { + return delegate.getParameterMappings(); + } } } diff --git a/src/main/java/org/apache/ibatis/scripting/xmltags/StaticTextSqlNode.java b/src/main/java/org/apache/ibatis/scripting/xmltags/StaticTextSqlNode.java index 9a938d5d0f2..e1e9f504350 100644 --- a/src/main/java/org/apache/ibatis/scripting/xmltags/StaticTextSqlNode.java +++ b/src/main/java/org/apache/ibatis/scripting/xmltags/StaticTextSqlNode.java @@ -27,7 +27,7 @@ public StaticTextSqlNode(String text) { @Override public boolean apply(DynamicContext context) { - context.appendSql(text); + context.appendSql(context.parseParam(text)); return true; } diff --git a/src/main/java/org/apache/ibatis/scripting/xmltags/TextSqlNode.java b/src/main/java/org/apache/ibatis/scripting/xmltags/TextSqlNode.java index bce1ab1e263..5bcc749223b 100644 --- a/src/main/java/org/apache/ibatis/scripting/xmltags/TextSqlNode.java +++ b/src/main/java/org/apache/ibatis/scripting/xmltags/TextSqlNode.java @@ -15,11 +15,8 @@ */ package org.apache.ibatis.scripting.xmltags; -import java.util.regex.Pattern; - import org.apache.ibatis.parsing.GenericTokenParser; import org.apache.ibatis.parsing.TokenHandler; -import org.apache.ibatis.scripting.ScriptingException; import org.apache.ibatis.type.SimpleTypeRegistry; /** @@ -27,15 +24,9 @@ */ public class TextSqlNode implements SqlNode { private final String text; - private final Pattern injectionFilter; public TextSqlNode(String text) { - this(text, null); - } - - public TextSqlNode(String text, Pattern injectionFilter) { this.text = text; - this.injectionFilter = injectionFilter; } public boolean isDynamic() { @@ -47,8 +38,8 @@ public boolean isDynamic() { @Override public boolean apply(DynamicContext context) { - GenericTokenParser parser = createParser(new BindingTokenParser(context, injectionFilter)); - context.appendSql(parser.parse(text)); + GenericTokenParser parser = createParser(new BindingTokenParser(context)); + context.appendSql(context.parseParam(parser.parse(text))); return true; } @@ -59,11 +50,9 @@ private GenericTokenParser createParser(TokenHandler handler) { private static class BindingTokenParser implements TokenHandler { private DynamicContext context; - private Pattern injectionFilter; - public BindingTokenParser(DynamicContext context, Pattern injectionFilter) { + public BindingTokenParser(DynamicContext context) { this.context = context; - this.injectionFilter = injectionFilter; } @Override @@ -75,15 +64,8 @@ public String handleToken(String content) { context.getBindings().put("value", parameter); } Object value = OgnlCache.getValue(content, context.getBindings()); - String srtValue = value == null ? "" : String.valueOf(value); // issue #274 return "" instead of "null" - checkInjection(srtValue); - return srtValue; - } - - private void checkInjection(String value) { - if (injectionFilter != null && !injectionFilter.matcher(value).matches()) { - throw new ScriptingException("Invalid input. Please conform to regex" + injectionFilter.pattern()); - } + // issue #274 return "" instead of "null" + return value == null ? "" : String.valueOf(value); } } diff --git a/src/main/java/org/apache/ibatis/scripting/xmltags/TrimSqlNode.java b/src/main/java/org/apache/ibatis/scripting/xmltags/TrimSqlNode.java index 7860214ee15..b6b720577c3 100644 --- a/src/main/java/org/apache/ibatis/scripting/xmltags/TrimSqlNode.java +++ b/src/main/java/org/apache/ibatis/scripting/xmltags/TrimSqlNode.java @@ -19,9 +19,9 @@ import java.util.Collections; import java.util.List; import java.util.Locale; -import java.util.Map; import java.util.StringTokenizer; +import org.apache.ibatis.mapping.ParameterMapping; import org.apache.ibatis.session.Configuration; /** @@ -76,11 +76,12 @@ private class FilteredDynamicContext extends DynamicContext { private StringBuilder sqlBuffer; public FilteredDynamicContext(DynamicContext delegate) { - super(configuration, null); + super(configuration, delegate.getParameterObject(), delegate.getParameterType(), delegate.isParamExists()); this.delegate = delegate; this.prefixApplied = false; this.suffixApplied = false; this.sqlBuffer = new StringBuilder(); + this.bindings.putAll(delegate.getBindings()); } public void applyAll() { @@ -93,16 +94,6 @@ public void applyAll() { delegate.appendSql(sqlBuffer.toString()); } - @Override - public Map getBindings() { - return delegate.getBindings(); - } - - @Override - public void bind(String name, Object value) { - delegate.bind(name, value); - } - @Override public int getUniqueNumber() { return delegate.getUniqueNumber(); @@ -118,6 +109,11 @@ public String getSql() { return delegate.getSql(); } + @Override + public List getParameterMappings() { + return delegate.getParameterMappings(); + } + private void applyPrefix(StringBuilder sql, String trimmedUppercaseSql) { if (!prefixApplied) { prefixApplied = true; diff --git a/src/test/java/org/apache/ibatis/builder/SqlSourceBuilderTest.java b/src/test/java/org/apache/ibatis/builder/SqlSourceBuilderTest.java index 871e2b88a00..12f48ce8b7e 100644 --- a/src/test/java/org/apache/ibatis/builder/SqlSourceBuilderTest.java +++ b/src/test/java/org/apache/ibatis/builder/SqlSourceBuilderTest.java @@ -15,41 +15,16 @@ */ package org.apache.ibatis.builder; -import org.apache.ibatis.mapping.BoundSql; -import org.apache.ibatis.mapping.SqlSource; -import org.apache.ibatis.session.Configuration; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class SqlSourceBuilderTest { +class SqlSourceBuilderTest { - private static Configuration configuration; - private static SqlSourceBuilder sqlSourceBuilder; private final String sqlFromXml = "\t\n\n SELECT * \n FROM user\n \t WHERE user_id = 1\n\t "; - @BeforeEach - void setUp() { - configuration = new Configuration(); - - sqlSourceBuilder = new SqlSourceBuilder(configuration); - } - - @Test - void testShrinkWhitespacesInSqlIsFalse() { - SqlSource sqlSource = sqlSourceBuilder.parse(sqlFromXml, null, null); - BoundSql boundSql = sqlSource.getBoundSql(null); - String actual = boundSql.getSql(); - Assertions.assertEquals(sqlFromXml, actual); - } - @Test void testShrinkWhitespacesInSqlIsTrue() { - configuration.setShrinkWhitespacesInSql(true); - SqlSource sqlSource = sqlSourceBuilder.parse(sqlFromXml, null, null); - BoundSql boundSql = sqlSource.getBoundSql(null); - String actual = boundSql.getSql(); - + String actual = SqlSourceBuilder.removeExtraWhitespaces(sqlFromXml); String shrankWhitespacesInSql = "SELECT * FROM user WHERE user_id = 1"; Assertions.assertEquals(shrankWhitespacesInSql, actual); } diff --git a/src/test/java/org/apache/ibatis/builder/xml/dynamic/DynamicSqlSourceTest.java b/src/test/java/org/apache/ibatis/builder/xml/dynamic/DynamicSqlSourceTest.java index c2c6033179a..a68e0ac5ff6 100644 --- a/src/test/java/org/apache/ibatis/builder/xml/dynamic/DynamicSqlSourceTest.java +++ b/src/test/java/org/apache/ibatis/builder/xml/dynamic/DynamicSqlSourceTest.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Stream; import org.apache.ibatis.BaseDataTest; import org.apache.ibatis.io.Resources; @@ -36,6 +37,7 @@ import org.apache.ibatis.scripting.xmltags.MixedSqlNode; import org.apache.ibatis.scripting.xmltags.SetSqlNode; import org.apache.ibatis.scripting.xmltags.SqlNode; +import org.apache.ibatis.scripting.xmltags.StaticTextSqlNode; import org.apache.ibatis.scripting.xmltags.TextSqlNode; import org.apache.ibatis.scripting.xmltags.WhereSqlNode; import org.apache.ibatis.session.Configuration; @@ -43,6 +45,9 @@ import org.apache.ibatis.session.SqlSessionFactoryBuilder; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; class DynamicSqlSourceTest extends BaseDataTest { @@ -324,9 +329,9 @@ void shouldIterateOnceForEachItemInCollection() throws Exception { BoundSql boundSql = source.getBoundSql(parameterObject); assertEquals(expected, boundSql.getSql()); assertEquals(3, boundSql.getParameterMappings().size()); - assertEquals("__frch_item_0", boundSql.getParameterMappings().get(0).getProperty()); - assertEquals("__frch_item_1", boundSql.getParameterMappings().get(1).getProperty()); - assertEquals("__frch_item_2", boundSql.getParameterMappings().get(2).getProperty()); + assertEquals("item", boundSql.getParameterMappings().get(0).getProperty()); + assertEquals("item", boundSql.getParameterMappings().get(1).getProperty()); + assertEquals("item", boundSql.getParameterMappings().get(2).getProperty()); } @Test @@ -371,9 +376,9 @@ void shouldPerformStrictMatchOnForEachVariableSubstitution() throws Exception { BoundSql boundSql = source.getBoundSql(param); assertEquals(4, boundSql.getParameterMappings().size()); assertEquals("uuu.u", boundSql.getParameterMappings().get(0).getProperty()); - assertEquals("__frch_u_0.id", boundSql.getParameterMappings().get(1).getProperty()); - assertEquals("__frch_u_0", boundSql.getParameterMappings().get(2).getProperty()); - assertEquals("__frch_u_0", boundSql.getParameterMappings().get(3).getProperty()); + assertEquals("u.id", boundSql.getParameterMappings().get(1).getProperty()); + assertEquals("u", boundSql.getParameterMappings().get(2).getProperty()); + assertEquals("u", boundSql.getParameterMappings().get(3).getProperty()); } private DynamicSqlSource createDynamicSqlSource(SqlNode... contents) throws IOException, SQLException { @@ -412,4 +417,20 @@ public void setId(String property) { } } + @MethodSource + @ParameterizedTest + void testShrinkWhitespacesInSql(SqlNode input, boolean shrinkWhitespaces, String expected) { + Configuration config = new Configuration(); + config.setShrinkWhitespacesInSql(shrinkWhitespaces); + String actual = new DynamicSqlSource(config, input).getBoundSql(null).getSql(); + assertEquals(expected, actual); + } + + static Stream testShrinkWhitespacesInSql() { + return Stream.of( + Arguments.arguments(new StaticTextSqlNode("\t\n\n SELECT * \n FROM user\n \t WHERE user_id = 1\n\t "), false, + "SELECT * \n FROM user\n \t WHERE user_id = 1"), + Arguments.arguments(new StaticTextSqlNode("\t\n\n SELECT * \n FROM user\n \t WHERE user_id = 1\n\t"), true, + "SELECT * FROM user WHERE user_id = 1")); + } } diff --git a/src/test/java/org/apache/ibatis/submitted/raw_sql_source/RawSqlSourceTest.java b/src/test/java/org/apache/ibatis/submitted/raw_sql_source/RawSqlSourceTest.java index 4e6572267ab..3a312013c74 100644 --- a/src/test/java/org/apache/ibatis/submitted/raw_sql_source/RawSqlSourceTest.java +++ b/src/test/java/org/apache/ibatis/submitted/raw_sql_source/RawSqlSourceTest.java @@ -15,19 +15,28 @@ */ package org.apache.ibatis.submitted.raw_sql_source; +import static org.junit.jupiter.api.Assertions.*; + import java.io.Reader; +import java.util.stream.Stream; import org.apache.ibatis.BaseDataTest; import org.apache.ibatis.io.Resources; import org.apache.ibatis.mapping.SqlSource; import org.apache.ibatis.scripting.defaults.RawSqlSource; import org.apache.ibatis.scripting.xmltags.DynamicSqlSource; +import org.apache.ibatis.scripting.xmltags.SqlNode; +import org.apache.ibatis.scripting.xmltags.StaticTextSqlNode; +import org.apache.ibatis.session.Configuration; import org.apache.ibatis.session.SqlSession; import org.apache.ibatis.session.SqlSessionFactory; import org.apache.ibatis.session.SqlSessionFactoryBuilder; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; class RawSqlSourceTest { @@ -72,4 +81,38 @@ private void test(String statement, Class sqlSource) { } } + @MethodSource + @ParameterizedTest + void testShrinkWhitespacesInSql(String input, boolean shrinkWhitespaces, String expected) { + Configuration config = new Configuration(); + config.setShrinkWhitespacesInSql(shrinkWhitespaces); + String actual = new RawSqlSource(config, input, null).getBoundSql(null).getSql(); + assertEquals(expected, actual); + } + + static Stream testShrinkWhitespacesInSql() { + return Stream.of( + Arguments.arguments("\t\n\n SELECT * \n FROM user\n \t WHERE user_id = 1\n\t ", false, + "\t\n\n SELECT * \n FROM user\n \t WHERE user_id = 1\n\t "), + Arguments.arguments("\t\n\n SELECT * \n FROM user\n \t WHERE user_id = 1\n\t", true, + "SELECT * FROM user WHERE user_id = 1")); + } + + @MethodSource + @ParameterizedTest + void testShrinkWhitespacesInSql_SqlNode(SqlNode input, boolean shrinkWhitespaces, String expected) { + Configuration config = new Configuration(); + config.setShrinkWhitespacesInSql(shrinkWhitespaces); + String actual = new RawSqlSource(config, input, null).getBoundSql(null).getSql(); + assertEquals(expected, actual); + } + + static Stream testShrinkWhitespacesInSql_SqlNode() { + return Stream.of( + Arguments.arguments( + new StaticTextSqlNode("\t\n\n SELECT * \n FROM user\n \t WHERE user_id = 1\n\t "), false, + "SELECT * \n FROM user\n \t WHERE user_id = 1"), + Arguments.arguments(new StaticTextSqlNode("\t\n\n SELECT * \n FROM user\n \t WHERE user_id = 1\n\t"), true, + "SELECT * FROM user WHERE user_id = 1")); + } }