Changeset 18675 in josm

Timestamp:

2023-02-22T19:14:00+01:00 (14 months ago)

Author:

taylor.smock

Message:

Fix #22684: IAE: Parameter 'en1' must not be null in PowerLines.addWaterWaySegments (patch by gaben, modified)

PowerLines.addWaterWaySegments now only adds segments where all nodes have
position data.

This additionally fixes a similar issue caused by inspecting an incomplete
way in InspectPrimitiveDataText. In this case, the centroid is still
calculated, but does not account for how nodes without positional data
will affect the centroid.

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/validation/tests/PowerLines.java (modified) (1 diff)
• src/org/openstreetmap/josm/tools/Geometry.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/data/validation/tests/PowerLinesTest.java (modified) (4 diffs)
• test/unit/org/openstreetmap/josm/tools/GeometryTest.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/data/validation/tests/PowerLines.java

@@ -131 +131 @@
         for (int i = 0; i < w.getNodesCount() - 1; i++) {
             final WaySegment es1 = new WaySegment(w, i);
-            CrossingWays.getSegments(this.cellSegmentsWater, es1.getFirstNode(), es1.getSecondNode()).forEach(list -> list.add(es1));
+            final Node first = es1.getFirstNode();
+            final Node second = es1.getSecondNode();
+
+            if (first.isLatLonKnown() && second.isLatLonKnown()) {
+                CrossingWays.getSegments(this.cellSegmentsWater, first, second).forEach(list -> list.add(es1));
+            }
         }
     }

trunk/src/org/openstreetmap/josm/tools/Geometry.java

@@ -939 +939 @@
      */
     public static EastNorth getCentroid(List<? extends INode> nodes) {
-        return getCentroidEN(nodes.stream().map(INode::getEastNorth).collect(Collectors.toList()));
+        return getCentroidEN(nodes.stream().filter(INode::isLatLonKnown).map(INode::getEastNorth).collect(Collectors.toList()));
     }
 
@@ -955 +955 @@
         } else if (size == 2) {
             return nodes.get(0).getCenter(nodes.get(1));
+        } else if (size == 0) {
+            return null;
         }
 

trunk/test/unit/org/openstreetmap/josm/data/validation/tests/PowerLinesTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.data.validation.tests;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -7 +8 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
+import org.openstreetmap.josm.TestUtils;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.Relation;
+import org.openstreetmap.josm.data.osm.RelationMember;
 import org.openstreetmap.josm.data.osm.TagMap;
 import org.openstreetmap.josm.data.osm.Way;
-import org.openstreetmap.josm.testutils.JOSMTestRules;
 import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
+import org.openstreetmap.josm.testutils.annotations.Projection;
 
 /**
@@ -21 +24 @@
  */
 @BasicPreferences
+@Projection
 class PowerLinesTest {
     private PowerLines powerLines;
     private DataSet ds;
-
-    @RegisterExtension
-    static JOSMTestRules josmTestRules = new JOSMTestRules().projection();
 
     @BeforeEach
@@ -119 +120 @@
         assertFalse(powerLines.getErrors().isEmpty());
     }
+
+    /**
+     * Ensure that incomplete relations don't cause problems
+     */
+    @Test
+    void testNonRegression22684() {
+        final Relation powerLine = TestUtils.newRelation("natural=water water=river",
+                new RelationMember("", TestUtils.newWay("", new Node(), new Node())));
+        assertDoesNotThrow(() -> this.powerLines.visit(powerLine));
+    }
 }

trunk/test/unit/org/openstreetmap/josm/tools/GeometryTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.tools;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -18 +20 @@
 import java.util.stream.Stream;
 
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
@@ -39 +39 @@
 import org.openstreetmap.josm.data.projection.Projections;
 import org.openstreetmap.josm.io.OsmReader;
-import org.openstreetmap.josm.testutils.JOSMTestRules;
+import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
 
 /**
  * Unit tests of {@link Geometry} class.
  */
+@BasicPreferences
+@org.openstreetmap.josm.testutils.annotations.Projection
 class GeometryTest {
-    /**
-     * Primitives need preferences and projection.
-     */
-    @RegisterExtension
-    @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    static JOSMTestRules test = new JOSMTestRules().preferences().projection();
-
     /**
      * Test of {@link Geometry#getLineLineIntersection} method.
@@ -578 +573 @@
         assertEquals(angle, Math.toDegrees(original.bearing(actual)), 0.000_001);
     }
+
+    /**
+     * A non-regression test for an issue found during the investigation of #22684 (see comment:3 by GerdP)
+     */
+    @Test
+    void testNonRegression22684() {
+        final EastNorth centroid1 = assertDoesNotThrow(() -> Geometry.getCentroid(Collections.singletonList(new Node())));
+        assertNull(centroid1);
+        final EastNorth centroid2 = assertDoesNotThrow(() -> Geometry.getCentroid(Arrays.asList(new Node(LatLon.ZERO), new Node())));
+        assertTrue(new EastNorth(0, 0).equalsEpsilon(centroid2, 1e-9));
+        final EastNorth centroid3 = assertDoesNotThrow(
+                () -> Geometry.getCentroid(Arrays.asList(new Node(LatLon.ZERO), new Node(), new Node(LatLon.ZERO))));
+        assertTrue(new EastNorth(0, 0).equalsEpsilon(centroid3, 1e-9));
+    }
 }