DATAMONGO-603 - Make sure geo queries and repositories get correct Metric applied.

Heavily refactored NearQuery to simplify implementation and testability. Made sure the metric of the maximum distance is applied if no metric had been defined before. Adapted query preparation code to re-enforce the correct metric being set to not rely on the auto-application behavior of NearQuery to be safe against potential further refactorings.
This commit is contained in:
Oliver Gierke
2013-02-01 14:27:44 +01:00
parent 4b1065cac5
commit c28e51cf86
6 changed files with 198 additions and 54 deletions

View File

@@ -0,0 +1,43 @@
/*
* Copyright 2013 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
*
* http://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.geo;
/**
* Value object to create custom {@link Metric}s on the fly.
*
* @author Oliver Gierke
*/
public class CustomMetric implements Metric {
private final double multiplier;
/**
* Creates a custom {@link Metric} using the given multiplier.
*
* @param multiplier
*/
public CustomMetric(double multiplier) {
this.multiplier = multiplier;
}
/*
* (non-Javadoc)
* @see org.springframework.data.mongodb.core.geo.Metric#getMultiplier()
*/
public double getMultiplier() {
return multiplier;
}
}

View File

@@ -1,5 +1,5 @@
/* /*
* Copyright 2011 the original author or authors. * Copyright 2011-2013 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@@ -15,6 +15,7 @@
*/ */
package org.springframework.data.mongodb.core.query; package org.springframework.data.mongodb.core.query;
import org.springframework.data.mongodb.core.geo.CustomMetric;
import org.springframework.data.mongodb.core.geo.Distance; import org.springframework.data.mongodb.core.geo.Distance;
import org.springframework.data.mongodb.core.geo.Metric; import org.springframework.data.mongodb.core.geo.Metric;
import org.springframework.data.mongodb.core.geo.Metrics; import org.springframework.data.mongodb.core.geo.Metrics;
@@ -31,10 +32,12 @@ import com.mongodb.DBObject;
*/ */
public class NearQuery { public class NearQuery {
private final DBObject criteria; private final Point point;
private Query query; private Query query;
private Double maxDistance; private Distance maxDistance;
private Metric metric; private Metric metric;
private boolean spherical;
private Integer num;
/** /**
* Creates a new {@link NearQuery}. * Creates a new {@link NearQuery}.
@@ -45,13 +48,11 @@ public class NearQuery {
Assert.notNull(point); Assert.notNull(point);
this.criteria = new BasicDBObject(); this.point = point;
this.criteria.put("near", point.asList()); this.spherical = false;
this.metric = metric;
if (metric != null) { if (metric != null) {
spherical(true); in(metric);
distanceMultiplier(metric);
} }
} }
@@ -105,12 +106,13 @@ public class NearQuery {
} }
/** /**
* Returns the {@link Metric} underlying the actual query. * Returns the {@link Metric} underlying the actual query. If no metric was set explicitly {@link Metrics#NEUTRAL}
* will be returned.
* *
* @return * @return will never be {@literal null}.
*/ */
public Metric getMetric() { public Metric getMetric() {
return metric; return metric == null ? Metrics.NEUTRAL : metric;
} }
/** /**
@@ -120,68 +122,96 @@ public class NearQuery {
* @return * @return
*/ */
public NearQuery num(int num) { public NearQuery num(int num) {
this.criteria.put("num", num); this.num = num;
return this; return this;
} }
/** /**
* Sets the max distance results shall have from the configured origin. Will normalize the given value using a * Sets the max distance results shall have from the configured origin. If a {@link Metric} was set before the given
* potentially already configured {@link Metric}. * value will be interpreted as being a value in that metric. E.g.
*
* <pre>
* NearQuery query = near(10.0, 20.0, Metrics.KILOMETERS).maxDistance(150);
* </pre>
*
* Will set the maximum distance to 150 kilometers.
* *
* @param maxDistance * @param maxDistance
* @return * @return
*/ */
public NearQuery maxDistance(double maxDistance) { public NearQuery maxDistance(double maxDistance) {
this.maxDistance = getNormalizedDistance(maxDistance, this.metric); return maxDistance(new Distance(maxDistance, getMetric()));
return this;
} }
/** /**
* Sets the maximum distance supplied in a given metric. Will normalize the distance but not reconfigure the query's * Sets the maximum distance supplied in a given metric. Will normalize the distance but not reconfigure the query's
* {@link Metric}. * result {@link Metric} if one was configured before.
* *
* @param maxDistance * @param maxDistance
* @param metric must not be {@literal null}. * @param metric must not be {@literal null}.
* @return * @return
*/ */
public NearQuery maxDistance(double maxDistance, Metric metric) { public NearQuery maxDistance(double maxDistance, Metric metric) {
Assert.notNull(metric); Assert.notNull(metric);
this.spherical(true); return maxDistance(new Distance(maxDistance, metric));
return maxDistance(getNormalizedDistance(maxDistance, metric));
} }
/** /**
* Sets the maximum distance to the given {@link Distance}. * Sets the maximum distance to the given {@link Distance}. Will set the returned {@link Metric} to be the one of the
* given {@link Distance} if no {@link Metric} was set before.
* *
* @param distance * @param distance must not be {@literal null}.
* @return * @return
*/ */
public NearQuery maxDistance(Distance distance) { public NearQuery maxDistance(Distance distance) {
Assert.notNull(distance); Assert.notNull(distance);
return maxDistance(distance.getValue(), distance.getMetric());
if (distance.getMetric() != Metrics.NEUTRAL) {
this.spherical(true);
}
if (this.metric == null) {
in(distance.getMetric());
}
this.maxDistance = distance;
return this;
} }
/** /**
* Configures a distance multiplier the resulting distances get applied. * Returns the maximum {@link Distance}.
*
* @return
*/
public Distance getMaxDistance() {
return this.maxDistance;
}
/**
* Configures a {@link CustomMetric} with the given multiplier.
* *
* @param distanceMultiplier * @param distanceMultiplier
* @return * @return
*/ */
public NearQuery distanceMultiplier(double distanceMultiplier) { public NearQuery distanceMultiplier(double distanceMultiplier) {
this.criteria.put("distanceMultiplier", distanceMultiplier);
this.metric = new CustomMetric(distanceMultiplier);
return this; return this;
} }
/** /**
* Configures the distance multiplier to the multiplier of the given {@link Metric}. Does <em>not</em> recalculate the * Configures the distance multiplier to the multiplier of the given {@link Metric}.
* {@link #maxDistance(double)}.
* *
* @deprecated use {@link #in(Metric)} instead.
* @param metric must not be {@literal null}. * @param metric must not be {@literal null}.
* @return * @return
*/ */
@Deprecated
public NearQuery distanceMultiplier(Metric metric) { public NearQuery distanceMultiplier(Metric metric) {
Assert.notNull(metric); Assert.notNull(metric);
return distanceMultiplier(metric.getMultiplier()); return in(metric);
} }
/** /**
@@ -191,10 +221,19 @@ public class NearQuery {
* @return * @return
*/ */
public NearQuery spherical(boolean spherical) { public NearQuery spherical(boolean spherical) {
this.criteria.put("spherical", spherical); this.spherical = spherical;
return this; return this;
} }
/**
* Returns whether spharical values will be returned.
*
* @return
*/
public boolean isSpherical() {
return this.spherical;
}
/** /**
* Will cause the results' distances being returned in kilometers. Sets {@link #distanceMultiplier(double)} and * Will cause the results' distances being returned in kilometers. Sets {@link #distanceMultiplier(double)} and
* {@link #spherical(boolean)} accordingly. * {@link #spherical(boolean)} accordingly.
@@ -215,6 +254,18 @@ public class NearQuery {
return adaptMetric(Metrics.MILES); return adaptMetric(Metrics.MILES);
} }
/**
* Will cause the results' distances being returned in the given metric. Sets {@link #distanceMultiplier(double)}
* accordingly as well as {@link #spherical(boolean)} if the given {@link Metric} is not {@link Metrics#NEUTRAL}.
*
* @param metric the metric the results shall be returned in. Uses {@link Metrics#NEUTRAL} if {@literal null} is
* passed.
* @return
*/
public NearQuery in(Metric metric) {
return adaptMetric(metric == null ? Metrics.NEUTRAL : metric);
}
/** /**
* Configures the given {@link Metric} to be used as base on for this query and recalculate the maximum distance if no * Configures the given {@link Metric} to be used as base on for this query and recalculate the maximum distance if no
* metric was set before. * metric was set before.
@@ -223,12 +274,12 @@ public class NearQuery {
*/ */
private NearQuery adaptMetric(Metric metric) { private NearQuery adaptMetric(Metric metric) {
if (this.metric == null && maxDistance != null) { if (metric != Metrics.NEUTRAL) {
maxDistance(this.maxDistance, metric); spherical(true);
} }
spherical(true); this.metric = metric;
return distanceMultiplier(metric); return this;
} }
/** /**
@@ -249,18 +300,27 @@ public class NearQuery {
*/ */
public DBObject toDBObject() { public DBObject toDBObject() {
BasicDBObject dbObject = new BasicDBObject(criteria.toMap()); BasicDBObject dbObject = new BasicDBObject();
if (query != null) { if (query != null) {
dbObject.put("query", query.getQueryObject()); dbObject.put("query", query.getQueryObject());
} }
if (maxDistance != null) { if (maxDistance != null) {
dbObject.put("maxDistance", maxDistance); dbObject.put("maxDistance", this.maxDistance.getNormalizedValue());
} }
if (metric != null) {
dbObject.put("distanceMultiplier", metric.getMultiplier());
}
if (num != null) {
dbObject.put("num", num);
}
dbObject.put("near", point.asList());
dbObject.put("spherical", spherical);
return dbObject; return dbObject;
} }
private double getNormalizedDistance(double distance, Metric metric) {
return metric == null ? distance : distance / metric.getMultiplier();
}
} }

View File

@@ -1,5 +1,5 @@
/* /*
* Copyright 2010-2012 the original author or authors. * Copyright 2010-2013 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@@ -254,7 +254,7 @@ public abstract class AbstractMongoQuery implements RepositoryQuery {
Distance maxDistance = accessor.getMaxDistance(); Distance maxDistance = accessor.getMaxDistance();
if (maxDistance != null) { if (maxDistance != null) {
nearQuery.maxDistance(maxDistance); nearQuery.maxDistance(maxDistance).in(maxDistance.getMetric());
} }
MongoEntityMetadata<?> metadata = method.getEntityInformation(); MongoEntityMetadata<?> metadata = method.getEntityInformation();

View File

@@ -1,5 +1,5 @@
/* /*
* Copyright 2010-2011 the original author or authors. * Copyright 2010-2013 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@@ -115,10 +115,12 @@ public class GeoSpatialTests {
@Test @Test
public void geoNear() { public void geoNear() {
NearQuery geoNear = NearQuery.near(-73, 40, Metrics.KILOMETERS).num(10).maxDistance(150);
GeoResults<Venue> geoNearResult = template.geoNear(geoNear, Venue.class);
assertThat(geoNearResult.getContent().size(), is(not(0))); NearQuery geoNear = NearQuery.near(-73, 40, Metrics.KILOMETERS).num(10).maxDistance(150);
GeoResults<Venue> result = template.geoNear(geoNear, Venue.class);
assertThat(result.getContent().size(), is(not(0)));
assertThat(result.getAverageDistance().getMetric(), is((Metric) Metrics.KILOMETERS));
} }
@Test @Test

View File

@@ -1,17 +1,35 @@
/*
* Copyright 2011-2013 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
*
* http://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.query; package org.springframework.data.mongodb.core.query;
import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*; import static org.junit.Assert.*;
import org.junit.Test; import org.junit.Test;
import org.springframework.data.mongodb.core.geo.Distance;
import org.springframework.data.mongodb.core.geo.Metric;
import org.springframework.data.mongodb.core.geo.Metrics; import org.springframework.data.mongodb.core.geo.Metrics;
/** /**
*
* @author Oliver Gierke * @author Oliver Gierke
*/ */
public class NearQueryUnitTests { public class NearQueryUnitTests {
private static final Distance ONE_FIFTY_KILOMETERS = new Distance(150, Metrics.KILOMETERS);
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
public void rejectsNullPoint() { public void rejectsNullPoint() {
NearQuery.near(null); NearQuery.near(null);
@@ -22,9 +40,12 @@ public class NearQueryUnitTests {
NearQuery query = NearQuery.near(2.5, 2.5, Metrics.KILOMETERS).maxDistance(150); NearQuery query = NearQuery.near(2.5, 2.5, Metrics.KILOMETERS).maxDistance(150);
assertThat((Double) query.toDBObject().get("maxDistance"), is(0.02351783914331097)); assertThat(query.getMaxDistance(), is(ONE_FIFTY_KILOMETERS));
assertThat((Boolean) query.toDBObject().get("spherical"), is(true)); assertThat(query.getMetric(), is((Metric) Metrics.KILOMETERS));
assertThat((Double) query.toDBObject().get("distanceMultiplier"), is(Metrics.KILOMETERS.getMultiplier())); assertThat(query.isSpherical(), is(true));
System.out.println(query.toDBObject().get("maxDistance"));
System.out.println(query.toDBObject().get("distanceMultiplier"));
} }
@Test @Test
@@ -32,13 +53,27 @@ public class NearQueryUnitTests {
NearQuery query = NearQuery.near(2.5, 2.5, Metrics.KILOMETERS).maxDistance(150); NearQuery query = NearQuery.near(2.5, 2.5, Metrics.KILOMETERS).maxDistance(150);
assertThat((Double) query.toDBObject().get("maxDistance"), is(0.02351783914331097));
assertThat((Double) query.toDBObject().get("distanceMultiplier"), is(Metrics.KILOMETERS.getMultiplier()));
query.inMiles(); query.inMiles();
assertThat((Double) query.toDBObject().get("distanceMultiplier"), is(Metrics.MILES.getMultiplier())); assertThat(query.getMetric(), is((Metric) Metrics.MILES));
}
NearQuery.near(2.5, 2.5).maxDistance(150).inKilometers(); @Test
assertThat((Double) query.toDBObject().get("maxDistance"), is(0.02351783914331097)); public void configuresResultMetricCorrectly() {
NearQuery query = NearQuery.near(2.5, 2.1);
assertThat(query.getMetric(), is((Metric) Metrics.NEUTRAL));
query = query.maxDistance(ONE_FIFTY_KILOMETERS);
assertThat(query.getMetric(), is((Metric) Metrics.KILOMETERS));
assertThat(query.getMaxDistance(), is(ONE_FIFTY_KILOMETERS));
assertThat(query.isSpherical(), is(true));
query = query.in(Metrics.MILES);
assertThat(query.getMetric(), is((Metric) Metrics.MILES));
assertThat(query.getMaxDistance(), is(ONE_FIFTY_KILOMETERS));
assertThat(query.isSpherical(), is(true));
query = query.maxDistance(new Distance(200, Metrics.KILOMETERS));
assertThat(query.getMetric(), is((Metric) Metrics.MILES));
} }
} }

View File

@@ -38,6 +38,7 @@ import org.springframework.data.mongodb.core.geo.Circle;
import org.springframework.data.mongodb.core.geo.Distance; import org.springframework.data.mongodb.core.geo.Distance;
import org.springframework.data.mongodb.core.geo.GeoPage; import org.springframework.data.mongodb.core.geo.GeoPage;
import org.springframework.data.mongodb.core.geo.GeoResults; import org.springframework.data.mongodb.core.geo.GeoResults;
import org.springframework.data.mongodb.core.geo.Metric;
import org.springframework.data.mongodb.core.geo.Metrics; import org.springframework.data.mongodb.core.geo.Metrics;
import org.springframework.data.mongodb.core.geo.Point; import org.springframework.data.mongodb.core.geo.Point;
import org.springframework.data.mongodb.core.geo.Polygon; import org.springframework.data.mongodb.core.geo.Polygon;
@@ -401,6 +402,9 @@ public abstract class AbstractPersonRepositoryIntegrationTests {
GeoPage<Person> results = repository.findByLocationNear(new Point(-73.99, 40.73), new Distance(2000, GeoPage<Person> results = repository.findByLocationNear(new Point(-73.99, 40.73), new Distance(2000,
Metrics.KILOMETERS), new PageRequest(0, 20)); Metrics.KILOMETERS), new PageRequest(0, 20));
assertThat(results.getContent().isEmpty(), is(false)); assertThat(results.getContent().isEmpty(), is(false));
// DATAMONGO-607
assertThat(results.getAverageDistance().getMetric(), is((Metric) Metrics.KILOMETERS));
} }
/** /**