Skip to content

Commit bf86f39

Browse files
christophstroblmp911de
christophstrobl
authored and
mp911de
committed
Fix id field target type conversion for document references.
This commit fixes an issue where a defined custom target type conversion for the id field was not properly considered when writing a document reference. Previously an eg. String was not being converted into an ObjectId correctly causing lookup queries to return empty results. Converting the id property value on write solves the issue. Includes a minor polish in the mapping centralizing pointer creation within the DocumentPointerFactory. Closes: #3782 Original pull request: #3785.
1 parent f662d7c commit bf86f39

File tree

3 files changed

+114
-18
lines changed

3 files changed

+114
-18
lines changed

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentPointerFactory.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,16 @@ DocumentPointer<?> computePointer(
8383
.getRequiredPersistentEntity(property.getAssociationTargetType());
8484

8585
if (usesDefaultLookup(property)) {
86-
return () -> persistentEntity.getIdentifierAccessor(value).getIdentifier();
86+
87+
MongoPersistentProperty idProperty = persistentEntity.getIdProperty();
88+
Object idValue = persistentEntity.getIdentifierAccessor(value).getIdentifier();
89+
90+
if (idProperty.hasExplicitWriteTarget()
91+
&& conversionService.canConvert(idValue.getClass(), idProperty.getFieldType())) {
92+
return () -> conversionService.convert(idValue, idProperty.getFieldType());
93+
}
94+
95+
return () -> idValue;
8796
}
8897

8998
MongoPersistentEntity<?> valueEntity = mappingContext.getPersistentEntity(value.getClass());

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -869,15 +869,12 @@ protected List<Object> createCollection(Collection<?> collection, MongoPersisten
869869
if (!property.isDbReference()) {
870870

871871
if (property.isAssociation()) {
872-
return writeCollectionInternal(collection.stream().map(it -> {
873-
if (conversionService.canConvert(it.getClass(), DocumentPointer.class)) {
874-
return conversionService.convert(it, DocumentPointer.class).getPointer();
875-
} else {
876-
// just take the id as a reference
877-
return mappingContext.getPersistentEntity(property.getAssociationTargetType()).getIdentifierAccessor(it)
878-
.getIdentifier();
879-
}
880-
}).collect(Collectors.toList()), ClassTypeInformation.from(DocumentPointer.class), new ArrayList<>());
872+
873+
List<Object> targetCollection = collection.stream().map(it -> {
874+
return documentPointerFactory.computePointer(mappingContext, property, it, property.getActualType()).getPointer();
875+
}).collect(Collectors.toList());
876+
877+
return writeCollectionInternal(targetCollection, ClassTypeInformation.from(DocumentPointer.class), new ArrayList<>());
881878
}
882879

883880
if (property.hasExplicitWriteTarget()) {
@@ -930,13 +927,7 @@ protected Bson createMap(Map<Object, Object> map, MongoPersistentProperty proper
930927
if (property.isDbReference()) {
931928
document.put(simpleKey, value != null ? createDBRef(value, property) : null);
932929
} else {
933-
if (conversionService.canConvert(value.getClass(), DocumentPointer.class)) {
934-
document.put(simpleKey, conversionService.convert(value, DocumentPointer.class).getPointer());
935-
} else {
936-
// just take the id as a reference
937-
document.put(simpleKey, mappingContext.getPersistentEntity(property.getAssociationTargetType())
938-
.getIdentifierAccessor(value).getIdentifier());
939-
}
930+
document.put(simpleKey, documentPointerFactory.computePointer(mappingContext, property, value, property.getActualType()).getPointer());
940931
}
941932

942933
} else {

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

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import java.util.Map;
3333

3434
import org.bson.Document;
35+
import org.bson.types.ObjectId;
3536
import org.junit.jupiter.api.BeforeEach;
3637
import org.junit.jupiter.api.Disabled;
3738
import org.junit.jupiter.api.Test;
@@ -44,6 +45,8 @@
4445
import org.springframework.data.mongodb.core.mapping.DocumentPointer;
4546
import org.springframework.data.mongodb.core.mapping.DocumentReference;
4647
import org.springframework.data.mongodb.core.mapping.Field;
48+
import org.springframework.data.mongodb.core.mapping.FieldType;
49+
import org.springframework.data.mongodb.core.mapping.MongoId;
4750
import org.springframework.data.mongodb.core.query.Update;
4851
import org.springframework.data.mongodb.test.util.Client;
4952
import org.springframework.data.mongodb.test.util.MongoClientExtension;
@@ -106,6 +109,26 @@ void writeSimpleTypeReference() {
106109
assertThat(target.get("simpleValueRef")).isEqualTo("ref-1");
107110
}
108111

112+
@Test // GH-3782
113+
void writeTypeReferenceHavingCustomizedIdTargetType() {
114+
115+
ObjectId expectedIdValue = new ObjectId();
116+
String rootCollectionName = template.getCollectionName(SingleRefRoot.class);
117+
118+
SingleRefRoot source = new SingleRefRoot();
119+
source.id = "root-1";
120+
source.customIdTargetRef = new ObjectRefHavingCustomizedIdTargetType(expectedIdValue.toString(),
121+
"me-the-referenced-object");
122+
123+
template.save(source);
124+
125+
Document target = template.execute(db -> {
126+
return db.getCollection(rootCollectionName).find(Filters.eq("_id", "root-1")).first();
127+
});
128+
129+
assertThat(target.get("customIdTargetRef")).isEqualTo(expectedIdValue);
130+
}
131+
109132
@Test // GH-3602
110133
void writeMapTypeReference() {
111134

@@ -126,6 +149,26 @@ void writeMapTypeReference() {
126149
assertThat(target.get("mapValueRef", Map.class)).containsEntry("frodo", "ref-1").containsEntry("bilbo", "ref-2");
127150
}
128151

152+
@Test // GH-3782
153+
void writeMapOfTypeReferenceHavingCustomizedIdTargetType() {
154+
155+
ObjectId expectedIdValue = new ObjectId();
156+
String rootCollectionName = template.getCollectionName(CollectionRefRoot.class);
157+
158+
CollectionRefRoot source = new CollectionRefRoot();
159+
source.id = "root-1";
160+
source.customIdTargetRefMap = Collections.singletonMap("frodo",
161+
new ObjectRefHavingCustomizedIdTargetType(expectedIdValue.toString(), "me-the-referenced-object"));
162+
163+
template.save(source);
164+
165+
Document target = template.execute(db -> {
166+
return db.getCollection(rootCollectionName).find(Filters.eq("_id", "root-1")).first();
167+
});
168+
169+
assertThat(target.get("customIdTargetRefMap", Map.class)).containsEntry("frodo", expectedIdValue);
170+
}
171+
129172
@Test // GH-3602
130173
void writeCollectionOfSimpleTypeReference() {
131174

@@ -145,6 +188,26 @@ void writeCollectionOfSimpleTypeReference() {
145188
assertThat(target.get("simpleValueRef", List.class)).containsExactly("ref-1", "ref-2");
146189
}
147190

191+
@Test // GH-3782
192+
void writeListOfTypeReferenceHavingCustomizedIdTargetType() {
193+
194+
ObjectId expectedIdValue = new ObjectId();
195+
String rootCollectionName = template.getCollectionName(CollectionRefRoot.class);
196+
197+
CollectionRefRoot source = new CollectionRefRoot();
198+
source.id = "root-1";
199+
source.customIdTargetRefList = Collections.singletonList(
200+
new ObjectRefHavingCustomizedIdTargetType(expectedIdValue.toString(), "me-the-referenced-object"));
201+
202+
template.save(source);
203+
204+
Document target = template.execute(db -> {
205+
return db.getCollection(rootCollectionName).find(Filters.eq("_id", "root-1")).first();
206+
});
207+
208+
assertThat(target.get("customIdTargetRefList", List.class)).containsExactly(expectedIdValue);
209+
}
210+
148211
@Test // GH-3602
149212
void writeObjectTypeReference() {
150213

@@ -739,6 +802,26 @@ void updateReferenceWithValue() {
739802
assertThat(target).containsEntry("toB", "b");
740803
}
741804

805+
@Test // GH-3782
806+
void updateReferenceHavingCustomizedIdTargetType() {
807+
808+
ObjectId expectedIdValue = new ObjectId();
809+
String rootCollectionName = template.getCollectionName(SingleRefRoot.class);
810+
811+
SingleRefRoot root = new SingleRefRoot();
812+
root.id = "root-1";
813+
template.save(root);
814+
815+
template.update(SingleRefRoot.class).apply(new Update().set("customIdTargetRef",
816+
new ObjectRefHavingCustomizedIdTargetType(expectedIdValue.toString(), "b"))).first();
817+
818+
Document target = template.execute(db -> {
819+
return db.getCollection(rootCollectionName).find(Filters.eq("_id", "root-1")).first();
820+
});
821+
822+
assertThat(target).containsEntry("customIdTargetRef", expectedIdValue);
823+
}
824+
742825
@Test // GH-3602
743826
void updateReferenceCollectionWithEntity() {
744827

@@ -998,6 +1081,8 @@ static class SingleRefRoot {
9981081

9991082
@DocumentReference(lookup = "{ 'refKey1' : '?#{refKey1}', 'refKey2' : '?#{refKey2}' }", lazy = true) //
10001083
ObjectRefOnNonIdField lazyObjectValueRefOnNonIdFields;
1084+
1085+
@DocumentReference ObjectRefHavingCustomizedIdTargetType customIdTargetRef;
10011086
}
10021087

10031088
@Data
@@ -1027,6 +1112,10 @@ static class CollectionRefRoot {
10271112

10281113
@DocumentReference(lookup = "{ 'refKey1' : '?#{refKey1}', 'refKey2' : '?#{refKey2}' }") //
10291114
List<ObjectRefOnNonIdField> objectValueRefOnNonIdFields;
1115+
1116+
@DocumentReference List<ObjectRefHavingCustomizedIdTargetType> customIdTargetRefList;
1117+
1118+
@DocumentReference Map<String, ObjectRefHavingCustomizedIdTargetType> customIdTargetRefMap;
10301119
}
10311120

10321121
@FunctionalInterface
@@ -1094,6 +1183,14 @@ public Object toReference() {
10941183
}
10951184
}
10961185

1186+
@Data
1187+
@AllArgsConstructor
1188+
static class ObjectRefHavingCustomizedIdTargetType {
1189+
1190+
@MongoId(targetType = FieldType.OBJECT_ID) String id;
1191+
String name;
1192+
}
1193+
10971194
static class ReferencableConverter implements Converter<ReferenceAble, DocumentPointer<Object>> {
10981195

10991196
@Nullable
@@ -1196,5 +1293,4 @@ static class UsingAtReference {
11961293
@Reference //
11971294
Publisher publisher;
11981295
}
1199-
12001296
}

0 commit comments

Comments
 (0)