Index: src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java (revision 7209) +++ src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java (working copy) @@ -5,21 +5,17 @@ import static org.openstreetmap.josm.tools.I18n.tr; import java.text.MessageFormat; -import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; -import java.util.List; import java.util.Set; import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.command.DeleteCommand; -import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.RelationMember; -import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.validation.Severity; import org.openstreetmap.josm.data.validation.Test; import org.openstreetmap.josm.data.validation.TestError; @@ -82,17 +78,22 @@ } } + private static class RolePreset { + public RolePreset(LinkedList roles, String name) { + this.roles = roles; + this.name = name; + } + private LinkedList roles; + private String name; + } + private static class RoleInfo { private int total = 0; - private Collection nodes = new LinkedList<>(); - private Collection ways = new LinkedList<>(); - private Collection openways = new LinkedList<>(); - private Collection relations = new LinkedList<>(); } @Override public void visit(Relation n) { - LinkedList allroles = buildAllRoles(n); + HashMap allroles = buildAllRoles(n); if (allroles.isEmpty() && n.hasTag("type", "route") && n.hasTag("route", "train", "subway", "monorail", "tram", "bus", "trolleybus", "aerialway", "ferry")) { errors.add(new TestError(this, Severity.WARNING, @@ -117,26 +118,17 @@ RoleInfo ri = map.get(role); if (ri == null) { ri = new RoleInfo(); + map.put(role, ri); } ri.total++; - if (m.isRelation()) { - ri.relations.add(m.getRelation()); - } else if(m.isWay()) { - ri.ways.add(m.getWay()); - if (!m.getWay().isClosed()) { - ri.openways.add(m.getWay()); - } - } - else if (m.isNode()) { - ri.nodes.add(m.getNode()); - } - map.put(role, ri); } return map; } - private LinkedList buildAllRoles(Relation n) { - LinkedList allroles = new LinkedList<>(); + // return Roles grouped by key + private HashMap buildAllRoles(Relation n) { + HashMap allroles = new HashMap<>(); + for (TaggingPreset p : relationpresets) { boolean matches = true; Roles r = null; @@ -152,97 +144,139 @@ } } if (matches && r != null) { - allroles.addAll(r.roles); + for(Role role: r.roles) { + String key = role.key; + LinkedList roleGroup = null; + if (allroles.containsKey(key)) { + roleGroup = allroles.get(key).roles; + } else { + roleGroup = new LinkedList<>(); + allroles.put(key, new RolePreset(roleGroup, p.name)); + } + roleGroup.add(role); + } } } return allroles; } - private void checkRoles(Relation n, LinkedList allroles, HashMap map) { - List done = new LinkedList<>(); - // Remove empty roles if several exist (like in route=hiking, see #9844) - List emptyRoles = new LinkedList<>(); - for (Role r : allroles) { - if ("".equals(r.key)) { - emptyRoles.add(r); + private boolean checkMemberType(Role r, RelationMember member) { + if (r.types != null) { + for (TaggingPresetType type: r.types) { + if (type.equals(TaggingPresetType.CLOSEDWAY) && member.isWay() && member.getWay().isClosed()) { + return true; + } + if(type.equals(TaggingPresetType.NODE) && member.isNode()) { + return true; + } + if(type.equals(TaggingPresetType.RELATION) && member.isRelation()) { + return true; + } + if(type.equals(TaggingPresetType.WAY) && member.isWay()) { + return true; + } } + } else { + // if no types specified, then test is passed + return true; } - if (emptyRoles.size() > 1) { - allroles.removeAll(emptyRoles); + // if no match is found, then test failed + return false; + } + + // get all role definition for specified key and check, if some definition matches + private boolean checkMemberExpressionAndType(RolePreset rolePreset, RelationMember member, Relation n) { + TestError possibleMatchError = null; + if (rolePreset == null || rolePreset.roles == null) { + // no restrictions on role types + return true; } - for (Role r : allroles) { - done.add(r.key); - String keyname = r.key; - if ("".equals(keyname)) { - keyname = tr(""); - } - RoleInfo ri = map.get(r.key); - checkRoleCounts(n, r, keyname, ri); - if (ri != null) { - if (r.types != null) { - checkRoleTypes(n, r, keyname, ri); - } + for (Role r: rolePreset.roles) { + if (checkMemberType(r, member)) { if (r.memberExpression != null) { - checkRoleMemberExpressions(n, r, keyname, ri); + OsmPrimitive primitive = member.getMember(); + if(primitive.isUsable()) { + if(r.memberExpression.match(primitive)) { + return true; + } else { + // possible match error + String s = marktr("Role member didn''t match expression {0} in template {1}"); + possibleMatchError = new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, + tr(s, r.memberExpression, rolePreset.name), s, WRONG_TYPE, + member.getMember().isUsable() ? member.getMember() : n); + + } + } else { + // if it is not usable, we can't verify memberExpression + return true; + } + } else { + return true; + } + } else { + + } + } + if( possibleMatchError != null) { + errors.add(possibleMatchError); + } else { + String s = marktr("Role member type {0} didn''t match accepted list of {1} in template {2}"); + Set> types = new HashSet<>(); + for (Role r: rolePreset.roles) { + types.addAll(r.types); + } + errors.add(new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, + tr(s, member.getType(), types, rolePreset.name), s, WRONG_TYPE, + member.getMember().isUsable() ? member.getMember() : n)); + } + return false; + } + + + private void checkRoles(Relation n, HashMap allroles, HashMap map) { + Set done = new HashSet<>(); + + // go through all members of relation + for (RelationMember member: n.getMembers()) { + String role = member.getRole(); + done.add(role); + + // error reporting done inside + checkMemberExpressionAndType(allroles.get(role), member, n); + } + + // verify role counts based on whole role sets + for(RolePreset rp: allroles.values()) { + for (Role r: rp.roles) { + String keyname = r.key; + if ("".equals(keyname)) { + keyname = tr(""); } + checkRoleCounts(n, r, keyname, map.get(r.key)); } } + // verify unwanted members for (String key : map.keySet()) { if (!done.contains(key)) { + StringBuilder templates = new StringBuilder(); + for (RolePreset rp: allroles.values()) { + templates.append(rp.name); + } + if (key.length() > 0) { - String s = marktr("Role {0} unknown"); + String s = marktr("Role {0} unknown in templates {1}"); + errors.add(new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, - tr(s, key), MessageFormat.format(s, key), ROLE_UNKNOWN, n)); + tr(s, key, templates.toString()), MessageFormat.format(s, key), ROLE_UNKNOWN, n)); } else { - String s = marktr("Empty role found"); + String s = marktr("Empty role found in templates {0}"); errors.add(new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, - tr(s), s, ROLE_EMPTY, n)); + tr(s, templates), s, ROLE_EMPTY, n)); } } } } - private void checkRoleMemberExpressions(Relation n, Role r, String keyname, RoleInfo ri) { - Set notMatching = new HashSet<>(); - Collection allPrimitives = new ArrayList<>(); - allPrimitives.addAll(ri.nodes); - allPrimitives.addAll(ri.ways); - allPrimitives.addAll(ri.relations); - for (OsmPrimitive p : allPrimitives) { - if (p.isUsable() && !r.memberExpression.match(p)) { - notMatching.add(p); - } - } - if (!notMatching.isEmpty()) { - String s = marktr("Member for role ''{0}'' does not match ''{1}''"); - LinkedList highlight = new LinkedList<>(notMatching); - highlight.addFirst(n); - errors.add(new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, - tr(s, keyname, r.memberExpression), MessageFormat.format(s, keyname, r.memberExpression), WRONG_TYPE, - highlight, notMatching)); - } - } - - private void checkRoleTypes(Relation n, Role r, String keyname, RoleInfo ri) { - Set wrongTypes = new HashSet<>(); - if (!r.types.contains(TaggingPresetType.WAY)) { - wrongTypes.addAll(r.types.contains(TaggingPresetType.CLOSEDWAY) ? ri.openways : ri.ways); - } - if (!r.types.contains(TaggingPresetType.NODE)) { - wrongTypes.addAll(ri.nodes); - } - if (!r.types.contains(TaggingPresetType.RELATION)) { - wrongTypes.addAll(ri.relations); - } - if (!wrongTypes.isEmpty()) { - String s = marktr("Member for role {0} of wrong type"); - LinkedList highlight = new LinkedList<>(wrongTypes); - highlight.addFirst(n); - errors.add(new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, - tr(s, keyname), MessageFormat.format(s, keyname), WRONG_TYPE, - highlight, wrongTypes)); - } - } private void checkRoleCounts(Relation n, Role r, String keyname, RoleInfo ri) { long count = (ri == null) ? 0 : ri.total;