diff --git a/pom.xml b/pom.xml index 79e5353b7f..1a7ae86263 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 4.0.0-SNAPSHOT + 4.0.0-GH-4026-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index c28a240d2c..3c7d25d0a4 100644 --- a/spring-data-mongodb-benchmarks/pom.xml +++ b/spring-data-mongodb-benchmarks/pom.xml @@ -7,7 +7,7 @@ org.springframework.data spring-data-mongodb-parent - 4.0.0-SNAPSHOT + 4.0.0-GH-4026-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index 5dedcf81ed..39ad464c63 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -15,7 +15,7 @@ org.springframework.data spring-data-mongodb-parent - 4.0.0-SNAPSHOT + 4.0.0-GH-4026-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index b26a926c7a..08ba67d787 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -12,7 +12,7 @@ org.springframework.data spring-data-mongodb-parent - 4.0.0-SNAPSHOT + 4.0.0-GH-4026-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MappedDocument.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MappedDocument.java index 7757cbaf88..83552abda7 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MappedDocument.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MappedDocument.java @@ -97,6 +97,16 @@ public Document getDocument() { return this.document; } + /** + * Updates the documents {@link #ID_FIELD}. + * + * @param value the {@literal _id} value to set. + * @since 3.4.1 + */ + public void updateId(Object value) { + document.put(ID_FIELD, value); + } + /** * An {@link UpdateDefinition} that indicates that the {@link #getUpdateObject() update object} has already been * mapped to the specific domain type. diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java index 3cce235379..2720a9f57b 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java @@ -30,6 +30,7 @@ import org.apache.commons.logging.LogFactory; import org.bson.Document; import org.bson.conversions.Bson; +import org.bson.types.ObjectId; import org.springframework.beans.BeansException; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; @@ -83,6 +84,7 @@ import org.springframework.data.mongodb.core.index.IndexOperationsProvider; import org.springframework.data.mongodb.core.index.MongoMappingEventPublisher; import org.springframework.data.mongodb.core.index.MongoPersistentEntityIndexCreator; +import org.springframework.data.mongodb.core.mapping.MongoId; import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; @@ -1392,18 +1394,20 @@ protected Object insertDocument(String collectionName, Document document, Class< collectionName)); } + MappedDocument mappedDocument = queryOperations.createInsertContext(MappedDocument.of(document)).prepareId(entityClass); + return execute(collectionName, collection -> { MongoAction mongoAction = new MongoAction(writeConcern, MongoActionOperation.INSERT, collectionName, entityClass, - document, null); + mappedDocument.getDocument(), null); WriteConcern writeConcernToUse = prepareWriteConcern(mongoAction); if (writeConcernToUse == null) { - collection.insertOne(document); + collection.insertOne(mappedDocument.getDocument()); } else { - collection.withWriteConcern(writeConcernToUse).insertOne(document); + collection.withWriteConcern(writeConcernToUse).insertOne(mappedDocument.getDocument()); } - return operations.forEntity(document).getId(); + return operations.forEntity(mappedDocument.getDocument()).getId(); }); } @@ -1454,7 +1458,9 @@ protected Object saveDocument(String collectionName, Document dbDoc, Class en : collection.withWriteConcern(writeConcernToUse); if (!mapped.hasId()) { - collectionToUse.insertOne(dbDoc); + + mapped = queryOperations.createInsertContext(mapped).prepareId(mappingContext.getPersistentEntity(entityClass)); + collectionToUse.insertOne(mapped.getDocument()); } else { MongoPersistentEntity entity = mappingContext.getPersistentEntity(entityClass); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryOperations.java index a8633fcf1e..4254aab66d 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryOperations.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryOperations.java @@ -28,7 +28,7 @@ import org.bson.BsonValue; import org.bson.Document; import org.bson.codecs.Codec; - +import org.bson.types.ObjectId; import org.springframework.data.mapping.PropertyPath; import org.springframework.data.mapping.PropertyReferenceException; import org.springframework.data.mapping.context.MappingContext; @@ -46,6 +46,7 @@ import org.springframework.data.mongodb.core.aggregation.TypedAggregation; import org.springframework.data.mongodb.core.convert.QueryMapper; import org.springframework.data.mongodb.core.convert.UpdateMapper; +import org.springframework.data.mongodb.core.mapping.MongoId; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; import org.springframework.data.mongodb.core.mapping.ShardKey; @@ -107,6 +108,14 @@ class QueryOperations { this.aggregationUtil = new AggregationUtil(queryMapper, mappingContext); } + InsertContext createInsertContext(Document source) { + return createInsertContext(MappedDocument.of(source)); + } + + InsertContext createInsertContext(MappedDocument mappedDocument) { + return new InsertContext(mappedDocument); + } + /** * Create a new {@link QueryContext} instance. * @@ -227,6 +236,57 @@ AggregationDefinition createAggregation(Aggregation aggregation, return new AggregationDefinition(aggregation, aggregationOperationContext); } + /** + * {@link InsertContext} encapsulates common tasks required to interact with {@link Document} to be inserted. + * + * @since 3.4.1 + */ + class InsertContext { + + private final MappedDocument source; + + private InsertContext(MappedDocument source) { + this.source = source; + } + + /** + * Prepare the {@literal _id} field. May generate a new {@literal id} value and convert it to the id properties + * {@link MongoPersistentProperty#getFieldType() target type}. + * + * @param type must not be {@literal null}. + * @param + * @return the {@link MappedDocument} containing the changes. + * @see #prepareId(MongoPersistentEntity) + */ + MappedDocument prepareId(Class type) { + return prepareId(mappingContext.getPersistentEntity(type)); + } + + /** + * Prepare the {@literal _id} field. May generate a new {@literal id} value and convert it to the id properties + * {@link MongoPersistentProperty#getFieldType() target type}. + * + * @param entity can be {@literal null}. + * @param + * @return the {@link MappedDocument} containing the changes. + */ + MappedDocument prepareId(@Nullable MongoPersistentEntity entity) { + + if (entity == null) { + return source; + } + + MongoPersistentProperty idProperty = entity.getIdProperty(); + if (idProperty != null + && (idProperty.hasExplicitWriteTarget() || idProperty.isAnnotationPresent(MongoId.class))) { + if (!ClassUtils.isAssignable(ObjectId.class, idProperty.getFieldType())) { + source.updateId(queryMapper.convertId(new ObjectId(), idProperty.getFieldType())); + } + } + return source; + } + } + /** * {@link QueryContext} encapsulates common tasks required to convert a {@link Query} into its MongoDB document * representation, mapping field names, as well as determining and applying {@link Collation collations}. @@ -288,8 +348,7 @@ Document getMappedQuery(@Nullable MongoPersistentEntity entity) { return queryMapper.getMappedObject(getQueryObject(), entity); } - Document getMappedFields(@Nullable MongoPersistentEntity entity, - EntityProjection projection) { + Document getMappedFields(@Nullable MongoPersistentEntity entity, EntityProjection projection) { Document fields = evaluateFields(entity); @@ -402,8 +461,7 @@ private DistinctQueryContext(@Nullable Object query, String fieldName) { } @Override - Document getMappedFields(@Nullable MongoPersistentEntity entity, - EntityProjection projection) { + Document getMappedFields(@Nullable MongoPersistentEntity entity, EntityProjection projection) { return getMappedFields(entity); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java index 67e36d3582..97a5b7a29d 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java @@ -1403,7 +1403,8 @@ protected Mono insertDocument(String collectionName, Document dbDoc, Cla .format("Inserting Document containing fields: " + dbDoc.keySet() + " in collection: " + collectionName)); } - Document document = new Document(dbDoc); + MappedDocument document = MappedDocument.of(dbDoc); + queryOperations.createInsertContext(document).prepareId(entityClass); Flux execute = execute(collectionName, collection -> { @@ -1413,10 +1414,10 @@ protected Mono insertDocument(String collectionName, Document dbDoc, Cla MongoCollection collectionToUse = prepareCollection(collection, writeConcernToUse); - return collectionToUse.insertOne(document); + return collectionToUse.insertOne(document.getDocument()); }); - return Flux.from(execute).last().map(success -> MappedDocument.of(document).getId()); + return Flux.from(execute).last().map(success -> document.getId()); } protected Flux insertDocumentList(String collectionName, List dbDocList) { @@ -1480,7 +1481,7 @@ protected Mono saveDocument(String collectionName, Document document, Cl Publisher publisher; if (!mapped.hasId()) { - publisher = collectionToUse.insertOne(document); + publisher = collectionToUse.insertOne(queryOperations.createInsertContext(mapped).prepareId(entityClass).getDocument()); } else { MongoPersistentEntity entity = mappingContext.getPersistentEntity(entityClass); 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 8c20be1766..653eca6b9b 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 @@ -42,6 +42,7 @@ import java.util.stream.IntStream; import java.util.stream.Stream; +import org.bson.Document; import org.bson.types.ObjectId; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -3635,6 +3636,38 @@ public void saveAndLoadStringThatIsAnObjectIdAsString() { assertThat(target).isEqualTo(source); } + @Test // GH-4026 + void saveShouldGenerateNewIdOfTypeIfExplicitlyDefined() { + + RawStringId source = new RawStringId(); + source.value = "new value"; + + template.save(source); + + template.execute(RawStringId.class, collection -> { + + org.bson.Document first = collection.find(new org.bson.Document()).first(); + assertThat(first.get("_id")).isInstanceOf(String.class); + return null; + }); + } + + @Test // GH-4026 + void insertShouldGenerateNewIdOfTypeIfExplicitlyDefined() { + + RawStringId source = new RawStringId(); + source.value = "new value"; + + template.insert(source); + + template.execute(RawStringId.class, collection -> { + + org.bson.Document first = collection.find(new org.bson.Document()).first(); + assertThat(first.get("_id")).isInstanceOf(String.class); + return null; + }); + } + @Test // DATAMONGO-2193 public void shouldNotConvertStringToObjectIdForNonIdField() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/QueryOperationsUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/QueryOperationsUnitTests.java index afd1658e94..0a01bed2b4 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/QueryOperationsUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/QueryOperationsUnitTests.java @@ -18,6 +18,8 @@ import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; +import org.bson.Document; +import org.bson.types.ObjectId; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -32,7 +34,10 @@ import org.springframework.data.mongodb.core.aggregation.TypeBasedAggregationOperationContext; import org.springframework.data.mongodb.core.convert.QueryMapper; import org.springframework.data.mongodb.core.convert.UpdateMapper; +import org.springframework.data.mongodb.core.mapping.MongoId; import org.springframework.data.mongodb.core.mapping.MongoMappingContext; +import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; +import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; /** * Unit tests for {@link QueryOperations}. @@ -109,7 +114,99 @@ void createAggregationContextUsesStrictlyTypedContextForTypedAggregationsWhenReq assertThat(ctx.getAggregationOperationContext()).isInstanceOf(TypeBasedAggregationOperationContext.class); } + @Test // GH-4026 + void insertContextDoesNotAddIdIfNoPersistentEntityCanBeFound() { + + assertThat(queryOperations.createInsertContext(new Document("value", "one")).prepareId(Person.class).getDocument())// + .satisfies(result -> { + assertThat(result).isEqualTo(new Document("value", "one")); + }); + } + + @Test // GH-4026 + void insertContextDoesNotAddIdIfNoIdPropertyCanBeFound() { + + MongoPersistentEntity entity = mock(MongoPersistentEntity.class); + when(entity.getIdProperty()).thenReturn(null); + when(mappingContext.getPersistentEntity(eq(Person.class))).thenReturn((MongoPersistentEntity) entity); + + assertThat(queryOperations.createInsertContext(new Document("value", "one")).prepareId(Person.class).getDocument())// + .satisfies(result -> { + assertThat(result).isEqualTo(new Document("value", "one")); + }); + } + + @Test // GH-4026 + void insertContextDoesNotAddConvertedIdForNonExplicitFieldTypes() { + + MongoPersistentEntity entity = mock(MongoPersistentEntity.class); + MongoPersistentProperty property = mock(MongoPersistentProperty.class); + when(entity.getIdProperty()).thenReturn(property); + when(property.hasExplicitWriteTarget()).thenReturn(false); + doReturn(entity).when(mappingContext).getPersistentEntity(eq(Person.class)); + + assertThat(queryOperations.createInsertContext(new Document("value", "one")).prepareId(Person.class).getDocument())// + .satisfies(result -> { + assertThat(result).isEqualTo(new Document("value", "one")); + }); + } + + @Test // GH-4026 + void insertContextAddsConvertedIdForExplicitFieldTypes() { + + MongoPersistentEntity entity = mock(MongoPersistentEntity.class); + MongoPersistentProperty property = mock(MongoPersistentProperty.class); + when(entity.getIdProperty()).thenReturn(property); + when(property.hasExplicitWriteTarget()).thenReturn(true); + doReturn(String.class).when(property).getFieldType(); + doReturn(entity).when(mappingContext).getPersistentEntity(eq(Person.class)); + + when(queryMapper.convertId(any(), eq(String.class))).thenReturn("☮"); + + assertThat(queryOperations.createInsertContext(new Document("value", "one")).prepareId(Person.class).getDocument())// + .satisfies(result -> { + assertThat(result).isEqualTo(new Document("value", "one").append("_id", "☮")); + }); + } + + @Test // GH-4026 + void insertContextAddsConvertedIdForMongoIdTypes() { + + MongoPersistentEntity entity = mock(MongoPersistentEntity.class); + MongoPersistentProperty property = mock(MongoPersistentProperty.class); + when(entity.getIdProperty()).thenReturn(property); + when(property.hasExplicitWriteTarget()).thenReturn(false); + when(property.isAnnotationPresent(eq(MongoId.class))).thenReturn(true); + doReturn(String.class).when(property).getFieldType(); + doReturn(entity).when(mappingContext).getPersistentEntity(eq(Person.class)); + + when(queryMapper.convertId(any(), eq(String.class))).thenReturn("☮"); + + assertThat(queryOperations.createInsertContext(new Document("value", "one")).prepareId(Person.class).getDocument())// + .satisfies(result -> { + assertThat(result).isEqualTo(new Document("value", "one").append("_id", "☮")); + }); + } + + @Test // GH-4026 + void insertContextDoesNotAddConvertedIdForMongoIdTypesTargetingObjectId() { + + MongoPersistentEntity entity = mock(MongoPersistentEntity.class); + MongoPersistentProperty property = mock(MongoPersistentProperty.class); + when(entity.getIdProperty()).thenReturn(property); + when(property.hasExplicitWriteTarget()).thenReturn(false); + when(property.isAnnotationPresent(eq(MongoId.class))).thenReturn(true); + doReturn(ObjectId.class).when(property).getFieldType(); + doReturn(entity).when(mappingContext).getPersistentEntity(eq(Person.class)); + + assertThat(queryOperations.createInsertContext(new Document("value", "one")).prepareId(Person.class).getDocument())// + .satisfies(result -> { + assertThat(result).isEqualTo(new Document("value", "one")); + }); + } + static class Person { } + } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java index ae52c56f7e..005c574759 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java @@ -71,6 +71,7 @@ import org.springframework.data.mongodb.core.index.GeospatialIndex; import org.springframework.data.mongodb.core.index.Index; import org.springframework.data.mongodb.core.index.IndexOperationsAdapter; +import org.springframework.data.mongodb.core.mapping.MongoId; import org.springframework.data.mongodb.core.mapping.event.AbstractMongoEventListener; import org.springframework.data.mongodb.core.mapping.event.AfterSaveEvent; import org.springframework.data.mongodb.core.query.Criteria; @@ -117,7 +118,8 @@ public class ReactiveMongoTemplateTests { void setUp() { template - .flush(Person.class, MyPerson.class, Sample.class, Venue.class, PersonWithVersionPropertyOfTypeInteger.class) // + .flush(Person.class, MyPerson.class, Sample.class, Venue.class, PersonWithVersionPropertyOfTypeInteger.class, + RawStringId.class) // .as(StepVerifier::create) // .verifyComplete(); @@ -180,6 +182,42 @@ void saveSetsId() { assertThat(person.getId()).isNotNull(); } + @Test // GH-4026 + void saveShouldGenerateNewIdOfTypeIfExplicitlyDefined() { + + RawStringId source = new RawStringId(); + source.value = "new value"; + + template.save(source).then().as(StepVerifier::create).verifyComplete(); + + template.execute(RawStringId.class, collection -> { + return collection.find(new org.bson.Document()).first(); + }) // + .map(it -> it.get("_id")) // + .as(StepVerifier::create) // + .consumeNextWith(id -> { + assertThat(id).isInstanceOf(String.class); + }).verifyComplete(); + } + + @Test // GH-4026 + void insertShouldGenerateNewIdOfTypeIfExplicitlyDefined() { + + RawStringId source = new RawStringId(); + source.value = "new value"; + + template.insert(source).then().as(StepVerifier::create).verifyComplete(); + + template.execute(RawStringId.class, collection -> { + return collection.find(new org.bson.Document()).first(); + }) // + .map(it -> it.get("_id")) // + .as(StepVerifier::create) // + .consumeNextWith(id -> { + assertThat(id).isInstanceOf(String.class); + }).verifyComplete(); + } + @Test // DATAMONGO-1444 void insertsSimpleEntityCorrectly() { @@ -1820,4 +1858,12 @@ public static class MyPerson { interface MyPersonProjection { String getName(); } + + @Data + static class RawStringId { + + @MongoId String id; + String value; + } + } diff --git a/src/main/asciidoc/reference/mapping.adoc b/src/main/asciidoc/reference/mapping.adoc index f837bfb6d2..9d5ec3a8c5 100644 --- a/src/main/asciidoc/reference/mapping.adoc +++ b/src/main/asciidoc/reference/mapping.adoc @@ -58,8 +58,8 @@ 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 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 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 no value is provided for the `id` field, a new `ObjectId` will be created and converted to the properties 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 no value is provided for the `id` field, a new `ObjectId` will be created and converted to the declared type. * 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.