From c22b4edf6ea2cab019c1d7f1aed33a04baf34003 Mon Sep 17 00:00:00 2001 From: Charles Date: Sun, 21 Nov 2021 20:32:08 -0600 Subject: [PATCH 1/4] fix flaky tests in RelationalExampleMapperTests --- .../query/RelationalExampleMapperTests.java | 56 +++++++++++++------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/repository/query/RelationalExampleMapperTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/repository/query/RelationalExampleMapperTests.java index 608618bd69..694afb9215 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/repository/query/RelationalExampleMapperTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/repository/query/RelationalExampleMapperTests.java @@ -25,7 +25,15 @@ import lombok.Data; import lombok.NoArgsConstructor; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collector; +import java.util.stream.Collectors; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -33,6 +41,7 @@ import org.springframework.data.domain.Example; import org.springframework.data.domain.ExampleMatcher; import org.springframework.data.relational.core.mapping.RelationalMappingContext; +import org.springframework.data.relational.core.query.CriteriaDefinition; import org.springframework.data.relational.core.query.Query; /** @@ -89,10 +98,11 @@ void queryByExampleWithFirstnameAndLastname() { Example example = Example.of(person); Query query = exampleMapper.getMappedExample(example); - - assertThat(query.getCriteria()) // - .map(Object::toString) // - .hasValue("(firstname = 'Frodo') AND (lastname = 'Baggins')"); + Set allPossibleRes = new HashSet(){{ + add("(firstname = 'Frodo') AND (lastname = 'Baggins')"); + add("(lastname = 'Baggins') AND (firstname = 'Frodo')"); + }}; + assertThat(allPossibleRes).contains(query.getCriteria().map(Object::toString).get()); } @Test // GH-929 @@ -123,9 +133,11 @@ void queryByExampleWithNullMatchingFirstnameAndLastname() { Query query = exampleMapper.getMappedExample(example); - assertThat(query.getCriteria()) // - .map(Object::toString) // - .hasValue("(firstname IS NULL OR firstname = 'Bilbo') AND (lastname IS NULL OR lastname = 'Baggins')"); + Set allPossibleRes = new HashSet(){{ + add("(firstname IS NULL OR firstname = 'Bilbo') AND (lastname IS NULL OR lastname = 'Baggins')"); + add("(lastname IS NULL OR lastname = 'Baggins') AND (firstname IS NULL OR firstname = 'Bilbo')"); + }}; + assertThat(allPossibleRes).contains(query.getCriteria().map(Object::toString).get()); } @Test // GH-929 @@ -367,9 +379,11 @@ void queryByExampleWithFirstnameOrLastname() { Query query = exampleMapper.getMappedExample(example); - assertThat(query.getCriteria()) // - .map(Object::toString) // - .hasValue("(firstname = 'Frodo') OR (lastname = 'Baggins')"); + Set allPossibleRes = new HashSet(){{ + add("(firstname = 'Frodo') OR (lastname = 'Baggins')"); + add("(lastname = 'Baggins') OR (firstname = 'Frodo')"); + }}; + assertThat(allPossibleRes).contains(query.getCriteria().map(Object::toString).get()); } @Test // GH-929 @@ -382,10 +396,11 @@ void queryByExampleEvenHandlesInvisibleFields() { Example example = Example.of(person); Query query = exampleMapper.getMappedExample(example); - - assertThat(query.getCriteria()) // - .map(Object::toString) // - .hasValue("(firstname = 'Frodo') AND (secret = 'I have the ring!')"); + Set allPossibleRes = new HashSet(){{ + add("(firstname = 'Frodo') AND (secret = 'I have the ring!')"); + add("(secret = 'I have the ring!') AND (firstname = 'Frodo')"); + }}; + assertThat(allPossibleRes).contains(query.getCriteria().map(Object::toString).get()); } @Test // GH-929 @@ -414,10 +429,15 @@ void queryByExampleSupportsPropertyTransforms() { Query query = exampleMapper.getMappedExample(example); - assertThat(query.getCriteria()) // - .map(Object::toString) // - .hasValue("(firstname = 'FRODO') AND (lastname = 'baggins') AND (secret = 'I have the ring!')"); - + Set allPossibleRes = new HashSet(){{ + add("(firstname = 'FRODO') AND (lastname = 'baggins') AND (secret = 'I have the ring!')"); + add("(firstname = 'FRODO') AND (secret = 'I have the ring!') AND (lastname = 'baggins')"); + add("(lastname = 'baggins') AND (firstname = 'FRODO') AND (secret = 'I have the ring!')"); + add("(lastname = 'baggins') AND (secret = 'I have the ring!') AND (firstname = 'FRODO')"); + add("(secret = 'I have the ring!') AND (lastname = 'baggins') AND (firstname = 'FRODO')"); + add("(secret = 'I have the ring!') AND (firstname = 'FRODO') AND (lastname = 'baggins')"); + }}; + assertThat(allPossibleRes).contains(query.getCriteria().map(Object::toString).get()); } @Data From 16ccfe8c7be216ecd82a0d9923b6bf79303f3a5c Mon Sep 17 00:00:00 2001 From: Charles Date: Tue, 30 Nov 2021 14:26:59 -0600 Subject: [PATCH 2/4] Add https://github.com/spring-projects/spring-data-jdbc/pull/1095 changes into current PR. --- .../convert/SqlIdentifierParameterSourceUnitTests.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlIdentifierParameterSourceUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlIdentifierParameterSourceUnitTests.java index bf3ba8973f..f9ccc2673d 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlIdentifierParameterSourceUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlIdentifierParameterSourceUnitTests.java @@ -21,6 +21,8 @@ import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.SqlIdentifier; +import java.util.Arrays; + /** * Tests for {@link SqlIdentifierParameterSource}. * @@ -95,10 +97,11 @@ public void addOtherDatabaseObjectIdentifierParameterSource() { parameters2.addValue(SqlIdentifier.unquoted("key3"), 222); parameters.addAll(parameters2); - + String[] allKeys = parameters.getParameterNames(); + Arrays.sort(allKeys); assertSoftly(softly -> { - softly.assertThat(parameters.getParameterNames()).isEqualTo(new String[] { "key1", "key2", "key3" }); + softly.assertThat(allKeys).isEqualTo(new String[] { "key1", "key2", "key3" }); softly.assertThat(parameters.getValue("key1")).isEqualTo(111); softly.assertThat(parameters.hasValue("key1")).isTrue(); softly.assertThat(parameters.getSqlType("key1")).isEqualTo(11); From f69a94ebf671bec986ed5bc2ddef95cfdf128c15 Mon Sep 17 00:00:00 2001 From: Charles Date: Tue, 30 Nov 2021 14:48:33 -0600 Subject: [PATCH 3/4] Merge https://github.com/spring-projects/spring-data-jdbc/pull/1096 into current PR with string splitted and ensure the result set of conditions is the one we expect. --- .../repository/query/PartTreeJdbcQueryUnitTests.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/PartTreeJdbcQueryUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/PartTreeJdbcQueryUnitTests.java index 8bff44d21e..f15f052241 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/PartTreeJdbcQueryUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/PartTreeJdbcQueryUnitTests.java @@ -591,12 +591,22 @@ public void createsQueryByEmbeddedObject() throws Exception { String expectedSql = BASE_SELECT + " WHERE (" + TABLE + ".\"USER_STREET\" = :user_street AND " + TABLE + ".\"USER_CITY\" = :user_city)"; + String actualSql = query.getQuery(); - assertThat(query.getQuery()).isEqualTo(expectedSql); + assertThat(compareSqlStr(expectedSql, actualSql)).isTrue(); assertThat(query.getParameterSource().getValue("user_street")).isEqualTo("Hello"); assertThat(query.getParameterSource().getValue("user_city")).isEqualTo("World"); } + private boolean compareSqlStr(String expectedSql, String actualSql) { + String[] expected = expectedSql.split("WHERE"); + String[] actual = actualSql.split("WHERE"); + if (!expected[0].equals(actual[0])) return false; + expected[1] = expected[1].trim().substring(1, expected[1].length() - 2); + String[] flakyParts = expected[1].split("AND"); + return actual[1].contains(flakyParts[0]) && actual[1].contains(flakyParts[1]); + } + @Test // DATAJDBC-318 public void createsQueryByEmbeddedProperty() throws Exception { From 3790462369f67cd646bd45ebd103578f53f3daa2 Mon Sep 17 00:00:00 2001 From: Charles Date: Tue, 30 Nov 2021 15:23:05 -0600 Subject: [PATCH 4/4] Update current change with string splitted and ensure the result of conditions is the one we expect. --- .../query/RelationalExampleMapperTests.java | 55 ++++++++----------- 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/repository/query/RelationalExampleMapperTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/repository/query/RelationalExampleMapperTests.java index 694afb9215..069fe60a59 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/repository/query/RelationalExampleMapperTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/repository/query/RelationalExampleMapperTests.java @@ -98,11 +98,9 @@ void queryByExampleWithFirstnameAndLastname() { Example example = Example.of(person); Query query = exampleMapper.getMappedExample(example); - Set allPossibleRes = new HashSet(){{ - add("(firstname = 'Frodo') AND (lastname = 'Baggins')"); - add("(lastname = 'Baggins') AND (firstname = 'Frodo')"); - }}; - assertThat(allPossibleRes).contains(query.getCriteria().map(Object::toString).get()); + String actual = query.getCriteria().map(Object::toString).get(); + String expected = "(firstname = 'Frodo') AND (lastname = 'Baggins')"; + assertThat(compareStrWithFlakiness(expected, actual, "AND")).isTrue(); } @Test // GH-929 @@ -132,12 +130,9 @@ void queryByExampleWithNullMatchingFirstnameAndLastname() { Example example = Example.of(person, matcher); Query query = exampleMapper.getMappedExample(example); - - Set allPossibleRes = new HashSet(){{ - add("(firstname IS NULL OR firstname = 'Bilbo') AND (lastname IS NULL OR lastname = 'Baggins')"); - add("(lastname IS NULL OR lastname = 'Baggins') AND (firstname IS NULL OR firstname = 'Bilbo')"); - }}; - assertThat(allPossibleRes).contains(query.getCriteria().map(Object::toString).get()); + String actual = query.getCriteria().map(Object::toString).get(); + String expected = "(firstname IS NULL OR firstname = 'Bilbo') AND (lastname IS NULL OR lastname = 'Baggins')"; + assertThat(compareStrWithFlakiness(expected, actual, "AND")).isTrue(); } @Test // GH-929 @@ -378,12 +373,9 @@ void queryByExampleWithFirstnameOrLastname() { Example example = Example.of(person, matcher); Query query = exampleMapper.getMappedExample(example); - - Set allPossibleRes = new HashSet(){{ - add("(firstname = 'Frodo') OR (lastname = 'Baggins')"); - add("(lastname = 'Baggins') OR (firstname = 'Frodo')"); - }}; - assertThat(allPossibleRes).contains(query.getCriteria().map(Object::toString).get()); + String actual = query.getCriteria().map(Object::toString).get(); + String expected = "(firstname = 'Frodo') OR (lastname = 'Baggins')"; + assertThat(compareStrWithFlakiness(expected, actual, "OR")).isTrue(); } @Test // GH-929 @@ -396,11 +388,9 @@ void queryByExampleEvenHandlesInvisibleFields() { Example example = Example.of(person); Query query = exampleMapper.getMappedExample(example); - Set allPossibleRes = new HashSet(){{ - add("(firstname = 'Frodo') AND (secret = 'I have the ring!')"); - add("(secret = 'I have the ring!') AND (firstname = 'Frodo')"); - }}; - assertThat(allPossibleRes).contains(query.getCriteria().map(Object::toString).get()); + String actual = query.getCriteria().map(Object::toString).get(); + String expected = "(firstname = 'Frodo') AND (secret = 'I have the ring!')"; + assertThat(compareStrWithFlakiness(expected, actual, "AND")).isTrue(); } @Test // GH-929 @@ -428,16 +418,19 @@ void queryByExampleSupportsPropertyTransforms() { Example example = Example.of(person, matcher); Query query = exampleMapper.getMappedExample(example); + String actual = query.getCriteria().map(Object::toString).get(); + String expected = "(firstname = 'FRODO') AND (lastname = 'baggins') AND (secret = 'I have the ring!')"; + assertThat(compareStrWithFlakiness(expected, actual, "AND")).isTrue(); + } - Set allPossibleRes = new HashSet(){{ - add("(firstname = 'FRODO') AND (lastname = 'baggins') AND (secret = 'I have the ring!')"); - add("(firstname = 'FRODO') AND (secret = 'I have the ring!') AND (lastname = 'baggins')"); - add("(lastname = 'baggins') AND (firstname = 'FRODO') AND (secret = 'I have the ring!')"); - add("(lastname = 'baggins') AND (secret = 'I have the ring!') AND (firstname = 'FRODO')"); - add("(secret = 'I have the ring!') AND (lastname = 'baggins') AND (firstname = 'FRODO')"); - add("(secret = 'I have the ring!') AND (firstname = 'FRODO') AND (lastname = 'baggins')"); - }}; - assertThat(allPossibleRes).contains(query.getCriteria().map(Object::toString).get()); + private boolean compareStrWithFlakiness(String expected, String actual, String regex) { + String[] flakyParts = expected.split(regex); + for (String part : flakyParts) { + if (!actual.contains(part)) { + return false; + } + } + return true; } @Data