From aa3f9c380b9ddde998caa5573d9c7388bf3fc00f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Tue, 17 Nov 2015 16:50:18 -0500 Subject: [PATCH] DATACOUCH-176 - Geo false positive autoelimination So far with geo queries a bounding box approximation was performed on Circle and Polygon queries (since Couchbase internally only supports Bounding Box queries). It was confusing since in previous iteration it was up to the user to remove these false positives "after the fact" in code calling the repository. This change allows for the framework to retain enough information to perform false positives elimination automatically and transparently for the user. --- .../repository/DimensionalQueryTests.java | 52 ++++-- .../data/couchbase/repository/Party.java | 1 + src/main/asciidoc/repository.adoc | 64 +------- .../query/SpatialViewBasedQuery.java | 23 ++- .../query/SpatialViewQueryCreator.java | 154 ++++++++++++++++-- .../repository/query/support/GeoUtils.java | 13 -- src/test/resources/logback.xml | 2 + 7 files changed, 199 insertions(+), 110 deletions(-) diff --git a/src/integration/java/org/springframework/data/couchbase/repository/DimensionalQueryTests.java b/src/integration/java/org/springframework/data/couchbase/repository/DimensionalQueryTests.java index 27edc43d..3ad49cab 100644 --- a/src/integration/java/org/springframework/data/couchbase/repository/DimensionalQueryTests.java +++ b/src/integration/java/org/springframework/data/couchbase/repository/DimensionalQueryTests.java @@ -83,7 +83,15 @@ public class DimensionalQueryTests { assertTrue(expectedKeys.contains(party.getKey())); } + //with this one, testparty-2 is within the bounding box but not in correct distance parties = repository.findByLocationNear(new Point(0, 0), new Distance(2.5)); + assertEquals(2, parties.size()); + for (Party party : parties) { + assertTrue(expectedKeys.contains(party.getKey())); + } + + //here we adjust the distance so that testparty-2 falls just on the edge + parties = repository.findByLocationNear(new Point(0, 0), new Distance(2.8284271247461903)); expectedKeys.add("testparty-2"); assertEquals(3, parties.size()); for (Party party : parties) { @@ -125,17 +133,19 @@ public class DimensionalQueryTests { @Test public void testByLocationInCircle() { - Circle zone1 = new Circle(new Point(-5,5), new Distance(5.5)); - Circle zone2 = new Circle(new Point(6, -6), new Distance(10)); + Circle zoneBboxFalse = new Circle(new Point(-5,5), new Distance(5.5)); + Circle zoneEdge = new Circle(new Point(-5, 0), new Distance(5)); + Circle zoneInside = new Circle(new Point(6, -6), new Distance(10)); Circle zoneEmpty = new Circle(new Point(6,6), new Distance(3)); - List parties = repository.findByLocationWithin(zone1); + List parties = repository.findByLocationWithin(zoneBboxFalse); + assertEquals(0, parties.size()); + parties = repository.findByLocationWithin(zoneEdge); assertEquals(1, parties.size()); assertEquals("testparty-0", parties.get(0).getKey()); - parties = repository.findByLocationWithin(zone2); - + parties = repository.findByLocationWithin(zoneInside); assertEquals(12, parties.size()); //all the parties except the special one at 100, 100 parties = repository.findByLocationWithin(zoneEmpty); @@ -163,14 +173,22 @@ public class DimensionalQueryTests { @Test public void testByLocationInPolygon() { - //bounding box of (-10.5, -0.5),(0.5, 10.5) - Polygon zone1 = new Polygon( - new Point(-10.5, -0.5), - new Point(-10.5, 10.5), - new Point(0.5, 10.5), - new Point(0.4, 1)); + //slightly skewed square triangle that cuts just short of (0,0) + //bounding box of (-1,-1),(0.5,1) + Polygon zoneFalsePositive = new Polygon( + new Point(-1, 1), + new Point(0.5, 1), + new Point(-1, -1) + ); + //square triangle that comes through (0,0) + //bounding box of (-1,-1),(1,1) + Polygon zoneEdge = new Polygon( + new Point(-1, 1), + new Point(1, 1), + new Point(-1, -1) + ); //bounding box of (-4, -16),(16,4) - Polygon zone2 = new Polygon( + Polygon zoneWithin = new Polygon( new Point(-4, -10), new Point(-3, 4), new Point(14, 2), @@ -186,13 +204,13 @@ public class DimensionalQueryTests { new Point(6, 5), new Point(6, 3)); - List parties = repository.findByLocationWithin(zone1); + List parties = repository.findByLocationWithin(zoneFalsePositive); + assertEquals("points outside a polygon but within bounding box shouldn't be considered within", 0, parties.size()); - assertEquals(1, parties.size()); - assertEquals("testparty-0", parties.get(0).getKey()); - - parties = repository.findByLocationWithin(zone2); + parties = repository.findByLocationWithin(zoneEdge); + assertEquals("point on edge of a polygon shouldn't be considered within", 0, parties.size()); + parties = repository.findByLocationWithin(zoneWithin); assertEquals(12, parties.size()); //all the parties except the special one at 100, 100 parties = repository.findByLocationWithin(zoneEmpty); diff --git a/src/integration/java/org/springframework/data/couchbase/repository/Party.java b/src/integration/java/org/springframework/data/couchbase/repository/Party.java index fe5cac9c..7456f86c 100644 --- a/src/integration/java/org/springframework/data/couchbase/repository/Party.java +++ b/src/integration/java/org/springframework/data/couchbase/repository/Party.java @@ -82,6 +82,7 @@ public class Party { return "Party{" + "name='" + name + '\'' + ", eventDate=" + eventDate + + ", location=" + location + '}'; } } diff --git a/src/main/asciidoc/repository.adoc b/src/main/asciidoc/repository.adoc index 418f4132..311e240e 100644 --- a/src/main/asciidoc/repository.adoc +++ b/src/main/asciidoc/repository.adoc @@ -350,7 +350,7 @@ Multi-dimensionality concept is interesting, it means you can craft views that a Couchbase's Spatial View support querying through ranges that represent "lowest" and "highest" values in each dimension, so for 2D it represents a bounding box, with the southwest-most point [x,y] as `startRange` and northeast-most point [x,y] as `endRange`. -WARNING: Some forms of Geographical queries can only be approximated, for instance querying with a `Polygon` or a `Circle` can only be approximated to the containing bounding `Box` (in DEBUG log level this will be made explicit with each such query). +TIP: Even though Couchbase Spatial View engine only support Bounding Box querying, the Spring Data Couchbase framework will attempt to remove false positives for you when querying with a `Polygon` or a `Circle` (in TRACE log level each false positive elimination will be logged). Note that a point on the edge of a `Polygon` is *not* considered within (whereas it is when dealing with a `Circle`). The following query derivation keywords and parameters relative to geographical data in Spring Data are supported for Spatial Views: @@ -367,67 +367,7 @@ The following query derivation keywords and parameters relative to geographical IMPORTANT: For "within" types of queries, the expected parameters map to geographical 2D data. Classes from the `org.springframework.data.geo` package are usually expected, but Polygon and Boxes can also be expressed as arrays of `Point`s. -TIP: Further dimensions are supported through keywords other than Within and Near and require numerical input. - -=== Removing False Positives -`Circle` and `Polygon` will be approximated to their surrounding bounding `Box`, but we provide `PointInShapeEvaluator` (`AwtPointInShapeEvaluator`) to be used on the results if you want to remove false positives. - -Here is an example of a repository using a spatial view with 3 dimensions (x, y and opening hours). -Second example demonstrates querying it within a `Polygon` then removing false positives in a second pass: - -.Example of dimensional repository -==== -[source,java] ----- -public interface StoreRepository extends PagingAndSortingRepository { - - @Dimensional(designDocument = "store", spatialViewName = "byLocationOpening", dimensions = 3) - public List findByLocationWithinAndOpenBefore(Polygon zone, Integer minOpeningHourMinute); - -} ----- -==== - -.Query a spatial view and eliminate false positives on the geo aspect -==== -[source,java] ----- -// Load the bean, or @Autowire it -StoreRepository repo = ctx.getBean(StoreRepository.class); - -// Prepare an arbitrary polygon (a triangle) -Polygon zone = new Polygon( - new Point(2.0, 0.0), - new Point(4.0, 0.0), - new Point(3.0, -5.5), - new Point(2.0, 0.0)); //we explicitly closed the polygon - -// Find all stores located inside the triangle. -// Use 3rd dimension to also query on opening hours, making sure one can go there at 8:00am -List stores = repo.findByLocationWithinAndOpenBefore(zone, 800); - -// The view will return all matching stores that are actually within the enclosing rectangle around the polygon -// So we remove false positives -PointInShapeEvaluator evaluator = new AwtPointInShapeEvaluator(); -List truePositives = new ArrayList(stores.size()); -for (Store store : stores) { - if (evaluator.pointInPolygon(store.getLocation(), zone)) { - truePositives.add(store); - } //else this is a false positive -} - -// Alternatively, use "removeFalsePositives" to get the list right away. -truePositives = evaluator.removeFalsePositives(stores, - //this method needs a Converter to know where to find the location - new Converter() { - @Override - public Point convert(Store source) { - return source.getLocation(); - } - }, - zone); ----- -==== +Further dimensions are supported through keywords other than Within and Near and require numerical input. [[couchbase.repository.consistency]] == Querying with consistency diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/SpatialViewBasedQuery.java b/src/main/java/org/springframework/data/couchbase/repository/query/SpatialViewBasedQuery.java index bae0f607..4f8f7909 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/SpatialViewBasedQuery.java +++ b/src/main/java/org/springframework/data/couchbase/repository/query/SpatialViewBasedQuery.java @@ -16,14 +16,26 @@ package org.springframework.data.couchbase.repository.query; +import java.beans.PropertyDescriptor; +import java.util.ArrayList; +import java.util.List; + import com.couchbase.client.java.view.SpatialViewQuery; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.BeanWrapper; +import org.springframework.beans.BeanWrapperImpl; import org.springframework.data.couchbase.core.CouchbaseOperations; +import org.springframework.data.couchbase.core.CouchbaseTemplate; +import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty; import org.springframework.data.couchbase.core.query.Dimensional; +import org.springframework.data.couchbase.repository.query.support.GeoUtils; +import org.springframework.data.mapping.PersistentEntity; +import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.repository.query.ParametersParameterAccessor; import org.springframework.data.repository.query.RepositoryQuery; +import org.springframework.data.repository.query.parser.Part; import org.springframework.data.repository.query.parser.PartTree; /** @@ -64,17 +76,20 @@ public class SpatialViewBasedQuery implements RepositoryQuery { SpatialViewQueryCreator creator = new SpatialViewQueryCreator(dimensions, tree, new ParametersParameterAccessor(method.getParameters(), runtimeParams), baseSpatialQuery, operations.getConverter()); - SpatialViewQuery finalQuery = creator.createQuery(); + SpatialViewQueryCreator.SpatialViewQueryWrapper finalQuery = creator.createQuery(); //execute the spatial query return execute(finalQuery); } - protected Object execute(SpatialViewQuery query) { + protected Object execute(SpatialViewQueryCreator.SpatialViewQueryWrapper query) { if (LOG.isDebugEnabled()) { - LOG.debug("Executing spatial view query: " + query.toString()); + LOG.debug("Executing spatial view query: " + query.getQuery().toString()); } - return operations.findBySpatialView(query, method.getEntityInformation().getJavaType()); + + return query.eliminate( + operations.findBySpatialView(query.getQuery(), method.getEntityInformation().getJavaType()) + ); } @Override diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/SpatialViewQueryCreator.java b/src/main/java/org/springframework/data/couchbase/repository/query/SpatialViewQueryCreator.java index ad0b4d23..7dd67bab 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/SpatialViewQueryCreator.java +++ b/src/main/java/org/springframework/data/couchbase/repository/query/SpatialViewQueryCreator.java @@ -16,18 +16,30 @@ package org.springframework.data.couchbase.repository.query; +import java.util.ArrayList; import java.util.Iterator; +import java.util.List; import com.couchbase.client.java.document.json.JsonArray; import com.couchbase.client.java.view.SpatialViewQuery; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.BeanWrapper; +import org.springframework.beans.BeanWrapperImpl; import org.springframework.data.couchbase.core.convert.CouchbaseConverter; import org.springframework.data.couchbase.core.query.Dimensional; +import org.springframework.data.couchbase.repository.query.support.AwtPointInShapeEvaluator; import org.springframework.data.couchbase.repository.query.support.GeoUtils; +import org.springframework.data.couchbase.repository.query.support.PointInShapeEvaluator; import org.springframework.data.domain.Sort; +import org.springframework.data.geo.Box; +import org.springframework.data.geo.Circle; import org.springframework.data.geo.Distance; import org.springframework.data.geo.Point; +import org.springframework.data.geo.Polygon; import org.springframework.data.geo.Shape; +import org.springframework.data.mapping.PropertyPath; import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.parser.AbstractQueryCreator; import org.springframework.data.repository.query.parser.Part; @@ -58,7 +70,9 @@ import org.springframework.data.repository.query.parser.PartTree; *

* Additionally, {@link PartTree#isLimiting()} will trigger usage of {@link SpatialViewQuery#limit(int) limit}. */ -public class SpatialViewQueryCreator extends AbstractQueryCreator { +public class SpatialViewQueryCreator extends AbstractQueryCreator { + + private static final Logger LOGGER = LoggerFactory.getLogger(SpatialViewQueryCreator.class); private final SpatialViewQuery query; private final PartTree tree; @@ -67,6 +81,7 @@ public class SpatialViewQueryCreator extends AbstractQueryCreator evaluators; public SpatialViewQueryCreator(int dimensions, PartTree tree, ParameterAccessor parameters, SpatialViewQuery query, CouchbaseConverter converter) { @@ -77,6 +92,7 @@ public class SpatialViewQueryCreator extends AbstractQueryCreator(); } @Override @@ -85,10 +101,10 @@ public class SpatialViewQueryCreator extends AbstractQueryCreator iterator) { + private static void applyWithin(JsonArray startRange, JsonArray endRange, Iterator iterator, + List evaluators, PropertyPath path) { if (!iterator.hasNext()) { throw new IllegalArgumentException("Not enough parameters for within"); } Object next = iterator.next(); - if (next instanceof Shape) { - GeoUtils.convertShapeTo2DRanges(startRange, endRange, (Shape) next); + if (next instanceof Circle) { + evaluators.add(new CircleFalsePositiveEvaluator(path, (Circle) next)); + GeoUtils.convertShapeTo2DRanges(startRange, endRange, (Circle) next); + } else if (next instanceof Polygon) { + evaluators.add(new PolygonFalsePositiveEvaluator(path, (Polygon) next)); + GeoUtils.convertShapeTo2DRanges(startRange, endRange, (Polygon) next); + } else if (next instanceof Box) { + GeoUtils.convertShapeTo2DRanges(startRange, endRange, (Box) next); } else if (next instanceof Point) { //expect another point for the other corner of the bounding box Point northwest = (Point) next; Point southeast = checkedNext(iterator, Point.class, "Cannot compute a bounding box for within, 2 Point needed"); GeoUtils.convertPointsTo2DRanges(startRange, endRange, true, northwest, southeast); } else if (next instanceof Point[]) { + evaluators.add(new PointArrayFalsePositiveEvaluator(path, (Point[]) next)); GeoUtils.convertPointsTo2DRanges(startRange, endRange, false, (Point[]) next); } else if (next instanceof JsonArray) { //discouraged, leaks Couchbase classes into signatures JsonArray first = (JsonArray) next; @@ -151,14 +175,12 @@ public class SpatialViewQueryCreator extends AbstractQueryCreator: same as polygon - //TODO Collection: one collection per range - //TODO Number[]: one array per range throw new IllegalArgumentException("Unsupported parameter type for within: " + next.getClass()); } } - private static void applyNear(JsonArray startRange, JsonArray endRange, Iterator iterator) { + private static void applyNear(JsonArray startRange, JsonArray endRange, Iterator iterator, + List evaluators, PropertyPath path) { if (!iterator.hasNext()) { throw new IllegalArgumentException("Not enough parameters for near"); } @@ -166,6 +188,8 @@ public class SpatialViewQueryCreator extends AbstractQueryCreator eliminators; + + public SpatialViewQueryWrapper(SpatialViewQuery query, List eliminators) { + this.query = query; + this.eliminators = eliminators; + } + + public SpatialViewQuery getQuery() { + return query; + } + + public List eliminate(List objects) { + List result = new ArrayList(objects.size()); + for (T object : objects) { + BeanWrapper bean = new BeanWrapperImpl(object); + boolean pass = true; + for (AbstractFalsePositiveEvaluator eliminator : eliminators) { + pass = pass && eliminator.evaluate(object, bean); + } + if (pass) { + result.add(object); + } else { + LOGGER.trace("Object {} was a false positive in geo query", object); + } + } + return result; + } + } + +} \ No newline at end of file diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/support/GeoUtils.java b/src/main/java/org/springframework/data/couchbase/repository/query/support/GeoUtils.java index b8a7210c..16a5eede 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/support/GeoUtils.java +++ b/src/main/java/org/springframework/data/couchbase/repository/query/support/GeoUtils.java @@ -34,8 +34,6 @@ import org.springframework.data.geo.Shape; */ public class GeoUtils { - private static final Logger LOGGER = LoggerFactory.getLogger(GeoUtils.class); - /** * Computes the bounding box approximation for a "near" query (max distance from a point of origin). * @@ -46,7 +44,6 @@ public class GeoUtils { */ public static double[] getBoundingBoxForNear(Point origin, Distance distance) { if (origin == null || distance == null) throw new NullPointerException("Origin and distance required"); - warnBoundingBoxApproximation("near a Point (within Distance)"); //since maxDistance COULD be negative, we have to make sure we have correct min/max double maxDistance = Math.abs(distance.getNormalizedValue()); @@ -90,7 +87,6 @@ public class GeoUtils { endRange.add(points[1].getX()).add(points[1].getY()); } else { //this is like a polygon, find the bounding box - warnBoundingBoxApproximation("within a sequence of Points (polygon)"); //find the lowest and highest X and Y to get the bounding box double xMin = Double.POSITIVE_INFINITY; double yMin = Double.POSITIVE_INFINITY; @@ -128,7 +124,6 @@ public class GeoUtils { .add(box.getSecond().getX()) .add(box.getSecond().getY()); } else if (shape instanceof Polygon) { - warnBoundingBoxApproximation("within a Polygon"); //find the lowest and highest X and Y to get the bounding box double xMin = Double.POSITIVE_INFINITY; double yMin = Double.POSITIVE_INFINITY; @@ -146,7 +141,6 @@ public class GeoUtils { startRange.add(xMin).add(yMin); endRange.add(xMax).add(yMax); } else if (shape instanceof Circle) { - warnBoundingBoxApproximation("within a Circle"); //here the bounding box is the box that contains the circle Circle circle = (Circle) shape; Point center = circle.getCenter(); @@ -163,11 +157,4 @@ public class GeoUtils { throw new IllegalArgumentException("Unsupported shape " + shape.getClass().getName()); } } - - private static void warnBoundingBoxApproximation(String kind) { - if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Spatial View queries " + kind + " are made using a bounding box approximation," + - "you probably want to remove false positives by applying a PointInShapeEvaluator"); - } - } } diff --git a/src/test/resources/logback.xml b/src/test/resources/logback.xml index 710d7c16..04281627 100644 --- a/src/test/resources/logback.xml +++ b/src/test/resources/logback.xml @@ -17,10 +17,12 @@ +