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.

Author: odrotbohm

src/main/java/org/springframework/data/keyvalue/core/SpelCriteria.java

@@ -18,42 +18,55 @@
 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 {
 
 	private final SpelExpression expression;
 	private final EvaluationContext context;
 
+	/**
+	 * Creates a new {@link SpelCriteria} for the given {@link SpelExpression}.
+	 * 
+	 * @param expression must not be {@literal null}.
+	 */
+	public SpelCriteria(SpelExpression expression) {
+		this(expression, new StandardEvaluationContext());
+	}
+
 	/**
 	 * Creates new {@link SpelCriteria}.
 	 * 
 	 * @param expression must not be {@literal null}.
-	 * @param context can be {@literal null} and will be defaulted to {@link StandardEvaluationContext}.
+	 * @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 == null ? new StandardEvaluationContext() : context;
+		this.context = context;
 	}
 
 	/**
-	 * @return never {@literal null}.
+	 * @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;
 	}
-
 }

src/main/java/org/springframework/data/keyvalue/core/SpelCriteriaAccessor.java

@@ -18,7 +18,6 @@
 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<SpelCriteria> {
 	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 @@ public SpelCriteria resolve(KeyValueQuery<?> query) {
 		}
 
 		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 @@ public SpelCriteria resolve(KeyValueQuery<?> query) {
 
 		throw new IllegalArgumentException("Cannot create SpelCriteria for " + query.getCritieria());
 	}
-
 }

src/main/java/org/springframework/data/keyvalue/repository/query/KeyValuePartTreeQuery.java

@@ -115,8 +115,8 @@ protected Object doExecute(Object[] parameters, KeyValueQuery<?> query) {
 
 			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 @@ protected Object doExecute(Object[] parameters, KeyValueQuery<?> query) {
 	@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) {

src/test/java/org/springframework/data/keyvalue/repository/query/KeyValuePartTreeQueryUnitTests.java

@@ -63,7 +63,10 @@ public void spelExpressionAndContextShouldNotBeReused() throws NoSuchMethodExcep
 
 		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 {