From 758e697aec04d07942d1d26bd0fa2b4bc7947193 Mon Sep 17 00:00:00 2001 From: xhaggi Date: Wed, 13 Jun 2018 16:05:37 +0200 Subject: [PATCH] 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 --- .../ElasticsearchPersistentProperty.java | 14 ++++- .../SimpleElasticsearchPersistentEntity.java | 22 +++---- ...SimpleElasticsearchPersistentProperty.java | 61 +++++++++++++------ ...pleElasticsearchPersistentEntityTests.java | 2 +- 4 files changed, 69 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/core/mapping/ElasticsearchPersistentProperty.java b/src/main/java/org/springframework/data/elasticsearch/core/mapping/ElasticsearchPersistentProperty.java index 906d4dfc0..fb38c8eb5 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/mapping/ElasticsearchPersistentProperty.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/mapping/ElasticsearchPersistentProperty.java @@ -35,13 +35,25 @@ public interface ElasticsearchPersistentProperty extends PersistentPropertypotential 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 { INSTANCE; diff --git a/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntity.java b/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntity.java index cefe8f6b9..cbefb7450 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntity.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntity.java @@ -170,18 +170,18 @@ public class SimpleElasticsearchPersistentEntity 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) { - Assert.isNull(this.parentIdProperty, "Only one field can hold a @Parent annotation"); - Assert.isNull(this.parentType, "Only one field can hold a @Parent annotation"); - Assert.isTrue(property.getType() == String.class, "Parent ID property should be String"); + if (parentProperty != null) { + throw new MappingException( + String.format("Attempt to add parent property %s but already have property %s registered " + + "as parent property. Check your mapping configuration!", property.getField(), parentProperty.getField())); + } + + Parent parentAnnotation = property.findAnnotation(Parent.class); this.parentIdProperty = property; - this.parentType = annotation.type(); - } - - if (property.isVersionProperty()) { - Assert.isTrue(property.getType() == Long.class, "Version property must be of type Long!"); + this.parentType = parentAnnotation.type(); } if (property.isScoreProperty()) { @@ -191,7 +191,7 @@ public class SimpleElasticsearchPersistentEntity 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; diff --git a/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentProperty.java b/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentProperty.java index 3aafc5ab5..d9ab47f9f 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentProperty.java +++ b/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.Set; +import java.util.List; +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 implements ElasticsearchPersistentProperty { - private static final Set> SUPPORTED_ID_TYPES = new HashSet<>(); - private static final Set SUPPORTED_ID_PROPERTY_NAMES = new HashSet<>(); - - private final boolean isScore; + private static final List SUPPORTED_ID_PROPERTY_NAMES = Arrays.asList("id", "document"); - static { - SUPPORTED_ID_TYPES.add(String.class); - SUPPORTED_ID_PROPERTY_NAMES.add("id"); - SUPPORTED_ID_PROPERTY_NAMES.add("documentId"); - } + private final boolean isScore; + private final boolean isParent; + private final boolean isId; public SimpleElasticsearchPersistentProperty(Property property, PersistentEntity 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 createAssociation() { return null; @@ -85,7 +103,7 @@ public class SimpleElasticsearchPersistentProperty extends public boolean isScoreProperty() { return isScore; } - + /* * (non-Javadoc) * @see org.springframework.data.mapping.model.AbstractPersistentProperty#isImmutable() @@ -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; + } } diff --git a/src/test/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntityTests.java b/src/test/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntityTests.java index c6bf94095..72a0e3bed 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntityTests.java +++ b/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);