Skip to content

Commit d2456bb

Browse files
mikereichemikereiche
committed
DATACOUCH-533 - Promote id field if no annotation id.
Promote id field if no annotation id. Note that there is no longer any Couchbase @id annotation - only the Spring @id annotation.
1 parent ecaf58d commit d2456bb

File tree

4 files changed

+209
-13
lines changed

4 files changed

+209
-13
lines changed

‎src/main/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentEntity.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import org.springframework.context.EnvironmentAware;
2424
import org.springframework.core.env.Environment;
2525
import org.springframework.data.annotation.Id;
26+
import org.springframework.data.mapping.MappingException;
27+
import org.springframework.data.mapping.PersistentProperty;
2628
import org.springframework.data.mapping.model.BasicPersistentEntity;
2729
import org.springframework.data.util.TypeInformation;
2830
import org.springframework.util.Assert;
@@ -76,24 +78,22 @@ protected CouchbasePersistentProperty returnPropertyIfBetterIdPropertyCandidateO
7678
}
7779

7880
// check existing ID vs new candidate
79-
boolean currentCbId = this.getIdProperty().isAnnotationPresent(Id.class);
8081
boolean currentSpringId = this.getIdProperty().isAnnotationPresent(org.springframework.data.annotation.Id.class);
81-
boolean candidateCbId = property.isAnnotationPresent(Id.class);
8282
boolean candidateSpringId = property.isAnnotationPresent(org.springframework.data.annotation.Id.class);
8383

84-
if (currentCbId && candidateSpringId) {
85-
// spring IDs will have priority over SDK IDs
84+
if (candidateSpringId && !currentSpringId) {
85+
// spring IDs will have priority over fields named id
8686
return property;
87-
} else if (currentSpringId && candidateCbId) {
88-
// ignore SDK's IDs if current is a Spring ID
87+
} else if (currentSpringId && !candidateSpringId) {
88+
// spring IDs will have priority over fields named id
8989
return null;
90+
} else {
91+
// do not allow two @Id fields or two fields named id (possible via @Field)
92+
throw new MappingException(String.format(
93+
"Attempt to add id property %s but already have property %s registered as id. Check your mapping configuration!",
94+
property.getField(), getIdProperty().getField()));
9095
}
91-
/* any of the following will throw:
92-
- current is a spring ID and the candidate bears another spring ID
93-
- current is a SDK ID and the candidate bears another SDK ID
94-
- any other combination involving something else than a SDK or Spring ID
95-
*/
96-
return super.returnPropertyIfBetterIdPropertyCandidateOrNull(property);
96+
9797
}
9898

9999
@Override

‎src/main/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentProperty.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828

2929
import com.couchbase.client.core.deps.com.fasterxml.jackson.annotation.JsonProperty;
3030

31+
import java.util.Locale;
32+
3133
/**
3234
* Implements annotated property representations of a given {@link Field} instance.
3335
* <p/>
@@ -97,6 +99,7 @@ public String getFieldName() {
9799
// DATACOUCH-145: allows SDK's @Id annotation to be used
98100
@Override
99101
public boolean isIdProperty() {
100-
return isAnnotationPresent(Id.class) || super.isIdProperty();
102+
return isAnnotationPresent(Id.class) || super.isIdProperty()
103+
|| this.getFieldName().toLowerCase(Locale.ROOT).equals("id");
101104
}
102105
}

‎src/test/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentPropertyTests.java

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.junit.jupiter.api.BeforeEach;
2424
import org.junit.jupiter.api.Test;
2525
import org.springframework.data.annotation.Id;
26+
import org.springframework.data.mapping.MappingException;
2627
import org.springframework.data.mapping.model.Property;
2728
import org.springframework.data.mapping.model.PropertyNameFieldNamingStrategy;
2829
import org.springframework.data.mapping.model.SimpleTypeHolder;
@@ -82,6 +83,98 @@ void testSdkIdAnnotationEvaluatedAfterSpringIdAnnotationIsIgnored() {
8283
assertThat(test.getIdProperty()).isEqualTo(springIdProperty);
8384
}
8485

86+
@Test
87+
void testAnnotationIdFieldOnly() { // only has @springId
88+
class TestIdField {
89+
@org.springframework.data.couchbase.core.mapping.Field String name;
90+
String description;
91+
@Id private String springId;
92+
}
93+
BasicCouchbasePersistentEntity<TestIdField> test = new BasicCouchbasePersistentEntity<>(
94+
ClassTypeInformation.from(TestIdField.class));
95+
Field springIdField = ReflectionUtils.findField(TestIdField.class, "springId");
96+
CouchbasePersistentProperty springIdProperty = getPropertyFor(springIdField);
97+
test.addPersistentProperty(springIdProperty);
98+
assertThat(test.getIdProperty()).isEqualTo(springIdProperty);
99+
}
100+
101+
@Test
102+
void testIdFieldOnly() { // only has id
103+
class TestIdField {
104+
@org.springframework.data.couchbase.core.mapping.Field String name;
105+
String description;
106+
private String id;
107+
}
108+
Field idField = ReflectionUtils.findField(TestIdField.class, "id");
109+
CouchbasePersistentProperty idProperty = getPropertyFor(idField);
110+
BasicCouchbasePersistentEntity<TestIdField> test = new BasicCouchbasePersistentEntity<>(
111+
ClassTypeInformation.from(TestIdField.class));
112+
test.addPersistentProperty(idProperty);
113+
assertThat(test.getIdProperty()).isEqualTo(idProperty);
114+
}
115+
116+
@Test
117+
void testIdFieldAndAnnotationIdField() { // has @springId and id
118+
class TestIdField {
119+
@org.springframework.data.couchbase.core.mapping.Field String name;
120+
String description;
121+
@Id private String springId;
122+
private String id;
123+
}
124+
BasicCouchbasePersistentEntity<TestIdField> test = new BasicCouchbasePersistentEntity<>(
125+
ClassTypeInformation.from(TestIdField.class));
126+
Field springIdField = ReflectionUtils.findField(TestIdField.class, "springId");
127+
Field idField = ReflectionUtils.findField(TestIdField.class, "id");
128+
CouchbasePersistentProperty idProperty = getPropertyFor(idField);
129+
CouchbasePersistentProperty springIdProperty = getPropertyFor(springIdField);
130+
// here this simulates the order in which the annotations would be found
131+
// when "overriding" Spring @Id with SDK's @Id...
132+
test.addPersistentProperty(idProperty);
133+
// replace id with springId
134+
test.addPersistentProperty(springIdProperty);
135+
assertThat(test.getIdProperty()).isEqualTo(springIdProperty);
136+
}
137+
138+
@Test
139+
void testTwoAnnotationIdFields() { // has @Id springId and @Id id
140+
class TestIdField {
141+
@org.springframework.data.couchbase.core.mapping.Field String name;
142+
String description;
143+
@Id private String springId;
144+
@Id private String id;
145+
}
146+
Field springIdField = ReflectionUtils.findField(TestIdField.class, "springId");
147+
Field idField = ReflectionUtils.findField(TestIdField.class, "id");
148+
CouchbasePersistentProperty idProperty = getPropertyFor(idField);
149+
CouchbasePersistentProperty springIdProperty = getPropertyFor(springIdField);
150+
BasicCouchbasePersistentEntity<TestIdField> test = new BasicCouchbasePersistentEntity<>(
151+
ClassTypeInformation.from(TestIdField.class));
152+
test.addPersistentProperty(springIdProperty);
153+
assertThatExceptionOfType(MappingException.class).isThrownBy(() -> {
154+
test.addPersistentProperty(idProperty);
155+
});
156+
}
157+
158+
@Test
159+
void testTwoIdFields() { // has @Field("id") springId and id
160+
class TestIdField {
161+
@org.springframework.data.couchbase.core.mapping.Field String name;
162+
String description;
163+
@org.springframework.data.couchbase.core.mapping.Field("id") private String springId;
164+
private String id;
165+
}
166+
Field springIdField = ReflectionUtils.findField(TestIdField.class, "springId");
167+
Field idField = ReflectionUtils.findField(TestIdField.class, "id");
168+
CouchbasePersistentProperty idProperty = getPropertyFor(idField);
169+
CouchbasePersistentProperty springIdProperty = getPropertyFor(springIdField);
170+
BasicCouchbasePersistentEntity<TestIdField> test = new BasicCouchbasePersistentEntity<>(
171+
ClassTypeInformation.from(TestIdField.class));
172+
test.addPersistentProperty(springIdProperty);
173+
assertThatExceptionOfType(MappingException.class).isThrownBy(() -> {
174+
test.addPersistentProperty(idProperty);
175+
});
176+
}
177+
85178
/**
86179
* Helper method to create a property out of the field.
87180
*

‎src/test/java/org/springframework/data/couchbase/core/mapping/MappingCouchbaseConverterTests.java

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,4 +955,104 @@ class Entity {
955955
assertThat(converted.getId()).isEqualTo(entity.id);
956956
assertThat(converted.getId()).isEqualTo(entity.prefix1 + '.' + entity.someId + '.' + entity.suffix);
957957
}
958+
959+
@Test
960+
void idHasIdFieldOnly() {
961+
class Entity {
962+
public String id = "123";
963+
}
964+
Entity entity = new Entity();
965+
CouchbaseDocument converted = new CouchbaseDocument();
966+
converter.write(entity, converted);
967+
assertThat(converted.getId()).isEqualTo(entity.id);
968+
}
969+
970+
@Test
971+
void idHasIdFieldAndAnnotatedId() {
972+
class Entity {
973+
public String id = "123";
974+
@Id public String otherId = "456";
975+
}
976+
Entity entity = new Entity();
977+
CouchbaseDocument converted = new CouchbaseDocument();
978+
converter.write(entity, converted);
979+
assertThat(converted.getId()).isEqualTo(entity.otherId);
980+
}
981+
982+
@Test
983+
void idHasIdFieldAndAnnotatedIdReverse() {
984+
class Entity {
985+
@Id public String otherId = "456";
986+
public String id = "123";
987+
}
988+
Entity entity = new Entity();
989+
CouchbaseDocument converted = new CouchbaseDocument();
990+
converter.write(entity, converted);
991+
assertThat(converted.getId()).isEqualTo(entity.otherId);
992+
}
993+
994+
@Test
995+
void idHasTwoAnnotatedIdFields() {
996+
class Entity {
997+
@Id public String id = "123";
998+
@Id public String otherId = "456";
999+
}
1000+
Entity entity = new Entity();
1001+
CouchbaseDocument converted = new CouchbaseDocument();
1002+
assertThatExceptionOfType(MappingException.class).isThrownBy(() -> {
1003+
converter.write(entity, converted);
1004+
});
1005+
}
1006+
1007+
@Test
1008+
void idHasTwoIdFields() {
1009+
class Entity {
1010+
public String id = "123";
1011+
@Field("id") public String otherId = "456";
1012+
}
1013+
Entity entity = new Entity();
1014+
CouchbaseDocument converted = new CouchbaseDocument();
1015+
assertThatExceptionOfType(MappingException.class).isThrownBy(() -> {
1016+
converter.write(entity, converted);
1017+
});
1018+
}
1019+
1020+
@Test
1021+
void idHasAnnotatedIdAndMultipleIdFields() {
1022+
class Entity { // @Id has precedence, multiple 'id' fields is irrelevant
1023+
@Id public String annotatedId = "123";
1024+
@Field("id") public String otherId0 = "456";
1025+
@Field("id") public String otherId1 = "789";
1026+
}
1027+
Entity entity = new Entity();
1028+
CouchbaseDocument converted = new CouchbaseDocument();
1029+
converter.write(entity, converted);
1030+
assertThat(converted.getId()).isEqualTo(entity.annotatedId);
1031+
}
1032+
1033+
@Test
1034+
void idHasAnnotatedIdAndMultipleIdFieldsReverse() {
1035+
class Entity { // exception will be thrown at otherId1 as it is the second 'id' before @Id has been processed
1036+
@Field("id") public String otherId0 = "456";
1037+
@Field("id") public String otherId1 = "789";
1038+
@Id public String annotatedId = "123";
1039+
}
1040+
Entity entity = new Entity();
1041+
CouchbaseDocument converted = new CouchbaseDocument();
1042+
assertThatExceptionOfType(MappingException.class).isThrownBy(() -> {
1043+
converter.write(entity, converted);
1044+
});
1045+
}
1046+
1047+
@Test
1048+
void idHasNoId() {
1049+
class Entity {
1050+
public String notId = "123";
1051+
}
1052+
Entity entity = new Entity();
1053+
CouchbaseDocument converted = new CouchbaseDocument();
1054+
assertThatExceptionOfType(MappingException.class).isThrownBy(() -> {
1055+
converter.write(entity, converted);
1056+
});
1057+
}
9581058
}

0 commit comments

Comments
 (0)