From 910ca7b6653ff60226c5663aa4d6e6e144264641 Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Fri, 5 Feb 2021 23:06:19 +0100 Subject: [PATCH] Allow fields with id-property names (#1681) Original Pull Request #1681 Closes #1680 --- ...SimpleElasticsearchPersistentProperty.java | 18 ++++++------------ .../{index => }/MappingContextBaseTests.java | 7 ++++--- .../index/MappingBuilderIntegrationTests.java | 1 + .../core/index/MappingBuilderUnitTests.java | 1 + .../core/index/MappingParametersTest.java | 1 + .../SimpleDynamicTemplatesMappingTests.java | 1 + .../SimpleElasticsearchDateMappingTests.java | 9 +++------ ...pleElasticsearchPersistentEntityTests.java | 19 ++++++++++++++++++- 8 files changed, 35 insertions(+), 22 deletions(-) rename src/test/java/org/springframework/data/elasticsearch/core/{index => }/MappingContextBaseTests.java (88%) 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 e045567a..681dc05a 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 @@ -22,9 +22,7 @@ import java.util.List; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.data.annotation.Id; import org.springframework.data.elasticsearch.annotations.DateFormat; -import org.springframework.data.elasticsearch.annotations.Document; import org.springframework.data.elasticsearch.annotations.Field; import org.springframework.data.elasticsearch.annotations.FieldType; import org.springframework.data.elasticsearch.annotations.GeoPointField; @@ -83,16 +81,8 @@ public class SimpleElasticsearchPersistentProperty extends this.annotatedFieldName = getAnnotatedFieldName(); this.fieldNamingStrategy = fieldNamingStrategy == null ? PropertyNameFieldNamingStrategy.INSTANCE : fieldNamingStrategy; - this.isId = super.isIdProperty() || SUPPORTED_ID_PROPERTY_NAMES.contains(getFieldName()); - - // deprecated since 4.1 - @Deprecated - boolean isIdWithoutAnnotation = isId && !isAnnotationPresent(Id.class); - if (isIdWithoutAnnotation && owner.isAnnotationPresent(Document.class)) { - LOGGER.warn("Using the property name of '{}' to identify the id property is deprecated." - + " Please annotate the id property with '@Id'", owner.getName() + "." + getName()); - } - + this.isId = super.isIdProperty() + || (SUPPORTED_ID_PROPERTY_NAMES.contains(getFieldName()) && !hasExplicitFieldName()); this.isParent = isAnnotationPresent(Parent.class); this.isSeqNoPrimaryTerm = SeqNoPrimaryTerm.class.isAssignableFrom(getRawType()); @@ -141,6 +131,10 @@ public class SimpleElasticsearchPersistentProperty extends return storeNullValue; } + protected boolean hasExplicitFieldName() { + return StringUtils.hasText(getAnnotatedFieldName()); + } + /** * Initializes an {@link ElasticsearchPersistentPropertyConverter} if this property is annotated as a Field with type * {@link FieldType#Date}, has a {@link DateFormat} set and if the type of the property is one of the Java8 temporal diff --git a/src/test/java/org/springframework/data/elasticsearch/core/index/MappingContextBaseTests.java b/src/test/java/org/springframework/data/elasticsearch/core/MappingContextBaseTests.java similarity index 88% rename from src/test/java/org/springframework/data/elasticsearch/core/index/MappingContextBaseTests.java rename to src/test/java/org/springframework/data/elasticsearch/core/MappingContextBaseTests.java index 7abb9e99..ec2dcab6 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/index/MappingContextBaseTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/MappingContextBaseTests.java @@ -13,18 +13,19 @@ * limitations under the License. */ -package org.springframework.data.elasticsearch.core.index; +package org.springframework.data.elasticsearch.core; import org.springframework.data.elasticsearch.config.ElasticsearchConfigurationSupport; import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter; import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter; +import org.springframework.data.elasticsearch.core.index.MappingBuilder; import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext; import org.springframework.data.util.Lazy; /** * @author Peter-Josef Meisch */ -abstract class MappingContextBaseTests { +public abstract class MappingContextBaseTests { protected final Lazy elasticsearchConverter = Lazy.of(this::setupElasticsearchConverter); @@ -41,7 +42,7 @@ abstract class MappingContextBaseTests { return mappingContext; } - final MappingBuilder getMappingBuilder() { + final protected MappingBuilder getMappingBuilder() { return new MappingBuilder(elasticsearchConverter.get()); } } diff --git a/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderIntegrationTests.java index 665074c4..8d4fcc47 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderIntegrationTests.java @@ -53,6 +53,7 @@ import org.springframework.data.annotation.Id; import org.springframework.data.elasticsearch.annotations.*; import org.springframework.data.elasticsearch.core.ElasticsearchOperations; import org.springframework.data.elasticsearch.core.IndexOperations; +import org.springframework.data.elasticsearch.core.MappingContextBaseTests; import org.springframework.data.elasticsearch.core.SearchHits; import org.springframework.data.elasticsearch.core.completion.Completion; import org.springframework.data.elasticsearch.core.geo.GeoPoint; diff --git a/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderUnitTests.java index 99d8a097..14764c46 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderUnitTests.java @@ -47,6 +47,7 @@ import org.junit.jupiter.api.Test; import org.springframework.data.annotation.Id; import org.springframework.data.annotation.Transient; import org.springframework.data.elasticsearch.annotations.*; +import org.springframework.data.elasticsearch.core.MappingContextBaseTests; import org.springframework.data.elasticsearch.core.completion.Completion; import org.springframework.data.elasticsearch.core.geo.GeoPoint; import org.springframework.data.elasticsearch.core.query.SeqNoPrimaryTerm; diff --git a/src/test/java/org/springframework/data/elasticsearch/core/index/MappingParametersTest.java b/src/test/java/org/springframework/data/elasticsearch/core/index/MappingParametersTest.java index 40abcfb2..d1fbac06 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/index/MappingParametersTest.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/index/MappingParametersTest.java @@ -11,6 +11,7 @@ import org.springframework.data.elasticsearch.annotations.Field; import org.springframework.data.elasticsearch.annotations.FieldType; import org.springframework.data.elasticsearch.annotations.InnerField; import org.springframework.data.elasticsearch.annotations.MultiField; +import org.springframework.data.elasticsearch.core.MappingContextBaseTests; import org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentEntity; import org.springframework.lang.Nullable; diff --git a/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleDynamicTemplatesMappingTests.java b/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleDynamicTemplatesMappingTests.java index e937063c..aa427ed4 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleDynamicTemplatesMappingTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleDynamicTemplatesMappingTests.java @@ -26,6 +26,7 @@ import org.springframework.data.elasticsearch.annotations.Document; import org.springframework.data.elasticsearch.annotations.DynamicTemplates; import org.springframework.data.elasticsearch.annotations.Field; import org.springframework.data.elasticsearch.annotations.FieldType; +import org.springframework.data.elasticsearch.core.MappingContextBaseTests; import org.springframework.lang.Nullable; /** diff --git a/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleElasticsearchDateMappingTests.java b/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleElasticsearchDateMappingTests.java index ab7a79a7..e2248c20 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleElasticsearchDateMappingTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleElasticsearchDateMappingTests.java @@ -20,9 +20,7 @@ import static org.springframework.data.elasticsearch.annotations.FieldType.*; import lombok.Data; -import java.io.IOException; import java.time.LocalDateTime; -import java.util.Date; import org.junit.jupiter.api.Test; import org.springframework.data.annotation.Id; @@ -30,6 +28,7 @@ import org.springframework.data.elasticsearch.annotations.DateFormat; import org.springframework.data.elasticsearch.annotations.Document; import org.springframework.data.elasticsearch.annotations.Field; import org.springframework.data.elasticsearch.annotations.FieldType; +import org.springframework.data.elasticsearch.core.MappingContextBaseTests; /** * @author Jakub Vavrik @@ -41,8 +40,7 @@ public class SimpleElasticsearchDateMappingTests extends MappingContextBaseTests private static final String EXPECTED_MAPPING = "{\"properties\":{\"message\":{\"store\":true," + "\"type\":\"text\",\"index\":false,\"analyzer\":\"standard\"},\"customFormatDate\":{\"type\":\"date\",\"format\":\"dd.MM.uuuu hh:mm\"}," - + "\"basicFormatDate\":{\"" - + "type\":\"date\",\"format\":\"basic_date\"}}}"; + + "\"basicFormatDate\":{\"" + "type\":\"date\",\"format\":\"basic_date\"}}}"; @Test // DATAES-568, DATAES-828 public void testCorrectDateMappings() { @@ -56,8 +54,7 @@ public class SimpleElasticsearchDateMappingTests extends MappingContextBaseTests * @author Jakub Vavrik */ @Data - @Document(indexName = "test-index-date-mapping-core", replicas = 0, - refreshInterval = "-1") + @Document(indexName = "test-index-date-mapping-core", replicas = 0, refreshInterval = "-1") static class SampleDateMappingEntity { @Id private String id; 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 d0694a37..a5d920c4 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 @@ -17,10 +17,14 @@ package org.springframework.data.elasticsearch.core.mapping; import static org.assertj.core.api.Assertions.*; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.data.annotation.Id; import org.springframework.data.annotation.Version; +import org.springframework.data.elasticsearch.annotations.Document; import org.springframework.data.elasticsearch.annotations.Field; +import org.springframework.data.elasticsearch.annotations.FieldType; +import org.springframework.data.elasticsearch.core.MappingContextBaseTests; import org.springframework.data.elasticsearch.core.query.SeqNoPrimaryTerm; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.model.Property; @@ -38,7 +42,7 @@ import org.springframework.util.ReflectionUtils; * @author Peter-Josef Meisch * @author Roman Puchkovskiy */ -public class SimpleElasticsearchPersistentEntityTests { +public class SimpleElasticsearchPersistentEntityTests extends MappingContextBaseTests { @Test public void shouldThrowExceptionGivenVersionPropertyIsNotLong() { @@ -135,6 +139,12 @@ public class SimpleElasticsearchPersistentEntityTests { .isInstanceOf(MappingException.class); } + @Test // #1680 + @DisplayName("should allow fields with id property names") + void shouldAllowFieldsWithIdPropertyNames() { + elasticsearchConverter.get().getMappingContext().getRequiredPersistentEntity(EntityWithIdNameFields.class); + } + private static SimpleElasticsearchPersistentProperty createProperty(SimpleElasticsearchPersistentEntity entity, String field) { @@ -193,4 +203,11 @@ public class SimpleElasticsearchPersistentEntityTests { private SeqNoPrimaryTerm seqNoPrimaryTerm; private SeqNoPrimaryTerm seqNoPrimaryTerm2; } + + @Document(indexName = "fieldnames") + private static class EntityWithIdNameFields { + @Id private String theRealId; + @Field(type = FieldType.Text, name = "document") private String document; + @Field(name = "id") private String renamedId; + } }