From 972cee084ff0d72cb0dd1f266eab099523bcfb35 Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Wed, 26 Aug 2020 20:59:37 -0400 Subject: [PATCH 1/8] Remove "then" conditions on some In conditions The case insensitive in conditions are actually specializations of the regular in conditions and not intended to be extended. --- .../where/condition/IsInCaseInsensitive.java | 22 +---------- .../IsInCaseInsensitiveWhenPresent.java | 11 ++++-- .../condition/IsNotInCaseInsensitive.java | 22 +---------- .../IsNotInCaseInsensitiveWhenPresent.java | 10 +++-- ...onditionsWithPredicatesAnimalDataTest.java | 39 ------------------- 5 files changed, 19 insertions(+), 85 deletions(-) diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java index 622a6ea40..e011fd82d 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java @@ -25,12 +25,8 @@ public class IsInCaseInsensitive extends AbstractListValueCondition { - protected IsInCaseInsensitive(Collection values) { - super(values, s -> s.map(StringUtilities::safelyUpperCase)); - } - protected IsInCaseInsensitive(Collection values, UnaryOperator> valueStreamTransformer) { - super(values, StringUtilities.upperCaseAfter(valueStreamTransformer)); + super(values, valueStreamTransformer); } @Override @@ -39,21 +35,7 @@ public String renderCondition(String columnName, Stream placeholders) { placeholders.collect(Collectors.joining(",", "in (", ")")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ } - /** - * This method allows you to modify the condition's values before they are placed into the parameter map. - * For example, you could filter nulls, or trim strings, etc. This process will run before final rendering of SQL. - * If you filter values out of the stream, then final condition will not reference those values. If you filter all - * values out of the stream, then the condition will not render. - * - * @param valueStreamTransformer a UnaryOperator that will transform the value stream before - * the values are placed in the parameter map - * @return new condition with the specified transformer - */ - public IsInCaseInsensitive then(UnaryOperator> valueStreamTransformer) { - return new IsInCaseInsensitive(values, valueStreamTransformer); - } - public static IsInCaseInsensitive of(Collection values) { - return new IsInCaseInsensitive(values); + return new IsInCaseInsensitive(values, s -> s.map(StringUtilities::safelyUpperCase)); } } diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.java index 90c70c657..f55019cc6 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.java @@ -15,16 +15,21 @@ */ package org.mybatis.dynamic.sql.where.condition; +import org.mybatis.dynamic.sql.util.StringUtilities; + import java.util.Collection; import java.util.Objects; +import java.util.function.UnaryOperator; +import java.util.stream.Stream; public class IsInCaseInsensitiveWhenPresent extends IsInCaseInsensitive { - protected IsInCaseInsensitiveWhenPresent(Collection values) { - super(values, s -> s.filter(Objects::nonNull)); + protected IsInCaseInsensitiveWhenPresent(Collection values, UnaryOperator> valueStreamTransformer) { + super(values, valueStreamTransformer); } public static IsInCaseInsensitiveWhenPresent of(Collection values) { - return new IsInCaseInsensitiveWhenPresent(values); + return new IsInCaseInsensitiveWhenPresent(values, + s -> s.filter(Objects::nonNull).map(StringUtilities::safelyUpperCase)); } } diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitive.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitive.java index 6c6a40269..3a3bb16d1 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitive.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitive.java @@ -25,12 +25,8 @@ public class IsNotInCaseInsensitive extends AbstractListValueCondition { - protected IsNotInCaseInsensitive(Collection values) { - super(values, s -> s.map(StringUtilities::safelyUpperCase)); - } - protected IsNotInCaseInsensitive(Collection values, UnaryOperator> valueStreamTransformer) { - super(values, StringUtilities.upperCaseAfter(valueStreamTransformer)); + super(values, valueStreamTransformer); } @Override @@ -40,21 +36,7 @@ public String renderCondition(String columnName, Stream placeholders) { Collectors.joining(",", "not in (", ")")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ } - /** - * This method allows you to modify the condition's values before they are placed into the parameter map. - * For example, you could filter nulls, or trim strings, etc. This process will run before final rendering of SQL. - * If you filter values out of the stream, then final condition will not reference those values. If you filter all - * values out of the stream, then the condition will not render. - * - * @param valueStreamTransformer a UnaryOperator that will transform the value stream before - * the values are placed in the parameter map - * @return new condition with the specified transformer - */ - public IsNotInCaseInsensitive then(UnaryOperator> valueStreamTransformer) { - return new IsNotInCaseInsensitive(values, valueStreamTransformer); - } - public static IsNotInCaseInsensitive of(Collection values) { - return new IsNotInCaseInsensitive(values); + return new IsNotInCaseInsensitive(values, s -> s.map(StringUtilities::safelyUpperCase)); } } diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitiveWhenPresent.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitiveWhenPresent.java index 85fdf3f5f..35de1744e 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitiveWhenPresent.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitiveWhenPresent.java @@ -15,16 +15,20 @@ */ package org.mybatis.dynamic.sql.where.condition; +import org.mybatis.dynamic.sql.util.StringUtilities; + import java.util.Collection; import java.util.Objects; +import java.util.function.UnaryOperator; +import java.util.stream.Stream; public class IsNotInCaseInsensitiveWhenPresent extends IsNotInCaseInsensitive { - protected IsNotInCaseInsensitiveWhenPresent(Collection values) { - super(values, s -> s.filter(Objects::nonNull)); + protected IsNotInCaseInsensitiveWhenPresent(Collection values, UnaryOperator> valueStreamTransformer) { + super(values, valueStreamTransformer); } public static IsNotInCaseInsensitiveWhenPresent of(Collection values) { - return new IsNotInCaseInsensitiveWhenPresent(values); + return new IsNotInCaseInsensitiveWhenPresent(values, s -> s.filter(Objects::nonNull).map(StringUtilities::safelyUpperCase)); } } diff --git a/src/test/java/examples/animal/data/OptionalConditionsWithPredicatesAnimalDataTest.java b/src/test/java/examples/animal/data/OptionalConditionsWithPredicatesAnimalDataTest.java index 258a840a5..5658f4d77 100644 --- a/src/test/java/examples/animal/data/OptionalConditionsWithPredicatesAnimalDataTest.java +++ b/src/test/java/examples/animal/data/OptionalConditionsWithPredicatesAnimalDataTest.java @@ -514,25 +514,6 @@ void testValueStreamTransformerWithCustomCondition() { } } - @Test - void testIsInCaseInsensitiveWhenWithSomeValues() { - try (SqlSession sqlSession = sqlSessionFactory.openSession()) { - AnimalDataMapper mapper = sqlSession.getMapper(AnimalDataMapper.class); - SelectStatementProvider selectStatement = select(id, animalName, bodyWeight, brainWeight) - .from(animalData) - .where(animalName, isInCaseInsensitive("mouse", null, "musk shrew").then(s -> s.filter(Objects::nonNull))) - .orderBy(id) - .build() - .render(RenderingStrategies.MYBATIS3); - List animals = mapper.selectMany(selectStatement); - assertAll( - () -> assertThat(selectStatement.getSelectStatement()).isEqualTo("select id, animal_name, body_weight, brain_weight from AnimalData where upper(animal_name) in (#{parameters.p1,jdbcType=VARCHAR},#{parameters.p2,jdbcType=VARCHAR}) order by id"), - () -> assertThat(animals).hasSize(2), - () -> assertThat(animals.get(0).getId()).isEqualTo(4) - ); - } - } - @Test void testIsInCaseInsensitiveWhenWithNoValues() { try (SqlSession sqlSession = sqlSessionFactory.openSession()) { @@ -613,26 +594,6 @@ void testIsNotInCaseInsensitiveWhenWithValue() { } } - @Test - void testIsNotInCaseInsensitiveWhenWithSomeValues() { - try (SqlSession sqlSession = sqlSessionFactory.openSession()) { - AnimalDataMapper mapper = sqlSession.getMapper(AnimalDataMapper.class); - SelectStatementProvider selectStatement = select(id, animalName, bodyWeight, brainWeight) - .from(animalData) - .where(animalName, isNotInCaseInsensitive("mouse", null, "musk shrew").then(s -> s.filter(Objects::nonNull))) - .and(id, isLessThanOrEqualTo(10)) - .orderBy(id) - .build() - .render(RenderingStrategies.MYBATIS3); - List animals = mapper.selectMany(selectStatement); - assertAll( - () -> assertThat(selectStatement.getSelectStatement()).isEqualTo("select id, animal_name, body_weight, brain_weight from AnimalData where upper(animal_name) not in (#{parameters.p1,jdbcType=VARCHAR},#{parameters.p2,jdbcType=VARCHAR}) and id <= #{parameters.p3,jdbcType=INTEGER} order by id"), - () -> assertThat(animals).hasSize(8), - () -> assertThat(animals.get(0).getId()).isEqualTo(1) - ); - } - } - @Test void testIsNotInCaseInsensitiveWhenWithNoValues() { try (SqlSession sqlSession = sqlSessionFactory.openSession()) { From 129552dfb0548e1c14012aa865c037d97b96052b Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Thu, 27 Aug 2020 09:08:10 -0400 Subject: [PATCH 2/8] Progress on implementing empty callback --- .../sql/AbstractListValueCondition.java | 27 ++++++++------ .../org/mybatis/dynamic/sql/Callback.java | 19 ++++++++++ .../mybatis/dynamic/sql/ConditionVisitor.java | 2 +- .../dynamic/sql/where/condition/IsIn.java | 18 ++++++---- .../where/condition/IsInCaseInsensitive.java | 13 ++++++- .../IsInCaseInsensitiveWhenPresent.java | 18 +++++++--- .../where/render/WhereConditionVisitor.java | 2 +- .../examples/animal/data/AnimalDataTest.java | 35 +++++++------------ 8 files changed, 88 insertions(+), 46 deletions(-) create mode 100644 src/main/java/org/mybatis/dynamic/sql/Callback.java diff --git a/src/main/java/org/mybatis/dynamic/sql/AbstractListValueCondition.java b/src/main/java/org/mybatis/dynamic/sql/AbstractListValueCondition.java index 1b589e2dc..3f552d844 100644 --- a/src/main/java/org/mybatis/dynamic/sql/AbstractListValueCondition.java +++ b/src/main/java/org/mybatis/dynamic/sql/AbstractListValueCondition.java @@ -22,18 +22,24 @@ import java.util.function.UnaryOperator; import java.util.stream.Stream; -public abstract class AbstractListValueCondition implements VisitableCondition { +public abstract class AbstractListValueCondition> implements VisitableCondition { protected final Collection values; protected final UnaryOperator> valueStreamTransformer; - protected boolean renderWhenEmpty = false; + protected final Callback callback; protected AbstractListValueCondition(Collection values) { - this(values, UnaryOperator.identity()); + this(values, UnaryOperator.identity(), () -> {}); } protected AbstractListValueCondition(Collection values, UnaryOperator> valueStreamTransformer) { + this(values, valueStreamTransformer, () -> {}); + } + + protected AbstractListValueCondition(Collection values, UnaryOperator> valueStreamTransformer, + Callback callback) { this.values = new ArrayList<>(Objects.requireNonNull(values)); this.valueStreamTransformer = Objects.requireNonNull(valueStreamTransformer); + this.callback = Objects.requireNonNull(callback); } public final Stream mapValues(Function mapper) { @@ -42,20 +48,19 @@ public final Stream mapValues(Function mapper) { @Override public boolean shouldRender() { - return !values.isEmpty() || renderWhenEmpty; + if (values.isEmpty()) { + callback.call(); + return false; + } else { + return true; + } } - /** - * Use with caution - this could cause the library to render invalid SQL like "where column in ()". - */ - protected void forceRenderingWhenEmpty() { - renderWhenEmpty = true; - } - @Override public R accept(ConditionVisitor visitor) { return visitor.visit(this); } + public abstract S withListEmptyCallback(Callback callback); public abstract String renderCondition(String columnName, Stream placeholders); } diff --git a/src/main/java/org/mybatis/dynamic/sql/Callback.java b/src/main/java/org/mybatis/dynamic/sql/Callback.java new file mode 100644 index 000000000..15051cc60 --- /dev/null +++ b/src/main/java/org/mybatis/dynamic/sql/Callback.java @@ -0,0 +1,19 @@ +package org.mybatis.dynamic.sql; + +import java.util.function.Function; + +@FunctionalInterface +public interface Callback { + void call(); + + static Callback runtimeExceptionThrowingCallback(String message) { + return exceptionThrowingCallback(message, RuntimeException::new); + } + + static Callback exceptionThrowingCallback(String message, + Function exceptionBuilder) { + return () -> { + throw exceptionBuilder.apply(message); + }; + } +} diff --git a/src/main/java/org/mybatis/dynamic/sql/ConditionVisitor.java b/src/main/java/org/mybatis/dynamic/sql/ConditionVisitor.java index 8063bc88a..f800ea5c3 100644 --- a/src/main/java/org/mybatis/dynamic/sql/ConditionVisitor.java +++ b/src/main/java/org/mybatis/dynamic/sql/ConditionVisitor.java @@ -16,7 +16,7 @@ package org.mybatis.dynamic.sql; public interface ConditionVisitor { - R visit(AbstractListValueCondition condition); + R visit(AbstractListValueCondition condition); R visit(AbstractNoValueCondition condition); diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsIn.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsIn.java index df6c48bc3..dde0b382c 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsIn.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsIn.java @@ -23,17 +23,23 @@ import java.util.stream.Stream; import org.mybatis.dynamic.sql.AbstractListValueCondition; +import org.mybatis.dynamic.sql.Callback; -public class IsIn extends AbstractListValueCondition { - - protected IsIn(Collection values, UnaryOperator> valueStreamTransformer) { - super(values, valueStreamTransformer); - } +public class IsIn extends AbstractListValueCondition> { protected IsIn(Collection values) { super(values); } + protected IsIn(Collection values, UnaryOperator> valueStreamTransformer, Callback callback) { + super(values, valueStreamTransformer, callback); + } + + @Override + public IsIn withListEmptyCallback(Callback callback) { + return new IsIn<>(values, valueStreamTransformer, callback); + } + @Override public String renderCondition(String columnName, Stream placeholders) { return spaceAfter(columnName) @@ -51,7 +57,7 @@ public String renderCondition(String columnName, Stream placeholders) { * @return new condition with the specified transformer */ public IsIn then(UnaryOperator> valueStreamTransformer) { - return new IsIn<>(values, valueStreamTransformer); + return new IsIn<>(values, valueStreamTransformer, callback); } public static IsIn of(Collection values) { diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java index e011fd82d..792a157f0 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java @@ -21,14 +21,25 @@ import java.util.stream.Stream; import org.mybatis.dynamic.sql.AbstractListValueCondition; +import org.mybatis.dynamic.sql.Callback; import org.mybatis.dynamic.sql.util.StringUtilities; -public class IsInCaseInsensitive extends AbstractListValueCondition { +public class IsInCaseInsensitive extends AbstractListValueCondition { protected IsInCaseInsensitive(Collection values, UnaryOperator> valueStreamTransformer) { super(values, valueStreamTransformer); } + protected IsInCaseInsensitive(Collection values, UnaryOperator> valueStreamTransformer, + Callback callback) { + super(values, valueStreamTransformer, callback); + } + + @Override + public IsInCaseInsensitive withListEmptyCallback(Callback callback) { + return new IsInCaseInsensitive(values, valueStreamTransformer, callback); + } + @Override public String renderCondition(String columnName, Stream placeholders) { return "upper(" + columnName + ") " + //$NON-NLS-1$ //$NON-NLS-2$ diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.java index f55019cc6..e73966273 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.java @@ -15,6 +15,7 @@ */ package org.mybatis.dynamic.sql.where.condition; +import org.mybatis.dynamic.sql.Callback; import org.mybatis.dynamic.sql.util.StringUtilities; import java.util.Collection; @@ -24,12 +25,21 @@ public class IsInCaseInsensitiveWhenPresent extends IsInCaseInsensitive { - protected IsInCaseInsensitiveWhenPresent(Collection values, UnaryOperator> valueStreamTransformer) { - super(values, valueStreamTransformer); + protected IsInCaseInsensitiveWhenPresent(Collection values) { + super(values, s -> s.filter(Objects::nonNull).map(StringUtilities::safelyUpperCase)); + } + + protected IsInCaseInsensitiveWhenPresent(Collection values, UnaryOperator> valueStreamTransformer, + Callback callback) { + super(values, valueStreamTransformer, callback); + } + + @Override + public IsInCaseInsensitiveWhenPresent withListEmptyCallback(Callback callback) { + return new IsInCaseInsensitiveWhenPresent(values, valueStreamTransformer, callback); } public static IsInCaseInsensitiveWhenPresent of(Collection values) { - return new IsInCaseInsensitiveWhenPresent(values, - s -> s.filter(Objects::nonNull).map(StringUtilities::safelyUpperCase)); + return new IsInCaseInsensitiveWhenPresent(values); } } diff --git a/src/main/java/org/mybatis/dynamic/sql/where/render/WhereConditionVisitor.java b/src/main/java/org/mybatis/dynamic/sql/where/render/WhereConditionVisitor.java index 2b5772b20..74787870b 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/render/WhereConditionVisitor.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/render/WhereConditionVisitor.java @@ -50,7 +50,7 @@ private WhereConditionVisitor(Builder builder) { } @Override - public FragmentAndParameters visit(AbstractListValueCondition condition) { + public FragmentAndParameters visit(AbstractListValueCondition condition) { FragmentCollector fc = condition.mapValues(this::toFragmentAndParameters) .collect(FragmentCollector.collect()); diff --git a/src/test/java/examples/animal/data/AnimalDataTest.java b/src/test/java/examples/animal/data/AnimalDataTest.java index 371466e91..309e3cbd7 100644 --- a/src/test/java/examples/animal/data/AnimalDataTest.java +++ b/src/test/java/examples/animal/data/AnimalDataTest.java @@ -33,7 +33,6 @@ 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; @@ -45,6 +44,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mybatis.dynamic.sql.BasicColumn; +import org.mybatis.dynamic.sql.Callback; import org.mybatis.dynamic.sql.SqlTable; import org.mybatis.dynamic.sql.delete.render.DeleteStatementProvider; import org.mybatis.dynamic.sql.insert.render.BatchInsert; @@ -571,30 +571,21 @@ 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)); - } + assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> + select(id, animalName, bodyWeight, brainWeight) + .from(animalData) + .where(id, isInRequired(Collections.emptyList())) + .build() + .render(RenderingStrategies.MYBATIS3) + ); } - public static class IsInRequired extends IsIn { - protected IsInRequired(Collection values) { - super(values); - forceRenderingWhenEmpty(); - } - - public static IsInRequired isIn(Collection values) { - return new IsInRequired<>(values); - } + public static IsIn isInRequired(Collection values) { + throw new RuntimeException("fred"); + // TODO... +// return IsIn.of(values).withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred")); } - + @Test void testInCaseSensitiveCondition() { try (SqlSession sqlSession = sqlSessionFactory.openSession()) { From 707dfdbc526132d0754777c548a1ec5718e6d48a Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Thu, 27 Aug 2020 14:22:51 -0400 Subject: [PATCH 3/8] Implement a callback for empty "in" conditions --- .github/dependabot.yml | 16 +++++ .../sql/AbstractListValueCondition.java | 29 +++++--- .../org/mybatis/dynamic/sql/Callback.java | 15 ++++ .../mybatis/dynamic/sql/ConditionVisitor.java | 2 +- .../dynamic/sql/util/StringUtilities.java | 7 -- .../dynamic/sql/where/condition/IsIn.java | 20 ++++-- .../where/condition/IsInCaseInsensitive.java | 27 +++---- .../IsInCaseInsensitiveWhenPresent.java | 11 ++- .../dynamic/sql/where/condition/IsNotIn.java | 20 ++++-- .../condition/IsNotInCaseInsensitive.java | 27 +++---- .../IsNotInCaseInsensitiveWhenPresent.java | 22 ++++-- .../examples/animal/data/AnimalDataTest.java | 72 +++++++------------ .../dynamic/sql/util/StringUtilitiesTest.java | 40 ----------- 13 files changed, 146 insertions(+), 162 deletions(-) delete mode 100644 src/test/java/org/mybatis/dynamic/sql/util/StringUtilitiesTest.java diff --git a/.github/dependabot.yml b/.github/dependabot.yml index a217b347e..c28504fbe 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,3 +1,19 @@ +# +# 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. +# 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. +# + version: 2 updates: - package-ecosystem: maven diff --git a/src/main/java/org/mybatis/dynamic/sql/AbstractListValueCondition.java b/src/main/java/org/mybatis/dynamic/sql/AbstractListValueCondition.java index 1c4c6c489..69ac7d289 100644 --- a/src/main/java/org/mybatis/dynamic/sql/AbstractListValueCondition.java +++ b/src/main/java/org/mybatis/dynamic/sql/AbstractListValueCondition.java @@ -22,19 +22,26 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -public abstract class AbstractListValueCondition implements VisitableCondition { +public abstract class AbstractListValueCondition> + implements VisitableCondition { protected final Collection values; protected final UnaryOperator> valueStreamTransformer; - protected boolean renderWhenEmpty = false; + protected final Callback emptyCallback; protected AbstractListValueCondition(Collection values) { - this(values, UnaryOperator.identity()); + this(values, UnaryOperator.identity(), () -> { }); } protected AbstractListValueCondition(Collection values, UnaryOperator> valueStreamTransformer) { + this(values, valueStreamTransformer, () -> { }); + } + + protected AbstractListValueCondition(Collection values, UnaryOperator> valueStreamTransformer, + Callback emptyCallback) { this.valueStreamTransformer = Objects.requireNonNull(valueStreamTransformer); this.values = valueStreamTransformer.apply(Objects.requireNonNull(values).stream()) .collect(Collectors.toList()); + this.emptyCallback = Objects.requireNonNull(emptyCallback); } public final Stream mapValues(Function mapper) { @@ -43,20 +50,20 @@ public final Stream mapValues(Function mapper) { @Override public boolean shouldRender() { - return !values.isEmpty() || renderWhenEmpty; + if (values.isEmpty()) { + emptyCallback.call(); + return false; + } else { + return true; + } } - /** - * Use with caution - this could cause the library to render invalid SQL like "where column in ()". - */ - protected void forceRenderingWhenEmpty() { - renderWhenEmpty = true; - } - @Override public R accept(ConditionVisitor visitor) { return visitor.visit(this); } + public abstract S withListEmptyCallback(Callback callback); + public abstract String renderCondition(String columnName, Stream placeholders); } diff --git a/src/main/java/org/mybatis/dynamic/sql/Callback.java b/src/main/java/org/mybatis/dynamic/sql/Callback.java index 15051cc60..7bebf6c83 100644 --- a/src/main/java/org/mybatis/dynamic/sql/Callback.java +++ b/src/main/java/org/mybatis/dynamic/sql/Callback.java @@ -1,3 +1,18 @@ +/** + * 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. + * 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.mybatis.dynamic.sql; import java.util.function.Function; diff --git a/src/main/java/org/mybatis/dynamic/sql/ConditionVisitor.java b/src/main/java/org/mybatis/dynamic/sql/ConditionVisitor.java index f800ea5c3..14a958c71 100644 --- a/src/main/java/org/mybatis/dynamic/sql/ConditionVisitor.java +++ b/src/main/java/org/mybatis/dynamic/sql/ConditionVisitor.java @@ -1,5 +1,5 @@ /** - * Copyright 2016-2018 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. diff --git a/src/main/java/org/mybatis/dynamic/sql/util/StringUtilities.java b/src/main/java/org/mybatis/dynamic/sql/util/StringUtilities.java index b21295a7d..0b54c9685 100644 --- a/src/main/java/org/mybatis/dynamic/sql/util/StringUtilities.java +++ b/src/main/java/org/mybatis/dynamic/sql/util/StringUtilities.java @@ -16,8 +16,6 @@ package org.mybatis.dynamic.sql.util; import java.util.Optional; -import java.util.function.UnaryOperator; -import java.util.stream.Stream; public interface StringUtilities { @@ -42,9 +40,4 @@ static String spaceBefore(String in) { static String safelyUpperCase(String s) { return s == null ? null : s.toUpperCase(); } - - static UnaryOperator> upperCaseAfter(UnaryOperator> valueModifier) { - UnaryOperator> ua = s -> s.map(StringUtilities::safelyUpperCase); - return t -> ua.apply(valueModifier.apply(t)); - } } diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsIn.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsIn.java index ec1bc75b9..f7c47ecb3 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsIn.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsIn.java @@ -23,15 +23,20 @@ import java.util.stream.Stream; import org.mybatis.dynamic.sql.AbstractListValueCondition; +import org.mybatis.dynamic.sql.Callback; -public class IsIn extends AbstractListValueCondition { +public class IsIn extends AbstractListValueCondition> { + + protected IsIn(Collection values) { + super(values); + } protected IsIn(Collection values, UnaryOperator> valueStreamTransformer) { super(values, valueStreamTransformer); } - protected IsIn(Collection values) { - super(values); + protected IsIn(Collection values, UnaryOperator> valueStreamTransformer, Callback emptyCallback) { + super(values, valueStreamTransformer, emptyCallback); } @Override @@ -40,6 +45,11 @@ public String renderCondition(String columnName, Stream placeholders) { + placeholders.collect(Collectors.joining(",", "in (", ")")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ } + @Override + public IsIn withListEmptyCallback(Callback callback) { + return new IsIn<>(values, valueStreamTransformer, callback); + } + /** * This method allows you to modify the condition's values before they are placed into the parameter map. * For example, you could filter nulls, or trim strings, etc. This process will run before final rendering of SQL. @@ -51,9 +61,7 @@ public String renderCondition(String columnName, Stream placeholders) { * @return new condition with the specified transformer */ public IsIn then(UnaryOperator> valueStreamTransformer) { - IsIn answer = new IsIn<>(values, valueStreamTransformer); - answer.renderWhenEmpty = renderWhenEmpty; - return answer; + return new IsIn<>(values, valueStreamTransformer, emptyCallback); } public static IsIn of(Collection values) { diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java index 158c49bec..a72173e69 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitive.java @@ -21,16 +21,22 @@ import java.util.stream.Stream; import org.mybatis.dynamic.sql.AbstractListValueCondition; +import org.mybatis.dynamic.sql.Callback; import org.mybatis.dynamic.sql.util.StringUtilities; -public class IsInCaseInsensitive extends AbstractListValueCondition { +public class IsInCaseInsensitive extends AbstractListValueCondition { protected IsInCaseInsensitive(Collection values) { super(values, s -> s.map(StringUtilities::safelyUpperCase)); } protected IsInCaseInsensitive(Collection values, UnaryOperator> valueStreamTransformer) { - super(values, StringUtilities.upperCaseAfter(valueStreamTransformer)); + super(values, valueStreamTransformer); + } + + protected IsInCaseInsensitive(Collection values, UnaryOperator> valueStreamTransformer, + Callback emptyCallback) { + super(values, valueStreamTransformer, emptyCallback); } @Override @@ -39,20 +45,9 @@ public String renderCondition(String columnName, Stream placeholders) { placeholders.collect(Collectors.joining(",", "in (", ")")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ } - /** - * This method allows you to modify the condition's values before they are placed into the parameter map. - * For example, you could filter nulls, or trim strings, etc. This process will run before final rendering of SQL. - * If you filter values out of the stream, then final condition will not reference those values. If you filter all - * values out of the stream, then the condition will not render. - * - * @param valueStreamTransformer a UnaryOperator that will transform the value stream before - * the values are placed in the parameter map - * @return new condition with the specified transformer - */ - public IsInCaseInsensitive then(UnaryOperator> valueStreamTransformer) { - IsInCaseInsensitive answer = new IsInCaseInsensitive(values, valueStreamTransformer); - answer.renderWhenEmpty = renderWhenEmpty; - return answer; + @Override + public IsInCaseInsensitive withListEmptyCallback(Callback callback) { + return new IsInCaseInsensitive(values, valueStreamTransformer, callback); } public static IsInCaseInsensitive of(Collection values) { diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.java index e73966273..6299b779b 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsInCaseInsensitiveWhenPresent.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. @@ -15,13 +15,12 @@ */ package org.mybatis.dynamic.sql.where.condition; -import org.mybatis.dynamic.sql.Callback; -import org.mybatis.dynamic.sql.util.StringUtilities; - import java.util.Collection; import java.util.Objects; import java.util.function.UnaryOperator; import java.util.stream.Stream; +import org.mybatis.dynamic.sql.Callback; +import org.mybatis.dynamic.sql.util.StringUtilities; public class IsInCaseInsensitiveWhenPresent extends IsInCaseInsensitive { @@ -29,8 +28,8 @@ protected IsInCaseInsensitiveWhenPresent(Collection values) { super(values, s -> s.filter(Objects::nonNull).map(StringUtilities::safelyUpperCase)); } - protected IsInCaseInsensitiveWhenPresent(Collection values, UnaryOperator> valueStreamTransformer, - Callback callback) { + protected IsInCaseInsensitiveWhenPresent(Collection values, + UnaryOperator> valueStreamTransformer, Callback callback) { super(values, valueStreamTransformer, callback); } diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotIn.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotIn.java index 9890292b9..f0546c5d7 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotIn.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotIn.java @@ -23,15 +23,20 @@ import java.util.stream.Stream; import org.mybatis.dynamic.sql.AbstractListValueCondition; +import org.mybatis.dynamic.sql.Callback; -public class IsNotIn extends AbstractListValueCondition { +public class IsNotIn extends AbstractListValueCondition> { + + protected IsNotIn(Collection values) { + super(values); + } protected IsNotIn(Collection values, UnaryOperator> valueStreamTransformer) { super(values, valueStreamTransformer); } - protected IsNotIn(Collection values) { - super(values); + protected IsNotIn(Collection values, UnaryOperator> valueStreamTransformer, Callback emptyCallback) { + super(values, valueStreamTransformer, emptyCallback); } @Override @@ -41,6 +46,11 @@ public String renderCondition(String columnName, Stream placeholders) { Collectors.joining(",", "not in (", ")")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ } + @Override + public IsNotIn withListEmptyCallback(Callback callback) { + return new IsNotIn<>(values, valueStreamTransformer, callback); + } + /** * This method allows you to modify the condition's values before they are placed into the parameter map. * For example, you could filter nulls, or trim strings, etc. This process will run before final rendering of SQL. @@ -52,9 +62,7 @@ public String renderCondition(String columnName, Stream placeholders) { * @return new condition with the specified transformer */ public IsNotIn then(UnaryOperator> valueStreamTransformer) { - IsNotIn answer = new IsNotIn<>(values, valueStreamTransformer); - answer.renderWhenEmpty = renderWhenEmpty; - return answer; + return new IsNotIn<>(values, valueStreamTransformer, emptyCallback); } public static IsNotIn of(Collection values) { diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitive.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitive.java index 8a2171632..68af996ff 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitive.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitive.java @@ -21,16 +21,22 @@ import java.util.stream.Stream; import org.mybatis.dynamic.sql.AbstractListValueCondition; +import org.mybatis.dynamic.sql.Callback; import org.mybatis.dynamic.sql.util.StringUtilities; -public class IsNotInCaseInsensitive extends AbstractListValueCondition { +public class IsNotInCaseInsensitive extends AbstractListValueCondition { protected IsNotInCaseInsensitive(Collection values) { super(values, s -> s.map(StringUtilities::safelyUpperCase)); } protected IsNotInCaseInsensitive(Collection values, UnaryOperator> valueStreamTransformer) { - super(values, StringUtilities.upperCaseAfter(valueStreamTransformer)); + super(values, valueStreamTransformer); + } + + protected IsNotInCaseInsensitive(Collection values, UnaryOperator> valueStreamTransformer, + Callback emptyCallback) { + super(values, valueStreamTransformer, emptyCallback); } @Override @@ -40,20 +46,9 @@ public String renderCondition(String columnName, Stream placeholders) { Collectors.joining(",", "not in (", ")")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ } - /** - * This method allows you to modify the condition's values before they are placed into the parameter map. - * For example, you could filter nulls, or trim strings, etc. This process will run before final rendering of SQL. - * If you filter values out of the stream, then final condition will not reference those values. If you filter all - * values out of the stream, then the condition will not render. - * - * @param valueStreamTransformer a UnaryOperator that will transform the value stream before - * the values are placed in the parameter map - * @return new condition with the specified transformer - */ - public IsNotInCaseInsensitive then(UnaryOperator> valueStreamTransformer) { - IsNotInCaseInsensitive answer = new IsNotInCaseInsensitive(values, valueStreamTransformer); - answer.renderWhenEmpty = renderWhenEmpty; - return answer; + @Override + public IsNotInCaseInsensitive withListEmptyCallback(Callback callback) { + return new IsNotInCaseInsensitive(values, valueStreamTransformer, callback); } public static IsNotInCaseInsensitive of(Collection values) { diff --git a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitiveWhenPresent.java b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitiveWhenPresent.java index 35de1744e..e1ab564f4 100644 --- a/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitiveWhenPresent.java +++ b/src/main/java/org/mybatis/dynamic/sql/where/condition/IsNotInCaseInsensitiveWhenPresent.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. @@ -15,20 +15,30 @@ */ package org.mybatis.dynamic.sql.where.condition; -import org.mybatis.dynamic.sql.util.StringUtilities; - import java.util.Collection; import java.util.Objects; import java.util.function.UnaryOperator; import java.util.stream.Stream; +import org.mybatis.dynamic.sql.Callback; +import org.mybatis.dynamic.sql.util.StringUtilities; public class IsNotInCaseInsensitiveWhenPresent extends IsNotInCaseInsensitive { - protected IsNotInCaseInsensitiveWhenPresent(Collection values, UnaryOperator> valueStreamTransformer) { - super(values, valueStreamTransformer); + protected IsNotInCaseInsensitiveWhenPresent(Collection values) { + super(values, s -> s.filter(Objects::nonNull).map(StringUtilities::safelyUpperCase)); + } + + protected IsNotInCaseInsensitiveWhenPresent(Collection values, + UnaryOperator> valueStreamTransformer, Callback callback) { + super(values, valueStreamTransformer, callback); + } + + @Override + public IsNotInCaseInsensitiveWhenPresent withListEmptyCallback(Callback callback) { + return new IsNotInCaseInsensitiveWhenPresent(values, valueStreamTransformer, callback); } public static IsNotInCaseInsensitiveWhenPresent of(Collection values) { - return new IsNotInCaseInsensitiveWhenPresent(values, s -> s.filter(Objects::nonNull).map(StringUtilities::safelyUpperCase)); + return new IsNotInCaseInsensitiveWhenPresent(values); } } diff --git a/src/test/java/examples/animal/data/AnimalDataTest.java b/src/test/java/examples/animal/data/AnimalDataTest.java index a17e8c9c6..6cd66e4a6 100644 --- a/src/test/java/examples/animal/data/AnimalDataTest.java +++ b/src/test/java/examples/animal/data/AnimalDataTest.java @@ -35,6 +35,7 @@ import java.util.Objects; 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; @@ -592,25 +593,18 @@ void testInConditionWithEventuallyEmptyList() { @Test void testInConditionWithEventuallyEmptyListForceRendering() { - try (SqlSession sqlSession = sqlSessionFactory.openSession()) { - AnimalDataMapper mapper = sqlSession.getMapper(AnimalDataMapper.class); - - List inValues = new ArrayList<>(); - inValues.add(null); - inValues.add(22); - inValues.add(null); - - SelectStatementProvider selectStatement = select(id, animalName, bodyWeight, brainWeight) - .from(animalData) - .where(id, IsInRequired.isIn(inValues).then(s -> s.filter(Objects::nonNull).filter(i -> i != 22))) - .build() - .render(RenderingStrategies.MYBATIS3); + List inValues = new ArrayList<>(); + inValues.add(null); + inValues.add(22); + inValues.add(null); - assertThat(selectStatement.getSelectStatement()) - .isEqualTo("select id, animal_name, body_weight, brain_weight from AnimalData where id in ()"); - - assertThatExceptionOfType(PersistenceException.class).isThrownBy(() -> mapper.selectMany(selectStatement)); - } + assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> + select(id, animalName, bodyWeight, brainWeight) + .from(animalData) + .where(id, isInRequired(inValues).then(s -> s.filter(Objects::nonNull).filter(i -> i != 22))) + .build() + .render(RenderingStrategies.MYBATIS3) + ); } @Test @@ -624,10 +618,8 @@ void testInConditionWithEmptyList() { ); } - public static IsIn isInRequired(Collection values) { - throw new RuntimeException("fred"); - // TODO... -// return IsIn.of(values).withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred")); + private static IsIn isInRequired(Collection values) { + return IsIn.of(values).withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred")); } @Test @@ -714,34 +706,20 @@ void testNotInConditionWithEventuallyEmptyList() { @Test void testNotInConditionWithEventuallyEmptyListForceRendering() { - try (SqlSession sqlSession = sqlSessionFactory.openSession()) { - AnimalDataMapper mapper = sqlSession.getMapper(AnimalDataMapper.class); - - SelectStatementProvider selectStatement = select(id, animalName, bodyWeight, brainWeight) - .from(animalData) - .where(id, IsNotInRequired.isNotIn(null, 22, null) - .then(s -> s.filter(Objects::nonNull).filter(i -> i != 22))) - .build() - .render(RenderingStrategies.MYBATIS3); - - assertThat(selectStatement.getSelectStatement()) - .isEqualTo("select id, animal_name, body_weight, brain_weight from AnimalData where id not in ()"); - - assertThatExceptionOfType(PersistenceException.class).isThrownBy(() -> mapper.selectMany(selectStatement)); - } + assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> + select(id, animalName, bodyWeight, brainWeight) + .from(animalData) + .where(id, isNotInRequired(null, 22, null) + .then(s -> s.filter(Objects::nonNull).filter(i -> i != 22))) + .build() + .render(RenderingStrategies.MYBATIS3)); } - public static class IsNotInRequired extends IsNotIn { - protected IsNotInRequired(Collection values) { - super(values); - forceRenderingWhenEmpty(); - } - - @SafeVarargs - public static IsNotInRequired isNotIn(T...values) { - return new IsNotInRequired<>(Arrays.asList(values)); - } + @SafeVarargs + private static IsNotIn isNotInRequired(T...values) { + return IsNotIn.of(Arrays.asList(values)).withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred")); } + @Test void testLikeCondition() { try (SqlSession sqlSession = sqlSessionFactory.openSession()) { diff --git a/src/test/java/org/mybatis/dynamic/sql/util/StringUtilitiesTest.java b/src/test/java/org/mybatis/dynamic/sql/util/StringUtilitiesTest.java deleted file mode 100644 index d49016fe7..000000000 --- a/src/test/java/org/mybatis/dynamic/sql/util/StringUtilitiesTest.java +++ /dev/null @@ -1,40 +0,0 @@ -/** - * 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. - * 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.mybatis.dynamic.sql.util; - -import static org.assertj.core.api.Assertions.assertThat; - -import java.util.List; -import java.util.function.UnaryOperator; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -import org.junit.jupiter.api.Test; - -class StringUtilitiesTest { - - @Test - void testThatUpperCaseIsAppliedAfter() { - Stream ss = Stream.of("fred", "wilma", "barney", "betty"); - - UnaryOperator> valueModifier = s -> s.filter(st -> st.equals("fred")); - - UnaryOperator> ua = StringUtilities.upperCaseAfter(valueModifier); - - List list = ua.apply(ss).collect(Collectors.toList()); - assertThat(list).containsExactly("FRED"); - } -} From 516870f30279d3794e54b1122bcd0be625d93f5a Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Thu, 27 Aug 2020 15:19:40 -0400 Subject: [PATCH 4/8] Coverage --- .../sql/select/SelectStatementTest.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/test/java/org/mybatis/dynamic/sql/select/SelectStatementTest.java b/src/test/java/org/mybatis/dynamic/sql/select/SelectStatementTest.java index 09670aa33..319b1d881 100644 --- a/src/test/java/org/mybatis/dynamic/sql/select/SelectStatementTest.java +++ b/src/test/java/org/mybatis/dynamic/sql/select/SelectStatementTest.java @@ -16,14 +16,17 @@ package org.mybatis.dynamic.sql.select; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.junit.jupiter.api.Assertions.assertAll; import static org.mybatis.dynamic.sql.SqlBuilder.*; import java.sql.JDBCType; +import java.util.Collections; import java.util.Date; import java.util.Map; import org.junit.jupiter.api.Test; +import org.mybatis.dynamic.sql.Callback; import org.mybatis.dynamic.sql.SqlColumn; import org.mybatis.dynamic.sql.SqlTable; import org.mybatis.dynamic.sql.render.RenderingStrategies; @@ -34,6 +37,7 @@ class SelectStatementTest { static final SqlTable table = SqlTable.of("foo"); static final SqlColumn column1 = table.column("column1", JDBCType.DATE); static final SqlColumn column2 = table.column("column2", JDBCType.INTEGER); + static final SqlColumn column3 = table.column("column3", JDBCType.VARCHAR); @Test void testSimpleCriteria() { @@ -257,4 +261,56 @@ void testGroupBySingleColumn() { () -> assertThat(parameters).containsEntry("p1", d) ); } + + @Test + void testInCaseInsensitiveEmptyList() { + SelectModel selectModel = select(column1, column3) + .from(table, "a") + .where(column3, isInCaseInsensitive(Collections.emptyList()) + .withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred"))) + .build(); + + assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> + selectModel.render(RenderingStrategies.MYBATIS3) + ); + } + + @Test + void testInCaseInsensitiveWhenPresentEmptyList() { + SelectModel selectModel = select(column1, column3) + .from(table, "a") + .where(column3, isInCaseInsensitiveWhenPresent(Collections.emptyList()) + .withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred"))) + .build(); + + assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> + selectModel.render(RenderingStrategies.MYBATIS3) + ); + } + + @Test + void testNotInCaseInsensitiveEmptyList() { + SelectModel selectModel = select(column1, column3) + .from(table, "a") + .where(column3, isNotInCaseInsensitive(Collections.emptyList()) + .withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred"))) + .build(); + + assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> + selectModel.render(RenderingStrategies.MYBATIS3) + ); + } + + @Test + void testNotInCaseInsensitiveWhenPresentEmptyList() { + SelectModel selectModel = select(column1, column3) + .from(table, "a") + .where(column3, isNotInCaseInsensitiveWhenPresent(Collections.emptyList()) + .withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred"))) + .build(); + + assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> + selectModel.render(RenderingStrategies.MYBATIS3) + ); + } } From 53d1476ed795e65b324fcad8fc6efd927fe74dda Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Thu, 27 Aug 2020 15:20:08 -0400 Subject: [PATCH 5/8] Style update from Sonar Suggestion --- .../examples/animal/data/AnimalDataTest.java | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/test/java/examples/animal/data/AnimalDataTest.java b/src/test/java/examples/animal/data/AnimalDataTest.java index 6cd66e4a6..49d08ef37 100644 --- a/src/test/java/examples/animal/data/AnimalDataTest.java +++ b/src/test/java/examples/animal/data/AnimalDataTest.java @@ -35,7 +35,6 @@ import java.util.Objects; 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; @@ -56,6 +55,7 @@ import org.mybatis.dynamic.sql.insert.render.InsertStatementProvider; import org.mybatis.dynamic.sql.render.RenderingStrategies; import org.mybatis.dynamic.sql.render.TableAliasCalculator; +import org.mybatis.dynamic.sql.select.SelectModel; import org.mybatis.dynamic.sql.select.render.SelectStatementProvider; import org.mybatis.dynamic.sql.update.render.UpdateStatementProvider; import org.mybatis.dynamic.sql.util.mybatis3.MyBatis3Utils; @@ -598,23 +598,25 @@ void testInConditionWithEventuallyEmptyListForceRendering() { inValues.add(22); inValues.add(null); - assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> - select(id, animalName, bodyWeight, brainWeight) + SelectModel selectModel = select(id, animalName, bodyWeight, brainWeight) .from(animalData) .where(id, isInRequired(inValues).then(s -> s.filter(Objects::nonNull).filter(i -> i != 22))) - .build() - .render(RenderingStrategies.MYBATIS3) + .build(); + + assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> + selectModel.render(RenderingStrategies.MYBATIS3) ); } @Test void testInConditionWithEmptyList() { + SelectModel selectModel = select(id, animalName, bodyWeight, brainWeight) + .from(animalData) + .where(id, isInRequired(Collections.emptyList())) + .build(); + assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> - select(id, animalName, bodyWeight, brainWeight) - .from(animalData) - .where(id, isInRequired(Collections.emptyList())) - .build() - .render(RenderingStrategies.MYBATIS3) + selectModel.render(RenderingStrategies.MYBATIS3) ); } @@ -706,13 +708,15 @@ void testNotInConditionWithEventuallyEmptyList() { @Test void testNotInConditionWithEventuallyEmptyListForceRendering() { - assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> - select(id, animalName, bodyWeight, brainWeight) + SelectModel selectModel = select(id, animalName, bodyWeight, brainWeight) .from(animalData) .where(id, isNotInRequired(null, 22, null) .then(s -> s.filter(Objects::nonNull).filter(i -> i != 22))) - .build() - .render(RenderingStrategies.MYBATIS3)); + .build(); + + assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> + selectModel.render(RenderingStrategies.MYBATIS3) + ); } @SafeVarargs From c962b37092b83e2ebc968e2a85ab0884ddee9efd Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Thu, 27 Aug 2020 16:08:37 -0400 Subject: [PATCH 6/8] Better name for callback convenience method --- src/main/java/org/mybatis/dynamic/sql/Callback.java | 2 +- src/test/java/examples/animal/data/AnimalDataTest.java | 4 ++-- .../mybatis/dynamic/sql/select/SelectStatementTest.java | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/mybatis/dynamic/sql/Callback.java b/src/main/java/org/mybatis/dynamic/sql/Callback.java index 7bebf6c83..ca42a901b 100644 --- a/src/main/java/org/mybatis/dynamic/sql/Callback.java +++ b/src/main/java/org/mybatis/dynamic/sql/Callback.java @@ -21,7 +21,7 @@ public interface Callback { void call(); - static Callback runtimeExceptionThrowingCallback(String message) { + static Callback exceptionThrowingCallback(String message) { return exceptionThrowingCallback(message, RuntimeException::new); } diff --git a/src/test/java/examples/animal/data/AnimalDataTest.java b/src/test/java/examples/animal/data/AnimalDataTest.java index 49d08ef37..0ca03a202 100644 --- a/src/test/java/examples/animal/data/AnimalDataTest.java +++ b/src/test/java/examples/animal/data/AnimalDataTest.java @@ -621,7 +621,7 @@ void testInConditionWithEmptyList() { } private static IsIn isInRequired(Collection values) { - return IsIn.of(values).withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred")); + return IsIn.of(values).withListEmptyCallback(Callback.exceptionThrowingCallback("Fred")); } @Test @@ -721,7 +721,7 @@ void testNotInConditionWithEventuallyEmptyListForceRendering() { @SafeVarargs private static IsNotIn isNotInRequired(T...values) { - return IsNotIn.of(Arrays.asList(values)).withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred")); + return IsNotIn.of(Arrays.asList(values)).withListEmptyCallback(Callback.exceptionThrowingCallback("Fred")); } @Test diff --git a/src/test/java/org/mybatis/dynamic/sql/select/SelectStatementTest.java b/src/test/java/org/mybatis/dynamic/sql/select/SelectStatementTest.java index 319b1d881..ad78e9eba 100644 --- a/src/test/java/org/mybatis/dynamic/sql/select/SelectStatementTest.java +++ b/src/test/java/org/mybatis/dynamic/sql/select/SelectStatementTest.java @@ -267,7 +267,7 @@ void testInCaseInsensitiveEmptyList() { SelectModel selectModel = select(column1, column3) .from(table, "a") .where(column3, isInCaseInsensitive(Collections.emptyList()) - .withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred"))) + .withListEmptyCallback(Callback.exceptionThrowingCallback("Fred"))) .build(); assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> @@ -280,7 +280,7 @@ void testInCaseInsensitiveWhenPresentEmptyList() { SelectModel selectModel = select(column1, column3) .from(table, "a") .where(column3, isInCaseInsensitiveWhenPresent(Collections.emptyList()) - .withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred"))) + .withListEmptyCallback(Callback.exceptionThrowingCallback("Fred"))) .build(); assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> @@ -293,7 +293,7 @@ void testNotInCaseInsensitiveEmptyList() { SelectModel selectModel = select(column1, column3) .from(table, "a") .where(column3, isNotInCaseInsensitive(Collections.emptyList()) - .withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred"))) + .withListEmptyCallback(Callback.exceptionThrowingCallback("Fred"))) .build(); assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> @@ -306,7 +306,7 @@ void testNotInCaseInsensitiveWhenPresentEmptyList() { SelectModel selectModel = select(column1, column3) .from(table, "a") .where(column3, isNotInCaseInsensitiveWhenPresent(Collections.emptyList()) - .withListEmptyCallback(Callback.runtimeExceptionThrowingCallback("Fred"))) + .withListEmptyCallback(Callback.exceptionThrowingCallback("Fred"))) .build(); assertThatExceptionOfType(RuntimeException.class).describedAs("Fred").isThrownBy(() -> From 602f0113dc737a9e93ba30ffd63ff5b99762c1eb Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Thu, 27 Aug 2020 16:50:36 -0400 Subject: [PATCH 7/8] Documentation --- CHANGELOG.md | 14 +++++++++++++- src/site/markdown/docs/conditions.md | 24 +++++++++++------------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5cf2cb9e..bc4be9632 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ This log will detail notable changes to MyBatis Dynamic SQL. Full details are available on the GitHub milestone pages. +## Release 1.2.1 - Unreleased + +GitHub milestone: [https://github.com/mybatis/mybatis-dynamic-sql/issues?q=milestone%3A1.2.1+](https://github.com/mybatis/mybatis-dynamic-sql/issues?q=milestone%3A1.2.1+) + +### Fixed + +- Fixed a bug where the In conditions could render incorrectly in certain circumstances. ([#239](https://github.com/mybatis/mybatis-dynamic-sql/issues/239)) + +### Added + +- Added a callback capability to the "In" conditions that will be called before rendering when the conditions are empty. Also, removed the option that forced the library to render invalid SQL in that case. + ## Release 1.2.0 - August 19, 2020 GitHub milestone: [https://github.com/mybatis/mybatis-dynamic-sql/issues?q=milestone%3A1.2.0+](https://github.com/mybatis/mybatis-dynamic-sql/issues?q=milestone%3A1.2.0+) @@ -14,7 +26,7 @@ This release includes a significant refactoring of the classes in the "org.mybat With this release, we deprecated several insert methods because they were inconsistently named or awkward. All deprecated methods have documented direct replacements. -All deprecated code will be removed in the next release. +All deprecated code will be removed in the next minor release. ### Added diff --git a/src/site/markdown/docs/conditions.md b/src/site/markdown/docs/conditions.md index c9873ec8e..fd3a3858d 100644 --- a/src/site/markdown/docs/conditions.md +++ b/src/site/markdown/docs/conditions.md @@ -124,22 +124,20 @@ 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: +We think it is a good thing that the library will not render invalid SQL. An "in" condition will always be dropped from rendering if the list of values is empty. 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 take some action when the list is empty. This can be especially helpful when you are applying filters to the value list or using one of the built in "when present" conditions. In that case, it is possible that the list of values could end up empty after a validation check. + +The "In" conditions support a callback that will be invoked when the value list is empty and just before the statement is rendered. You could use the callback to create a condition that will throw an exception when the list is empty. For example: ```java - public class IsInRequired extends IsIn { - protected IsInRequired(Collection values) { - super(values); - forceRenderingWhenEmpty(); // calling this method will force the condition to render even if the values list is empty - } - - public static IsInRequired isIn(Collection values) { - return new IsInRequired<>(values); - } + private static IsIn isInRequired(Collection values) { + return IsIn.of(values).withListEmptyCallback(() -> { throw new RuntimeException("In values cannot be empty"); } ); } -``` -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. + // Alternatively, there is a convenience method in the Callback interface + private static IsIn isInRequired(Collection values) { + return IsIn.of(values).withListEmptyCallback(Callback.exceptionThrowingCallback("In values cannot be empty")); + } +``` 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: @@ -201,4 +199,4 @@ Then the condition could be used in a query as follows: .render(RenderingStrategies.MYBATIS3); ``` -You can apply value stream operations to the conditions `IsIn`, `IsInCaseInsensitive`, `IsNotIn`, and `IsNotInCaseInsensitive`. With the case-insensitive conditions, the library will automatically convert non-null strings to upper case after any value stream operation you specify. +You can apply value stream operations to the conditions `IsIn` and `IsNotIn`. The built in conditions `IsInCaseInsensitive`, `IsInCaseInsensitiveWhenPresent`, `IsNotInCaseInsensitive`, and `IsNotInCaseInsensitiveWhenPresent` do not support additional value stream operations as they already implement a value stream operation as part of the condition. From fb27186e56bcef39cb6a2a88695e59fdc32288ff Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Thu, 27 Aug 2020 17:05:13 -0400 Subject: [PATCH 8/8] Add PR info to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc4be9632..2199233e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ GitHub milestone: [https://github.com/mybatis/mybatis-dynamic-sql/issues?q=miles ### Added -- Added a callback capability to the "In" conditions that will be called before rendering when the conditions are empty. Also, removed the option that forced the library to render invalid SQL in that case. +- Added a callback capability to the "In" conditions that will be called before rendering when the conditions are empty. Also, removed the option that forced the library to render invalid SQL in that case. ([#241](https://github.com/mybatis/mybatis-dynamic-sql/pull/241)) ## Release 1.2.0 - August 19, 2020