Changeset 17289 in josm

Timestamp:

2020-11-02T09:04:47+01:00 (5 years ago)

Author:

GerdP

Message:

fix #20013: When combining two ways and one of them needs to have the direction changed then JOSM doesn't consider effects on stop and give way signs

Location:

trunk

Files:

• src/org/openstreetmap/josm/actions/corrector/ReverseWayTagCorrector.java (modified) (4 diffs)
• test/unit/org/openstreetmap/josm/actions/corrector/ReverseWayTagCorrectorTest.java (modified) (4 diffs)

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

@@ -14 +14 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 import org.openstreetmap.josm.command.Command;
@@ -25 +26 @@
 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;
@@ -174 +174 @@
      * 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();
     }
 
@@ -193 +189 @@
      */
     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());
     }
 

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

@@ -8 +8 @@
 
 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;
@@ -108 +108 @@
     }
 
-    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);
@@ -114 +114 @@
         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);
     }
@@ -136 +141 @@
         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));
+    }
+
 }