DATAES-33 - Polishing

* Move @Parent property recognition to ElasticsearchPersistentProperty
* Move type contraints for @Version and @Parent fields to ElasticsearchPersistentProperty to fail faster
* remove unused constant SUPPORTED_ID_TYPES from SimpleElasticsearchPersistentProperty

Original pull request: #208

src/main/java/org/springframework/data/elasticsearch/core/mapping/ElasticsearchPersistentProperty.java

@@ -35,13 +35,25 @@ public interface ElasticsearchPersistentProperty extends PersistentProperty<Elas
 	 * {@link ElasticsearchPersistentEntity}. This method is mainly used by {@link ElasticsearchPersistentEntity}
 	 * implementation to discover score property candidates on {@link ElasticsearchPersistentEntity} creation you should
 	 * rather call {@link ElasticsearchPersistentEntity#isScoreProperty(PersistentProperty)} to determine whether the
-	 * current property is the version property of that {@link ElasticsearchPersistentEntity} under consideration.
+	 * current property is the score property of that {@link ElasticsearchPersistentEntity} under consideration.
 	 *
 	 * @return
 	 * @since 3.1
 	 */
 	boolean isScoreProperty();
 
+	/**
+	 * Returns whether the current property is a <em>potential</em> parent property of the owning
+	 * {@link ElasticsearchPersistentEntity}. This method is mainly used by {@link ElasticsearchPersistentEntity}
+	 * implementation to discover parent property candidates on {@link ElasticsearchPersistentEntity} creation you should
+	 * rather call {@link ElasticsearchPersistentEntity#isParentProperty()} to determine whether the current property is
+	 * the parent property of that {@link ElasticsearchPersistentEntity} under consideration.
+	 *
+	 * @return
+	 * @since 3.1
+	 */
+	boolean isParentProperty();
+
 	public enum PropertyToFieldNameConverter implements Converter<ElasticsearchPersistentProperty, String> {
 
 		INSTANCE;

src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntity.java

@@ -170,18 +170,18 @@ public class SimpleElasticsearchPersistentEntity<T> extends BasicPersistentEntit
 	public void addPersistentProperty(ElasticsearchPersistentProperty property) {
 		super.addPersistentProperty(property);
 
-		Parent annotation = property.findAnnotation(Parent.class);
+		if (property.isParentProperty()) {
+			ElasticsearchPersistentProperty parentProperty = this.parentIdProperty;
 
-		if (annotation != null) {
+			if (parentProperty != null) {
-			Assert.isNull(this.parentIdProperty, "Only one field can hold a @Parent annotation");
+				throw new MappingException(
-			Assert.isNull(this.parentType, "Only one field can hold a @Parent annotation");
+						String.format("Attempt to add parent property %s but already have property %s registered "
-			Assert.isTrue(property.getType() == String.class, "Parent ID property should be String");
+								+ "as parent property. Check your mapping configuration!", property.getField(), parentProperty.getField()));
-			this.parentIdProperty = property;
-			this.parentType = annotation.type();
 			}
 
-		if (property.isVersionProperty()) {
+			Parent parentAnnotation = property.findAnnotation(Parent.class);
-			Assert.isTrue(property.getType() == Long.class, "Version property must be of type Long!");
+			this.parentIdProperty = property;
+			this.parentType = parentAnnotation.type();
 		}
 
 		if (property.isScoreProperty()) {
@@ -191,7 +191,7 @@ public class SimpleElasticsearchPersistentEntity<T> extends BasicPersistentEntit
 			if (scoreProperty != null) {
 				throw new MappingException(
 						String.format("Attempt to add score property %s but already have property %s registered "
-								+ "as version. Check your mapping configuration!", property.getField(), scoreProperty.getField()));
+								+ "as score property. Check your mapping configuration!", property.getField(), scoreProperty.getField()));
 			}
 
 			this.scoreProperty = property;

src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentProperty.java

@@ -16,9 +16,9 @@
 package org.springframework.data.elasticsearch.core.mapping;
 
 import java.util.Arrays;
-import java.util.HashSet;
+import java.util.List;
-import java.util.Set;
 
+import org.springframework.data.elasticsearch.annotations.Parent;
 import org.springframework.data.elasticsearch.annotations.Score;
 import org.springframework.data.mapping.Association;
 import org.springframework.data.mapping.MappingException;
@@ -39,39 +39,57 @@ import org.springframework.data.mapping.model.SimpleTypeHolder;
 public class SimpleElasticsearchPersistentProperty extends
 		AnnotationBasedPersistentProperty<ElasticsearchPersistentProperty> implements ElasticsearchPersistentProperty {
 
-	private static final Set<Class<?>> SUPPORTED_ID_TYPES = new HashSet<>();
+	private static final List<String> SUPPORTED_ID_PROPERTY_NAMES = Arrays.asList("id", "document");
-	private static final Set<String> SUPPORTED_ID_PROPERTY_NAMES = new HashSet<>();
 
 	private final boolean isScore;
-
+	private final boolean isParent;
-	static {
+	private final boolean isId;
-		SUPPORTED_ID_TYPES.add(String.class);
-		SUPPORTED_ID_PROPERTY_NAMES.add("id");
-		SUPPORTED_ID_PROPERTY_NAMES.add("documentId");
-	}
 
 	public SimpleElasticsearchPersistentProperty(Property property,
 			PersistentEntity<?, ElasticsearchPersistentProperty> owner, SimpleTypeHolder simpleTypeHolder) {
 
 		super(property, owner, simpleTypeHolder);
 
+		this.isId = super.isIdProperty() || SUPPORTED_ID_PROPERTY_NAMES.contains(getFieldName());
 		this.isScore = isAnnotationPresent(Score.class);
+		this.isParent = isAnnotationPresent(Parent.class);
+
+		if (isVersionProperty() && getType() != Long.class) {
+			throw new MappingException(String.format("Version property %s must be of type Long!", property.getName()));
+		}
 
 		if (isScore && !Arrays.asList(Float.TYPE, Float.class).contains(getType())) {
-			throw new MappingException(String.format("Score property %s must be either of type float or Float!", property.getName()));
+			throw new MappingException(
+					String.format("Score property %s must be either of type float or Float!", property.getName()));
+		}
+
+		if (isParent && getType() != String.class) {
+			throw new MappingException(String.format("Parent property %s must be of type String!", property.getName()));
 		}
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentProperty#getFieldName()
+	 */
 	@Override
 	public String getFieldName() {
 		return getProperty().getName();
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mapping.model.AnnotationBasedPersistentProperty#isIdProperty()
+	 */
 	@Override
 	public boolean isIdProperty() {
-		return super.isIdProperty() || SUPPORTED_ID_PROPERTY_NAMES.contains(getFieldName());
+		return isId;
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mapping.model.AbstractPersistentProperty#createAssociation()
+	 */
 	@Override
 	protected Association<ElasticsearchPersistentProperty> createAssociation() {
 		return null;
@@ -94,4 +112,13 @@ public class SimpleElasticsearchPersistentProperty extends
 	public boolean isImmutable() {
 		return false;
 	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentProperty#isParentProperty()
+	 */
+	@Override
+	public boolean isParentProperty() {
+		return isParent;
+	}
 }

src/test/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntityTests.java

@@ -37,7 +37,7 @@ import org.springframework.util.ReflectionUtils;
  */
 public class SimpleElasticsearchPersistentEntityTests {
 
-	@Test(expected = IllegalArgumentException.class)
+	@Test(expected = MappingException.class)
 	public void shouldThrowExceptionGivenVersionPropertyIsNotLong() throws NoSuchFieldException, IntrospectionException {
 		// given
 		TypeInformation typeInformation = ClassTypeInformation.from(EntityWithWrongVersionType.class);