From 723f20ee54d4b84f421fe697166802bf73389261 Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Thu, 27 Aug 2020 12:52:43 -0400 Subject: [PATCH 1/4] Failing tests for Issue 239 --- .../examples/animal/data/AnimalDataTest.java | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/src/test/java/examples/animal/data/AnimalDataTest.java b/src/test/java/examples/animal/data/AnimalDataTest.java index 371466e91..9debb9995 100644 --- a/src/test/java/examples/animal/data/AnimalDataTest.java +++ b/src/test/java/examples/animal/data/AnimalDataTest.java @@ -27,10 +27,12 @@ import java.sql.Connection; import java.sql.DriverManager; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import org.apache.ibatis.datasource.unpooled.UnpooledDataSource; import org.apache.ibatis.exceptions.PersistenceException; @@ -57,6 +59,7 @@ 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.condition.IsNotIn; import org.mybatis.dynamic.sql.where.render.WhereClauseProvider; class AnimalDataTest { @@ -569,6 +572,47 @@ void testInCondition() { } } + @Test + void testInConditionWithEventuallyEmptyList() { + try (SqlSession sqlSession = sqlSessionFactory.openSession()) { + AnimalDataMapper mapper = sqlSession.getMapper(AnimalDataMapper.class); + + SelectStatementProvider selectStatement = select(id, animalName, bodyWeight, brainWeight) + .from(animalData) + .where(id, isIn(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"); + List animals = mapper.selectMany(selectStatement); + assertThat(animals).hasSize(65); + } + } + + @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); + + assertThat(selectStatement.getSelectStatement()) + .isEqualTo("select id, animal_name, body_weight, brain_weight from AnimalData where id in ()"); + + assertThatExceptionOfType(PersistenceException.class).isThrownBy(() -> mapper.selectMany(selectStatement)); + } + } + @Test void testInConditionWithEmptyList() { try (SqlSession sqlSession = sqlSessionFactory.openSession()) { @@ -659,6 +703,54 @@ void testNotInCaseSensitiveConditionWithNull() { } } + @Test + void testNotInConditionWithEventuallyEmptyList() { + try (SqlSession sqlSession = sqlSessionFactory.openSession()) { + AnimalDataMapper mapper = sqlSession.getMapper(AnimalDataMapper.class); + + SelectStatementProvider selectStatement = select(id, animalName, bodyWeight, brainWeight) + .from(animalData) + .where(id, 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"); + List animals = mapper.selectMany(selectStatement); + assertThat(animals).hasSize(65); + } + } + + @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)); + } + } + + 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)); + } + } @Test void testLikeCondition() { try (SqlSession sqlSession = sqlSessionFactory.openSession()) { From e6f289ae818dba1679bbfc911b41625effaec042 Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Thu, 27 Aug 2020 13:03:22 -0400 Subject: [PATCH 2/4] Apply the value transform immediately on construction This will allow the rendering decision to be accurate --- .github/dependabot.yml | 16 ++++++++++++++++ .../dynamic/sql/AbstractListValueCondition.java | 7 ++++--- 2 files changed, 20 insertions(+), 3 deletions(-) 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 1b589e2dc..1c4c6c489 100644 --- a/src/main/java/org/mybatis/dynamic/sql/AbstractListValueCondition.java +++ b/src/main/java/org/mybatis/dynamic/sql/AbstractListValueCondition.java @@ -15,11 +15,11 @@ */ package org.mybatis.dynamic.sql; -import java.util.ArrayList; import java.util.Collection; import java.util.Objects; import java.util.function.Function; import java.util.function.UnaryOperator; +import java.util.stream.Collectors; import java.util.stream.Stream; public abstract class AbstractListValueCondition implements VisitableCondition { @@ -32,12 +32,13 @@ protected AbstractListValueCondition(Collection values) { } protected AbstractListValueCondition(Collection values, UnaryOperator> valueStreamTransformer) { - this.values = new ArrayList<>(Objects.requireNonNull(values)); this.valueStreamTransformer = Objects.requireNonNull(valueStreamTransformer); + this.values = valueStreamTransformer.apply(Objects.requireNonNull(values).stream()) + .collect(Collectors.toList()); } public final Stream mapValues(Function mapper) { - return valueStreamTransformer.apply(values.stream()).map(mapper); + return values.stream().map(mapper); } @Override From 6f3241d739f27f2663444a6071ba4eedb456f602 Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Thu, 27 Aug 2020 13:05:20 -0400 Subject: [PATCH 3/4] Carry the renderWhenEmpty flag throughout This is getting ugly. Mutable classes are a nightmare. We'll fix this in another issue. --- .../java/org/mybatis/dynamic/sql/where/condition/IsIn.java | 6 ++++-- .../dynamic/sql/where/condition/IsInCaseInsensitive.java | 6 ++++-- .../org/mybatis/dynamic/sql/where/condition/IsNotIn.java | 6 ++++-- .../dynamic/sql/where/condition/IsNotInCaseInsensitive.java | 6 ++++-- 4 files changed, 16 insertions(+), 8 deletions(-) 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..ec1bc75b9 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 @@ -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. @@ -51,7 +51,9 @@ public String renderCondition(String columnName, Stream placeholders) { * @return new condition with the specified transformer */ public IsIn then(UnaryOperator> valueStreamTransformer) { - return new IsIn<>(values, valueStreamTransformer); + IsIn answer = new IsIn<>(values, valueStreamTransformer); + answer.renderWhenEmpty = renderWhenEmpty; + return answer; } 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 622a6ea40..158c49bec 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 @@ -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. @@ -50,7 +50,9 @@ public String renderCondition(String columnName, Stream placeholders) { * @return new condition with the specified transformer */ public IsInCaseInsensitive then(UnaryOperator> valueStreamTransformer) { - return new IsInCaseInsensitive(values, valueStreamTransformer); + IsInCaseInsensitive answer = new IsInCaseInsensitive(values, valueStreamTransformer); + answer.renderWhenEmpty = renderWhenEmpty; + return answer; } public static IsInCaseInsensitive of(Collection values) { 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 ec617468e..9890292b9 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 @@ -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. @@ -52,7 +52,9 @@ public String renderCondition(String columnName, Stream placeholders) { * @return new condition with the specified transformer */ public IsNotIn then(UnaryOperator> valueStreamTransformer) { - return new IsNotIn<>(values, valueStreamTransformer); + IsNotIn answer = new IsNotIn<>(values, valueStreamTransformer); + answer.renderWhenEmpty = renderWhenEmpty; + return answer; } 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 6c6a40269..8a2171632 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 @@ -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. @@ -51,7 +51,9 @@ public String renderCondition(String columnName, Stream placeholders) { * @return new condition with the specified transformer */ public IsNotInCaseInsensitive then(UnaryOperator> valueStreamTransformer) { - return new IsNotInCaseInsensitive(values, valueStreamTransformer); + IsNotInCaseInsensitive answer = new IsNotInCaseInsensitive(values, valueStreamTransformer); + answer.renderWhenEmpty = renderWhenEmpty; + return answer; } public static IsNotInCaseInsensitive of(Collection values) { From 867fd91d2663ac100231ef215de84a37396a3c85 Mon Sep 17 00:00:00 2001 From: Jeff Butler Date: Thu, 27 Aug 2020 13:09:48 -0400 Subject: [PATCH 4/4] Revert unrelated --- .github/dependabot.yml | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index c28504fbe..a217b347e 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,19 +1,3 @@ -# -# 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