Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactors WrapperQueryBuilder and Parser #12037

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -556,8 +556,8 @@ public static WrapperQueryBuilder wrapperQuery(BytesReference source) {
/**
* A Query builder which allows building a query thanks to a JSON string or binary data.
*/
public static WrapperQueryBuilder wrapperQuery(byte[] source, int offset, int length) {
return new WrapperQueryBuilder(source, offset, length);
public static WrapperQueryBuilder wrapperQuery(byte[] source) {
return new WrapperQueryBuilder(source);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,16 @@
package org.elasticsearch.index.query;

import com.google.common.base.Charsets;

import org.apache.lucene.search.Query;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;

import java.io.IOException;
import java.util.Arrays;

/**
* A Query builder which allows building a query given JSON string or binary data provided as input. This is useful when you want
Expand All @@ -44,46 +49,86 @@ public class WrapperQueryBuilder extends AbstractQueryBuilder<WrapperQueryBuilde

public static final String NAME = "wrapper";
private final byte[] source;
private final int offset;
private final int length;
static final WrapperQueryBuilder PROTOTYPE = new WrapperQueryBuilder(null, -1, -1);
static final WrapperQueryBuilder PROTOTYPE = new WrapperQueryBuilder((byte[]) null);

/**
* Creates a query builder given a query provided as a string
*/
public WrapperQueryBuilder(String source) {
this.source = source.getBytes(Charsets.UTF_8);
this.offset = 0;
this.length = this.source.length;
}

/**
* Creates a query builder given a query provided as a bytes array
*/
public WrapperQueryBuilder(byte[] source, int offset, int length) {
public WrapperQueryBuilder(byte[] source) {
this.source = source;
this.offset = offset;
this.length = length;
}

/**
* Creates a query builder given a query provided as a {@link BytesReference}
*/
public WrapperQueryBuilder(BytesReference source) {
this.source = source.array();
this.offset = source.arrayOffset();
this.length = source.length();
}

public byte[] source() {
return this.source;
}

@Override
public String getName() {
return NAME;
}

@Override
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(NAME);
builder.field("query", source, offset, length);
builder.field("query", source);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment I added to the original discussion on the outdated diff.

builder.endObject();
}

@Override
public String getWriteableName() {
return NAME;
}

@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
try (XContentParser qSourceParser = XContentFactory.xContent(source).createParser(source)) {
final QueryParseContext parseContext = new QueryParseContext(context);
parseContext.reset(qSourceParser);
QueryBuilder queryBuilder = parseContext.parseInnerQueryBuilder();
return queryBuilder.toQuery(context);
}
}

@Override
public QueryValidationException validate() {
QueryValidationException validationException = null;
if (this.source == null || this.source.length == 0) {
validationException = addValidationError("query source text cannot be null or empty", validationException);
}
return validationException;
}

@Override
protected WrapperQueryBuilder doReadFrom(StreamInput in) throws IOException {
return new WrapperQueryBuilder(in.readByteArray());
}

@Override
protected void doWriteTo(StreamOutput out) throws IOException {
out.writeByteArray(this.source);
}

@Override
protected int doHashCode() {
return Arrays.hashCode(source);
}

@Override
protected boolean doEquals(WrapperQueryBuilder other) {
return Arrays.equals(source, other.source); // otherwise we compare pointers
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,15 @@

package org.elasticsearch.index.query;

import org.apache.lucene.search.Query;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;

import java.io.IOException;

/**
* Query parser for JSON Queries.
*/
public class WrapperQueryParser extends BaseQueryParserTemp {
public class WrapperQueryParser extends BaseQueryParser {

@Inject
public WrapperQueryParser() {
Expand All @@ -41,8 +39,7 @@ public String[] names() {
}

@Override
public Query parse(QueryShardContext context) throws IOException, QueryParsingException {
QueryParseContext parseContext = context.parseContext();
public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
XContentParser parser = parseContext.parser();

XContentParser.Token token = parser.nextToken();
Expand All @@ -55,15 +52,14 @@ public Query parse(QueryShardContext context) throws IOException, QueryParsingEx
}
parser.nextToken();

byte[] querySource = parser.binaryValue();
try (XContentParser qSourceParser = XContentFactory.xContent(querySource).createParser(querySource)) {
final QueryShardContext contextCopy = new QueryShardContext(context.index(), context.indexQueryParserService());
contextCopy.reset(qSourceParser);
Query result = contextCopy.parseContext().parseInnerQuery();
parser.nextToken();
context.combineNamedQueries(contextCopy);
return result;
byte[] source = parser.binaryValue();

parser.nextToken();

if (source == null) {
throw new QueryParsingException(parseContext, "wrapper query has no [query] specified");
}
return new WrapperQueryBuilder(source);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you 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.elasticsearch.index.query;

import org.apache.lucene.search.Query;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.junit.Test;

import java.io.IOException;

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;

public class WrapperQueryBuilderTest extends BaseQueryTestCase<WrapperQueryBuilder> {

@Override
protected boolean supportsBoostAndQueryName() {
return false;
}

@Override
protected WrapperQueryBuilder doCreateTestQueryBuilder() {
QueryBuilder wrappedQuery = RandomQueryBuilder.createQuery(random());
switch (randomInt(2)) {
case 0:
return new WrapperQueryBuilder(wrappedQuery.toString());
case 1:
return new WrapperQueryBuilder(wrappedQuery.buildAsBytes().toBytes());
case 2:
return new WrapperQueryBuilder(wrappedQuery.buildAsBytes());
default:
throw new UnsupportedOperationException();
}
}

@Override
protected void doAssertLuceneQuery(WrapperQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
try (XContentParser qSourceParser = XContentFactory.xContent(queryBuilder.source()).createParser(queryBuilder.source())) {
final QueryParseContext parseContext = new QueryParseContext(context);
parseContext.reset(qSourceParser);
Query expected = parseContext.parseInnerQueryBuilder().toQuery(context);
if (expected != null) {
expected.setBoost(AbstractQueryBuilder.DEFAULT_BOOST);
}
assertThat(query, equalTo(expected));
}
}

@Test
public void testValidate() {
WrapperQueryBuilder wrapperQueryBuilder = new WrapperQueryBuilder((byte[]) null);
assertThat(wrapperQueryBuilder.validate().validationErrors().size(), is(1));

wrapperQueryBuilder = new WrapperQueryBuilder("");
assertThat(wrapperQueryBuilder.validate().validationErrors().size(), is(1));
}
}
6 changes: 6 additions & 0 deletions docs/reference/migration/migrate_query_refactoring.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ Removed the setter `queryName(String queryName)` since this field is not support
in this type of query. Use `FQueryFilterBuilder.queryName(String queryName)` instead
when in need to wrap a named query as a filter.

==== WrapperQueryBuilder

Removed `wrapperQueryBuilder(byte[] source, int offset, int length)`. Instead simply
use `wrapperQueryBuilder(byte[] source)`. Updated the static factory methods in
QueryBuilders accordingly.

==== Operator

Removed the enums called `Operator` from `MatchQueryBuilder`, `QueryStringQueryBuilder`,
Expand Down