Changeset 9932 in josm

Timestamp:

2016-03-05T17:57:04+01:00 (8 years ago)

Author:

simon04

Message:

see #8460 - fix typo from r9931, fix matching, improve conditions, add unit test

Location:

trunk

Files:

• data/defaultpresets.xml (modified) (2 diffs)
• src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java (modified) (2 diffs)
• src/org/openstreetmap/josm/gui/tagging/presets/TaggingPreset.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetItem.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/data/validation/tests/RelationCheckerTest.groovy (modified) (1 diff)

trunk/data/defaultpresets.xml

@@ -3089 +3089 @@
             </optional>
             <roles>
-                <role key="stop" text="stop position" requisite="optional" type="node" member_expression="public_transport=stop_position (train OR subway OR monorail OR tram OR light_rail)" />
-                <role key="stop_exit_only" text="stop position (exit only)" requisite="optional" type="node" member_expression="public_transport=stop_position (train OR subway OR monorail OR tram OR light_rail)" />
-                <role key="stop_entry_only" text="stop position (entry only)" requisite="optional" type="node" member_expression="public_transport=stop_position (train OR subway OR monorail OR tram OR light_rail)" />
-                <role key="platform" text="platform" requisite="optional" type="node,way,closedway,multipolygon" member_expression="public_transport=stop_position (train OR subway OR monorail OR tram OR light_rail)" />
-                <role key="platform_exit_only" text="platform (exit only)" requisite="optional" type="node,way,closedway" member_expression="public_transport=stop_position (train OR subway OR monorail OR tram OR light_rail)" />
-                <role key="platform_entry_only" text="platform (entry only)" requisite="optional" type="node,way,closedway" member_expression="public_transport=stop_position (train OR subway OR monorail OR tram OR light_rail)" />
+                <role key="stop" text="stop position" requisite="optional" type="node" member_expression="public_transport=stop_position (train=yes OR subway=yes OR monorail=yes OR tram=yes OR light_rail=yes)" />
+                <role key="stop_exit_only" text="stop position (exit only)" requisite="optional" type="node" member_expression="public_transport=stop_position (train=yes OR subway=yes OR monorail=yes OR tram=yes OR light_rail=yes)" />
+                <role key="stop_entry_only" text="stop position (entry only)" requisite="optional" type="node" member_expression="public_transport=stop_position (train=yes OR subway=yes OR monorail=yes OR tram=yes OR light_rail=yes)" />
+                <role key="platform" text="platform" requisite="optional" type="node,way,closedway,multipolygon" member_expression="public_transport=platform (train=yes OR subway=yes OR monorail=yes OR tram=yes OR light_rail=yes)" />
+                <role key="platform_exit_only" text="platform (exit only)" requisite="optional" type="node,way,closedway" member_expression="public_transport=platform (train=yes OR subway=yes OR monorail=yes OR tram=yes OR light_rail=yes)" />
+                <role key="platform_entry_only" text="platform (entry only)" requisite="optional" type="node,way,closedway" member_expression="public_transport=platform (train=yes OR subway=yes OR monorail=yes OR tram=yes OR light_rail=yes)" />
                 <role key="" text="route segment" requisite="required" type="way,closedway" member_expression="railway" />
             </roles>
@@ -3109 +3109 @@
             </optional>
             <roles>
-                <role key="stop" text="stop position" requisite="optional" type="node" member_expression="public_transport=stop_position (bus OR trolleybus)" />
-                <role key="stop_exit_only" text="stop position (exit only)" requisite="optional" type="node" member_expression="public_transport=stop_position (bus OR trolleybus)" />
-                <role key="stop_entry_only" text="stop position (entry only)" requisite="optional" type="node" member_expression="public_transport=stop_position (bus OR trolleybus)" />
-                <role key="platform" text="platform" requisite="optional" type="node,way,closedway,multipolygon" member_expression="public_transport=platform (bus OR trolleybus)" />
-                <role key="platform_exit_only" text="platform (exit only)" requisite="optional" type="node,way,closedway" member_expression="public_transport=platform (bus OR trolleybus)" />
-                <role key="platform_entry_only" text="platform (entry only)" requisite="optional" type="node,way,closedway" member_expression="public_transport=platform (bus OR trolleybus)" />
+                <role key="stop" text="stop position" requisite="optional" type="node" member_expression="public_transport=stop_position (bus=yes OR trolleybus=yes)" />
+                <role key="stop_exit_only" text="stop position (exit only)" requisite="optional" type="node" member_expression="public_transport=stop_position (bus=yes OR trolleybus=yes)" />
+                <role key="stop_entry_only" text="stop position (entry only)" requisite="optional" type="node" member_expression="public_transport=stop_position (bus=yes OR trolleybus=yes)" />
+                <role key="platform" text="platform" requisite="optional" type="node,way,closedway,multipolygon" member_expression="public_transport=platform (bus=yes OR trolleybus=yes)" />
+                <role key="platform_exit_only" text="platform (exit only)" requisite="optional" type="node,way,closedway" member_expression="public_transport=platform (bus=yes OR trolleybus=yes)" />
+                <role key="platform_entry_only" text="platform (entry only)" requisite="optional" type="node,way,closedway" member_expression="public_transport=platform (bus=yes OR trolleybus=yes)" />
                 <role key="" text="route segment" requisite="required" type="way,closedway" member_expression="highway" />
             </roles>

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

@@ -25 +25 @@
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetType;
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresets;
-import org.openstreetmap.josm.gui.tagging.presets.items.Key;
+import org.openstreetmap.josm.gui.tagging.presets.items.KeyedItem;
 import org.openstreetmap.josm.gui.tagging.presets.items.Roles;
 import org.openstreetmap.josm.gui.tagging.presets.items.Roles.Role;
@@ -145 +145 @@
 
         for (TaggingPreset p : relationpresets) {
-            boolean matches = true;
-            Roles r = null;
-            for (TaggingPresetItem i : p.data) {
-                if (i instanceof Key) {
-                    Key k = (Key) i;
-                    if (!k.value.equals(n.get(k.key))) {
-                        matches = false;
-                        break;
-                    }
-                } else if (i instanceof Roles) {
-                    r = (Roles) i;
-                }
-            }
+            final boolean matches = TaggingPresetItem.matches(Utils.filteredCollection(p.data, KeyedItem.class), n.getKeys());
+            final Roles r = Utils.find(p.data, Roles.class);
             if (matches && r != null) {
                 for (Role role: r.roles) {

trunk/src/org/openstreetmap/josm/gui/tagging/presets/TaggingPreset.java

@@ -562 +562 @@
         else if (!typeMatches(t))
             return false;
-        boolean atLeastOnePositiveMatch = false;
-        for (TaggingPresetItem item : data) {
-            Boolean m = item.matches(tags);
-            if (m != null && !m)
-                return false;
-            else if (m != null) {
-                atLeastOnePositiveMatch = true;
-            }
-        }
-        return atLeastOnePositiveMatch;
+        else
+            return TaggingPresetItem.matches(data, tags);
     }
 

trunk/src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetItem.java

@@ -134 +134 @@
         return imgProv.get();
     }
+
+    /**
+     * Determine whether the given preset items match the tags
+     * @param data the preset items
+     * @param tags the tags to match
+     * @return whether the given preset items match the tags
+     * @sice 9932
+     */
+    public static boolean matches(Iterable<? extends TaggingPresetItem> data, Map<String, String> tags) {
+        boolean atLeastOnePositiveMatch = false;
+        for (TaggingPresetItem item : data) {
+            Boolean m = item.matches(tags);
+            if (m != null && !m)
+                return false;
+            else if (m != null) {
+                atLeastOnePositiveMatch = true;
+            }
+        }
+        return atLeastOnePositiveMatch;
+    }
 }

trunk/test/unit/org/openstreetmap/josm/data/validation/tests/RelationCheckerTest.groovy

@@ -118 +118 @@
         assert errors.get(0).getDescription() == "Role member does not match expression power in template Power Route"
     }
+
+    void testRouteMemberExpression() {
+        def r = createRelation("type=route route=tram public_transport:version=2")
+        r.addMember(new RelationMember("", createPrimitive("way railway=tram")))
+        r.addMember(new RelationMember("stop", createPrimitive("node public_transport=stop_position tram=yes")))
+        r.addMember(new RelationMember("platform", createPrimitive("node public_transport=platform tram=yes")))
+        assert testRelation(r).size() == 0
+
+        r.addMember(new RelationMember("", createPrimitive("way no-rail-way=yes")))
+        assert testRelation(r).size() == 1
+        assert testRelation(r).get(0).getDescription() == "Role member does not match expression railway in template Public Transport Route (Rail)"
+
+        r.removeMember(3)
+        r.addMember(new RelationMember("stop", createPrimitive("way no-rail-way=yes")))
+        assert testRelation(r).size() == 1
+        assert testRelation(r).get(0).getDescription() == "Role member type way does not match accepted list of node in template Public Transport Route (Rail)"
+
+        r.removeMember(3)
+        r.addMember(new RelationMember("stop", createPrimitive("node public_transport=stop_position bus=yes")))
+        assert testRelation(r).size() == 1
+        assert testRelation(r).get(0).getDescription() == "Role member does not match expression public_transport=stop_position && (train=yes || subway=yes || monorail=yes || tram=yes || light_rail=yes) in template Public Transport Route (Rail)"
+    }
 }