diff --git a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java index 83d9a8178ca5a..ab9e4f5b29f79 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java +++ b/server/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.Version; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.settings.Settings; @@ -74,7 +73,7 @@ *
  • "min_input_length": 50 (default)
    • *
  • "contexts" : CONTEXTS
    • * - * see {@link ContextMappings#load(Object, Version)} for CONTEXTS
    + * see {@link ContextMappings#load(Object, org.elasticsearch.index.mapper.Mapper.TypeParser.ParserContext)} for CONTEXTS
    * see {@link #parse(ParseContext)} for acceptable inputs for indexing
    *

    * This field type constructs completion queries that are run @@ -144,7 +143,7 @@ public static class TypeParser implements Mapper.TypeParser { builder.maxInputLength(Integer.parseInt(fieldNode.toString())); iterator.remove(); } else if (Fields.CONTEXTS.match(fieldName, LoggingDeprecationHandler.INSTANCE)) { - builder.contextMappings(ContextMappings.load(fieldNode, parserContext.indexVersionCreated())); + builder.contextMappings(ContextMappings.load(fieldNode, parserContext)); iterator.remove(); } else if (parseMultiField(builder, name, parserContext, fieldName, fieldNode)) { iterator.remove(); @@ -376,7 +375,7 @@ public Builder maxInputLength(int maxInputLength) { /** * Add context mapping to this field - * @param contextMappings see {@link ContextMappings#load(Object, Version)} + * @param contextMappings see {@link ContextMappings#load(Object, org.elasticsearch.index.mapper.Mapper.TypeParser.ParserContext)} */ public Builder contextMappings(ContextMappings contextMappings) { this.contextMappings = contextMappings; diff --git a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/CategoryContextMapping.java b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/CategoryContextMapping.java index 073e7da3accb2..1df288449473a 100644 --- a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/CategoryContextMapping.java +++ b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/CategoryContextMapping.java @@ -24,11 +24,11 @@ import org.apache.lucene.document.StoredField; import org.apache.lucene.index.IndexableField; import org.elasticsearch.ElasticsearchParseException; -import org.elasticsearch.Version; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser.Token; import org.elasticsearch.index.mapper.KeywordFieldMapper; +import org.elasticsearch.index.mapper.Mapper; import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.index.mapper.ParseContext.Document; import org.elasticsearch.index.mapper.StringFieldType; @@ -79,7 +79,7 @@ public String getFieldName() { /** * Loads a named {@link CategoryContextMapping} instance * from a map. - * see {@link ContextMappings#load(Object, Version)} + * see {@link ContextMappings#load(Object, Mapper.TypeParser.ParserContext)} * * Acceptable map param: path */ @@ -134,7 +134,7 @@ public Set parseContext(ParseContext parseContext, XContentParser } @Override - public Set parseContext(Document document) { + public Set parseContext(Mapper.TypeParser.ParserContext parserContext, Document document) { Set values = null; if (fieldName != null) { IndexableField[] fields = document.getFields(fieldName); diff --git a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMapping.java b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMapping.java index 1aa82eeb2190a..f98659fe2f8c2 100644 --- a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMapping.java +++ b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMapping.java @@ -28,6 +28,7 @@ import org.elasticsearch.common.xcontent.XContentParser.Token; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.index.mapper.CompletionFieldMapper; +import org.elasticsearch.index.mapper.Mapper; import org.elasticsearch.index.mapper.ParseContext; import java.io.IOException; @@ -96,7 +97,7 @@ public String name() { /** * Retrieves a set of context from a document at index-time. */ - protected abstract Set parseContext(ParseContext.Document document); + protected abstract Set parseContext(Mapper.TypeParser.ParserContext parserContext, ParseContext.Document document); /** * Prototype for the query context diff --git a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java index 3c0f0e80cebdb..c41f521fb71ed 100644 --- a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java +++ b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.mapper.CompletionFieldMapper; import org.elasticsearch.index.mapper.DocumentMapperParser; +import org.elasticsearch.index.mapper.Mapper; import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.search.suggest.completion.context.ContextMapping.Type; @@ -54,8 +55,9 @@ public class ContextMappings implements ToXContent { private final List> contextMappings; private final Map> contextNameMap; + private final Mapper.TypeParser.ParserContext parserContext; - public ContextMappings(List> contextMappings) { + public ContextMappings(List> contextMappings, Mapper.TypeParser.ParserContext parserContext) { if (contextMappings.size() > 255) { // we can support more, but max of 255 (1 byte) unique context types per suggest field // seems reasonable? @@ -66,6 +68,7 @@ public ContextMappings(List> contextMappings) { for (ContextMapping mapping : contextMappings) { contextNameMap.put(mapping.name(), mapping); } + this.parserContext = parserContext; } /** @@ -134,7 +137,7 @@ protected Iterable contexts() { scratch.setCharAt(0, (char) typeId); scratch.setLength(1); ContextMapping mapping = contextMappings.get(typeId); - Set contexts = new HashSet<>(mapping.parseContext(document)); + Set contexts = new HashSet<>(mapping.parseContext(parserContext, document)); if (this.contexts.get(mapping.name()) != null) { contexts.addAll(this.contexts.get(mapping.name())); } @@ -216,23 +219,24 @@ public Map> getNamedContexts(List contex * [{"name": .., "type": .., ..}, {..}] * */ - public static ContextMappings load(Object configuration, Version indexVersionCreated) throws ElasticsearchParseException { + public static ContextMappings load(Object configuration, Mapper.TypeParser.ParserContext parserContext) + throws ElasticsearchParseException { final List> contextMappings; if (configuration instanceof List) { contextMappings = new ArrayList<>(); List configurations = (List) configuration; for (Object contextConfig : configurations) { - contextMappings.add(load((Map) contextConfig, indexVersionCreated)); + contextMappings.add(load((Map) contextConfig, parserContext.indexVersionCreated())); } if (contextMappings.size() == 0) { throw new ElasticsearchParseException("expected at least one context mapping"); } } else if (configuration instanceof Map) { - contextMappings = Collections.singletonList(load(((Map) configuration), indexVersionCreated)); + contextMappings = Collections.singletonList(load(((Map) configuration), parserContext.indexVersionCreated())); } else { throw new ElasticsearchParseException("expected a list or an entry of context mapping"); } - return new ContextMappings(contextMappings); + return new ContextMappings(contextMappings, parserContext); } private static ContextMapping load(Map contextConfig, Version indexVersionCreated) { diff --git a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/GeoContextMapping.java b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/GeoContextMapping.java index 48aaf705099da..9b2fcd09908c8 100644 --- a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/GeoContextMapping.java +++ b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/GeoContextMapping.java @@ -19,8 +19,8 @@ package org.elasticsearch.search.suggest.completion.context; -import org.apache.lucene.document.StringField; -import org.apache.lucene.index.DocValuesType; +import org.apache.lucene.document.LatLonDocValuesField; +import org.apache.lucene.document.LatLonPoint; import org.apache.lucene.index.IndexableField; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.geo.GeoPoint; @@ -31,6 +31,7 @@ import org.elasticsearch.common.xcontent.XContentParser.Token; import org.elasticsearch.index.mapper.GeoPointFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.index.mapper.Mapper; import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.index.mapper.ParseContext.Document; @@ -180,35 +181,19 @@ public Set parseContext(ParseContext parseContext, XContentParser } @Override - public Set parseContext(Document document) { + public Set parseContext(Mapper.TypeParser.ParserContext parserContext, Document document) { final Set geohashes = new HashSet<>(); - if (fieldName != null) { + MappedFieldType fieldType = parserContext.mapperService().fullName(fieldName); + if (!(fieldType instanceof GeoPointFieldMapper.GeoPointFieldType)) { + throw new ElasticsearchParseException("cannot parse geo context field [{}], it must be mapped as a geo_point", fieldName); + } IndexableField[] fields = document.getFields(fieldName); GeoPoint spare = new GeoPoint(); - if (fields.length == 0) { - IndexableField[] lonFields = document.getFields(fieldName + ".lon"); - IndexableField[] latFields = document.getFields(fieldName + ".lat"); - if (lonFields.length > 0 && latFields.length > 0) { - for (int i = 0; i < lonFields.length; i++) { - IndexableField lonField = lonFields[i]; - IndexableField latField = latFields[i]; - assert lonField.fieldType().docValuesType() == latField.fieldType().docValuesType(); - // we write doc values fields differently: one field for all values, so we need to only care about indexed fields - if (lonField.fieldType().docValuesType() == DocValuesType.NONE) { - spare.reset(latField.numericValue().doubleValue(), lonField.numericValue().doubleValue()); - geohashes.add(stringEncode(spare.getLon(), spare.getLat(), precision)); - } - } - } - } else { - for (IndexableField field : fields) { - if (field instanceof StringField) { - spare.resetFromString(field.stringValue()); - } else { - // todo return this to .stringValue() once LatLonPoint implements it - spare.resetFromIndexableField(field); - } + for (IndexableField field : fields) { + if (field instanceof LatLonPoint || field instanceof LatLonDocValuesField){ + // todo return this to .stringValue() once LatLonPoint implements it + spare.resetFromIndexableField(field); geohashes.add(spare.geohash()); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldTypeTests.java index 587ac2e0605cc..0d9e93ef5f29e 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldTypeTests.java @@ -52,7 +52,8 @@ public void modify(MappedFieldType ft) { @Override public void modify(MappedFieldType ft) { CompletionFieldMapper.CompletionFieldType cft = (CompletionFieldMapper.CompletionFieldType)ft; - ContextMappings contextMappings = new ContextMappings(Arrays.asList(ContextBuilder.category("foo").build(), ContextBuilder.geo("geo").build())); + ContextMappings contextMappings = new ContextMappings( + Arrays.asList(ContextBuilder.category("foo").build(), ContextBuilder.geo("geo").build()), null); cft.setContextMappings(contextMappings); } }); diff --git a/server/src/test/java/org/elasticsearch/search/suggest/ContextCompletionSuggestSearchIT.java b/server/src/test/java/org/elasticsearch/search/suggest/ContextCompletionSuggestSearchIT.java index d95db778a6a3f..40c197d42582c 100644 --- a/server/src/test/java/org/elasticsearch/search/suggest/ContextCompletionSuggestSearchIT.java +++ b/server/src/test/java/org/elasticsearch/search/suggest/ContextCompletionSuggestSearchIT.java @@ -20,6 +20,7 @@ import com.carrotsearch.randomizedtesting.generators.RandomStrings; import org.apache.lucene.util.LuceneTestCase.SuppressCodecs; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.action.search.SearchResponse; @@ -493,14 +494,19 @@ public void testGeoNeighbours() throws Exception { } public void testGeoField() throws Exception { -// Version version = VersionUtils.randomVersionBetween(random(), Version.V_2_0_0, Version.V_5_0_0_alpha5); -// Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, version).build(); XContentBuilder mapping = jsonBuilder(); mapping.startObject(); mapping.startObject(TYPE); mapping.startObject("properties"); mapping.startObject("pin"); mapping.field("type", "geo_point"); + // Enable store and disable indexing sometimes + if (randomBoolean()) { + mapping.field("store", "true"); + } + if (randomBoolean()) { + mapping.field("index", "false"); + } mapping.endObject(); mapping.startObject(FIELD); mapping.field("type", "completion"); @@ -551,6 +557,97 @@ public void testGeoField() throws Exception { assertEquals("Hotel Amsterdam in Berlin", searchResponse.getSuggest().getSuggestion(suggestionName).iterator().next().getOptions().iterator().next().getText().string()); } + public void testMalformedGeoField() throws Exception { + XContentBuilder mapping = jsonBuilder(); + mapping.startObject(); + mapping.startObject(TYPE); + mapping.startObject("properties"); + mapping.startObject("pin"); + String type = randomFrom("text", "keyword", "long", "object"); + mapping.field("type", type); + mapping.endObject(); + mapping.startObject(FIELD); + mapping.field("type", "completion"); + mapping.field("analyzer", "simple"); + + mapping.startArray("contexts"); + mapping.startObject(); + mapping.field("name", "st"); + mapping.field("type", "geo"); + mapping.field("path", "pin"); + mapping.field("precision", 5); + mapping.endObject(); + mapping.endArray(); + + mapping.endObject(); + mapping.endObject(); + mapping.endObject(); + mapping.endObject(); + + assertAcked(prepareCreate(INDEX).addMapping(TYPE, mapping)); + + if ("object".equals(type)) { + XContentBuilder source1 = jsonBuilder() + .startObject() + .startObject("pin") + .field("type", "shape") + .endObject() + .startObject(FIELD) + .array("input", "Hotel Amsterdam in Berlin") + .endObject() + .endObject(); + assertThat(expectThrows(ElasticsearchParseException.class, () -> + client().prepareIndex(INDEX, TYPE, "1").setSource(source1).execute().actionGet()).getMessage(), + equalTo("cannot parse geo context field [pin], it must be mapped as a geo_point")); + + XContentBuilder source2 = jsonBuilder() + .startObject() + .startObject("pin") + .field("lat", 10.0) + .endObject() + .startObject(FIELD) + .array("input", "Hotel Berlin in Amsterdam") + .endObject() + .endObject(); + assertThat(expectThrows(ElasticsearchParseException.class, () -> + client().prepareIndex(INDEX, TYPE, "2").setSource(source2).execute().actionGet()).getMessage(), + equalTo("cannot parse geo context field [pin], it must be mapped as a geo_point")); + } else if ("long".equals(type)) { + XContentBuilder source2 = jsonBuilder() + .startObject() + .field("pin", 1000) + .startObject(FIELD) + .array("input", "Hotel Berlin in Amsterdam") + .endObject() + .endObject(); + assertThat(expectThrows(ElasticsearchParseException.class, () -> + client().prepareIndex(INDEX, TYPE, "2").setSource(source2).execute().actionGet()).getMessage(), + equalTo("cannot parse geo context field [pin], it must be mapped as a geo_point")); + } else { + XContentBuilder source1 = jsonBuilder() + .startObject() + .field("pin", "52.529172, 13.407333") + .startObject(FIELD) + .array("input", "Hotel Amsterdam in Berlin") + .endObject() + .endObject(); + assertThat(expectThrows(ElasticsearchParseException.class, () -> + client().prepareIndex(INDEX, TYPE, "1").setSource(source1).execute().actionGet()).getMessage(), + equalTo("cannot parse geo context field [pin], it must be mapped as a geo_point")); + + XContentBuilder source2 = jsonBuilder() + .startObject() + .field("pin", "u173zhryfg5n") + .startObject(FIELD) + .array("input", "Hotel Berlin in Amsterdam") + .endObject() + .endObject(); + assertThat(expectThrows(ElasticsearchParseException.class, () -> + client().prepareIndex(INDEX, TYPE, "2").setSource(source2).execute().actionGet()).getMessage(), + equalTo("cannot parse geo context field [pin], it must be mapped as a geo_point")); + } + } + public void testSkipDuplicatesWithContexts() throws Exception { LinkedHashMap> map = new LinkedHashMap<>(); map.put("type", ContextBuilder.category("type").field("type").build()); diff --git a/server/src/test/java/org/elasticsearch/search/suggest/completion/CategoryContextMappingTests.java b/server/src/test/java/org/elasticsearch/search/suggest/completion/CategoryContextMappingTests.java index f9b252f0e136b..4593d267d9e3e 100644 --- a/server/src/test/java/org/elasticsearch/search/suggest/completion/CategoryContextMappingTests.java +++ b/server/src/test/java/org/elasticsearch/search/suggest/completion/CategoryContextMappingTests.java @@ -724,7 +724,7 @@ public void testParsingContextFromDocument() throws Exception { document.add(new Field(keyword.name(), new BytesRef("category1"), keyword)); // Ignore doc values document.add(new SortedSetDocValuesField(keyword.name(), new BytesRef("category1"))); - Set context = mapping.parseContext(document); + Set context = mapping.parseContext(null, document); assertThat(context.size(), equalTo(1)); assertTrue(context.contains("category1")); @@ -735,23 +735,23 @@ public void testParsingContextFromDocument() throws Exception { document.add(new Field(text.name(), "category1", text)); // Ignore stored field document.add(new StoredField(text.name(), "category1", text)); - context = mapping.parseContext(document); + context = mapping.parseContext(null, document); assertThat(context.size(), equalTo(1)); assertTrue(context.contains("category1")); document = new ParseContext.Document(); document.add(new SortedSetDocValuesField("category", new BytesRef("category"))); - context = mapping.parseContext(document); + context = mapping.parseContext(null, document); assertThat(context.size(), equalTo(0)); document = new ParseContext.Document(); document.add(new SortedDocValuesField("category", new BytesRef("category"))); - context = mapping.parseContext(document); + context = mapping.parseContext(null, document); assertThat(context.size(), equalTo(0)); final ParseContext.Document doc = new ParseContext.Document(); doc.add(new IntPoint("category", 36)); - IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> mapping.parseContext(doc)); + IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> mapping.parseContext(null, doc)); assertThat(exc.getMessage(), containsString("Failed to parse context field [category]")); } diff --git a/server/src/test/java/org/elasticsearch/search/suggest/completion/CompletionSuggesterBuilderTests.java b/server/src/test/java/org/elasticsearch/search/suggest/completion/CompletionSuggesterBuilderTests.java index 88e6ce6466622..c873378d14714 100644 --- a/server/src/test/java/org/elasticsearch/search/suggest/completion/CompletionSuggesterBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/suggest/completion/CompletionSuggesterBuilderTests.java @@ -167,7 +167,7 @@ protected void mutateSpecificParameters(CompletionSuggestionBuilder builder) thr protected MappedFieldType mockFieldType(String fieldName) { CompletionFieldType completionFieldType = new CompletionFieldType(); completionFieldType.setName(fieldName); - completionFieldType.setContextMappings(new ContextMappings(contextMappings)); + completionFieldType.setContextMappings(new ContextMappings(contextMappings, null)); return completionFieldType; } @@ -180,7 +180,7 @@ protected void assertSuggestionContext(CompletionSuggestionBuilder builder, Sugg assertEquals(builder.fuzzyOptions, completionSuggestionCtx.getFuzzyOptions()); Map> parsedContextBytes; parsedContextBytes = CompletionSuggestionBuilder.parseContextBytes(builder.contextBytes, xContentRegistry(), - new ContextMappings(contextMappings)); + new ContextMappings(contextMappings, null)); Map> queryContexts = completionSuggestionCtx.getQueryContexts(); assertEquals(parsedContextBytes.keySet(), queryContexts.keySet()); for (String contextName : queryContexts.keySet()) {