Changeset 34826 in osm for applications/editors/josm/plugins

Timestamp:

2019-01-16T09:13:03+01:00 (6 years ago)

Author:

gerdp

Message:

fix #14545 - Open edited pbf file fails (negative IDs)

• use AbstractReader.buildPrimitive()
• add unit test

Location:

applications/editors/josm/plugins/pbf

Files:

• .project (modified) (1 diff)
• src/org/openstreetmap/josm/plugins/pbf/io/PbfReader.java (modified) (11 diffs)
• test/data/regress/14545 (added)
• test/data/regress/14545/reg14545.osm.pbf (added)
• test/unit/org/openstreetmap/josm/plugins/pbf/io/PbfImporterTest.java (modified) (1 diff)

applications/editors/josm/plugins/pbf/.project

@@ -4 +4 @@
         <comment></comment>
         <projects>
+                <project>josm</project>
         </projects>
         <buildSpec>

applications/editors/josm/plugins/pbf/src/org/openstreetmap/josm/plugins/pbf/io/PbfReader.java

@@ -17 +17 @@
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.osm.NodeData;
+import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
+import org.openstreetmap.josm.data.osm.PrimitiveData;
+import org.openstreetmap.josm.data.osm.RelationData;
+import org.openstreetmap.josm.data.osm.RelationMemberData;
 import org.openstreetmap.josm.data.osm.UploadPolicy;
-import org.openstreetmap.josm.data.osm.Node;
-import org.openstreetmap.josm.data.osm.OsmPrimitive;
-import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
-import org.openstreetmap.josm.data.osm.Relation;
-import org.openstreetmap.josm.data.osm.RelationMemberData;
 import org.openstreetmap.josm.data.osm.User;
-import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.data.osm.WayData;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
@@ -92 +92 @@
         }
 
-        private void setMetadata(OsmPrimitive osm, Info info) throws IllegalDataException {
+        private void setMetadata(PrimitiveData osm, Info info) throws IllegalDataException {
             if (info.hasChangeset()) {
                 checkChangesetId(info.getChangeset());
@@ -146 +146 @@
                     for (int i = 0; i < nodes.getIdCount(); i++) {
                         // Id (delta) and version (normal)
-                        Node node = new Node(nodeId += nodes.getId(i), nodes.hasDenseinfo() ? nodes.getDenseinfo().getVersion(i) : 1);
+                        NodeData nd = new NodeData(nodeId += nodes.getId(i));
+                        nd.setVersion(nodes.hasDenseinfo() ? nodes.getDenseinfo().getVersion(i) : 1);
                         // Lat/Lon (delta)
-                        node.setCoor(new LatLon(parseLat(nodeLat += nodes.getLat(i)),
+                        nd.setCoor(new LatLon(parseLat(nodeLat += nodes.getLat(i)),
                                                 parseLon(nodeLon += nodes.getLon(i))).getRoundedToOsmPrecision());
-                        checkCoordinates(node.getCoor());
+                        checkCoordinates(nd.getCoor());
                         if (nodes.hasDenseinfo()) {
                             DenseInfo info = nodes.getDenseinfo();
@@ -156 +157 @@
                             if (info.getChangesetCount() > i) {
                                 checkChangesetId(changesetId += info.getChangeset(i));
-                                node.setChangesetId((int) changesetId);
+                                nd.setChangesetId((int) changesetId);
                             }
                             // User (delta)
                             if (info.getUidCount() > i && info.getUserSidCount() > i) {
-                                node.setUser(User.createOsmUser(uid += info.getUid(i),
+                                nd.setUser(User.createOsmUser(uid += info.getUid(i),
                                                  getStringById(suid += info.getUserSid(i))));
                             }
@@ -166 +167 @@
                             if (info.getTimestampCount() > i) {
                                 checkTimestamp(timestamp += info.getTimestamp(i));
-                                node.setTimestamp(new Date(date_granularity * timestamp));
+                                nd.setTimestamp(new Date(date_granularity * timestamp));
                             }
                         }
@@ -183 +184 @@
                             }
                         }
-                        node.setKeys(keys);
-                        externalIdMap.put(node.getPrimitiveId(), node);
+                        nd.setKeys(keys);
+                        buildPrimitive(nd);
                     }
                 } catch (IllegalDataException e) {
@@ -200 +201 @@
                         if (!info.hasVersion())
                             discourageUpload = true;
-                        final Node node = new Node(n.getId(), info.hasVersion() ? info.getVersion() : 1);
-                        node.setCoor(new LatLon(parseLat(n.getLat()), parseLon(n.getLon())).getRoundedToOsmPrecision());
-                        checkCoordinates(node.getCoor());
-                        setMetadata(node, info);
+                        NodeData nd = new NodeData(n.getId());
+                        nd.setVersion(info.hasVersion() ? info.getVersion() : 1);
+                        nd.setCoor(new LatLon(parseLat(n.getLat()), parseLon(n.getLon())).getRoundedToOsmPrecision());
+                        checkCoordinates(nd.getCoor());
+                        setMetadata(nd, info);
                         Map<String, String> keys = new HashMap<>();
                         for (int i = 0; i < n.getKeysCount(); i++) {
                             keys.put(getStringById(n.getKeys(i)), getStringById(n.getVals(i)));
                         }
-                        node.setKeys(keys);
-                        externalIdMap.put(node.getPrimitiveId(), node);
+                        nd.setKeys(keys);
+                        buildPrimitive(nd);
                     }
                 } catch (IllegalDataException e) {
@@ -225 +227 @@
                         if (!info.hasVersion())
                             discourageUpload = true;
-                        final Way way = new Way(w.getId(), info.hasVersion() ? info.getVersion() : 1);
-                        setMetadata(way, info);
+                        final WayData wd = new WayData(w.getId());
+                        wd.setVersion(info.hasVersion() ? info.getVersion() : 1);
+                        setMetadata(wd, info);
                         Map<String, String> keys = new HashMap<>();
                         for (int i = 0; i < w.getKeysCount(); i++) {
                             keys.put(getStringById(w.getKeys(i)), getStringById(w.getVals(i)));
                         }
-                        way.setKeys(keys);
+                        wd.setKeys(keys);
                         long previousId = 0; // Node ids are delta coded
                         Collection<Long> nodeIds = new ArrayList<>();
@@ -237 +240 @@
                             nodeIds.add(previousId += id);
                         }
-                        ways.put(way.getUniqueId(), nodeIds);
-                        externalIdMap.put(way.getPrimitiveId(), way);
+                        ways.put(wd.getUniqueId(), nodeIds);
+                        buildPrimitive(wd);
                     }
                 } catch (IllegalDataException e) {
@@ -254 +257 @@
                         if (!info.hasVersion())
                             discourageUpload = true;
-                        final Relation rel = new Relation(r.getId(), info.hasVersion() ? info.getVersion() : 1);
-                        setMetadata(rel, info);
+                        final RelationData rd = new RelationData(r.getId());
+                        rd.setVersion(info.hasVersion() ? info.getVersion() : 1);
+                        setMetadata(rd, info);
                         Map<String, String> keys = new HashMap<>();
                         for (int i = 0; i < r.getKeysCount(); i++) {
                             keys.put(getStringById(r.getKeys(i)), getStringById(r.getVals(i)));
                         }
-                        rel.setKeys(keys);
+                        rd.setKeys(keys);
                         long previousId = 0; // Member ids are delta coded
                         Collection<RelationMemberData> members = new ArrayList<>();
@@ -269 +273 @@
                                     previousId += r.getMemids(i)));
                         }
-                        relations.put(rel.getUniqueId(), members);
-                        externalIdMap.put(rel.getPrimitiveId(), rel);
+                        relations.put(rd.getUniqueId(), members);
+                        buildPrimitive(rd);
                     }
                 } catch (IllegalDataException e) {

applications/editors/josm/plugins/pbf/test/unit/org/openstreetmap/josm/plugins/pbf/io/PbfImporterTest.java

@@ -80 +80 @@
         assertEquals(97, ds.getRelations().size());
     }
+    
+    /**
+     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/14545">Ticket #14545</a>.
+     * @throws Exception if an error occurs
+     */
+    @Test
+    public void testTicket14545() throws Exception {
+        DataSet ds = new PbfImporter().parseDataSet(TestUtils.getRegressionDataFile(14545, "reg14545.osm.pbf"));
+        assertNotNull(ds);
+        assertEquals(12, ds.getNodes().size());
+        assertEquals(2, ds.getWays().size());
+        assertEquals(1, ds.getRelations().size());
+    }
+
+    
 }