CASSJAVA-108 Update org.json (and very likely ESRI) dependency #2052
+85
−20
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ade to esri-geometry-api 2.x
…CGeometry) in favor of equals(Object) (which was also used when we were using 1.2.1)
absurdfarce
commented
Aug 22, 2025
| @@ -176,7 +176,7 @@ public boolean equals(Object o) { | |||
| return false; | |||
| } | |||
| DefaultGeometry that = (DefaultGeometry) o; | |||
| return this.getOgcGeometry().equals(that.getOgcGeometry()); | |||
| return this.getOgcGeometry().equals((Object) that.getOgcGeometry()); | |||
There was a problem hiding this comment.
ESRI 1.2.1 used to test equality by a literal comparison across fields of OGCGeometry types. As of 2.2.4 this was changed to two distinct comparisons: equals(Object) (the standard Java equality op) and equals(OGCGeometry). The later was subsequently deprecated and moved to an Equals(OGCGeometry) method. The code above was calling the (now deprecated) method.
More info can be found on a thread starting from the relevant esri-geometry-api PR
There was a problem hiding this comment.
Note: without this change you get failures like the following:
Per-Commit / Matrix - SERVER_VERSION = 'dse-6.8.30', JABBA_VERSION = '[email protected]' / Execute-Tests / com.datastax.dse.driver.api.core.data.geometry.PointIT.should_insert_as_map_keys
...
Error Message
Multiple entries with same key: POINT (1.7976931348623157e+308 4.9000000000000000e-324)=4 and POINT (0 0)=1
Stacktrace
java.lang.IllegalArgumentException: Multiple entries with same key: POINT (1.7976931348623157e+308 4.9000000000000000e-324)=4 and POINT (0 0)=1
at com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap.conflictException(ImmutableMap.java:382)
at com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap.checkNoConflict(ImmutableMap.java:376)
at com.datastax.oss.driver.shaded.guava.common.collect.RegularImmutableMap.checkNoConflictInKeyBucket(RegularImmutableMap.java:249)
at com.datastax.oss.driver.shaded.guava.common.collect.RegularImmutableMap.fromEntryArrayCheckingBucketOverflow(RegularImmutableMap.java:136)
at com.datastax.oss.driver.shaded.guava.common.collect.RegularImmutableMap.fromEntryArray(RegularImmutableMap.java:98)
at com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap$Builder.build(ImmutableMap.java:579)
at com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap$Builder.buildOrThrow(ImmutableMap.java:607)
at com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap$Builder.build(ImmutableMap.java:594)
at com.datastax.dse.driver.api.core.data.geometry.GeometryIT.should_insert_as_map_keys(GeometryIT.java:236)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
at org.apache.maven.surefire.junitcore.pc.Scheduler$1.run(Scheduler.java:345)
at org.apache.maven.surefire.junitcore.pc.InvokerStrategy.schedule(InvokerStrategy.java:47)
at org.apache.maven.surefire.junitcore.pc.Scheduler.schedule(Scheduler.java:316)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:54)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:54)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
at org.junit.runners.Suite.runChild(Suite.java:128)
at org.junit.runners.Suite.runChild(Suite.java:27)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
at org.apache.maven.surefire.junitcore.pc.Scheduler$1.run(Scheduler.java:345)
at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:829)
absurdfarce
commented
Aug 22, 2025
| assertThat(elemArray.size()).isEqualTo(2); | ||
| assertThat(elemArray.get(0).asDouble()).isEqualTo(expected[i][0]); | ||
| assertThat(elemArray.get(1).asDouble()).isEqualTo(expected[i][1]); | ||
| } |
There was a problem hiding this comment.
Perhaps the most contentious change in the whole PR. With this code we no longer concern ourselves with the format of the GeoJSON returned by ESRI... we only seek to confirm that we can find the fields we expect at the places we expect them. Any additional content or formatting isn't our concern.
My rationale is that this is really the heart of the matter. We don't claim to follow a specific GeoJSON spec or that any responses will contain only some properties but not any others. With that in mind our objective should be to make sure we don't see any regressions on the props customers actually expect, specifically the "type" and "coordinates" fields.
There was a problem hiding this comment.
Note: without this change you get failures like the following:
Per-Commit / Matrix - SERVER_VERSION = 'dse-6.9.0', JABBA_VERSION = '[email protected]' / Execute-Tests / com.datastax.dse.driver.internal.core.data.geometry.DefaultLineStringTest.should_convert_to_geo_json
...
Error Message
expected:<...","coordinates":[[30[.0,10.0],[10.0,30.0],[40.0,40.0]]]}"> but was:<...","coordinates":[[30[,10],[10,30],[40,40]],"crs":{"type":"name","properties":{"name":"EPSG:4326"}}]}">
Stacktrace
org.junit.ComparisonFailure: expected:<...","coordinates":[[30[.0,10.0],[10.0,30.0],[40.0,40.0]]]}"> but was:<...","coordinates":[[30[,10],[10,30],[40,40]],"crs":{"type":"name","properties":{"name":"EPSG:4326"}}]}">
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at com.datastax.dse.driver.internal.core.data.geometry.DefaultLineStringTest.should_convert_to_geo_json(DefaultLineStringTest.java:105)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
at org.junit.runners.Suite.runChild(Suite.java:128)
at org.junit.runners.Suite.runChild(Suite.java:27)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
at org.apache.maven.surefire.junitcore.JUnitCore.run(JUnitCore.java:49)
at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.createRequestAndRun(JUnitCoreWrapper.java:120)
at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.executeEager(JUnitCoreWrapper.java:95)
at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.execute(JUnitCoreWrapper.java:75)
at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.execute(JUnitCoreWrapper.java:69)
at org.apache.maven.surefire.junitcore.JUnitCoreProvider.invoke(JUnitCoreProvider.java:146)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)
absurdfarce
commented
Aug 22, 2025
| 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. | ||
| */ |
There was a problem hiding this 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update ESRI + json versions to match what's in current versions of DSE