Skip to content

DATAMONGO-2059 - Favor collection.countDocuments() over collection.count() which will be removed. #604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>2.3.0.BUILD-SNAPSHOT</version>
<version>2.3.0.DATAMONGO-2059-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data MongoDB</name>
Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb-benchmarks/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>2.3.0.BUILD-SNAPSHOT</version>
<version>2.3.0.DATAMONGO-2059-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb-distribution/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>2.3.0.BUILD-SNAPSHOT</version>
<version>2.3.0.DATAMONGO-2059-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>2.3.0.BUILD-SNAPSHOT</version>
<version>2.3.0.DATAMONGO-2059-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,227 @@
/*
* Copyright 2019 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
*
* https://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;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;

import org.bson.Document;

import org.springframework.data.geo.Point;
import org.springframework.lang.Nullable;
import org.springframework.util.ObjectUtils;

/**
* Value object representing a count query. Count queries using {@code $near} or {@code $nearSphere} require a rewrite
* to {@code $geoWithin}.
*
* @author Christoph Strobl
* @author Mark Paluch
* @since 2.3
*/
class CountQuery {

private Document source;

private CountQuery(Document source) {
this.source = source;
}

public static CountQuery of(Document source) {
return new CountQuery(source);
}

/**
* Returns the query {@link Document} that can be used with {@code countDocuments()}. Potentially rewrites the query
* to be usable with {@code countDocuments()}.
*
* @return the query {@link Document} that can be used with {@code countDocuments()}.
*/
public Document toQueryDocument() {

if (!requiresRewrite(source)) {
return source;
}

Document target = new Document();

for (Map.Entry<String, Object> entry : source.entrySet()) {

if (entry.getValue() instanceof Document && requiresRewrite(entry.getValue())) {

Document theValue = (Document) entry.getValue();
target.putAll(createGeoWithin(entry.getKey(), theValue, source.get("$and")));
continue;
}

if (entry.getValue() instanceof Collection && requiresRewrite(entry.getValue())) {

Collection<?> source = (Collection<?>) entry.getValue();

target.put(entry.getKey(), rewriteCollection(source));
continue;
}

if ("$and".equals(entry.getKey()) && target.containsKey("$and")) {
// Expect $and to be processed with Document and createGeoWithin.
continue;
}

target.put(entry.getKey(), entry.getValue());
}

return target;
}

/**
* @param valueToInspect
* @return {@code true} if the enclosing element needs to be rewritten.
*/
private boolean requiresRewrite(Object valueToInspect) {

if (valueToInspect instanceof Document) {
return requiresRewrite((Document) valueToInspect);
}

if (valueToInspect instanceof Collection) {
return requiresRewrite((Collection) valueToInspect);
}

return false;
}

private boolean requiresRewrite(Collection<?> collection) {

for (Object o : collection) {
if (o instanceof Document && requiresRewrite((Document) o)) {
return true;
}
}

return false;
}

private boolean requiresRewrite(Document document) {

if (containsNear(document)) {
return true;
}

for (Object entry : document.values()) {

if (requiresRewrite(entry)) {
return true;
}
}

return false;
}

private Collection<Object> rewriteCollection(Collection<?> source) {

Collection<Object> rewrittenCollection = new ArrayList<>(source.size());

for (Object item : source) {
if (item instanceof Document && requiresRewrite(item)) {
rewrittenCollection.add(CountQuery.of((Document) item).toQueryDocument());
} else {
rewrittenCollection.add(item);
}
}

return rewrittenCollection;
}

/**
* Rewrite the near query for field {@code key} to {@code $geoWithin}.
*
* @param key the queried field.
* @param source source {@link Document}.
* @param $and potentially existing {@code $and} condition.
* @return the rewritten query {@link Document}.
*/
private static Document createGeoWithin(String key, Document source, @Nullable Object $and) {

boolean spheric = source.containsKey("$nearSphere");
Object $near = spheric ? source.get("$nearSphere") : source.get("$near");

Number maxDistance = source.containsKey("$maxDistance") ? (Number) source.get("$maxDistance") : Double.MAX_VALUE;
List<Object> $centerMax = Arrays.asList(toCenterCoordinates($near), maxDistance);
Document $geoWithinMax = new Document("$geoWithin",
new Document(spheric ? "$centerSphere" : "$center", $centerMax));

if (!containsNearWithMinDistance(source)) {
return new Document(key, $geoWithinMax);
}

Number minDistance = (Number) source.get("$minDistance");
List<Object> $centerMin = Arrays.asList(toCenterCoordinates($near), minDistance);
Document $geoWithinMin = new Document("$geoWithin",
new Document(spheric ? "$centerSphere" : "$center", $centerMin));

List<Document> criteria = new ArrayList<>();

if ($and != null) {
if ($and instanceof Collection) {
criteria.addAll((Collection) $and);
} else {
throw new IllegalArgumentException(
"Cannot rewrite query as it contains an '$and' element that is not a Collection!: Offending element: "
+ $and);
}
}

criteria.add(new Document("$nor", Collections.singletonList(new Document(key, $geoWithinMin))));
criteria.add(new Document(key, $geoWithinMax));
return new Document("$and", criteria);
}

private static boolean containsNear(Document source) {
return source.containsKey("$near") || source.containsKey("$nearSphere");
}

private static boolean containsNearWithMinDistance(Document source) {

if (!containsNear(source)) {
return false;
}

return source.containsKey("$minDistance");
}

private static Object toCenterCoordinates(Object value) {

if (ObjectUtils.isArray(value)) {
return value;
}

if (value instanceof Point) {
return Arrays.asList(((Point) value).getX(), ((Point) value).getY());
}

if (value instanceof Document && ((Document) value).containsKey("x")) {

Document point = (Document) value;
return Arrays.asList(point.get("x"), point.get("y"));
}

return value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.springframework.data.geo.GeoResult;
import org.springframework.data.geo.GeoResults;
import org.springframework.data.geo.Metric;
import org.springframework.data.geo.Point;
import org.springframework.data.mapping.PropertyPath;
import org.springframework.data.mapping.PropertyReferenceException;
import org.springframework.data.mapping.callback.EntityCallbacks;
Expand Down Expand Up @@ -1191,11 +1192,8 @@ protected long doCount(String collectionName, Document filter, CountOptions opti
LOGGER.debug("Executing count: {} in collection: {}", serializeToJsonSafely(filter), collectionName);
}

if (MongoDatabaseUtils.isTransactionActive(getMongoDbFactory())) {
return execute(collectionName, collection -> collection.countDocuments(filter, options));
}

return execute(collectionName, collection -> collection.count(filter, options));
return execute(collectionName,
collection -> collection.countDocuments(CountQuery.of(filter).toQueryDocument(), options));
}

/*
Expand Down Expand Up @@ -3523,19 +3521,5 @@ public MongoDatabase getDb() {
// native MongoDB objects that offer methods with ClientSession must not be proxied.
return delegate.getDb();
}

/*
* (non-Javadoc)
* @see org.springframework.data.mongodb.core.MongoTemplate#doCount(java.lang.String, org.bson.Document, com.mongodb.client.model.CountOptions)
*/
@Override
protected long doCount(String collectionName, Document filter, CountOptions options) {

if (!session.hasActiveTransaction()) {
return super.doCount(collectionName, filter, options);
}

return execute(collectionName, collection -> collection.countDocuments(filter, options));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1300,9 +1300,8 @@ public Mono<Long> count(Query query, @Nullable Class<?> entityClass, String coll
*/
protected Mono<Long> doCount(String collectionName, Document filter, CountOptions options) {

return ReactiveMongoDatabaseUtils.isTransactionActive(mongoDatabaseFactory) //
.flatMap(txActive -> createMono(collectionName,
collection -> txActive ? collection.countDocuments(filter, options) : collection.count(filter, options)));
return createMono(collectionName,
collection -> collection.countDocuments(CountQuery.of(filter).toQueryDocument(), options));
}

/*
Expand Down Expand Up @@ -3323,20 +3322,6 @@ public MongoDatabase getMongoDatabase() {
// native MongoDB objects that offer methods with ClientSession must not be proxied.
return delegate.getMongoDatabase();
}

/*
* (non-Javadoc)
* @see org.springframework.data.mongodb.core.ReactiveMongoTemplate#count(java.lang.String, org.bson.Document, com.mongodb.client.model.CountOptions)
*/
@Override
public Mono<Long> doCount(String collectionName, Document filter, CountOptions options) {

if (!session.hasActiveTransaction()) {
return super.doCount(collectionName, filter, options);
}

return createMono(collectionName, collection -> collection.countDocuments(filter, options));
}
}

@RequiredArgsConstructor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.bson.Document;
import org.bson.conversions.Bson;
import org.bson.types.ObjectId;

import org.springframework.core.convert.ConversionService;
import org.springframework.core.convert.converter.Converter;
import org.springframework.data.domain.Example;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ public void justMoveOnIfNoOverloadWithSessionAvailable() {
public void usesCacheForMethodLookup() {

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

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

collection.count();
collection.countDocuments();

assertThat(cache.contains(countMethod, MongoCollection.class)).isTrue();
}
Expand Down
Loading