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.

Sign up for GitHub

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

Jump to bottom

Return positions of parse errors found in JSON #10837

Closed
wants to merge 6 commits into from
Closed

Return positions of parse errors found in JSON #10837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0540597
Base changes - adding location information into query parsing exceptions
markharwood Apr 27, 2015
a383561
Added Junit serialisation test , changed QueryParsingException to hol…
markharwood Apr 27, 2015
4041365
Added parser position support into the SearchParseException class and…
markharwood Apr 27, 2015
7a7d7e1
Minor changes from review comments
markharwood Apr 28, 2015
2df6e6d
Changed QueryParsingException to take QueryParseContext in constructo…
markharwood Apr 28, 2015
90fc249
Moved TestQueryParsingException out as a standalone class in same pac…
markharwood Apr 29, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/main/java/org/elasticsearch/common/xcontent/XContentLocation.java
View file
Open in desktop
@@ -0,0 +1,37 @@
/*
* 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.common.xcontent;

/**
* Simple data structure representing the line and column number of a position
* in some XContent e.g. JSON. Locations are typically used to communicate the
* position of a parsing error to end users and consequently have line and
* column numbers starting from 1.
*/
public class XContentLocation {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have some javadocs here and please make the class an all members final. I wonder if we should not even have getters here and make it a very simple struct

public final int lineNumber;
public final int columnNumber;

public XContentLocation(int lineNumber, int columnNumber) {
super();
this.lineNumber = lineNumber;
this.columnNumber = columnNumber;
}
}
8 changes: 8 additions & 0 deletions src/main/java/org/elasticsearch/common/xcontent/XContentParser.java
View file
Open in desktop
Expand Up @@ -241,4 +241,12 @@ enum NumberType {
*
*/
byte[] binaryValue() throws IOException;

/**
* Used for error reporting to highlight where syntax errors occur in
* content being parsed.
*
* @return last token's location or null if cannot be determined
*/
XContentLocation getTokenLocation();
}
12 changes: 12 additions & 0 deletions src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java
View file
Open in desktop
Expand Up @@ -19,11 +19,14 @@

package org.elasticsearch.common.xcontent.json;

import com.fasterxml.jackson.core.JsonLocation;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;

import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.ElasticsearchIllegalStateException;
import org.elasticsearch.common.xcontent.XContentLocation;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.common.xcontent.support.AbstractXContentParser;

Expand Down Expand Up @@ -188,6 +191,15 @@ public byte[] binaryValue() throws IOException {
return parser.getBinaryValue();
}

@Override
public XContentLocation getTokenLocation() {
JsonLocation loc = parser.getTokenLocation();
if (loc == null) {
return null;
}
return new XContentLocation(loc.getLineNr(), loc.getColumnNr());
}

@Override
public void close() {
IOUtils.closeWhileHandlingException(parser);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/elasticsearch/index/percolator/PercolatorQueriesRegistry.java
View file
Open in desktop
Expand Up @@ -223,7 +223,7 @@ private Query parseQuery(String type, XContentParser parser) {
context.setMapUnmappedFieldAsString(mapUnmappedFieldsAsString ? true : false);
return queryParserService.parseInnerQuery(context);
} catch (IOException e) {
throw new QueryParsingException(queryParserService.index(), "Failed to parse", e);
throw new QueryParsingException(context, "Failed to parse", e);
} finally {
if (type != null) {
QueryParseContext.setTypes(previousTypes);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/elasticsearch/index/query/AndFilterParser.java
View file
Open in desktop
Expand Up @@ -100,14 +100,14 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
} else if ("_cache_key".equals(currentFieldName) || "_cacheKey".equals(currentFieldName)) {
cacheKey = new HashedBytesRef(parser.text());
} else {
throw new QueryParsingException(parseContext.index(), "[and] filter does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[and] filter does not support [" + currentFieldName + "]");
}
}
}
}

if (!filtersFound) {
throw new QueryParsingException(parseContext.index(), "[and] filter requires 'filters' to be set on it'");
throw new QueryParsingException(parseContext, "[and] filter requires 'filters' to be set on it'");
}

if (filters.isEmpty()) {
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/elasticsearch/index/query/BoolFilterParser.java
View file
Open in desktop
Expand Up @@ -85,7 +85,7 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
boolFilter.add(new BooleanClause(filter, BooleanClause.Occur.SHOULD));
}
} else {
throw new QueryParsingException(parseContext.index(), "[bool] filter does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[bool] filter does not support [" + currentFieldName + "]");
}
} else if (token == XContentParser.Token.START_ARRAY) {
if ("must".equals(currentFieldName)) {
Expand Down Expand Up @@ -114,7 +114,7 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
}
}
} else {
throw new QueryParsingException(parseContext.index(), "[bool] filter does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[bool] filter does not support [" + currentFieldName + "]");
}
} else if (token.isValue()) {
if ("_cache".equals(currentFieldName)) {
Expand All @@ -124,13 +124,13 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
} else if ("_cache_key".equals(currentFieldName) || "_cacheKey".equals(currentFieldName)) {
cacheKey = new HashedBytesRef(parser.text());
} else {
throw new QueryParsingException(parseContext.index(), "[bool] filter does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[bool] filter does not support [" + currentFieldName + "]");
}
}
}

if (!hasAnyFilter) {
throw new QueryParsingException(parseContext.index(), "[bool] filter has no inner should/must/must_not elements");
throw new QueryParsingException(parseContext, "[bool] filter has no inner should/must/must_not elements");
}

if (boolFilter.clauses().isEmpty()) {
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/elasticsearch/index/query/BoolQueryParser.java
View file
Open in desktop
Expand Up @@ -85,7 +85,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
clauses.add(new BooleanClause(query, BooleanClause.Occur.SHOULD));
}
} else {
throw new QueryParsingException(parseContext.index(), "[bool] query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[bool] query does not support [" + currentFieldName + "]");
}
} else if (token == XContentParser.Token.START_ARRAY) {
if ("must".equals(currentFieldName)) {
Expand All @@ -110,7 +110,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
}
}
} else {
throw new QueryParsingException(parseContext.index(), "bool query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "bool query does not support [" + currentFieldName + "]");
}
} else if (token.isValue()) {
if ("disable_coord".equals(currentFieldName) || "disableCoord".equals(currentFieldName)) {
Expand All @@ -126,7 +126,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
} else if ("_name".equals(currentFieldName)) {
queryName = parser.text();
} else {
throw new QueryParsingException(parseContext.index(), "[bool] query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[bool] query does not support [" + currentFieldName + "]");
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/org/elasticsearch/index/query/BoostingQueryParser.java
View file
Open in desktop
Expand Up @@ -66,27 +66,27 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
negativeQuery = parseContext.parseInnerQuery();
negativeQueryFound = true;
} else {
throw new QueryParsingException(parseContext.index(), "[boosting] query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[boosting] query does not support [" + currentFieldName + "]");
}
} else if (token.isValue()) {
if ("negative_boost".equals(currentFieldName) || "negativeBoost".equals(currentFieldName)) {
negativeBoost = parser.floatValue();
} else if ("boost".equals(currentFieldName)) {
boost = parser.floatValue();
} else {
throw new QueryParsingException(parseContext.index(), "[boosting] query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[boosting] query does not support [" + currentFieldName + "]");
}
}
}

if (positiveQuery == null && !positiveQueryFound) {
throw new QueryParsingException(parseContext.index(), "[boosting] query requires 'positive' query to be set'");
throw new QueryParsingException(parseContext, "[boosting] query requires 'positive' query to be set'");
}
if (negativeQuery == null && !negativeQueryFound) {
throw new QueryParsingException(parseContext.index(), "[boosting] query requires 'negative' query to be set'");
throw new QueryParsingException(parseContext, "[boosting] query requires 'negative' query to be set'");
}
if (negativeBoost == -1) {
throw new QueryParsingException(parseContext.index(), "[boosting] query requires 'negative_boost' to be set'");
throw new QueryParsingException(parseContext, "[boosting] query requires 'negative_boost' to be set'");
}

// parsers returned null
Expand Down
19 changes: 10 additions & 9 deletions src/main/java/org/elasticsearch/index/query/CommonTermsQueryParser.java
View file
Open in desktop
Expand Up @@ -66,7 +66,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
XContentParser parser = parseContext.parser();
XContentParser.Token token = parser.nextToken();
if (token != XContentParser.Token.FIELD_NAME) {
throw new QueryParsingException(parseContext.index(), "[common] query malformed, no field");
throw new QueryParsingException(parseContext, "[common] query malformed, no field");
}
String fieldName = parser.currentName();
Object value = null;
Expand Down Expand Up @@ -97,20 +97,21 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
} else if ("high_freq".equals(innerFieldName) || "highFreq".equals(innerFieldName)) {
highFreqMinimumShouldMatch = parser.text();
} else {
throw new QueryParsingException(parseContext.index(), "[common] query does not support [" + innerFieldName + "] for [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[common] query does not support [" + innerFieldName
+ "] for [" + currentFieldName + "]");
}
}
}
} else {
throw new QueryParsingException(parseContext.index(), "[common] query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[common] query does not support [" + currentFieldName + "]");
}
} else if (token.isValue()) {
if ("query".equals(currentFieldName)) {
value = parser.objectText();
} else if ("analyzer".equals(currentFieldName)) {
String analyzer = parser.text();
if (parseContext.analysisService().analyzer(analyzer) == null) {
throw new QueryParsingException(parseContext.index(), "[common] analyzer [" + parser.text() + "] not found");
throw new QueryParsingException(parseContext, "[common] analyzer [" + parser.text() + "] not found");
}
queryAnalyzer = analyzer;
} else if ("disable_coord".equals(currentFieldName) || "disableCoord".equals(currentFieldName)) {
Expand All @@ -124,7 +125,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
} else if ("and".equalsIgnoreCase(op)) {
highFreqOccur = BooleanClause.Occur.MUST;
} else {
throw new QueryParsingException(parseContext.index(),
throw new QueryParsingException(parseContext,
"[common] query requires operator to be either 'and' or 'or', not [" + op + "]");
}
} else if ("low_freq_operator".equals(currentFieldName) || "lowFreqOperator".equals(currentFieldName)) {
Expand All @@ -134,7 +135,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
} else if ("and".equalsIgnoreCase(op)) {
lowFreqOccur = BooleanClause.Occur.MUST;
} else {
throw new QueryParsingException(parseContext.index(),
throw new QueryParsingException(parseContext,
"[common] query requires operator to be either 'and' or 'or', not [" + op + "]");
}
} else if ("minimum_should_match".equals(currentFieldName) || "minimumShouldMatch".equals(currentFieldName)) {
Expand All @@ -144,7 +145,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
} else if ("_name".equals(currentFieldName)) {
queryName = parser.text();
} else {
throw new QueryParsingException(parseContext.index(), "[common] query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[common] query does not support [" + currentFieldName + "]");
}
}
}
Expand All @@ -155,13 +156,13 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
token = parser.nextToken();
if (token != XContentParser.Token.END_OBJECT) {
throw new QueryParsingException(
parseContext.index(),
parseContext,
"[common] query parsed in simplified form, with direct field name, but included more options than just the field name, possibly use its 'options' form, with 'query' element?");
}
}

if (value == null) {
throw new QueryParsingException(parseContext.index(), "No text specified for text query");
throw new QueryParsingException(parseContext, "No text specified for text query");
}
FieldMapper<?> mapper = null;
String field;
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/elasticsearch/index/query/ConstantScoreQueryParser.java
View file
Open in desktop
Expand Up @@ -71,7 +71,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
query = parseContext.parseInnerQuery();
queryFound = true;
} else {
throw new QueryParsingException(parseContext.index(), "[constant_score] query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[constant_score] query does not support [" + currentFieldName + "]");
}
} else if (token.isValue()) {
if ("boost".equals(currentFieldName)) {
Expand All @@ -81,12 +81,12 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
} else if ("_cache_key".equals(currentFieldName) || "_cacheKey".equals(currentFieldName)) {
cacheKey = new HashedBytesRef(parser.text());
} else {
throw new QueryParsingException(parseContext.index(), "[constant_score] query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[constant_score] query does not support [" + currentFieldName + "]");
}
}
}
if (!filterFound && !queryFound) {
throw new QueryParsingException(parseContext.index(), "[constant_score] requires either 'filter' or 'query' element");
throw new QueryParsingException(parseContext, "[constant_score] requires either 'filter' or 'query' element");
}

if (query == null && filter == null) {
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/elasticsearch/index/query/DisMaxQueryParser.java
View file
Open in desktop
Expand Up @@ -70,7 +70,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
queries.add(query);
}
} else {
throw new QueryParsingException(parseContext.index(), "[dis_max] query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[dis_max] query does not support [" + currentFieldName + "]");
}
} else if (token == XContentParser.Token.START_ARRAY) {
if ("queries".equals(currentFieldName)) {
Expand All @@ -83,7 +83,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
token = parser.nextToken();
}
} else {
throw new QueryParsingException(parseContext.index(), "[dis_max] query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[dis_max] query does not support [" + currentFieldName + "]");
}
} else {
if ("boost".equals(currentFieldName)) {
Expand All @@ -93,13 +93,13 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
} else if ("_name".equals(currentFieldName)) {
queryName = parser.text();
} else {
throw new QueryParsingException(parseContext.index(), "[dis_max] query does not support [" + currentFieldName + "]");
throw new QueryParsingException(parseContext, "[dis_max] query does not support [" + currentFieldName + "]");
}
}
}

if (!queriesFound) {
throw new QueryParsingException(parseContext.index(), "[dis_max] requires 'queries' field");
throw new QueryParsingException(parseContext, "[dis_max] requires 'queries' field");
}

if (queries.isEmpty()) {
Expand Down