Fix bug with SDG Repository derived queries using the IN operator with numeric values.

Resolves gh-483.
This commit is contained in:
John Blum
2021-02-16 18:05:07 -08:00
committed by John Blum
parent fb1f93ff54
commit 0ba8e45bef
6 changed files with 295 additions and 20 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2021 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.
@@ -16,8 +16,6 @@
package org.springframework.data.gemfire.repository.query;
import static org.springframework.data.gemfire.util.CollectionUtils.nullSafeIsEmpty;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -28,6 +26,8 @@ import org.apache.geode.cache.Region;
import org.springframework.data.domain.Sort;
import org.springframework.data.gemfire.repository.query.support.OqlKeyword;
import org.springframework.data.gemfire.util.CollectionUtils;
import org.springframework.lang.NonNull;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;
@@ -59,9 +59,10 @@ public class QueryString {
private static final String TRACE_OQL_TEMPLATE = "<TRACE> %1$s";
// OQL Query Regular Expression Patterns
private static final String IN_PATTERN = "(?<=IN (SET|LIST) )\\$\\d";
private static final String IN_PARAMETER_PATTERN = "(?<=IN (SET|LIST) \\$)\\d";
private static final String REGION_PATTERN = "\\/(\\/?\\w)+";
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)+";
private static final String COUNT_QUERY = "count(*)";
private static final String STAR_QUERY = "*";
@@ -193,17 +194,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 (!nullSafeIsEmpty(values)) {
return QueryString.of(this.query.replaceFirst(IN_PATTERN, String.format("(%s)",
StringUtils.collectionToDelimitedString(values, ", ", "'", "'"))));
if (!CollectionUtils.nullSafeIsEmpty(values)) {
boolean isNumeric = values.stream().anyMatch(Number.class::isInstance);
String delimiter = ", ";
String prefix = isNumeric ? "" : "'";
String suffix = prefix;
String query = this.query.replaceFirst(IN_PATTERN, String.format(IN_VALUES_TEMPLATE,
StringUtils.collectionToDelimitedString(values, delimiter, prefix, suffix)));
return QueryString.of(query);
}
return this;

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2021 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.
@@ -26,6 +26,7 @@ import org.apache.geode.cache.query.SelectResults;
import org.springframework.dao.IncorrectResultSizeDataAccessException;
import org.springframework.data.gemfire.GemfireTemplate;
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;
@@ -166,8 +167,10 @@ public class StringBasedGemfireRepositoryQuery extends GemfireRepositoryQuery {
query = isUserDefinedQuery() ? query
: query.fromRegion(queryMethod.getEntityInformation().getJavaType(), getTemplate().getRegion());
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)));
@@ -226,7 +229,7 @@ public class StringBasedGemfireRepositoryQuery extends GemfireRepositoryQuery {
Collection<?> toCollection(Object source) {
if (source instanceof SelectResults) {
return ((SelectResults) source).asList();
return ((SelectResults<?>) source).asList();
}
if (source instanceof Collection) {
@@ -240,6 +243,7 @@ public class StringBasedGemfireRepositoryQuery extends GemfireRepositoryQuery {
return source.getClass().isArray() ? CollectionUtils.arrayToList(source) : Collections.singletonList(source);
}
@SuppressWarnings("rawtypes")
enum ProvidedQueryPostProcessors implements QueryPostProcessor<Repository, String> {
HINT {
@@ -283,7 +287,7 @@ public class StringBasedGemfireRepositoryQuery extends GemfireRepositoryQuery {
@Override
public String postProcess(QueryMethod queryMethod, String query, Object... arguments) {
if (queryMethod instanceof GemfireQueryMethod) {
if (queryMethod instanceof GemfireQueryMethod) {
GemfireQueryMethod gemfireQueryMethod = (GemfireQueryMethod) queryMethod;

View File

@@ -0,0 +1,58 @@
/*
* 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 example.app.model;
import org.springframework.data.annotation.Id;
import org.springframework.data.gemfire.mapping.annotation.Region;
import org.springframework.lang.NonNull;
import org.springframework.lang.Nullable;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.RequiredArgsConstructor;
import lombok.Setter;
import lombok.ToString;
/**
* Abstract Data Type (ADT) modeling a user.
*
* @author John Blum
* @see org.springframework.data.gemfire.mapping.annotation.Region
* @since 2.5.0
*/
@Getter
@ToString
@EqualsAndHashCode
@Setter(AccessLevel.PROTECTED)
@Region("Users")
@NoArgsConstructor
@RequiredArgsConstructor(staticName = "as")
@SuppressWarnings("unused")
public class User {
@Id
private Integer id;
@lombok.NonNull
private String name;
public @NonNull User identifiedBy(@Nullable Integer id) {
setId(id);
return this;
}
}

View File

@@ -0,0 +1,49 @@
/*
* 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 example.app.repo;
import java.util.List;
import java.util.Set;
import org.springframework.data.gemfire.repository.GemfireRepository;
import example.app.model.User;
/**
* Spring Data [Geode] {@link GemfireRepository} used to perform basic CRUD and simple OQL query data access operations
* on {@link User} objects to and from an Apache Geode {@link org.apache.geode.cache.GemFireCache}
* {@link org.apache.geode.cache.Region}.
*
* @author John Blum
* @see org.apache.geode.cache.GemFireCache
* @see org.apache.geode.cache.Region
* @see org.springframework.data.gemfire.repository.GemfireRepository
* @see example.app.model.User
* @since 2.5.0
*/
@SuppressWarnings("unused")
public interface UserRepository extends GemfireRepository<User, Integer> {
List<User> findByIdInOrderById(Integer... identifiers);
//@Query("SELECT u FROM /Users u WHERE u.id IN ($1) ORDER BY u.id")
List<User> findByIdInOrderById(Set<Integer> identifiers);
List<User> findByNameInOrderByName(String... usersnames);
List<User> findByNameInOrderByName(Set<String> usernames);
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2021 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.
@@ -308,7 +308,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

View File

@@ -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 <a href="https://github.com/spring-projects/spring-data-geode/issues/483">Fix bug with SDG Repository derived queries using the IN operator with numeric values.</a>
* @since 2.5.0
*/
@RunWith(SpringRunner.class)
@ContextConfiguration
@SuppressWarnings("unused")
public class RepositoryQueryUsingInOperatorIntegrationTests {
private static final AtomicBoolean initialized = new AtomicBoolean(false);
private List<User> 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<User> usersById = this.userRepository.findByIdInOrderById(2, 4);
assertThat(usersById).isNotNull();
assertThat(usersById).hasSize(2);
assertThat(usersById).containsExactly(findUsersByIds(2, 4));
}
@Test
public void findByUserIdsInSetIsCorrect() {
List<User> 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<User> 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<User> 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 { }
}