Modify

Opened 5 months ago

Closed 3 months ago

#16854 closed enhancement (fixed)

Stability of created primitive IDs

Reported by: Don-vip Owned by: Don-vip
Priority: normal Milestone: 18.12
Component: Core Version:
Keywords: Cc:

Description

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

     
    1313import java.util.List;
    1414import java.util.Map;
    1515import java.util.Map.Entry;
     16import java.util.OptionalLong;
    1617import java.util.function.Consumer;
    1718
    1819import org.openstreetmap.josm.data.Bounds;
     
    322323        } catch (IOException e) {
    323324            throw new IllegalDataException(e);
    324325        } finally {
     326            OptionalLong minId = externalIdMap.values().stream().mapToLong(AbstractPrimitive::getUniqueId).min();
     327            if (minId.isPresent() && minId.getAsLong() < AbstractPrimitive.currentUniqueId()) {
     328                AbstractPrimitive.advanceUniqueId(minId.getAsLong());
     329            }
    325330            progressMonitor.finishTask();
    326331            progressMonitor.removeCancelListener(cancelListener);
    327332        }
     
    603608        return !Double.isNaN(lat) && !Double.isNaN(lon);
    604609    }
    605610
     611    @SuppressWarnings("unchecked")
     612    private <T extends OsmPrimitive> T buildPrimitive(PrimitiveData pd) {
     613        OsmPrimitive p;
     614        if (pd.getUniqueId() < AbstractPrimitive.currentUniqueId()) {
     615            p = pd.getType().newInstance(pd.getUniqueId(), true);
     616        } else {
     617            p = pd.getType().newVersionedInstance(pd.getId(), pd.getVersion());
     618        }
     619        p.setVisible(pd.isVisible());
     620        p.load(pd);
     621        externalIdMap.put(pd.getPrimitiveId(), p);
     622        return (T) p;
     623    }
     624
    606625    private Node addNode(NodeData nd, NodeReader nodeReader) throws IllegalDataException {
    607         Node n = new Node(nd.getId(), nd.getVersion());
    608         n.setVisible(nd.isVisible());
    609         n.load(nd);
     626        Node n = buildPrimitive(nd);
    610627        nodeReader.accept(n);
    611         externalIdMap.put(nd.getPrimitiveId(), n);
    612628        return n;
    613629    }
    614630
    615631    protected final Node parseNode(double lat, double lon, CommonReader commonReader, NodeReader nodeReader)
    616632            throws IllegalDataException {
    617         NodeData nd = new NodeData();
     633        NodeData nd = new NodeData(0);
    618634        LatLon ll = null;
    619635        if (areLatLonDefined(lat, lon)) {
    620636            try {
     
    653669    }
    654670
    655671    protected final Way parseWay(CommonReader commonReader, WayReader wayReader) throws IllegalDataException {
    656         WayData wd = new WayData();
     672        WayData wd = new WayData(0);
    657673        commonReader.accept(wd);
    658         Way w = new Way(wd.getId(), wd.getVersion());
    659         w.setVisible(wd.isVisible());
    660         w.load(wd);
    661         externalIdMap.put(wd.getPrimitiveId(), w);
     674        Way w = buildPrimitive(wd);
    662675
    663676        Collection<Long> nodeIds = new ArrayList<>();
    664677        wayReader.accept(w, nodeIds);
     
    671684    }
    672685
    673686    protected final Relation parseRelation(CommonReader commonReader, RelationReader relationReader) throws IllegalDataException {
    674         RelationData rd = new RelationData();
     687        RelationData rd = new RelationData(0);
    675688        commonReader.accept(rd);
    676         Relation r = new Relation(rd.getId(), rd.getVersion());
    677         r.setVisible(rd.isVisible());
    678         r.load(rd);
    679         externalIdMap.put(rd.getPrimitiveId(), r);
     689        Relation r = buildPrimitive(rd);
    680690
    681691        Collection<RelationMemberData> members = new ArrayList<>();
    682692        relationReader.accept(r, members);

Attachments (0)

Change History (9)

comment:1 Changed 5 months ago by 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?

comment:2 Changed 5 months ago by 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.

comment:3 Changed 5 months ago by Don-vip

Milestone: 18.1018.11

comment:4 Changed 4 months ago by Don-vip

Milestone: 18.1118.12

comment:5 Changed 4 months ago by Don-vip

In 14535/josm:

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

comment:6 Changed 4 months ago by Don-vip

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

comment:7 Changed 4 months ago by Don-vip

In 14543/josm:

see #16854 - fix java warning

comment:8 Changed 4 months ago by Don-vip

Owner: changed from team to Don-vip
Status: newassigned

comment:9 Changed 3 months ago by Don-vip

Resolution: fixed
Status: assignedclosed

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.