Skip to content

Commit 909c51d

Browse files
christophstroblmp911de
christophstrobl
authored and
mp911de
committed
DATAMONGO-2059 - Replace usage of deprecated collection.count() with collection.countDocuments().
This commit switches from simple collection.count(), operating on potentially false collection statistic, to countDocuments() using an aggregation for accurate results. The transition required query modifications at some points because $match does not support $near and $nearSphere but require $geoWithin along with $center or $centerSphere which does not support $minDistance (see https://jira.mongodb.org/browse/SERVER-37043). $geoWithin further more does not sort results by distance, but this fact can be ignored when just counting matches. Examples: { location : { $near : [-73.99171, 40.738868], $maxDistance : 1.1 } } { location : { $geoWithin : { $center: [ [-73.99171, 40.738868], 1.1] } } } { location : { $near : [-73.99171, 40.738868], $minDistance : 0.1, $maxDistance : 1.1 } } {$and :[ { $nor :[ { location :{ $geoWithin :{ $center :[ [-73.99171, 40.738868 ], 0.01] } } } ]}, { location :{ $geoWithin :{ $center :[ [-73.99171, 40.738868 ], 1.1] } } } ] } Original pull request: #604.
1 parent 4a04e82 commit 909c51d

13 files changed

+243
-69
lines changed

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import org.springframework.data.geo.GeoResult;
5454
import org.springframework.data.geo.GeoResults;
5555
import org.springframework.data.geo.Metric;
56+
import org.springframework.data.geo.Point;
5657
import org.springframework.data.mapping.PropertyPath;
5758
import org.springframework.data.mapping.PropertyReferenceException;
5859
import org.springframework.data.mapping.callback.EntityCallbacks;
@@ -1191,11 +1192,7 @@ protected long doCount(String collectionName, Document filter, CountOptions opti
11911192
LOGGER.debug("Executing count: {} in collection: {}", serializeToJsonSafely(filter), collectionName);
11921193
}
11931194

1194-
if (MongoDatabaseUtils.isTransactionActive(getMongoDbFactory())) {
1195-
return execute(collectionName, collection -> collection.countDocuments(filter, options));
1196-
}
1197-
1198-
return execute(collectionName, collection -> collection.count(filter, options));
1195+
return execute(collectionName, collection -> collection.countDocuments(QueryMapper.processCountFilter(filter), options));
11991196
}
12001197

12011198
/*
@@ -3523,19 +3520,5 @@ public MongoDatabase getDb() {
35233520
// native MongoDB objects that offer methods with ClientSession must not be proxied.
35243521
return delegate.getDb();
35253522
}
3526-
3527-
/*
3528-
* (non-Javadoc)
3529-
* @see org.springframework.data.mongodb.core.MongoTemplate#doCount(java.lang.String, org.bson.Document, com.mongodb.client.model.CountOptions)
3530-
*/
3531-
@Override
3532-
protected long doCount(String collectionName, Document filter, CountOptions options) {
3533-
3534-
if (!session.hasActiveTransaction()) {
3535-
return super.doCount(collectionName, filter, options);
3536-
}
3537-
3538-
return execute(collectionName, collection -> collection.countDocuments(filter, options));
3539-
}
35403523
}
35413524
}

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,9 +1300,8 @@ public Mono<Long> count(Query query, @Nullable Class<?> entityClass, String coll
13001300
*/
13011301
protected Mono<Long> doCount(String collectionName, Document filter, CountOptions options) {
13021302

1303-
return ReactiveMongoDatabaseUtils.isTransactionActive(mongoDatabaseFactory) //
1304-
.flatMap(txActive -> createMono(collectionName,
1305-
collection -> txActive ? collection.countDocuments(filter, options) : collection.count(filter, options)));
1303+
return createMono(collectionName,
1304+
collection -> collection.countDocuments(QueryMapper.processCountFilter(filter), options));
13061305
}
13071306

13081307
/*
@@ -3323,20 +3322,6 @@ public MongoDatabase getMongoDatabase() {
33233322
// native MongoDB objects that offer methods with ClientSession must not be proxied.
33243323
return delegate.getMongoDatabase();
33253324
}
3326-
3327-
/*
3328-
* (non-Javadoc)
3329-
* @see org.springframework.data.mongodb.core.ReactiveMongoTemplate#count(java.lang.String, org.bson.Document, com.mongodb.client.model.CountOptions)
3330-
*/
3331-
@Override
3332-
public Mono<Long> doCount(String collectionName, Document filter, CountOptions options) {
3333-
3334-
if (!session.hasActiveTransaction()) {
3335-
return super.doCount(collectionName, filter, options);
3336-
}
3337-
3338-
return createMono(collectionName, collection -> collection.countDocuments(filter, options));
3339-
}
33403325
}
33413326

33423327
@RequiredArgsConstructor

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java

Lines changed: 95 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,8 @@
1515
*/
1616
package org.springframework.data.mongodb.core.convert;
1717

18-
import java.util.ArrayList;
19-
import java.util.Arrays;
20-
import java.util.Collections;
21-
import java.util.HashSet;
22-
import java.util.Iterator;
23-
import java.util.LinkedHashMap;
24-
import java.util.List;
25-
import java.util.Map;
18+
import java.util.*;
2619
import java.util.Map.Entry;
27-
import java.util.Optional;
28-
import java.util.Set;
2920
import java.util.regex.Matcher;
3021
import java.util.regex.Pattern;
3122

@@ -36,6 +27,7 @@
3627
import org.springframework.core.convert.ConversionService;
3728
import org.springframework.core.convert.converter.Converter;
3829
import org.springframework.data.domain.Example;
30+
import org.springframework.data.geo.Point;
3931
import org.springframework.data.mapping.Association;
4032
import org.springframework.data.mapping.MappingException;
4133
import org.springframework.data.mapping.PersistentEntity;
@@ -1290,4 +1282,97 @@ public String convert(MongoPersistentProperty source) {
12901282
public MappingContext<? extends MongoPersistentEntity<?>, MongoPersistentProperty> getMappingContext() {
12911283
return mappingContext;
12921284
}
1285+
1286+
public static Document processCountFilter(Document source) {
1287+
1288+
Document target = new Document();
1289+
for (Entry<String, Object> entry : source.entrySet()) {
1290+
1291+
if (entry.getValue() instanceof Document) {
1292+
1293+
Document theValue = (Document) entry.getValue();
1294+
if (containsNear(theValue)) {
1295+
target.putAll(createGeoWithin(entry.getKey(), theValue));
1296+
} else {
1297+
target.put(entry.getKey(), entry.getValue());
1298+
}
1299+
} else if (entry.getValue() instanceof Collection) {
1300+
1301+
Collection<Object> tmp = new ArrayList<>();
1302+
for (Object val : (Collection) entry.getValue()) {
1303+
if (val instanceof Document) {
1304+
tmp.add(processCountFilter((Document) val));
1305+
} else {
1306+
tmp.add(val);
1307+
}
1308+
}
1309+
target.put(entry.getKey(), tmp);
1310+
} else {
1311+
target.put(entry.getKey(), entry.getValue());
1312+
}
1313+
}
1314+
return target;
1315+
}
1316+
1317+
private static Document createGeoWithin(String key, Document source) {
1318+
1319+
boolean spheric = source.containsKey("$nearSphere");
1320+
Object $near = spheric ? source.get("$nearSphere") : source.get("$near");
1321+
1322+
Number maxDistance = source.containsKey("$maxDistance") ? (Number) source.get("$maxDistance") : Double.MAX_VALUE;
1323+
List<Object> $centerMax = Arrays.asList(toCenterCoordinates($near), maxDistance);
1324+
Document $geoWithinMax = new Document("$geoWithin",
1325+
new Document(spheric ? "$centerSphere" : "$center", $centerMax));
1326+
1327+
if (!containsNearWithMinDistance(source)) {
1328+
return new Document(key, $geoWithinMax);
1329+
}
1330+
1331+
Number minDistance = (Number) source.get("$minDistance");
1332+
List<Object> $centerMin = Arrays.asList(toCenterCoordinates($near), minDistance);
1333+
Document $geoWithinMin = new Document("$geoWithin",
1334+
new Document(spheric ? "$centerSphere" : "$center", $centerMin));
1335+
1336+
List<Document> criteria = new ArrayList<>();
1337+
criteria.add(new Document("$nor", Arrays.asList(new Document(key, $geoWithinMin))));
1338+
criteria.add(new Document(key, $geoWithinMax));
1339+
return new Document("$and", criteria);
1340+
}
1341+
1342+
private static boolean containsNear(Document source) {
1343+
1344+
if (source.containsKey("$near") || source.containsKey("$nearSphere")) {
1345+
return true;
1346+
}
1347+
1348+
return false;
1349+
}
1350+
1351+
private static boolean containsNearWithMinDistance(Document source) {
1352+
1353+
if (!containsNear(source)) {
1354+
return false;
1355+
}
1356+
1357+
return source.containsKey("$minDistance");
1358+
}
1359+
1360+
private static Object toCenterCoordinates(Object value) {
1361+
1362+
if (ObjectUtils.isArray(value)) {
1363+
return value;
1364+
}
1365+
1366+
if (value instanceof Point) {
1367+
return Arrays.asList(((Point) value).getX(), ((Point) value).getY());
1368+
}
1369+
1370+
if (value instanceof Document && ((Document) value).containsKey("x")) {
1371+
1372+
Document point = (Document) value;
1373+
return Arrays.asList(point.get("x"), point.get("y"));
1374+
}
1375+
1376+
return value;
1377+
}
12931378
}

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/SessionAwareMethodInterceptorUnitTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,11 @@ public void justMoveOnIfNoOverloadWithSessionAvailable() {
107107
public void usesCacheForMethodLookup() {
108108

109109
MethodCache cache = (MethodCache) ReflectionTestUtils.getField(SessionAwareMethodInterceptor.class, "METHOD_CACHE");
110-
Method countMethod = ClassUtils.getMethod(MongoCollection.class, "count");
110+
Method countMethod = ClassUtils.getMethod(MongoCollection.class, "countDocuments");
111111

112112
assertThat(cache.contains(countMethod, MongoCollection.class)).isFalse();
113113

114-
collection.count();
114+
collection.countDocuments();
115115

116116
assertThat(cache.contains(countMethod, MongoCollection.class)).isTrue();
117117
}

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public void setUp() {
154154
when(db.runCommand(any(), any(Class.class))).thenReturn(commandResultDocument);
155155
when(collection.find(any(org.bson.Document.class), any(Class.class))).thenReturn(findIterable);
156156
when(collection.mapReduce(any(), any(), eq(Document.class))).thenReturn(mapReduceIterable);
157-
when(collection.count(any(Bson.class), any(CountOptions.class))).thenReturn(1L); // TODO: MongoDB 4 - fix me
157+
when(collection.countDocuments(any(Bson.class), any(CountOptions.class))).thenReturn(1L); // TODO: MongoDB 4 - fix me
158158
when(collection.getNamespace()).thenReturn(new MongoNamespace("db.mock-collection"));
159159
when(collection.aggregate(any(List.class), any())).thenReturn(aggregateIterable);
160160
when(collection.withReadPreference(any())).thenReturn(collection);
@@ -735,7 +735,7 @@ public void existsShouldUseCollationWhenPresent() {
735735
template.exists(new BasicQuery("{}").collation(Collation.of("fr")), AutogenerateableId.class);
736736

737737
ArgumentCaptor<CountOptions> options = ArgumentCaptor.forClass(CountOptions.class);
738-
verify(collection).count(any(), options.capture());
738+
verify(collection).countDocuments(any(), options.capture());
739739

740740
assertThat(options.getValue().getCollation())
741741
.isEqualTo(com.mongodb.client.model.Collation.builder().locale("fr").build());
@@ -926,7 +926,7 @@ public void countShouldUseCollationWhenPresent() {
926926
template.count(new BasicQuery("{}").collation(Collation.of("fr")), AutogenerateableId.class);
927927

928928
ArgumentCaptor<CountOptions> options = ArgumentCaptor.forClass(CountOptions.class);
929-
verify(collection).count(any(), options.capture());
929+
verify(collection).countDocuments(any(), options.capture());
930930

931931
assertThat(options.getValue().getCollation())
932932
.isEqualTo(com.mongodb.client.model.Collation.builder().locale("fr").build());
@@ -939,7 +939,7 @@ public void countShouldApplyQueryHintIfPresent() {
939939
template.count(new BasicQuery("{}").withHint(queryHint), AutogenerateableId.class);
940940

941941
ArgumentCaptor<CountOptions> options = ArgumentCaptor.forClass(CountOptions.class);
942-
verify(collection).count(any(), options.capture());
942+
verify(collection).countDocuments(any(), options.capture());
943943

944944
assertThat(options.getValue().getHint()).isEqualTo(queryHint);
945945
}
@@ -1068,7 +1068,7 @@ public void usesQueryOffsetForCountOperation() {
10681068
template.count(new BasicQuery("{}").skip(100), AutogenerateableId.class);
10691069

10701070
ArgumentCaptor<CountOptions> options = ArgumentCaptor.forClass(CountOptions.class);
1071-
verify(collection).count(any(), options.capture());
1071+
verify(collection).countDocuments(any(), options.capture());
10721072

10731073
assertThat(options.getValue().getSkip()).isEqualTo(100);
10741074
}
@@ -1079,7 +1079,7 @@ public void usesQueryLimitForCountOperation() {
10791079
template.count(new BasicQuery("{}").limit(10), AutogenerateableId.class);
10801080

10811081
ArgumentCaptor<CountOptions> options = ArgumentCaptor.forClass(CountOptions.class);
1082-
verify(collection).count(any(), options.capture());
1082+
verify(collection).countDocuments(any(), options.capture());
10831083

10841084
assertThat(options.getValue().getLimit()).isEqualTo(10);
10851085
}
@@ -1150,7 +1150,7 @@ public void existsShouldUseDefaultCollationWhenPresent() {
11501150
template.exists(new BasicQuery("{}"), Sith.class);
11511151

11521152
ArgumentCaptor<CountOptions> options = ArgumentCaptor.forClass(CountOptions.class);
1153-
verify(collection).count(any(), options.capture());
1153+
verify(collection).countDocuments(any(), options.capture());
11541154

11551155
assertThat(options.getValue().getCollation())
11561156
.isEqualTo(com.mongodb.client.model.Collation.builder().locale("de_AT").build());

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public void setUp() {
126126
when(collection.find(any(Document.class), any(Class.class))).thenReturn(findPublisher);
127127
when(collection.aggregate(anyList())).thenReturn(aggregatePublisher);
128128
when(collection.aggregate(anyList(), any(Class.class))).thenReturn(aggregatePublisher);
129-
when(collection.count(any(), any(CountOptions.class))).thenReturn(Mono.just(0L));
129+
when(collection.countDocuments(any(), any(CountOptions.class))).thenReturn(Mono.just(0L));
130130
when(collection.updateOne(any(), any(Bson.class), any(UpdateOptions.class))).thenReturn(updateResultPublisher);
131131
when(collection.updateMany(any(Bson.class), any(Bson.class), any())).thenReturn(updateResultPublisher);
132132
when(collection.findOneAndUpdate(any(), any(Bson.class), any(FindOneAndUpdateOptions.class)))
@@ -391,7 +391,7 @@ public void countShouldUseSkipFromQuery() {
391391
template.count(new Query().skip(10), Person.class, "star-wars").subscribe();
392392

393393
ArgumentCaptor<CountOptions> options = ArgumentCaptor.forClass(CountOptions.class);
394-
verify(collection).count(any(), options.capture());
394+
verify(collection).countDocuments(any(), options.capture());
395395

396396
assertThat(options.getValue().getSkip()).isEqualTo(10);
397397
}
@@ -402,7 +402,7 @@ public void countShouldUseLimitFromQuery() {
402402
template.count(new Query().limit(100), Person.class, "star-wars").subscribe();
403403

404404
ArgumentCaptor<CountOptions> options = ArgumentCaptor.forClass(CountOptions.class);
405-
verify(collection).count(any(), options.capture());
405+
verify(collection).countDocuments(any(), options.capture());
406406

407407
assertThat(options.getValue().getLimit()).isEqualTo(100);
408408
}
@@ -414,7 +414,7 @@ public void countShouldApplyQueryHintIfPresent() {
414414
template.count(new Query().withHint(queryHint), Person.class, "star-wars").subscribe();
415415

416416
ArgumentCaptor<CountOptions> options = ArgumentCaptor.forClass(CountOptions.class);
417-
verify(collection).count(any(), options.capture());
417+
verify(collection).countDocuments(any(), options.capture());
418418

419419
assertThat(options.getValue().getHint()).isEqualTo(queryHint);
420420
}

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveSessionBoundMongoTemplateUnitTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public void setUp() {
111111
when(collection.deleteMany(any(ClientSession.class), any(), any())).thenReturn(resultPublisher);
112112
when(collection.insertOne(any(ClientSession.class), any(Document.class))).thenReturn(resultPublisher);
113113
when(collection.aggregate(any(ClientSession.class), anyList(), any(Class.class))).thenReturn(aggregatePublisher);
114-
when(collection.count(any(ClientSession.class), any(), any(CountOptions.class))).thenReturn(resultPublisher);
114+
when(collection.countDocuments(any(ClientSession.class), any(), any(CountOptions.class))).thenReturn(resultPublisher);
115115
when(collection.drop(any(ClientSession.class))).thenReturn(resultPublisher);
116116
when(collection.findOneAndUpdate(any(ClientSession.class), any(), any(Bson.class), any()))
117117
.thenReturn(resultPublisher);
@@ -224,7 +224,7 @@ public void countShouldUseProxiedCollection() {
224224

225225
template.count(new Query(), Person.class).subscribe();
226226

227-
verify(collection).count(eq(clientSession), any(), any(CountOptions.class));
227+
verify(collection).countDocuments(eq(clientSession), any(), any(CountOptions.class));
228228
}
229229

230230
@Test // DATAMONGO-1880

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SessionBoundMongoTemplateTests.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,6 @@ public void countShouldWorkInTransactions() {
284284
}
285285

286286
@Test // DATAMONGO-2012
287-
@Ignore("error 2 (BadValue): $match does not support $geoNear, $near, and $nearSphere")
288287
public void countWithGeoInTransaction() {
289288

290289
if (!template.collectionExists(Person.class)) {

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/SessionBoundMongoTemplateUnitTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ public void countShouldUseProxiedCollection() {
226226

227227
template.count(new Query(), Person.class);
228228

229-
verify(collection).count(eq(clientSession), any(), any(CountOptions.class));
229+
verify(collection).countDocuments(eq(clientSession), any(), any(CountOptions.class));
230230
}
231231

232232
@Test // DATAMONGO-1880

0 commit comments

Comments
 (0)