Merge pull request #5773 from ibi-group/swap-lat-lon

Correct order for x,y when constructing JTS coordinates in tests
opentripplanner · Apr 2, 2024 · 827d5c0 · 827d5c0
2 parents 8d15a0d + 258398d
commit 827d5c0
Show file tree

Hide file tree

Showing 9 changed files with 113 additions and 90 deletions.
diff --git a/src/ext-test/java/org/opentripplanner/ext/flex/AreaStopsToVerticesMapperTest.java b/src/ext-test/java/org/opentripplanner/ext/flex/AreaStopsToVerticesMapperTest.java
@@ -2,7 +2,6 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.DynamicTest.dynamicTest;
 import static org.opentripplanner._support.geometry.Coordinates.BERLIN;
 import static org.opentripplanner._support.geometry.Coordinates.HAMBURG;
 import static org.opentripplanner.street.model.StreetTraversalPermission.ALL;
@@ -13,9 +12,8 @@
 
 import java.util.List;
 import java.util.Set;
-import java.util.stream.Stream;
-import org.junit.jupiter.api.DynamicTest;
-import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.locationtech.jts.geom.Coordinate;
 import org.opentripplanner._support.geometry.Polygons;
 import org.opentripplanner.routing.graph.Graph;
@@ -42,44 +40,38 @@ class AreaStopsToVerticesMapperTest {
 
   public static final TransitModel TRANSIT_MODEL = new TransitModel(STOP_MODEL, new Deduplicator());
 
-  private final List<TestCase> testCases = List.of(
-    new TestCase(BERLIN, ALL, Set.of(BERLIN_AREA_STOP)),
-    new TestCase(BERLIN, PEDESTRIAN_AND_CAR, Set.of(BERLIN_AREA_STOP)),
-    new TestCase(BERLIN, BICYCLE_AND_CAR, Set.of()),
-    new TestCase(HAMBURG, ALL, Set.of()),
-    new TestCase(BERLIN, PEDESTRIAN, Set.of()),
-    new TestCase(HAMBURG, PEDESTRIAN, Set.of()),
-    new TestCase(BERLIN, CAR, Set.of())
-  );
+  static List<TestCase> testCases() {
+    return List.of(
+      new TestCase(BERLIN, ALL, Set.of(BERLIN_AREA_STOP)),
+      new TestCase(BERLIN, PEDESTRIAN_AND_CAR, Set.of(BERLIN_AREA_STOP)),
+      new TestCase(BERLIN, BICYCLE_AND_CAR, Set.of()),
+      new TestCase(HAMBURG, ALL, Set.of()),
+      new TestCase(BERLIN, PEDESTRIAN, Set.of()),
+      new TestCase(HAMBURG, PEDESTRIAN, Set.of()),
+      new TestCase(BERLIN, CAR, Set.of())
+    );
+  }
 
-  @TestFactory
-  Stream<DynamicTest> mapAreaStopsInVertex() {
-    return testCases
-      .stream()
-      .map(tc ->
-        dynamicTest(
-          tc.toString(),
-          () -> {
-            var graph = new Graph();
+  @ParameterizedTest
+  @MethodSource("testCases")
+  void mapAreaStopsInVertex(TestCase tc) {
+    var graph = new Graph();
 
-            var fromVertex = StreetModelForTest.intersectionVertex(tc.coordinate);
-            var toVertex = StreetModelForTest.intersectionVertex(tc.coordinate);
+    var fromVertex = StreetModelForTest.intersectionVertex(tc.coordinate);
+    var toVertex = StreetModelForTest.intersectionVertex(tc.coordinate);
 
-            var edge = StreetModelForTest.streetEdge(fromVertex, toVertex);
-            edge.setPermission(tc.permission);
-            fromVertex.addOutgoing(edge);
+    var edge = StreetModelForTest.streetEdge(fromVertex, toVertex);
+    edge.setPermission(tc.permission);
+    fromVertex.addOutgoing(edge);
 
-            graph.addVertex(fromVertex);
-            assertTrue(fromVertex.areaStops().isEmpty());
+    graph.addVertex(fromVertex);
+    assertTrue(fromVertex.areaStops().isEmpty());
 
-            var mapper = new AreaStopsToVerticesMapper(graph, TRANSIT_MODEL);
+    var mapper = new AreaStopsToVerticesMapper(graph, TRANSIT_MODEL);
 
-            mapper.buildGraph();
+    mapper.buildGraph();
 
-            assertEquals(tc.expectedAreaStops, fromVertex.areaStops());
-          }
-        )
-      );
+    assertEquals(tc.expectedAreaStops, fromVertex.areaStops());
   }
 
   private record TestCase(

diff --git a/src/test/java/org/opentripplanner/_support/geometry/Coordinates.java b/src/test/java/org/opentripplanner/_support/geometry/Coordinates.java
@@ -4,9 +4,18 @@
 
 public class Coordinates {
 
-  public static final Coordinate BERLIN = new Coordinate(52.5212, 13.4105);
-  public static final Coordinate BERLIN_BRANDENBURG_GATE = new Coordinate(52.51627, 13.37770);
-  public static final Coordinate HAMBURG = new Coordinate(53.5566, 10.0003);
-  public static final Coordinate KONGSBERG_PLATFORM_1 = new Coordinate(59.67216, 9.65107);
-  public static final Coordinate BOSTON = new Coordinate(42.36541, -71.06129);
+  public static final Coordinate BERLIN = of(52.5212, 13.4105);
+  public static final Coordinate BERLIN_BRANDENBURG_GATE = of(52.51627, 13.37770);
+  public static final Coordinate HAMBURG = of(53.5566, 10.0003);
+  public static final Coordinate KONGSBERG_PLATFORM_1 = of(59.67216, 9.65107);
+  public static final Coordinate BOSTON = of(42.36541, -71.06129);
+
+  /**
+   * Because it is a frequent mistake to swap x/y and longitude/latitude when
+   * constructing JTS Coordinates, this static factory method makes is clear
+   * which is which.
+   */
+  public static Coordinate of(double latitude, double longitude) {
+    return new Coordinate(longitude, latitude);
+  }
 }
diff --git a/src/test/java/org/opentripplanner/_support/geometry/Polygons.java b/src/test/java/org/opentripplanner/_support/geometry/Polygons.java
@@ -3,29 +3,65 @@
 import java.util.Arrays;
 import org.geojson.LngLatAlt;
 import org.locationtech.jts.geom.Coordinate;
+import org.locationtech.jts.geom.GeometryFactory;
 import org.locationtech.jts.geom.Polygon;
 import org.opentripplanner.framework.geometry.GeometryUtils;
 
 public class Polygons {
 
-  public static final Polygon BERLIN = GeometryUtils
-    .getGeometryFactory()
-    .createPolygon(
-      new Coordinate[] {
-        new Coordinate(52.616841, 13.224692810),
-        new Coordinate(52.6168419, 13.224692810),
-        new Coordinate(52.3915238, 13.646107734),
-        new Coordinate(52.616841, 13.646107734),
-        new Coordinate(52.616841, 13.224692810),
-      }
-    );
+  private static final GeometryFactory FAC = GeometryUtils.getGeometryFactory();
+  public static final Polygon BERLIN = FAC.createPolygon(
+    new Coordinate[] {
+      Coordinates.of(52.616841, 13.224692810),
+      Coordinates.of(52.616841, 13.646107734),
+      Coordinates.of(52.3915238, 13.646107734),
+      Coordinates.of(52.396421, 13.2268067),
+      Coordinates.of(52.616841, 13.224692810),
+    }
+  );
+
+  public static Polygon OSLO = FAC.createPolygon(
+    new Coordinate[] {
+      Coordinates.of(59.961055202323195, 10.62535658370308),
+      Coordinates.of(59.889009435700416, 10.62535658370308),
+      Coordinates.of(59.889009435700416, 10.849791142928694),
+      Coordinates.of(59.961055202323195, 10.849791142928694),
+      Coordinates.of(59.961055202323195, 10.62535658370308),
+    }
+  );
+  public static Polygon OSLO_FROGNER_PARK = FAC.createPolygon(
+    new Coordinate[] {
+      Coordinates.of(59.92939032560119, 10.69770054003061),
+      Coordinates.of(59.929138466684975, 10.695210909925208),
+      Coordinates.of(59.92745319808358, 10.692696865071184),
+      Coordinates.of(59.92709930323093, 10.693774304996225),
+      Coordinates.of(59.92745914988427, 10.69495957972947),
+      Coordinates.of(59.92736919590291, 10.697664535925895),
+      Coordinates.of(59.924837887427856, 10.697927604125255),
+      Coordinates.of(59.924447953413335, 10.697448767354985),
+      Coordinates.of(59.92378800804022, 10.697819761729818),
+      Coordinates.of(59.92329018587293, 10.699196446969069),
+      Coordinates.of(59.92347619027632, 10.700285749621997),
+      Coordinates.of(59.92272030268688, 10.704714696822037),
+      Coordinates.of(59.92597766029715, 10.71001707489603),
+      Coordinates.of(59.92676341291536, 10.707838597058043),
+      Coordinates.of(59.92790300889098, 10.708389137506913),
+      Coordinates.of(59.928376832499424, 10.707060536853078),
+      Coordinates.of(59.92831087551576, 10.705803789539402),
+      Coordinates.of(59.92953431964068, 10.706641515204467),
+      Coordinates.of(59.93046383654274, 10.70484606360543),
+      Coordinates.of(59.93008590667682, 10.701817874860211),
+      Coordinates.of(59.93028982601595, 10.700525251174469),
+      Coordinates.of(59.92939032560119, 10.69770054003061),
+    }
+  );
 
   public static org.geojson.Polygon toGeoJson(Polygon polygon) {
     var ret = new org.geojson.Polygon();
 
     var coordinates = Arrays
       .stream(polygon.getCoordinates())
-      .map(c -> new LngLatAlt(c.y, c.x))
+      .map(c -> new LngLatAlt(c.x, c.y))
       .toList();
     ret.add(coordinates);
 

diff --git a/src/test/java/org/opentripplanner/framework/geometry/WgsCoordinateTest.java b/src/test/java/org/opentripplanner/framework/geometry/WgsCoordinateTest.java
@@ -109,15 +109,15 @@ void testGreenwich() {
   void roundingTo10m() {
     var hamburg = new WgsCoordinate(Coordinates.HAMBURG);
     var rounded = hamburg.roundToApproximate10m();
-    assertEquals(10.0003, rounded.latitude());
-    assertEquals(53.5566, rounded.longitude());
+    assertEquals(10.0003, rounded.longitude());
+    assertEquals(53.5566, rounded.latitude());
   }
 
   @Test
   void roundingTo100m() {
     var hamburg = new WgsCoordinate(Coordinates.HAMBURG);
     var rounded = hamburg.roundToApproximate100m();
-    assertEquals(10, rounded.latitude());
-    assertEquals(53.557, rounded.longitude());
+    assertEquals(10, rounded.longitude());
+    assertEquals(53.557, rounded.latitude());
   }
 }
diff --git a/src/test/java/org/opentripplanner/graph_builder/module/StreetLinkerModuleTest.java b/src/test/java/org/opentripplanner/graph_builder/module/StreetLinkerModuleTest.java
@@ -102,12 +102,12 @@ private static class TestModel {
 
     public TestModel() {
       var from = StreetModelForTest.intersectionVertex(
-        KONGSBERG_PLATFORM_1.x - DELTA,
-        KONGSBERG_PLATFORM_1.y - DELTA
+        KONGSBERG_PLATFORM_1.y - DELTA,
+        KONGSBERG_PLATFORM_1.x - DELTA
       );
       var to = StreetModelForTest.intersectionVertex(
-        KONGSBERG_PLATFORM_1.x + DELTA,
-        KONGSBERG_PLATFORM_1.y + DELTA
+        KONGSBERG_PLATFORM_1.y + DELTA,
+        KONGSBERG_PLATFORM_1.x + DELTA
       );
 
       Graph graph = new Graph();

diff --git a/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java b/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java
@@ -39,8 +39,8 @@ public class TemporaryVerticesContainerTest {
   private final Graph g = new Graph(new Deduplicator());
 
   private final StreetVertex a = StreetModelForTest.intersectionVertex("A", 1.0, 1.0);
-  private final StreetVertex b = StreetModelForTest.intersectionVertex("B", 0.0, 1.0);
-  private final StreetVertex c = StreetModelForTest.intersectionVertex("C", 1.0, 0.0);
+  private final StreetVertex b = StreetModelForTest.intersectionVertex("B", 1.0, 0.0);
+  private final StreetVertex c = StreetModelForTest.intersectionVertex("C", 0.0, 1.0);
   private final List<Vertex> permanentVertexes = Arrays.asList(a, b, c);
   // - And travel *origin* is 0,4 degrees on the road from B to A
   private final GenericLocation from = new GenericLocation(1.0, 0.4);

diff --git a/src/test/java/org/opentripplanner/street/model/_data/StreetModelForTest.java b/src/test/java/org/opentripplanner/street/model/_data/StreetModelForTest.java
@@ -29,16 +29,16 @@ public class StreetModelForTest {
   public static StreetVertex V4 = intersectionVertex("V4", 3, 3);
 
   public static IntersectionVertex intersectionVertex(Coordinate c) {
-    return intersectionVertex(c.x, c.y);
+    return intersectionVertex(c.y, c.x);
   }
 
   public static IntersectionVertex intersectionVertex(double lat, double lon) {
     var label = "%s_%s".formatted(lat, lon);
-    return new LabelledIntersectionVertex(label, lat, lon, false, false);
+    return new LabelledIntersectionVertex(label, lon, lat, false, false);
   }
 
   public static IntersectionVertex intersectionVertex(String label, double lat, double lon) {
-    return new LabelledIntersectionVertex(label, lat, lon, false, false);
+    return new LabelledIntersectionVertex(label, lon, lat, false, false);
   }
 
   @Nonnull

diff --git a/src/test/java/org/opentripplanner/street/model/edge/StreetEdgeTest.java b/src/test/java/org/opentripplanner/street/model/edge/StreetEdgeTest.java
@@ -46,7 +46,7 @@ public class StreetEdgeTest {
   public void before() {
     v0 = intersectionVertex("maple_0th", 0.0, 0.0); // label, X, Y
     v1 = intersectionVertex("maple_1st", 2.0, 2.0);
-    v2 = intersectionVertex("maple_2nd", 1.0, 2.0);
+    v2 = intersectionVertex("maple_2nd", 2.0, 1.0);
 
     this.proto =
       StreetSearchRequest
@@ -73,7 +73,7 @@ public void testInAndOutAngles() {
     assertEquals(90, e1.getOutAngle());
 
     // 2 new ones
-    StreetVertex u = intersectionVertex("test1", 2.0, 1.0);
+    StreetVertex u = intersectionVertex("test1", 1.0, 2.0);
     StreetVertex v = intersectionVertex("test2", 2.0, 2.0);
 
     // Second edge, heading straight North

diff --git a/src/test/java/org/opentripplanner/updater/vehicle_rental/GeofencingVertexUpdaterTest.java b/src/test/java/org/opentripplanner/updater/vehicle_rental/GeofencingVertexUpdaterTest.java
@@ -8,10 +8,10 @@
 
 import java.util.List;
 import org.junit.jupiter.api.Test;
-import org.locationtech.jts.geom.Coordinate;
 import org.locationtech.jts.geom.GeometryFactory;
 import org.locationtech.jts.geom.MultiPolygon;
 import org.locationtech.jts.geom.Polygon;
+import org.opentripplanner._support.geometry.Polygons;
 import org.opentripplanner.framework.geometry.GeometryUtils;
 import org.opentripplanner.service.vehiclerental.model.GeofencingZone;
 import org.opentripplanner.service.vehiclerental.street.BusinessAreaBorder;
@@ -35,30 +35,16 @@ class GeofencingVertexUpdaterTest {
   final GeofencingVertexUpdater updater = new GeofencingVertexUpdater(ignored ->
     List.of(insideFrognerPark, halfInHalfOutFrognerPark, businessBorder)
   );
-  StreetEdge outsideFrognerPark = streetEdge(outsideFrognerPark1, outsideFrognerPark2);
-
-  GeometryFactory fac = GeometryUtils.getGeometryFactory();
-  Polygon frognerPark = fac.createPolygon(
-    new Coordinate[] {
-      new Coordinate(59.93112978539807, 10.691099320272173),
-      new Coordinate(59.92231848097069, 10.691099320272173),
-      new Coordinate(59.92231848097069, 10.711758464910503),
-      new Coordinate(59.92231848097069, 10.691099320272173),
-      new Coordinate(59.93112978539807, 10.691099320272173),
-    }
-  );
-  final GeofencingZone zone = new GeofencingZone(id("frogner-park"), frognerPark, true, false);
-  Polygon osloPolygon = fac.createPolygon(
-    new Coordinate[] {
-      new Coordinate(59.961055202323195, 10.62535658370308),
-      new Coordinate(59.889009435700416, 10.62535658370308),
-      new Coordinate(59.889009435700416, 10.849791142928694),
-      new Coordinate(59.961055202323195, 10.849791142928694),
-      new Coordinate(59.961055202323195, 10.62535658370308),
-    }
+
+  static GeometryFactory fac = GeometryUtils.getGeometryFactory();
+  final GeofencingZone zone = new GeofencingZone(
+    id("frogner-park"),
+    Polygons.OSLO_FROGNER_PARK,
+    true,
+    false
   );
 
-  MultiPolygon osloMultiPolygon = fac.createMultiPolygon(new Polygon[] { osloPolygon });
+  MultiPolygon osloMultiPolygon = fac.createMultiPolygon(new Polygon[] { Polygons.OSLO });
   final GeofencingZone businessArea = new GeofencingZone(
     id("oslo"),
     osloMultiPolygon,