From 081e0e9d612ad430f7b5d28b36e02b1194ede122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 4 Dec 2015 13:33:30 +0100 Subject: [PATCH] Changes after rebase on master --- .../geo/builders/LineStringBuilder.java | 7 +- .../geo/builders/MultiLineStringBuilder.java | 2 +- .../geo/builders/MultiPointBuilder.java | 2 +- .../common/geo/builders/PolygonBuilder.java | 144 ++++++++++++++++++ .../AbstractShapeBuilderTestCase.java | 18 +-- .../geo/builders/CircleBuilderTests.java | 24 +-- .../geo/builders/EnvelopeBuilderTests.java | 24 +-- .../GeometryCollectionBuilderTests.java | 40 ++--- .../geo/builders/LineStringBuilderTests.java | 24 +-- .../builders/MultiLineStringBuilderTests.java | 16 +- .../geo/builders/MultiPointBuilderTests.java | 16 +- .../builders/MultiPolygonBuilderTests.java | 32 ++-- .../geo/builders/PointBuilderTests.java | 18 ++- .../geo/builders/PolygonBuilderTests.java | 22 +-- .../query/GeoShapeQueryBuilderTests.java | 7 - 15 files changed, 291 insertions(+), 105 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java index cec2a66e757..464d72c8d8c 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java @@ -148,7 +148,7 @@ public class LineStringBuilder extends PointCollection { @Override public int hashCode() { - return Objects.hash(points, translated()); + return Objects.hash(points); } @Override @@ -160,8 +160,7 @@ public class LineStringBuilder extends PointCollection { return false; } LineStringBuilder other = (LineStringBuilder) obj; - return Objects.equals(points, other.points) && - (translated() == other.translated()); + return Objects.equals(points, other.points); } @Override @@ -170,7 +169,6 @@ public class LineStringBuilder extends PointCollection { for (Coordinate point : points) { writeCoordinateTo(point, out); } - out.writeBoolean(translated()); } @Override @@ -180,7 +178,6 @@ public class LineStringBuilder extends PointCollection { for (int i=0; i < size; i++) { lineStringBuilder.point(readCoordinateFrom(in)); } - lineStringBuilder.translated(in.readBoolean()); return lineStringBuilder; } } diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java index c0a79611dec..4703ac19b08 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java @@ -131,7 +131,7 @@ public class MultiLineStringBuilder extends ShapeBuilder { public MultiLineStringBuilder readFrom(StreamInput in) throws IOException { MultiLineStringBuilder multiLineStringBuilder = new MultiLineStringBuilder(); int size = in.readVInt(); - for (int i=0; i < size; i++) { + for (int i = 0; i < size; i++) { multiLineStringBuilder.linestring(LineStringBuilder.PROTOTYPE.readFrom(in)); } return multiLineStringBuilder; diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java index f1b403d42b9..a4d236e3557 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java @@ -56,7 +56,7 @@ public class MultiPointBuilder extends PointCollection { for (Coordinate coord : points) { shapes.add(SPATIAL_CONTEXT.makePoint(coord.x, coord.y)); } - XShapeCollection multiPoints = new XShapeCollection<>(shapes, SPATIAL_CONTEXT); + XShapeCollection multiPoints = new XShapeCollection<>(shapes, SPATIAL_CONTEXT); multiPoints.setPointsOnly(true); return multiPoints; } diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java index 80591dbb3eb..fefbcb348ca 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java @@ -541,6 +541,150 @@ public class PolygonBuilder extends ShapeBuilder { return points.length-1; } + /** + * Create a connected list of a list of coordinates + * + * @param points + * array of point + * @param offset + * index of the first point + * @param length + * number of points + * @return Array of edges + */ + private static Edge[] ring(int component, boolean direction, boolean handedness, LineStringBuilder shell, + Coordinate[] points, int offset, Edge[] edges, int toffset, int length, final AtomicBoolean translated) { + // calculate the direction of the points: + // find the point a the top of the set and check its + // neighbors orientation. So direction is equivalent + // to clockwise/counterclockwise + final int top = top(points, offset, length); + final int prev = (offset + ((top + length - 1) % length)); + final int next = (offset + ((top + 1) % length)); + boolean orientation = points[offset + prev].x > points[offset + next].x; + + // OGC requires shell as ccw (Right-Handedness) and holes as cw (Left-Handedness) + // since GeoJSON doesn't specify (and doesn't need to) GEO core will assume OGC standards + // thus if orientation is computed as cw, the logic will translate points across dateline + // and convert to a right handed system + + // compute the bounding box and calculate range + double[] range = range(points, offset, length); + final double rng = range[1] - range[0]; + // translate the points if the following is true + // 1. shell orientation is cw and range is greater than a hemisphere (180 degrees) but not spanning 2 hemispheres + // (translation would result in a collapsed poly) + // 2. the shell of the candidate hole has been translated (to preserve the coordinate system) + boolean incorrectOrientation = component == 0 && handedness != orientation; + if ( (incorrectOrientation && (rng > DATELINE && rng != 2*DATELINE)) || (translated.get() && component != 0)) { + translate(points); + // flip the translation bit if the shell is being translated + if (component == 0) { + translated.set(true); + } + // correct the orientation post translation (ccw for shell, cw for holes) + if (component == 0 || (component != 0 && handedness == orientation)) { + orientation = !orientation; + } + } + return concat(component, direction ^ orientation, points, offset, edges, toffset, length); + } + + private static final int top(Coordinate[] points, int offset, int length) { + int top = 0; // we start at 1 here since top points to 0 + for (int i = 1; i < length; i++) { + if (points[offset + i].y < points[offset + top].y) { + top = i; + } else if (points[offset + i].y == points[offset + top].y) { + if (points[offset + i].x < points[offset + top].x) { + top = i; + } + } + } + return top; + } + + private static final double[] range(Coordinate[] points, int offset, int length) { + double minX = points[0].x; + double maxX = points[0].x; + double minY = points[0].y; + double maxY = points[0].y; + // compute the bounding coordinates (@todo: cleanup brute force) + for (int i = 1; i < length; ++i) { + if (points[offset + i].x < minX) { + minX = points[offset + i].x; + } + if (points[offset + i].x > maxX) { + maxX = points[offset + i].x; + } + if (points[offset + i].y < minY) { + minY = points[offset + i].y; + } + if (points[offset + i].y > maxY) { + maxY = points[offset + i].y; + } + } + return new double[] {minX, maxX, minY, maxY}; + } + + /** + * Concatenate a set of points to a polygon + * + * @param component + * component id of the polygon + * @param direction + * direction of the ring + * @param points + * list of points to concatenate + * @param pointOffset + * index of the first point + * @param edges + * Array of edges to write the result to + * @param edgeOffset + * index of the first edge in the result + * @param length + * number of points to use + * @return the edges creates + */ + private static Edge[] concat(int component, boolean direction, Coordinate[] points, final int pointOffset, Edge[] edges, final int edgeOffset, + int length) { + assert edges.length >= length+edgeOffset; + assert points.length >= length+pointOffset; + edges[edgeOffset] = new Edge(points[pointOffset], null); + for (int i = 1; i < length; i++) { + if (direction) { + edges[edgeOffset + i] = new Edge(points[pointOffset + i], edges[edgeOffset + i - 1]); + edges[edgeOffset + i].component = component; + } else if(!edges[edgeOffset + i - 1].coordinate.equals(points[pointOffset + i])) { + edges[edgeOffset + i - 1].next = edges[edgeOffset + i] = new Edge(points[pointOffset + i], null); + edges[edgeOffset + i - 1].component = component; + } else { + throw new InvalidShapeException("Provided shape has duplicate consecutive coordinates at: " + points[pointOffset + i]); + } + } + + if (direction) { + edges[edgeOffset].setNext(edges[edgeOffset + length - 1]); + edges[edgeOffset].component = component; + } else { + edges[edgeOffset + length - 1].setNext(edges[edgeOffset]); + edges[edgeOffset + length - 1].component = component; + } + + return edges; + } + + /** + * Transforms coordinates in the eastern hemisphere (-180:0) to a (180:360) range + */ + private static void translate(Coordinate[] points) { + for (Coordinate c : points) { + if (c.x < 0) { + c.x += 2*DATELINE; + } + } + } + @Override public int hashCode() { return Objects.hash(shell, holes, orientation); diff --git a/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java b/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java index a7fbfb8e380..10a5070f3f8 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java +++ b/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java @@ -69,7 +69,7 @@ public abstract class AbstractShapeBuilderTestCase exte /** * mutate the given shape so the returned shape is different */ - protected abstract SB mutate(SB original) throws IOException; + protected abstract SB createMutation(SB original) throws IOException; /** * Test that creates new shape from a random test shape and checks both for equality @@ -95,10 +95,11 @@ public abstract class AbstractShapeBuilderTestCase exte /** * Test serialization and deserialization of the test shape. */ + @SuppressWarnings("unchecked") public void testSerialization() throws IOException { for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) { SB testShape = createTestShapeBuilder(); - SB deserializedShape = copyShape(testShape); + SB deserializedShape = (SB) copyShape(testShape); assertEquals(testShape, deserializedShape); assertEquals(testShape.hashCode(), deserializedShape.hashCode()); assertNotSame(testShape, deserializedShape); @@ -108,6 +109,7 @@ public abstract class AbstractShapeBuilderTestCase exte /** * Test equality and hashCode properties */ + @SuppressWarnings("unchecked") public void testEqualsAndHashcode() throws IOException { for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) { SB firstShape = createTestShapeBuilder(); @@ -116,15 +118,15 @@ public abstract class AbstractShapeBuilderTestCase exte assertTrue("shape is not equal to self", firstShape.equals(firstShape)); assertThat("same shape's hashcode returns different values if called multiple times", firstShape.hashCode(), equalTo(firstShape.hashCode())); - assertThat("different shapes should not be equal", mutate(firstShape), not(equalTo(firstShape))); + assertThat("different shapes should not be equal", createMutation(firstShape), not(equalTo(firstShape))); - SB secondShape = copyShape(firstShape); + SB secondShape = (SB) copyShape(firstShape); assertTrue("shape is not equal to self", secondShape.equals(secondShape)); assertTrue("shape is not equal to its copy", firstShape.equals(secondShape)); assertTrue("equals is not symmetric", secondShape.equals(firstShape)); assertThat("shape copy's hashcode is different from original hashcode", secondShape.hashCode(), equalTo(firstShape.hashCode())); - SB thirdShape = copyShape(secondShape); + SB thirdShape = (SB) copyShape(secondShape); assertTrue("shape is not equal to self", thirdShape.equals(thirdShape)); assertTrue("shape is not equal to its copy", secondShape.equals(thirdShape)); assertThat("shape copy's hashcode is different from original hashcode", secondShape.hashCode(), equalTo(thirdShape.hashCode())); @@ -135,14 +137,12 @@ public abstract class AbstractShapeBuilderTestCase exte } } - protected SB copyShape(SB original) throws IOException { + static ShapeBuilder copyShape(ShapeBuilder original) throws IOException { try (BytesStreamOutput output = new BytesStreamOutput()) { original.writeTo(output); try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) { ShapeBuilder prototype = (ShapeBuilder) namedWriteableRegistry.getPrototype(ShapeBuilder.class, original.getWriteableName()); - @SuppressWarnings("unchecked") - SB copy = (SB) prototype.readFrom(in); - return copy; + return prototype.readFrom(in); } } } diff --git a/core/src/test/java/org/elasticsearch/common/geo/builders/CircleBuilderTests.java b/core/src/test/java/org/elasticsearch/common/geo/builders/CircleBuilderTests.java index 17ed5e19876..1db9da428ad 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/builders/CircleBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/common/geo/builders/CircleBuilderTests.java @@ -27,20 +27,18 @@ import java.io.IOException; public class CircleBuilderTests extends AbstractShapeBuilderTestCase { - final static CircleBuilderTests PROTOTYPE = new CircleBuilderTests(); - @Override protected CircleBuilder createTestShapeBuilder() { - double centerX = randomDoubleBetween(-180, 180, false); - double centerY = randomDoubleBetween(-90, 90, false); - return new CircleBuilder() - .center(new Coordinate(centerX, centerY)) - .radius(randomDoubleBetween(0.1, 10.0, false), randomFrom(DistanceUnit.values())); + return createRandomShape(); } @Override - protected CircleBuilder mutate(CircleBuilder original) throws IOException { - CircleBuilder mutation = copyShape(original); + protected CircleBuilder createMutation(CircleBuilder original) throws IOException { + return mutate(original); + } + + static CircleBuilder mutate(CircleBuilder original) throws IOException { + CircleBuilder mutation = (CircleBuilder) copyShape(original); double radius = original.radius(); DistanceUnit unit = original.unit(); @@ -57,4 +55,12 @@ public class CircleBuilderTests extends AbstractShapeBuilderTestCase { - static final EnvelopeBuilderTests PROTOTYPE = new EnvelopeBuilderTests(); - @Override protected EnvelopeBuilder createTestShapeBuilder() { - EnvelopeBuilder envelope = new EnvelopeBuilder(randomFrom(Orientation.values())); - Rectangle box = RandomShapeGenerator.xRandomRectangle(getRandom(), RandomShapeGenerator.xRandomPoint(getRandom())); - envelope.topLeft(box.getMinX(), box.getMaxY()) - .bottomRight(box.getMaxX(), box.getMinY()); - return envelope; + return createRandomShape(); } @Override - protected EnvelopeBuilder mutate(EnvelopeBuilder original) throws IOException { - EnvelopeBuilder mutation = copyShape(original); + protected EnvelopeBuilder createMutation(EnvelopeBuilder original) throws IOException { + return mutate(original); + } + + static EnvelopeBuilder mutate(EnvelopeBuilder original) throws IOException { + EnvelopeBuilder mutation = (EnvelopeBuilder) copyShape(original); if (randomBoolean()) { // toggle orientation mutation.orientation = (original.orientation == Orientation.LEFT ? Orientation.RIGHT : Orientation.LEFT); @@ -65,4 +63,12 @@ public class EnvelopeBuilderTests extends AbstractShapeBuilderTestCase { - static final LineStringBuilderTests PROTOTYPE = new LineStringBuilderTests(); - @Override protected LineStringBuilder createTestShapeBuilder() { - LineStringBuilder lsb = (LineStringBuilder) RandomShapeGenerator.createShape(getRandom(), ShapeType.LINESTRING); - if (randomBoolean()) { - lsb.close(); - } - return lsb; + return createRandomShape(); } @Override - protected LineStringBuilder mutate(LineStringBuilder original) throws IOException { - LineStringBuilder mutation = copyShape(original); + protected LineStringBuilder createMutation(LineStringBuilder original) throws IOException { + return mutate(original); + } + + static LineStringBuilder mutate(LineStringBuilder original) throws IOException { + LineStringBuilder mutation = (LineStringBuilder) copyShape(original); Coordinate[] coordinates = original.coordinates(false); Coordinate coordinate = randomFrom(coordinates); if (randomBoolean()) { @@ -59,4 +57,12 @@ public class LineStringBuilderTests extends AbstractShapeBuilderTestCase { - static final MultiLineStringBuilderTests PROTOTYPE = new MultiLineStringBuilderTests(); - @Override protected MultiLineStringBuilder createTestShapeBuilder() { - return (MultiLineStringBuilder) RandomShapeGenerator.createShape(getRandom(), ShapeType.MULTILINESTRING); + return createRandomShape(); } @Override - protected MultiLineStringBuilder mutate(MultiLineStringBuilder original) throws IOException { - MultiLineStringBuilder mutation = copyShape(original); + protected MultiLineStringBuilder createMutation(MultiLineStringBuilder original) throws IOException { + return mutate(original); + } + + static MultiLineStringBuilder mutate(MultiLineStringBuilder original) throws IOException { + MultiLineStringBuilder mutation = (MultiLineStringBuilder) copyShape(original); Coordinate[][] coordinates = mutation.coordinates(); int lineToChange = randomInt(coordinates.length - 1); for (int i = 0; i < coordinates.length; i++) { @@ -61,4 +63,8 @@ public class MultiLineStringBuilderTests extends AbstractShapeBuilderTestCase { - static final MultiPointBuilderTests PROTOTYPE = new MultiPointBuilderTests(); - @Override protected MultiPointBuilder createTestShapeBuilder() { - return (MultiPointBuilder) RandomShapeGenerator.createShape(getRandom(), ShapeType.MULTIPOINT); + return createRandomShape(); } @Override - protected MultiPointBuilder mutate(MultiPointBuilder original) throws IOException { - MultiPointBuilder mutation = copyShape(original); + protected MultiPointBuilder createMutation(MultiPointBuilder original) throws IOException { + return mutate(original); + } + + static MultiPointBuilder mutate(MultiPointBuilder original) throws IOException { + MultiPointBuilder mutation = (MultiPointBuilder) copyShape(original); Coordinate[] coordinates = original.coordinates(false); Coordinate coordinate = randomFrom(coordinates); if (randomBoolean()) { @@ -55,4 +57,8 @@ public class MultiPointBuilderTests extends AbstractShapeBuilderTestCase { - static final MultiPolygonBuilderTests PROTOTYPE = new MultiPolygonBuilderTests(); - @Override protected MultiPolygonBuilder createTestShapeBuilder() { - MultiPolygonBuilder mpb = new MultiPolygonBuilder(randomFrom(Orientation.values())); - int polys = randomIntBetween(1, 10); - for (int i = 0; i < polys; i++) { - PolygonBuilder pgb = (PolygonBuilder) RandomShapeGenerator.createShape(getRandom(), ShapeType.POLYGON); - pgb.orientation = mpb.orientation; - // NORELEASE translated might have been changed by createShape, but won't survive xContent->Parse roundtrip - pgb.shell().translated(false); - mpb.polygon(pgb); - } - return mpb; + return createRandomShape(); } @Override - protected MultiPolygonBuilder mutate(MultiPolygonBuilder original) throws IOException { - MultiPolygonBuilder mutation = copyShape(original); + protected MultiPolygonBuilder createMutation(MultiPolygonBuilder original) throws IOException { + return mutate(original); + } + + static MultiPolygonBuilder mutate(MultiPolygonBuilder original) throws IOException { + MultiPolygonBuilder mutation = (MultiPolygonBuilder) copyShape(original); if (randomBoolean()) { // toggle orientation mutation.orientation = (original.orientation == Orientation.LEFT ? Orientation.RIGHT : Orientation.LEFT); @@ -55,4 +48,15 @@ public class MultiPolygonBuilderTests extends AbstractShapeBuilderTestCase { +import java.io.IOException; - final static PointBuilderTests PROTOTYPE = new PointBuilderTests(); +public class PointBuilderTests extends AbstractShapeBuilderTestCase { @Override protected PointBuilder createTestShapeBuilder() { - return (PointBuilder) RandomShapeGenerator.createShape(getRandom(), ShapeType.POINT); + return createRandomShape(); } @Override - protected PointBuilder mutate(PointBuilder original) { + protected PointBuilder createMutation(PointBuilder original) throws IOException { + return mutate(original); + } + + static PointBuilder mutate(PointBuilder original) { return new PointBuilder().coordinate(new Coordinate(original.longitude() / 2, original.latitude() / 2)); } + + static PointBuilder createRandomShape() { + return (PointBuilder) RandomShapeGenerator.createShape(getRandom(), ShapeType.POINT); + } + + } diff --git a/core/src/test/java/org/elasticsearch/common/geo/builders/PolygonBuilderTests.java b/core/src/test/java/org/elasticsearch/common/geo/builders/PolygonBuilderTests.java index b7cbf85a601..69457419727 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/builders/PolygonBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/common/geo/builders/PolygonBuilderTests.java @@ -29,20 +29,18 @@ import java.io.IOException; public class PolygonBuilderTests extends AbstractShapeBuilderTestCase { - static final PolygonBuilderTests PROTOTYPE = new PolygonBuilderTests(); - @Override protected PolygonBuilder createTestShapeBuilder() { - PolygonBuilder pgb = (PolygonBuilder) RandomShapeGenerator.createShape(getRandom(), ShapeType.POLYGON); - pgb.orientation = randomFrom(Orientation.values()); - // NORELEASE translated might have been changed by createShape, but won't survive xContent->Parse roundtrip - pgb.shell().translated(false); - return pgb; + return createRandomShape(); } @Override - protected PolygonBuilder mutate(PolygonBuilder original) throws IOException { - PolygonBuilder mutation = copyShape(original); + protected PolygonBuilder createMutation(PolygonBuilder original) throws IOException { + return mutate(original); + } + + static PolygonBuilder mutate(PolygonBuilder original) throws IOException { + PolygonBuilder mutation = (PolygonBuilder) copyShape(original); return mutatePolygonBuilder(mutation); } @@ -75,4 +73,10 @@ public class PolygonBuilderTests extends AbstractShapeBuilderTestCase