Merge pull request #229 from jeffgbutler/gh-228

Add Ability to Write "in" Conditions that will render even when the list of value is empty

Author: jeffgbutler

CHANGELOG.md

@@ -24,6 +24,7 @@ In the next major release of the library, all deprecated code will be removed.
 - Added the `applyOperator` function to make it easy to use non-standard database operators in expressions ([#220](https://github.com/mybatis/mybatis-dynamic-sql/issues/220))
 - Added convenience methods for count(column) and count(distinct column) ([#221](https://github.com/mybatis/mybatis-dynamic-sql/issues/221))
 - Added support for union queries in Kotlin ([#187](https://github.com/mybatis/mybatis-dynamic-sql/issues/187))
+- Added the ability to write "in" conditions that will render even if empty ([#228](https://github.com/mybatis/mybatis-dynamic-sql/issues/228))
 - Many enhancements for Spring including:
   - Fixed a bug where multi-row insert statements did not render properly for Spring  ([#224](https://github.com/mybatis/mybatis-dynamic-sql/issues/224))
   - Added support for a parameter type converter for use cases where the Java type of a column does not match the database column type  ([#131](https://github.com/mybatis/mybatis-dynamic-sql/issues/131))

src/main/java/org/mybatis/dynamic/sql/AbstractListValueCondition.java

@@ -1,5 +1,5 @@
 /**
- *    Copyright 2016-2019 the original author or authors.
+ *    Copyright 2016-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.
@@ -25,6 +25,7 @@
 public abstract class AbstractListValueCondition<T> implements VisitableCondition<T> {
     protected Collection<T> values;
     protected UnaryOperator<Stream<T>> valueStreamTransformer;
+    protected boolean skipRenderingWhenEmpty = true;
 
     protected AbstractListValueCondition(Collection<T> values) {
         this(values, UnaryOperator.identity());
@@ -39,6 +40,17 @@ public final <R> Stream<R> mapValues(Function<T, R> mapper) {
         return valueStreamTransformer.apply(values.stream()).map(mapper);
     }
 
+    public boolean skipRenderingWhenEmpty() {
+        return skipRenderingWhenEmpty;
+    }
+    
+    /**
+     * Use with caution - this could cause the library to render invalid SQL like "where column in ()".
+     */
+    protected void forceRenderingWhenEmpty() {
+        skipRenderingWhenEmpty = false;
+    }
+    
     @Override
     public <R> R accept(ConditionVisitor<T, R> visitor) {
         return visitor.visit(this);

src/main/java/org/mybatis/dynamic/sql/where/render/WhereConditionVisitor.java

@@ -55,7 +55,7 @@ public Optional<FragmentAndParameters> visit(AbstractListValueCondition<T> condi
         FragmentCollector fc = condition.mapValues(this::toFragmentAndParameters)
                 .collect(FragmentCollector.collect());
 
-        if (fc.isEmpty()) {
+        if (fc.isEmpty() && condition.skipRenderingWhenEmpty()) {
             return Optional.empty();
         }
 

src/site/markdown/docs/conditions.md

@@ -124,6 +124,23 @@ The library supplies several specializations of optional conditions to be used i
 ### Optionality with the "In" Conditions
 Optionality with the "in" and "not in" conditions is a bit more complex than the other types of conditions. The first thing to know is that no "in" or "not in" condition will render if the list of values is empty. For example, there will never be rendered SQL like `where name in ()`. So optionality of the "in" conditions is more about optionality of the *values* of the condition. The library comes with functions that will filter out null values, and will upper case String values to enable case insensitive queries. There are extension points to add additional filtering and mapping if you so desire.
 
+We think it is a good thing that the library will not render invalid SQL. Normally an "in" condition will be dropped from rendering if the list of values is empty - either through filtering or from the creation of the list. But there is some danger with this stance. Because the condition could be dropped from the rendered SQL, more rows could be impacted than expected if the list ends up empty for whatever reason. Our recommended solution is to make sure that you validate list values - especially if they are coming from direct user input. Another option is to force the conditions to render even if they are empty - which will cause a database error in most cases. If you want to force "in" conditions to render even if they are empty, you will need to create your own condition and configure it to render when empty. This is easily done by subclassing one of the existing conditions. For example:
+
+```java
+    public class IsInRequired<T> extends IsIn<T> {
+        protected IsInRequired(Collection<T> values) {
+            super(values);
+            forceRenderingWhenEmpty(); // calling this method will force the condition to render even if the values list is empty
+        }
+        
+        public static <T> IsInRequired<T> isIn(Collection<T> values) {
+            return new IsInRequired<>(values);
+        }
+    }
+```
+
+Note that we do not supply conditions like this as a part of the standard library because we believe that forcing the library to render invalid SQL is an extreme measure and should be undertaken with care.
+
 The following table shows the different supplied In conditions and how they will render for different sets of inputs. The table assumes the following types of input:
 
 - Example 1 assumes an input list of ("foo", null, "bar") - like `where(name, isIn("foo", null, "bar"))`

src/test/java/examples/animal/data/AnimalDataTest.java

@@ -17,6 +17,7 @@
 
 import static examples.animal.data.AnimalDataDynamicSqlSupport.*;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.within;
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.mybatis.dynamic.sql.SqlBuilder.*;
@@ -26,10 +27,13 @@
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
 import org.apache.ibatis.datasource.unpooled.UnpooledDataSource;
+import org.apache.ibatis.exceptions.PersistenceException;
 import org.apache.ibatis.jdbc.ScriptRunner;
 import org.apache.ibatis.mapping.Environment;
 import org.apache.ibatis.session.Configuration;
@@ -52,6 +56,7 @@
 import org.mybatis.dynamic.sql.select.render.SelectStatementProvider;
 import org.mybatis.dynamic.sql.update.render.UpdateStatementProvider;
 import org.mybatis.dynamic.sql.util.mybatis3.MyBatis3Utils;
+import org.mybatis.dynamic.sql.where.condition.IsIn;
 import org.mybatis.dynamic.sql.where.render.WhereClauseProvider;
 
 class AnimalDataTest {
@@ -564,6 +569,34 @@ void testInCondition() {
         }
     }
 
+    @Test
+    void testInConditionWithEmptyList() {
+        try (SqlSession sqlSession = sqlSessionFactory.openSession()) {
+            AnimalDataMapper mapper = sqlSession.getMapper(AnimalDataMapper.class);
+            
+            SelectStatementProvider selectStatement = select(id, animalName, bodyWeight, brainWeight)
+                    .from(animalData)
+                    .where(id, IsInRequired.isIn(Collections.emptyList()))
+                    .build()
+                    .render(RenderingStrategies.MYBATIS3);
+
+            assertThatExceptionOfType(PersistenceException.class).isThrownBy(() -> {
+                mapper.selectMany(selectStatement);
+            });
+        }
+    }
+
+    public static class IsInRequired<T> extends IsIn<T> {
+        protected IsInRequired(Collection<T> values) {
+            super(values);
+            forceRenderingWhenEmpty();
+        }
+        
+        public static <T> IsInRequired<T> isIn(Collection<T> values) {
+            return new IsInRequired<>(values);
+        }
+    }
+    
     @Test
     void testInCaseSensitiveCondition() {
         try (SqlSession sqlSession = sqlSessionFactory.openSession()) {