From 722b6eb3898a16638b8db81e56acd617517c2193 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 11 Mar 2016 12:18:02 +0100 Subject: [PATCH] DATAMONGO-1348 - Polishing. Add tests for aggregation using GeoJsonPoint. Extract GeoJson checks in own methods. Update license headers. Extract multiplier conversion in MetricConversion. Fix distanceMultiplier calculation to TargetUnit/BaseUnit instead of BaseUnit/TargetUnit. Original pull request: #339. --- .../mongodb/core/query/MetricConversion.java | 159 ++++++++++++++++++ .../data/mongodb/core/query/NearQuery.java | 37 ++-- .../mongodb/core/MongoTemplateUnitTests.java | 6 +- .../core/aggregation/AggregationTests.java | 51 +++++- .../core/query/MetricConversionUnitTests.java | 79 +++++++++ .../core/query/NearQueryUnitTests.java | 64 ++++++- 6 files changed, 365 insertions(+), 31 deletions(-) create mode 100644 spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/MetricConversion.java create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/MetricConversionUnitTests.java diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/MetricConversion.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/MetricConversion.java new file mode 100644 index 000000000..1c5bc86b3 --- /dev/null +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/MetricConversion.java @@ -0,0 +1,159 @@ +/* + * Copyright 2016 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; + +import java.math.BigDecimal; +import java.math.RoundingMode; + +import org.springframework.data.geo.Distance; +import org.springframework.data.geo.Metric; +import org.springframework.data.geo.Metrics; + +/** + * {@link Metric} and {@link Distance} conversions using the metric system. + * + * @author Mark Paluch + * @since 1.10 + */ +class MetricConversion { + + private static final BigDecimal METERS_MULTIPLIER = new BigDecimal(Metrics.KILOMETERS.getMultiplier()) + .multiply(new BigDecimal(1000)); + + // to achieve a calculation that is accurate to 0.3 meters + private static final int PRECISION = 8; + + /** + * Return meters to {@code metric} multiplier. + * + * @param metric + * @return + */ + protected static double getMetersToMetricMultiplier(Metric metric) { + + ConversionMultiplier conversionMultiplier = ConversionMultiplier.builder().from(METERS_MULTIPLIER).to(metric) + .build(); + return conversionMultiplier.multiplier().doubleValue(); + } + + /** + * Return {@code distance} in meters. + * + * @param distance + * @return + */ + protected static double getDistanceInMeters(Distance distance) { + return new BigDecimal(distance.getValue()).multiply(getMetricToMetersMultiplier(distance.getMetric())) + .doubleValue(); + } + + /** + * Return {@code metric} to meters multiplier. + * + * @param metric + * @return + */ + private static BigDecimal getMetricToMetersMultiplier(Metric metric) { + + ConversionMultiplier conversionMultiplier = ConversionMultiplier.builder().from(metric).to(METERS_MULTIPLIER) + .build(); + return conversionMultiplier.multiplier(); + } + + /** + * Provides a multiplier to convert between various metrics. Metrics must share the same base scale and provide a + * multiplier to convert between the base scale and its own metric. + * + * @author Mark Paluch + */ + private static class ConversionMultiplier { + + private final BigDecimal source; + private final BigDecimal target; + + ConversionMultiplier(Number source, Number target) { + + if (source instanceof BigDecimal) { + this.source = (BigDecimal) source; + } else { + this.source = new BigDecimal(source.doubleValue()); + } + + if (target instanceof BigDecimal) { + this.target = (BigDecimal) target; + } else { + this.target = new BigDecimal(target.doubleValue()); + } + } + + /** + * Returns the multiplier to convert a number from the {@code source} metric to the {@code target} metric. + * + * @return + */ + BigDecimal multiplier() { + return target.divide(source, PRECISION, RoundingMode.HALF_UP); + } + + /** + * Creates a new {@link ConversionMultiplierBuilder}. + * + * @return + */ + static ConversionMultiplierBuilder builder() { + return new ConversionMultiplierBuilder(); + } + + } + + /** + * Builder for {@link ConversionMultiplier}. + * + * @author Mark Paluch + */ + private static class ConversionMultiplierBuilder { + + private Number from; + private Number to; + + ConversionMultiplierBuilder() {} + + ConversionMultiplierBuilder from(Number from) { + this.from = from; + return this; + } + + ConversionMultiplierBuilder from(Metric from) { + this.from = from.getMultiplier(); + return this; + } + + ConversionMultiplierBuilder to(Number to) { + this.to = to; + return this; + } + + ConversionMultiplierBuilder to(Metric to) { + this.to = to.getMultiplier(); + return this; + } + + ConversionMultiplier build() { + return new ConversionMultiplier(this.from, this.to); + } + } +} diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/NearQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/NearQuery.java index fba9c6ad0..dbd99571f 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/NearQuery.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/NearQuery.java @@ -15,8 +15,6 @@ */ package org.springframework.data.mongodb.core.query; -import java.math.BigDecimal; -import java.math.RoundingMode; import java.util.Arrays; import org.bson.Document; @@ -51,8 +49,6 @@ public final class NearQuery { private @Nullable Long num; private @Nullable Long skip; - private static int PRECISION = 8; - /** * Creates a new {@link NearQuery}. * @@ -430,45 +426,38 @@ public final class NearQuery { } if (metric != null) { - document.put( - "distanceMultiplier", - usesMetricSystem() ? getMetricSystemNormalizer(metric).divide(new BigDecimal(1000), PRECISION + 3, - RoundingMode.HALF_UP).doubleValue() : metric.getMultiplier()); + document.put("distanceMultiplier", getDistanceMultiplier()); } if (num != null) { document.put("num", num); } - if (point instanceof GeoJsonPoint) { + if (usesGeoJson()) { document.put("near", point); } else { document.put("near", Arrays.asList(point.getX(), point.getY())); } - document.put("spherical", spherical ? spherical : point instanceof GeoJson); + document.put("spherical", spherical ? spherical : usesGeoJson()); return document; } + private double getDistanceMultiplier() { + return usesMetricSystem() ? MetricConversion.getMetersToMetricMultiplier(metric) : metric.getMultiplier(); + } + private double getDistanceValueInRadiantsOrMeters(Distance distance) { - return usesMetricSystem() ? getDistanceInMeters(distance) : distance.getNormalizedValue(); - } - - private double getDistanceInMeters(Distance distance) { - - return new BigDecimal(distance.getValue()).multiply(getMetricSystemNormalizer(distance.getMetric())) - .multiply(new BigDecimal(1000)).doubleValue(); - } - - private BigDecimal getMetricSystemNormalizer(Metric metric) { - - return new BigDecimal(Metrics.KILOMETERS.getMultiplier()).divide(new BigDecimal(metric.getMultiplier()), PRECISION, - RoundingMode.HALF_UP); + return usesMetricSystem() ? MetricConversion.getDistanceInMeters(distance) : distance.getNormalizedValue(); } private boolean usesMetricSystem() { - return point instanceof GeoJson; + return usesGeoJson(); + } + + private boolean usesGeoJson() { + return point instanceof GeoJsonPoint; } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java index 057766160..74b5bddec 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java @@ -1001,10 +1001,8 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests { ArgumentCaptor capture = ArgumentCaptor.forClass(Document.class); verify(this.db, times(1)).runCommand(capture.capture(), any(Class.class)); - assertThat( - capture.getValue(), - IsBsonObject.isBsonObject().containing("near.type", "Point").containing("near.coordinates.[0]", 1D) - .containing("near.coordinates.[1]", 2D)); + assertThat(capture.getValue(), IsBsonObject.isBsonObject().containing("near.type", "Point") + .containing("near.coordinates.[0]", 1D).containing("near.coordinates.[1]", 2D)); } static class WithNamedFields { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java index 5d5ef06da..b6f4c1efb 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java @@ -23,6 +23,7 @@ import static org.springframework.data.mongodb.core.aggregation.Fields.*; import static org.springframework.data.mongodb.core.query.Criteria.*; import static org.springframework.data.mongodb.test.util.IsBsonObject.*; +import com.mongodb.DBObject; import lombok.Builder; import java.io.BufferedInputStream; @@ -63,6 +64,8 @@ import org.springframework.data.mongodb.core.Venue; import org.springframework.data.mongodb.core.aggregation.AggregationTests.CarDescriptor.Entry; import org.springframework.data.mongodb.core.aggregation.BucketAutoOperation.Granularities; import org.springframework.data.mongodb.core.aggregation.VariableOperators.Let.ExpressionVariable; +import org.springframework.data.mongodb.core.geo.GeoJsonPoint; +import org.springframework.data.mongodb.core.index.GeoSpatialIndexType; import org.springframework.data.mongodb.core.index.GeospatialIndex; import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.core.query.NearQuery; @@ -517,7 +520,7 @@ public class AggregationTests { /* //complex mongodb aggregation framework example from https://docs.mongodb.org/manual/tutorial/aggregation-examples/#largest-and-smallest-cities-by-state - + db.zipcodes.aggregate( { $group: { @@ -1482,6 +1485,52 @@ public class AggregationTests { assertThat((Double) firstResult.get("distance"), closeTo(117.620092203928, 0.00001)); } + @Test // DATAMONGO-1348 + public void shouldSupportGeoJsonInGeoNearQueriesForAggregationWithDistanceField() { + + 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").typed(GeoSpatialIndexType.GEO_2DSPHERE)); + + NearQuery geoNear = NearQuery.near(new GeoJsonPoint(-73, 40), Metrics.KILOMETERS).num(10).maxDistance(150); + + Aggregation agg = newAggregation(Aggregation.geoNear(geoNear, "distance")); + AggregationResults result = mongoTemplate.aggregate(agg, Venue.class, Document.class); + + assertThat(result.getMappedResults(), hasSize(3)); + + Document firstResult = result.getMappedResults().get(0); + assertThat(firstResult.containsKey("distance"), is(true)); + assertThat((Double) firstResult.get("distance"), closeTo(117.61940988193759, 0.00001)); + } + + @Test // DATAMONGO-1348 + public void shouldSupportGeoJsonInGeoNearQueriesForAggregationWithDistanceFieldInMiles() { + + 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").typed(GeoSpatialIndexType.GEO_2DSPHERE)); + + NearQuery geoNear = NearQuery.near(new GeoJsonPoint(-73, 40), Metrics.KILOMETERS).num(10).maxDistance(150) + .inMiles(); + + Aggregation agg = newAggregation(Aggregation.geoNear(geoNear, "distance")); + AggregationResults result = mongoTemplate.aggregate(agg, Venue.class, Document.class); + + assertThat(result.getMappedResults(), hasSize(3)); + + Document firstResult = result.getMappedResults().get(0); + assertThat(firstResult.containsKey("distance"), is(true)); + assertThat((Double) firstResult.get("distance"), closeTo(73.08517, 0.00001)); + } + + @Test // DATAMONGO-1133 public void shouldHonorFieldAliasesForFieldReferences() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/MetricConversionUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/MetricConversionUnitTests.java new file mode 100644 index 000000000..303d15451 --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/MetricConversionUnitTests.java @@ -0,0 +1,79 @@ +/* + * Copyright 2016 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; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import org.junit.Test; +import org.springframework.data.geo.Distance; +import org.springframework.data.geo.Metrics; + +/** + * Unit tests for {@link MetricConversion}. + * + * @author Mark Paluch + */ +public class MetricConversionUnitTests { + + /** + * @see DATAMONGO-1348 + */ + @Test + public void shouldConvertMilesToMeters() { + + Distance distance = new Distance(1, Metrics.MILES); + double distanceInMeters = MetricConversion.getDistanceInMeters(distance); + + assertThat(distanceInMeters, is(closeTo(1609.3438343d, 0.000000001))); + } + + /** + * @see DATAMONGO-1348 + */ + @Test + public void shouldConvertKilometersToMeters() { + + Distance distance = new Distance(1, Metrics.KILOMETERS); + double distanceInMeters = MetricConversion.getDistanceInMeters(distance); + + assertThat(distanceInMeters, is(closeTo(1000, 0.000000001))); + } + + /** + * @see DATAMONGO-1348 + */ + @Test + public void shouldCalculateMetersToKilometersMultiplier() { + + double multiplier = MetricConversion.getMetersToMetricMultiplier(Metrics.KILOMETERS); + + assertThat(multiplier, is(closeTo(0.001, 0.000000001))); + } + + /** + * @see DATAMONGO-1348 + */ + @Test + public void shouldCalculateMetersToMilesMultiplier() { + + double multiplier = MetricConversion.getMetersToMetricMultiplier(Metrics.MILES); + + assertThat(multiplier, is(closeTo(0.00062137, 0.000000001))); + } + +} diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/NearQueryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/NearQueryUnitTests.java index 1472ca3ca..eb67e802b 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/NearQueryUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/NearQueryUnitTests.java @@ -19,6 +19,9 @@ import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; import static org.springframework.data.mongodb.test.util.IsBsonObject.*; +import java.math.BigDecimal; +import java.math.RoundingMode; + import org.junit.Test; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; @@ -144,14 +147,51 @@ public class NearQueryUnitTests { assertThat(DocumentTestUtils.getTypedValue(query.toDocument(), "num", Long.class), is(num)); } + @Test // DATAMONGO-1348 + public void shouldNotUseSphericalForLegacyPoint() { + + NearQuery query = NearQuery.near(new Point(27.987901, 86.9165379)); + + assertThat(query.toDocument(), isBsonObject().containing("spherical", false)); + } + + @Test // DATAMONGO-1348 + public void shouldUseSphericalForLegacyPointIfSet() { + + NearQuery query = NearQuery.near(new Point(27.987901, 86.9165379)); + query.spherical(true); + + assertThat(query.toDocument(), isBsonObject().containing("spherical", true)); + } + + @Test // DATAMONGO-1348 + public void shouldUseSphericalForGeoJsonData() { + + NearQuery query = NearQuery.near(new GeoJsonPoint(27.987901, 86.9165379)); + + assertThat(query.toDocument(), isBsonObject().containing("spherical", true)); + } + + @Test // DATAMONGO-1348 + public void shouldUseSphericalForGeoJsonDataIfSphericalIsFalse() { + + NearQuery query = NearQuery.near(new GeoJsonPoint(27.987901, 86.9165379)); + query.spherical(false); + + assertThat(query.toDocument(), isBsonObject().containing("spherical", true)); + } + @Test // DATAMONGO-1348 public void shouldUseMetersForGeoJsonData() { NearQuery query = NearQuery.near(new GeoJsonPoint(27.987901, 86.9165379)); query.maxDistance(1); + double meterToRadianMultiplier = BigDecimal.valueOf(1 / Metrics.KILOMETERS.getMultiplier() / 1000).// + setScale(8, RoundingMode.HALF_UP).// + doubleValue(); assertThat(query.toDocument(), isBsonObject().containing("maxDistance", Metrics.KILOMETERS.getMultiplier() * 1000) - .containing("distanceMultiplier", Metrics.KILOMETERS.getMultiplier() / 1000)); + .containing("distanceMultiplier", meterToRadianMultiplier)); } @Test // DATAMONGO-1348 @@ -171,6 +211,26 @@ public class NearQueryUnitTests { query.maxDistance(new Distance(1, Metrics.MILES)); assertThat(query.toDocument(), - isBsonObject().containing("maxDistance", 1609.34383D).containing("distanceMultiplier", 0.00160934383D)); + isBsonObject().containing("maxDistance", 1609.3438343D).containing("distanceMultiplier", 0.00062137D)); + } + + @Test // DATAMONGO-1348 + public void shouldUseKilometersForDistanceWhenMaxDistanceInMiles() { + + NearQuery query = NearQuery.near(new GeoJsonPoint(27.987901, 86.9165379)); + query.maxDistance(new Distance(1, Metrics.MILES)).in(Metrics.KILOMETERS); + + assertThat(query.toDocument(), + isBsonObject().containing("maxDistance", 1609.3438343D).containing("distanceMultiplier", 0.001D)); + } + + @Test // DATAMONGO-1348 + public void shouldUseMilesForDistanceWhenMaxDistanceInKilometers() { + + NearQuery query = NearQuery.near(new GeoJsonPoint(27.987901, 86.9165379)); + query.maxDistance(new Distance(1, Metrics.KILOMETERS)).in(Metrics.MILES); + + assertThat(query.toDocument(), + isBsonObject().containing("maxDistance", 1000D).containing("distanceMultiplier", 0.00062137D)); } }