From 5d39191f0b9ba94efbdb8f022128d71d87399c9a Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 2 Nov 2018 18:37:09 +0100 Subject: [PATCH] DATAMONGO-1798 - Introduce @MongoId annotation for fine grained id conversion control. @MongoId allows more fine grained control over id conversion by specifying the intended id target type. This allows to skip the automatic to ObjectId conversion of values that happen to be valid ObjectId hex strings. public class PlainStringId { @MongoId String id; // treated as String no matter what } public class PlainObjectId { @MongoId ObjectId id; // treated as ObjectId } public class StringToObjectId { @MongoId(FieldType.OBJECT_ID) String id; // treated as ObjectId if the value is a valid ObjectId hex string } Original pull request: #617. --- .../springframework/data/mongodb/MongoId.java | 59 +++++++++++++ .../core/convert/MappingMongoConverter.java | 5 +- .../mongodb/core/convert/MongoConverter.java | 47 +++++++++- .../mongodb/core/convert/QueryMapper.java | 56 +++++++----- .../core/mapping/MongoPersistentEntity.java | 16 ++++ .../core/mapping/MongoPersistentProperty.java | 29 ++++++ .../mongodb/core/MongoTemplateDbRefTests.java | 88 ++++++++++++++++++- .../data/mongodb/core/MongoTemplateTests.java | 35 +++++++- .../MappingMongoConverterUnitTests.java | 21 +++++ .../core/convert/QueryMapperUnitTests.java | 4 +- .../BasicMongoPersistentEntityUnitTests.java | 29 ++++++ ...BasicMongoPersistentPropertyUnitTests.java | 42 +++++++++ src/main/asciidoc/reference/mongodb.adoc | 25 ++++++ 13 files changed, 425 insertions(+), 31 deletions(-) create mode 100644 spring-data-mongodb/src/main/java/org/springframework/data/mongodb/MongoId.java diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/MongoId.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/MongoId.java new file mode 100644 index 000000000..97749f05f --- /dev/null +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/MongoId.java @@ -0,0 +1,59 @@ +/* + * Copyright 2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mongodb; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.springframework.core.annotation.AliasFor; +import org.springframework.data.annotation.Id; + +/** + * {@link MongoId} represents a MongoDB specific {@link Id} annotation that allows tweaking {@literal id} conversion. By + * default {@link Object Class<Object>} will be used as the {@literal id's} target type. This means that the + * actual property value is used. No conversion attempts to any other type is made.
+ * In contrast to {@link Id @Id}, {@link String} {@literal id's} are stored as the such even when the actual value + * represents a valid {@link org.bson.types.ObjectId#isValid(String) ObjectId hex String}. To trigger {@link String} to + * {@link org.bson.types.ObjectId} conversion use {@link MongoId#targetType() @MongoId(ObjectId.class)}. + * + * @author Christoph Strobl + * @since 2.2 + */ +@Id +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.FIELD, ElementType.METHOD, ElementType.ANNOTATION_TYPE }) +public @interface MongoId { + + /** + * @return the preferred id type. + * @see #targetType() + */ + @AliasFor("targetType") + Class value() default Object.class; + + /** + * Get the preferred {@literal _id} type to be used. Defaulted to {@link Object Class<Object>} which used the + * property's type. If defined different, the given value is attempted to be converted into the desired target type + * via {@link org.springframework.data.mongodb.core.convert.MongoConverter#convertId(Object, Class)}. + * + * @return the preferred {@literal id} type. {@link Object Class<Object>} by default. + */ + @AliasFor("value") + Class targetType() default Object.class; + +} 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 9b8ce5563..024ef0b4b 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 @@ -20,6 +20,7 @@ import java.util.Map.Entry; import org.bson.Document; import org.bson.conversions.Bson; +import org.bson.types.ObjectId; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.BeansException; @@ -516,7 +517,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App if (idProperty != null && !dbObjectAccessor.hasValue(idProperty)) { - Object value = idMapper.convertId(accessor.getProperty(idProperty)); + Object value = idMapper.convertId(accessor.getProperty(idProperty), idProperty.getIdType()); if (value != null) { dbObjectAccessor.put(idProperty, value); @@ -981,7 +982,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App throw new MappingException("Cannot create a reference to an object with a NULL id."); } - return dbRefResolver.createDbRef(property == null ? null : property.getDBRef(), entity, idMapper.convertId(id)); + return dbRefResolver.createDbRef(property == null ? null : property.getDBRef(), entity, idMapper.convertId(id, idProperty != null ? idProperty.getIdType() : ObjectId.class)); } throw new MappingException("No id property found on class " + entity.getType()); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConverter.java index c007193b3..38fe3fbc6 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConverter.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConverter.java @@ -18,6 +18,8 @@ package org.springframework.data.mongodb.core.convert; import org.bson.BsonValue; import org.bson.Document; import org.bson.conversions.Bson; +import org.bson.types.ObjectId; +import org.springframework.core.convert.ConversionException; import org.springframework.data.convert.EntityConverter; import org.springframework.data.convert.EntityReader; import org.springframework.data.convert.TypeMapper; @@ -83,7 +85,18 @@ public interface MongoConverter if (sourceDocument.containsKey("$ref") && sourceDocument.containsKey("$id")) { - sourceDocument = dbRefResolver.fetch(new DBRef(sourceDocument.getString("$ref"), sourceDocument.get("$id"))); + Object id = sourceDocument.get("$id"); + String collection = sourceDocument.getString("$ref"); + + MongoPersistentEntity entity = getMappingContext().getPersistentEntity(targetType); + if (entity.getIdType() != null) { + id = convertId(id, entity.getIdType()); + } + + DBRef ref = sourceDocument.containsKey("$db") ? new DBRef(sourceDocument.getString("$db"), collection, id) + : new DBRef(collection, id); + + sourceDocument = dbRefResolver.fetch(ref); if (sourceDocument == null) { return null; } @@ -102,4 +115,36 @@ public interface MongoConverter } return getConversionService().convert(source, targetType); } + + /** + * Converts the given raw id value into either {@link ObjectId} or {@link String}. + * + * @param id + * @return {@literal null} if source {@literal id} is already {@literal null}. + * @since 2.2 + */ + @Nullable + default Object convertId(@Nullable Object id, Class targetType) { + + if (id == null) { + return null; + } + + if (ClassUtils.isAssignable(ObjectId.class, targetType)) { + + if (id instanceof String) { + + if (ObjectId.isValid(id.toString())) { + return new ObjectId(id.toString()); + } + } + } + + try { + return getConversionService().canConvert(id.getClass(), targetType) + ? getConversionService().convert(id, targetType) : convertToMongoType(id, null); + } catch (ConversionException o_O) { + return convertToMongoType(id, null); + } + } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java index 1b89e8a95..00687fa9d 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java @@ -28,7 +28,6 @@ import org.bson.BsonValue; import org.bson.Document; import org.bson.conversions.Bson; import org.bson.types.ObjectId; -import org.springframework.core.convert.ConversionException; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.converter.Converter; import org.springframework.data.domain.Example; @@ -50,6 +49,7 @@ import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import com.mongodb.BasicDBList; import com.mongodb.BasicDBObject; @@ -322,11 +322,11 @@ public class QueryMapper { String inKey = valueDbo.containsField("$in") ? "$in" : "$nin"; List ids = new ArrayList(); for (Object id : (Iterable) valueDbo.get(inKey)) { - ids.add(convertId(id)); + ids.add(convertId(id, getIdTypeForField(documentField))); } resultDbo.put(inKey, ids); } else if (valueDbo.containsField("$ne")) { - resultDbo.put("$ne", convertId(valueDbo.get("$ne"))); + resultDbo.put("$ne", convertId(valueDbo.get("$ne"), getIdTypeForField(documentField))); } else { return getMappedObject(resultDbo, Optional.empty()); } @@ -341,18 +341,18 @@ public class QueryMapper { String inKey = valueDbo.containsKey("$in") ? "$in" : "$nin"; List ids = new ArrayList(); for (Object id : (Iterable) valueDbo.get(inKey)) { - ids.add(convertId(id)); + ids.add(convertId(id, getIdTypeForField(documentField))); } resultDbo.put(inKey, ids); } else if (valueDbo.containsKey("$ne")) { - resultDbo.put("$ne", convertId(valueDbo.get("$ne"))); + resultDbo.put("$ne", convertId(valueDbo.get("$ne"), getIdTypeForField(documentField))); } else { return getMappedObject(resultDbo, Optional.empty()); } return resultDbo; } else { - return convertId(value); + return convertId(value, getIdTypeForField(documentField)); } } @@ -367,6 +367,14 @@ public class QueryMapper { return convertSimpleOrDocument(value, documentField.getPropertyEntity()); } + private boolean isIdField(Field documentField) { + return documentField.getProperty() != null && documentField.getProperty().isIdProperty(); + } + + private Class getIdTypeForField(Field documentField) { + return isIdField(documentField) ? documentField.getProperty().getIdType() : ObjectId.class; + } + /** * Returns whether the given {@link Field} represents an association reference that together with the given value * requires conversion to a {@link org.springframework.data.mongodb.core.mapping.DBRef} object. We check whether the @@ -468,7 +476,14 @@ public class QueryMapper { if (source instanceof DBRef) { DBRef ref = (DBRef) source; - return new DBRef(ref.getCollectionName(), convertId(ref.getId())); + Object id = convertId(ref.getId(), + property != null && property.isIdProperty() ? property.getIdType() : ObjectId.class); + + if (StringUtils.hasText(ref.getDatabaseName())) { + return new DBRef(ref.getDatabaseName(), ref.getCollectionName(), id); + } else { + return new DBRef(ref.getCollectionName(), id); + } } if (source instanceof Iterable) { @@ -549,24 +564,23 @@ public class QueryMapper { * * @param id * @return + * @since 2.2 */ @Nullable public Object convertId(@Nullable Object id) { + return convertId(id, ObjectId.class); + } - if (id == null) { - return null; - } - - if (id instanceof String) { - return ObjectId.isValid(id.toString()) ? conversionService.convert(id, ObjectId.class) : id; - } - - try { - return conversionService.canConvert(id.getClass(), ObjectId.class) ? conversionService.convert(id, ObjectId.class) - : delegateConvertToMongoType(id, null); - } catch (ConversionException o_O) { - return delegateConvertToMongoType(id, null); - } + /** + * Converts the given raw id value into either {@link ObjectId} or {@literal targetType}. + * + * @param id can be {@literal null}. + * @return the converted {@literal id} or {@literal null} if the source was already {@literal null}. + * @since 2.2 + */ + @Nullable + public Object convertId(@Nullable Object id, Class targetType) { + return converter.convertId(id, targetType); } /** diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentEntity.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentEntity.java index 192641202..ababe1458 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentEntity.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentEntity.java @@ -59,4 +59,20 @@ public interface MongoPersistentEntity extends PersistentEntity getIdType() { + + if (!hasIdProperty()) { + return null; + } + + return getIdProperty().getIdType(); + } + } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java index 65419d296..5a91ec9a5 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java @@ -15,10 +15,12 @@ */ package org.springframework.data.mongodb.core.mapping; +import org.bson.types.ObjectId; import org.springframework.core.convert.converter.Converter; import org.springframework.data.annotation.Id; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; +import org.springframework.data.mongodb.MongoId; import org.springframework.lang.Nullable; /** @@ -61,6 +63,33 @@ public interface MongoPersistentProperty extends PersistentProperty + * {@link org.bson.types.ObjectId Class<ObjectId>} indicates the attempt to parse the given raw value as + * {@link org.bson.types.ObjectId}. + * @throws IllegalStateException if the property is not considered an id property. Please make sure to check + * {@link #isIdProperty()}. + * @since 2.2 + */ + default Class getIdType() { + + if (!isIdProperty()) { + + throw new IllegalStateException(String.format("Property '%s' is not considerd an 'id' property", + getField() != null ? getField().getName() : getFieldName())); + } + + MongoId idAnnotation = findAnnotation(MongoId.class); + if (idAnnotation == null) { + return ObjectId.class; + } + + return Object.class.equals(idAnnotation.targetType()) ? getActualType() : idAnnotation.targetType(); + } + /** * Returns true whether the property indicates the documents language either by having a {@link #getFieldName()} equal * to {@literal language} or being annotated with {@link Language}. diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateDbRefTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateDbRefTests.java index 382c3ae30..5e6683462 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateDbRefTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateDbRefTests.java @@ -21,15 +21,19 @@ import static org.springframework.data.mongodb.core.query.Query.*; import lombok.Data; +import java.util.ArrayList; + +import org.bson.types.ObjectId; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; import org.springframework.data.annotation.Id; +import org.springframework.data.mongodb.MongoId; +import org.springframework.data.mongodb.core.convert.LazyLoadingProxy; import org.springframework.data.mongodb.core.mapping.DBRef; import org.springframework.data.mongodb.core.mapping.Document; -import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import com.mongodb.MongoClient; +import com.mongodb.client.model.Filters; /** * {@link org.springframework.data.mongodb.core.mapping.DBRef} related integration tests for @@ -49,6 +53,8 @@ public class MongoTemplateDbRefTests { template.dropCollection(RefCycleLoadingIntoDifferentTypeRoot.class); template.dropCollection(RefCycleLoadingIntoDifferentTypeIntermediate.class); template.dropCollection(RefCycleLoadingIntoDifferentTypeRootView.class); + template.dropCollection(WithDBRefOnRawStringId.class); + template.dropCollection(WithLazyDBRefOnRawStringId.class); } @Test // DATAMONGO-1703 @@ -82,6 +88,63 @@ public class MongoTemplateDbRefTests { assertThat(loaded.getRefToIntermediate().getRefToRootView().getContent()).isEqualTo("jon snow"); } + @Test // DATAMONGO-1798 + public void stringDBRefLoading() { + + RawStringId ref = new RawStringId(); + ref.id = new ObjectId().toHexString(); + ref.value = "new value"; + + template.save(ref); + + WithDBRefOnRawStringId source = new WithDBRefOnRawStringId(); + source.id = "foo"; + source.value = ref; + + template.save(source); + + org.bson.Document result = template + .execute(db -> (org.bson.Document) db.getCollection(template.getCollectionName(WithDBRefOnRawStringId.class)) + .find(Filters.eq("_id", source.id)).limit(1).into(new ArrayList()).iterator().next()); + + assertThat(result).isNotNull(); + assertThat(result.get("value")) + .isEqualTo(new com.mongodb.DBRef(template.getCollectionName(RawStringId.class), ref.getId())); + + WithDBRefOnRawStringId target = template.findOne(query(where("id").is(source.id)), WithDBRefOnRawStringId.class); + assertThat(target.value).isEqualTo(ref); + } + + @Test // DATAMONGO-1798 + public void stringDBRefLazyLoading() { + + RawStringId ref = new RawStringId(); + ref.id = new ObjectId().toHexString(); + ref.value = "new value"; + + template.save(ref); + + WithLazyDBRefOnRawStringId source = new WithLazyDBRefOnRawStringId(); + source.id = "foo"; + source.value = ref; + + template.save(source); + + org.bson.Document result = template.execute( + db -> (org.bson.Document) db.getCollection(template.getCollectionName(WithLazyDBRefOnRawStringId.class)) + .find(Filters.eq("_id", source.id)).limit(1).into(new ArrayList()).iterator().next()); + + assertThat(result).isNotNull(); + assertThat(result.get("value")) + .isEqualTo(new com.mongodb.DBRef(template.getCollectionName(RawStringId.class), ref.getId())); + + WithLazyDBRefOnRawStringId target = template.findOne(query(where("id").is(source.id)), + WithLazyDBRefOnRawStringId.class); + + assertThat(target.value).isInstanceOf(LazyLoadingProxy.class); + assertThat(target.getValue()).isEqualTo(ref); + } + @Data @Document("cycle-with-different-type-root") static class RefCycleLoadingIntoDifferentTypeRoot { @@ -107,4 +170,25 @@ public class MongoTemplateDbRefTests { String content; } + @Data + static class RawStringId { + + @MongoId String id; + String value; + } + + @Data + static class WithDBRefOnRawStringId { + + @Id String id; + @org.springframework.data.mongodb.core.mapping.DBRef RawStringId value; + } + + @Data + static class WithLazyDBRefOnRawStringId { + + @Id String id; + @org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) RawStringId value; + } + } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java index 93dd66e08..5250e5165 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java @@ -26,8 +26,6 @@ import static org.springframework.data.mongodb.core.query.Criteria.*; import static org.springframework.data.mongodb.core.query.Query.*; import static org.springframework.data.mongodb.core.query.Update.*; -import com.mongodb.BasicDBObject; -import com.mongodb.DBObject; import lombok.AllArgsConstructor; import lombok.Data; import lombok.EqualsAndHashCode; @@ -44,7 +42,6 @@ import java.util.*; import java.util.stream.Collectors; import java.util.stream.IntStream; -import org.bson.Document; import org.bson.types.ObjectId; import org.hamcrest.collection.IsMapContaining; import org.joda.time.DateTime; @@ -75,6 +72,7 @@ import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.context.PersistentEntities; import org.springframework.data.mongodb.InvalidMongoDbApiUsageException; import org.springframework.data.mongodb.MongoDbFactory; +import org.springframework.data.mongodb.MongoId; import org.springframework.data.mongodb.core.convert.DbRefResolver; import org.springframework.data.mongodb.core.convert.DefaultDbRefResolver; import org.springframework.data.mongodb.core.convert.LazyLoadingProxy; @@ -104,6 +102,8 @@ import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; +import com.mongodb.BasicDBObject; +import com.mongodb.DBObject; import com.mongodb.DBRef; import com.mongodb.Mongo; import com.mongodb.MongoException; @@ -113,6 +113,7 @@ import com.mongodb.client.FindIterable; import com.mongodb.client.ListIndexesIterable; import com.mongodb.client.MongoCollection; import com.mongodb.client.MongoCursor; +import com.mongodb.client.model.Filters; import com.mongodb.client.result.DeleteResult; import com.mongodb.client.result.UpdateResult; @@ -233,6 +234,7 @@ public class MongoTemplateTests { template.dropCollection(WithGeoJson.class); template.dropCollection(DocumentWithNestedTypeHavingStringIdProperty.class); template.dropCollection(ImmutableAudited.class); + template.dropCollection(RawStringId.class); } @Test @@ -3633,6 +3635,26 @@ public class MongoTemplateTests { assertThat(read.modified).isEqualTo(result.modified).describedAs("Expected auditing information to be read!"); } + @Test // DATAMONGO-1798 + public void saveAndLoadStringThatIsAnObjectIdAsString() { + + RawStringId source = new RawStringId(); + source.id = new ObjectId().toHexString(); + source.value = "new value"; + + template.save(source); + + org.bson.Document result = template + .execute(db -> (org.bson.Document) db.getCollection(template.getCollectionName(RawStringId.class)) + .find(Filters.eq("_id", source.id)).limit(1).into(new ArrayList()).iterator().next()); + + assertThat(result).isNotNull(); + assertThat(result.get("_id")).isEqualTo(source.id); + + RawStringId target = template.findOne(query(where("id").is(source.id)), RawStringId.class); + assertThat(target).isEqualTo(source); + } + static class TypeWithNumbers { @Id String id; @@ -4134,4 +4156,11 @@ public class MongoTemplateTests { @Id String id; @LastModifiedDate Instant modified; } + + @Data + static class RawStringId { + + @MongoId String id; + String value; + } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java index 1f8b8873f..655d64705 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java @@ -1913,6 +1913,27 @@ public class MappingMongoConverterUnitTests { assertThat(target).doesNotContainKeys("_class"); } + @Test // DATAMONGO-1798 + public void convertStringIdThatIsAnObjectIdHexToObjectIdIfTargetIsObjectId() { + + ObjectId source = new ObjectId(); + assertThat(converter.convertId(source.toHexString(), ObjectId.class)).isEqualTo(source); + } + + @Test // DATAMONGO-1798 + public void donNotConvertStringIdThatIsAnObjectIdHexToObjectIdIfTargetIsString() { + + ObjectId source = new ObjectId(); + assertThat(converter.convertId(source.toHexString(), String.class)).isEqualTo(source.toHexString()); + } + + @Test // DATAMONGO-1798 + public void donNotConvertStringIdThatIsAnObjectIdHexToObjectIdIfTargetIsObject() { + + ObjectId source = new ObjectId(); + assertThat(converter.convertId(source.toHexString(), Object.class)).isEqualTo(source.toHexString()); + } + static class GenericType { T content; } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java index a55c2ccee..37898b3a5 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java @@ -615,12 +615,12 @@ public class QueryMapperUnitTests { assertThat(document, equalTo(new org.bson.Document().append("_id", 1))); } - @Test // DATAMONGO-1070 + @Test // DATAMONGO-1070, DATAMONGO-1798 public void mapsIdReferenceToDBRefCorrectly() { ObjectId id = new ObjectId(); - org.bson.Document query = new org.bson.Document("reference.id", new com.mongodb.DBRef("reference", id.toString())); + org.bson.Document query = new org.bson.Document("reference.id", new com.mongodb.DBRef("reference", id)); org.bson.Document result = mapper.getMappedObject(query, context.getPersistentEntity(WithDBRef.class)); assertThat(result.containsKey("reference"), is(true)); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentEntityUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentEntityUnitTests.java index a173dd188..4838a9b41 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentEntityUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentEntityUnitTests.java @@ -237,6 +237,35 @@ public class BasicMongoPersistentEntityUnitTests { assertThat(entity.getCollection()).isEqualTo("collectionName"); } + @Test // DATAMONGO-1798 + public void idTypeShouldReadFromIdProperty() { + + doReturn(true).when(propertyMock).isIdProperty(); + doReturn(String.class).when(propertyMock).getIdType(); + + BasicMongoPersistentEntity entity = new BasicMongoPersistentEntity( + ClassTypeInformation.from(AnyDocument.class)); + entity.addPersistentProperty(propertyMock); + + assertThat(entity.getIdType()).isEqualTo(String.class); + + verify(propertyMock).getIdType(); + } + + @Test // DATAMONGO-1798 + public void idTypeShouldReturnNullForNonIdProperty() { + + doReturn(false).when(propertyMock).isIdProperty(); + + BasicMongoPersistentEntity entity = new BasicMongoPersistentEntity( + ClassTypeInformation.from(AnyDocument.class)); + entity.addPersistentProperty(propertyMock); + + assertThat(entity.getIdType()).isNull(); + + verify(propertyMock, never()).getIdType(); + } + @Document("contacts") class Contact {} diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentPropertyUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentPropertyUnitTests.java index 26380d884..bac44394d 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentPropertyUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentPropertyUnitTests.java @@ -28,6 +28,8 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; +import org.assertj.core.api.Assertions; +import org.bson.types.ObjectId; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -40,6 +42,7 @@ import org.springframework.data.mapping.model.FieldNamingStrategy; import org.springframework.data.mapping.model.Property; import org.springframework.data.mapping.model.PropertyNameFieldNamingStrategy; import org.springframework.data.mapping.model.SimpleTypeHolder; +import org.springframework.data.mongodb.MongoId; import org.springframework.data.util.ClassTypeInformation; import org.springframework.util.ReflectionUtils; @@ -202,6 +205,35 @@ public class BasicMongoPersistentPropertyUnitTests { assertThat(properties).containsExactly("first", "second", "third"); } + @Test // DATAMONGO-1798 + public void idTypeShouldThrowExceptionForNonIdProperties() { + + MongoPersistentProperty property = getPropertyFor(Person.class, "lastname"); + Assertions.assertThatThrownBy(() -> property.getIdType()).isInstanceOf(IllegalStateException.class) + .hasMessageStartingWith("Property 'lastname'"); + } + + @Test // DATAMONGO-1798 + public void idTypeShouldBeObjectIdForPropertiesAnnotatedWithCommonsId() { + + MongoPersistentProperty property = getPropertyFor(Person.class, "id"); + assertThat(property.getIdType()).isEqualTo(ObjectId.class); + } + + @Test // DATAMONGO-1798 + public void idTypeShouldBeStringForPropertiesAnnotatedWithMongoId() { + + MongoPersistentProperty property = getPropertyFor(WithStringMongoId.class, "id"); + assertThat(property.getIdType()).isEqualTo(String.class); + } + + @Test // DATAMONGO-1798 + public void idTypeShouldBeObjectIdForPropertiesAnnotatedWithMongoIdAndTargetTypeObjectId() { + + MongoPersistentProperty property = getPropertyFor(WithStringMongoIdMappedToObjectId.class, "id"); + assertThat(property.getIdType()).isEqualTo(ObjectId.class); + } + private MongoPersistentProperty getPropertyFor(Field field) { return getPropertyFor(entity, field); } @@ -298,4 +330,14 @@ public class BasicMongoPersistentPropertyUnitTests { @Id static @interface ComposedIdAnnotation { } + + static class WithStringMongoId { + + @MongoId String id; + } + + static class WithStringMongoIdMappedToObjectId { + + @MongoId(ObjectId.class) String id; + } } diff --git a/src/main/asciidoc/reference/mongodb.adoc b/src/main/asciidoc/reference/mongodb.adoc index 60a9f473b..bf9d3e20c 100644 --- a/src/main/asciidoc/reference/mongodb.adoc +++ b/src/main/asciidoc/reference/mongodb.adoc @@ -669,6 +669,31 @@ If no field or property specified in the previous sets of rules is present in th When querying and updating, `MongoTemplate` uses the converter that corresponds to the preceding rules for saving documents so that field names and types used in your queries can match what is in your domain classes. +Some environments however require a different approach to mapping `Id` values. Maybe data has feed to mongodb not running throught the Spring Data mapping layer, an thus containing plain `String` values as `id` that represent a valid `ObjectId`. +Reading documents from the store back to the domain type works just fine but querying for documents via their `id` is cumbersome due to the `ObjectId` conversion. Therefore documents cannot be retrieved that way. +For those cases `@MongoId` provides more control over the actual id mapping attempts. + +.`@MongoId` mapping +==== +[source,java] +---- +public class PlainStringId { + @MongoId String id; <1> +} + +public class PlainObjectId { + @MongoId ObjectId id; <2> +} + +public class StringToObjectId { + @MongoId(ObjectId.class) String id; <3> +} +---- +<1> The id is treated as `String` no matter what. +<2> The id is treated as `ObjectId`. +<3> The id is treated as `ObjectId` if the given `String` is a valid ObjectId hex, otherwise as String. +==== + [[mongo-template.type-mapping]] === Type Mapping