From 40fe2addacf254bb6427acf6e7f9ed0ee9cd9f05 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sun, 7 Feb 2016 17:33:13 +0100 Subject: [PATCH] DATAKV-115 - Polishing. Introduced dedicated constructor on SpelCriteria to default the EvaluationContext to StandardEvaluationContext. Switched to ternary if in KeyValuePartTreeQuery for both application of Pageable and Sort. Tweaked unit test to actually compile in Eclipse. Original pull request: #16. --- .../data/keyvalue/core/SpelCriteria.java | 31 +++++++++++++------ .../keyvalue/core/SpelCriteriaAccessor.java | 13 +++++--- .../query/KeyValuePartTreeQuery.java | 27 +++++++--------- .../query/KeyValuePartTreeQueryUnitTests.java | 5 ++- 4 files changed, 46 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/springframework/data/keyvalue/core/SpelCriteria.java b/src/main/java/org/springframework/data/keyvalue/core/SpelCriteria.java index 0eb5419..8c73e93 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/SpelCriteria.java +++ b/src/main/java/org/springframework/data/keyvalue/core/SpelCriteria.java @@ -18,12 +18,14 @@ package org.springframework.data.keyvalue.core; import org.springframework.expression.EvaluationContext; import org.springframework.expression.spel.standard.SpelExpression; import org.springframework.expression.spel.support.StandardEvaluationContext; +import org.springframework.util.Assert; /** * {@link SpelCriteria} allows to pass on a {@link SpelExpression} and {@link EvaluationContext} to the actual query * processor. This decouples the {@link SpelExpression} from the context it is used in. * * @author Christoph Strobl + * @author Oliver Gierke */ public class SpelCriteria { @@ -31,29 +33,40 @@ public class SpelCriteria { private final EvaluationContext context; /** - * Creates new {@link SpelCriteria}. + * Creates a new {@link SpelCriteria} for the given {@link SpelExpression}. * * @param expression must not be {@literal null}. - * @param context can be {@literal null} and will be defaulted to {@link StandardEvaluationContext}. */ - public SpelCriteria(SpelExpression expression, EvaluationContext context) { - - this.expression = expression; - this.context = context == null ? new StandardEvaluationContext() : context; + public SpelCriteria(SpelExpression expression) { + this(expression, new StandardEvaluationContext()); } /** - * @return never {@literal null}. + * Creates new {@link SpelCriteria}. + * + * @param expression must not be {@literal null}. + * @param context must not be {@literal null}. + */ + public SpelCriteria(SpelExpression expression, EvaluationContext context) { + + Assert.notNull(expression, "SpEL expression must not be null!"); + Assert.notNull(context, "EvaluationContext must not be null!"); + + this.expression = expression; + this.context = context; + } + + /** + * @return will never be {@literal null}. */ public EvaluationContext getContext() { return context; } /** - * @return never {@literal null}. + * @return will never be {@literal null}. */ public SpelExpression getExpression() { return expression; } - } diff --git a/src/main/java/org/springframework/data/keyvalue/core/SpelCriteriaAccessor.java b/src/main/java/org/springframework/data/keyvalue/core/SpelCriteriaAccessor.java index 93c3924..710a6b2 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/SpelCriteriaAccessor.java +++ b/src/main/java/org/springframework/data/keyvalue/core/SpelCriteriaAccessor.java @@ -18,7 +18,6 @@ package org.springframework.data.keyvalue.core; import org.springframework.data.keyvalue.core.query.KeyValueQuery; import org.springframework.expression.spel.standard.SpelExpression; import org.springframework.expression.spel.standard.SpelExpressionParser; -import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.util.Assert; /** @@ -32,14 +31,21 @@ class SpelCriteriaAccessor implements CriteriaAccessor { private final SpelExpressionParser parser; /** + * Creates a new {@link SpelCriteriaAccessor} using the given {@link SpelExpressionParser}. + * * @param parser must not be {@literal null}. */ public SpelCriteriaAccessor(SpelExpressionParser parser) { Assert.notNull(parser, "SpelExpressionParser must not be null!"); + this.parser = parser; } + /* + * (non-Javadoc) + * @see org.springframework.data.keyvalue.core.CriteriaAccessor#resolve(org.springframework.data.keyvalue.core.query.KeyValueQuery) + */ @Override public SpelCriteria resolve(KeyValueQuery query) { @@ -48,11 +54,11 @@ class SpelCriteriaAccessor implements CriteriaAccessor { } if (query.getCritieria() instanceof SpelExpression) { - return new SpelCriteria((SpelExpression) query.getCritieria(), new StandardEvaluationContext()); + return new SpelCriteria((SpelExpression) query.getCritieria()); } if (query.getCritieria() instanceof String) { - return new SpelCriteria(parser.parseRaw((String) query.getCritieria()), new StandardEvaluationContext()); + return new SpelCriteria(parser.parseRaw((String) query.getCritieria())); } if (query.getCritieria() instanceof SpelCriteria) { @@ -61,5 +67,4 @@ class SpelCriteriaAccessor implements CriteriaAccessor { throw new IllegalArgumentException("Cannot create SpelCriteria for " + query.getCritieria()); } - } diff --git a/src/main/java/org/springframework/data/keyvalue/repository/query/KeyValuePartTreeQuery.java b/src/main/java/org/springframework/data/keyvalue/repository/query/KeyValuePartTreeQuery.java index 8c9e945..0f92054 100644 --- a/src/main/java/org/springframework/data/keyvalue/repository/query/KeyValuePartTreeQuery.java +++ b/src/main/java/org/springframework/data/keyvalue/repository/query/KeyValuePartTreeQuery.java @@ -115,8 +115,8 @@ public class KeyValuePartTreeQuery implements RepositoryQuery { Iterable result = this.keyValueOperations.find(query, queryMethod.getEntityInformation().getJavaType()); - long count = queryMethod.isSliceQuery() ? 0 : keyValueOperations.count(query, queryMethod.getEntityInformation() - .getJavaType()); + long count = queryMethod.isSliceQuery() ? 0 + : keyValueOperations.count(query, queryMethod.getEntityInformation().getJavaType()); return new PageImpl(IterableConverter.toList(result), page, count); @@ -136,35 +136,30 @@ public class KeyValuePartTreeQuery implements RepositoryQuery { @SuppressWarnings({ "rawtypes", "unchecked" }) protected KeyValueQuery prepareQuery(Object[] parameters) { - ParametersParameterAccessor accessor = new ParametersParameterAccessor(getQueryMethod().getParameters(), parameters); + ParametersParameterAccessor accessor = new ParametersParameterAccessor(getQueryMethod().getParameters(), + parameters); if (this.query == null) { this.query = createQuery(accessor); } - KeyValueQuery q = new KeyValueQuery(this.query.getCritieria()); + KeyValueQuery query = new KeyValueQuery(this.query.getCritieria()); if (this.query.getCritieria() instanceof SpelExpression) { EvaluationContext context = this.evaluationContextProvider.getEvaluationContext(getQueryMethod().getParameters(), parameters); - SpelCriteria spelCriteria = new SpelCriteria((SpelExpression) this.query.getCritieria(), context); - q = new KeyValueQuery(spelCriteria); - } - - if (accessor.getPageable() != null) { - q.setOffset(accessor.getPageable().getOffset()); - q.setRows(accessor.getPageable().getPageSize()); - } else { - q.setOffset(-1); - q.setRows(-1); + query = new KeyValueQuery(new SpelCriteria((SpelExpression) this.query.getCritieria(), context)); } + Pageable pageable = accessor.getPageable(); Sort sort = accessor.getSort(); - q.setSort(sort != null ? sort : query.getSort()); + query.setOffset(pageable == null ? -1 : pageable.getOffset()); + query.setRows(pageable == null ? -1 : pageable.getPageSize()); + query.setSort(sort == null ? this.query.getSort() : sort); - return q; + return query; } public KeyValueQuery createQuery(ParameterAccessor accessor) { diff --git a/src/test/java/org/springframework/data/keyvalue/repository/query/KeyValuePartTreeQueryUnitTests.java b/src/test/java/org/springframework/data/keyvalue/repository/query/KeyValuePartTreeQueryUnitTests.java index 8fe4f97..0e305aa 100644 --- a/src/test/java/org/springframework/data/keyvalue/repository/query/KeyValuePartTreeQueryUnitTests.java +++ b/src/test/java/org/springframework/data/keyvalue/repository/query/KeyValuePartTreeQueryUnitTests.java @@ -63,7 +63,10 @@ public class KeyValuePartTreeQueryUnitTests { Object[] args = new Object[] { "foo" }; - assertThat(query.prepareQuery(args).getCritieria(), not(sameInstance(query.prepareQuery(args).getCritieria()))); + Object first = query.prepareQuery(args).getCritieria(); + Object second = query.prepareQuery(args).getCritieria(); + + assertThat(first, not(sameInstance(second))); } static interface Repo {