Skip to content

Commit

Permalink
[GEO] Fix validate_* merge policy for GeoPointFieldMapper
Browse files Browse the repository at this point in the history 
Fail merge if validate_lat or validate_lon values are not equal. This will prevent inconsistencies between geo_points in a merged index, and parse exceptions for bounding_box and distance filters.

Also merged separate GeoPoint test classes into a single GeoPointFieldMapperTest to be consistent with GeoShapeFieldMapperTests.

closes #10164
  • Loading branch information
@nknize
nknize committed Mar 24, 2015
1 parent 7f092bd commit 7b6364d
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 145 deletions.
10 changes: 5 additions & 5 deletions src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java
View file
Expand Up @@ -637,11 +637,11 @@ public void merge(Mapper mergeWith, MergeContext mergeContext) throws MergeMappi
if (!Objects.equal(this.precisionStep, fieldMergeWith.precisionStep)) {
mergeContext.addConflict("mapper [" + names.fullName() + "] has different precision_step");
}


if (!mergeContext.mergeFlags().simulate()) {
this.validateLat = fieldMergeWith.validateLat;
this.validateLon = fieldMergeWith.validateLon;
if (this.validateLat != fieldMergeWith.validateLat) {
mergeContext.addConflict("mapper [" + names.fullName() + "] has different validate_lat");
}
if (this.validateLon != fieldMergeWith.validateLon) {
mergeContext.addConflict("mapper [" + names.fullName() + "] has different validate_lon");
}
}

Expand Down
182 changes: 135 additions & 47 deletions ...apper/geo/LatLonMappingGeoPointTests.java → .../mapper/geo/GeoPointFieldMapperTests.java
View file
Expand Up @@ -16,23 +16,123 @@
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.index.mapper.geo;

import org.elasticsearch.common.geo.GeoHashUtils;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.DocumentMapperParser;
import org.elasticsearch.index.mapper.MapperParsingException;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.test.ElasticsearchSingleNodeTest;
import org.junit.Test;

import java.util.ArrayList;
import java.util.Arrays;

import static org.elasticsearch.index.mapper.DocumentMapper.MergeFlags.mergeFlags;
import static org.hamcrest.Matchers.*;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;

/**
*
*/
public class LatLonMappingGeoPointTests extends ElasticsearchSingleNodeTest {
public class GeoPointFieldMapperTests extends ElasticsearchSingleNodeTest {
@Test
public void testLatLonValues() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).endObject().endObject()
.endObject().endObject().string();

DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);

ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder()
.startObject()
.startObject("point").field("lat", 1.2).field("lon", 1.3).endObject()
.endObject()
.bytes());

assertThat(doc.rootDoc().getField("point.lat"), notNullValue());
assertThat(doc.rootDoc().getField("point.lat").fieldType().stored(), is(false));
assertThat(doc.rootDoc().getField("point.lon"), notNullValue());
assertThat(doc.rootDoc().getField("point.lon").fieldType().stored(), is(false));
assertThat(doc.rootDoc().getField("point.geohash"), nullValue());
assertThat(doc.rootDoc().get("point"), equalTo("1.2,1.3"));
}

@Test
public void testLatLonValuesWithGeohash() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).field("geohash", true).endObject().endObject()
.endObject().endObject().string();

DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);

ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder()
.startObject()
.startObject("point").field("lat", 1.2).field("lon", 1.3).endObject()
.endObject()
.bytes());

assertThat(doc.rootDoc().getField("point.lat"), notNullValue());
assertThat(doc.rootDoc().getField("point.lon"), notNullValue());
assertThat(doc.rootDoc().get("point.geohash"), equalTo(GeoHashUtils.encode(1.2, 1.3)));
}

@Test
public void testLatLonInOneValueWithGeohash() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).field("geohash", true).endObject().endObject()
.endObject().endObject().string();

DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);

ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder()
.startObject()
.field("point", "1.2,1.3")
.endObject()
.bytes());

assertThat(doc.rootDoc().getField("point.lat"), notNullValue());
assertThat(doc.rootDoc().getField("point.lon"), notNullValue());
assertThat(doc.rootDoc().get("point.geohash"), equalTo(GeoHashUtils.encode(1.2, 1.3)));
}

@Test
public void testGeoHashIndexValue() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).field("geohash", true).endObject().endObject()
.endObject().endObject().string();

DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);

ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder()
.startObject()
.field("point", GeoHashUtils.encode(1.2, 1.3))
.endObject()
.bytes());

assertThat(doc.rootDoc().getField("point.lat"), notNullValue());
assertThat(doc.rootDoc().getField("point.lon"), notNullValue());
assertThat(doc.rootDoc().get("point.geohash"), equalTo(GeoHashUtils.encode(1.2, 1.3)));
}

@Test
public void testGeoHashValue() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).endObject().endObject()
.endObject().endObject().string();

DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);

ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder()
.startObject()
.field("point", GeoHashUtils.encode(1.2, 1.3))
.endObject()
.bytes());

assertThat(doc.rootDoc().getField("point.lat"), notNullValue());
assertThat(doc.rootDoc().getField("point.lon"), notNullValue());
assertThat(doc.rootDoc().get("point"), notNullValue());
}

@Test
public void testNormalizeLatLonValuesDefault() throws Exception {
Expand Down Expand Up @@ -128,7 +228,6 @@ public void testValidateLatLonValues() throws Exception {
}
}


@Test
public void testNoValidateLatLonValues() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
Expand Down Expand Up @@ -169,28 +268,6 @@ public void testNoValidateLatLonValues() throws Exception {
.bytes());
}

@Test
public void testLatLonValues() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).endObject().endObject()
.endObject().endObject().string();

DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);

ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder()
.startObject()
.startObject("point").field("lat", 1.2).field("lon", 1.3).endObject()
.endObject()
.bytes());

assertThat(doc.rootDoc().getField("point.lat"), notNullValue());
assertThat(doc.rootDoc().getField("point.lat").fieldType().stored(), is(false));
assertThat(doc.rootDoc().getField("point.lon"), notNullValue());
assertThat(doc.rootDoc().getField("point.lon").fieldType().stored(), is(false));
assertThat(doc.rootDoc().getField("point.geohash"), nullValue());
assertThat(doc.rootDoc().get("point"), equalTo("1.2,1.3"));
}

@Test
public void testLatLonValuesStored() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
Expand Down Expand Up @@ -307,25 +384,6 @@ public void testLatLonInOneValueArray() throws Exception {
assertThat(doc.rootDoc().getFields("point")[1].stringValue(), equalTo("1.4,1.5"));
}

@Test
public void testGeoHashValue() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).endObject().endObject()
.endObject().endObject().string();

DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);

ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder()
.startObject()
.field("point", GeoHashUtils.encode(1.2, 1.3))
.endObject()
.bytes());

assertThat(doc.rootDoc().getField("point.lat"), notNullValue());
assertThat(doc.rootDoc().getField("point.lon"), notNullValue());
assertThat(doc.rootDoc().get("point"), notNullValue());
}

@Test
public void testLonLatArray() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
Expand Down Expand Up @@ -413,4 +471,34 @@ public void testLonLatArrayArrayStored() throws Exception {
assertThat(doc.rootDoc().getFields("point.lon")[1].numericValue().doubleValue(), equalTo(1.5));
assertThat(doc.rootDoc().getFields("point")[1].stringValue(), equalTo("1.4,1.5"));
}

@Test
public void testGeoPointMapperMerge() throws Exception {
String stage1Mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).field("geohash", true)
.field("validate", true).endObject().endObject()
.endObject().endObject().string();
DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
DocumentMapper stage1 = parser.parse(stage1Mapping);
String stage2Mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).field("geohash", true)
.field("validate", false).endObject().endObject()
.endObject().endObject().string();
DocumentMapper stage2 = parser.parse(stage2Mapping);

DocumentMapper.MergeResult mergeResult = stage1.merge(stage2, mergeFlags().simulate(false));
assertThat(mergeResult.hasConflicts(), equalTo(true));
assertThat(mergeResult.conflicts().length, equalTo(2));
// todo better way of checking conflict?
assertThat("mapper [point] has different validate_lat", isIn(new ArrayList<>(Arrays.asList(mergeResult.conflicts()))));

// correct mapping and ensure no failures
stage2Mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).field("geohash", true)
.field("validate", true).field("normalize", true).endObject().endObject()
.endObject().endObject().string();
stage2 = parser.parse(stage2Mapping);
mergeResult = stage1.merge(stage2, mergeFlags().simulate(false));
assertThat(mergeResult.hasConflicts(), equalTo(false));
}
}
93 changes: 0 additions & 93 deletions src/test/java/org/elasticsearch/index/mapper/geo/LatLonAndGeohashMappingGeoPointTests.java
View file

This file was deleted.

0 comments on commit 7b6364d

Please sign in to comment.