From 2aee9a241cf300736b41d0c36287c05a1bbed744 Mon Sep 17 00:00:00 2001 From: markramach Date: Wed, 6 Jul 2016 15:44:13 -0500 Subject: [PATCH] DATACOUCH-237 - Prevent groupby on generated count queries Fixes #119 --- .../query/N1qlCountQueryCreator.java | 125 +++++++++++++++++ .../query/PartTreeN1qlBasedQuery.java | 4 +- .../query/PartTreeN1qBasedQueryTest.java | 128 ++++++++++++++++++ 3 files changed, 255 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/springframework/data/couchbase/repository/query/N1qlCountQueryCreator.java create mode 100644 src/test/java/org/springframework/data/couchbase/repository/query/PartTreeN1qBasedQueryTest.java diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/N1qlCountQueryCreator.java b/src/main/java/org/springframework/data/couchbase/repository/query/N1qlCountQueryCreator.java new file mode 100644 index 00000000..e457252d --- /dev/null +++ b/src/main/java/org/springframework/data/couchbase/repository/query/N1qlCountQueryCreator.java @@ -0,0 +1,125 @@ +/* + * Copyright 2012-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.couchbase.repository.query; + +import java.util.Iterator; + +import org.springframework.data.couchbase.core.convert.CouchbaseConverter; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; +import org.springframework.data.repository.query.ParameterAccessor; +import org.springframework.data.repository.query.parser.PartTree; + +import com.couchbase.client.java.query.dsl.Expression; +import com.couchbase.client.java.query.dsl.path.LimitPath; +import com.couchbase.client.java.query.dsl.path.WherePath; + +/** + * + * @author Mark Ramach + * + */ +public class N1qlCountQueryCreator extends N1qlQueryCreator { + + public N1qlCountQueryCreator(PartTree tree, ParameterAccessor parameters, WherePath selectFrom, + CouchbaseConverter converter, CouchbaseQueryMethod queryMethod) { + super(tree, new CountParameterAccessor(parameters), selectFrom, converter, queryMethod); + } + + @Override + protected LimitPath complete(Expression criteria, Sort sort) { + // Sorting is not allowed on aggregate count queries. + return super.complete(criteria, null); + } + + private static class CountParameterAccessor implements ParameterAccessor { + + private ParameterAccessor delegate; + + public CountParameterAccessor(ParameterAccessor delegate) { + this.delegate = delegate; + } + + public Pageable getPageable() { + return delegate.getPageable() != null ? new CountPageable(delegate.getPageable()) : null; + } + + public Sort getSort() { + return null; + } + + public Class getDynamicProjection() { + return delegate.getDynamicProjection(); + } + + public Object getBindableValue(int index) { + return delegate.getBindableValue(index); + } + + public boolean hasBindableNullValue() { + return delegate.hasBindableNullValue(); + } + + public Iterator iterator() { + return delegate.iterator(); + } + + } + + private static class CountPageable implements Pageable { + + private Pageable delegate; + + public CountPageable(Pageable delegate) { + this.delegate = delegate; + } + + public int getPageNumber() { + return delegate.getPageNumber(); + } + + public int getPageSize() { + return delegate.getPageSize(); + } + + public int getOffset() { + return delegate.getOffset(); + } + + public Sort getSort() { + // Sorting is not allowed on aggregate count queries. + return null; + } + + public Pageable next() { + return delegate.next(); + } + + public Pageable previousOrFirst() { + return delegate.previousOrFirst(); + } + + public Pageable first() { + return delegate.first(); + } + + public boolean hasPrevious() { + return delegate.hasPrevious(); + } + + } + +} diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/PartTreeN1qlBasedQuery.java b/src/main/java/org/springframework/data/couchbase/repository/query/PartTreeN1qlBasedQuery.java index f57c1b28..08ab5b16 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/PartTreeN1qlBasedQuery.java +++ b/src/main/java/org/springframework/data/couchbase/repository/query/PartTreeN1qlBasedQuery.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors + * Copyright 2012-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. @@ -58,7 +58,7 @@ public class PartTreeN1qlBasedQuery extends AbstractN1qlBasedQuery { Expression bucket = i(getCouchbaseOperations().getCouchbaseBucket().name()); WherePath countFrom = select(count("*").as(CountFragment.COUNT_ALIAS)).from(bucket); - N1qlQueryCreator queryCreator = new N1qlQueryCreator(partTree, accessor, countFrom, + N1qlQueryCreator queryCreator = new N1qlCountQueryCreator(partTree, accessor, countFrom, getCouchbaseOperations().getConverter(), getQueryMethod()); return queryCreator.createQuery(); } diff --git a/src/test/java/org/springframework/data/couchbase/repository/query/PartTreeN1qBasedQueryTest.java b/src/test/java/org/springframework/data/couchbase/repository/query/PartTreeN1qBasedQueryTest.java new file mode 100644 index 00000000..01d39760 --- /dev/null +++ b/src/test/java/org/springframework/data/couchbase/repository/query/PartTreeN1qBasedQueryTest.java @@ -0,0 +1,128 @@ +package org.springframework.data.couchbase.repository.query; + +import static org.junit.Assert.*; +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; + +import java.lang.reflect.Method; +import java.util.Arrays; + +import org.junit.Test; +import org.springframework.core.convert.converter.Converter; +import org.springframework.data.couchbase.core.Beer; +import org.springframework.data.couchbase.core.CouchbaseOperations; +import org.springframework.data.couchbase.core.convert.CouchbaseConverter; +import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; +import org.springframework.data.domain.Sort.Direction; +import org.springframework.data.mapping.PropertyPath; +import org.springframework.data.mapping.context.MappingContext; +import org.springframework.data.mapping.context.PersistentPropertyPath; +import org.springframework.data.projection.ProjectionFactory; +import org.springframework.data.repository.CrudRepository; +import org.springframework.data.repository.core.EntityMetadata; +import org.springframework.data.repository.core.RepositoryMetadata; +import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; +import org.springframework.data.repository.query.ParameterAccessor; + +import com.couchbase.client.java.CouchbaseBucket; +import com.couchbase.client.java.query.Statement; + +public class PartTreeN1qBasedQueryTest { + + @Test + public void testGetCountExcludesStaticSortClause() throws Exception { + + PageRequest pr = new PageRequest(0, 10); + + CouchbaseOperations couchbaseOperations = mock(CouchbaseOperations.class); + CouchbaseBucket couchbaseBucket = mock(CouchbaseBucket.class); + CouchbaseConverter couchbaseConverter = mock(CouchbaseConverter.class); + MappingContext mappingContext = mock(MappingContext.class); + PersistentPropertyPath persistentPropertyPath = mock(PersistentPropertyPath.class); + CouchbasePersistentProperty leafProperty = mock(CouchbasePersistentProperty.class); + EntityMetadata entityInformation = mock(EntityMetadata.class); + ParameterAccessor accessor = mock(ParameterAccessor.class); + ProjectionFactory factory = mock(ProjectionFactory.class); + + Method method = TestRepository.class.getMethod("findByNameOrderByName", String.class, Pageable.class); + RepositoryMetadata metadata = new DefaultRepositoryMetadata(TestRepository.class); + + CouchbaseQueryMethod queryMethod = new CouchbaseQueryMethod(method, metadata, factory, mappingContext); + + when(entityInformation.getJavaType()).thenReturn(Beer.class); + when(couchbaseOperations.getCouchbaseBucket()).thenReturn(couchbaseBucket); + when(couchbaseBucket.name()).thenReturn("default"); + when(couchbaseOperations.getConverter()).thenReturn(couchbaseConverter); + when(couchbaseConverter.getMappingContext()).thenReturn(mappingContext); + when(mappingContext.getPersistentPropertyPath(isA(PropertyPath.class))).thenReturn(persistentPropertyPath); + when(persistentPropertyPath.toDotPath(isA(Converter.class))).thenReturn("name"); + when(persistentPropertyPath.getLeafProperty()).thenReturn(leafProperty); + when(leafProperty.getType()).thenReturn((Class) String.class); + when(accessor.iterator()).thenReturn(Arrays.asList((Object) "value", pr).iterator()); + when(couchbaseConverter.getTypeKey()).thenReturn("_class"); + when(couchbaseConverter.convertForWriteIfNeeded(eq("value"))).thenReturn("value"); + + PartTreeN1qlBasedQuery query = new PartTreeN1qlBasedQuery(queryMethod, couchbaseOperations); + Statement statement = query.getCount(accessor, new Object[] { "value", pr }); + + assertEquals("SELECT COUNT(*) AS count FROM `default` WHERE name = \"value\" " + + "AND `_class` = \"org.springframework.data.couchbase.core.Beer\"", statement.toString()); + + } + + @Test + public void testGetCountExcludesDynamicSortClause() throws Exception { + + Sort sort = new Sort(Direction.ASC, "name"); + PageRequest pr = new PageRequest(0, 10, sort); + + CouchbaseOperations couchbaseOperations = mock(CouchbaseOperations.class); + CouchbaseBucket couchbaseBucket = mock(CouchbaseBucket.class); + CouchbaseConverter couchbaseConverter = mock(CouchbaseConverter.class); + MappingContext mappingContext = mock(MappingContext.class); + PersistentPropertyPath persistentPropertyPath = mock(PersistentPropertyPath.class); + CouchbasePersistentProperty leafProperty = mock(CouchbasePersistentProperty.class); + EntityMetadata entityInformation = mock(EntityMetadata.class); + ParameterAccessor accessor = mock(ParameterAccessor.class); + ProjectionFactory factory = mock(ProjectionFactory.class); + + Method method = TestRepository.class.getMethod("findByName", String.class, Pageable.class); + RepositoryMetadata metadata = new DefaultRepositoryMetadata(TestRepository.class); + + CouchbaseQueryMethod queryMethod = new CouchbaseQueryMethod(method, metadata, factory, mappingContext); + + when(entityInformation.getJavaType()).thenReturn(Beer.class); + when(couchbaseOperations.getCouchbaseBucket()).thenReturn(couchbaseBucket); + when(couchbaseBucket.name()).thenReturn("default"); + when(couchbaseOperations.getConverter()).thenReturn(couchbaseConverter); + when(couchbaseConverter.getMappingContext()).thenReturn(mappingContext); + when(mappingContext.getPersistentPropertyPath(isA(PropertyPath.class))).thenReturn(persistentPropertyPath); + when(persistentPropertyPath.toDotPath(isA(Converter.class))).thenReturn("name"); + when(persistentPropertyPath.getLeafProperty()).thenReturn(leafProperty); + when(leafProperty.getType()).thenReturn((Class) String.class); + when(accessor.iterator()).thenReturn(Arrays.asList((Object) "value", pr).iterator()); + when(accessor.getSort()).thenReturn(sort); + when(couchbaseConverter.getTypeKey()).thenReturn("_class"); + when(couchbaseConverter.convertForWriteIfNeeded(eq("value"))).thenReturn("value"); + + PartTreeN1qlBasedQuery query = new PartTreeN1qlBasedQuery(queryMethod, couchbaseOperations); + Statement statement = query.getCount(accessor, new Object[] { "value", pr }); + + assertEquals("SELECT COUNT(*) AS count FROM `default` WHERE name = \"value\" " + + "AND `_class` = \"org.springframework.data.couchbase.core.Beer\"", statement.toString()); + + } + + public static interface TestRepository extends CrudRepository { + + Page findByNameOrderByName(String name, Pageable pageRequest); + + Page findByName(String name, Pageable pageRequest); + + } + +}