Spring/spring-data-keyvalue
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Polishing.

Browse Source 
Use switch expressions, extract variables where possible. Simplify comparisons and combine common predicate blocks. Use simpler types where possible.

See #603
Original pull request: #604
This commit is contained in:
Mark Paluch
2024-12-16 15:22:48 +01:00
parent 768d0a8a67
commit 8d97fdd7dc
4 changed files with 107 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/main/antora/modules/ROOT/pages/repositories.adoc
View File

@@ -5,4 +5,3 @@ This chapter explains the basic foundations of Spring Data repositories and KeyV
Before continuing to the specifics, make sure you have a sound understanding of the basic concepts.
The goal of the Spring Data repository abstraction is to significantly reduce the amount of boilerplate code required to implement data access layers for various persistence stores.

88
src/main/java/org/springframework/data/keyvalue/repository/query/PredicateQueryCreator.java
View File

@@ -42,6 +42,7 @@ import org.springframework.util.ObjectUtils;
*
* @author Christoph Strobl
* @author Tom Van Wemmel
* @author Mark Paluch
* @since 3.3
*/
public class PredicateQueryCreator extends AbstractQueryCreator<KeyValueQuery<Predicate<?>>, Predicate<?>> {
@@ -55,68 +56,47 @@ public class PredicateQueryCreator extends AbstractQueryCreator<KeyValueQuery<Pr
@Override
protected Predicate<?> create(Part part, Iterator<Object> iterator) {
switch (part.getType()) {
case TRUE:
return PredicateBuilder.propertyValueOf(part).isTrue();
case FALSE:
return PredicateBuilder.propertyValueOf(part).isFalse();
case SIMPLE_PROPERTY:
return PredicateBuilder.propertyValueOf(part).isEqualTo(iterator.next());
case NEGATING_SIMPLE_PROPERTY:
return PredicateBuilder.propertyValueOf(part).isEqualTo(iterator.next()).negate();
case IS_NULL:
return PredicateBuilder.propertyValueOf(part).isNull();
case IS_NOT_NULL:
return PredicateBuilder.propertyValueOf(part).isNotNull();
case LIKE:
return PredicateBuilder.propertyValueOf(part).contains(iterator.next());
case NOT_LIKE:
return PredicateBuilder.propertyValueOf(part).contains(iterator.next()).negate();
case STARTING_WITH:
return PredicateBuilder.propertyValueOf(part).startsWith(iterator.next());
case AFTER:
case GREATER_THAN:
return PredicateBuilder.propertyValueOf(part).isGreaterThan(iterator.next());
case GREATER_THAN_EQUAL:
return PredicateBuilder.propertyValueOf(part).isGreaterThanEqual(iterator.next());
case BEFORE:
case LESS_THAN:
return PredicateBuilder.propertyValueOf(part).isLessThan(iterator.next());
case LESS_THAN_EQUAL:
return PredicateBuilder.propertyValueOf(part).isLessThanEqual(iterator.next());
case ENDING_WITH:
return PredicateBuilder.propertyValueOf(part).endsWith(iterator.next());
case BETWEEN:
return PredicateBuilder.propertyValueOf(part).isGreaterThan(iterator.next())
.and(PredicateBuilder.propertyValueOf(part).isLessThan(iterator.next()));
case REGEX:
return PredicateBuilder.propertyValueOf(part).matches(iterator.next());
case IN:
return PredicateBuilder.propertyValueOf(part).in(iterator.next());
case NOT_IN:
return PredicateBuilder.propertyValueOf(part).in(iterator.next()).negate();
default:
throw new InvalidDataAccessApiUsageException(String.format("Found invalid part '%s' in query", part.getType()));
PredicateBuilder builder = PredicateBuilder.propertyValueOf(part);
}
return switch (part.getType()) {
case TRUE -> builder.isTrue();
case FALSE -> builder.isFalse();
case SIMPLE_PROPERTY -> builder.isEqualTo(iterator.next());
case NEGATING_SIMPLE_PROPERTY -> builder.isEqualTo(iterator.next()).negate();
case IS_NULL -> builder.isNull();
case IS_NOT_NULL -> builder.isNotNull();
case LIKE -> builder.contains(iterator.next());
case NOT_LIKE -> builder.contains(iterator.next()).negate();
case STARTING_WITH -> builder.startsWith(iterator.next());
case AFTER, GREATER_THAN -> builder.isGreaterThan(iterator.next());
case GREATER_THAN_EQUAL -> builder.isGreaterThanEqual(iterator.next());
case BEFORE, LESS_THAN -> builder.isLessThan(iterator.next());
case LESS_THAN_EQUAL -> builder.isLessThanEqual(iterator.next());
case ENDING_WITH -> builder.endsWith(iterator.next());
case BETWEEN -> builder.isGreaterThan(iterator.next()).and(builder.isLessThan(iterator.next()));
case REGEX -> builder.matches(iterator.next());
case IN -> builder.in(iterator.next());
case NOT_IN -> builder.in(iterator.next()).negate();
default ->
throw new InvalidDataAccessApiUsageException(String.format("Found invalid part '%s' in query", part.getType()));
};
}
@Override
@SuppressWarnings({ "unchecked", "rawtypes" })
protected Predicate<?> and(Part part, Predicate<?> base, Iterator<Object> iterator) {
return base.and((Predicate) create(part, iterator));
}
@Override
@SuppressWarnings({ "unchecked", "rawtypes" })
protected Predicate<?> or(Predicate<?> base, Predicate<?> criteria) {
return base.or((Predicate) criteria);
}
@Override
protected KeyValueQuery<Predicate<?>> complete(@Nullable Predicate<?> criteria, Sort sort) {
if (criteria == null) {
return new KeyValueQuery<>(it -> true, sort);
}
return new KeyValueQuery<>(criteria, sort);
return criteria == null ? new KeyValueQuery<>(it -> true, sort) : new KeyValueQuery<>(criteria, sort);
}
static class PredicateBuilder {
@@ -127,6 +107,7 @@ public class PredicateQueryCreator extends AbstractQueryCreator<KeyValueQuery<Pr
this.part = part;
}
@SuppressWarnings("unchecked")
static <T> Comparator<T> comparator() {
return (Comparator<T>) COMPARATOR;
}
@@ -165,19 +146,19 @@ public class PredicateQueryCreator extends AbstractQueryCreator<KeyValueQuery<Pr
}
public Predicate<Object> isLessThan(Object value) {
return new ValueComparingPredicate(part.getProperty(), o -> comparator().compare(o, value) == -1 ? true : false);
return new ValueComparingPredicate(part.getProperty(), o -> comparator().compare(o, value) < 0);
}
public Predicate<Object> isLessThanEqual(Object value) {
return new ValueComparingPredicate(part.getProperty(), o -> comparator().compare(o, value) <= 0 ? true : false);
return new ValueComparingPredicate(part.getProperty(), o -> comparator().compare(o, value) <= 0);
}
public Predicate<Object> isGreaterThan(Object value) {
return new ValueComparingPredicate(part.getProperty(), o -> comparator().compare(o, value) == 1 ? true : false);
return new ValueComparingPredicate(part.getProperty(), o -> comparator().compare(o, value) > 0);
}
public Predicate<Object> isGreaterThanEqual(Object value) {
return new ValueComparingPredicate(part.getProperty(), o -> comparator().compare(o, value) >= 0 ? true : false);
return new ValueComparingPredicate(part.getProperty(), o -> comparator().compare(o, value) >= 0);
}
public Predicate<Object> matches(Pattern pattern) {
@@ -217,8 +198,9 @@ public class PredicateQueryCreator extends AbstractQueryCreator<KeyValueQuery<Pr
if (value instanceof Collection<?> collection) {
if (o instanceof Collection<?> subSet) {
collection.containsAll(subSet);
return collection.containsAll(subSet);
}
return collection.contains(o);
}
if (ObjectUtils.isArray(value)) {
@@ -246,7 +228,7 @@ public class PredicateQueryCreator extends AbstractQueryCreator<KeyValueQuery<Pr
}
if (o instanceof Map<?, ?> map) {
return map.values().contains(value);
return map.containsValue(value);
}
if (value == null) {

38
src/main/java/org/springframework/data/keyvalue/repository/query/SpelQueryCreator.java
View File

@@ -119,10 +119,14 @@ public class SpelQueryCreator extends AbstractQueryCreator<KeyValueQuery<SpelExp
partBuilder.append("?.equals(false)");
break;
case SIMPLE_PROPERTY:
partBuilder.append("?.equals(").append("[").append(parameterIndex++).append("])");
break;
case NEGATING_SIMPLE_PROPERTY:
partBuilder.append("?.equals(").append("[").append(parameterIndex++).append("]) == false");
partBuilder.append("?.equals(").append("[").append(parameterIndex++).append("])");
if (part.getType() == Type.NEGATING_SIMPLE_PROPERTY) {
partBuilder.append(" == false");
}
break;
case IS_NULL:
partBuilder.append(" == null");
@@ -131,10 +135,14 @@ public class SpelQueryCreator extends AbstractQueryCreator<KeyValueQuery<SpelExp
partBuilder.append(" != null");
break;
case LIKE:
partBuilder.append("?.contains(").append("[").append(parameterIndex++).append("])");
break;
case NOT_LIKE:
partBuilder.append("?.contains(").append("[").append(parameterIndex++).append("]) == false");
partBuilder.append("?.contains(").append("[").append(parameterIndex++).append("])");
if (part.getType() == Type.NOT_LIKE) {
partBuilder.append(" == false");
}
break;
case STARTING_WITH:
partBuilder.append("?.startsWith(").append("[").append(parameterIndex++).append("])");
@@ -174,32 +182,30 @@ public class SpelQueryCreator extends AbstractQueryCreator<KeyValueQuery<SpelExp
partBuilder.append(" matches ").append("[").append(parameterIndex++).append("]");
break;
case NOT_IN:
case IN:
partBuilder.append("[").append(parameterIndex++).append("].contains(");
partBuilder.append("#it?.");
partBuilder.append(part.getProperty().toDotPath().replace(".", "?."));
partBuilder.append(")");
break;
case NOT_IN:
if (part.getType() == Type.NOT_IN) {
partBuilder.append(" == false");
}
partBuilder.append("[").append(parameterIndex++).append("].contains(");
partBuilder.append("#it?.");
partBuilder.append(part.getProperty().toDotPath().replace(".", "?."));
partBuilder.append(") == false");
break;
case CONTAINING:
case NOT_CONTAINING:
case EXISTS:
default:
throw new InvalidDataAccessApiUsageException(
"Found invalid part '%s' in query".formatted(part.getType()));
throw new InvalidDataAccessApiUsageException("Found invalid part '%s' in query".formatted(part.getType()));
}
if (partIter.hasNext()) {
partBuilder.append("&&");
partBuilder.append(" && ");
}
partCnt++;
@@ -212,7 +218,7 @@ public class SpelQueryCreator extends AbstractQueryCreator<KeyValueQuery<SpelExp
}
if (orPartIter.hasNext()) {
sb.append("||");
sb.append(" || ");
}
}

69
src/test/java/org/springframework/data/keyvalue/repository/query/AbstractQueryCreatorTestBase.java
View File

@@ -23,11 +23,13 @@ import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.dao.InvalidDataAccessApiUsageException;
import org.springframework.data.annotation.Id;
import org.springframework.data.keyvalue.core.query.KeyValueQuery;
@@ -52,16 +54,15 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
static final Person RICKON = new Person("rickon", 4);
static final Person BRAN = new Person("bran", 9)//
.skinChanger(true).bornAt(Date.from(ZonedDateTime.parse("2013-01-31T06:00:00Z", FORMATTER).toInstant()));
.skinChanger(true).bornAt(Date.from(ZonedDateTime.parse("2013-01-31T06:00:00Z", FORMATTER).toInstant()));
static final Person ARYA = new Person("arya", 13);
static final Person ROBB = new Person("robb", 16)//
.named("stark").bornAt(Date.from(ZonedDateTime.parse("2010-09-20T06:00:00Z", FORMATTER).toInstant()));
.named("stark").bornAt(Date.from(ZonedDateTime.parse("2010-09-20T06:00:00Z", FORMATTER).toInstant()));
static final Person JON = new Person("jon", 17).named("snow");
@Mock RepositoryMetadata metadataMock;
@Test
// DATACMNS-525
@Test // DATACMNS-525
void equalsReturnsTrueWhenMatching() {
assertThat(evaluate("findByFirstname", BRAN.firstname).against(BRAN)).isTrue();
}
@@ -71,8 +72,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
assertThat(evaluate("findByFirstname", BRAN.firstname).against(RICKON)).isFalse();
}
@Test
// GH-603
@Test // GH-603
void notEqualsReturnsTrueWhenMatching() {
assertThat(evaluate("findByFirstnameNot", BRAN.firstname).against(RICKON)).isTrue();
}
@@ -286,7 +286,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
@Test // DATAKV-169
void inReturnsMatchCorrectly() {
ArrayList<String> list = new ArrayList<>();
List<String> list = new ArrayList<>();
list.add(ROBB.firstname);
assertThat(evaluate("findByFirstnameIn", list).against(ROBB)).isTrue();
@@ -295,7 +295,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
@Test // DATAKV-169
void inNotMatchingReturnsCorrectly() {
ArrayList<String> list = new ArrayList<>();
List<String> list = new ArrayList<>();
list.add(ROBB.firstname);
assertThat(evaluate("findByFirstnameIn", list).against(JON)).isFalse();
@@ -304,7 +304,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
@Test // DATAKV-169
void inWithNullCompareValuesCorrectly() {
ArrayList<String> list = new ArrayList<>();
List<String> list = new ArrayList<>();
list.add(null);
assertThat(evaluate("findByFirstnameIn", list).against(JON)).isFalse();
@@ -313,7 +313,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
@Test // DATAKV-169
void inWithNullSourceValuesMatchesCorrectly() {
ArrayList<String> list = new ArrayList<>();
List<String> list = new ArrayList<>();
list.add(ROBB.firstname);
assertThat(evaluate("findByFirstnameIn", list).against(new PredicateQueryCreatorUnitTests.Person(null, 10)))
@@ -323,7 +323,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
@Test // DATAKV-169
void inMatchesNullValuesCorrectly() {
ArrayList<String> list = new ArrayList<>();
List<String> list = new ArrayList<>();
list.add(null);
boolean contains = list.contains(null);
@@ -335,7 +335,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
@Test // GH-603
void notInReturnsMatchCorrectly() {
ArrayList<String> list = new ArrayList<>();
List<String> list = new ArrayList<>();
list.add(ROBB.firstname);
assertThat(evaluate("findByFirstnameNotIn", list).against(JON)).isTrue();
@@ -344,7 +344,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
@Test // GH-603
void notInNotMatchingReturnsCorrectly() {
ArrayList<String> list = new ArrayList<>();
List<String> list = new ArrayList<>();
list.add(ROBB.firstname);
assertThat(evaluate("findByFirstnameNotIn", list).against(ROBB)).isFalse();
@@ -353,7 +353,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
@Test // GH-603
void notInWithNullCompareValuesCorrectly() {
ArrayList<String> list = new ArrayList<>();
List<String> list = new ArrayList<>();
list.add(null);
assertThat(evaluate("findByFirstnameNotIn", list).against(JON)).isTrue();
@@ -362,7 +362,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
@Test // GH-603
void notInWithNullSourceValuesMatchesCorrectly() {
ArrayList<String> list = new ArrayList<>();
List<String> list = new ArrayList<>();
list.add(ROBB.firstname);
assertThat(evaluate("findByFirstnameNotIn", list).against(new PredicateQueryCreatorUnitTests.Person(null, 10)))
@@ -372,7 +372,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
@Test // GH-603
void notInMatchesNullValuesCorrectly() {
ArrayList<String> list = new ArrayList<>();
List<String> list = new ArrayList<>();
list.add(null);
assertThat(evaluate("findByFirstnameNotIn", list).against(new PredicateQueryCreatorUnitTests.Person(null, 10)))
@@ -407,7 +407,7 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
}
}
Method method = PersonRepository.class.getMethod(methodName, argTypes);
Method method = getMethod(PersonRepository.class, methodName, argTypes);
doReturn(Person.class).when(metadataMock).getReturnedDomainClass(method);
doReturn(TypeInformation.of(Person.class)).when(metadataMock).getDomainTypeInformation();
doReturn(TypeInformation.of(Person.class)).when(metadataMock).getReturnType(method);
@@ -420,6 +420,37 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
return finalizeQuery(q, args);
}
private Method getMethod(Class<?> type, String methodName, Class<?>[] argTypes) throws NoSuchMethodException {
for (Method declaredMethod : type.getDeclaredMethods()) {
if (!declaredMethod.getName().equals(methodName)) {
continue;
}
if (declaredMethod.getParameterCount() != argTypes.length) {
continue;
}
Class<?>[] types = declaredMethod.getParameterTypes();
boolean assigable = true;
for (int i = 0; i < types.length; i++) {
if (!types[i].isAssignableFrom(argTypes[i])) {
assigable = false;
break;
}
}
if (assigable) {
return declaredMethod;
}
}
throw new NoSuchMethodException("Method " + methodName + " not found in " + type);
}
protected abstract QUERY_CREATOR queryCreator(PartTree partTree, ParametersParameterAccessor accessor);
protected abstract KeyValueQuery<CRITERIA> finalizeQuery(KeyValueQuery<CRITERIA> query, Object... args);
@@ -490,10 +521,10 @@ public abstract class AbstractQueryCreatorTestBase<QUERY_CREATOR extends Abstrac
Person findByLastnameMatches(String lastname);
// Type.IN
Person findByFirstnameIn(ArrayList<String> in);
Person findByFirstnameIn(List<String> in);
// Type.NOT_IN
Person findByFirstnameNotIn(ArrayList<String> in);
Person findByFirstnameNotIn(List<String> in);
}