From c15ab4c9460f82fcd6c6a6cf8d37f05d59adf160 Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@pivotal.io>
Date: Mon, 6 May 2019 11:34:31 +0200
Subject: [PATCH] DATAMONGO-2264 - Polishing.

Store result to getPersistentEntity() in local variable to reduce invocation count. Use numbered postfix instead of object instantiation to generate a unique distance field name.

Deprecate geoNear methods with hint to aggregations. Fix distance field usage in ReactiveMongoTemplate.

Tweak docs.

Original pull request: #744.
---
 .../data/mongodb/core/EntityOperations.java   | 11 ++---
 .../data/mongodb/core/MongoOperations.java    | 29 ++++++++++++
 .../data/mongodb/core/MongoTemplate.java      |  5 +--
 .../mongodb/core/ReactiveMongoOperations.java | 45 +++++++++++++++----
 .../mongodb/core/ReactiveMongoTemplate.java   |  6 +--
 .../core/ReactiveMongoTemplateUnitTests.java  |  1 -
 .../ReactiveMongoRepositoryTests.java         | 10 ++++-
 src/main/asciidoc/new-features.adoc           |  2 +-
 src/main/asciidoc/reference/mongodb.adoc      | 10 ++---
 9 files changed, 90 insertions(+), 29 deletions(-)

diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java
index 24078c973..b2891531e 100644
--- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java
+++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java
@@ -24,6 +24,7 @@ import java.util.Map;
 import java.util.Optional;
 
 import org.bson.Document;
+
 import org.springframework.core.convert.ConversionService;
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.mapping.IdentifierAccessor;
@@ -44,7 +45,6 @@ import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
-import org.springframework.util.ObjectUtils;
 
 /**
  * Common operations performed on an entity in the context of it's mapping metadata.
@@ -173,14 +173,15 @@ class EntityOperations {
 	 */
 	public String nearQueryDistanceFieldName(Class<?> domainType) {
 
-		if (!context.hasPersistentEntityFor(domainType)
-				|| context.getPersistentEntity(domainType).getPersistentProperty("dis") == null) {
+		MongoPersistentEntity<?> persistentEntity = context.getPersistentEntity(domainType);
+		if (persistentEntity == null || persistentEntity.getPersistentProperty("dis") == null) {
 			return "dis";
 		}
 
 		String distanceFieldName = "calculated-distance";
-		while (context.getPersistentEntity(domainType).getPersistentProperty(distanceFieldName) != null) {
-			distanceFieldName += "-" + ObjectUtils.getIdentityHexString(new Object());
+		int counter = 0;
+		while (persistentEntity.getPersistentProperty(distanceFieldName) != null) {
+			distanceFieldName += "-" + (counter++);
 		}
 
 		return distanceFieldName;
diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoOperations.java
index 82a7badee..91b5ae989 100644
--- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoOperations.java
+++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoOperations.java
@@ -22,6 +22,7 @@ import java.util.function.Consumer;
 import java.util.function.Supplier;
 
 import org.bson.Document;
+
 import org.springframework.data.geo.GeoResults;
 import org.springframework.data.mongodb.core.BulkOperations.BulkMode;
 import org.springframework.data.mongodb.core.aggregation.Aggregation;
@@ -641,24 +642,52 @@ public interface MongoOperations extends FluentMongoOperations {
 	 * information to determine the collection the query is ran against. Note, that MongoDB limits the number of results
 	 * by default. Make sure to add an explicit limit to the {@link NearQuery} if you expect a particular number of
 	 * results.
+	 * <p>
+	 * MongoDB 4.2 has removed the {@code geoNear} command. This method uses since version 2.2 aggregations and the
+	 * {@code $geoNear} aggregation command to emulate {@code geoNear} command functionality. We recommend using
+	 * aggregations directly:
+	 * </p>
+	 *
+	 * <pre class="code">
+	 * TypedAggregation&lt;T&gt; geoNear = TypedAggregation.newAggregation(entityClass, Aggregation.geoNear(near, "dis"))
+	 * 		.withOptions(AggregationOptions.builder().collation(near.getCollation()).build());
+	 * AggregationResults&lt;Document&gt; results = aggregate(geoNear, Document.class);
+	 * </pre>
 	 *
 	 * @param near must not be {@literal null}.
 	 * @param entityClass must not be {@literal null}.
 	 * @return
+	 * @deprecated since 2.2. The {@code eval} command has been removed in MongoDB Server 4.2.0. Use Aggregations with
+	 *             {@link Aggregation#geoNear(NearQuery, String)} instead.
 	 */
+	@Deprecated
 	<T> GeoResults<T> geoNear(NearQuery near, Class<T> entityClass);
 
 	/**
 	 * Returns {@link GeoResults} for all entities matching the given {@link NearQuery}. Note, that MongoDB limits the
 	 * number of results by default. Make sure to add an explicit limit to the {@link NearQuery} if you expect a
 	 * particular number of results.
+	 * <p>
+	 * MongoDB 4.2 has removed the {@code geoNear} command. This method uses since version 2.2 aggregations and the
+	 * {@code $geoNear} aggregation command to emulate {@code geoNear} command functionality. We recommend using
+	 * aggregations directly:
+	 * </p>
+	 *
+	 * <pre class="code">
+	 * TypedAggregation&lt;T&gt; geoNear = TypedAggregation.newAggregation(entityClass, Aggregation.geoNear(near, "dis"))
+	 * 		.withOptions(AggregationOptions.builder().collation(near.getCollation()).build());
+	 * AggregationResults&lt;Document&gt; results = aggregate(geoNear, Document.class);
+	 * </pre>
 	 *
 	 * @param near must not be {@literal null}.
 	 * @param entityClass must not be {@literal null}.
 	 * @param collectionName the collection to trigger the query against. If no collection name is given the entity class
 	 *          will be inspected. Must not be {@literal null} nor empty.
 	 * @return
+	 * @deprecated since 2.2. The {@code eval} command has been removed in MongoDB Server 4.2.0. Use Aggregations with
+	 *             {@link Aggregation#geoNear(NearQuery, String)} instead.
 	 */
+	@Deprecated
 	<T> GeoResults<T> geoNear(NearQuery near, Class<T> entityClass, String collectionName);
 
 	/**
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 389e9f48e..673f64d0e 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
@@ -35,6 +35,7 @@ import org.bson.codecs.Codec;
 import org.bson.conversions.Bson;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 import org.springframework.beans.BeansException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
@@ -3302,9 +3303,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
 
 			double distance = Double.NaN;
 			if (object.containsKey(distanceField)) {
-
-				distance = NumberUtils.convertNumberToTargetClass(object.get(distanceField, Number.class), Double.class)
-						.doubleValue();
+				distance = NumberUtils.convertNumberToTargetClass(object.get(distanceField, Number.class), Double.class);
 			}
 
 			T doWith = delegate.doWith(object);
diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoOperations.java
index 3beb083ca..de27e8297 100644
--- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoOperations.java
+++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoOperations.java
@@ -25,6 +25,7 @@ import java.util.function.Supplier;
 import org.bson.Document;
 import org.reactivestreams.Publisher;
 import org.reactivestreams.Subscription;
+
 import org.springframework.data.geo.GeoResult;
 import org.springframework.data.mongodb.ReactiveMongoDatabaseFactory;
 import org.springframework.data.mongodb.core.aggregation.Aggregation;
@@ -612,24 +613,52 @@ public interface ReactiveMongoOperations extends ReactiveFluentMongoOperations {
 	 * entity mapping information to determine the collection the query is ran against. Note, that MongoDB limits the
 	 * number of results by default. Make sure to add an explicit limit to the {@link NearQuery} if you expect a
 	 * particular number of results.
+	 * <p>
+	 * MongoDB 4.2 has removed the {@code geoNear} command. This method uses since version 2.2 aggregations and the
+	 * {@code $geoNear} aggregation command to emulate {@code geoNear} command functionality. We recommend using
+	 * aggregations directly:
+	 * </p>
+	 *
+	 * <pre class="code">
+	 * TypedAggregation&lt;T&gt; geoNear = TypedAggregation.newAggregation(entityClass, Aggregation.geoNear(near, "dis"))
+	 * 		.withOptions(AggregationOptions.builder().collation(near.getCollation()).build());
+	 * Flux&lt;Document&gt; results = aggregate(geoNear, Document.class);
+	 * </pre>
 	 *
 	 * @param near must not be {@literal null}.
 	 * @param entityClass must not be {@literal null}.
 	 * @return the converted {@link GeoResult}s.
+	 * @deprecated since 2.2. The {@code eval} command has been removed in MongoDB Server 4.2.0. Use Aggregations with
+	 *             {@link Aggregation#geoNear(NearQuery, String)} instead.
 	 */
+	@Deprecated
 	<T> Flux<GeoResult<T>> geoNear(NearQuery near, Class<T> entityClass);
 
 	/**
 	 * Returns {@link Flux} of {@link GeoResult} for all entities matching the given {@link NearQuery}. Note, that MongoDB
 	 * limits the number of results by default. Make sure to add an explicit limit to the {@link NearQuery} if you expect
 	 * a particular number of results.
+	 * <p>
+	 * MongoDB 4.2 has removed the {@code geoNear} command. This method uses since version 2.2 aggregations and the
+	 * {@code $geoNear} aggregation command to emulate {@code geoNear} command functionality. We recommend using
+	 * aggregations directly:
+	 * </p>
+	 *
+	 * <pre class="code">
+	 * TypedAggregation&lt;T&gt; geoNear = TypedAggregation.newAggregation(entityClass, Aggregation.geoNear(near, "dis"))
+	 * 		.withOptions(AggregationOptions.builder().collation(near.getCollation()).build());
+	 * Flux&lt;Document&gt; results = aggregate(geoNear, Document.class);
+	 * </pre>
 	 *
 	 * @param near must not be {@literal null}.
 	 * @param entityClass must not be {@literal null}.
 	 * @param collectionName the collection to trigger the query against. If no collection name is given the entity class
 	 *          will be inspected.
 	 * @return the converted {@link GeoResult}s.
+	 * @deprecated since 2.2. The {@code eval} command has been removed in MongoDB Server 4.2.0. Use Aggregations with
+	 *             {@link Aggregation#geoNear(NearQuery, String)} instead.
 	 */
+	@Deprecated
 	<T> Flux<GeoResult<T>> geoNear(NearQuery near, Class<T> entityClass, String collectionName);
 
 	/**
@@ -933,8 +962,8 @@ public interface ReactiveMongoOperations extends ReactiveFluentMongoOperations {
 	 * If you object has an "Id' property, it will be set with the generated Id from MongoDB. If your Id property is a
 	 * String then MongoDB ObjectId will be used to populate that string. Otherwise, the conversion from ObjectId to your
 	 * property type will be handled by Spring's BeanWrapper class that leverages Type Conversion API. See
-	 * <a href="https://docs.spring.io/spring/docs/current/spring-framework-reference/core.html#validation" > Spring's Type
-	 * Conversion"</a> for more details.
+	 * <a href="https://docs.spring.io/spring/docs/current/spring-framework-reference/core.html#validation" > Spring's
+	 * Type Conversion"</a> for more details.
 	 * <p/>
 	 * <p/>
 	 * Insert is used to initially store the object into the database. To update an existing object use the save method.
@@ -993,8 +1022,8 @@ public interface ReactiveMongoOperations extends ReactiveFluentMongoOperations {
 	 * If you object has an "Id' property, it will be set with the generated Id from MongoDB. If your Id property is a
 	 * String then MongoDB ObjectId will be used to populate that string. Otherwise, the conversion from ObjectId to your
 	 * property type will be handled by Spring's BeanWrapper class that leverages Type Conversion API. See
-	 * <a href="https://docs.spring.io/spring/docs/current/spring-framework-reference/core.html#validation" > Spring's Type
-	 * Conversion"</a> for more details.
+	 * <a href="https://docs.spring.io/spring/docs/current/spring-framework-reference/core.html#validation" > Spring's
+	 * Type Conversion"</a> for more details.
 	 * <p/>
 	 * <p/>
 	 * Insert is used to initially store the object into the database. To update an existing object use the save method.
@@ -1041,8 +1070,8 @@ public interface ReactiveMongoOperations extends ReactiveFluentMongoOperations {
 	 * If you object has an "Id' property, it will be set with the generated Id from MongoDB. If your Id property is a
 	 * String then MongoDB ObjectId will be used to populate that string. Otherwise, the conversion from ObjectId to your
 	 * property type will be handled by Spring's BeanWrapper class that leverages Type Conversion API. See
-	 * <a href="https://docs.spring.io/spring/docs/current/spring-framework-reference/core.html#validation" > Spring's Type
-	 * Conversion"</a> for more details.
+	 * <a href="https://docs.spring.io/spring/docs/current/spring-framework-reference/core.html#validation" > Spring's
+	 * Type Conversion"</a> for more details.
 	 *
 	 * @param objectToSave the object to store in the collection. Must not be {@literal null}.
 	 * @return the saved object.
@@ -1078,8 +1107,8 @@ public interface ReactiveMongoOperations extends ReactiveFluentMongoOperations {
 	 * If you object has an "Id' property, it will be set with the generated Id from MongoDB. If your Id property is a
 	 * String then MongoDB ObjectId will be used to populate that string. Otherwise, the conversion from ObjectId to your
 	 * property type will be handled by Spring's BeanWrapper class that leverages Type Conversion API. See
-	 * <a href="https://docs.spring.io/spring/docs/current/spring-framework-reference/core.html#validation" > Spring's Type
-	 * Conversion"</a> for more details.
+	 * <a href="https://docs.spring.io/spring/docs/current/spring-framework-reference/core.html#validation" > Spring's
+	 * Type Conversion"</a> for more details.
 	 *
 	 * @param objectToSave the object to store in the collection. Must not be {@literal null}.
 	 * @return the saved object.
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 103ca5f49..1eb878ab9 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
@@ -1055,7 +1055,7 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati
 		GeoNearResultDocumentCallback<T> callback = new GeoNearResultDocumentCallback<>(distanceField,
 				new ProjectingReadCallback<>(mongoConverter, entityClass, returnType, collection), near.getMetric());
 
-		Aggregation $geoNear = TypedAggregation.newAggregation(entityClass, Aggregation.geoNear(near, "dis"))
+		Aggregation $geoNear = TypedAggregation.newAggregation(entityClass, Aggregation.geoNear(near, distanceField))
 				.withOptions(AggregationOptions.builder().collation(near.getCollation()).build());
 
 		return aggregate($geoNear, collection, Document.class) //
@@ -3040,9 +3040,7 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati
 
 			double distance = Double.NaN;
 			if (object.containsKey(distanceField)) {
-
-				distance = NumberUtils.convertNumberToTargetClass(object.get(distanceField, Number.class), Double.class)
-						.doubleValue();
+				distance = NumberUtils.convertNumberToTargetClass(object.get(distanceField, Number.class), Double.class);
 			}
 
 			T doWith = delegate.doWith(object);
diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java
index a015c1db8..65c31741a 100644
--- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java
+++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java
@@ -136,7 +136,6 @@ public class ReactiveMongoTemplateUnitTests {
 		this.mappingContext = new MongoMappingContext();
 		this.converter = new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, mappingContext);
 		this.template = new ReactiveMongoTemplate(factory, converter);
-
 	}
 
 	@Test(expected = IllegalArgumentException.class) // DATAMONGO-1444
diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/ReactiveMongoRepositoryTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/ReactiveMongoRepositoryTests.java
index c12d581c1..9176c88e3 100644
--- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/ReactiveMongoRepositoryTests.java
+++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/ReactiveMongoRepositoryTests.java
@@ -321,12 +321,15 @@ public class ReactiveMongoRepositoryTests {
 	}
 
 	@Test // DATAMONGO-1444
-	public void findsPeoplePageableGeoresultByLocationWithinBox() {
+	public void findsPeoplePageableGeoresultByLocationWithinBox() throws InterruptedException {
 
 		Point point = new Point(-73.99171, 40.738868);
 		dave.setLocation(point);
 		StepVerifier.create(repository.save(dave)).expectNextCount(1).verifyComplete();
 
+		// Allow for index creation
+		Thread.sleep(500);
+
 		StepVerifier.create(repository.findByLocationNear(new Point(-73.99, 40.73), //
 				new Distance(2000, Metrics.KILOMETERS), //
 				PageRequest.of(0, 10))) //
@@ -338,12 +341,15 @@ public class ReactiveMongoRepositoryTests {
 	}
 
 	@Test // DATAMONGO-1444
-	public void findsPeopleByLocationWithinBox() {
+	public void findsPeopleByLocationWithinBox() throws InterruptedException {
 
 		Point point = new Point(-73.99171, 40.738868);
 		dave.setLocation(point);
 		StepVerifier.create(repository.save(dave)).expectNextCount(1).verifyComplete();
 
+		// Allow for index creation
+		Thread.sleep(500);
+
 		StepVerifier.create(repository.findPersonByLocationNear(new Point(-73.99, 40.73), //
 				new Distance(2000, Metrics.KILOMETERS))) //
 				.expectNext(dave) //
diff --git a/src/main/asciidoc/new-features.adoc b/src/main/asciidoc/new-features.adoc
index c4adf5561..36b94e4d3 100644
--- a/src/main/asciidoc/new-features.adoc
+++ b/src/main/asciidoc/new-features.adoc
@@ -3,7 +3,7 @@
 
 [[new-features.2-2-0]]
 == What's New in Spring Data MongoDB 2.2
-* Compatible with MongoDB 4.2.
+* Compatibility with MongoDB 4.2 deprecating `eval`, `group` and `geoNear` Template API methods.
 * <<mongo.query.kotlin-support,Type-safe Queries for Kotlin>>
 * <<mongodb.reactive.repositories.queries.type-safe,Querydsl support for reactive repositories>> via `ReactiveQuerydslPredicateExecutor`.
 * <<reactive.gridfs,Reactive GridFS support>>.
diff --git a/src/main/asciidoc/reference/mongodb.adoc b/src/main/asciidoc/reference/mongodb.adoc
index d6f94af34..6fed59d09 100644
--- a/src/main/asciidoc/reference/mongodb.adoc
+++ b/src/main/asciidoc/reference/mongodb.adoc
@@ -1343,16 +1343,16 @@ Spring Data MongoDB 2.2 `MongoOperations#geoNear` uses the `$geoNear` https://do
 instead of the `geoNear` command to run a `NearQuery`.
 
 The calculated distance (the `dis` when using a geoNear command) previously returned within a wrapper type now is embedded
-into the resulting document. If the given domain type already contains a property with equal name, the calculated distance
-is named `calculated-distance` or if that name is taken as well, postfixed with a random hash.
+into the resulting document.
+If the given domain type already contains a property with that name, the calculated distance
+is named `calculated-distance` with a potentially random postfix.
 
-Target types may contain a property named after the returned distance to (additionally) read it back directly into the domain
-type as shown below.
+Target types may contain a property named after the returned distance to (additionally) read it back directly into the domain type as shown below.
 
 [source,java]
 ----
 GeoResults<VenueWithDisField> = template.query(Venue.class) <1>
-    .as(VenueWithDisField.class) <2>
+    .as(VenueWithDisField.class)                            <2>
     .near(NearQuery.near(new GeoJsonPoint(-73.99, 40.73), KILOMETERS))
     .all();
 ----