From 34f062e8c116b6c6b103f37dad6e116bf7a4dda8 Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 7 Nov 2017 09:29:35 +0000 Subject: [PATCH 1/4] Improve error message for parse failures of completion fields --- .../index/mapper/CompletionFieldMapper.java | 2 +- .../mapper/CompletionFieldMapperTests.java | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java index 1c92150676c19..17400d9c51ddb 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java @@ -560,7 +560,7 @@ private void parse(ParseContext parseContext, Token token, XContentParser parser } } } else { - throw new ElasticsearchParseException("failed to parse expected text or object got" + token.name()); + throw new ElasticsearchParseException("failed to parse [" + parser.currentName() + "][" + parser.getTokenLocation() + "]: expected text or object, but got " + token.name()); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java index 5da524b69c0e6..1214120be0c0d 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java @@ -163,6 +163,28 @@ public void testParsingMinimal() throws Exception { assertSuggestFields(fields, 1); } + public void testParsingFailure() throws Exception { + String mapping = jsonBuilder().startObject().startObject("type1") + .startObject("properties").startObject("completion") + .field("type", "completion") + .endObject().endObject() + .endObject().endObject().string(); + + DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse("type1", new CompressedXContent(mapping)); + + try { + defaultMapper.parse(SourceToParse.source("test", "type1", "1", XContentFactory.jsonBuilder() + .startObject() + .field("completion", 1.0) + .endObject() + .bytes(), + XContentType.JSON)); + fail("Expected parse() to throw an exception, but it did not."); + } catch (MapperParsingException e) { + assertEquals("failed to parse [completion][1:15]: expected text or object, but got VALUE_NUMBER", e.getCause().getMessage()); + } + } + public void testParsingMultiValued() throws Exception { String mapping = jsonBuilder().startObject().startObject("type1") .startObject("properties").startObject("completion") From ab544150b49b29a0c8a831d71e0c2a03cc36665c Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 7 Nov 2017 10:07:10 +0000 Subject: [PATCH 2/4] Use expectThrows --- .../index/mapper/CompletionFieldMapperTests.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java index 1214120be0c0d..e3e9e77e6878a 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java @@ -172,17 +172,14 @@ public void testParsingFailure() throws Exception { DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse("type1", new CompressedXContent(mapping)); - try { + MapperParsingException e = expectThrows(MapperParsingException.class, () -> defaultMapper.parse(SourceToParse.source("test", "type1", "1", XContentFactory.jsonBuilder() .startObject() .field("completion", 1.0) .endObject() .bytes(), - XContentType.JSON)); - fail("Expected parse() to throw an exception, but it did not."); - } catch (MapperParsingException e) { - assertEquals("failed to parse [completion][1:15]: expected text or object, but got VALUE_NUMBER", e.getCause().getMessage()); - } + XContentType.JSON))); + assertEquals("failed to parse [completion][1:15]: expected text or object, but got VALUE_NUMBER", e.getCause().getMessage()); } public void testParsingMultiValued() throws Exception { From 48c8e1d3d07f83a5c4bcd110f3820ffe8b1998f5 Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 7 Nov 2017 10:24:05 +0000 Subject: [PATCH 3/4] Use ParsingException, which has a more standard way to capture the token location --- .../org/elasticsearch/index/mapper/CompletionFieldMapper.java | 3 ++- .../elasticsearch/index/mapper/CompletionFieldMapperTests.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java index 17400d9c51ddb..2a849177d74fd 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java @@ -33,6 +33,7 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.common.util.set.Sets; @@ -560,7 +561,7 @@ private void parse(ParseContext parseContext, Token token, XContentParser parser } } } else { - throw new ElasticsearchParseException("failed to parse [" + parser.currentName() + "][" + parser.getTokenLocation() + "]: expected text or object, but got " + token.name()); + throw new ParsingException(parser.getTokenLocation(), "failed to parse [" + parser.currentName() + "]: expected text or object, but got " + token.name()); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java index e3e9e77e6878a..74183ae864a60 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java @@ -179,7 +179,7 @@ public void testParsingFailure() throws Exception { .endObject() .bytes(), XContentType.JSON))); - assertEquals("failed to parse [completion][1:15]: expected text or object, but got VALUE_NUMBER", e.getCause().getMessage()); + assertEquals("failed to parse [completion]: expected text or object, but got VALUE_NUMBER", e.getCause().getMessage()); } public void testParsingMultiValued() throws Exception { From 4ab04f9402ef08830471e10a42e29b8cbe4a80f7 Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 7 Nov 2017 10:26:02 +0000 Subject: [PATCH 4/4] Unused import --- .../org/elasticsearch/index/mapper/CompletionFieldMapper.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java index 2a849177d74fd..186334c85cb33 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java @@ -30,7 +30,6 @@ import org.apache.lucene.search.suggest.document.PrefixCompletionQuery; import org.apache.lucene.search.suggest.document.RegexCompletionQuery; import org.apache.lucene.search.suggest.document.SuggestField; -import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException;