Changeset 17429 in josm for trunk/src/org/openstreetmap

Timestamp:

2020-12-30T11:52:34+01:00 (4 years ago)

Author:

GerdP

Message:

fix #20325: Update Multipolygon removes tags instead of moving them to relation

• rewrite handling of update multipolygon cases
• let removeTagsFromWaysIfNeeded() check if getDataset() returns null instead of checking isNew(). I assume it was always meant to work like this. JoinAreasAction works fine with that and I hope no plugin relies on the old behaviour.
• add regression unit test and more unit tests to improve coverage

File:

• trunk/src/org/openstreetmap/josm/actions/CreateMultipolygonAction.java (modified) (4 diffs)

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

@@ -341 +341 @@
         boolean changedMembers = rr.a != rr.b;
         final Relation existingRelation = rr.a;
-        final Relation relation = changedMembers ? rr.b : new Relation(rr.a);
+        final Relation relation = changedMembers ? rr.b : rr.a;
 
         final List<Command> list = removeTagsFromWaysIfNeeded(relation);
@@ -349 +349 @@
             commandName = getName(false);
         } else {
-            boolean changedKeys = !relation.getKeys().equals(existingRelation.getKeys());
-            if (changedKeys && changedMembers)
-                list.add(new ChangeCommand(existingRelation, relation));
-            else if (changedMembers) {
-                list.add(new ChangeMembersCommand(existingRelation, new ArrayList<>(relation.getMembers())));
-            } else if (changedKeys) {
-                list.add(ChangePropertyCommand.build(existingRelation, relation));
+            if (changedMembers) {
+                if (!relation.getKeys().equals(existingRelation.getKeys())) {
+                    list.add(new ChangeCommand(existingRelation, relation));
+                } else {
+                    list.add(new ChangeMembersCommand(existingRelation, new ArrayList<>(relation.getMembers())));
+                }
             }
             if (list.isEmpty()) {
-                if (!changedMembers) {
-                    MultipolygonTest mpTest = new MultipolygonTest();
-                    mpTest.visit(existingRelation);
-                    if (!mpTest.getErrors().isEmpty()) {
-                        showErrors(mpTest.getErrors());
-                        return null;
-                    }
+                MultipolygonTest mpTest = new MultipolygonTest();
+                mpTest.visit(existingRelation);
+                if (!mpTest.getErrors().isEmpty()) {
+                    showErrors(mpTest.getErrors());
+                    return null;
                 }
 
@@ -404 +401 @@
      * This method removes tags/value pairs from inner and outer ways and put them on relation if necessary.
      * Function was extended in reltoolbox plugin by Zverikk and copied back to the core
-     * @param relation the multipolygon style relation to process. If it is new, the tags might be
+     * @param relation the multipolygon style relation to process. If it not linked to a dataset, the tags might be
      * modified, else the list of commands will contain a command to modify its tags
      * @return a list of commands to execute
@@ -488 +485 @@
         if (moveTags && !values.isEmpty()) {
             // add those tag values to the relation
-            boolean fixed = false;
             Map<String, String> tagsToAdd = new HashMap<>();
             for (Entry<String, String> entry : values.entrySet()) {
                 String key = entry.getKey();
                 if (!relation.hasKey(key)) {
-                    if (relation.isNew())
+                    if (relation.getDataSet() == null)
                         relation.put(key, entry.getValue());
                     else
                         tagsToAdd.put(key, entry.getValue());
-                    fixed = true;
-                }
-            }
-            if (fixed && !relation.isNew()) {
-                DataSet ds = relation.getDataSet();
-                if (ds == null) {
-                    ds = MainApplication.getLayerManager().getEditDataSet();
-                }
-                commands.add(new ChangePropertyCommand(ds, Collections.singleton(relation), tagsToAdd));
+                }
+            }
+            if (!tagsToAdd.isEmpty()) {
+                commands.add(new ChangePropertyCommand(Collections.singleton(relation), tagsToAdd));
             }
         }