Skip to content

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

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

Merged
merged 3 commits into from
Aug 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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());
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
17 changes: 17 additions & 0 deletions src/site/markdown/docs/conditions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"))`
Expand Down
33 changes: 33 additions & 0 deletions src/test/java/examples/animal/data/AnimalDataTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand All @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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()) {
Expand Down