From 7db077c2c6da8d14b1ea141c1db47f8e53e3c0c4 Mon Sep 17 00:00:00 2001 From: Artur Konczak Date: Thu, 17 Sep 2015 09:11:57 +0100 Subject: [PATCH] DATAES-171 - BACKPORT from master - added support for missing query keywords --- pom.xml | 7 + .../core/CriteriaQueryProcessor.java | 68 +++++++-- .../core/ElasticsearchTemplate.java | 38 +++--- .../elasticsearch/core/query/Criteria.java | 74 ++++++++-- .../parser/ElasticsearchQueryCreator.java | 9 +- .../data/elasticsearch/entities/Product.java | 105 ++------------ .../CustomMethodRepositoryTests.java | 4 +- .../repositories/query/ProductRepository.java | 43 ++++++ .../repository/support/QueryKeywordsTest.java | 129 ++++++++++++++++++ .../resources/repository-query-support.xml | 18 +++ 10 files changed, 354 insertions(+), 141 deletions(-) create mode 100644 src/test/java/org/springframework/data/elasticsearch/repositories/query/ProductRepository.java create mode 100644 src/test/java/org/springframework/data/elasticsearch/repository/support/QueryKeywordsTest.java create mode 100644 src/test/resources/repository-query-support.xml diff --git a/pom.xml b/pom.xml index b3ce1ce66..041b11445 100644 --- a/pom.xml +++ b/pom.xml @@ -121,6 +121,13 @@ test + + org.projectlombok + lombok + 1.16.4 + test + + diff --git a/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java b/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java index 739344855..f91d8c348 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java @@ -24,10 +24,11 @@ import java.util.List; import java.util.ListIterator; import org.apache.lucene.queryparser.flexible.core.util.StringUtils; -import org.elasticsearch.index.query.*; -import org.springframework.data.elasticsearch.core.geo.GeoPoint; +import org.elasticsearch.index.query.BoolQueryBuilder; +import org.elasticsearch.index.query.BoostableQueryBuilder; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryStringQueryBuilder; import org.springframework.data.elasticsearch.core.query.Criteria; -import org.springframework.data.geo.Point; import org.springframework.util.Assert; /** @@ -36,6 +37,7 @@ import org.springframework.util.Assert; * @author Rizwan Idrees * @author Mohsin Husen * @author Franck Marchand + * @author Artur Konczak */ class CriteriaQueryProcessor { @@ -49,11 +51,19 @@ class CriteriaQueryProcessor { List mustQueryBuilderList = new LinkedList(); ListIterator chainIterator = criteria.getCriteriaChain().listIterator(); + + QueryBuilder firstQuery = null; + boolean negateFirstQuery = false; + while (chainIterator.hasNext()) { Criteria chainedCriteria = chainIterator.next(); QueryBuilder queryFragmentForCriteria = createQueryFragmentForCriteria(chainedCriteria); - if (queryFragmentForCriteria != null) { + if (firstQuery == null) { + firstQuery = queryFragmentForCriteria; + negateFirstQuery = chainedCriteria.isNegating(); + continue; + } if (chainedCriteria.isOr()) { shouldQueryBuilderList.add(queryFragmentForCriteria); } else if (chainedCriteria.isNegating()) { @@ -64,6 +74,18 @@ class CriteriaQueryProcessor { } } + if (firstQuery != null) { + if (!shouldQueryBuilderList.isEmpty() && mustNotQueryBuilderList.isEmpty() && mustQueryBuilderList.isEmpty()) { + shouldQueryBuilderList.add(0, firstQuery); + } else { + if (negateFirstQuery) { + mustNotQueryBuilderList.add(0, firstQuery); + } else { + mustQueryBuilderList.add(0, firstQuery); + } + } + } + BoolQueryBuilder query = null; if (!shouldQueryBuilderList.isEmpty() || !mustNotQueryBuilderList.isEmpty() || !mustQueryBuilderList.isEmpty()) { @@ -98,12 +120,12 @@ class CriteriaQueryProcessor { if (singeEntryCriteria) { Criteria.CriteriaEntry entry = it.next(); - query = processCriteriaEntry(entry.getKey(), entry.getValue(), fieldName); + query = processCriteriaEntry(entry, fieldName); } else { query = boolQuery(); while (it.hasNext()) { Criteria.CriteriaEntry entry = it.next(); - ((BoolQueryBuilder) query).must(processCriteriaEntry(entry.getKey(), entry.getValue(), fieldName)); + ((BoolQueryBuilder) query).must(processCriteriaEntry(entry, fieldName)); } } @@ -112,14 +134,18 @@ class CriteriaQueryProcessor { } - private QueryBuilder processCriteriaEntry(OperationKey key, Object value, String fieldName) { + private QueryBuilder processCriteriaEntry(Criteria.CriteriaEntry entry,/* OperationKey key, Object value,*/ String fieldName) { + Object value = entry.getValue(); if (value == null) { return null; } + OperationKey key = entry.getKey(); QueryBuilder query = null; String searchText = StringUtils.toString(value); + Iterable collection = null; + switch (key) { case EQUALS: query = queryString(searchText).field(fieldName).defaultOperator(QueryStringQueryBuilder.Operator.AND); @@ -134,24 +160,42 @@ class CriteriaQueryProcessor { query = queryString("*" + searchText).field(fieldName).analyzeWildcard(true); break; case EXPRESSION: - query = queryString((String) value).field(fieldName); + query = queryString(searchText).field(fieldName); + break; + case LESS_EQUAL: + query = rangeQuery(fieldName).lte(value); + break; + case GREATER_EQUAL: + query = rangeQuery(fieldName).gte(value); break; case BETWEEN: Object[] ranges = (Object[]) value; query = rangeQuery(fieldName).from(ranges[0]).to(ranges[1]); break; + case LESS: + query = rangeQuery(fieldName).lt(value); + break; + case GREATER: + query = rangeQuery(fieldName).gt(value); + break; case FUZZY: - query = fuzzyQuery(fieldName, (String) value); + query = fuzzyQuery(fieldName, searchText); break; case IN: query = boolQuery(); - Iterable collection = (Iterable) value; + collection = (Iterable) value; for (Object item : collection) { - ((BoolQueryBuilder) query).should(queryString((String) item).field(fieldName)); + ((BoolQueryBuilder) query).should(queryString(item.toString()).field(fieldName)); + } + break; + case NOT_IN: + query = boolQuery(); + collection = (Iterable) value; + for (Object item : collection) { + ((BoolQueryBuilder) query).mustNot(queryString(item.toString()).field(fieldName)); } break; } - return query; } diff --git a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java index b3db7f126..90209487a 100755 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java @@ -15,6 +15,21 @@ */ package org.springframework.data.elasticsearch.core; +import static org.apache.commons.collections.CollectionUtils.isNotEmpty; +import static org.apache.commons.lang.StringUtils.*; +import static org.elasticsearch.action.search.SearchType.*; +import static org.elasticsearch.client.Requests.*; +import static org.elasticsearch.cluster.metadata.AliasAction.Type.*; +import static org.elasticsearch.common.collect.Sets.*; +import static org.elasticsearch.index.VersionType.*; +import static org.springframework.data.elasticsearch.core.MappingBuilder.*; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.lang.reflect.Method; +import java.util.*; + import org.apache.commons.collections.CollectionUtils; import org.elasticsearch.action.ListenableActionFuture; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; @@ -81,23 +96,6 @@ import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.util.CloseableIterator; import org.springframework.util.Assert; -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStreamReader; -import java.lang.reflect.Method; -import java.util.*; - -import static org.apache.commons.collections.CollectionUtils.isNotEmpty; -import static org.apache.commons.lang.StringUtils.isBlank; -import static org.apache.commons.lang.StringUtils.isNotBlank; -import static org.elasticsearch.action.search.SearchType.SCAN; -import static org.elasticsearch.client.Requests.indicesExistsRequest; -import static org.elasticsearch.client.Requests.refreshRequest; -import static org.elasticsearch.cluster.metadata.AliasAction.Type.ADD; -import static org.elasticsearch.common.collect.Sets.newHashSet; -import static org.elasticsearch.index.VersionType.EXTERNAL; -import static org.springframework.data.elasticsearch.core.MappingBuilder.buildMapping; - /** * ElasticsearchTemplate * @@ -314,6 +312,9 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, Applicati if (elasticsearchFilter != null) searchRequestBuilder.setPostFilter(elasticsearchFilter); + if (logger.isDebugEnabled()) { + logger.debug("doSearch query:\n" + searchRequestBuilder.toString()); + } SearchResponse response = getSearchResponse(searchRequestBuilder .execute()); @@ -401,7 +402,6 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, Applicati } throw new NoSuchElementException(); } - }; } @@ -536,7 +536,7 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, Applicati Assert.notNull(query.getUpdateRequest(), "No IndexRequest define for Query"); UpdateRequestBuilder updateRequestBuilder = client.prepareUpdate(indexName, type, query.getId()); - if(query.getUpdateRequest().script() == null) { + if (query.getUpdateRequest().script() == null) { // doc if (query.DoUpsert()) { updateRequestBuilder.setDocAsUpsert(true) diff --git a/src/main/java/org/springframework/data/elasticsearch/core/query/Criteria.java b/src/main/java/org/springframework/data/elasticsearch/core/query/Criteria.java index 0ee92f413..bd41529e0 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/query/Criteria.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/query/Criteria.java @@ -35,6 +35,17 @@ import org.springframework.util.Assert; */ public class Criteria { + @Override + public String toString() { + return "Criteria{" + + "field=" + field.getName() + + ", boost=" + boost + + ", negating=" + negating + + ", queryCriteria=" + StringUtils.join(queryCriteria, '|') + + ", filterCriteria=" + StringUtils.join(filterCriteria, '|') + + '}'; + } + public static final String WILDCARD = "*"; public static final String CRITERIA_VALUE_SEPERATOR = " "; @@ -71,7 +82,6 @@ public class Criteria { public Criteria(Field field) { Assert.notNull(field, "Field for criteria must not be null"); Assert.hasText(field.getName(), "Field.name for criteria must not be null/empty"); - this.criteriaChain.add(this); this.field = field; } @@ -304,7 +314,18 @@ public class Criteria { * @return */ public Criteria lessThanEqual(Object upperBound) { - between(null, upperBound); + if (upperBound == null) { + throw new InvalidDataAccessApiUsageException("UpperBound can't be null"); + } + queryCriteria.add(new CriteriaEntry(OperationKey.LESS_EQUAL, upperBound)); + return this; + } + + public Criteria lessThan(Object upperBound) { + if (upperBound == null) { + throw new InvalidDataAccessApiUsageException("UpperBound can't be null"); + } + queryCriteria.add(new CriteriaEntry(OperationKey.LESS, upperBound)); return this; } @@ -315,7 +336,18 @@ public class Criteria { * @return */ public Criteria greaterThanEqual(Object lowerBound) { - between(lowerBound, null); + if (lowerBound == null) { + throw new InvalidDataAccessApiUsageException("LowerBound can't be null"); + } + queryCriteria.add(new CriteriaEntry(OperationKey.GREATER_EQUAL, lowerBound)); + return this; + } + + public Criteria greaterThan(Object lowerBound) { + if (lowerBound == null) { + throw new InvalidDataAccessApiUsageException("LowerBound can't be null"); + } + queryCriteria.add(new CriteriaEntry(OperationKey.GREATER, lowerBound)); return this; } @@ -326,12 +358,7 @@ public class Criteria { * @return */ public Criteria in(Object... values) { - if (values.length == 0 || (values.length > 1 && values[1] instanceof Collection)) { - throw new InvalidDataAccessApiUsageException("At least one element " - + (values.length > 0 ? ("of argument of type " + values[1].getClass().getName()) : "") - + " has to be present."); - } - return in(Arrays.asList(values)); + return in(toCollection(values)); } /** @@ -346,6 +373,25 @@ public class Criteria { return this; } + private List toCollection(Object... values) { + if (values.length == 0 || (values.length > 1 && values[1] instanceof Collection)) { + throw new InvalidDataAccessApiUsageException("At least one element " + + (values.length > 0 ? ("of argument of type " + values[1].getClass().getName()) : "") + + " has to be present."); + } + return Arrays.asList(values); + } + + public Criteria notIn(Object... values) { + return notIn(toCollection(values)); + } + + public Criteria notIn(Iterable values) { + Assert.notNull(values, "Collection of 'NotIn' values must not be null"); + queryCriteria.add(new CriteriaEntry(OperationKey.NOT_IN, values)); + return this; + } + /** * Creates new CriteriaEntry for {@code location WITHIN distance} * @@ -522,7 +568,7 @@ public class Criteria { } public enum OperationKey { - EQUALS, CONTAINS, STARTS_WITH, ENDS_WITH, EXPRESSION, BETWEEN, FUZZY, IN, WITHIN, BBOX, NEAR; + EQUALS, CONTAINS, STARTS_WITH, ENDS_WITH, EXPRESSION, BETWEEN, FUZZY, IN, NOT_IN, WITHIN, BBOX, NEAR, LESS, LESS_EQUAL, GREATER, GREATER_EQUAL; } public static class CriteriaEntry { @@ -542,5 +588,13 @@ public class Criteria { public Object getValue() { return value; } + + @Override + public String toString() { + return "CriteriaEntry{" + + "key=" + key + + ", value=" + value + + '}'; + } } } diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java index 8417147d2..9b54f5f59 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java @@ -41,6 +41,7 @@ import org.springframework.data.repository.query.parser.PartTree; * @author Rizwan Idrees * @author Mohsin Husen * @author Franck Marchand + * @author Artur Konczak */ public class ElasticsearchQueryCreator extends AbstractQueryCreator { @@ -112,12 +113,14 @@ public class ElasticsearchQueryCreator extends AbstractQueryCreator getTitle() { - return title; - } - - public void setTitle(List title) { - this.title = title; - } - - public String getDescription() { - return description; - } - - public void setDescription(String description) { - this.description = description; - } - - public List getCategories() { - return categories; - } - - public void setCategories(List categories) { - this.categories = categories; - } - - public Float getWeight() { - return weight; - } - - public void setWeight(Float weight) { - this.weight = weight; - } - - public Float getPrice() { - return price; - } - - public void setPrice(Float price) { - this.price = price; - } - - public Integer getPopularity() { - return popularity; - } - - public void setPopularity(Integer popularity) { - this.popularity = popularity; - } - - public boolean isAvailable() { - return available; - } - - public void setAvailable(boolean available) { - this.available = available; - } - - public String getName() { - return name; - } - - public void setName(String name) { - this.name = name; - } - - public String getLocation() { - return location; - } - - public void setLocation(String location) { - this.location = location; - } - - public Date getLastModified() { - return lastModified; - } - - public void setLastModified(Date lastModified) { - this.lastModified = lastModified; - } @Override public int hashCode() { @@ -166,12 +89,4 @@ public class Product { } return true; } - - public String getText() { - return text; - } - - public void setText(String text) { - this.text = text; - } } diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/CustomMethodRepositoryTests.java b/src/test/java/org/springframework/data/elasticsearch/repositories/CustomMethodRepositoryTests.java index 0ab5f9fc6..9137268a2 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/CustomMethodRepositoryTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/CustomMethodRepositoryTests.java @@ -122,7 +122,7 @@ public class CustomMethodRepositoryTests { SampleEntity sampleEntity = new SampleEntity(); sampleEntity.setId(documentId); sampleEntity.setType("test"); - sampleEntity.setRate(10); + sampleEntity.setRate(9); sampleEntity.setMessage("some message"); repository.save(sampleEntity); @@ -753,7 +753,7 @@ public class CustomMethodRepositoryTests { SampleEntity sampleEntity = new SampleEntity(); sampleEntity.setId(documentId); sampleEntity.setType("test"); - sampleEntity.setRate(10); + sampleEntity.setRate(9); sampleEntity.setMessage("some message"); repository.save(sampleEntity); diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/query/ProductRepository.java b/src/test/java/org/springframework/data/elasticsearch/repositories/query/ProductRepository.java new file mode 100644 index 000000000..57c07b67e --- /dev/null +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/query/ProductRepository.java @@ -0,0 +1,43 @@ +package org.springframework.data.elasticsearch.repositories.query; + +import java.util.List; + +import org.springframework.data.elasticsearch.entities.Product; +import org.springframework.data.repository.PagingAndSortingRepository; + +/** + * Created by akonczak on 04/09/15. + */ +public interface ProductRepository extends PagingAndSortingRepository { + + public List findByNameAndText(String name, String text); + + public List findByNameAndPrice(String name, Float price); + + public List findByNameOrText(String name, String text); + + public List findByNameOrPrice(String name, Float price); + + + public List findByAvailableTrue(); + + public List findByAvailableFalse(); + + public List findByPriceIn(List floats); + + public List findByPriceNotIn(List floats); + + public List findByPriceNot(float v); + + public List findByPriceBetween(float v, float v1); + + public List findByPriceLessThan(float v); + + public List findByPriceLessThanEqual(float v); + + public List findByPriceGreaterThan(float v); + + public List findByPriceGreaterThanEqual(float v); + + public List findByIdNotIn(List strings); +} diff --git a/src/test/java/org/springframework/data/elasticsearch/repository/support/QueryKeywordsTest.java b/src/test/java/org/springframework/data/elasticsearch/repository/support/QueryKeywordsTest.java new file mode 100644 index 000000000..cefeb3c99 --- /dev/null +++ b/src/test/java/org/springframework/data/elasticsearch/repository/support/QueryKeywordsTest.java @@ -0,0 +1,129 @@ +package org.springframework.data.elasticsearch.repository.support; + +import static org.hamcrest.core.Is.*; +import static org.junit.Assert.*; + +import java.util.Arrays; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.elasticsearch.core.ElasticsearchTemplate; +import org.springframework.data.elasticsearch.entities.Product; +import org.springframework.data.elasticsearch.repositories.query.ProductRepository; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +/** + * @author Artur Konczak + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration("classpath:/repository-query-support.xml") +public class QueryKeywordsTest { + + @Autowired + private ProductRepository repository; + + @Autowired + private ElasticsearchTemplate elasticsearchTemplate; + + @Before + public void before() { + elasticsearchTemplate.deleteIndex(Product.class); + elasticsearchTemplate.createIndex(Product.class); + elasticsearchTemplate.putMapping(Product.class); + elasticsearchTemplate.refresh(Product.class, true); + + repository.save(Arrays.asList( + Product.builder().id("1").name("Sugar").text("Cane sugar").price(1.0f).available(false).build() + , Product.builder().id("2").name("Sugar").text("Cane sugar").price(1.2f).available(true).build() + , Product.builder().id("3").name("Sugar").text("Beet sugar").price(1.1f).available(true).build() + , Product.builder().id("4").name("Salt").text("Rock salt").price(1.9f).available(true).build() + , Product.builder().id("5").name("Salt").text("Sea salt").price(2.1f).available(false).build())); + + elasticsearchTemplate.refresh(Product.class, true); + } + + @Test + public void shouldSupportAND() { + //given + + //when + + //then + assertThat(repository.findByNameAndText("Sugar", "Cane sugar").size(), is(2)); + assertThat(repository.findByNameAndPrice("Sugar", 1.1f).size(), is(1)); + } + + @Test + public void shouldSupportOR() { + //given + + //when + + //then + assertThat(repository.findByNameOrPrice("Sugar", 1.9f).size(), is(4)); + assertThat(repository.findByNameOrText("Salt", "Beet sugar").size(), is(3)); + } + + @Test + public void shouldSupportTrueAndFalse() { + //given + + //when + + //then + assertThat(repository.findByAvailableTrue().size(), is(3)); + assertThat(repository.findByAvailableFalse().size(), is(2)); + } + + @Test + public void shouldSupportInAndNotInAndNot() { + //given + + //when + + //then + assertThat(repository.findByPriceIn(Arrays.asList(1.2f, 1.1f)).size(), is(2)); + assertThat(repository.findByPriceNotIn(Arrays.asList(1.2f, 1.1f)).size(), is(3)); + assertThat(repository.findByPriceNot(1.2f).size(), is(4)); + } + + /* + DATAES-171 + */ + @Test + public void shouldWorkWithNotIn() { + //given + + //when + + //then + assertThat(repository.findByIdNotIn(Arrays.asList("2", "3")).size(), is(3)); + } + + @Test + public void shouldSupportBetween() { + //given + + //when + + //then + assertThat(repository.findByPriceBetween(1.0f, 2.0f).size(), is(4)); + } + + @Test + public void shouldSupportLessThanAndGreaterThan() { + //given + + //when + + //then + assertThat(repository.findByPriceLessThan(1.1f).size(), is(1)); + assertThat(repository.findByPriceLessThanEqual(1.1f).size(), is(2)); + + assertThat(repository.findByPriceGreaterThan(1.9f).size(), is(1)); + assertThat(repository.findByPriceGreaterThanEqual(1.9f).size(), is(2)); + } +} diff --git a/src/test/resources/repository-query-support.xml b/src/test/resources/repository-query-support.xml new file mode 100644 index 000000000..ebc9ee8b4 --- /dev/null +++ b/src/test/resources/repository-query-support.xml @@ -0,0 +1,18 @@ + + + + + + + + + + + + + \ No newline at end of file