From bf86f39b2d5bc2d3c197fbff90551b85b440474a Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 24 Aug 2021 07:31:25 +0200 Subject: [PATCH] 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. --- .../core/convert/DocumentPointerFactory.java | 11 ++- .../core/convert/MappingMongoConverter.java | 23 ++--- .../MongoTemplateDocumentReferenceTests.java | 98 ++++++++++++++++++- 3 files changed, 114 insertions(+), 18 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentPointerFactory.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentPointerFactory.java index 09d69e4b2..b30aa957d 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentPointerFactory.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentPointerFactory.java @@ -83,7 +83,16 @@ class DocumentPointerFactory { .getRequiredPersistentEntity(property.getAssociationTargetType()); if (usesDefaultLookup(property)) { - return () -> persistentEntity.getIdentifierAccessor(value).getIdentifier(); + + MongoPersistentProperty idProperty = persistentEntity.getIdProperty(); + Object idValue = persistentEntity.getIdentifierAccessor(value).getIdentifier(); + + if (idProperty.hasExplicitWriteTarget() + && conversionService.canConvert(idValue.getClass(), idProperty.getFieldType())) { + return () -> conversionService.convert(idValue, idProperty.getFieldType()); + } + + return () -> idValue; } MongoPersistentEntity valueEntity = mappingContext.getPersistentEntity(value.getClass()); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java index 48505559c..a60c853c3 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java @@ -869,15 +869,12 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App if (!property.isDbReference()) { if (property.isAssociation()) { - return writeCollectionInternal(collection.stream().map(it -> { - if (conversionService.canConvert(it.getClass(), DocumentPointer.class)) { - return conversionService.convert(it, DocumentPointer.class).getPointer(); - } else { - // just take the id as a reference - return mappingContext.getPersistentEntity(property.getAssociationTargetType()).getIdentifierAccessor(it) - .getIdentifier(); - } - }).collect(Collectors.toList()), ClassTypeInformation.from(DocumentPointer.class), new ArrayList<>()); + + List targetCollection = collection.stream().map(it -> { + return documentPointerFactory.computePointer(mappingContext, property, it, property.getActualType()).getPointer(); + }).collect(Collectors.toList()); + + return writeCollectionInternal(targetCollection, ClassTypeInformation.from(DocumentPointer.class), new ArrayList<>()); } if (property.hasExplicitWriteTarget()) { @@ -930,13 +927,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App if (property.isDbReference()) { document.put(simpleKey, value != null ? createDBRef(value, property) : null); } else { - if (conversionService.canConvert(value.getClass(), DocumentPointer.class)) { - document.put(simpleKey, conversionService.convert(value, DocumentPointer.class).getPointer()); - } else { - // just take the id as a reference - document.put(simpleKey, mappingContext.getPersistentEntity(property.getAssociationTargetType()) - .getIdentifierAccessor(value).getIdentifier()); - } + document.put(simpleKey, documentPointerFactory.computePointer(mappingContext, property, value, property.getActualType()).getPointer()); } } else { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateDocumentReferenceTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateDocumentReferenceTests.java index fa1deb4f1..d6bcc10e4 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateDocumentReferenceTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateDocumentReferenceTests.java @@ -32,6 +32,7 @@ import java.util.List; import java.util.Map; import org.bson.Document; +import org.bson.types.ObjectId; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; @@ -44,6 +45,8 @@ import org.springframework.data.mongodb.core.convert.LazyLoadingTestUtils; import org.springframework.data.mongodb.core.mapping.DocumentPointer; import org.springframework.data.mongodb.core.mapping.DocumentReference; import org.springframework.data.mongodb.core.mapping.Field; +import org.springframework.data.mongodb.core.mapping.FieldType; +import org.springframework.data.mongodb.core.mapping.MongoId; import org.springframework.data.mongodb.core.query.Update; import org.springframework.data.mongodb.test.util.Client; import org.springframework.data.mongodb.test.util.MongoClientExtension; @@ -106,6 +109,26 @@ public class MongoTemplateDocumentReferenceTests { assertThat(target.get("simpleValueRef")).isEqualTo("ref-1"); } + @Test // GH-3782 + void writeTypeReferenceHavingCustomizedIdTargetType() { + + ObjectId expectedIdValue = new ObjectId(); + String rootCollectionName = template.getCollectionName(SingleRefRoot.class); + + SingleRefRoot source = new SingleRefRoot(); + source.id = "root-1"; + source.customIdTargetRef = new ObjectRefHavingCustomizedIdTargetType(expectedIdValue.toString(), + "me-the-referenced-object"); + + template.save(source); + + Document target = template.execute(db -> { + return db.getCollection(rootCollectionName).find(Filters.eq("_id", "root-1")).first(); + }); + + assertThat(target.get("customIdTargetRef")).isEqualTo(expectedIdValue); + } + @Test // GH-3602 void writeMapTypeReference() { @@ -126,6 +149,26 @@ public class MongoTemplateDocumentReferenceTests { assertThat(target.get("mapValueRef", Map.class)).containsEntry("frodo", "ref-1").containsEntry("bilbo", "ref-2"); } + @Test // GH-3782 + void writeMapOfTypeReferenceHavingCustomizedIdTargetType() { + + ObjectId expectedIdValue = new ObjectId(); + String rootCollectionName = template.getCollectionName(CollectionRefRoot.class); + + CollectionRefRoot source = new CollectionRefRoot(); + source.id = "root-1"; + source.customIdTargetRefMap = Collections.singletonMap("frodo", + new ObjectRefHavingCustomizedIdTargetType(expectedIdValue.toString(), "me-the-referenced-object")); + + template.save(source); + + Document target = template.execute(db -> { + return db.getCollection(rootCollectionName).find(Filters.eq("_id", "root-1")).first(); + }); + + assertThat(target.get("customIdTargetRefMap", Map.class)).containsEntry("frodo", expectedIdValue); + } + @Test // GH-3602 void writeCollectionOfSimpleTypeReference() { @@ -145,6 +188,26 @@ public class MongoTemplateDocumentReferenceTests { assertThat(target.get("simpleValueRef", List.class)).containsExactly("ref-1", "ref-2"); } + @Test // GH-3782 + void writeListOfTypeReferenceHavingCustomizedIdTargetType() { + + ObjectId expectedIdValue = new ObjectId(); + String rootCollectionName = template.getCollectionName(CollectionRefRoot.class); + + CollectionRefRoot source = new CollectionRefRoot(); + source.id = "root-1"; + source.customIdTargetRefList = Collections.singletonList( + new ObjectRefHavingCustomizedIdTargetType(expectedIdValue.toString(), "me-the-referenced-object")); + + template.save(source); + + Document target = template.execute(db -> { + return db.getCollection(rootCollectionName).find(Filters.eq("_id", "root-1")).first(); + }); + + assertThat(target.get("customIdTargetRefList", List.class)).containsExactly(expectedIdValue); + } + @Test // GH-3602 void writeObjectTypeReference() { @@ -739,6 +802,26 @@ public class MongoTemplateDocumentReferenceTests { assertThat(target).containsEntry("toB", "b"); } + @Test // GH-3782 + void updateReferenceHavingCustomizedIdTargetType() { + + ObjectId expectedIdValue = new ObjectId(); + String rootCollectionName = template.getCollectionName(SingleRefRoot.class); + + SingleRefRoot root = new SingleRefRoot(); + root.id = "root-1"; + template.save(root); + + template.update(SingleRefRoot.class).apply(new Update().set("customIdTargetRef", + new ObjectRefHavingCustomizedIdTargetType(expectedIdValue.toString(), "b"))).first(); + + Document target = template.execute(db -> { + return db.getCollection(rootCollectionName).find(Filters.eq("_id", "root-1")).first(); + }); + + assertThat(target).containsEntry("customIdTargetRef", expectedIdValue); + } + @Test // GH-3602 void updateReferenceCollectionWithEntity() { @@ -998,6 +1081,8 @@ public class MongoTemplateDocumentReferenceTests { @DocumentReference(lookup = "{ 'refKey1' : '?#{refKey1}', 'refKey2' : '?#{refKey2}' }", lazy = true) // ObjectRefOnNonIdField lazyObjectValueRefOnNonIdFields; + + @DocumentReference ObjectRefHavingCustomizedIdTargetType customIdTargetRef; } @Data @@ -1027,6 +1112,10 @@ public class MongoTemplateDocumentReferenceTests { @DocumentReference(lookup = "{ 'refKey1' : '?#{refKey1}', 'refKey2' : '?#{refKey2}' }") // List objectValueRefOnNonIdFields; + + @DocumentReference List customIdTargetRefList; + + @DocumentReference Map customIdTargetRefMap; } @FunctionalInterface @@ -1094,6 +1183,14 @@ public class MongoTemplateDocumentReferenceTests { } } + @Data + @AllArgsConstructor + static class ObjectRefHavingCustomizedIdTargetType { + + @MongoId(targetType = FieldType.OBJECT_ID) String id; + String name; + } + static class ReferencableConverter implements Converter> { @Nullable @@ -1196,5 +1293,4 @@ public class MongoTemplateDocumentReferenceTests { @Reference // Publisher publisher; } - }