absurdfarce commented on code in PR #2052:
URL:
https://github.com/apache/cassandra-java-driver/pull/2052#discussion_r2292895502
##########
core/src/test/java/com/datastax/dse/driver/internal/core/data/geometry/DefaultPolygonTest.java:
##########
@@ -109,8 +112,44 @@ public void should_parse_valid_geo_json() {
}
@Test
- public void should_convert_to_geo_json() {
- assertThat(polygon.asGeoJson()).isEqualTo(json);
+ public void should_convert_to_geo_json() throws Exception {
+
+ ObjectMapper mapper = new ObjectMapper();
+ JsonNode root = mapper.readTree(polygon.asGeoJson());
+ assertThat(root.get("type").toString()).isEqualTo("\"Polygon\"");
+
+ /*
+ Note that the order of values in expected differs from the order of
insertion when creating
+ the Polygon. OGC expects the "exterior" ring of the polygon to be listed
in counter-clockwise
+ order... and that's what this sequence represents (draw it out on a graph
if you don't believe me).
+
+ Weirdly this is the opposite of the order used for this test when we were
using ESRI 1.2.1.
+ That fact (combined with the fact that only ESRI classes are used for
serialization here) makes me
+ think that the earlier version was just doing it wrong... or at least
doing it in a way that
+ didn't agree with the spec. Either way it is clearly correct that we
should go counter-clockwise...
+ so that's what we're doing.
+ */
Review Comment:
Part of the explanation here might be that GeoJSON appears to be somewhat
looser in ordering the points in a path. There's actually an old ticket for
DSE asking for GeoJSON encoding to preserve the OGC ordering even though it
isn't required to do so. That ticket was never implemented... which presumably
explains why this test looked the way it did (although I'm speculating a bit
there).
Either way it is the case that this test (a) uses only ESRI classes locally
and (b) is serializing and deserializing data internally (i.e. no interaction
with anything outside of ESRI is happening here). It appears that the GeoJSON
impl in the new version of esri-geometry-api is more strict about ordering of
points... which presumably explains why we've started seeing failures since the
upgrade.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
