honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Enforce query instance checking before it wrapper as a filter 

We have the default QueryWrapperFilter as well as our custom one while
our wrapper is explicitly marked as no_cache such that it will never
be included in a cache. This was not consistenly used and caused several
problems during tests where p/c related queries were used as filters
and ended up in the cache. This commit adds the QueryWrapperFilter
ctor to the forbidden APIs to enforce the query instance checks.
This commit is contained in:
Simon Willnauer 2014-03-14 14:13:06 +01:00
parent 36a0cb99d7
commit 821173b5cf
9 changed files with 52 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
core-signatures.txt
View File

@ -30,3 +30,6 @@ org.apache.lucene.index.IndexWriter#forceMerge(int) @ use Merges#forceMerge
org.apache.lucene.index.IndexWriter#forceMerge(int,boolean) @ use Merges#forceMerge
org.apache.lucene.index.IndexWriter#forceMergeDeletes() @ use Merges#forceMergeDeletes
org.apache.lucene.index.IndexWriter#forceMergeDeletes(boolean) @ use Merges#forceMergeDeletes
@defaultMessage QueryWrapperFilter is cachable by default - use Queries#wrap instead
org.apache.lucene.search.QueryWrapperFilter#<init>(org.apache.lucene.search.Query)

1
pom.xml
View File

@ -989,6 +989,7 @@
<exclude>org/elasticsearch/Version.class</exclude>
<exclude>org/apache/lucene/queries/XTermsFilter.class</exclude>
<exclude>org/elasticsearch/index/merge/Merges.class</exclude>
<exclude>org/elasticsearch/common/lucene/search/Queries$QueryWrapperFilterFactory.class</exclude>
<!-- end excludes for valid system-out -->
<!-- start excludes for Unsafe -->
<exclude>org/elasticsearch/common/util/UnsafeUtils.class</exclude>

25
src/main/java/org/elasticsearch/common/lucene/search/Queries.java
View File

@ -19,11 +19,9 @@
package org.elasticsearch.common.lucene.search;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.Filter;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.*;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.index.search.child.CustomQueryWrappingFilter;
import java.util.List;
import java.util.regex.Pattern;
@ -166,4 +164,23 @@ public class Queries {
optionalClauseCount : (result < 0 ? 0 : result));
}
public static Filter wrap(Query query) {
return FACTORY.wrap(query);
}
private static final QueryWrapperFilterFactory FACTORY = new QueryWrapperFilterFactory();
// NOTE: This is a separate class since we added QueryWrapperFilter as a forbidden API
// that way we can exclude only the inner class without excluding the entire Queries class
// and potentially miss a forbidden API usage!
private static final class QueryWrapperFilterFactory {
public Filter wrap(Query query) {
if (CustomQueryWrappingFilter.shouldUseCustomQueryWrappingFilter(query)) {
return new CustomQueryWrappingFilter(query);
} else {
return new QueryWrapperFilter(query);
}
}
}
}

11
src/main/java/org/elasticsearch/index/query/FQueryFilterParser.java
View File

@ -21,8 +21,8 @@ package org.elasticsearch.index.query;
import org.apache.lucene.search.Filter;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.QueryWrapperFilter;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.cache.filter.support.CacheKeyFilter;
import org.elasticsearch.index.search.child.CustomQueryWrappingFilter;
@ -86,12 +86,7 @@ public class FQueryFilterParser implements FilterParser {
if (query == null) {
return null;
}
Filter filter;
if (CustomQueryWrappingFilter.shouldUseCustomQueryWrappingFilter(query)) {
filter = new CustomQueryWrappingFilter(query);
} else {
filter = new QueryWrapperFilter(query);
}
Filter filter = Queries.wrap(query);
if (cache) {
filter = parseContext.cacheFilter(filter, cacheKey);
}
@ -100,4 +95,4 @@ public class FQueryFilterParser implements FilterParser {
}
return filter;
}
}
}

6
src/main/java/org/elasticsearch/index/query/NestedFilterParser.java
View File

@ -21,11 +21,11 @@ package org.elasticsearch.index.query;
import org.apache.lucene.search.Filter;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.QueryWrapperFilter;
import org.apache.lucene.search.join.ScoreMode;
import org.apache.lucene.search.join.ToParentBlockJoinQuery;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.lucene.search.XConstantScoreQuery;
import org.elasticsearch.common.lucene.search.XFilteredQuery;
import org.elasticsearch.common.xcontent.XContentParser;
@ -154,9 +154,9 @@ public class NestedFilterParser implements FilterParser {
Filter nestedFilter;
if (join) {
ToParentBlockJoinQuery joinQuery = new ToParentBlockJoinQuery(query, parentFilter, ScoreMode.None);
nestedFilter = new QueryWrapperFilter(joinQuery);
nestedFilter = Queries.wrap(joinQuery);
} else {
nestedFilter = new QueryWrapperFilter(query);
nestedFilter = Queries.wrap(query);
}
if (cache) {

9
src/main/java/org/elasticsearch/index/query/QueryFilterParser.java
View File

@ -21,9 +21,8 @@ package org.elasticsearch.index.query;
import org.apache.lucene.search.Filter;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.QueryWrapperFilter;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.index.search.child.CustomQueryWrappingFilter;
import org.elasticsearch.common.lucene.search.Queries;
import java.io.IOException;
@ -49,10 +48,6 @@ public class QueryFilterParser implements FilterParser {
if (query == null) {
return null;
}
if (CustomQueryWrappingFilter.shouldUseCustomQueryWrappingFilter(query)) {
return new CustomQueryWrappingFilter(query);
} else {
return new QueryWrapperFilter(query);
}
return Queries.wrap(query);
}
}

4
src/main/java/org/elasticsearch/index/query/QueryParseContext.java
View File

@ -25,12 +25,12 @@ import org.apache.lucene.queryparser.classic.MapperQueryParser;
import org.apache.lucene.queryparser.classic.QueryParserSettings;
import org.apache.lucene.search.Filter;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.QueryWrapperFilter;
import org.apache.lucene.search.similarities.Similarity;
import org.elasticsearch.cache.recycler.CacheRecycler;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.lucene.search.NoCacheFilter;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.analysis.AnalysisService;
@ -185,7 +185,7 @@ public class QueryParseContext {
}
public void addNamedQuery(String name, Query query) {
namedFilters.put(name, new QueryWrapperFilter(query));
namedFilters.put(name, Queries.wrap(query));
}
public ImmutableMap<String, Filter> copyNamedFilters() {

2
src/main/java/org/elasticsearch/search/facet/query/QueryFacetExecutor.java
View File

@ -51,7 +51,7 @@ public class QueryFacetExecutor extends FacetExecutor {
if (possibleFilter != null) {
this.filter = possibleFilter;
} else {
this.filter = new QueryWrapperFilter(query);
this.filter = Queries.wrap(query);
}
}

25
src/test/java/org/elasticsearch/search/child/SimpleChildQuerySearchTests.java
View File

@ -301,19 +301,26 @@ public class SimpleChildQuerySearchTests extends ElasticsearchIntegrationTest {
.addMapping("parent")
.addMapping("child", "_parent", "type=parent"));
ensureGreen();
List<IndexRequestBuilder> builders = new ArrayList<IndexRequestBuilder>();
// index simple data
for (int i = 0; i < 10; i++) {
client().prepareIndex("test", "parent", Integer.toString(i)).setSource("p_field", i).get();
builders.add(client().prepareIndex("test", "parent", Integer.toString(i)).setSource("p_field", i));
}
for (int i = 0; i < 10; i++) {
client().prepareIndex("test", "child", Integer.toString(i)).setSource("c_field", i).setParent("" + 0).get();
indexRandom(randomBoolean(), builders);
builders.clear();
for (int j = 0; j < 2; j++) {
for (int i = 0; i < 10; i++) {
builders.add(client().prepareIndex("test", "child", Integer.toString(i)).setSource("c_field", i).setParent("" + 0));
}
for (int i = 0; i < 10; i++) {
builders.add(client().prepareIndex("test", "child", Integer.toString(i + 10)).setSource("c_field", i + 10).setParent(Integer.toString(i)));
}
if (randomBoolean()) {
break; // randomly break out and dont' have deletes / updates
}
}
for (int i = 0; i < 10; i++) {
client().prepareIndex("test", "child", Integer.toString(i + 10)).setSource("c_field", i + 10).setParent(Integer.toString(i))
.get();
}
flushAndRefresh();
indexRandom(true, builders);
for (int i = 1; i <= 10; i++) {
logger.info("Round {}", i);