Skip to content
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

DATAMONGO-1127 - Add support for geoNear queries with distance information. #261

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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
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>1.7.0.BUILD-SNAPSHOT</version>
<version>1.7.0.DATAMONGO-1127-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data MongoDB</name>
Expand Down
4 changes: 2 additions & 2 deletions spring-data-mongodb-cross-store/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>1.7.0.BUILD-SNAPSHOT</version>
<version>1.7.0.DATAMONGO-1127-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down Expand Up @@ -48,7 +48,7 @@
<dependency>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb</artifactId>
<version>1.7.0.BUILD-SNAPSHOT</version>
<version>1.7.0.DATAMONGO-1127-SNAPSHOT</version>
</dependency>

<dependency>
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 @@ -13,7 +13,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>1.7.0.BUILD-SNAPSHOT</version>
<version>1.7.0.DATAMONGO-1127-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb-log4j/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>1.7.0.BUILD-SNAPSHOT</version>
<version>1.7.0.DATAMONGO-1127-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>1.7.0.BUILD-SNAPSHOT</version>
<version>1.7.0.DATAMONGO-1127-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2014 the original author or authors.
* Copyright 2013-2015 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.
Expand Down Expand Up @@ -27,6 +27,7 @@
import org.springframework.data.mongodb.core.aggregation.ExposedFields.FieldReference;
import org.springframework.data.mongodb.core.aggregation.Fields.AggregationField;
import org.springframework.data.mongodb.core.query.Criteria;
import org.springframework.data.mongodb.core.query.NearQuery;
import org.springframework.data.mongodb.core.query.SerializationUtils;
import org.springframework.util.Assert;

Expand Down Expand Up @@ -291,6 +292,19 @@ public static Fields bind(String name, String target) {
return Fields.from(field(name, target));
}

/**
* Creates a new {@link GeoNearOperation} instance from the given {@link NearQuery} and the distanceField. The
* {@code distanceField} defines output field that contains the calculated distance.
*
* @param query must not be {@literal null}.
* @param distanceField must not be {@literal null} or empty.
* @return
* @since 1.7
*/
public static GeoNearOperation geoNear(NearQuery query, String distanceField) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we actually add an overload without the distance field applied so that basically people can mimic what they got when using new GeoNearOperation(nearQuery) before?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the geoNear(...) method was not exposed before this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that. Still, we need to keep the constructor in GeoNearOperations which then raises the question why geoNear(query, distance) is available as factory method but geoNear(query) is not. We should stay symmetric here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack.

return new GeoNearOperation(query, distanceField);
}

/**
* Returns a new {@link AggregationOptions.Builder}.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013 the original author or authors.
* Copyright 2013-2015 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.
Expand Down Expand Up @@ -28,11 +28,15 @@
public class GeoNearOperation implements AggregationOperation {

private final NearQuery nearQuery;
private final String distanceField;

public GeoNearOperation(NearQuery nearQuery) {
public GeoNearOperation(NearQuery nearQuery, String distanceField) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have to overload the constructor to not break existing clients using the old constructor. I suggest to add a hint to favor Aggregation.geoNear(…) over using the constructors directly. Should we actually do that for all of the operation constructors that actually have a static factory method equivalent in Aggregation?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Regarding the hint yes thats a good idea I'll add that.


Assert.notNull(nearQuery);
Assert.notNull(distanceField, "DistanceField must not be null.");

this.nearQuery = nearQuery;
this.distanceField = distanceField;
}

/*
Expand All @@ -41,6 +45,10 @@ public GeoNearOperation(NearQuery nearQuery) {
*/
@Override
public DBObject toDBObject(AggregationOperationContext context) {
return new BasicDBObject("$geoNear", context.getMappedObject(nearQuery.toDBObject()));

BasicDBObject command = (BasicDBObject) context.getMappedObject(nearQuery.toDBObject());
command.put("distanceField", distanceField);

return new BasicDBObject("$geoNear", command);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2011-2014 the original author or authors.
* Copyright 2011-2015 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.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013 the original author or authors.
* Copyright 2013-2015 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.
Expand Down Expand Up @@ -47,11 +47,14 @@
import org.springframework.dao.DataAccessException;
import org.springframework.data.annotation.Id;
import org.springframework.data.domain.Sort.Direction;
import org.springframework.data.geo.Metrics;
import org.springframework.data.mapping.model.MappingException;
import org.springframework.data.mongodb.core.CollectionCallback;
import org.springframework.data.mongodb.core.MongoTemplate;
import org.springframework.data.mongodb.core.Venue;
import org.springframework.data.mongodb.core.aggregation.AggregationTests.CarDescriptor.Entry;
import org.springframework.data.mongodb.core.mapping.Document;
import org.springframework.data.mongodb.core.index.GeospatialIndex;
import org.springframework.data.mongodb.core.query.NearQuery;
import org.springframework.data.mongodb.core.query.Query;
import org.springframework.data.mongodb.repository.Person;
import org.springframework.data.util.Version;
Expand Down Expand Up @@ -1018,6 +1021,34 @@ public void shouldRetrieveDateTimeFragementsCorrectly() throws Exception {
assertThat(dbo.get("dayOfYearPlus1DayManually"), is((Object) dateTime.plusDays(1).getDayOfYear()));
}

/**
* @see DATAMONGO-1127
*/
@Test
public void shouldSupportGeoNearQueriesForAggregation() {

mongoTemplate.dropCollection(Venue.class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should handle these cleanup operations in JUnit callback methods (the same for the drop of the collection at the very end) to make sure they get applied even if exceptions occur.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack


mongoTemplate.insert(new Venue("Penn Station", -73.99408, 40.75057));
mongoTemplate.insert(new Venue("10gen Office", -73.99171, 40.738868));
mongoTemplate.insert(new Venue("Flatiron Building", -73.988135, 40.741404));

mongoTemplate.indexOps(Venue.class).ensureIndex(new GeospatialIndex("location"));

NearQuery geoNear = NearQuery.near(-73, 40, Metrics.KILOMETERS).num(10).maxDistance(150);

Aggregation agg = newAggregation(Aggregation.geoNear(geoNear, "distance"));
AggregationResults<DBObject> result = mongoTemplate.aggregate(agg, Venue.class, DBObject.class);

assertThat(result.getMappedResults(), hasSize(3));

DBObject firstResult = result.getMappedResults().get(0);
assertThat(firstResult.containsField("distance"), is(true));
assertThat(firstResult.get("distance"), is((Object) 117.620092203928));

mongoTemplate.dropCollection(Venue.class);
}

private void assertLikeStats(LikeStats like, String id, long count) {

assertThat(like, is(notNullValue()));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013 the original author or authors.
* Copyright 2013-2015 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.
Expand All @@ -22,23 +22,30 @@
import org.springframework.data.mongodb.core.DBObjectTestUtils;
import org.springframework.data.mongodb.core.query.NearQuery;

import com.mongodb.BasicDBObject;
import com.mongodb.DBObject;

/**
* Unit tests for {@link GeoNearOperation}.
*
* @author Oliver Gierke
* @author Thomas Darimont
*/
public class GeoNearOperationUnitTests {

/**
* @see DATAMONGO-1127
*/
@Test
public void rendersNearQueryAsAggregationOperation() {

NearQuery query = NearQuery.near(10.0, 10.0);
GeoNearOperation operation = new GeoNearOperation(query);
GeoNearOperation operation = new GeoNearOperation(query, "distance");
DBObject dbObject = operation.toDBObject(Aggregation.DEFAULT_CONTEXT);

DBObject nearClause = DBObjectTestUtils.getAsDBObject(dbObject, "$geoNear");
assertThat(nearClause, is(query.toDBObject()));

DBObject expected = (DBObject) new BasicDBObject(query.toDBObject().toMap()).append("distanceField", "distance");
assertThat(nearClause, is(expected));
}
}