Changeset 17406 in josm

Timestamp:

2020-12-13T16:04:00+01:00 (3 years ago)

Author:

GerdP

Message:

fix #20230: update multipolygon action removes tags from ways without adding them to relation

• create ChangeCommand if tags of updated relation were changed

Todo: refactoring to remove class CreateUpdateMultipolygonTask

Location:

trunk

Files:

• src/org/openstreetmap/josm/actions/CreateMultipolygonAction.java (modified) (3 diffs)
• test/data/regress/20230 (added)
• test/data/regress/20230/data.osm (added)
• test/unit/org/openstreetmap/josm/actions/CreateMultipolygonActionTest.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/actions/CreateMultipolygonAction.java

@@ -26 +26 @@
 import org.openstreetmap.josm.actions.relation.DownloadSelectedIncompleteMembersAction;
 import org.openstreetmap.josm.command.AddCommand;
+import org.openstreetmap.josm.command.ChangeCommand;
 import org.openstreetmap.josm.command.ChangeMembersCommand;
 import org.openstreetmap.josm.command.ChangePropertyCommand;
@@ -112 +113 @@
                 return;
             }
-            final Command command = commandAndRelation.a;
 
             // to avoid EDT violations
             SwingUtilities.invokeLater(() -> {
-                if (multipolygonRelation != null) {
-                    // rather ugly: update generated a ChangeMembersCommand with a copy of the member list, so clear the list now
-                    commandAndRelation.b.setMembers(null); // #see 19885
-                }
-                UndoRedoHandler.getInstance().add(command);
+                UndoRedoHandler.getInstance().add(commandAndRelation.a);
+                Relation calculatedRel = commandAndRelation.b;
+                if (calculatedRel.getDataSet() == null) {
+                    calculatedRel.setMembers(null); // see #19885
+                }
                 final Relation relation = (Relation) MainApplication.getLayerManager().getEditDataSet()
-                        .getPrimitiveById(commandAndRelation.b);
+                        .getPrimitiveById(calculatedRel);
                 if (relation == null || relation.getDataSet() == null)
                     return; // should not happen
@@ -350 +350 @@
         } else {
             if (!unchanged) {
-                list.add(new ChangeMembersCommand(existingRelation, new ArrayList<>(relation.getMembers())));
+                if (relation.getKeys().equals(existingRelation.getKeys()))
+                    list.add(new ChangeMembersCommand(existingRelation, new ArrayList<>(relation.getMembers())));
+                else
+                    list.add(new ChangeCommand(existingRelation, relation));
             }
             if (list.isEmpty()) {

trunk/test/unit/org/openstreetmap/josm/actions/CreateMultipolygonActionTest.java

@@ -174 +174 @@
         assertEquals(numCoastlineWays, ds.getWays().stream().filter(w -> "coastline".equals(w.get("natural"))).count());
     }
+
+    /**
+     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/20230">Bug #20230</a>.
+     * @throws Exception if an error occurs
+     */
+    @Test
+    void testTicket20230() throws Exception {
+        DataSet ds = OsmReader.parseDataSet(TestUtils.getRegressionDataStream(20230, "data.osm"), null);
+        assertEquals(1, ds.getRelations().size());
+        Relation mp = ds.getRelations().iterator().next();
+        Relation modMp = createMultipolygon(ds.getWays(), "type:way", mp, true);
+        assertNotNull(modMp);
+        assertEquals(1, ds.getRelations().size());
+        modMp = ds.getRelations().iterator().next();
+        assertTrue(modMp.hasTag("building", "yes"));
+        assertEquals(0, ds.getWays().stream().filter(w -> w.hasTag("building", "yes")).count());
+    }
 }