Index: src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (revision 14436) +++ src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (working copy) @@ -53,8 +53,6 @@ public static final int WRONG_MEMBER_ROLE = 1602; /** Multipolygon is not closed */ public static final int NON_CLOSED_WAY = 1603; - /** No outer way for multipolygon */ - public static final int MISSING_OUTER_WAY = 1604; /** Multipolygon inner way is outside */ public static final int INNER_WAY_OUTSIDE = 1605; /** Intersection between multipolygon ways */ @@ -132,37 +130,22 @@ @Override public void visit(Relation r) { if (r.isMultipolygon() && r.getMembersCount() > 0) { - checkMembersAndRoles(r); - checkOuterWay(r); + List tmpErrors = new ArrayList<>(30); + boolean hasUnexpectedWayRoles = checkMembersAndRoles(r, tmpErrors); boolean hasRepeatedMembers = checkRepeatedWayMembers(r); // Rest of checks is only for complete multipolygon - if (!hasRepeatedMembers && !r.hasIncompleteMembers()) { + if (!hasUnexpectedWayRoles && !hasRepeatedMembers && !r.hasIncompleteMembers()) { Multipolygon polygon = new Multipolygon(r); checkStyleConsistency(r, polygon); checkGeometryAndRoles(r, polygon); + } else { + // if we are not able to calculate the geometry we report the messages found by simple checks + errors.addAll(tmpErrors); } } } /** - * Checks that multipolygon has at least an outer way: - * @param r relation - */ - private void checkOuterWay(Relation r) { - for (RelationMember m : r.getMembers()) { - if (m.isWay() && "outer".equals(m.getRole())) { - return; - } - } - errors.add(TestError.builder(this, Severity.WARNING, MISSING_OUTER_WAY) - .message(r.isBoundary() ? tr("No outer way for boundary") : tr("No outer way for multipolygon")) - .primitives(r) - .build()); - } - - /** * Various style-related checks: * @param r relation + * @param tmpErrors list that will contain found errors + * @return true if ways with roles other than inner, outer or empty where found */ - private void checkMembersAndRoles(Relation r) { + private boolean checkMembersAndRoles(Relation r, List tmpErrors) { + boolean hasUnexpectedWayRole = false; for (RelationMember rm : r.getMembers()) { if (rm.isWay()) { - if (!(rm.hasRole("inner", "outer") || !rm.hasRole())) { - errors.add(TestError.builder(this, Severity.WARNING, WRONG_MEMBER_ROLE) - .message(tr("No useful role for multipolygon member")) + if (rm.hasRole() && !(rm.hasRole("inner", "outer"))) + hasUnexpectedWayRole = true; + if (!(rm.hasRole("inner", "outer")) || !rm.hasRole()) { + tmpErrors.add(TestError.builder(this, Severity.ERROR, WRONG_MEMBER_ROLE) + .message(tr("Role for multipolygon way member should be inner or outer")) .primitives(Arrays.asList(r, rm.getMember())) .build()); } } else { if (!r.isBoundary() || !rm.hasRole("admin_centre", "label", "subarea", "land_area")) { - errors.add(TestError.builder(this, Severity.WARNING, WRONG_MEMBER_TYPE) + tmpErrors.add(TestError.builder(this, Severity.WARNING, WRONG_MEMBER_TYPE) .message(r.isBoundary() ? tr("Non-Way in boundary") : tr("Non-Way in multipolygon")) .primitives(Arrays.asList(r, rm.getMember())) .build()); @@ -723,6 +711,7 @@ } } } + return hasUnexpectedWayRole; } private static Collection combineRelAndPrimitives(Relation r, Collection primitives) { @@ -748,6 +737,8 @@ boolean hasDups = false; Map> seenMemberPrimitives = new HashMap<>(); for (RelationMember rm : r.getMembers()) { + if (!rm.isWay()) + continue; List list = seenMemberPrimitives.get(rm.getMember()); if (list == null) { list = new ArrayList<>(2); Index: src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java (revision 14436) +++ src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java (working copy) @@ -18,9 +18,11 @@ import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.RelationMember; +import org.openstreetmap.josm.data.validation.OsmValidator; import org.openstreetmap.josm.data.validation.Severity; import org.openstreetmap.josm.data.validation.Test; import org.openstreetmap.josm.data.validation.TestError; +import org.openstreetmap.josm.gui.progress.ProgressMonitor; import org.openstreetmap.josm.gui.tagging.presets.TaggingPreset; import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetItem; import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetType; @@ -60,6 +62,7 @@ * @since 6731 */ public static final String ROLE_VERIF_PROBLEM_MSG = tr("Role verification problem"); + private boolean ignoreMultiPolygons; /** * Constructor @@ -99,7 +102,30 @@ } @Override + public void startTest(ProgressMonitor progressMonitor) { + super.startTest(progressMonitor); + + for (Test t : OsmValidator.getEnabledTests(false)) { + if (t instanceof MultipolygonTest) { + ignoreMultiPolygons = true; + break; + } + } + } + + @Override public void visit(Relation n) { + Map map = buildRoleInfoMap(n); + if (map.isEmpty()) { + errors.add(TestError.builder(this, Severity.ERROR, RELATION_EMPTY) + .message(tr("Relation is empty")) + .primitives(n) + .build()); + } + if (ignoreMultiPolygons && n.isMultipolygon()) { + // see #17010: don't report same problem twice + return; + } Map allroles = buildAllRoles(n); if (allroles.isEmpty() && n.hasTag("type", "route") && n.hasTag("route", "train", "subway", "monorail", "tram", "bus", "trolleybus", "aerialway", "ferry")) { @@ -114,13 +140,7 @@ .build()); } - Map map = buildRoleInfoMap(n); - if (map.isEmpty()) { - errors.add(TestError.builder(this, Severity.ERROR, RELATION_EMPTY) - .message(tr("Relation is empty")) - .primitives(n) - .build()); - } else if (!allroles.isEmpty()) { + if (!map.isEmpty() && !allroles.isEmpty()) { checkRoles(n, allroles, map); } }