Ticket #19898: 19898.2.patch

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

@@ -5 +5 @@
 
 import java.awt.geom.Area;
 import java.util.ArrayList;
-import java.util.Collection;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -13 +13 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.openstreetmap.josm.data.DataSource;
 import org.openstreetmap.josm.data.conflict.Conflict;
@@ -78 +79 @@
      * my primitive.
      *
      * @param source the primitive to merge
-     * @param candidates a set of possible candidates for a new primitive
+     * @param mergeNewHelper a class that manages possible candidates for a new primitive
      */
-    protected void mergePrimitive(OsmPrimitive source, Collection<? extends OsmPrimitive> candidates) {
+    private void mergePrimitive(OsmPrimitive source, MergeNewHelper mergeNewHelper) {
         if (!source.isNew()) {
             // try to merge onto a matching primitive with the same defined id
             //
@@ -93 +94 @@
             // try to merge onto a primitive  which has no id assigned
             // yet but which is equal in its semantic attributes
             //
-            for (OsmPrimitive target : candidates) {
+            for (OsmPrimitive target : mergeNewHelper.getCandidates(source)) {
                 if (!target.isNew() || target.isDeleted()) {
-                    continue;
+                    continue; // should not happen
                 }
                 if (target.hasEqualSemanticAttributes(source)) {
                     mergedMap.put(source.getPrimitiveId(), target.getPrimitiveId());
@@ -423 +424 @@
         if (progressMonitor != null) {
             progressMonitor.beginTask(tr("Merging data..."), sourceDataSet.allPrimitives().size());
         }
+
         targetDataSet.update(() -> {
-            List<? extends OsmPrimitive> candidates = null;
-            for (Node node: sourceDataSet.getNodes()) {
-                // lazy initialisation to improve performance, see #19898
-                if (candidates == null) {
-                    candidates = new ArrayList<>(targetDataSet.getNodes());
+            for (Class<? extends OsmPrimitive> clazz : Arrays.asList(Node.class, Way.class, Relation.class)) {
+                MergeNewHelper mergeNewHelper = new MergeNewHelper(targetDataSet);
+                for (OsmPrimitive source: sourceDataSet.getPrimitives(clazz::isInstance)) {
+                    mergePrimitive(source, mergeNewHelper);
+                    if (progressMonitor != null) {
+                        progressMonitor.worked(1);
+                    }
                 }
-                mergePrimitive(node, candidates);
-                if (progressMonitor != null) {
-                    progressMonitor.worked(1);
-                }
             }
-            candidates = null;
-            for (Way way: sourceDataSet.getWays()) {
-                // lazy initialisation to improve performance
-                if (candidates == null) {
-                    candidates = new ArrayList<>(targetDataSet.getWays());
-                }
-                mergePrimitive(way, candidates);
-                if (progressMonitor != null) {
-                    progressMonitor.worked(1);
-                }
-            }
-            candidates = null;
-            for (Relation relation: sourceDataSet.getRelations()) {
-                // lazy initialisation to improve performance
-                if (candidates == null) {
-                    candidates = new ArrayList<>(targetDataSet.getRelations());
-                }
-                mergePrimitive(relation, candidates);
-                if (progressMonitor != null) {
-                    progressMonitor.worked(1);
-                }
-            }
-            candidates = null;
             fixReferences();
 
             Area a = targetDataSet.getDataSourceArea();
@@ -512 +489 @@
     public ConflictCollection getConflicts() {
         return conflicts;
     }
+
+    /**
+     * Helper class to avoid unnecessary calculation of the candidates list for new primitives.
+     *
+     * @author Gerd Petermann
+     *
+     */
+    private static class MergeNewHelper {
+        private final DataSet target;
+        private List<OsmPrimitive> candidates;
+
+        MergeNewHelper(DataSet target) {
+            this.target = target;
+        }
+
+        /**
+         * Get possible merge candidates for a new primitive.
+         * @param primitive the new primitive
+         * @return a list of new and undeleted objects of the same type already existing in the target dataset. Might be empty but it not null.
+         */
+        public List<OsmPrimitive> getCandidates(OsmPrimitive primitive) {
+            if (candidates == null) {
+                candidates = target.getPrimitives(primitive.getClass()::isInstance).stream()
+                        .filter(p -> p.isNew() && !p.isDeleted()).collect(Collectors.toList());
+            }
+            return candidates;
+        }
+
+    }
 }