diff --git a/src/main/java/org/springframework/data/keyvalue/core/SpelCriteria.java b/src/main/java/org/springframework/data/keyvalue/core/SpelCriteria.java new file mode 100644 index 0000000..0eb5419 --- /dev/null +++ b/src/main/java/org/springframework/data/keyvalue/core/SpelCriteria.java @@ -0,0 +1,59 @@ +/* + * Copyright 2016 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 + * + * http://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.keyvalue.core; + +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.spel.standard.SpelExpression; +import org.springframework.expression.spel.support.StandardEvaluationContext; + +/** + * {@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 + */ +public class SpelCriteria { + + private final SpelExpression expression; + private final EvaluationContext context; + + /** + * Creates new {@link SpelCriteria}. + * + * @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; + } + + /** + * @return never {@literal null}. + */ + public EvaluationContext getContext() { + return context; + } + + /** + * @return never {@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 06e2233..93c3924 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/SpelCriteriaAccessor.java +++ b/src/main/java/org/springframework/data/keyvalue/core/SpelCriteriaAccessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 2014-2016 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. @@ -18,6 +18,7 @@ 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; /** @@ -26,7 +27,7 @@ import org.springframework.util.Assert; * @author Christoph Strobl * @author Oliver Gierke */ -class SpelCriteriaAccessor implements CriteriaAccessor { +class SpelCriteriaAccessor implements CriteriaAccessor { private final SpelExpressionParser parser; @@ -40,20 +41,25 @@ class SpelCriteriaAccessor implements CriteriaAccessor { } @Override - public SpelExpression resolve(KeyValueQuery query) { + public SpelCriteria resolve(KeyValueQuery query) { if (query.getCritieria() == null) { return null; } if (query.getCritieria() instanceof SpelExpression) { - return (SpelExpression) query.getCritieria(); + return new SpelCriteria((SpelExpression) query.getCritieria(), new StandardEvaluationContext()); } if (query.getCritieria() instanceof String) { - return parser.parseRaw((String) query.getCritieria()); + return new SpelCriteria(parser.parseRaw((String) query.getCritieria()), new StandardEvaluationContext()); + } + + if (query.getCritieria() instanceof SpelCriteria) { + return (SpelCriteria) query.getCritieria(); } throw new IllegalArgumentException("Cannot create SpelCriteria for " + query.getCritieria()); } + } diff --git a/src/main/java/org/springframework/data/keyvalue/core/SpelQueryEngine.java b/src/main/java/org/springframework/data/keyvalue/core/SpelQueryEngine.java index d7356cf..e9875dc 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/SpelQueryEngine.java +++ b/src/main/java/org/springframework/data/keyvalue/core/SpelQueryEngine.java @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 2014-2016 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. @@ -35,7 +35,7 @@ import org.springframework.expression.spel.standard.SpelExpressionParser; * @author Oliver Gierke * @param */ -class SpelQueryEngine extends QueryEngine> { +class SpelQueryEngine extends QueryEngine> { private static final SpelExpressionParser PARSER = new SpelExpressionParser(); @@ -51,7 +51,7 @@ class SpelQueryEngine extends QueryEngine execute(SpelExpression criteria, Comparator sort, int offset, int rows, Serializable keyspace) { + public Collection execute(SpelCriteria criteria, Comparator sort, int offset, int rows, Serializable keyspace) { return sortAndFilterMatchingRange(getAdapter().getAllOf(keyspace), criteria, sort, offset, rows); } @@ -60,12 +60,12 @@ class SpelQueryEngine extends QueryEngine sortAndFilterMatchingRange(Iterable source, SpelExpression criteria, Comparator sort, int offset, + private List sortAndFilterMatchingRange(Iterable source, SpelCriteria criteria, Comparator sort, int offset, int rows) { List tmp = IterableConverter.toList(source); @@ -76,7 +76,7 @@ class SpelQueryEngine extends QueryEngine List filterMatchingRange(Iterable source, SpelExpression criteria, int offset, int rows) { + private static List filterMatchingRange(Iterable source, SpelCriteria criteria, int offset, int rows) { List result = new ArrayList(); @@ -90,10 +90,11 @@ class SpelQueryEngine extends QueryEngine query = prepareQuery(parameters, accessor); + KeyValueQuery query = prepareQuery(parameters); ResultProcessor processor = queryMethod.getResultProcessor().withDynamicProjection(accessor); return processor.processResult(doExecute(parameters, query)); @@ -114,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); @@ -133,7 +134,9 @@ public class KeyValuePartTreeQuery implements RepositoryQuery { } @SuppressWarnings({ "rawtypes", "unchecked" }) - private KeyValueQuery prepareQuery(Object[] parameters, ParameterAccessor accessor) { + protected KeyValueQuery prepareQuery(Object[] parameters) { + + ParametersParameterAccessor accessor = new ParametersParameterAccessor(getQueryMethod().getParameters(), parameters); if (this.query == null) { this.query = createQuery(accessor); @@ -141,6 +144,14 @@ public class KeyValuePartTreeQuery implements RepositoryQuery { KeyValueQuery q = 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()); @@ -153,12 +164,6 @@ public class KeyValuePartTreeQuery implements RepositoryQuery { q.setSort(sort != null ? sort : query.getSort()); - if (q.getCritieria() instanceof SpelExpression) { - EvaluationContext context = this.evaluationContextProvider.getEvaluationContext(getQueryMethod().getParameters(), - parameters); - ((SpelExpression) q.getCritieria()).setEvaluationContext(context); - } - return q; } diff --git a/src/test/java/org/springframework/data/keyvalue/core/SpelQueryEngineUnitTests.java b/src/test/java/org/springframework/data/keyvalue/core/SpelQueryEngineUnitTests.java index a27b058..98ac8f9 100644 --- a/src/test/java/org/springframework/data/keyvalue/core/SpelQueryEngineUnitTests.java +++ b/src/test/java/org/springframework/data/keyvalue/core/SpelQueryEngineUnitTests.java @@ -16,7 +16,6 @@ package org.springframework.data.keyvalue.core; import static org.hamcrest.Matchers.*; -import static org.hamcrest.Matchers.contains; import static org.junit.Assert.*; import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; @@ -33,14 +32,12 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.springframework.data.annotation.Id; -import org.springframework.data.keyvalue.core.query.KeyValueQuery; import org.springframework.data.keyvalue.repository.query.SpelQueryCreator; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.query.ParametersParameterAccessor; import org.springframework.data.repository.query.QueryMethod; import org.springframework.data.repository.query.parser.PartTree; -import org.springframework.expression.spel.standard.SpelExpression; import org.springframework.expression.spel.support.StandardEvaluationContext; /** @@ -77,8 +74,8 @@ public class SpelQueryEngineUnitTests { doReturn(people).when(adapter).getAllOf(anyString()); - assertThat((Collection) engine.execute(createQueryForMethodWithArgs("findByFirstname", "bob"), null, -1, -1, - anyString()), contains(BOB_WITH_FIRSTNAME)); + assertThat((Collection) engine.execute(createQueryForMethodWithArgs("findByFirstname", "bob"), null, -1, + -1, anyString()), contains(BOB_WITH_FIRSTNAME)); } /** @@ -92,7 +89,7 @@ public class SpelQueryEngineUnitTests { assertThat(engine.count(createQueryForMethodWithArgs("findByFirstname", "bob"), anyString()), is(1L)); } - private static SpelExpression createQueryForMethodWithArgs(String methodName, Object... args) throws Exception { + private static SpelCriteria createQueryForMethodWithArgs(String methodName, Object... args) throws Exception { List> types = new ArrayList>(args.length); @@ -105,13 +102,10 @@ public class SpelQueryEngineUnitTests { doReturn(method.getReturnType()).when(metadata).getReturnedDomainClass(method); PartTree partTree = new PartTree(method.getName(), method.getReturnType()); - SpelQueryCreator creator = new SpelQueryCreator(partTree, new ParametersParameterAccessor( - new QueryMethod(method, metadata, new SpelAwareProxyProjectionFactory()).getParameters(), args)); + SpelQueryCreator creator = new SpelQueryCreator(partTree, new ParametersParameterAccessor(new QueryMethod(method, + metadata, new SpelAwareProxyProjectionFactory()).getParameters(), args)); - KeyValueQuery query = creator.createQuery(); - query.getCritieria().setEvaluationContext(new StandardEvaluationContext(args)); - - return query.getCritieria(); + return new SpelCriteria(creator.createQuery().getCritieria(), new StandardEvaluationContext(args)); } static interface PersonRepository { 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 new file mode 100644 index 0000000..8fe4f97 --- /dev/null +++ b/src/test/java/org/springframework/data/keyvalue/repository/query/KeyValuePartTreeQueryUnitTests.java @@ -0,0 +1,73 @@ +/* + * Copyright 2015-2016 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 + * + * http://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.keyvalue.repository.query; + +import static org.hamcrest.core.IsNot.*; +import static org.hamcrest.core.IsSame.*; +import static org.junit.Assert.*; +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; + +import java.lang.reflect.Method; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.data.keyvalue.Person; +import org.springframework.data.keyvalue.core.KeyValueOperations; +import org.springframework.data.projection.ProjectionFactory; +import org.springframework.data.repository.core.RepositoryMetadata; +import org.springframework.data.repository.query.DefaultEvaluationContextProvider; +import org.springframework.data.repository.query.QueryMethod; + +/** + * @author Christoph Strobl + */ +@RunWith(MockitoJUnitRunner.class) +public class KeyValuePartTreeQueryUnitTests { + + @Mock KeyValueOperations kvOpsMock; + @Mock RepositoryMetadata metadataMock; + @Mock ProjectionFactory projectionFactoryMock; + + /** + * @see DATAKV-115 + */ + @Test + @SuppressWarnings({ "unchecked", "rawtypes" }) + public void spelExpressionAndContextShouldNotBeReused() throws NoSuchMethodException, SecurityException { + + when(metadataMock.getDomainType()).thenReturn((Class) Person.class); + when(metadataMock.getReturnedDomainClass(any(Method.class))).thenReturn((Class) Person.class); + + QueryMethod qm = new QueryMethod(Repo.class.getMethod("findByFirstname", String.class), metadataMock, + projectionFactoryMock); + + KeyValuePartTreeQuery query = new KeyValuePartTreeQuery(qm, DefaultEvaluationContextProvider.INSTANCE, kvOpsMock, + SpelQueryCreator.class); + + Object[] args = new Object[] { "foo" }; + + assertThat(query.prepareQuery(args).getCritieria(), not(sameInstance(query.prepareQuery(args).getCritieria()))); + } + + static interface Repo { + + List findByFirstname(String firstname); + } +}