Restrict implicit id mapping to document types

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentProperty.java

@@ -51,6 +51,7 @@
  * @author Christoph Strobl
  * @author Mark Paluch
  * @author Divya Srivastava
+ * @author dragonfsky
  */
 public class BasicMongoPersistentProperty extends AnnotationBasedPersistentProperty<MongoPersistentProperty>
 		implements MongoPersistentProperty {
@@ -62,6 +63,7 @@ public class BasicMongoPersistentProperty extends AnnotationBasedPersistentPrope
 	private static final Set<String> SUPPORTED_ID_PROPERTY_NAMES = Set.of("id", ID_FIELD_NAME);
 
 	private final FieldNamingStrategy fieldNamingStrategy;
+	private final boolean autoIdFieldMappingOnlyForDocumentTypes;
 
 	/**
 	 * Creates a new {@link BasicMongoPersistentProperty}.
@@ -74,9 +76,16 @@ public class BasicMongoPersistentProperty extends AnnotationBasedPersistentPrope
 	public BasicMongoPersistentProperty(Property property, MongoPersistentEntity<?> owner,
 			SimpleTypeHolder simpleTypeHolder, @Nullable FieldNamingStrategy fieldNamingStrategy) {
 
+		this(property, owner, simpleTypeHolder, fieldNamingStrategy, false);
+	}
+
+	BasicMongoPersistentProperty(Property property, MongoPersistentEntity<?> owner, SimpleTypeHolder simpleTypeHolder,
+			@Nullable FieldNamingStrategy fieldNamingStrategy, boolean autoIdFieldMappingOnlyForDocumentTypes) {
+
 		super(property, owner, simpleTypeHolder);
 		this.fieldNamingStrategy = fieldNamingStrategy == null ? PropertyNameFieldNamingStrategy.INSTANCE
 				: fieldNamingStrategy;
+		this.autoIdFieldMappingOnlyForDocumentTypes = autoIdFieldMappingOnlyForDocumentTypes;
 	}
 
 	/**
@@ -93,7 +102,11 @@ public boolean isIdProperty() {
 
 		// We need to support a wider range of ID types than just the ones that can be converted to an ObjectId
 		// but still we need to check if there happens to be an explicit name set
-		return SUPPORTED_ID_PROPERTY_NAMES.contains(getName()) && !hasExplicitFieldName();
+		if (!SUPPORTED_ID_PROPERTY_NAMES.contains(getName()) || hasExplicitFieldName()) {
+			return false;
+		}
+
+		return !autoIdFieldMappingOnlyForDocumentTypes || getOwner().isAnnotationPresent(Document.class);
 	}
 
 	@Override

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/CachingMongoPersistentProperty.java

@@ -27,6 +27,7 @@
  * @author Oliver Gierke
  * @author Mark Paluch
  * @author Christoph Strobl
+ * @author dragonfsky
  */
 public class CachingMongoPersistentProperty extends BasicMongoPersistentProperty {
 
@@ -60,6 +61,11 @@ public CachingMongoPersistentProperty(Property property, MongoPersistentEntity<?
 		super(property, owner, simpleTypeHolder, fieldNamingStrategy);
 	}
 
+	CachingMongoPersistentProperty(Property property, MongoPersistentEntity<?> owner, SimpleTypeHolder simpleTypeHolder,
+			@Nullable FieldNamingStrategy fieldNamingStrategy, boolean autoIdFieldMappingOnlyForDocumentTypes) {
+		super(property, owner, simpleTypeHolder, fieldNamingStrategy, autoIdFieldMappingOnlyForDocumentTypes);
+	}
+
 	@Override
 	public boolean isEntity() {
 		return isEntity.get();

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoMappingContext.java

@@ -38,6 +38,7 @@
  * @author Jon Brisbin
  * @author Oliver Gierke
  * @author Christoph Strobl
+ * @author dragonfsky
  */
 public class MongoMappingContext extends AbstractMappingContext<MongoPersistentEntity<?>, MongoPersistentProperty>
 		implements ApplicationContextAware {
@@ -46,6 +47,7 @@ public class MongoMappingContext extends AbstractMappingContext<MongoPersistentE
 
 	private FieldNamingStrategy fieldNamingStrategy = DEFAULT_NAMING_STRATEGY;
 	private boolean autoIndexCreation = false;
+	private boolean autoIdFieldMappingOnlyForDocumentTypes = false;
 
 	private @Nullable ApplicationContext applicationContext;
 
@@ -81,7 +83,8 @@ protected boolean shouldCreatePersistentEntityFor(TypeInformation<?> type) {
 	public MongoPersistentProperty createPersistentProperty(Property property, MongoPersistentEntity<?> owner,
 			SimpleTypeHolder simpleTypeHolder) {
 
-		CachingMongoPersistentProperty cachingMongoPersistentProperty = new CachingMongoPersistentProperty(property, owner, simpleTypeHolder, fieldNamingStrategy);
+		CachingMongoPersistentProperty cachingMongoPersistentProperty = new CachingMongoPersistentProperty(property, owner,
+				simpleTypeHolder, fieldNamingStrategy, autoIdFieldMappingOnlyForDocumentTypes);
 		cachingMongoPersistentProperty.validate();
 		return cachingMongoPersistentProperty;
 	}
@@ -125,6 +128,30 @@ public void setAutoIndexCreation(boolean autoCreateIndexes) {
 		this.autoIndexCreation = autoCreateIndexes;
 	}
 
+	/**
+	 * Returns whether implicit {@code id} field mapping to {@code _id} should be applied only to types annotated with
+	 * {@link Document}.
+	 *
+	 * @return {@literal true} to restrict implicit {@code id} field mapping to {@link Document} types.
+	 * @since 5.1
+	 */
+	public boolean isAutoIdFieldMappingOnlyForDocumentTypes() {
+		return autoIdFieldMappingOnlyForDocumentTypes;
+	}
+
+	/**
+	 * Enables/disables restricting implicit {@code id} field mapping to {@code _id} to types annotated with
+	 * {@link Document}. Defaults to {@literal false} to retain the historic behavior of treating unannotated {@code id}
+	 * properties as id properties on every mapped type.
+	 *
+	 * @param autoIdFieldMappingOnlyForDocumentTypes set to {@literal true} to restrict implicit {@code id} field mapping
+	 *          to {@link Document} types.
+	 * @since 5.1
+	 */
+	public void setAutoIdFieldMappingOnlyForDocumentTypes(boolean autoIdFieldMappingOnlyForDocumentTypes) {
+		this.autoIdFieldMappingOnlyForDocumentTypes = autoIdFieldMappingOnlyForDocumentTypes;
+	}
+
 	@Override
 	public @Nullable MongoPersistentEntity<?> getPersistentEntity(MongoPersistentProperty persistentProperty) {
 

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java

@@ -126,6 +126,7 @@
  * @author Roman Puchkovskiy
  * @author Heesu Jung
  * @author Julia Lee
+ * @author dragonfsky
  */
 @ExtendWith(MockitoExtension.class)
 class MappingMongoConverterUnitTests {
@@ -1810,6 +1811,38 @@ void readShouldUseExplicitFieldnameForIdPropertyWhenAnnotated() {
 		assertThat(sink.nested.id).isEqualTo("nestedId");
 	}
 
+	@Test // GH-3351
+	void writeShouldPreserveImplicitIdFieldNameForNestedTypeWhenRestrictedToDocumentTypes() {
+
+		mappingContext.setAutoIdFieldMappingOnlyForDocumentTypes(true);
+
+		DocumentWithNestedImplicitIdField source = new DocumentWithNestedImplicitIdField();
+		source.id = "rootId";
+		source.nested = new ClassWithNamedIdField();
+		source.nested.id = "nestedId";
+
+		org.bson.Document sink = new org.bson.Document();
+		converter.write(source, sink);
+
+		assertThat(sink.get("_id")).isEqualTo("rootId");
+		assertThat(sink.get("nested")).isEqualTo(new org.bson.Document().append("id", "nestedId"));
+	}
+
+	@Test // GH-3351
+	void readShouldPreserveImplicitIdFieldNameForNestedTypeWhenRestrictedToDocumentTypes() {
+
+		mappingContext.setAutoIdFieldMappingOnlyForDocumentTypes(true);
+
+		org.bson.Document source = new org.bson.Document().append("_id", "rootId").append("nested",
+				new org.bson.Document("id", "nestedId"));
+
+		DocumentWithNestedImplicitIdField sink = converter.read(DocumentWithNestedImplicitIdField.class, source);
+
+		assertThat(sink.id).isEqualTo("rootId");
+		assertThat(sink.nested).isNotNull();
+		assertThat(sink.nested.id).isEqualTo("nestedId");
+	}
+
 	@Test // DATAMONGO-1050
 	void namedIdFieldShouldExtractValueFromUnderscoreIdField() {
 
@@ -4112,6 +4145,13 @@ static class RootForClassWithNamedIdField {
 		ClassWithNamedIdField nested;
 	}
 
+	@Document
+	static class DocumentWithNestedImplicitIdField {
+
+		String id;
+		ClassWithNamedIdField nested;
+	}
+
 	static class ClassWithNamedIdField {
 
 		String id;

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java

@@ -88,6 +88,7 @@
  * @author Mark Paluch
  * @author David Julia
  * @author Gyungrai Wang
+ * @author dragonfsky
  */
 public class QueryMapperUnitTests {
 
@@ -856,6 +857,20 @@ void shouldUseExplicitlySetFieldnameForIdPropertyCandidatesUsedInSortClause() {
 		assertThat(document).isEqualTo(new org.bson.Document().append("nested.id", 1));
 	}
 
+	@Test // GH-3351
+	void shouldPreserveImplicitIdFieldNameForNestedTypeWhenRestrictedToDocumentTypes() {
+
+		context.setAutoIdFieldMappingOnlyForDocumentTypes(true);
+
+		String idHex = new ObjectId().toHexString();
+		Query query = query(where("nested.id").is(idHex));
+
+		org.bson.Document document = mapper.getMappedObject(query.getQueryObject(),
+				context.getPersistentEntity(DocumentWithNestedImplicitIdField.class));
+
+		assertThat(document).isEqualTo(new org.bson.Document("nested.id", idHex));
+	}
+
 	@Test // DATAMONGO-1135
 	void nearShouldUseGeoJsonRepresentationOnUnmappedProperty() {
 
@@ -1888,6 +1903,18 @@ static class RootForClassWithExplicitlyRenamedIdField {
 		ClassWithExplicitlyRenamedField nested;
 	}
 
+	@Document
+	static class DocumentWithNestedImplicitIdField {
+
+		String id;
+		ClassWithImplicitIdField nested;
+	}
+
+	static class ClassWithImplicitIdField {
+
+		String id;
+	}
+
 	static class ClassWithExplicitlyRenamedField {
 
 		@Field("id") String id;

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java

@@ -67,6 +67,7 @@
  * @author Pavel Vodrazka
  * @author David Julia
  * @author Divya Srivastava
+ * @author dragonfsky
  */
 @ExtendWith(MockitoExtension.class)
 class UpdateMapperUnitTests {
@@ -629,6 +630,31 @@ void mappingEachOperatorShouldNotAddTypeInfoForNonInterfaceNonAbstractTypes() {
 				.doesNotContainKey("$addToSet.nestedDocs.$each.[1]._class");
 	}
 
+	@Test // GH-3351
+	void mappingShouldPreserveImplicitIdFieldNameForNestedTypeWhenRestrictedToDocumentTypes() {
+
+		context.setAutoIdFieldMappingOnlyForDocumentTypes(true);
+
+		Update update = new Update().set("nested.id", "nested-1");
+		Document mappedUpdate = mapper.getMappedObject(update.getUpdateObject(),
+				context.getPersistentEntity(DocumentWithNestedImplicitIdField.class));
+
+		assertThat(mappedUpdate).isEqualTo(new Document("$set", new Document("nested.id", "nested-1")));
+	}
+
+	@Test // GH-3351
+	void mappingNestedValueShouldPreserveImplicitIdFieldNameWhenRestrictedToDocumentTypes() {
+
+		context.setAutoIdFieldMappingOnlyForDocumentTypes(true);
+
+		Update update = new Update().set("nested", new NestedTypeWithImplicitId("nested-1"));
+		Document mappedUpdate = mapper.getMappedObject(update.getUpdateObject(),
+				context.getPersistentEntity(DocumentWithNestedImplicitIdField.class));
+
+		assertThat(mappedUpdate).containsEntry("$set.nested.id", "nested-1");
+		assertThat(mappedUpdate).doesNotContainKey("$set.nested._id");
+	}
+
 	@Test // DATAMONGO-1210
 	void mappingEachOperatorShouldAddTypeHintForInterfaceTypes() {
 
@@ -1558,6 +1584,22 @@ static class DocumentWithNestedCollection {
 		List<NestedDocument> nestedDocs;
 	}
 
+	@org.springframework.data.mongodb.core.mapping.Document
+	static class DocumentWithNestedImplicitIdField {
+
+		String id;
+		NestedTypeWithImplicitId nested;
+	}
+
+	static class NestedTypeWithImplicitId {
+
+		String id;
+
+		NestedTypeWithImplicitId(String id) {
+			this.id = id;
+		}
+	}
+
 	static class NestedDocument {
 
 		String name;

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentPropertyUnitTests.java

@@ -50,6 +50,7 @@
  * @author Christoph Strobl
  * @author Mark Paluch
  * @author Divya Srivastava
+ * @author dragonfsky
  */
 public class BasicMongoPersistentPropertyUnitTests {
 
@@ -170,6 +171,30 @@ void shouldConsiderPropertyAsIdWhenExplicitlyAnnotatedWithIdEvenWhenExplicitlyNa
 		assertThat(property.isIdProperty()).isTrue();
 	}
 
+	@Test // GH-3351
+	void shouldRestrictImplicitIdPropertyMappingToDocumentTypes() {
+
+		MongoMappingContext context = new MongoMappingContext();
+		context.setAutoIdFieldMappingOnlyForDocumentTypes(true);
+
+		MongoPersistentEntity<?> documentEntity = context.getRequiredPersistentEntity(DocumentWithImplicitIdProperty.class);
+		MongoPersistentEntity<?> nestedEntity = context.getRequiredPersistentEntity(NestedTypeWithImplicitIdProperty.class);
+
+		assertThat(documentEntity.getRequiredPersistentProperty("id").isIdProperty()).isTrue();
+		assertThat(nestedEntity.getRequiredPersistentProperty("id").isIdProperty()).isFalse();
+	}
+
+	@Test // GH-3351
+	void shouldKeepExplicitIdPropertyWhenRestrictingImplicitIdPropertyMapping() {
+
+		MongoMappingContext context = new MongoMappingContext();
+		context.setAutoIdFieldMappingOnlyForDocumentTypes(true);
+
+		MongoPersistentEntity<?> entity = context.getRequiredPersistentEntity(NestedTypeWithExplicitIdProperty.class);
+
+		assertThat(entity.getRequiredPersistentProperty("id").isIdProperty()).isTrue();
+	}
+
 	@Test // DATAMONGO-1373
 	void shouldConsiderComposedAnnotationsForIdField() {
 
@@ -344,6 +369,22 @@ static class DocumentWithExplicitlyRenamedIdPropertyHavingIdAnnotation {
 		@org.springframework.data.mongodb.core.mapping.Field("id") String id;
 	}
 
+	@org.springframework.data.mongodb.core.mapping.Document
+	static class DocumentWithImplicitIdProperty {
+
+		String id;
+	}
+
+	static class NestedTypeWithImplicitIdProperty {
+
+		String id;
+	}
+
+	static class NestedTypeWithExplicitIdProperty {
+
+		@Id String id;
+	}
+
 	static class DocumentWithComposedAnnotations {
 
 		@ComposedIdAnnotation