Index: /applications/editors/josm/plugins/reltoolbox/build.xml =================================================================== --- /applications/editors/josm/plugins/reltoolbox/build.xml (revision 28702) +++ /applications/editors/josm/plugins/reltoolbox/build.xml (revision 28703) @@ -30,5 +30,5 @@ - + Index: /applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/SortAndFixAction.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/SortAndFixAction.java (revision 28702) +++ /applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/SortAndFixAction.java (revision 28703) @@ -37,13 +37,18 @@ rel.addChosenRelationListener(this); setEnabled(false); - + // construct all available fixers fixers = new ArrayList(); - fixers.add(new BoundaryFixer()); // should be before multipolygon as takes special case of multipolygon relation - boundary - fixers.add(new MultipolygonFixer()); - fixers.add(new AssociatedStreetFixer()); - + //should be before multipolygon as takes special case of multipolygon relation - boundary + fixers.add(new BoundaryFixer()); // boundary, multipolygon, boundary=administrative + fixers.add(new MultipolygonFixer()); // multipolygon + fixers.add(new AssociatedStreetFixer()); //associatedStreet + + for(RelationFixer fix : fixers) { + fix.setFixAction(this); + } } + @Override public void actionPerformed( ActionEvent e ) { Command c = fixRelation(rel.get()); @@ -52,4 +57,5 @@ } + @Override public void chosenRelationChanged( Relation oldRelation, Relation newRelation ) { setEnabled(newRelation != null && needsFixing( newRelation)); @@ -59,5 +65,5 @@ return !isIncomplete(rel) && !getFixer(rel).isRelationGood(rel); } - + private RelationFixer getFixer( Relation rel ) { for(RelationFixer fixer : fixers) Index: /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/AssociatedStreetFixer.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/AssociatedStreetFixer.java (revision 28702) +++ /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/AssociatedStreetFixer.java (revision 28703) @@ -1,3 +1,5 @@ package relcontext.relationfix; + +import static org.openstreetmap.josm.tools.I18n.tr; import java.util.ArrayList; @@ -15,6 +17,4 @@ import org.openstreetmap.josm.data.osm.Way; -import static org.openstreetmap.josm.tools.I18n.tr; - public class AssociatedStreetFixer extends RelationFixer { @@ -26,13 +26,20 @@ public boolean isRelationGood(Relation rel) { for (RelationMember m : rel.getMembers()) { - if (m.getType().equals(OsmPrimitiveType.NODE) && !"house".equals(m.getRole())) + if (m.getType().equals(OsmPrimitiveType.NODE) && !"house".equals(m.getRole())) { + setWarningMessage(tr("Node without 'house' role found")); return false; - if (m.getType().equals(OsmPrimitiveType.WAY) && !("house".equals(m.getRole()) || "street".equals(m.getRole()))) + } + if (m.getType().equals(OsmPrimitiveType.WAY) && !("house".equals(m.getRole()) || "street".equals(m.getRole()))) { + setWarningMessage(tr("Way without 'house' or 'street' role found")); + return false; + } + if (m.getType().equals(OsmPrimitiveType.RELATION) && !"house".equals(m.getRole())) { + setWarningMessage(tr("Relation without 'house' role found")); return false; - if (m.getType().equals(OsmPrimitiveType.RELATION) && !"house".equals(m.getRole())) - return false; + } } // relation should have name if (!rel.hasKey("name")) { + setWarningMessage(tr("Relation does not have name")); return false; } @@ -40,11 +47,14 @@ String streetName = rel.get("name"); for (RelationMember m : rel.getMembers()) { - if (m.getRole().equals("street") && !m.getWay().get("name").equals(streetName)) - return false; + if (m.getRole().equals("street") && !m.getWay().get("name").equals(streetName)) { + setWarningMessage(tr("Relation has streets with different names")); + return false; + } } + clearWarningMessage(); return true; } - @Override + @Override public Command fixRelation(Relation source) { // any way with highway tag -> street @@ -54,11 +64,11 @@ Relation rel = new Relation(source); boolean fixed = false; - + for (int i = 0; i < rel.getMembersCount(); i++) { RelationMember m = rel.getMember(i); - + if (m.isNode()) { Node node = m.getNode(); - if (!"house".equals(m.getRole()) && + if (!"house".equals(m.getRole()) && (node.hasKey("building") || node.hasKey("addr:housenumber"))) { fixed = true; @@ -70,5 +80,5 @@ fixed = true; rel.setMember(i, new RelationMember("street", way)); - } else if (!"house".equals(m.getRole()) && + } else if (!"house".equals(m.getRole()) && (way.hasKey("building") || way.hasKey("addr:housenumber"))) { fixed = true; @@ -77,5 +87,5 @@ } else if (m.isRelation()) { Relation relation = m.getRelation(); - if (!"house".equals(m.getRole()) && + if (!"house".equals(m.getRole()) && (relation.hasKey("building") || relation.hasKey("addr:housenumber") || "multipolygon".equals(relation.get("type")))) { fixed = true; @@ -84,14 +94,14 @@ } } - + // fill relation name Map streetNames = new HashMap(); - for (RelationMember m : rel.getMembers()) + for (RelationMember m : rel.getMembers()) if ("street".equals(m.getRole()) && m.isWay()) { String name = m.getWay().get("name"); if (name == null || name.isEmpty()) continue; - + Integer count = streetNames.get(name); - + streetNames.put(name, count != null? count + 1 : 1); } @@ -104,6 +114,6 @@ } } - - if (!rel.hasKey("name")) { + + if (!rel.hasKey("name") && !commonName.isEmpty()) { fixed = true; rel.put("name", commonName); @@ -111,23 +121,23 @@ commonName = ""; // set empty common name - if we already have name on relation, do not overwrite it } - + List commandList = new ArrayList(); if (fixed) { commandList.add(new ChangeCommand(source, rel)); } - + /*if (!commonName.isEmpty()) // fill common name to streets - for (RelationMember m : rel.getMembers()) + for (RelationMember m : rel.getMembers()) if ("street".equals(m.getRole()) && m.isWay()) { String name = m.getWay().get("name"); if (commonName.equals(name)) continue; - + // TODO: ask user if he really wants to overwrite street name?? - + Way oldWay = m.getWay(); Way newWay = new Way(oldWay); newWay.put("name", commonName); - + commandList.add(new ChangeCommand(oldWay, newWay)); } Index: /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/BoundaryFixer.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/BoundaryFixer.java (revision 28702) +++ /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/BoundaryFixer.java (revision 28703) @@ -1,3 +1,5 @@ package relcontext.relationfix; + +import static org.openstreetmap.josm.tools.I18n.tr; import org.openstreetmap.josm.command.ChangeCommand; @@ -15,9 +17,9 @@ public BoundaryFixer() { - super(new String[]{"boundary", "multipolygon"}); + super("boundary", "multipolygon"); } - + /** - * For boundary relations both "boundary" and "multipolygon" types are applicable, but + * For boundary relations both "boundary" and "multipolygon" types are applicable, but * it should also have key boundary=administrative to be fully boundary. * @see http://wiki.openstreetmap.org/wiki/Relation:boundary @@ -27,15 +29,22 @@ return super.isFixerApplicable(rel) && "administrative".equals(rel.get("boundary")); } - + @Override public boolean isRelationGood(Relation rel) { for( RelationMember m : rel.getMembers() ) { - if (m.getType().equals(OsmPrimitiveType.RELATION) && !"subarea".equals(m.getRole())) + if (m.getType().equals(OsmPrimitiveType.RELATION) && !"subarea".equals(m.getRole())) { + setWarningMessage(tr("Relation without 'subarea' role found")); return false; - if (m.getType().equals(OsmPrimitiveType.NODE) && !("label".equals(m.getRole()) || "admin_centre".equals(m.getRole()))) + } + if (m.getType().equals(OsmPrimitiveType.NODE) && !("label".equals(m.getRole()) || "admin_centre".equals(m.getRole()))) { + setWarningMessage(tr("Node without 'label' or 'admin_centre' role found")); return false; - if (m.getType().equals(OsmPrimitiveType.WAY) && !("outer".equals(m.getRole()) || "inner".equals(m.getRole()))) + } + if (m.getType().equals(OsmPrimitiveType.WAY) && !("outer".equals(m.getRole()) || "inner".equals(m.getRole()))) { + setWarningMessage(tr("Way without 'inner' or 'outer' role found")); return false; + } } + clearWarningMessage(); return true; } Index: /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/MultipolygonFixer.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/MultipolygonFixer.java (revision 28702) +++ /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/MultipolygonFixer.java (revision 28703) @@ -1,3 +1,5 @@ package relcontext.relationfix; + +import static org.openstreetmap.josm.tools.I18n.tr; import java.util.ArrayList; @@ -23,6 +25,6 @@ super("multipolygon"); } - - protected MultipolygonFixer(String[] types) { + + protected MultipolygonFixer(String...types) { super(types); } @@ -32,6 +34,9 @@ public boolean isRelationGood(Relation rel) { for (RelationMember m : rel.getMembers()) - if (m.getType().equals(OsmPrimitiveType.WAY) && !("outer".equals(m.getRole()) || "inner".equals(m.getRole()))) - return false; + if (m.getType().equals(OsmPrimitiveType.WAY) && !("outer".equals(m.getRole()) || "inner".equals(m.getRole()))) { + setWarningMessage(tr("Way without 'inner' or 'outer' role found")); + return false; + } + clearWarningMessage(); return true; } Index: /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/RelationFixer.java =================================================================== --- /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/RelationFixer.java (revision 28702) +++ /applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/RelationFixer.java (revision 28703) @@ -1,41 +1,38 @@ package relcontext.relationfix; + +import static org.openstreetmap.josm.tools.I18n.tr; import java.util.ArrayList; import java.util.List; +import javax.swing.Action; + import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.data.osm.Relation; +import relcontext.actions.SortAndFixAction; + public abstract class RelationFixer { - + private List applicableTypes; - - { - applicableTypes = new ArrayList(); - } + private SortAndFixAction sortAndFixAction; + /** - * Construct new RelationFixer by only one applicable relation type - * @param type - */ - public RelationFixer(String type) { - applicableTypes.add(type); - } - - /** - * Construct new RelationFixer by an array of applicable types + * Construct new RelationFixer by a list of applicable types * @param types */ - public RelationFixer(String[] types) { + public RelationFixer(String... types) { + applicableTypes = new ArrayList(); for(String type: types) { applicableTypes.add(type); } } - + /** * Check if given relation is of needed type. You may override this method to check first type * and then check desired relation properties. * Note that this only verifies if current RelationFixer can be used to check and fix given relation - * Deeper relation checking is at {@link isRelationGood} - * + * Deeper relation checking is at {@link isRelationGood} + * * @param rel Relation to check * @return true if relation can be verified by current RelationFixer @@ -46,29 +43,44 @@ if (!rel.hasKey("type")) return false; - + String type = rel.get("type"); for(String oktype: applicableTypes) - if (oktype.equals(type)) + if (oktype.equals(type)) return true; - + return false; } - + /** * Check if given relation is OK. That means if all roles are given properly, all tags exist as expected etc. * Should be written in children classes. - * + * * @param rel Relation to verify * @return true if given relation is OK */ public abstract boolean isRelationGood(Relation rel); - + /** * Fix relation and return new relation with fixed tags, roles etc. * Note that is not obligatory to return true for isRelationGood for new relation - * + * * @param rel Relation to fix * @return command that fixes the relation {@code null} if it cannot be fixed or is already OK */ public abstract Command fixRelation(Relation rel); + + public void setFixAction(SortAndFixAction sortAndFixAction) { + this.sortAndFixAction = sortAndFixAction; + } + protected void setWarningMessage(String text) { + if (text == null) { + clearWarningMessage(); + } else { + sortAndFixAction.putValue(Action.SHORT_DESCRIPTION, text); + } + } + protected void clearWarningMessage() { + sortAndFixAction.putValue(Action.SHORT_DESCRIPTION, tr("Fix roles of the chosen relation members")); + } + }