Ticket #18258: 18258.1.patch

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

@@ -42 +42 @@
     protected XMLStreamReader parser;
 
     protected boolean convertUnknownToTags;
+    protected boolean saveOriginalId;
 
     private static final Set<String> COMMON_XML_ATTRIBUTES = new TreeSet<>();
 
@@ -64 +65 @@
      * @see #parseDataSet(InputStream, ProgressMonitor)
      */
     protected OsmReader() {
-        this(false);
+        this(false, false);
     }
 
     /**
@@ -75 +76 @@
      * @since 15470
      */
     protected OsmReader(boolean convertUnknownToTags) {
+        this(convertUnknownToTags, false);
         // Restricts visibility
+    }
+    /**
+     * constructor (for private and subclasses use only)
+     * @param convertUnknownToTags if true, keep unknown xml attributes as tags
+     * @param saveOriginalId if true, save the original id in a "current_id" tag
+     *
+     * @see #parseDataSet(InputStream, ProgressMonitor)
+     * @since xxx
+     */
+    protected OsmReader(boolean convertUnknownToTags, boolean saveOriginalId) {
+        // Restricts visibility
         this.convertUnknownToTags = convertUnknownToTags;
     }
 
@@ -425 +438 @@
             parseAction(current, parser.getAttributeValue(null, "action"));
             parseChangeset(current, parser.getAttributeValue(null, "changeset"));
 
+            if (saveOriginalId) {
+                parseTag(current, "current_id", Long.toString(getLong("id")));
+            }
             if (convertUnknownToTags) {
                 for (int i = 0; i < parser.getAttributeCount(); i++) {
                     if (!COMMON_XML_ATTRIBUTES.contains(parser.getAttributeLocalName(i))) {
@@ -496 +512 @@
      * @throws IllegalArgumentException if source is null
      */
     public static DataSet parseDataSet(InputStream source, ProgressMonitor progressMonitor) throws IllegalDataException {
-        return parseDataSet(source, progressMonitor, false);
+        return parseDataSet(source, progressMonitor, false, false);
     }
 
     /**
@@ -513 +529 @@
      */
     public static DataSet parseDataSet(InputStream source, ProgressMonitor progressMonitor, boolean convertUnknownToTags)
             throws IllegalDataException {
-        return new OsmReader(convertUnknownToTags).doParseDataSet(source, progressMonitor);
+        return parseDataSet(source, progressMonitor, convertUnknownToTags, false);
     }
+
+    /**
+     * Parse the given input source and return the dataset.
+     *
+     * @param source the source input stream. Must not be null.
+     * @param progressMonitor the progress monitor. If null, {@link NullProgressMonitor#INSTANCE} is assumed
+     * @param convertUnknownToTags true if unknown xml attributes should be kept as tags
+     * @param saveOriginalId if true, keep the original id (as a tag, "current_id")
+     *
+     * @return the dataset with the parsed data
+     * @throws IllegalDataException if an error was found while parsing the data from the source
+     * @throws IllegalArgumentException if source is null
+     * @since xxx
+     */
+    public static DataSet parseDataSet(InputStream source, ProgressMonitor progressMonitor, boolean convertUnknownToTags, boolean saveOriginalId)
+            throws IllegalDataException {
+        return new OsmReader(convertUnknownToTags, saveOriginalId).doParseDataSet(source, progressMonitor);
+    }
 }

test/unit/org/openstreetmap/josm/io/OsmReaderTest.java

@@ -6 +6 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
@@ -85 +86 @@
             assertTrue(OsmReader.parseDataSet(in, NullProgressMonitor.INSTANCE, parseUnknownAttributes).allPrimitives()
                     .isEmpty());
         }
+        testUnknown(osm, parseUnknownAttributes, true);
+        testUnknown(osm, parseUnknownAttributes, true);
     }
 
+    private static void testUnknown(String osm, boolean parseUnknownAttributes, boolean keepOriginalId)
+            throws Exception {
+        try (InputStream in = new ByteArrayInputStream(
+                ("<?xml version='1.0' encoding='UTF-8'?>" + osm).getBytes(StandardCharsets.UTF_8))) {
+            assertTrue(OsmReader.parseDataSet(in, NullProgressMonitor.INSTANCE, parseUnknownAttributes, keepOriginalId)
+                    .allPrimitives().isEmpty());
+        }
+    }
+
     /**
      * Unit test of {@link OsmReader#parseUnknown} - root case.
      * @throws Exception if any error occurs
@@ -152 +164 @@
     }
 
     /**
+     * Test valid data.
+     *
+     * @param osm                    OSM data without XML prefix
+     * @param parseUnknownAttributes if true, attempt to parse unknown xml
+     *                               attributes
+     * @param keepOriginalId         if true, keep the original id of the object (if
+     *                               a negative id)
+     * @return parsed data set
+     * @throws Exception if any error occurs
+     */
+    private static DataSet testValidData(String osm, boolean parseUnknownAttributes, boolean keepOriginalId)
+            throws Exception {
+        try (InputStream in = new ByteArrayInputStream(
+                ("<?xml version='1.0' encoding='UTF-8'?>" + osm).getBytes(StandardCharsets.UTF_8))) {
+            return OsmReader.parseDataSet(in, NullProgressMonitor.INSTANCE, parseUnknownAttributes, keepOriginalId);
+        }
+    }
+    /**
      * Test invalid data.
      * @param osm OSM data without XML prefix
      * @param expectedError expected error message
@@ -326 +356 @@
         testValidData(gdprChangeset);
         testValidData(gdprChangeset, true);
         testValidData(gdprChangeset, false);
+        testValidData(gdprChangeset, false, true);
+        testValidData(gdprChangeset, true, false);
+        testValidData(gdprChangeset, false, false);
+        testValidData(gdprChangeset, true, true);
     }
 
     /**
@@ -405 +439 @@
                 "<meta osm_base=\"2018-08-30T12:46:02Z\" areas=\"2018-08-30T12:40:02Z\"/>\r\n" +
                 "<remark>runtime error: Query ran out of memory in \"query\" at line 5.</remark>\r\n" +
                 "</osm>";
-        for (DataSet ds : Arrays.asList(testValidData(query), testValidData(query, true), testValidData(query, false))) {
+        for (DataSet ds : Arrays.asList(testValidData(query), testValidData(query, true), testValidData(query, false),
+                testValidData(query, true, false), testValidData(query, false, false), testValidData(query, true, true),
+                testValidData(query, false, true))) {
             assertEquals("runtime error: Query ran out of memory in \"query\" at line 5.", ds.getRemark());
         }
     }
@@ -420 +456 @@
                 + "<node id='1' version='1' visible='true' changeset='82' randomkey='randomvalue'></node>" + "</osm>";
         DataSet ds = testValidData(testData);
         assertEquals(0, ds.getNodes().iterator().next().getKeys().size());
+        assertEquals(1, ds.getNodes().iterator().next().getUniqueId());
 
-        ds = testValidData(testData, true);
-        Node firstNode = ds.getNodes().iterator().next();
-        assertEquals(1, firstNode.getKeys().size());
-        assertEquals("randomvalue", firstNode.get("randomkey"));
+        for (DataSet ds1 : Arrays.asList(testValidData(testData, true), testValidData(testData, true, false),
+                testValidData(testData, true, true))) {
+            ds = ds1;
+            Node firstNode = ds.getNodes().iterator().next();
+            assertEquals(1, firstNode.getKeys().size());
+            assertEquals("randomvalue", firstNode.get("randomkey"));
+            assertEquals(1, ds.getNodes().iterator().next().getUniqueId());
+        }
 
-        ds = testValidData(testData, false);
-        assertEquals(0, ds.getNodes().iterator().next().getKeys().size());
+        for (DataSet ds1 : Arrays.asList(testValidData(testData, false), testValidData(testData, false, false),
+                testValidData(testData, false, true))) {
+            ds = ds1;
+            assertEquals(0, ds.getNodes().iterator().next().getKeys().size());
+            assertEquals(1, ds.getNodes().iterator().next().getUniqueId());
+        }
     }
+
+    /**
+     * Test reading a file with negative ids in osm primitives
+     * 
+     * @throws Exception if any error occurs
+     */
+    @Test
+    public void testNegativeIds() throws Exception {
+        String testData = "<osm version=\"0.6\" generator=\"fake generator\">"
+                + "<node id='-1' version='1' visible='true' changeset='82' randomkey='randomvalue'></node>" + "</osm>";
+        DataSet ds = testValidData(testData);
+        assertNotEquals(-1L, ds.getNodes().iterator().next().getUniqueId());
+
+        for (DataSet ds1 : Arrays.asList(testValidData(testData, true), testValidData(testData, true, false),
+                testValidData(testData, true, false))) {
+            ds = ds1;
+            assertNotEquals(-1L, ds.getNodes().iterator().next().getUniqueId());
+        }
+
+        for (DataSet ds1 : Arrays.asList(testValidData(testData, true, true), testValidData(testData, false, true))) {
+            ds = ds1;
+            assertEquals(-1L, ds.getNodes().iterator().next().getUniqueId()); // if we set the id to the original id
+            assertEquals("-1", ds.getNodes().iterator().next().get("current_id")); // if we just add a tag
+        }
+    }
 }