Stability of created primitive IDs

[Don-vip]

This patch should add stability to negative ids of created primitives, when reloading an .osm file.
It needs further tests before merging.

This is needed to edit our internal boundaries file (see #14833, #15036, #15870) and saving command stack state to session files (#12726).

src/org/openstreetmap/josm/io/AbstractReader.java

@@ -13 +13 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.OptionalLong;
 import java.util.function.Consumer;
 
 import org.openstreetmap.josm.data.Bounds;
@@ -322 +323 @@
         } catch (IOException e) {
             throw new IllegalDataException(e);
         } finally {
+            OptionalLong minId = externalIdMap.values().stream().mapToLong(AbstractPrimitive::getUniqueId).min();
+            if (minId.isPresent() && minId.getAsLong() < AbstractPrimitive.currentUniqueId()) {
+                AbstractPrimitive.advanceUniqueId(minId.getAsLong());
+            }
             progressMonitor.finishTask();
             progressMonitor.removeCancelListener(cancelListener);
         }
@@ -603 +608 @@
         return !Double.isNaN(lat) && !Double.isNaN(lon);
     }
 
+    @SuppressWarnings("unchecked")
+    private <T extends OsmPrimitive> T buildPrimitive(PrimitiveData pd) {
+        OsmPrimitive p;
+        if (pd.getUniqueId() < AbstractPrimitive.currentUniqueId()) {
+            p = pd.getType().newInstance(pd.getUniqueId(), true);
+        } else {
+            p = pd.getType().newVersionedInstance(pd.getId(), pd.getVersion());
+        }
+        p.setVisible(pd.isVisible());
+        p.load(pd);
+        externalIdMap.put(pd.getPrimitiveId(), p);
+        return (T) p;
+    }
+
     private Node addNode(NodeData nd, NodeReader nodeReader) throws IllegalDataException {
-        Node n = new Node(nd.getId(), nd.getVersion());
-        n.setVisible(nd.isVisible());
-        n.load(nd);
+        Node n = buildPrimitive(nd);
         nodeReader.accept(n);
-        externalIdMap.put(nd.getPrimitiveId(), n);
         return n;
     }
 
     protected final Node parseNode(double lat, double lon, CommonReader commonReader, NodeReader nodeReader)
             throws IllegalDataException {
-        NodeData nd = new NodeData();
+        NodeData nd = new NodeData(0);
         LatLon ll = null;
         if (areLatLonDefined(lat, lon)) {
             try {
@@ -653 +669 @@
     }
 
     protected final Way parseWay(CommonReader commonReader, WayReader wayReader) throws IllegalDataException {
-        WayData wd = new WayData();
+        WayData wd = new WayData(0);
         commonReader.accept(wd);
-        Way w = new Way(wd.getId(), wd.getVersion());
-        w.setVisible(wd.isVisible());
-        w.load(wd);
-        externalIdMap.put(wd.getPrimitiveId(), w);
+        Way w = buildPrimitive(wd);
 
         Collection<Long> nodeIds = new ArrayList<>();
         wayReader.accept(w, nodeIds);
@@ -671 +684 @@
     }
 
     protected final Relation parseRelation(CommonReader commonReader, RelationReader relationReader) throws IllegalDataException {
-        RelationData rd = new RelationData();
+        RelationData rd = new RelationData(0);
         commonReader.accept(rd);
-        Relation r = new Relation(rd.getId(), rd.getVersion());
-        r.setVisible(rd.isVisible());
-        r.load(rd);
-        externalIdMap.put(rd.getPrimitiveId(), r);
+        Relation r = buildPrimitive(rd);
 
         Collection<RelationMemberData> members = new ArrayList<>();
         relationReader.accept(r, members);

[stoecker]

I have trouble understanding the process to keep ID stable from the code above. Can you please explain the algorithms with words?

What happens when joining two loaded files and in similar more complex cases?

[Don-vip]

Honestly I don't remember right now it's an update of a patch I made years ago maybe I forgot one part. I'll check it after Karlsruhe.

[Don-vip]

In 14535/josm:

see #16073 - check response contents
see #16854 - stability of created primitive IDs (accidental commit...)

[Don-vip]

I've accidentally committed the changes. Let's see tomorrow if it causes problems I didn't see in my earlier tests.

[Don-vip]

In 14543/josm:

see #16854 - fix java warning

[Don-vip]

I've done a few tests and JOSM behaves OK. Plus, no-one complained about weird issues, so the code seems OK.