From 3b575fd945557a5067f535745dd1db6283fec3c9 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Fri, 22 May 2015 10:32:34 +0200 Subject: [PATCH] Add Lucene query assertions. Missing after latest refactoring: Creation of Lucene query to compare against. --- .../index/query/SimpleQueryStringBuilder.java | 69 ++++++++--- .../index/query/SimpleQueryStringParser.java | 16 +-- .../query/SimpleQueryStringBuilderTest.java | 108 ++++++++++++++++-- .../test/ElasticsearchTestCase.java | 2 - 4 files changed, 153 insertions(+), 42 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java b/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java index 6ab84e53c06e6..1a48753f6d7c1 100644 --- a/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java +++ b/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java @@ -27,11 +27,11 @@ import org.apache.lucene.search.BooleanClause.Occur; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.io.stream.Streamable; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; +import java.util.Collections; import java.util.HashMap; import java.util.Locale; import java.util.Map; @@ -41,7 +41,7 @@ * SimpleQuery is a query parser that acts similar to a query_string * query, but won't throw exceptions for any weird string syntax. */ -public class SimpleQueryStringBuilder extends QueryBuilder implements Streamable {// TODO Doesn't have a boost value so far +public class SimpleQueryStringBuilder extends QueryBuilder {// TODO Doesn't have a boost value so far private Map fields = new HashMap<>(); private String analyzer; private String queryName; @@ -66,6 +66,10 @@ public SimpleQueryStringBuilder(String text) { this.queryText = text; } + public String text() { + return this.queryText; + } + /** * Add a field to run the query against */ @@ -81,7 +85,14 @@ public SimpleQueryStringBuilder field(String field, float boost) { this.fields.put(field, boost); return this; } - + + /** + * Returns the fields including their respective boosts to run the query against. + * */ + public Map fields() { + return Collections.unmodifiableMap(this.fields); + } + /** * Specify an analyzer to use for the query */ @@ -124,16 +135,28 @@ public SimpleQueryStringBuilder queryName(String queryName) { this.queryName = queryName; return this; } + + public String queryName() { + return queryName; + } public SimpleQueryStringBuilder lowercaseExpandedTerms(boolean lowercaseExpandedTerms) { this.settings.lowercaseExpandedTerms(lowercaseExpandedTerms); return this; } + + public boolean lowercaseExpandedTerms() { + return this.settings.lowercaseExpandedTerms; + } public SimpleQueryStringBuilder locale(Locale locale) { this.settings.locale(locale); return this; } + + public Locale locale() { + return this.settings.locale; + } public SimpleQueryStringBuilder lenient(boolean lenient) { this.settings.lenient(lenient); @@ -149,6 +172,10 @@ public SimpleQueryStringBuilder minimumShouldMatch(String minimumShouldMatch) { this.minimumShouldMatch = minimumShouldMatch; return this; } + + public String minimumShouldMatch() { + return minimumShouldMatch; + } @Override public Query toQuery(QueryParseContext parseContext) { @@ -156,9 +183,14 @@ public Query toQuery(QueryParseContext parseContext) { if (queryText == null) { throw new QueryParsingException(parseContext, "[" + parserName() + "] query text missing"); } + // Use the default field (_all) if no fields specified + if (fields.isEmpty()) { + String field = parseContext.defaultField(); + fields.put(field, 1.0F); + } - Analyzer luceneAnalyzer; // Use standard analyzer by default if none specified + Analyzer luceneAnalyzer; if (analyzer == null) { luceneAnalyzer = parseContext.mapperService().searchAnalyzer(); } else { @@ -316,40 +348,41 @@ public boolean equals(Object obj) { } @Override - public void readFrom(StreamInput in) throws IOException { - queryText = in.readOptionalString(); + public SimpleQueryStringBuilder readFrom(StreamInput in) throws IOException { + SimpleQueryStringBuilder qb = new SimpleQueryStringBuilder(in.readOptionalString()); int size = in.readInt(); for (int i = 0; i < size; i++) { String field = in.readString(); Float weight = in.readFloat(); - fields.put(field, weight); + qb.field(field, weight); } - flags = in.readInt(); - analyzer = in.readOptionalString(); + qb.flags(in.readInt()); + qb.analyzer(in.readOptionalString()); String opStr = in.readOptionalString(); if ("and".equals(opStr)) { - operator = Operator.AND; + qb.defaultOperator(Operator.AND); } else if ("or".equals(opStr)) { - operator = Operator.OR; + qb.defaultOperator(Operator.OR); } Boolean value = in.readOptionalBoolean(); - if (value != null) settings.lowercaseExpandedTerms(value); + if (value != null) qb.lowercaseExpandedTerms(value); value = in.readOptionalBoolean(); - if(value != null) settings.lenient(value); + if(value != null) qb.lenient(value); value = in.readOptionalBoolean(); - if (value != null) settings.analyzeWildcard(value); + if (value != null) qb.analyzeWildcard(value); if (in.readBoolean()) { String localeStr = in.readString(); - settings.locale(Locale.forLanguageTag(localeStr)); + qb.locale(Locale.forLanguageTag(localeStr)); } - queryName = in.readOptionalString(); - minimumShouldMatch = in.readOptionalString(); + qb.queryName(in.readOptionalString()); + qb.minimumShouldMatch(in.readOptionalString()); + return qb; } @Override @@ -380,7 +413,7 @@ public void writeTo(StreamOutput out) throws IOException { @Override public int hashCode() { return Objects.hash(getClass(), fields, analyzer, operator, queryText, queryName, minimumShouldMatch, - settings.lowercaseExpandedTerms, settings.lenient, settings.analyzeWildcard, settings.locale); + settings.lowercaseExpandedTerms, settings.lenient, settings.analyzeWildcard, settings.locale.toLanguageTag()); } @Override diff --git a/src/main/java/org/elasticsearch/index/query/SimpleQueryStringParser.java b/src/main/java/org/elasticsearch/index/query/SimpleQueryStringParser.java index 966c6ee660f4b..382e594ea6989 100644 --- a/src/main/java/org/elasticsearch/index/query/SimpleQueryStringParser.java +++ b/src/main/java/org/elasticsearch/index/query/SimpleQueryStringParser.java @@ -28,7 +28,6 @@ import org.elasticsearch.index.query.SimpleQueryStringBuilder.Operator; import java.io.IOException; -import java.util.Collections; import java.util.HashMap; import java.util.Locale; import java.util.Map; @@ -88,7 +87,7 @@ public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOExcept String queryName = null; String field = null; String minimumShouldMatch = null; - Map fieldsAndWeights = null; + Map fieldsAndWeights = new HashMap<>(); Operator defaultOperator = null; String analyzerName = null; int flags = -1; @@ -117,10 +116,6 @@ public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOExcept fField = parser.text(); } - if (fieldsAndWeights == null) { - fieldsAndWeights = new HashMap<>(); - } - if (Regex.isSimpleMatchPattern(fField)) { for (String fieldName : parseContext.mapperService().simpleMatchToIndexNames(fField)) { fieldsAndWeights.put(fieldName, fBoost); @@ -190,15 +185,6 @@ public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOExcept field = currentFieldName; } - // Use the default field (_all) if no fields specified - if (fieldsAndWeights == null) { - field = parseContext.defaultField(); - } - - if (fieldsAndWeights == null) { - fieldsAndWeights = Collections.singletonMap(field, 1.0F); - } - // TODO after switching to writable this should move to a constructor SimpleQueryStringBuilder qb = new SimpleQueryStringBuilder(queryBody); qb.queryName(queryName); diff --git a/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTest.java b/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTest.java index 1877564421fd2..87abc3d8ee7b2 100644 --- a/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTest.java +++ b/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTest.java @@ -19,13 +19,20 @@ package org.elasticsearch.index.query; +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; +import org.apache.lucene.search.TermQuery; import org.elasticsearch.index.query.SimpleQueryStringBuilder.Operator; +import org.junit.Test; import java.io.IOException; import java.util.HashSet; import java.util.Set; +import static org.hamcrest.Matchers.*; + public class SimpleQueryStringBuilderTest extends BaseQueryTestCase { @Override @@ -36,7 +43,7 @@ protected SimpleQueryStringBuilder createTestQueryBuilder() { if (randomBoolean()) qb.lenient(randomBoolean()); if (randomBoolean()) qb.lowercaseExpandedTerms(randomBoolean()); if (randomBoolean()) qb.locale(randomLocale(getRandom())); - if (randomBoolean()) qb.minimumShouldMatch(randomFrom(new String[]{"1", "10%", "0.1"})); + if (randomBoolean()) qb.minimumShouldMatch(randomFrom(new String[]{"1", "-1", "75%", "-25%", "2<75%", "2<-25%"})); if (randomBoolean()) qb.analyzer("simple"); if (randomBoolean()) qb.defaultOperator(randomFrom(Operator.AND, Operator.OR)); @@ -49,17 +56,98 @@ protected SimpleQueryStringBuilder createTestQueryBuilder() { qb.flags(flags.toArray(new SimpleQueryStringFlag[flags.size()])); } - if (randomBoolean()) { - qb.field(randomAsciiOfLength(8)); - } else { - qb.field(randomAsciiOfLength(8), 2.0f / randomIntBetween(1, 20)); + int fieldCount = randomIntBetween(0, 10); + for (int i = 0; i < fieldCount; i++) { + if (randomBoolean()) { + qb.field(randomAsciiOfLength(8)); + } else { + qb.field(randomAsciiOfLength(8), 2.0f / randomIntBetween(1, 20)); + } } return qb; } + // Check operator handling, and default field handling. + @Test + public void testDefaultOperatorHandling() { + SimpleQueryStringBuilder qb = new SimpleQueryStringBuilder("The quick brown fox."); + BooleanQuery boolQuery = (BooleanQuery) qb.toQuery(createContext()); + assertThat(shouldClauses(boolQuery), is(4)); + + qb.defaultOperator(Operator.AND); + boolQuery = (BooleanQuery) qb.toQuery(createContext()); + assertThat(shouldClauses(boolQuery), is(0)); + + qb.defaultOperator(Operator.OR); + boolQuery = (BooleanQuery) qb.toQuery(createContext()); + assertThat(shouldClauses(boolQuery), is(4)); + } + + /* + * This assumes that Lucene query parsing is being checked already, adding checks only for our parsing extensions. + * + * Also this relies on {@link SimpleQueryStringTests} to test most of the actual functionality of query parsing. + */ @Override - protected void assertLuceneQuery(SimpleQueryStringBuilder queryBuilder, Query query, QueryParseContext context) throws IOException { - // Huge TODO + protected void assertLuceneQuery(SimpleQueryStringBuilder queryBuilder, Query query, QueryParseContext context) { + if (queryBuilder.queryName() != null) { + Query namedQuery = context.copyNamedFilters().get(queryBuilder.queryName()); + assertThat(namedQuery, equalTo(query)); + } + + if (query instanceof BooleanQuery) { + assertThat(queryBuilder.fields().size(), greaterThan(1)); + + BooleanQuery boolQuery = (BooleanQuery) query; + if (queryBuilder.lowercaseExpandedTerms()) { + for (BooleanClause clause : boolQuery.clauses()) { + if (clause.getQuery() instanceof TermQuery) { + TermQuery inner = (TermQuery) clause.getQuery(); + assertThat(inner.getTerm().bytes().toString(), is(inner.getTerm().bytes().toString().toLowerCase())); + } + } + } + + if (queryBuilder.minimumShouldMatch() != null) { + if ("1".equals(queryBuilder.minimumShouldMatch())) { + assertThat(boolQuery.getMinimumNumberShouldMatch(), greaterThan(0)); + } else if ("-1".equals(queryBuilder.minimumShouldMatch())) { + assertThat(boolQuery.getMinimumNumberShouldMatch(), greaterThan(0)); + } else if ("75%".equals(queryBuilder.minimumShouldMatch())) { + assertThat(boolQuery.getMinimumNumberShouldMatch(), greaterThan(0)); + } else if ("-25%".equals(queryBuilder.minimumShouldMatch())) { + assertThat(boolQuery.getMinimumNumberShouldMatch(), greaterThan(0)); + } else if ("2<75%".equals(queryBuilder.minimumShouldMatch())) { + if (shouldClauses(boolQuery) > 2) { + assertThat(boolQuery.getMinimumNumberShouldMatch(), greaterThan(0)); + } + } else if ("2<-25%".equals(queryBuilder.minimumShouldMatch())) { + if (shouldClauses(boolQuery) > 2) { + assertThat(boolQuery.getMinimumNumberShouldMatch(), greaterThan(0)); + } + } + } + } else if (query instanceof TermQuery) { + TermQuery termQuery = (TermQuery) query; + assertThat(termQuery.getTerm().field(), is(queryBuilder.fields().keySet().iterator().next())); + + if (queryBuilder.lowercaseExpandedTerms()) { + assertThat(termQuery.getTerm().bytes().toString(), is(termQuery.getTerm().bytes().toString().toLowerCase())); + } + + } else if (! (query instanceof MatchNoDocsQuery)){ + fail("Encountered lucene query type we do not have a validation implementation for in our SimpleQueryStringBuilderTest"); + } + } + + private int shouldClauses(BooleanQuery query) { + int result = 0; + for (BooleanClause c : query.clauses()) { + if (c.getOccur() == BooleanClause.Occur.SHOULD) { + result++; + } + } + return result; } @Override @@ -67,4 +155,10 @@ protected SimpleQueryStringBuilder createEmptyQueryBuilder() { return new SimpleQueryStringBuilder(""); } + @Override + protected Query createExpectedQuery(SimpleQueryStringBuilder queryBuilder, QueryParseContext context) throws IOException { + // TODO + return null; + } + } diff --git a/src/test/java/org/elasticsearch/test/ElasticsearchTestCase.java b/src/test/java/org/elasticsearch/test/ElasticsearchTestCase.java index 844b06032953c..299a60a9715f6 100644 --- a/src/test/java/org/elasticsearch/test/ElasticsearchTestCase.java +++ b/src/test/java/org/elasticsearch/test/ElasticsearchTestCase.java @@ -31,7 +31,6 @@ import com.carrotsearch.randomizedtesting.rules.TestRuleAdapter; import com.google.common.base.Predicate; -import org.apache.commons.lang3.EnumUtils; import org.apache.lucene.uninverting.UninvertingReader; import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.LuceneTestCase.SuppressCodecs; @@ -53,7 +52,6 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.env.Environment; import org.elasticsearch.env.NodeEnvironment; -import org.elasticsearch.index.query.SimpleQueryStringFlag; import org.elasticsearch.test.cache.recycler.MockBigArrays; import org.elasticsearch.test.cache.recycler.MockPageCacheRecycler; import org.elasticsearch.test.junit.listeners.LoggingListener;