diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/query/QueryString.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/query/QueryString.java index acfd98c4..310e5f25 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/query/QueryString.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/query/QueryString.java @@ -72,6 +72,7 @@ public class QueryString { protected static final String COUNT_PROJECTION = "count(*)"; protected static final String IN_PATTERN = "(?<=IN (SET|LIST) )\\$\\d"; protected static final String IN_PARAMETER_PATTERN = "(?<=IN (SET|LIST) \\$)\\d"; + protected static final String IN_VALUES_TEMPLATE = "(%s)"; protected static final String REGION_PATTERN = "\\/(\\/?\\w)+"; protected static final String STAR_PROJECTION = "*"; @@ -335,17 +336,28 @@ public class QueryString { } /** - * Binds the given {@link Collection} of values into the {@literal IN} parameters of the OQL Query by expanding - * the given values into a comma-separated {@link String}. + * Binds the given {@link Collection} of values into the first {@literal IN} parameter of the OQL Query + * ({@link String}) by expanding the given values into a comma-separated list. * - * @param values the values to bind, returns the {@link QueryString} as is if {@literal null} is given. - * @return a Query String having "in" parameters bound with values. + * @param values {@link Collection} of values to bind; must not be {@literal null} or {@literal empty}. + * @return a new {@link QueryString} having {@literal IN} parameter bound with values + * or returns this {@link QueryString} if the {@link Collection} of values is {@literal null} or {@literal empty}. + * @see java.util.Collection */ - public QueryString bindIn(Collection values) { + public @NonNull QueryString bindIn(@NonNull Collection values) { if (!CollectionUtils.nullSafeIsEmpty(values)) { - return QueryString.of(getQuery().replaceFirst(IN_PATTERN, String.format("(%s)", - StringUtils.collectionToDelimitedString(values, ", ", "'", "'")))); + + boolean isNumeric = values.stream().anyMatch(Number.class::isInstance); + + String delimiter = ", "; + String prefix = isNumeric ? "" : "'"; + String suffix = prefix; + + String query = getQuery().replaceFirst(IN_PATTERN, String.format(IN_VALUES_TEMPLATE, + StringUtils.collectionToDelimitedString(values, delimiter, prefix, suffix))); + + return QueryString.of(query); } return this; diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/query/StringBasedGemfireRepositoryQuery.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/query/StringBasedGemfireRepositoryQuery.java index 24fe9ee8..d5f4fb2b 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/query/StringBasedGemfireRepositoryQuery.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/query/StringBasedGemfireRepositoryQuery.java @@ -35,6 +35,7 @@ import org.springframework.data.gemfire.repository.query.support.PagingUtils; import org.springframework.data.gemfire.repository.query.support.TemplateBasedOqlQueryExecutor; import org.springframework.data.gemfire.util.CollectionUtils; import org.springframework.data.repository.Repository; +import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.ParametersParameterAccessor; import org.springframework.data.repository.query.QueryMethod; import org.springframework.data.repository.query.RepositoryQuery; @@ -270,8 +271,10 @@ public class StringBasedGemfireRepositoryQuery extends GemfireRepositoryQuery { private QueryString bindInParameters(QueryMethod queryMethod, QueryString query, Object[] arguments) { + Parameters queryMethodParameters = queryMethod.getParameters(); + ParametersParameterAccessor parameterAccessor = - new ParametersParameterAccessor(queryMethod.getParameters(), arguments); + new ParametersParameterAccessor(queryMethodParameters, arguments); for (Integer index : query.getInParameterIndexes()) { query = query.bindIn(toCollection(parameterAccessor.getBindableValue(index - 1))); diff --git a/spring-data-geode/src/test/java/example/app/repo/UserRepository.java b/spring-data-geode/src/test/java/example/app/repo/UserRepository.java index 6f5d4a90..70a3f67c 100644 --- a/spring-data-geode/src/test/java/example/app/repo/UserRepository.java +++ b/spring-data-geode/src/test/java/example/app/repo/UserRepository.java @@ -15,6 +15,9 @@ */ package example.app.repo; +import java.util.List; +import java.util.Set; + import org.springframework.data.gemfire.repository.GemfireRepository; import example.app.model.User; @@ -31,6 +34,16 @@ import example.app.model.User; * @see example.app.model.User * @since 2.5.0 */ +@SuppressWarnings("unused") public interface UserRepository extends GemfireRepository { + List findByIdInOrderById(Integer... identifiers); + + //@Query("SELECT u FROM /Users u WHERE u.id IN ($1) ORDER BY u.id") + List findByIdInOrderById(Set identifiers); + + List findByNameInOrderByName(String... usersnames); + + List findByNameInOrderByName(Set usernames); + } diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/query/QueryStringUnitTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/query/QueryStringUnitTests.java index e2ead4e4..8e34f9ad 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/query/QueryStringUnitTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/query/QueryStringUnitTests.java @@ -428,7 +428,7 @@ public class QueryStringUnitTests { QueryString query = QueryString.of("SELECT * FROM /Collection WHERE elements IN SET $1"); assertThat(query.bindIn(Arrays.asList(1, 2, 3)).toString()) - .isEqualTo("SELECT * FROM /Collection WHERE elements IN SET ('1', '2', '3')"); + .isEqualTo("SELECT * FROM /Collection WHERE elements IN SET (1, 2, 3)"); } @Test diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/query/RepositoryQueryUsingInOperatorIntegrationTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/query/RepositoryQueryUsingInOperatorIntegrationTests.java new file mode 100644 index 00000000..1ffdc4e0 --- /dev/null +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/query/RepositoryQueryUsingInOperatorIntegrationTests.java @@ -0,0 +1,152 @@ +/* + * Copyright 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 + * + * https://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.springframework.data.gemfire.repository.query; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import java.util.Comparator; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.apache.geode.cache.client.ClientRegionShortcut; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.gemfire.config.annotation.ClientCacheApplication; +import org.springframework.data.gemfire.config.annotation.EnableEntityDefinedRegions; +import org.springframework.data.gemfire.repository.config.EnableGemfireRepositories; +import org.springframework.data.gemfire.util.ArrayUtils; +import org.springframework.data.gemfire.util.CollectionUtils; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringRunner; + +import example.app.model.User; +import example.app.repo.UserRepository; + +/** + * Integration Tests testing and asserting the use of the {@literal IN} operator in an OQL query + * + * @author John Blum + * @see org.junit.Test + * @see org.springframework.data.gemfire.repository.GemfireRepository + * @see org.springframework.data.gemfire.repository.config.EnableGemfireRepositories + * @see org.springframework.test.context.ContextConfiguration + * @see org.springframework.test.context.junit4.SpringRunner + * @see Fix bug with SDG Repository derived queries using the IN operator with numeric values. + * @since 2.5.0 + */ +@RunWith(SpringRunner.class) +@ContextConfiguration +@SuppressWarnings("unused") +public class RepositoryQueryUsingInOperatorIntegrationTests { + + private static final AtomicBoolean initialized = new AtomicBoolean(false); + + private List users = Arrays.asList( + User.as("jonDoe").identifiedBy(1), + User.as("janeDoe").identifiedBy(2), + User.as("cookieDoe").identifiedBy(3), + User.as("pieDoe").identifiedBy(4), + User.as("sourDoe").identifiedBy(5) + ); + + @Autowired + private UserRepository userRepository; + + private static boolean doOnce() { + return initialized.compareAndSet(false, true); + } + + @Before + public void setup() { + + assertThat(this.userRepository).isNotNull(); + + if (doOnce()) { + this.userRepository.saveAll(this.users); + } + + assertThat(this.userRepository.count()).isEqualTo(this.users.size()); + } + + private User[] findUsersByIds(Integer... ids) { + + return this.users.stream() + .filter(user -> Arrays.asList(ArrayUtils.nullSafeArray(ids, Integer.class)).contains(user.getId())) + .sorted(Comparator.comparing(User::getId)) + .toArray(User[]::new); + } + + private User[] findUsersByNames(String... names) { + + return this.users.stream() + .filter(user -> Arrays.asList(ArrayUtils.nullSafeArray(names, String.class)).contains(user.getName())) + .sorted(Comparator.comparing(User::getName)) + .toArray(User[]::new); + } + + @Test + public void findByUserIdsInArrayIsCorrect() { + + List usersById = this.userRepository.findByIdInOrderById(2, 4); + + assertThat(usersById).isNotNull(); + assertThat(usersById).hasSize(2); + assertThat(usersById).containsExactly(findUsersByIds(2, 4)); + } + + @Test + public void findByUserIdsInSetIsCorrect() { + + List usersById = this.userRepository.findByIdInOrderById(CollectionUtils.asSet(2, 4, 3)); + + assertThat(usersById).isNotNull(); + assertThat(usersById).hasSize(3); + assertThat(usersById).containsExactlyInAnyOrder(findUsersByIds(2, 3, 4)); + } + + @Test + public void findByUserNamesInArrayIsCorrect() { + + List usersByName = + this.userRepository.findByNameInOrderByName("pieDoe", "cookieDoe", "sourDoe"); + + assertThat(usersByName).isNotNull(); + assertThat(usersByName).hasSize(3); + assertThat(usersByName).containsExactly(findUsersByNames("cookieDoe", "pieDoe", "sourDoe")); + } + + @Test + public void findByUserNamesInSetIsCorrect() { + + List usersByName = + this.userRepository.findByNameInOrderByName(CollectionUtils.asSet("sourDoe", "jonDoe")); + + assertThat(usersByName).isNotNull(); + assertThat(usersByName).hasSize(2); + assertThat(usersByName).containsExactly(findUsersByNames("jonDoe", "sourDoe")); + } + + @ClientCacheApplication(name = "RepositoryQueryUsingInOperatorIntegrationTests") + @EnableEntityDefinedRegions(basePackageClasses = User.class, clientRegionShortcut = ClientRegionShortcut.LOCAL) + @EnableGemfireRepositories(basePackageClasses = UserRepository.class) + static class TestGeodeConfiguration { } + +}