Changeset 5787 in josm

Timestamp:

2013-03-19T23:10:02+01:00 (12 years ago)

Author:

Don-vip

Message:

fix #8518 - Incorrect automatic tag correction when reversing a way

Location:

trunk

Files:

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

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

@@ -5 +5 @@
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
@@ -18 +17 @@
 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.Way;
 
@@ -28 +29 @@
  * for the user to confirm.
  */
-
 public class ReverseWayTagCorrector extends TagCorrector<Way> {
 
-    private static class PrefixSuffixSwitcher {
-
-        private static final String SEPARATOR = "[:_]";
+    private static final String SEPARATOR = "[:_]";
+
+    private static final Pattern getPatternFor(String s) {
+        return getPatternFor(s, false);
+    }
+
+    private static final Pattern getPatternFor(String s, boolean exactMatch) {
+        if (exactMatch) {
+            return Pattern.compile("(^)(" + s + ")($)");
+        } else {
+            return Pattern.compile("(^|.*" + SEPARATOR + ")(" + s + ")(" + SEPARATOR + ".*|$)",
+                    Pattern.CASE_INSENSITIVE);
+        }
+    }
+    
+    private static final Collection<Pattern> ignoredKeys = new ArrayList<Pattern>();
+    static {
+        for (String s : OsmPrimitive.getUninterestingKeys()) {
+            ignoredKeys.add(getPatternFor(s));
+        }
+        for (String s : new String[]{"name", "ref", "tiger:county"}) {
+            ignoredKeys.add(getPatternFor(s, false));
+        }
+        for (String s : new String[]{"tiger:county", "turn:lanes", "change:lanes", "placement"}) {
+            ignoredKeys.add(getPatternFor(s, true));
+        }
+    }
+    
+    private static class StringSwitcher {
 
         private final String a;
@@ -39 +65 @@
         private final Pattern pattern;
 
-        public PrefixSuffixSwitcher(String a, String b) {
+        public StringSwitcher(String a, String b) {
             this.a = a;
             this.b = b;
-            this.pattern = Pattern.compile(
-                    "(^|.*" + SEPARATOR + ")(" + a + "|" + b + ")(" + SEPARATOR + ".*|$)",
-                    Pattern.CASE_INSENSITIVE);
+            this.pattern = getPatternFor(a + "|" + b);
         }
 
@@ -64 +88 @@
     }
 
-    private static final PrefixSuffixSwitcher[] prefixSuffixSwitchers =
-        new PrefixSuffixSwitcher[] {
-        new PrefixSuffixSwitcher("left", "right"),
-        new PrefixSuffixSwitcher("forward", "backward"),
-        new PrefixSuffixSwitcher("forwards", "backwards"),
-        new PrefixSuffixSwitcher("up", "down"),
-        new PrefixSuffixSwitcher("east", "west"),
-        new PrefixSuffixSwitcher("north", "south"),
-    };
-
     /**
-     * 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.
-     * @param way
-     * @return false if tags should be changed to keep semantic, true otherwise.
+     * Reverses a given tag.
+     * @since 5787
      */
-    public static boolean isReversible(Way way) {
-        for (String key : way.keySet()) {
-            for (String k : Arrays.asList("oneway", "incline", "direction")) {
-                if (key.startsWith(k) || key.endsWith(k)) {
-                    return false;
-                }
-            }
-            String value = way.get(key);
-            for (PrefixSuffixSwitcher prefixSuffixSwitcher : prefixSuffixSwitchers) {
-                if (!key.equals(prefixSuffixSwitcher.apply(key)) || !value.equals(prefixSuffixSwitcher.apply(value))) {
-                    return false;
-                }
-            }
-        }
-        return true;
-    }
-
-    public static List<Way> irreversibleWays(List<Way> ways) {
-        List<Way> newWays = new ArrayList<Way>(ways);
-        for (Way way : ways) {
-            if (isReversible(way)) {
-                newWays.remove(way);
-            }
-        }
-        return newWays;
-    }
-
-    public String invertNumber(String value) {
-        Pattern pattern = Pattern.compile("^([+-]?)(\\d.*)$", Pattern.CASE_INSENSITIVE);
-        Matcher matcher = pattern.matcher(value);
-        if (!matcher.matches()) return value;
-        String sign = matcher.group(1);
-        String rest = matcher.group(2);
-        sign = sign.equals("-") ? "" : "-";
-        return sign + rest;
-    }
-
-    @Override
-    public Collection<Command> execute(Way oldway, Way way) throws UserCancelException {
-        Map<OsmPrimitive, List<TagCorrection>> tagCorrectionsMap =
-            new HashMap<OsmPrimitive, List<TagCorrection>>();
-
-        ArrayList<TagCorrection> tagCorrections = new ArrayList<TagCorrection>();
-        for (String key : way.keySet()) {
+    public static class TagSwitcher {
+        
+        /**
+         * Reverses a given tag.
+         * @param tag The tag to reverse
+         * @return The reversed tag (is equal to <code>tag</code> if no change is needed)
+         */
+        public static final Tag apply(final Tag tag) {
+            return apply(tag.getKey(), tag.getValue());
+        }
+        
+        /**
+         * Reverses a given tag (key=value).
+         * @param key The tag key
+         * @param value The tag value
+         * @return The reversed tag (is equal to <code>key=value</code> if no change is needed)
+         */
+        public static final Tag apply(final String key, final String value) {
             String newKey = key;
-            String value = way.get(key);
             String newValue = value;
 
@@ -136 +121 @@
             } else if (key.startsWith("incline") || key.endsWith("incline")
                     || key.startsWith("direction") || key.endsWith("direction")) {
-                PrefixSuffixSwitcher switcher = new PrefixSuffixSwitcher("up", "down");
-                newValue = switcher.apply(value);
+                newValue = UP_DOWN.apply(value);
                 if (newValue.equals(value)) {
                     newValue = invertNumber(value);
                 }
-            } else if (!ignoreKeyForPrefixSuffixCorrection(key)) {
-                for (PrefixSuffixSwitcher prefixSuffixSwitcher : prefixSuffixSwitchers) {
+            } else if (key.endsWith(":forward") || key.endsWith(":backward")) {
+                // Change key but not left/right value (fix #8518)
+                newKey = FORWARD_BACKWARD.apply(key);
+                
+            } else if (!ignoreKeyForCorrection(key)) {
+                for (StringSwitcher prefixSuffixSwitcher : stringSwitchers) {
                     newKey = prefixSuffixSwitcher.apply(key);
                     if (!key.equals(newKey)) {
@@ -153 +141 @@
                 }
             }
+            return new Tag(newKey, newValue);
+        }
+    }
+    
+    private static final StringSwitcher FORWARD_BACKWARD = new StringSwitcher("forward", "backward");
+    private static final StringSwitcher UP_DOWN = new StringSwitcher("up", "down");
+
+    private static final StringSwitcher[] stringSwitchers = new StringSwitcher[] {
+        new StringSwitcher("left", "right"),
+        new StringSwitcher("forwards", "backwards"),
+        new StringSwitcher("east", "west"),
+        new StringSwitcher("north", "south"),
+        FORWARD_BACKWARD, UP_DOWN
+    };
+
+    /**
+     * 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.
+     * @param way
+     * @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;
+    }
+
+    public static List<Way> irreversibleWays(List<Way> ways) {
+        List<Way> newWays = new ArrayList<Way>(ways);
+        for (Way way : ways) {
+            if (isReversible(way)) {
+                newWays.remove(way);
+            }
+        }
+        return newWays;
+    }
+
+    public static String invertNumber(String value) {
+        Pattern pattern = Pattern.compile("^([+-]?)(\\d.*)$", Pattern.CASE_INSENSITIVE);
+        Matcher matcher = pattern.matcher(value);
+        if (!matcher.matches()) return value;
+        String sign = matcher.group(1);
+        String rest = matcher.group(2);
+        sign = sign.equals("-") ? "" : "-";
+        return sign + rest;
+    }
+
+    @Override
+    public Collection<Command> execute(Way oldway, Way way) throws UserCancelException {
+        Map<OsmPrimitive, List<TagCorrection>> tagCorrectionsMap =
+            new HashMap<OsmPrimitive, List<TagCorrection>>();
+
+        ArrayList<TagCorrection> tagCorrections = new ArrayList<TagCorrection>();
+        for (String key : way.keySet()) {
+            String value = way.get(key);
+            Tag newTag = TagSwitcher.apply(key, value);
+            String newKey = newTag.getKey();
+            String newValue = newTag.getValue();
 
             boolean needsCorrection = !key.equals(newKey);
@@ -190 +239 @@
                 boolean found = false;
                 String newRole = null;
-                for (PrefixSuffixSwitcher prefixSuffixSwitcher : prefixSuffixSwitchers) {
+                for (StringSwitcher prefixSuffixSwitcher : stringSwitchers) {
                     newRole = prefixSuffixSwitcher.apply(member.getRole());
                     if (!newRole.equals(member.getRole())) {
@@ -215 +264 @@
     }
 
-    private static boolean ignoreKeyForPrefixSuffixCorrection(String key) {
-        return key.contains("name") || key.equals("tiger:county")
-                || key.equalsIgnoreCase("fixme") || key.startsWith("note");
+    private static boolean ignoreKeyForCorrection(String key) {
+        for (Pattern ignoredKey : ignoredKeys) {
+            if (ignoredKey.matcher(key).matches()) {
+                return true;
+            }
+        }
+        return false;
     }
 }