Index: /trunk/src/org/openstreetmap/josm/actions/CreateMultipolygonAction.java =================================================================== --- /trunk/src/org/openstreetmap/josm/actions/CreateMultipolygonAction.java (revision 6563) +++ /trunk/src/org/openstreetmap/josm/actions/CreateMultipolygonAction.java (revision 6564) @@ -10,4 +10,5 @@ import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; @@ -33,5 +34,7 @@ import org.openstreetmap.josm.gui.Notification; import org.openstreetmap.josm.gui.dialogs.relation.RelationEditor; +import org.openstreetmap.josm.tools.Pair; import org.openstreetmap.josm.tools.Shortcut; +import org.openstreetmap.josm.tools.Utils; /** @@ -56,5 +59,5 @@ */ public CreateMultipolygonAction() { - super(tr("Create multipolygon"), "multipoly_create", tr("Create multipolygon."), + super(tr("Create multipolygon"), "multipoly_create", tr("Create multipolygon"), Shortcut.registerShortcut("tools:multipoly", tr("Tool: {0}", tr("Create multipolygon")), KeyEvent.VK_A, Shortcut.ALT_CTRL), true); @@ -76,5 +79,6 @@ } - Collection selectedWays = Main.main.getCurrentDataSet().getSelectedWays(); + final Collection selectedWays = Main.main.getCurrentDataSet().getSelectedWays(); + final Collection selectedRelations = Main.main.getCurrentDataSet().getSelectedRelations(); if (selectedWays.size() < 1) { @@ -89,49 +93,85 @@ } - MultipolygonCreate polygon = this.analyzeWays(selectedWays); - - if (polygon == null) - return; //could not make multipolygon. - - final Relation relation = this.createRelation(polygon); - - if (Main.pref.getBoolean("multipoly.show-relation-editor", false)) { - //Open relation edit window, if set up in preferences - RelationEditor editor = RelationEditor.getEditor(Main.main.getEditLayer(), relation, null); - - editor.setModal(true); - editor.setVisible(true); - - //TODO: cannot get the resulting relation from RelationEditor :(. - /* - if (relationCountBefore < relationCountAfter) { - //relation saved, clean up the tags - List list = this.removeTagsFromInnerWays(relation); - if (list.size() > 0) - { - Main.main.undoRedo.add(new SequenceCommand(tr("Remove tags from multipolygon inner ways"), list)); - } - } - */ - + final Pair commandAndRelation = createMultipolygonCommand(selectedWays, selectedRelations); + final Command command = commandAndRelation.a; + final Relation relation = commandAndRelation.b; + if (command == null) { + return; + } + Main.main.undoRedo.add(command); + + // Use 'SwingUtilities.invokeLater' to make sure the relationListDialog + // knows about the new relation before we try to select it. + // (Yes, we are already in event dispatch thread. But DatasetEventManager + // uses 'SwingUtilities.invokeLater' to fire events so we have to do + // the same.) + SwingUtilities.invokeLater(new Runnable() { + @Override + public void run() { + Main.map.relationListDialog.selectRelation(relation); + if (Main.pref.getBoolean("multipoly.show-relation-editor", false)) { + //Open relation edit window, if set up in preferences + RelationEditor editor = RelationEditor.getEditor(Main.main.getEditLayer(), relation, null); + + editor.setModal(true); + editor.setVisible(true); + } + } + }); + + } + + private static Relation getSelectedMultipolygonRelation(Collection selectedRelations) { + return selectedRelations.size() == 1 && "multipolygon".equals(selectedRelations.iterator().next().get("type")) + ? selectedRelations.iterator().next() + : null; + } + + /** + * Returns a {@link Pair} of the old multipolygon {@link Relation} (or null) and the newly created/modified multipolygon {@link Relation}. + */ + public static Pair createMultipolygonRelation(Collection selectedWays, Collection selectedRelations) { + + final Relation selectedMultipolygonRelation = getSelectedMultipolygonRelation(selectedRelations); + if (selectedMultipolygonRelation != null) { + // add ways of existing relation to include them in polygon analysis + selectedWays = new HashSet(selectedWays); + selectedWays.addAll(Utils.filteredCollection(selectedMultipolygonRelation.getMemberPrimitives(), Way.class)); + } + + final MultipolygonCreate polygon = analyzeWays(selectedWays); + if (polygon == null) { + return null; //could not make multipolygon. + } + + if (selectedMultipolygonRelation != null) { + return Pair.create(selectedMultipolygonRelation, createRelation(polygon, new Relation(selectedMultipolygonRelation))); } else { - //Just add the relation - List list = this.removeTagsFromWaysIfNeeded(relation); + return Pair.create(null, createRelation(polygon, new Relation())); + } + } + + /** + * Returns a pair of a multipolygon creating/modifying {@link Command} as well as the multipolygon {@link Relation}. + */ + public static Pair createMultipolygonCommand(Collection selectedWays, Collection selectedRelations) { + + final Pair rr = createMultipolygonRelation(selectedWays, selectedRelations); + if (rr == null) { + return null; + } + final Relation existingRelation = rr.a; + final Relation relation = rr.b; + + final List list = removeTagsFromWaysIfNeeded(relation); + final String commandName; + if (existingRelation == null) { list.add(new AddCommand(relation)); - Main.main.undoRedo.add(new SequenceCommand(tr("Create multipolygon"), list)); - // Use 'SwingUtilities.invokeLater' to make sure the relationListDialog - // knows about the new relation before we try to select it. - // (Yes, we are already in event dispatch thread. But DatasetEventManager - // uses 'SwingUtilities.invokeLater' to fire events so we have to do - // the same.) - SwingUtilities.invokeLater(new Runnable() { - @Override - public void run() { - Main.map.relationListDialog.selectRelation(relation); - } - }); - } - - + commandName = tr("Create multipolygon"); + } else { + list.add(new ChangeCommand(existingRelation, relation)); + commandName = tr("Update multipolygon"); + } + return Pair.create(new SequenceCommand(commandName, list), relation); } @@ -152,4 +192,8 @@ @Override protected void updateEnabledState(Collection < ? extends OsmPrimitive > selection) { setEnabled(selection != null && !selection.isEmpty()); + putValue(NAME, getSelectedMultipolygonRelation(getCurrentDataSet().getSelectedRelations()) != null + ? tr("Update multipolygon") + : tr("Create multipolygon") + ); } @@ -159,5 +203,5 @@ * @return null, if there was a problem with the ways. */ - private MultipolygonCreate analyzeWays(Collection < Way > selectedWays) { + private static MultipolygonCreate analyzeWays(Collection < Way > selectedWays) { MultipolygonCreate pol = new MultipolygonCreate(); @@ -179,21 +223,31 @@ * @return multipolygon relation */ - private Relation createRelation(MultipolygonCreate pol) { + private static Relation createRelation(MultipolygonCreate pol, final Relation rel) { // Create new relation - Relation rel = new Relation(); rel.put("type", "multipolygon"); // Add ways to it for (JoinedPolygon jway:pol.outerWays) { - for (Way way:jway.ways) { - rel.addMember(new RelationMember("outer", way)); - } + addMembers(jway, rel, "outer"); } for (JoinedPolygon jway:pol.innerWays) { - for (Way way:jway.ways) { - rel.addMember(new RelationMember("inner", way)); - } + addMembers(jway, rel, "inner"); } return rel; + } + + private static void addMembers(JoinedPolygon polygon, Relation rel, String role) { + final int count = rel.getMembersCount(); + final HashSet ways = new HashSet(polygon.ways); + for (int i = 0; i < count; i++) { + final RelationMember m = rel.getMember(i); + if (ways.contains(m.getMember()) && !role.equals(m.getRole())) { + rel.setMember(i, new RelationMember(role, m.getMember())); + } + } + ways.removeAll(rel.getMemberPrimitives()); + for (final Way way : ways) { + rel.addMember(new RelationMember(role, way)); + } } @@ -206,5 +260,5 @@ * @return a list of commands to execute */ - private List removeTagsFromWaysIfNeeded( Relation relation ) { + private static List removeTagsFromWaysIfNeeded( Relation relation ) { Map values = new HashMap(); Index: /trunk/src/org/openstreetmap/josm/data/osm/Relation.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/Relation.java (revision 6563) +++ /trunk/src/org/openstreetmap/josm/data/osm/Relation.java (revision 6564) @@ -5,4 +5,5 @@ import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -14,4 +15,5 @@ import org.openstreetmap.josm.data.osm.visitor.Visitor; import org.openstreetmap.josm.tools.CopyList; +import org.openstreetmap.josm.tools.Predicate; import org.openstreetmap.josm.tools.Utils; @@ -319,21 +321,5 @@ */ public void removeMembersFor(OsmPrimitive primitive) { - if (primitive == null) - return; - - boolean locked = writeLock(); - try { - List todelete = new ArrayList(); - for (RelationMember member: members) { - if (member.getMember() == primitive) { - todelete.add(member); - } - } - List members = getMembers(); - members.removeAll(todelete); - setMembers(members); - } finally { - writeUnlock(locked); - } + removeMembersFor(Collections.singleton(primitive)); } @@ -356,4 +342,17 @@ /** + * Obtains all members with member.member == primitive + * @param primitives the primitives to check for + */ + public Collection getMembersFor(final Collection primitives) { + return Utils.filter(getMembers(), new Predicate() { + @Override + public boolean evaluate(RelationMember member) { + return primitives.contains(member.getMember()); + } + }); + } + + /** * removes all members with member.member == primitive * @@ -367,12 +366,6 @@ boolean locked = writeLock(); try { - List todelete = new ArrayList(); - for (RelationMember member: members) { - if (primitives.contains(member.getMember())) { - todelete.add(member); - } - } List members = getMembers(); - members.removeAll(todelete); + members.removeAll(getMembersFor(primitives)); setMembers(members); } finally { Index: /trunk/test/data/create_multipolygon.osm =================================================================== --- /trunk/test/data/create_multipolygon.osm (revision 6564) +++ /trunk/test/data/create_multipolygon.osm (revision 6564) @@ -0,0 +1,60 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Index: /trunk/test/unit/org/openstreetmap/josm/actions/CreateMultipolygonActionTest.groovy =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/actions/CreateMultipolygonActionTest.groovy (revision 6564) +++ /trunk/test/unit/org/openstreetmap/josm/actions/CreateMultipolygonActionTest.groovy (revision 6564) @@ -0,0 +1,63 @@ +package org.openstreetmap.josm.actions + +import org.openstreetmap.TestUtils +import org.openstreetmap.josm.Main +import org.openstreetmap.josm.actions.search.SearchCompiler +import org.openstreetmap.josm.data.osm.Relation +import org.openstreetmap.josm.data.osm.Way +import org.openstreetmap.josm.data.projection.Projections +import org.openstreetmap.josm.io.OsmReader +import org.openstreetmap.josm.tools.Utils + +class CreateMultipolygonActionTest extends GroovyTestCase { + + @Override + void setUp() { + Main.initApplicationPreferences() + Main.setProjection(Projections.getProjectionByCode("EPSG:3857")); + } + + def getRefToRoleMap(Relation relation) { + def refToRole = new TreeMap() + for (i in relation.getMembers()) { + refToRole.put(i.member.get("ref"), i.role); + } + return refToRole; + } + + void testCreate1() { + def ds = OsmReader.parseDataSet(new FileInputStream(TestUtils.getTestDataRoot() + "create_multipolygon.osm"), null); + def mp = CreateMultipolygonAction.createMultipolygonCommand(ds.getWays(), Collections.emptyList()) + assert mp.a.getDescriptionText() == "Sequence: Create multipolygon" + assert getRefToRoleMap(mp.b).toString() == "[1:outer, 1.1:inner, 1.1.1:outer, 1.1.2:outer, 1.2:inner]" + } + + void testCreate2() { + def ds = OsmReader.parseDataSet(new FileInputStream(TestUtils.getTestDataRoot() + "create_multipolygon.osm"), null); + def ways = Utils.filter(ds.getWays(), SearchCompiler.compile("ref=1 OR ref:1.1.", false, false)) + def mp = CreateMultipolygonAction.createMultipolygonCommand(ways as Collection, Collections.emptyList()) + assert getRefToRoleMap(mp.b).toString() == "[1:outer, 1.1.1:inner, 1.1.2:inner]" + } + + void testUpdate1() { + def ds = OsmReader.parseDataSet(new FileInputStream(TestUtils.getTestDataRoot() + "create_multipolygon.osm"), null); + def ways = Utils.filter(ds.getWays(), SearchCompiler.compile("ref=\".*1\$\"", false, true)) + def mp = CreateMultipolygonAction.createMultipolygonCommand(ways as Collection, Collections.emptyList()) + assert mp.b.getMembersCount() == 3 + assert getRefToRoleMap(mp.b).toString() == "[1:outer, 1.1:inner, 1.1.1:outer]" + def ways2 = Utils.filter(ds.getWays(), SearchCompiler.compile("ref=1.2", false, true)) + def mp2 = CreateMultipolygonAction.createMultipolygonCommand(ways2 as Collection, Collections.singleton(mp.b)) + assert mp2.b.getMembersCount() == 4 + assert getRefToRoleMap(mp2.b).toString() == "[1:outer, 1.1:inner, 1.1.1:outer, 1.2:inner]" + } + + void testUpdate2() { + def ds = OsmReader.parseDataSet(new FileInputStream(TestUtils.getTestDataRoot() + "create_multipolygon.osm"), null); + def ways = Utils.filter(ds.getWays(), SearchCompiler.compile("ref=1 OR ref:1.1.1", false, false)) + def mp = CreateMultipolygonAction.createMultipolygonCommand(ways as Collection, Collections.emptyList()) + assert getRefToRoleMap(mp.b).toString() == "[1:outer, 1.1.1:inner]" + def ways2 = Utils.filter(ds.getWays(), SearchCompiler.compile("ref=1.1 OR ref=1.2 OR ref=1.1.2", false, true)) + def mp2 = CreateMultipolygonAction.createMultipolygonCommand(ways2 as Collection, Collections.singleton(mp.b)) + assert getRefToRoleMap(mp2.b).toString() == "[1:outer, 1.1:inner, 1.1.1:outer, 1.1.2:outer, 1.2:inner]" + } +}