Updating translation gate check to disregard order of hole vertices for non dateline crossing polys.

Updating comments and code readability

Correcting code formatting

src/main/java/org/elasticsearch/common/geo/builders/BasePolygonBuilder.java

@@ -403,9 +403,10 @@ public abstract class BasePolygonBuilder<E extends BasePolygonBuilder<E>> extend
             // The connect method creates a new edge for these paired edges in the linked list. 
             // For boundary conditions (e.g., intersect but not crossing) there is no sibling edge 
             // to connect. Thus the first logic check enforces the pairwise rule
-            // 2. the second logic ensures the two candidate edges aren't already connected by an
-            //    existing along the dateline - this is necessary due to a logic change that
-            //    computes dateline edges as valid intersect points in support of OGC standards
+            // 2. the second logic check ensures the two candidate edges aren't already connected by an
+            //    existing edge along the dateline - this is necessary due to a logic change in
+            //    ShapeBuilder.intersection that computes dateline edges as valid intersect points 
+            //    in support of OGC standards
             if (e1.intersect != Edge.MAX_COORDINATE && e2.intersect != Edge.MAX_COORDINATE 
                     && !(e1.next.next.coordinate.equals3D(e2.coordinate) && Math.abs(e1.next.coordinate.x) == DATELINE
                     && Math.abs(e2.coordinate.x) == DATELINE) ) {

src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java

@@ -486,7 +486,7 @@ public abstract class ShapeBuilder implements ToXContent {
          * @return Array of edges
          */
         protected static Edge[] ring(int component, boolean direction, BaseLineStringBuilder<?> shell, Coordinate[] points, int offset, 
-                                     Edge[] edges, int toffset, int length) {
+                Edge[] edges, int toffset, int length) {
             // calculate the direction of the points:
             // find the point a the top of the set and check its
             // neighbors orientation. So direction is equivalent
@@ -505,11 +505,11 @@ public abstract class ShapeBuilder implements ToXContent {
             Pair<Pair, Pair> range = range(points, offset, length);
             final double rng = (Double)range.getLeft().getRight() - (Double)range.getLeft().getLeft();
             // translate the points if the following is true
-            //   1.  range is greater than a hemisphere (180 degrees) but not spanning 2 hemispheres (translation would result in
-            //         a collapsed poly)
+            //   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)
-            if ((rng > DATELINE && rng != 2*DATELINE && orientation) || (shell.translated && component != 0)) {
-                transform(points);
+            if ((rng > DATELINE && rng != 2*DATELINE && orientation && component == 0) || (shell.translated && component != 0)) {
+                translate(points);
                 // flip the translation bit if the shell is being translated
                 if (component == 0 && !shell.translated) {
                     shell.translated = true;
@@ -526,7 +526,7 @@ public abstract class ShapeBuilder implements ToXContent {
          * Transforms coordinates in the eastern hemisphere (-180:0) to a (180:360) range 
          * @param points
          */
-        protected static void transform(Coordinate[] points) {
+        protected static void translate(Coordinate[] points) {
             for (Coordinate c : points) {
                 if (c.x < 0) {
                     c.x += 2*DATELINE;

src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java

@@ -210,8 +210,8 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
     }
 
     @Test
-    public void testParse_polygonNoHolesOGCCompliance() throws IOException {
-        // test ccw poly (large poly intended to not cross dateline)
+    public void testParse_OGCPolygonWithoutHoles() throws IOException {
+        // test ccw poly not crossing dateline
         String polygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Polygon")
                 .startArray("coordinates")
                 .startArray()
@@ -228,8 +228,10 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
         XContentParser parser = JsonXContent.jsonXContent.createParser(polygonGeoJson);
         parser.nextToken();
         Shape shape = ShapeBuilder.parse(parser).build();
+        
+        ElasticsearchGeoAssertions.assertPolygon(shape);
 
-        // test cw poly (small poly expected to cross dateline)
+        // test ccw poly crossing dateline
         polygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Polygon")
                 .startArray("coordinates")
                 .startArray()
@@ -246,13 +248,65 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
         parser = JsonXContent.jsonXContent.createParser(polygonGeoJson);
         parser.nextToken();
         shape = ShapeBuilder.parse(parser).build();
+        
+        ElasticsearchGeoAssertions.assertMultiPolygon(shape);
     }
 
-//    @Test
-//    public void testParse_polygonWithHolesWKTReorder() throws IOException {
-//
-//    }
+    @Test
+    public void testParse_OGCPolygonWithHoles() throws IOException {
+        // test ccw poly not crossing dateline
+        String polygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Polygon")
+                .startArray("coordinates")
+                .startArray()
+                .startArray().value(176.0).value(15.0).endArray()
+                .startArray().value(-177.0).value(10.0).endArray()
+                .startArray().value(-177.0).value(-10.0).endArray()
+                .startArray().value(176.0).value(-15.0).endArray()
+                .startArray().value(172.0).value(0.0).endArray()
+                .startArray().value(176.0).value(15.0).endArray()
+                .endArray()
+                .startArray()
+                .startArray().value(-172.0).value(8.0).endArray()
+                .startArray().value(174.0).value(10.0).endArray()
+                .startArray().value(-172.0).value(-8.0).endArray()
+                .startArray().value(-172.0).value(8.0).endArray()
+                .endArray()
+                .endArray()
+                .endObject().string();
+
+        XContentParser parser = JsonXContent.jsonXContent.createParser(polygonGeoJson);
+        parser.nextToken();
+        Shape shape = ShapeBuilder.parse(parser).build();
+
+        ElasticsearchGeoAssertions.assertPolygon(shape);
 
+        // test ccw poly crossing dateline
+        polygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "Polygon")
+                .startArray("coordinates")
+                .startArray()
+                .startArray().value(-177.0).value(10.0).endArray()
+                .startArray().value(176.0).value(15.0).endArray()
+                .startArray().value(172.0).value(0.0).endArray()
+                .startArray().value(176.0).value(-15.0).endArray()
+                .startArray().value(-177.0).value(-10.0).endArray()
+                .startArray().value(-177.0).value(10.0).endArray()
+                .endArray()
+                .startArray()
+                .startArray().value(178.0).value(8.0).endArray()
+                .startArray().value(-178.0).value(8.0).endArray()
+                .startArray().value(-180.0).value(-8.0).endArray()
+                .startArray().value(178.0).value(8.0).endArray()
+                .endArray()
+                .endArray()
+                .endObject().string();
+
+        parser = JsonXContent.jsonXContent.createParser(polygonGeoJson);
+        parser.nextToken();
+        shape = ShapeBuilder.parse(parser).build();
+
+        ElasticsearchGeoAssertions.assertMultiPolygon(shape);
+    }
+    
     @Test
     public void testParse_invalidPolygon() throws IOException {
         /**

src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java

@@ -246,35 +246,35 @@ public class ShapeBuilderTests extends ElasticsearchTestCase {
 
         // a giant c shape
         PolygonBuilder builder = ShapeBuilder.newPolygon()
-                .point(174,0)
-                .point(-176,0)
-                .point(-176,3)
-                .point(177,3)
-                .point(177,5)
-                .point(-176,5)
-                .point(-176,8)
-                .point(174,8)
-                .point(174,0);
+            .point(174,0)
+            .point(-176,0)
+            .point(-176,3)
+            .point(177,3)
+            .point(177,5)
+            .point(-176,5)
+            .point(-176,8)
+            .point(174,8)
+            .point(174,0);
 
         // 3/4 of an embedded 'c', crossing dateline once
         builder.hole()
-                .point(175, 1)
-                .point(175, 7)
-                .point(-178, 7)
-                .point(-178, 6)
-                .point(176, 6)
-                .point(176, 2)
-                .point(179, 2)
-                .point(179,1)
-                .point(175, 1);
+            .point(175, 1)
+            .point(175, 7)
+            .point(-178, 7)
+            .point(-178, 6)
+            .point(176, 6)
+            .point(176, 2)
+            .point(179, 2)
+            .point(179,1)
+            .point(175, 1);
 
         // embedded hole right of the dateline
         builder.hole()
-                .point(-179, 1)
-                .point(-179, 2)
-                .point(-177, 2)
-                .point(-177,1)
-                .point(-179,1);
+            .point(-179, 1)
+            .point(-179, 2)
+            .point(-177, 2)
+            .point(-177,1)
+            .point(-179,1);
 
         Shape shape = builder.close().build();