Ticket #24851: 24851-powerlines-v3.patch

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

@@ -98 +98 @@
 
     @Override
     public void visit(Node n) {
-        boolean nodeInLineOrCable = false;
-        boolean connectedToUnrelated = false;
-        for (Way parent : n.getParentWays()) {
-            if (parent.hasTag(POWER, "line", MINOR_LINE, "cable"))
-                nodeInLineOrCable = true;
-            else if (!isRelatedToPower(parent))
-                connectedToUnrelated = true;
+        if (!n.isConnectionNode() || n.referrers(Way.class).noneMatch(PowerLines::isPowerLineOrCable))
+            return;
+
+        boolean connectedToUnrelated = n.referrers(Way.class).anyMatch(w -> !isPowerLineOrCable(w) && !isRelatedToPower(w));
+        if (connectedToUnrelated) {
+            badConnections.add(n);
         }
-        if (nodeInLineOrCable && connectedToUnrelated)
-            badConnections.add(n);
     }
 
     @Override
@@ -164 +161 @@
         // Then return the errors
         for (Node n : missingTags) {
             if (!isInPowerStation(n)) {
+                List<OsmPrimitive> primitives = new ArrayList<>();
+                primitives.add(n);
+                n.referrers(Way.class).filter(PowerLines::isPowerLine).forEach(primitives::add);
+
                 errors.add(TestError.builder(this, Severity.WARNING, POWER_SUPPORT)
                         // the "missing tag" grouping can become broken if the MapCSS message get reworded
                         .message(tr("missing tag"), tr("node without power=*"))
-                        .primitives(n)
+                        .primitives(primitives)
+                        .highlight(n)
                         .build());
             }
         }
 
         for (Node n : badConnections) {
+            List<OsmPrimitive> primitives = new ArrayList<>();
+            primitives.add(n);
+            n.referrers(Way.class).filter(w -> !isPowerLineOrCable(w) && !isRelatedToPower(w)).forEach(primitives::add);
+            n.referrers(Way.class).filter(PowerLines::isPowerLineOrCable).forEach(primitives::add);
+
             errors.add(TestError.builder(this, Severity.WARNING, POWER_CONNECTION)
                     .message(tr("Node connects a power line or cable with an object "
                             + "which is not related to the power infrastructure"))
-                    .primitives(n)
+                    .primitives(primitives)
+                    .highlight(n)
                     .build());
         }
 
@@ -669 +677 @@
     }
 
     /**
+     * Determines if the specified way denotes a power line or cable.
+     * @param w The way to be tested
+     * @return {@code true} if power key is set and equal to line, minor_line or cable
+     */
+    protected static boolean isPowerLineOrCable(Way w) {
+        return isPowerIn(w, Arrays.asList("line", MINOR_LINE, "cable"));
+    }
+
+    /**
      * Determines if the specified primitive denotes a power station.
      * @param p The primitive to be tested
      * @return {@code true} if power key is set and equal to generator/substation/plant

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

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 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;
-
-import java.util.ArrayList;
-
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.openstreetmap.josm.TestUtils;
@@ -17 +11 @@
 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.data.validation.TestError;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
 import org.openstreetmap.josm.testutils.annotations.Projection;
 
+import java.util.ArrayList;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
 /**
  * Test class for {@link PowerLines}
  * @since 18553
@@ -159 +160 @@
         this.powerLines.endTest();
         assertTrue(this.powerLines.getErrors().isEmpty());
     }
-}
+
+    /**
+     * Test for ticket #24851.
+     * Simulates connecting a power line to an existing highway node without power tags.
+     * Validates that the resulting error contains both the Node AND the Way, so it
+     * doesn't get filtered out during partial validation on upload.
+     */
+    @Test
+    void testTicket24851_ReportExistingNonPowerNodes() {
+        Node sharedNode = new Node(new LatLon(0, 0)); // no power tag attached
+
+        // unrelated highway way
+        Way highway = TestUtils.newWay("highway=unclassified",
+                sharedNode, new Node(new LatLon(0.1, 0)));
+
+        // power line way
+        Way powerline = TestUtils.newWay("power=line",
+                sharedNode, new Node(new LatLon(0, 0.1)));
+
+        // second node has a valid tag
+        powerline.getNode(1).put("power", "tower");
+
+        ds.addPrimitiveRecursive(highway);
+        ds.addPrimitiveRecursive(powerline);
+
+        powerLines.startTest(NullProgressMonitor.INSTANCE);
+        for (Way w : ds.getWays()) {
+            powerLines.visit(w);
+        }
+        for (Node n : ds.getNodes()) {
+            powerLines.visit(n);
+        }
+        powerLines.endTest();
+
+        assertFalse(powerLines.getErrors().isEmpty(), "Errors should be generated for the missing tag and bad connection");
+
+        boolean foundSupportError = false;
+        boolean foundConnectionError = false;
+
+        for (TestError error : powerLines.getErrors()) {
+            // verify POWER_SUPPORT behavior (missing tag)
+            if (error.getCode() == PowerLines.POWER_SUPPORT && error.getPrimitives().contains(sharedNode)) {
+                foundSupportError = true;
+                assertTrue(error.getPrimitives().contains(powerline),
+                        "MUST contain the parent powerline way. This prevents JOSM from discarding the error " +
+                                "during partial validation if the node itself was unmodified.");
+            }
+            // verify POWER_CONNECTION behavior (bad connection)
+            if (error.getCode() == PowerLines.POWER_CONNECTION && error.getPrimitives().contains(sharedNode)) {
+                foundConnectionError = true;
+                assertTrue(error.getPrimitives().contains(highway),
+                        "MUST contain the unrelated parent way. This prevents JOSM from discarding the error" +
+                                "during partial validation if the node itself was unmodified.");
+            }
+        }
+
+        assertTrue(foundSupportError, "Should report missing power tag on shared node");
+        assertTrue(foundConnectionError, "Should report bad connection on shared node");
+    }
+}
+ No newline at end of file