From 70c927567faf8ee59dd779a866981362df0f70b6 Mon Sep 17 00:00:00 2001 From: Stuart Stevenson Date: Sun, 25 Aug 2013 19:21:20 +0100 Subject: [PATCH 1/2] mappingbuilder circular reference issue I found that by trying to save a spatial4j Point object using a repository that MappingBuilder threw a StackOverFlow exception because it contains a circular reference to a field Point class called 'center'. mapEntity method recursively calls itself and has no way to avoid infinite loops. Commited a test with dummy object CircularObject that contains a reference to itself as a field. First commit was to build a list of previously visited classes. Second commit was to change this to be an property on the Field annotation with idea that users can specify the fields that should be ignored when mapping the objects. This doesn't stop lazy users being stung but should be more configurable. --- .../data/elasticsearch/annotations/Field.java | 2 ++ .../elasticsearch/core/MappingBuilder.java | 12 ++++++++- .../elasticsearch/core/CircularObject.java | 18 +++++++++++++ .../core/MappingBuilderTests.java | 25 +++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/springframework/data/elasticsearch/core/CircularObject.java create mode 100644 src/test/java/org/springframework/data/elasticsearch/core/MappingBuilderTests.java diff --git a/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java b/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java index d52675067..6149fc5d0 100644 --- a/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java +++ b/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java @@ -38,4 +38,6 @@ public @interface Field { String indexAnalyzer() default ""; + String [] ignoreFields() default {}; + } diff --git a/src/main/java/org/springframework/data/elasticsearch/core/MappingBuilder.java b/src/main/java/org/springframework/data/elasticsearch/core/MappingBuilder.java index ee43c9b99..c6962c38d 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/MappingBuilder.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/MappingBuilder.java @@ -23,6 +23,7 @@ import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import java.io.IOException; +import java.util.Arrays; import java.util.Map; import static org.apache.commons.lang.StringUtils.EMPTY; @@ -68,7 +69,7 @@ class MappingBuilder { } for (java.lang.reflect.Field field : fields) { - if (isEntity(field)) { + if (isEntity(field) && !isInIgnoreFields(field)) { mapEntity(xContentBuilder, field.getType(), false, EMPTY, field.getName()); } Field singleField = field.getAnnotation(Field.class); @@ -226,4 +227,13 @@ class MappingBuilder { private static boolean isIdField(java.lang.reflect.Field field, String idFieldName) { return idFieldName.equals(field.getName()); } + + private static boolean isInIgnoreFields(java.lang.reflect.Field field) { + Field fieldAnnotation = field.getAnnotation(Field.class); + if ( null != fieldAnnotation ) { + String [] ignoreFields = fieldAnnotation.ignoreFields(); + return Arrays.asList(ignoreFields).contains(field.getName()); + } + return false; + } } diff --git a/src/test/java/org/springframework/data/elasticsearch/core/CircularObject.java b/src/test/java/org/springframework/data/elasticsearch/core/CircularObject.java new file mode 100644 index 000000000..0438edbcd --- /dev/null +++ b/src/test/java/org/springframework/data/elasticsearch/core/CircularObject.java @@ -0,0 +1,18 @@ +package org.springframework.data.elasticsearch.core; + +import org.springframework.data.annotation.Id; +import org.springframework.data.elasticsearch.annotations.Document; +import org.springframework.data.elasticsearch.annotations.Field; +import org.springframework.data.elasticsearch.annotations.FieldType; + +/** + * @author Stuart Stevenson + */ +@Document(indexName = "circular-objects", type = "circular-object" , indexStoreType = "memory", shards = 1, replicas = 0, refreshInterval = "-1") +public class CircularObject { + + @Id + private String id; + @Field(type = FieldType.Object, ignoreFields = {"circularObject"}) + private CircularObject circularObject; +} diff --git a/src/test/java/org/springframework/data/elasticsearch/core/MappingBuilderTests.java b/src/test/java/org/springframework/data/elasticsearch/core/MappingBuilderTests.java new file mode 100644 index 000000000..b8bb1a1d0 --- /dev/null +++ b/src/test/java/org/springframework/data/elasticsearch/core/MappingBuilderTests.java @@ -0,0 +1,25 @@ +package org.springframework.data.elasticsearch.core; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +/** + * @author Stuart Stevenson + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration("classpath:elasticsearch-template-test.xml") +public class MappingBuilderTests { + + @Autowired + private ElasticsearchTemplate elasticsearchTemplate; + + @Test + public void shouldNotFailOnCircularReference() { + elasticsearchTemplate.createIndex(CircularObject.class); + elasticsearchTemplate.putMapping(CircularObject.class); + } + +} From 598b162c940c270ae80d0cb8b06507dffb5cc3b1 Mon Sep 17 00:00:00 2001 From: Stuart Stevenson Date: Sun, 25 Aug 2013 19:21:20 +0100 Subject: [PATCH 2/2] mappingbuilder circular reference issue I found that by trying to save a spatial4j Point object using a repository that MappingBuilder threw a StackOverFlow exception because it contains a circular reference to a field Point class called 'center'. mapEntity method recursively calls itself and has no way to avoid infinite loops. Commited a test with dummy object CircularObject that contains a reference to itself as a field. First commit was to build a list of previously visited classes. Second commit was to change this to be an property on the Field annotation with idea that users can specify the fields that should be ignored when mapping the objects. This doesn't stop lazy users being stung but should be more configurable. --- .../data/elasticsearch/annotations/Field.java | 2 ++ .../elasticsearch/core/MappingBuilder.java | 12 ++++++++- .../elasticsearch/core/CircularObject.java | 18 +++++++++++++ .../core/MappingBuilderTests.java | 25 +++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/springframework/data/elasticsearch/core/CircularObject.java create mode 100644 src/test/java/org/springframework/data/elasticsearch/core/MappingBuilderTests.java diff --git a/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java b/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java index d52675067..6149fc5d0 100644 --- a/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java +++ b/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java @@ -38,4 +38,6 @@ public @interface Field { String indexAnalyzer() default ""; + String [] ignoreFields() default {}; + } diff --git a/src/main/java/org/springframework/data/elasticsearch/core/MappingBuilder.java b/src/main/java/org/springframework/data/elasticsearch/core/MappingBuilder.java index ee43c9b99..c6962c38d 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/MappingBuilder.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/MappingBuilder.java @@ -23,6 +23,7 @@ import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import java.io.IOException; +import java.util.Arrays; import java.util.Map; import static org.apache.commons.lang.StringUtils.EMPTY; @@ -68,7 +69,7 @@ class MappingBuilder { } for (java.lang.reflect.Field field : fields) { - if (isEntity(field)) { + if (isEntity(field) && !isInIgnoreFields(field)) { mapEntity(xContentBuilder, field.getType(), false, EMPTY, field.getName()); } Field singleField = field.getAnnotation(Field.class); @@ -226,4 +227,13 @@ class MappingBuilder { private static boolean isIdField(java.lang.reflect.Field field, String idFieldName) { return idFieldName.equals(field.getName()); } + + private static boolean isInIgnoreFields(java.lang.reflect.Field field) { + Field fieldAnnotation = field.getAnnotation(Field.class); + if ( null != fieldAnnotation ) { + String [] ignoreFields = fieldAnnotation.ignoreFields(); + return Arrays.asList(ignoreFields).contains(field.getName()); + } + return false; + } } diff --git a/src/test/java/org/springframework/data/elasticsearch/core/CircularObject.java b/src/test/java/org/springframework/data/elasticsearch/core/CircularObject.java new file mode 100644 index 000000000..0438edbcd --- /dev/null +++ b/src/test/java/org/springframework/data/elasticsearch/core/CircularObject.java @@ -0,0 +1,18 @@ +package org.springframework.data.elasticsearch.core; + +import org.springframework.data.annotation.Id; +import org.springframework.data.elasticsearch.annotations.Document; +import org.springframework.data.elasticsearch.annotations.Field; +import org.springframework.data.elasticsearch.annotations.FieldType; + +/** + * @author Stuart Stevenson + */ +@Document(indexName = "circular-objects", type = "circular-object" , indexStoreType = "memory", shards = 1, replicas = 0, refreshInterval = "-1") +public class CircularObject { + + @Id + private String id; + @Field(type = FieldType.Object, ignoreFields = {"circularObject"}) + private CircularObject circularObject; +} diff --git a/src/test/java/org/springframework/data/elasticsearch/core/MappingBuilderTests.java b/src/test/java/org/springframework/data/elasticsearch/core/MappingBuilderTests.java new file mode 100644 index 000000000..b8bb1a1d0 --- /dev/null +++ b/src/test/java/org/springframework/data/elasticsearch/core/MappingBuilderTests.java @@ -0,0 +1,25 @@ +package org.springframework.data.elasticsearch.core; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +/** + * @author Stuart Stevenson + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration("classpath:elasticsearch-template-test.xml") +public class MappingBuilderTests { + + @Autowired + private ElasticsearchTemplate elasticsearchTemplate; + + @Test + public void shouldNotFailOnCircularReference() { + elasticsearchTemplate.createIndex(CircularObject.class); + elasticsearchTemplate.putMapping(CircularObject.class); + } + +}