Ticket #20013: 20013.2.patch

src/org/openstreetmap/josm/actions/corrector/ReverseWayTagCorrector.java

@@ -13 +13 @@
 import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.data.correction.RoleCorrection;
@@ -24 +25 @@
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
 import org.openstreetmap.josm.data.osm.Tag;
-import org.openstreetmap.josm.data.osm.TagCollection;
 import org.openstreetmap.josm.data.osm.Tagged;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.tools.Logging;
@@ -173 +173 @@
     /**
      * Tests whether way can be reversed without semantic change, i.e., whether tags have to be changed.
      * Looks for keys like oneway, oneway:bicycle, cycleway:right:oneway, left/right.
+     * Also tests the nodes, e.g. a highway=stop with direction, see #20013.
      * @param way way to test
      * @return false if tags should be changed to keep semantic, true otherwise.
      */
     public static boolean isReversible(Way way) {
-        for (Tag tag : TagCollection.from(way)) {
-            if (!tag.equals(TagSwitcher.apply(tag))) {
-                return false;
-            }
-        }
-        return true;
+        return getTagCorrectionsMap(way).isEmpty();
     }
 
     /**
@@ -192 +188 @@
      * @see #isReversible(Way)
      */
     public static List<Way> irreversibleWays(List<Way> ways) {
-        List<Way> newWays = new ArrayList<>(ways);
-        for (Way way : ways) {
-            if (isReversible(way)) {
-                newWays.remove(way);
-            }
-        }
-        return newWays;
+        return ways.stream().filter(w -> !isReversible(w)).collect(Collectors.toList());
     }
 
     /**

test/unit/org/openstreetmap/josm/actions/corrector/ReverseWayTagCorrectorTest.java

@@ -7 +7 @@
 import java.util.stream.Stream;
 
 import org.junit.Assert;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
-import org.junit.jupiter.api.Test;
 import org.openstreetmap.josm.data.correction.TagCorrection;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -107 +107 @@
         Assert.assertEquals(newTag, ReverseWayTagCorrector.TagSwitcher.apply(oldTag));
     }
 
-    private Map<OsmPrimitive, List<TagCorrection>> getTagCorrectionsForWay(String middleNodeTags) {
+    private Way buildWayWithMiddleNode(String middleNodeTags) {
         final OsmPrimitive n1 = OsmUtils.createPrimitive("node");
         final OsmPrimitive n2 = OsmUtils.createPrimitive("node " + middleNodeTags);
         final OsmPrimitive n3 = OsmUtils.createPrimitive("node");
         final Way w = new Way();
         Stream.of(n1, n2, n3).map(Node.class::cast).forEach(w::addNode);
+        return w;
+    }
+
+    private Map<OsmPrimitive, List<TagCorrection>> getTagCorrectionsForWay(String middleNodeTags) {
+        Way w = buildWayWithMiddleNode(middleNodeTags);
         return ReverseWayTagCorrector.getTagCorrectionsMap(w);
     }
 
@@ -135 +140 @@
         Assert.assertEquals(0, getTagCorrectionsForWay("direction=SSW").size());
         Assert.assertEquals(0, getTagCorrectionsForWay("direction=145").size());
     }
+
+    /**
+     * Tests that IsReversible() also works for nodes. See #20013
+     */
+    @Test
+    void testIsReversible() {
+        Way w0 = buildWayWithMiddleNode("highway=stop");
+        Assert.assertTrue(ReverseWayTagCorrector.isReversible(w0));
+        Way w1 = buildWayWithMiddleNode("direction=forward");
+        Assert.assertFalse(ReverseWayTagCorrector.isReversible(w1));
+        Assert.assertEquals(3, w1.getNodesCount());
+        w1.getNodes().forEach(n -> n.setKeys(null));
+        Assert.assertTrue(ReverseWayTagCorrector.isReversible(w1));
+        w1.put("oneway", "yes");
+        Assert.assertFalse(ReverseWayTagCorrector.isReversible(w1));
+    }
+
 }