Skip to content

Commit

Permalink
Re-enable support for array-valued geo_shape fields. (#58786) (#58943)
Browse files Browse the repository at this point in the history 
A regression in the mapping code led to geo_shape no longer supporting
array-valued fields. This commit fixes this support and adds an integration
test to make sure this problem does not return!
  • Loading branch information
@talevy
talevy committed Jul 2, 2020
1 parent d825d43 commit d516959
Show file tree
Hide file tree
Showing 7 changed files with 180 additions and 5 deletions.
5 changes: 0 additions & 5 deletions server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java
View file
Expand Up @@ -286,11 +286,6 @@ protected void parseCreateField(ParseContext context) throws IOException {
protected abstract void addDocValuesFields(String name, Processed geometry, List<IndexableField> fields, ParseContext context);
protected abstract void addMultiFields(ParseContext context, Processed geometry) throws IOException;

@Override
public final boolean parsesArrayValue() {
return true;
}

/** parsing logic for geometry indexing */
@Override
public void parse(ParseContext context) throws IOException {
Expand Down
5 changes: 5 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java
View file
Expand Up @@ -130,6 +130,11 @@ protected AbstractPointGeometryFieldMapper(String simpleName, FieldType fieldTyp
this.nullValue = nullValue;
}

@Override
public final boolean parsesArrayValue() {
return true;
}

@Override
protected void mergeOptions(FieldMapper other, List<String> conflicts) {
AbstractPointGeometryFieldMapper gpfm = (AbstractPointGeometryFieldMapper)other;
Expand Down
5 changes: 5 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java
View file
Expand Up @@ -198,6 +198,11 @@ protected AbstractShapeGeometryFieldMapper(String simpleName, FieldType fieldTyp
this.orientation = orientation;
}

@Override
public final boolean parsesArrayValue() {
return false;
}

@Override
protected final void mergeOptions(FieldMapper other, List<String> conflicts) {
AbstractShapeGeometryFieldMapper gsfm = (AbstractShapeGeometryFieldMapper)other;
Expand Down
41 changes: 41 additions & 0 deletions server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java
View file
Expand Up @@ -18,13 +18,16 @@
*/
package org.elasticsearch.index.mapper;

import org.apache.lucene.index.IndexableField;
import org.elasticsearch.common.Explicit;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.geo.builders.ShapeBuilder;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.InternalSettingsPlugin;
import org.elasticsearch.test.TestGeoShapeFieldMapperPlugin;
Expand All @@ -38,6 +41,7 @@
import static org.elasticsearch.index.mapper.AbstractGeometryFieldMapper.Names.IGNORE_Z_VALUE;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;

public class GeoShapeFieldMapperTests extends FieldMapperTestCase<GeoShapeFieldMapper.Builder> {
Expand Down Expand Up @@ -302,6 +306,43 @@ public void testSerializeDefaults() throws Exception {
}
}

public void testGeoShapeArrayParsing() throws Exception {
String mapping = Strings.toString(XContentFactory.jsonBuilder()
.startObject()
.startObject("_doc")
.startObject("properties")
.startObject("location")
.field("type", "geo_shape")
.endObject()
.endObject()
.endObject()
.endObject());

DocumentMapper mapper = createIndex("test").mapperService().documentMapperParser()
.parse("_doc", new CompressedXContent(mapping));

BytesReference arrayedDoc = BytesReference.bytes(XContentFactory.jsonBuilder()
.startObject()
.startArray("shape")
.startObject()
.field("type", "Point")
.startArray("coordinates").value(176.0).value(15.0).endArray()
.endObject()
.startObject()
.field("type", "Point")
.startArray("coordinates").value(76.0).value(-15.0).endArray()
.endObject()
.endArray()
.endObject()
);

SourceToParse sourceToParse = new SourceToParse("test", "_doc", "1", arrayedDoc, XContentType.JSON);
ParsedDocument document = mapper.parse(sourceToParse);
assertThat(document.docs(), hasSize(1));
IndexableField[] fields = document.docs().get(0).getFields("shape.type");
assertThat(fields.length, equalTo(2));
}

public String toXContentString(GeoShapeFieldMapper mapper, boolean includeDefaults) throws IOException {
XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
ToXContent.Params params;
Expand Down
43 changes: 43 additions & 0 deletions server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java
View file
Expand Up @@ -18,6 +18,7 @@
*/
package org.elasticsearch.index.mapper;

import org.apache.lucene.index.IndexableField;
import org.apache.lucene.spatial.prefix.PrefixTreeStrategy;
import org.apache.lucene.spatial.prefix.RecursivePrefixTreeStrategy;
import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree;
Expand All @@ -26,6 +27,7 @@
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.Explicit;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.geo.GeoUtils;
import org.elasticsearch.common.geo.ShapeRelation;
Expand All @@ -34,6 +36,7 @@
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.geometry.Point;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.plugins.Plugin;
Expand All @@ -49,6 +52,7 @@
import static org.elasticsearch.index.mapper.AbstractGeometryFieldMapper.Names.IGNORE_Z_VALUE;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.not;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -783,6 +787,45 @@ public void testSerialization() throws IOException {
);
}

public void testGeoShapeArrayParsing() throws Exception {
String mapping = Strings.toString(XContentFactory.jsonBuilder()
.startObject()
.startObject("_doc")
.startObject("properties")
.startObject("location")
.field("type", "geo_shape")
.field("tree", "quadtree")
.endObject()
.endObject()
.endObject()
.endObject());

DocumentMapper mapper = createIndex("test").mapperService().documentMapperParser()
.parse("_doc", new CompressedXContent(mapping));

BytesReference arrayedDoc = BytesReference.bytes(XContentFactory.jsonBuilder()
.startObject()
.startArray("shape")
.startObject()
.field("type", "Point")
.startArray("coordinates").value(176.0).value(15.0).endArray()
.endObject()
.startObject()
.field("type", "Point")
.startArray("coordinates").value(76.0).value(-15.0).endArray()
.endObject()
.endArray()
.endObject()
);

SourceToParse sourceToParse = new SourceToParse("test", "_doc", "1", arrayedDoc, XContentType.JSON);
ParsedDocument document = mapper.parse(sourceToParse);
assertThat(document.docs(), hasSize(1));
IndexableField[] fields = document.docs().get(0).getFields("shape.type");
assertThat(fields.length, equalTo(2));
assertFieldWarnings("tree");
}

public String toXContentString(LegacyGeoShapeFieldMapper mapper, boolean includeDefaults) throws IOException {
XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
ToXContent.Params params;
Expand Down
43 changes: 43 additions & 0 deletions ...a/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java
View file
Expand Up @@ -24,20 +24,25 @@
*/
package org.elasticsearch.xpack.spatial.index.mapper;

import org.apache.lucene.index.IndexableField;
import org.elasticsearch.Version;
import org.elasticsearch.common.Explicit;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.geo.builders.ShapeBuilder;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.DocumentMapperParser;
import org.elasticsearch.index.mapper.FieldMapperTestCase;
import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.mapper.SourceToParse;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.InternalSettingsPlugin;
import org.elasticsearch.xpack.core.LocalStateCompositeXPackPlugin;
Expand All @@ -52,6 +57,7 @@
import static org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper.Names.IGNORE_Z_VALUE;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;

public class GeoShapeWithDocValuesFieldMapperTests extends FieldMapperTestCase<GeoShapeWithDocValuesFieldMapper.Builder> {
Expand Down Expand Up @@ -394,6 +400,43 @@ public void testSerializeDocValues() throws IOException {
assertTrue(serialized, serialized.contains("\"doc_values\":" + docValues));
}

public void testGeoShapeArrayParsing() throws Exception {
String mapping = Strings.toString(XContentFactory.jsonBuilder()
.startObject()
.startObject("_doc")
.startObject("properties")
.startObject("location")
.field("type", "geo_shape")
.endObject()
.endObject()
.endObject()
.endObject());

DocumentMapper mapper = createIndex("test").mapperService().documentMapperParser()
.parse("_doc", new CompressedXContent(mapping));

BytesReference arrayedDoc = BytesReference.bytes(XContentFactory.jsonBuilder()
.startObject()
.startArray("shape")
.startObject()
.field("type", "Point")
.startArray("coordinates").value(176.0).value(15.0).endArray()
.endObject()
.startObject()
.field("type", "Point")
.startArray("coordinates").value(76.0).value(-15.0).endArray()
.endObject()
.endArray()
.endObject()
);

SourceToParse sourceToParse = new SourceToParse("test", "_doc", "1", arrayedDoc, XContentType.JSON);
ParsedDocument document = mapper.parse(sourceToParse);
assertThat(document.docs(), hasSize(1));
IndexableField[] fields = document.docs().get(0).getFields("shape.type");
assertThat(fields.length, equalTo(2));
}

public String toXContentString(GeoShapeWithDocValuesFieldMapper mapper, boolean includeDefaults) throws IOException {
XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
ToXContent.Params params;
Expand Down
43 changes: 43 additions & 0 deletions ...ial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java
View file
Expand Up @@ -5,24 +5,30 @@
*/
package org.elasticsearch.xpack.spatial.index.mapper;

import org.apache.lucene.index.IndexableField;
import org.elasticsearch.common.Explicit;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.geo.builders.ShapeBuilder;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.DocumentMapperParser;
import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.mapper.SourceToParse;

import java.io.IOException;
import java.util.Collections;

import static org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper.Names.IGNORE_Z_VALUE;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;

/** testing for {@link org.elasticsearch.xpack.spatial.index.mapper.ShapeFieldMapper} */
Expand Down Expand Up @@ -268,6 +274,43 @@ public void testSerializeDefaults() throws Exception {
}
}

public void testShapeArrayParsing() throws Exception {
String mapping = Strings.toString(XContentFactory.jsonBuilder()
.startObject()
.startObject("_doc")
.startObject("properties")
.startObject("location")
.field("type", "shape")
.endObject()
.endObject()
.endObject()
.endObject());

DocumentMapper mapper = createIndex("test").mapperService().documentMapperParser()
.parse("_doc", new CompressedXContent(mapping));

BytesReference arrayedDoc = BytesReference.bytes(XContentFactory.jsonBuilder()
.startObject()
.startArray("shape")
.startObject()
.field("type", "Point")
.startArray("coordinates").value(176.0).value(15.0).endArray()
.endObject()
.startObject()
.field("type", "Point")
.startArray("coordinates").value(76.0).value(-15.0).endArray()
.endObject()
.endArray()
.endObject()
);

SourceToParse sourceToParse = new SourceToParse("test", "_doc", "1", arrayedDoc, XContentType.JSON);
ParsedDocument document = mapper.parse(sourceToParse);
assertThat(document.docs(), hasSize(1));
IndexableField[] fields = document.docs().get(0).getFields("shape.type");
assertThat(fields.length, equalTo(2));
}

public String toXContentString(ShapeFieldMapper mapper, boolean includeDefaults) throws IOException {
XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
ToXContent.Params params;
Expand Down

0 comments on commit d516959

Please sign in to comment.