Skip to content

Commit bc73d24

Browse files
christophstroblThomas Darimont
christophstrobl
authored and
Thomas Darimont
committed
DATAMONGO-1050 - Explicitly annotated Field should not be considered Id.
We changed the id resolution to skip properties having an explicit name set via @field unless they are marked with @id. This means that @field(“id”) String id; will be stored as “id” within mongodb. Prior to this change the fieldname would have been changed to “_id”. Added tests to ensure proper field mapping for various "id" field variants. Original pull request: #225.
1 parent 1c9e32f commit bc73d24

File tree

4 files changed

+225
-25
lines changed

4 files changed

+225
-25
lines changed

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentProperty.java‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ public boolean isIdProperty() {
100100
}
101101

102102
// We need to support a wider range of ID types than just the ones that can be converted to an ObjectId
103-
return SUPPORTED_ID_PROPERTY_NAMES.contains(getName());
103+
// but still we need to check if there happens to be an explicit name set
104+
return SUPPORTED_ID_PROPERTY_NAMES.contains(getName()) && !hasExplicitFieldName();
104105
}
105106

106107
/*
@@ -134,10 +135,8 @@ public String getFieldName() {
134135
}
135136
}
136137

137-
org.springframework.data.mongodb.core.mapping.Field annotation = findAnnotation(org.springframework.data.mongodb.core.mapping.Field.class);
138-
139-
if (annotation != null && StringUtils.hasText(annotation.value())) {
140-
return annotation.value();
138+
if (hasExplicitFieldName()) {
139+
return getAnnotatedFieldName();
141140
}
142141

143142
String fieldName = fieldNamingStrategy.getFieldName(this);
@@ -150,6 +149,26 @@ public String getFieldName() {
150149
return fieldName;
151150
}
152151

152+
/**
153+
* @return true if {@link org.springframework.data.mongodb.core.mapping.Field} having non blank
154+
* {@link org.springframework.data.mongodb.core.mapping.Field#value()} present.
155+
* @since 1.7
156+
*/
157+
protected boolean hasExplicitFieldName() {
158+
return StringUtils.hasText(getAnnotatedFieldName());
159+
}
160+
161+
private String getAnnotatedFieldName() {
162+
163+
org.springframework.data.mongodb.core.mapping.Field annotation = findAnnotation(org.springframework.data.mongodb.core.mapping.Field.class);
164+
165+
if (annotation != null && StringUtils.hasText(annotation.value())) {
166+
return annotation.value();
167+
}
168+
169+
return null;
170+
}
171+
153172
/*
154173
* (non-Javadoc)
155174
* @see org.springframework.data.mongodb.core.mapping.MongoPersistentProperty#getFieldOrder()

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java‎

Lines changed: 122 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,26 @@
1515
*/
1616
package org.springframework.data.mongodb.core.convert;
1717

18-
import static org.hamcrest.Matchers.*;
19-
import static org.junit.Assert.*;
20-
import static org.mockito.Mockito.*;
21-
import static org.springframework.data.mongodb.core.DBObjectTestUtils.*;
18+
import static org.hamcrest.Matchers.arrayWithSize;
19+
import static org.hamcrest.Matchers.equalTo;
20+
import static org.hamcrest.Matchers.hasItem;
21+
import static org.hamcrest.Matchers.hasItems;
22+
import static org.hamcrest.Matchers.hasSize;
23+
import static org.hamcrest.Matchers.instanceOf;
24+
import static org.hamcrest.Matchers.is;
25+
import static org.hamcrest.Matchers.not;
26+
import static org.hamcrest.Matchers.notNullValue;
27+
import static org.hamcrest.Matchers.nullValue;
28+
import static org.junit.Assert.assertEquals;
29+
import static org.junit.Assert.assertThat;
30+
import static org.junit.Assert.assertTrue;
31+
import static org.junit.Assert.fail;
32+
import static org.mockito.Mockito.mock;
33+
import static org.mockito.Mockito.times;
34+
import static org.mockito.Mockito.verify;
35+
import static org.mockito.Mockito.when;
36+
import static org.springframework.data.mongodb.core.DBObjectTestUtils.getAsDBObject;
37+
import static org.springframework.data.mongodb.core.DBObjectTestUtils.getTypedValue;
2238

2339
import java.math.BigDecimal;
2440
import java.math.BigInteger;
@@ -83,6 +99,7 @@
8399

84100
import com.mongodb.BasicDBList;
85101
import com.mongodb.BasicDBObject;
102+
import com.mongodb.BasicDBObjectBuilder;
86103
import com.mongodb.DB;
87104
import com.mongodb.DBObject;
88105
import com.mongodb.DBRef;
@@ -1869,6 +1886,81 @@ public void readShouldRespectExplicitFieldNameForDbRef() {
18691886
Mockito.any(DbRefResolverCallback.class), Mockito.any(DbRefProxyHandler.class));
18701887
}
18711888

1889+
/**
1890+
* @see DATAMONGO-1050
1891+
*/
1892+
@Test
1893+
public void writeShouldUseExplicitFieldnameForIdPropertyWhenAnnotated() {
1894+
1895+
RootForClassWithExplicitlyRenamedIdField source = new RootForClassWithExplicitlyRenamedIdField();
1896+
source.id = "rootId";
1897+
source.nested = new ClassWithExplicitlyRenamedField();
1898+
source.nested.id = "nestedId";
1899+
1900+
DBObject sink = new BasicDBObject();
1901+
converter.write(source, sink);
1902+
1903+
assertThat((String) sink.get("_id"), is("rootId"));
1904+
assertThat((DBObject) sink.get("nested"), is(new BasicDBObjectBuilder().add("id", "nestedId").get()));
1905+
}
1906+
1907+
/**
1908+
* @see DATAMONGO-1050
1909+
*/
1910+
@Test
1911+
public void readShouldUseExplicitFieldnameForIdPropertyWhenAnnotated() {
1912+
1913+
DBObject source = new BasicDBObjectBuilder().add("_id", "rootId")
1914+
.add("nested", new BasicDBObject("id", "nestedId")).get();
1915+
1916+
RootForClassWithExplicitlyRenamedIdField sink = converter.read(RootForClassWithExplicitlyRenamedIdField.class,
1917+
source);
1918+
1919+
assertThat(sink.id, is("rootId"));
1920+
assertThat(sink.nested, notNullValue());
1921+
assertThat(sink.nested.id, is("nestedId"));
1922+
}
1923+
1924+
/**
1925+
* @see DATAMONGO-1050
1926+
*/
1927+
@Test
1928+
public void namedIdFieldShouldExtractValueFromUnderscoreIdField() {
1929+
1930+
DBObject dbo = new BasicDBObjectBuilder().add("_id", "A").add("id", "B").get();
1931+
1932+
ClassWithNamedIdField withNamedIdField = converter.read(ClassWithNamedIdField.class, dbo);
1933+
1934+
assertThat(withNamedIdField.id, is("A"));
1935+
}
1936+
1937+
/**
1938+
* @see DATAMONGO-1050
1939+
*/
1940+
@Test
1941+
public void explicitlyRenamedIfFieldShouldExtractValueFromIdField() {
1942+
1943+
DBObject dbo = new BasicDBObjectBuilder().add("_id", "A").add("id", "B").get();
1944+
1945+
ClassWithExplicitlyRenamedField withExplicitlyRenamedField = converter.read(ClassWithExplicitlyRenamedField.class,
1946+
dbo);
1947+
1948+
assertThat(withExplicitlyRenamedField.id, is("B"));
1949+
}
1950+
1951+
/**
1952+
* @see DATAMONGO-1050
1953+
*/
1954+
@Test
1955+
public void annotatedIdFieldShouldExtractValueFromUnderscoreIdField() {
1956+
1957+
DBObject dbo = new BasicDBObjectBuilder().add("_id", "A").add("id", "B").get();
1958+
1959+
ClassWithAnnotatedIdField withAnnotatedIdField = converter.read(ClassWithAnnotatedIdField.class, dbo);
1960+
1961+
assertThat(withAnnotatedIdField.key, is("A"));
1962+
}
1963+
18721964
static class GenericType<T> {
18731965
T content;
18741966
}
@@ -2128,6 +2220,32 @@ class ClassWithExplicitlyNamedDBRefProperty {
21282220
public ClassWithIntId getDbRefProperty() {
21292221
return dbRefProperty;
21302222
}
2223+
}
2224+
2225+
static class RootForClassWithExplicitlyRenamedIdField {
2226+
2227+
@Id String id;
2228+
ClassWithExplicitlyRenamedField nested;
2229+
}
2230+
2231+
static class ClassWithExplicitlyRenamedField {
2232+
2233+
@Field("id") String id;
2234+
}
2235+
2236+
static class RootForClassWithNamedIdField {
2237+
2238+
String id;
2239+
ClassWithNamedIdField nested;
2240+
}
2241+
2242+
static class ClassWithNamedIdField {
2243+
2244+
String id;
2245+
}
2246+
2247+
static class ClassWithAnnotatedIdField {
21312248

2249+
@Id String key;
21322250
}
21332251
}

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,7 @@ public void getMappedSortIgnoresTextScoreWhenNotSortedByScore() {
659659
}
660660

661661
/**
662+
<<<<<<< HEAD
662663
* @see DATAMONGO-1070
663664
*/
664665
@Test
@@ -673,6 +674,34 @@ public void mapsIdReferenceToDBRefCorrectly() {
673674
com.mongodb.DBRef reference = getTypedValue(result, "reference", com.mongodb.DBRef.class);
674675
assertThat(reference.getId(), is(instanceOf(ObjectId.class)));
675676
}
677+
678+
/**
679+
* @see DATAMONGO-1050
680+
*/
681+
@Test
682+
public void shouldUseExplicitlySetFieldnameForIdPropertyCandidates() {
683+
684+
Query query = query(where("nested.id").is("bar"));
685+
686+
DBObject dbo = mapper.getMappedObject(query.getQueryObject(),
687+
context.getPersistentEntity(RootForClassWithExplicitlyRenamedIdField.class));
688+
689+
assertThat(dbo, equalTo(new BasicDBObjectBuilder().add("nested.id", "bar").get()));
690+
}
691+
692+
/**
693+
* @see DATAMONGO-1050
694+
*/
695+
@Test
696+
public void shouldUseExplicitlySetFieldnameForIdPropertyCandidatesUsedInSortClause() {
697+
698+
Query query = new Query().with(new Sort("nested.id"));
699+
700+
DBObject dbo = mapper.getMappedSort(query.getSortObject(),
701+
context.getPersistentEntity(RootForClassWithExplicitlyRenamedIdField.class));
702+
703+
assertThat(dbo, equalTo(new BasicDBObjectBuilder().add("nested.id", 1).get()));
704+
}
676705

677706
@Document
678707
public class Foo {
@@ -755,4 +784,15 @@ class WithTextScoreProperty {
755784
@Id String id;
756785
@TextScore @Field("score") Float textScore;
757786
}
787+
788+
static class RootForClassWithExplicitlyRenamedIdField {
789+
790+
@Id String id;
791+
ClassWithExplicitlyRenamedField nested;
792+
}
793+
794+
static class ClassWithExplicitlyRenamedField {
795+
796+
@Field("id") String id;
797+
}
758798
}

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentPropertyUnitTests.java‎

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,7 @@ public void rejectsInvalidValueReturnedByFieldNamingStrategy() {
130130
@Test
131131
public void shouldDetectAnnotatedLanguagePropertyCorrectly() {
132132

133-
BasicMongoPersistentEntity<DocumentWithLanguageProperty> persistentEntity = new BasicMongoPersistentEntity<DocumentWithLanguageProperty>(
134-
ClassTypeInformation.from(DocumentWithLanguageProperty.class));
135-
136-
MongoPersistentProperty property = getPropertyFor(persistentEntity, "lang");
133+
MongoPersistentProperty property = getPropertyFor(DocumentWithLanguageProperty.class, "lang");
137134
assertThat(property.isLanguageProperty(), is(true));
138135
}
139136

@@ -143,10 +140,7 @@ public void shouldDetectAnnotatedLanguagePropertyCorrectly() {
143140
@Test
144141
public void shouldDetectIplicitLanguagePropertyCorrectly() {
145142

146-
BasicMongoPersistentEntity<DocumentWithImplicitLanguageProperty> persistentEntity = new BasicMongoPersistentEntity<DocumentWithImplicitLanguageProperty>(
147-
ClassTypeInformation.from(DocumentWithImplicitLanguageProperty.class));
148-
149-
MongoPersistentProperty property = getPropertyFor(persistentEntity, "language");
143+
MongoPersistentProperty property = getPropertyFor(DocumentWithImplicitLanguageProperty.class, "language");
150144
assertThat(property.isLanguageProperty(), is(true));
151145
}
152146

@@ -156,10 +150,7 @@ public void shouldDetectIplicitLanguagePropertyCorrectly() {
156150
@Test
157151
public void shouldDetectTextScorePropertyCorrectly() {
158152

159-
BasicMongoPersistentEntity<DocumentWithTextScoreProperty> persistentEntity = new BasicMongoPersistentEntity<DocumentWithTextScoreProperty>(
160-
ClassTypeInformation.from(DocumentWithTextScoreProperty.class));
161-
162-
MongoPersistentProperty property = getPropertyFor(persistentEntity, "score");
153+
MongoPersistentProperty property = getPropertyFor(DocumentWithTextScoreProperty.class, "score");
163154
assertThat(property.isTextScoreProperty(), is(true));
164155
}
165156

@@ -169,17 +160,39 @@ public void shouldDetectTextScorePropertyCorrectly() {
169160
@Test
170161
public void shouldDetectTextScoreAsReadOnlyProperty() {
171162

172-
BasicMongoPersistentEntity<DocumentWithTextScoreProperty> persistentEntity = new BasicMongoPersistentEntity<DocumentWithTextScoreProperty>(
173-
ClassTypeInformation.from(DocumentWithTextScoreProperty.class));
174-
175-
MongoPersistentProperty property = getPropertyFor(persistentEntity, "score");
163+
MongoPersistentProperty property = getPropertyFor(DocumentWithTextScoreProperty.class, "score");
176164
assertThat(property.isWritable(), is(false));
177165
}
178166

167+
/**
168+
* @see DATAMONGO-1050
169+
*/
170+
@Test
171+
public void shouldNotConsiderExplicitlyNameFieldAsIdProperty() {
172+
173+
MongoPersistentProperty property = getPropertyFor(DocumentWithExplicitlyRenamedIdProperty.class, "id");
174+
assertThat(property.isIdProperty(), is(false));
175+
}
176+
177+
/**
178+
* @see DATAMONGO-1050
179+
*/
180+
@Test
181+
public void shouldConsiderPropertyAsIdWhenExplicitlyAnnotatedWithIdEvenWhenExplicitlyNamePresent() {
182+
183+
MongoPersistentProperty property = getPropertyFor(DocumentWithExplicitlyRenamedIdPropertyHavingIdAnnotation.class,
184+
"id");
185+
assertThat(property.isIdProperty(), is(true));
186+
}
187+
179188
private MongoPersistentProperty getPropertyFor(Field field) {
180189
return getPropertyFor(entity, field);
181190
}
182191

192+
private <T> MongoPersistentProperty getPropertyFor(Class<T> type, String fieldname) {
193+
return getPropertyFor(new BasicMongoPersistentEntity<T>(ClassTypeInformation.from(type)), fieldname);
194+
}
195+
183196
private MongoPersistentProperty getPropertyFor(MongoPersistentEntity<?> persistentEntity, String fieldname) {
184197
return getPropertyFor(persistentEntity, ReflectionUtils.findField(persistentEntity.getType(), fieldname));
185198
}
@@ -230,4 +243,14 @@ static class DocumentWithImplicitLanguageProperty {
230243
static class DocumentWithTextScoreProperty {
231244
@TextScore Float score;
232245
}
246+
247+
static class DocumentWithExplicitlyRenamedIdProperty {
248+
249+
@org.springframework.data.mongodb.core.mapping.Field("id") String id;
250+
}
251+
252+
static class DocumentWithExplicitlyRenamedIdPropertyHavingIdAnnotation {
253+
254+
@Id @org.springframework.data.mongodb.core.mapping.Field("id") String id;
255+
}
233256
}

0 commit comments

Comments
 (0)