From 4c6f79387092995087ad620e3bd78aa07f0d9196 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 13 Nov 2018 14:14:40 +0100 Subject: [PATCH] DATAMONGO-1798 - Polishing. Introduce FieldType to express the desired field type to use for MongoDB Id conversion. Adapt Querydsl Id conversion so Id values are converted in the QueryMapper and no longer in SpringDataMongodbSerializer. Adapt tests. Move MongoId from o.s.d.mongodb to o.s.d.m.c.core. Javadoc, reference docs. Original pull request: #617. --- .../core/convert/MappingMongoConverter.java | 5 +- .../mongodb/core/convert/MongoConverter.java | 8 ++- .../mongodb/core/convert/QueryMapper.java | 18 +++-- .../mapping/BasicMongoPersistentProperty.java | 29 ++++++++- .../CachingMongoPersistentProperty.java | 15 +++++ .../data/mongodb/core/mapping/FieldType.java | 65 +++++++++++++++++++ .../mongodb/{ => core/mapping}/MongoId.java | 24 +++---- .../core/mapping/MongoPersistentEntity.java | 16 ----- .../core/mapping/MongoPersistentProperty.java | 38 +++-------- .../support/SpringDataMongodbSerializer.java | 19 ------ .../mongodb/core/MongoTemplateDbRefTests.java | 2 +- .../data/mongodb/core/MongoTemplateTests.java | 2 +- .../BasicMongoPersistentEntityUnitTests.java | 29 --------- ...BasicMongoPersistentPropertyUnitTests.java | 21 +++--- .../QuerydslRepositorySupportTests.java | 42 ++++++++++++ .../SpringDataMongodbSerializerUnitTests.java | 37 ++--------- src/main/asciidoc/reference/mapping.adoc | 5 +- src/main/asciidoc/reference/mongodb.adoc | 10 +-- 18 files changed, 217 insertions(+), 168 deletions(-) create mode 100644 spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/FieldType.java rename spring-data-mongodb/src/main/java/org/springframework/data/mongodb/{ => core/mapping}/MongoId.java (62%) 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 024ef0b4b..4e83dc9a2 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 @@ -517,7 +517,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App if (idProperty != null && !dbObjectAccessor.hasValue(idProperty)) { - Object value = idMapper.convertId(accessor.getProperty(idProperty), idProperty.getIdType()); + Object value = idMapper.convertId(accessor.getProperty(idProperty), idProperty.getFieldType()); if (value != null) { dbObjectAccessor.put(idProperty, value); @@ -982,7 +982,8 @@ 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, idProperty != null ? idProperty.getIdType() : ObjectId.class)); + return dbRefResolver.createDbRef(property == null ? null : property.getDBRef(), entity, + idMapper.convertId(id, idProperty != null ? idProperty.getFieldType() : 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 38fe3fbc6..5e98ac81d 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 @@ -89,8 +89,8 @@ public interface MongoConverter String collection = sourceDocument.getString("$ref"); MongoPersistentEntity entity = getMappingContext().getPersistentEntity(targetType); - if (entity.getIdType() != null) { - id = convertId(id, entity.getIdType()); + if (entity != null && entity.hasIdProperty()) { + id = convertId(id, entity.getIdProperty().getFieldType()); } DBRef ref = sourceDocument.containsKey("$db") ? new DBRef(sourceDocument.getString("$db"), collection, id) @@ -120,6 +120,7 @@ public interface MongoConverter * Converts the given raw id value into either {@link ObjectId} or {@link String}. * * @param id + * @param targetType * @return {@literal null} if source {@literal id} is already {@literal null}. * @since 2.2 */ @@ -142,7 +143,8 @@ public interface MongoConverter try { return getConversionService().canConvert(id.getClass(), targetType) - ? getConversionService().convert(id, targetType) : convertToMongoType(id, null); + ? 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 00687fa9d..fd6a18155 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 @@ -244,7 +244,16 @@ public class QueryMapper { */ protected Field createPropertyField(@Nullable MongoPersistentEntity entity, String key, MappingContext, MongoPersistentProperty> mappingContext) { - return entity == null ? new Field(key) : new MetadataBackedField(key, entity, mappingContext); + + if (entity == null) { + return new Field(key); + } + + if (Field.ID_KEY.equals(key)) { + return new MetadataBackedField(key, entity, mappingContext, entity.getIdProperty()); + } + + return new MetadataBackedField(key, entity, mappingContext); } /** @@ -372,7 +381,7 @@ public class QueryMapper { } private Class getIdTypeForField(Field documentField) { - return isIdField(documentField) ? documentField.getProperty().getIdType() : ObjectId.class; + return isIdField(documentField) ? documentField.getProperty().getFieldType() : ObjectId.class; } /** @@ -477,7 +486,7 @@ public class QueryMapper { DBRef ref = (DBRef) source; Object id = convertId(ref.getId(), - property != null && property.isIdProperty() ? property.getIdType() : ObjectId.class); + property != null && property.isIdProperty() ? property.getFieldType() : ObjectId.class); if (StringUtils.hasText(ref.getDatabaseName())) { return new DBRef(ref.getDatabaseName(), ref.getCollectionName(), id); @@ -572,9 +581,10 @@ public class QueryMapper { } /** - * Converts the given raw id value into either {@link ObjectId} or {@literal targetType}. + * Converts the given raw id value into either {@link ObjectId} or {@link Class targetType}. * * @param id can be {@literal null}. + * @param targetType * @return the converted {@literal id} or {@literal null} if the source was already {@literal null}. * @since 2.2 */ diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentProperty.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentProperty.java index 59de91d0d..1a6ba33a9 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentProperty.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentProperty.java @@ -67,8 +67,7 @@ public class BasicMongoPersistentProperty extends AnnotationBasedPersistentPrope /** * Creates a new {@link BasicMongoPersistentProperty}. * - * @param field - * @param propertyDescriptor + * @param property * @param owner * @param simpleTypeHolder * @param fieldNamingStrategy @@ -144,6 +143,32 @@ public class BasicMongoPersistentProperty extends AnnotationBasedPersistentPrope return fieldName; } + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.core.mapping.MongoPersistentProperty#getFieldType() + */ + @Override + public Class getFieldType() { + + if (!isIdProperty()) { + return getType(); + } + + MongoId idAnnotation = findAnnotation(MongoId.class); + + if (idAnnotation == null) { + return FieldType.OBJECT_ID.getJavaClass(); + } + + FieldType fieldType = idAnnotation.targetType(); + + if (fieldType == FieldType.IMPLICIT) { + return getType(); + } + + return fieldType.getJavaClass(); + } + /** * @return true if {@link org.springframework.data.mongodb.core.mapping.Field} having non blank * {@link org.springframework.data.mongodb.core.mapping.Field#value()} present. diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/CachingMongoPersistentProperty.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/CachingMongoPersistentProperty.java index 8a64b59b5..577ee90a4 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/CachingMongoPersistentProperty.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/CachingMongoPersistentProperty.java @@ -33,6 +33,7 @@ public class CachingMongoPersistentProperty extends BasicMongoPersistentProperty private @Nullable boolean dbRefResolved; private @Nullable DBRef dbref; private @Nullable String fieldName; + private @Nullable Class fieldType; private @Nullable Boolean usePropertyAccess; private @Nullable Boolean isTransient; @@ -89,6 +90,20 @@ public class CachingMongoPersistentProperty extends BasicMongoPersistentProperty return this.fieldName; } + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.core.mapping.BasicMongoPersistentProperty#getFieldType() + */ + @Override + public Class getFieldType() { + + if (this.fieldType == null) { + this.fieldType = super.getFieldType(); + } + + return this.fieldType; + } + /* * (non-Javadoc) * @see org.springframework.data.mapping.model.AnnotationBasedPersistentProperty#usePropertyAccess() diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/FieldType.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/FieldType.java new file mode 100644 index 000000000..0fe7bbb24 --- /dev/null +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/FieldType.java @@ -0,0 +1,65 @@ +/* + * 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.core.mapping; + +import org.bson.types.ObjectId; + +/** + * Enumeration of field value types that can be used to represent a {@link org.bson.Document} field value. This + * enumeration contains a subset of {@link org.bson.BsonType} that is supported by the mapping and conversion + * components. + *

+ * Bson types are identified by a {@code byte} {@link #getBsonType() value}. This enumeration typically returns the + * according bson type value except for {@link #IMPLICIT} which is a marker to derive the field type from a property. + * + * @author Mark Paluch + * @since 2.2 + * @see org.bson.BsonType + */ +public enum FieldType { + + /** + * Implicit type that is derived from the property value. + */ + IMPLICIT(-1, Object.class), STRING(2, String.class), OBJECT_ID(7, ObjectId.class); + + private final int bsonType; + private final Class javaClass; + + FieldType(int bsonType, Class javaClass) { + + this.bsonType = bsonType; + this.javaClass = javaClass; + } + + /** + * Returns the BSON type identifier. Can be {@code -1} if {@link FieldType} maps to a synthetic Bson type. + * + * @return the BSON type identifier. Can be {@code -1} if {@link FieldType} maps to a synthetic Bson type. + */ + public int getBsonType() { + return bsonType; + } + + /** + * Returns the Java class used to represent the type. + * + * @return the Java class used to represent the type. + */ + public Class getJavaClass() { + return javaClass; + } +} 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/core/mapping/MongoId.java similarity index 62% rename from spring-data-mongodb/src/main/java/org/springframework/data/mongodb/MongoId.java rename to spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoId.java index 97749f05f..05a770c69 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/MongoId.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoId.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.data.mongodb; +package org.springframework.data.mongodb.core.mapping; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; @@ -24,14 +24,16 @@ 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.
+ * {@link MongoId} represents a MongoDB specific {@link Id} annotation that allows customizing {@literal id} conversion. + * Id properties use {@link org.springframework.data.mongodb.core.mapping.FieldType#IMPLICIT} as the default + * {@literal id's} target type. This means that the actual property value is used. No conversion attempts to any other + * type are 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)}. + * {@link org.bson.types.ObjectId} conversion use {@link MongoId#targetType() @MongoId(FieldType.OBJECT_ID)}. * * @author Christoph Strobl + * @author Mark Paluch * @since 2.2 */ @Id @@ -44,16 +46,16 @@ public @interface MongoId { * @see #targetType() */ @AliasFor("targetType") - Class value() default Object.class; + FieldType value() default FieldType.IMPLICIT; /** - * 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)}. + * Get the preferred {@literal _id} type to be used. Defaults to {@link FieldType#IMPLICIT} which uses 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. + * @return the preferred {@literal id} type. {@link FieldType#IMPLICIT} by default. */ @AliasFor("value") - Class targetType() default Object.class; + FieldType targetType() default FieldType.IMPLICIT; } 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 ababe1458..192641202 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,20 +59,4 @@ 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 5a91ec9a5..981eac19e 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,12 +15,10 @@ */ 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; /** @@ -40,6 +38,15 @@ public interface MongoPersistentProperty extends PersistentProperty getFieldType(); + /** * Returns the order of the field if defined. Will return -1 if undefined. * @@ -63,33 +70,6 @@ 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/main/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializer.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializer.java index 8a2e8f4f4..1ba15c5e0 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializer.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializer.java @@ -120,28 +120,9 @@ class SpringDataMongodbSerializer extends MongodbDocumentSerializer { value = value instanceof Optional ? ((Optional) value).orElse(null) : value; - if (ID_KEY.equals(key) || (key != null && key.endsWith("." + ID_KEY))) { - return convertId(key, value); - } - return super.asDocument(key, value instanceof Pattern ? value : converter.convertToMongoType(value)); } - /** - * Convert a given, already known to be an {@literal id} or even a nested document id, value into the according id - * representation following the conversion rules of {@link QueryMapper#convertId(Object)}. - * - * @param key the property path to the given value. - * @param idValue the raw {@literal id} value. - * @return the {@literal id} representation in the required format. - */ - private Document convertId(String key, Object idValue) { - - Object convertedId = mapper.convertId(idValue); - - return mapper.getMappedObject(super.asDocument(key, convertedId), Optional.empty()); - } - /* * (non-Javadoc) * @see com.querydsl.mongodb.MongodbSerializer#isReference(com.querydsl.core.types.Path) 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 5e6683462..463062f50 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 @@ -27,7 +27,7 @@ import org.bson.types.ObjectId; import org.junit.Before; import org.junit.Test; import org.springframework.data.annotation.Id; -import org.springframework.data.mongodb.MongoId; +import org.springframework.data.mongodb.core.mapping.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; 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 5250e5165..b6cbdfa34 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 @@ -72,7 +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.mapping.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; 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 4838a9b41..a173dd188 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,35 +237,6 @@ 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 bac44394d..0d8f9a10a 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,7 +28,6 @@ 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; @@ -42,7 +41,6 @@ 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; @@ -206,32 +204,31 @@ public class BasicMongoPersistentPropertyUnitTests { } @Test // DATAMONGO-1798 - public void idTypeShouldThrowExceptionForNonIdProperties() { + public void fieldTypeShouldReturnActualTypeForNonIdProperties() { MongoPersistentProperty property = getPropertyFor(Person.class, "lastname"); - Assertions.assertThatThrownBy(() -> property.getIdType()).isInstanceOf(IllegalStateException.class) - .hasMessageStartingWith("Property 'lastname'"); + assertThat(property.getFieldType()).isEqualTo(String.class); } @Test // DATAMONGO-1798 - public void idTypeShouldBeObjectIdForPropertiesAnnotatedWithCommonsId() { + public void fieldTypeShouldBeObjectIdForPropertiesAnnotatedWithCommonsId() { MongoPersistentProperty property = getPropertyFor(Person.class, "id"); - assertThat(property.getIdType()).isEqualTo(ObjectId.class); + assertThat(property.getFieldType()).isEqualTo(ObjectId.class); } @Test // DATAMONGO-1798 - public void idTypeShouldBeStringForPropertiesAnnotatedWithMongoId() { + public void fieldTypeShouldBeImplicitForPropertiesAnnotatedWithMongoId() { MongoPersistentProperty property = getPropertyFor(WithStringMongoId.class, "id"); - assertThat(property.getIdType()).isEqualTo(String.class); + assertThat(property.getFieldType()).isEqualTo(String.class); } @Test // DATAMONGO-1798 - public void idTypeShouldBeObjectIdForPropertiesAnnotatedWithMongoIdAndTargetTypeObjectId() { + public void fieldTypeShouldBeObjectIdForPropertiesAnnotatedWithMongoIdAndTargetTypeObjectId() { MongoPersistentProperty property = getPropertyFor(WithStringMongoIdMappedToObjectId.class, "id"); - assertThat(property.getIdType()).isEqualTo(ObjectId.class); + assertThat(property.getFieldType()).isEqualTo(ObjectId.class); } private MongoPersistentProperty getPropertyFor(Field field) { @@ -338,6 +335,6 @@ public class BasicMongoPersistentPropertyUnitTests { static class WithStringMongoIdMappedToObjectId { - @MongoId(ObjectId.class) String id; + @MongoId(FieldType.OBJECT_ID) String id; } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/QuerydslRepositorySupportTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/QuerydslRepositorySupportTests.java index e8fd4b7c4..382c45c16 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/QuerydslRepositorySupportTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/QuerydslRepositorySupportTests.java @@ -29,8 +29,10 @@ import org.junit.runner.RunWith; import org.springframework.beans.DirectFieldAccessor; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.annotation.Id; +import org.springframework.data.mongodb.core.mapping.MongoId; import org.springframework.data.mongodb.core.MongoOperations; import org.springframework.data.mongodb.core.mapping.Document; +import org.springframework.data.mongodb.core.mapping.FieldType; import org.springframework.data.mongodb.core.query.Query; import org.springframework.data.mongodb.repository.Person; import org.springframework.data.mongodb.repository.QPerson; @@ -43,6 +45,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; * * @author Oliver Gierke * @author Christoph Strobl + * @author Mark Paluch */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("classpath:infrastructure.xml") @@ -213,6 +216,38 @@ public class QuerydslRepositorySupportTests { assertThat(query.fetchOne(), equalTo(outer)); } + @Test // DATAMONGO-1798 + public void shouldRetainIdPropertyTypeIfInvalidObjectId() { + + Outer outer = new Outer(); + outer.id = "foobar"; + + operations.save(outer); + + QQuerydslRepositorySupportTests_Outer o = QQuerydslRepositorySupportTests_Outer.outer; + SpringDataMongodbQuery query = repoSupport.from(o).where(o.id.eq(outer.id)); + + assertThat(query.fetchOne(), equalTo(outer)); + } + + @Test // DATAMONGO-1798 + public void shouldUseStringForValidObjectIdHexStrings() { + + WithMongoId document = new WithMongoId(); + document.id = new ObjectId().toHexString(); + + operations.save(document); + + QQuerydslRepositorySupportTests_WithMongoId o = QQuerydslRepositorySupportTests_WithMongoId.withMongoId; + SpringDataMongodbQuery eqQuery = repoSupport.from(o).where(o.id.eq(document.id)); + + assertThat(eqQuery.fetchOne(), equalTo(document)); + + SpringDataMongodbQuery inQuery = repoSupport.from(o).where(o.id.in(document.id)); + + assertThat(inQuery.fetchOne(), equalTo(document)); + } + @Data @Document public static class Outer { @@ -227,4 +262,11 @@ public class QuerydslRepositorySupportTests { @Id String id; String value; } + + @Data + @Document + public static class WithMongoId { + + @MongoId(FieldType.STRING) String id; + } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializerUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializerUnitTests.java index 14f51c091..fb454fa7a 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializerUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializerUnitTests.java @@ -110,8 +110,8 @@ public class SpringDataMongodbSerializerUnitTests { assertThat(serializer.getKeyForPath(address, address.getMetadata()), is("")); } - @Test // DATAMONGO-467 - public void convertsIdPropertyCorrectly() { + @Test // DATAMONGO-467, DATAMONGO-1798 + public void retainsIdPropertyType() { ObjectId id = new ObjectId(); @@ -120,8 +120,8 @@ public class SpringDataMongodbSerializerUnitTests { Document result = (Document) serializer.visit((BooleanOperation) idPath.eq(id.toString()), null); assertThat(result.get("_id"), is(notNullValue())); - assertThat(result.get("_id"), is(instanceOf(ObjectId.class))); - assertThat(result.get("_id"), is(id)); + assertThat(result.get("_id"), is(instanceOf(String.class))); + assertThat(result.get("_id"), is(id.toString())); } @Test // DATAMONGO-761 @@ -134,35 +134,6 @@ public class SpringDataMongodbSerializerUnitTests { assertThat(path, is("0")); } - @Test // DATAMONGO-969 - public void shouldConvertObjectIdEvenWhenNestedInOperatorDbObject() { - - ObjectId value = new ObjectId("53bb9fd14438765b29c2d56e"); - Document serialized = serializer.asDocument("_id", new Document("$ne", value.toString())); - - Document _id = getTypedValue(serialized, "_id", Document.class); - ObjectId $ne = getTypedValue(_id, "$ne", ObjectId.class); - assertThat($ne, is(value)); - } - - @Test // DATAMONGO-969 - public void shouldConvertCollectionOfObjectIdEvenWhenNestedInOperatorDocument() { - - ObjectId firstId = new ObjectId("53bb9fd14438765b29c2d56e"); - ObjectId secondId = new ObjectId("53bb9fda4438765b29c2d56f"); - - List objectIds = new ArrayList<>(); - objectIds.add(firstId.toString()); - objectIds.add(secondId.toString()); - - Document serialized = serializer.asDocument("_id", new Document("$in", objectIds)); - - Document _id = getTypedValue(serialized, "_id", Document.class); - List $in = getTypedValue(_id, "$in", List.class); - - assertThat($in, IsIterableContainingInOrder. contains(firstId, secondId)); - } - @Test // DATAMONGO-1485 public void takesCustomConversionForEnumsIntoAccount() { diff --git a/src/main/asciidoc/reference/mapping.adoc b/src/main/asciidoc/reference/mapping.adoc index d7be327d3..706d4ec07 100644 --- a/src/main/asciidoc/reference/mapping.adoc +++ b/src/main/asciidoc/reference/mapping.adoc @@ -53,7 +53,9 @@ The following outlines what field will be mapped to the `_id` document field: The following outlines what type conversion, if any, will be done on the property mapped to the _id document field. -* If a field named `id` is declared as a String or BigInteger in the Java class it will be converted to and stored as an ObjectId if possible. ObjectId as a field type is also valid. If you specify a value for `id` in your application, the conversion to an ObjectId is detected to the MongoDBdriver. If the specified `id` value cannot be converted to an ObjectId, then the value will be stored as is in the document's _id field. +* If a field named `id` is declared as a String or BigInteger in the Java class it will be converted to and stored as an ObjectId if possible. ObjectId as a field type is also valid. If you specify a value for `id` in your application, the conversion to an ObjectId is detected to the MongoDB driver. If the specified `id` value cannot be converted to an ObjectId, then the value will be stored as is in the document's _id field. This also applies if the field is annotated with `@Id`. +* If a field is annotated with `@MongoId` in the Java class it will be converted to and stored as using its actual type. No further conversion happens unless `@MongoId` declares a desired field type. +* If a field is annotated with `@MongoId(FieldType.…)` in the Java class it will be attempted to convert the value to the declared `FieldType.` * If a field named `id` id field is not declared as a String, BigInteger, or ObjectID in the Java class then you should assign it a value in your application so it can be stored 'as-is' in the document's _id field. * If no field named `id` is present in the Java class then an implicit `_id` file will be generated by the driver but not mapped to a property or field of the Java class. @@ -384,6 +386,7 @@ IMPORTANT: Automatic index creation is only done for types annotated with `@Docu The MappingMongoConverter can use metadata to drive the mapping of objects to documents. The following annotations are available: * `@Id`: Applied at the field level to mark the field used for identity purpose. +* `@MongoId`: Applied at the field level to mark the field used for identity purpose. Accepts an optional `FieldType` to customize id conversion. * `@Document`: Applied at the class level to indicate this class is a candidate for mapping to the database. You can specify the name of the collection where the database will be stored. * `@DBRef`: Applied at the field to indicate it is to be stored using a com.mongodb.DBRef. * `@Indexed`: Applied at the field level to describe how to index the field. diff --git a/src/main/asciidoc/reference/mongodb.adoc b/src/main/asciidoc/reference/mongodb.adoc index bf9d3e20c..09b20330b 100644 --- a/src/main/asciidoc/reference/mongodb.adoc +++ b/src/main/asciidoc/reference/mongodb.adoc @@ -669,8 +669,8 @@ 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. +Some environments require a customized approach to map `Id` values such as data stored in MongoDB that did not run through the Spring Data mapping layer. Documents can contain `_id` values that can be represented either as `ObjectId` or as `String`. +Reading documents from the store back to the domain type works just fine. Querying for documents via their `id` can be cumbersome due to the implicit `ObjectId` conversion. Therefore documents cannot be retrieved that way. For those cases `@MongoId` provides more control over the actual id mapping attempts. .`@MongoId` mapping @@ -686,12 +686,12 @@ public class PlainObjectId { } public class StringToObjectId { - @MongoId(ObjectId.class) String id; <3> + @MongoId(FieldType.OBJECT_ID) String id; <3> } ---- -<1> The id is treated as `String` no matter what. +<1> The id is treated as `String` without further conversion. <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. +<3> The id is treated as `ObjectId` if the given `String` is a valid `ObjectId` hex, otherwise as `String`. Corresponds to `@Id` usage. ==== [[mongo-template.type-mapping]]