Ticket #20091: 20091-full.patch

src/org/openstreetmap/josm/actions/relation/DownloadMembersAction.java

@@ -6 +6 @@
 
 import java.awt.event.ActionEvent;
 import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
 
 import org.openstreetmap.josm.data.osm.IPrimitive;
-import org.openstreetmap.josm.data.osm.Relation;
+import org.openstreetmap.josm.data.osm.PrimitiveId;
 import org.openstreetmap.josm.gui.MainApplication;
-import org.openstreetmap.josm.gui.dialogs.relation.DownloadRelationTask;
+import org.openstreetmap.josm.gui.io.DownloadPrimitivesTask;
 import org.openstreetmap.josm.tools.ImageProvider;
-import org.openstreetmap.josm.tools.SubclassFilteredCollection;
-import org.openstreetmap.josm.tools.Utils;
 
 /**
  * The action for downloading members of relations
@@ -34 +34 @@
     @Override
     public void actionPerformed(ActionEvent e) {
         if (!isEnabled() || relations.isEmpty() || !MainApplication.isDisplayingMapView()) return;
-        MainApplication.worker.submit(new DownloadRelationTask(
-                Utils.filteredCollection(relations, Relation.class), MainApplication.getLayerManager().getEditLayer()));
+        List<PrimitiveId> members = relations.stream()
+                .flatMap(r -> r.getMemberPrimitivesList().stream().filter(osm -> !osm.isNew()).map(IPrimitive::getOsmPrimitiveId))
+                .distinct()
+                .collect(Collectors.toList());
+
+        MainApplication.worker.submit(new DownloadPrimitivesTask(MainApplication.getLayerManager().getEditLayer(), members, false));
     }
 
     @Override
     public void setPrimitives(Collection<? extends IPrimitive> primitives) {
-        // selected non-new relations
-        this.relations = SubclassFilteredCollection.filter(getRelations(primitives), r -> !r.isNew());
+        this.relations = getRelations(primitives);
         updateEnabledState();
     }
 

src/org/openstreetmap/josm/data/osm/DataSetMerger.java

@@ -9 +9 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -63 +64 @@
         conflicts = new ConflictCollection();
         mergedMap = new HashMap<>();
         objectsWithChildrenToMerge = new HashSet<>();
-        objectsToDelete = new HashSet<>();
+        objectsToDelete = new LinkedHashSet<>();
     }
 
     /**
@@ -297 +298 @@
         // found a corresponding target, remember it
         mergedMap.put(source.getPrimitiveId(), target.getPrimitiveId());
 
+        boolean mergeFromSource = false;
         if (target.getVersion() > source.getVersion())
             // target.version > source.version => keep target version
             return true;
 
-        if (target.isIncomplete() && !source.isIncomplete()) {
+        if (!target.isModified() && source.isDeleted()) {
+            // target not modified. We can assume that source is the most recent version,
+            // so mark it to be deleted.
+            //
+            objectsToDelete.add(target);
+        } else if (target.isIncomplete() && !source.isIncomplete()) {
             // target is incomplete, source completes it
             // => merge source into target
             //
-            target.mergeFrom(source);
-            objectsWithChildrenToMerge.add(source.getPrimitiveId());
+            mergeFromSource = true;
         } else if (!target.isIncomplete() && source.isIncomplete()) {
             // target is complete and source is incomplete
             // => keep target, it has more information already
@@ -323 +329 @@
             // We shouldn't merge that datasets.
             throw new DataIntegrityProblemException(tr("Conflict in ''visible'' attribute for object of type {0} with id {1}",
                     target.getType(), target.getId()));
+        } else if (target.isDeleted() && source.isDeleted() && target.getVersion() < source.getVersion()) {
+            // both deleted. Source is newer. Take source. See #19783
+            mergeFromSource = true;
         } else if (target.isDeleted() && !source.isDeleted() && target.getVersion() == source.getVersion()) {
             // same version, but target is deleted. Assume target takes precedence
             // otherwise too many conflicts when refreshing from the server
@@ -339 +348 @@
                     break;
                 }
             }
-        } else if (!target.isModified() && source.isDeleted()) {
-            // target not modified. We can assume that source is the most recent version,
-            // so mark it to be deleted.
-            //
-            objectsToDelete.add(target);
         } else if (!target.isModified() && source.isModified()) {
             // target not modified. We can assume that source is the most recent version.
             // clone it into target.
-            target.mergeFrom(source);
-            objectsWithChildrenToMerge.add(source.getPrimitiveId());
+            mergeFromSource = true;
         } else if (!target.isModified() && !source.isModified() && target.getVersion() == source.getVersion()) {
             // both not modified. Merge nevertheless.
             // This helps when updating "empty" relations, see #4295
-            target.mergeFrom(source);
-            objectsWithChildrenToMerge.add(source.getPrimitiveId());
+            mergeFromSource = true;
         } else if (!target.isModified() && !source.isModified() && target.getVersion() < source.getVersion()) {
             // my not modified but other is newer. clone other onto mine.
             //
-            target.mergeFrom(source);
-            objectsWithChildrenToMerge.add(source.getPrimitiveId());
+            mergeFromSource = true;
         } else if (target.isModified() && !source.isModified() && target.getVersion() == source.getVersion()) {
             // target is same as source but target is modified
             // => keep target and reset modified flag if target and source are semantically equal
@@ -380 +381 @@
             // technical attributes like timestamp or user information. Semantic
             // attributes should already be equal if we get here.
             //
+            mergeFromSource = true;
+        }
+        if (mergeFromSource) {
             target.mergeFrom(source);
             objectsWithChildrenToMerge.add(source.getPrimitiveId());
         }