From b81b24092495657098725f4c8b97ef973ab3da56 Mon Sep 17 00:00:00 2001 From: Areek Zillur Date: Fri, 18 Jul 2014 15:33:56 -0400 Subject: [PATCH] [Fix] CompletionMapper throws misleading error on null value closes #6399 --- .../index/mapper/FieldMapper.java | 2 ++ .../mapper/core/AbstractFieldMapper.java | 5 +++ .../mapper/core/CompletionFieldMapper.java | 5 +++ .../index/mapper/object/ObjectMapper.java | 5 +++ .../suggest/CompletionSuggestSearchTests.java | 32 +++++++++++++++++++ 5 files changed, 49 insertions(+) diff --git a/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index d72eca7e7ec..59a07ad0333 100644 --- a/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -286,6 +286,8 @@ public interface FieldMapper extends Mapper { boolean isSortable(); + boolean supportsNullValue(); + boolean hasDocValues(); Loading normsLoading(Loading defaultLoading); diff --git a/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java index ed951b9cf20..689c9a994f7 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java @@ -842,6 +842,11 @@ public abstract class AbstractFieldMapper implements FieldMapper { return true; } + @Override + public boolean supportsNullValue() { + return true; + } + public boolean hasDocValues() { return docValues; } diff --git a/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java index af8885480f1..3a2f3477f78 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java @@ -451,6 +451,11 @@ public class CompletionFieldMapper extends AbstractFieldMapper { return false; } + @Override + public boolean supportsNullValue() { + return false; + } + @Override public FieldType defaultFieldType() { return Defaults.FIELD_TYPE; diff --git a/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java b/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java index 92249324292..44504a3593d 100644 --- a/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java @@ -532,6 +532,11 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { // we can only handle null values if we have mappings for them Mapper mapper = mappers.get(lastFieldName); if (mapper != null) { + if (mapper instanceof FieldMapper) { + if (!((FieldMapper) mapper).supportsNullValue()) { + throw new MapperParsingException("no object mapping found for null value in [" + lastFieldName + "]"); + } + } mapper.parse(context); } } diff --git a/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java b/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java index 5dbd9858efc..e1879c7f28a 100644 --- a/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java +++ b/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java @@ -1039,6 +1039,38 @@ public class CompletionSuggestSearchTests extends ElasticsearchIntegrationTest { } } + // see issue #6399 + @Test + public void testIndexingUnrelatedNullValue() throws Exception { + String mapping = jsonBuilder() + .startObject() + .startObject(TYPE) + .startObject("properties") + .startObject(FIELD) + .field("type", "completion") + .endObject() + .endObject() + .endObject() + .endObject() + .string(); + + assertAcked(client().admin().indices().prepareCreate(INDEX).addMapping(TYPE, mapping).get()); + ensureGreen(); + + client().prepareIndex(INDEX, TYPE, "1").setSource(FIELD, "strings make me happy", FIELD + "_1", "nulls make me sad") + .setRefresh(true).get(); + + try { + client().prepareIndex(INDEX, TYPE, "2").setSource(FIELD, null, FIELD + "_1", "nulls make me sad") + .setRefresh(true).get(); + fail("Expected MapperParsingException for null value"); + } catch (MapperParsingException e) { + // make sure that the exception has the name of the field causing the error + assertTrue(e.getDetailedMessage().contains(FIELD)); + } + + } + private static String replaceReservedChars(String input, char replacement) { char[] charArray = input.toCharArray(); for (int i = 0; i < charArray.length; i++) {