Skip to content

Commit d8130d0

Browse files
sothawosothawo
committed
Allow fields with id-property names
Closes spring-projects#1680
1 parent 8fa2613 commit d8130d0

8 files changed

+36
-22
lines changed

Diff for: ‎src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentProperty.java

+7-12
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,7 @@
2222

2323
import org.slf4j.Logger;
2424
import org.slf4j.LoggerFactory;
25-
import org.springframework.data.annotation.Id;
2625
import org.springframework.data.elasticsearch.annotations.DateFormat;
27-
import org.springframework.data.elasticsearch.annotations.Document;
2826
import org.springframework.data.elasticsearch.annotations.Field;
2927
import org.springframework.data.elasticsearch.annotations.FieldType;
3028
import org.springframework.data.elasticsearch.annotations.GeoPointField;
@@ -83,16 +81,8 @@ public SimpleElasticsearchPersistentProperty(Property property,
8381
this.annotatedFieldName = getAnnotatedFieldName();
8482
this.fieldNamingStrategy = fieldNamingStrategy == null ? PropertyNameFieldNamingStrategy.INSTANCE
8583
: fieldNamingStrategy;
86-
this.isId = super.isIdProperty() || SUPPORTED_ID_PROPERTY_NAMES.contains(getFieldName());
87-
88-
// deprecated since 4.1
89-
@Deprecated
90-
boolean isIdWithoutAnnotation = isId && !isAnnotationPresent(Id.class);
91-
if (isIdWithoutAnnotation && owner.isAnnotationPresent(Document.class)) {
92-
LOGGER.warn("Using the property name of '{}' to identify the id property is deprecated."
93-
+ " Please annotate the id property with '@Id'", owner.getName() + "." + getName());
94-
}
95-
84+
this.isId = super.isIdProperty()
85+
|| (SUPPORTED_ID_PROPERTY_NAMES.contains(getFieldName()) && !hasExplicitFieldName());
9686
this.isParent = isAnnotationPresent(Parent.class);
9787
this.isSeqNoPrimaryTerm = SeqNoPrimaryTerm.class.isAssignableFrom(getRawType());
9888

@@ -106,6 +96,7 @@ public SimpleElasticsearchPersistentProperty(Property property,
10696
throw new MappingException(String.format("Parent property %s must be of type String!", property.getName()));
10797
}
10898

99+
109100
if (isField && isAnnotationPresent(MultiField.class)) {
110101
throw new MappingException("@Field annotation must not be used on a @MultiField property.");
111102
}
@@ -141,6 +132,10 @@ public boolean storeNullValue() {
141132
return storeNullValue;
142133
}
143134

135+
protected boolean hasExplicitFieldName() {
136+
return StringUtils.hasText(getAnnotatedFieldName());
137+
}
138+
144139
/**
145140
* Initializes an {@link ElasticsearchPersistentPropertyConverter} if this property is annotated as a Field with type
146141
* {@link FieldType#Date}, has a {@link DateFormat} set and if the type of the property is one of the Java8 temporal

Diff for: ‎src/test/java/org/springframework/data/elasticsearch/core/index/MappingContextBaseTests.java renamed to ‎src/test/java/org/springframework/data/elasticsearch/core/MappingContextBaseTests.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,19 @@
1313
* limitations under the License.
1414
*/
1515

16-
package org.springframework.data.elasticsearch.core.index;
16+
package org.springframework.data.elasticsearch.core;
1717

1818
import org.springframework.data.elasticsearch.config.ElasticsearchConfigurationSupport;
1919
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
2020
import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
21+
import org.springframework.data.elasticsearch.core.index.MappingBuilder;
2122
import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
2223
import org.springframework.data.util.Lazy;
2324

2425
/**
2526
* @author Peter-Josef Meisch
2627
*/
27-
abstract class MappingContextBaseTests {
28+
public abstract class MappingContextBaseTests {
2829

2930
protected final Lazy<ElasticsearchConverter> elasticsearchConverter = Lazy.of(this::setupElasticsearchConverter);
3031

@@ -41,7 +42,7 @@ private SimpleElasticsearchMappingContext setupMappingContext() {
4142
return mappingContext;
4243
}
4344

44-
final MappingBuilder getMappingBuilder() {
45+
final protected MappingBuilder getMappingBuilder() {
4546
return new MappingBuilder(elasticsearchConverter.get());
4647
}
4748
}

Diff for: ‎src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderIntegrationTests.java

+1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import org.springframework.data.elasticsearch.annotations.*;
5454
import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
5555
import org.springframework.data.elasticsearch.core.IndexOperations;
56+
import org.springframework.data.elasticsearch.core.MappingContextBaseTests;
5657
import org.springframework.data.elasticsearch.core.SearchHits;
5758
import org.springframework.data.elasticsearch.core.completion.Completion;
5859
import org.springframework.data.elasticsearch.core.geo.GeoPoint;

Diff for: ‎src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderUnitTests.java

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.springframework.data.annotation.Id;
4848
import org.springframework.data.annotation.Transient;
4949
import org.springframework.data.elasticsearch.annotations.*;
50+
import org.springframework.data.elasticsearch.core.MappingContextBaseTests;
5051
import org.springframework.data.elasticsearch.core.completion.Completion;
5152
import org.springframework.data.elasticsearch.core.geo.GeoPoint;
5253
import org.springframework.data.elasticsearch.core.query.SeqNoPrimaryTerm;

Diff for: ‎src/test/java/org/springframework/data/elasticsearch/core/index/MappingParametersTest.java

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.springframework.data.elasticsearch.annotations.FieldType;
1212
import org.springframework.data.elasticsearch.annotations.InnerField;
1313
import org.springframework.data.elasticsearch.annotations.MultiField;
14+
import org.springframework.data.elasticsearch.core.MappingContextBaseTests;
1415
import org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentEntity;
1516
import org.springframework.lang.Nullable;
1617

Diff for: ‎src/test/java/org/springframework/data/elasticsearch/core/index/SimpleDynamicTemplatesMappingTests.java

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.springframework.data.elasticsearch.annotations.DynamicTemplates;
2727
import org.springframework.data.elasticsearch.annotations.Field;
2828
import org.springframework.data.elasticsearch.annotations.FieldType;
29+
import org.springframework.data.elasticsearch.core.MappingContextBaseTests;
2930
import org.springframework.lang.Nullable;
3031

3132
/**

Diff for: ‎src/test/java/org/springframework/data/elasticsearch/core/index/SimpleElasticsearchDateMappingTests.java

+3-6
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,15 @@
2020

2121
import lombok.Data;
2222

23-
import java.io.IOException;
2423
import java.time.LocalDateTime;
25-
import java.util.Date;
2624

2725
import org.junit.jupiter.api.Test;
2826
import org.springframework.data.annotation.Id;
2927
import org.springframework.data.elasticsearch.annotations.DateFormat;
3028
import org.springframework.data.elasticsearch.annotations.Document;
3129
import org.springframework.data.elasticsearch.annotations.Field;
3230
import org.springframework.data.elasticsearch.annotations.FieldType;
31+
import org.springframework.data.elasticsearch.core.MappingContextBaseTests;
3332

3433
/**
3534
* @author Jakub Vavrik
@@ -41,8 +40,7 @@ public class SimpleElasticsearchDateMappingTests extends MappingContextBaseTests
4140

4241
private static final String EXPECTED_MAPPING = "{\"properties\":{\"message\":{\"store\":true,"
4342
+ "\"type\":\"text\",\"index\":false,\"analyzer\":\"standard\"},\"customFormatDate\":{\"type\":\"date\",\"format\":\"dd.MM.uuuu hh:mm\"},"
44-
+ "\"basicFormatDate\":{\""
45-
+ "type\":\"date\",\"format\":\"basic_date\"}}}";
43+
+ "\"basicFormatDate\":{\"" + "type\":\"date\",\"format\":\"basic_date\"}}}";
4644

4745
@Test // DATAES-568, DATAES-828
4846
public void testCorrectDateMappings() {
@@ -56,8 +54,7 @@ public void testCorrectDateMappings() {
5654
* @author Jakub Vavrik
5755
*/
5856
@Data
59-
@Document(indexName = "test-index-date-mapping-core", replicas = 0,
60-
refreshInterval = "-1")
57+
@Document(indexName = "test-index-date-mapping-core", replicas = 0, refreshInterval = "-1")
6158
static class SampleDateMappingEntity {
6259

6360
@Id private String id;

Diff for: ‎src/test/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntityTests.java

+18-1
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,14 @@
1717

1818
import static org.assertj.core.api.Assertions.*;
1919

20+
import org.junit.jupiter.api.DisplayName;
2021
import org.junit.jupiter.api.Test;
2122
import org.springframework.data.annotation.Id;
2223
import org.springframework.data.annotation.Version;
24+
import org.springframework.data.elasticsearch.annotations.Document;
2325
import org.springframework.data.elasticsearch.annotations.Field;
26+
import org.springframework.data.elasticsearch.annotations.FieldType;
27+
import org.springframework.data.elasticsearch.core.MappingContextBaseTests;
2428
import org.springframework.data.elasticsearch.core.query.SeqNoPrimaryTerm;
2529
import org.springframework.data.mapping.MappingException;
2630
import org.springframework.data.mapping.model.Property;
@@ -38,7 +42,7 @@
3842
* @author Peter-Josef Meisch
3943
* @author Roman Puchkovskiy
4044
*/
41-
public class SimpleElasticsearchPersistentEntityTests {
45+
public class SimpleElasticsearchPersistentEntityTests extends MappingContextBaseTests {
4246

4347
@Test
4448
public void shouldThrowExceptionGivenVersionPropertyIsNotLong() {
@@ -135,6 +139,12 @@ void shouldNotAllowMoreThanOneSeqNoPrimaryTermProperties() {
135139
.isInstanceOf(MappingException.class);
136140
}
137141

142+
@Test // #1680
143+
@DisplayName("should allow fields with id property names")
144+
void shouldAllowFieldsWithIdPropertyNames() {
145+
elasticsearchConverter.get().getMappingContext().getRequiredPersistentEntity(EntityWithIdNameFields.class);
146+
}
147+
138148
private static SimpleElasticsearchPersistentProperty createProperty(SimpleElasticsearchPersistentEntity<?> entity,
139149
String field) {
140150

@@ -193,4 +203,11 @@ private static class EntityWithSeqNoPrimaryTerm {
193203
private SeqNoPrimaryTerm seqNoPrimaryTerm;
194204
private SeqNoPrimaryTerm seqNoPrimaryTerm2;
195205
}
206+
207+
@Document(indexName = "fieldnames")
208+
private static class EntityWithIdNameFields {
209+
@Id private String theRealId;
210+
@Field(type = FieldType.Text, name = "document") private String document;
211+
@Field(name = "id") private String renamedId;
212+
}
196213
}

0 commit comments

Comments
 (0)