Ticket #18258: 18258.5.patch

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

@@ -734 +734 @@
                     Arrays.asList(
                             "created_by",
                             "converted_by",
+                            "current_id",
                             "geobase:datasetName",
                             "geobase:uuid",
                             "KSJ2:ADS",

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

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.io.InputStream;
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Objects;
 import java.util.Set;
 import java.util.TreeSet;
@@ -39 +41 @@
  */
 public class OsmReader extends AbstractReader {
 
+    /**
+     * Options are used to change how the xml data is parsed.
+     * For example, {@link Options#CONVERT_UNKNOWN_TO_TAGS} is used to convert unknown
+     * XML attributes to a tag for the object.
+     */
+    public enum Options {
+        /** Convert unknown XML attributes to tags */
+        CONVERT_UNKNOWN_TO_TAGS,
+        /** Save the original id of an object (currently stored in `current_id`) */
+        SAVE_ORIGINAL_ID
+    }
+
     protected XMLStreamReader parser;
 
-    protected boolean convertUnknownToTags;
+    /** The {@link OsmReader.Options} to use when parsing the xml data */
+    protected Collection<Options> options;
 
     private static final Set<String> COMMON_XML_ATTRIBUTES = new TreeSet<>();
 
@@ -64 +79 @@
      * @see #parseDataSet(InputStream, ProgressMonitor)
      */
     protected OsmReader() {
-        this(false);
+        this((Options) null);
     }
 
     /**
      * constructor (for private and subclasses use only)
-     * @param convertUnknownToTags if true, keep unknown xml attributes as tags
+     * @param options The options to use when reading data
      *
      * @see #parseDataSet(InputStream, ProgressMonitor)
-     * @since 15470
+     * @since xxx
      */
-    protected OsmReader(boolean convertUnknownToTags) {
+    protected OsmReader(Options... options) {
         // Restricts visibility
-        this.convertUnknownToTags = convertUnknownToTags;
+        this.options = options == null ? Collections.emptyList() : Arrays.asList(options);
     }
 
     protected void setParser(XMLStreamReader parser) {
@@ -425 +440 @@
             parseAction(current, parser.getAttributeValue(null, "action"));
             parseChangeset(current, parser.getAttributeValue(null, "changeset"));
 
-            if (convertUnknownToTags) {
+            if (options.contains(Options.SAVE_ORIGINAL_ID)) {
+                parseTag(current, "current_id", Long.toString(getLong("id")));
+            }
+            if (options.contains(Options.CONVERT_UNKNOWN_TO_TAGS)) {
                 for (int i = 0; i < parser.getAttributeCount(); i++) {
                     if (!COMMON_XML_ATTRIBUTES.contains(parser.getAttributeLocalName(i))) {
                         parseTag(current, parser.getAttributeLocalName(i), parser.getAttributeValue(i));
@@ -496 +514 @@
      * @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, (Options) null);
     }
 
     /**
@@ -504 +522 @@
      *
      * @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 options The options to use when parsing the dataset
      *
      * @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 15470
+     * @since xxx
      */
-    public static DataSet parseDataSet(InputStream source, ProgressMonitor progressMonitor, boolean convertUnknownToTags)
+    public static DataSet parseDataSet(InputStream source, ProgressMonitor progressMonitor, Options... options)
             throws IllegalDataException {
-        return new OsmReader(convertUnknownToTags).doParseDataSet(source, progressMonitor);
+        return new OsmReader(options).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;
@@ -22 +23 @@
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
+import org.openstreetmap.josm.io.OsmReader.Options;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -82 +84 @@
     private static void testUnknown(String osm, boolean parseUnknownAttributes) 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).allPrimitives()
+            assertTrue(OsmReader.parseDataSet(in, NullProgressMonitor.INSTANCE, Options.CONVERT_UNKNOWN_TO_TAGS).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, Options.CONVERT_UNKNOWN_TO_TAGS, Options.SAVE_ORIGINAL_ID)
+                    .allPrimitives().isEmpty());
+        }
+    }
+
     /**
      * Unit test of {@link OsmReader#parseUnknown} - root case.
      * @throws Exception if any error occurs
@@ -147 +160 @@
     private static DataSet testValidData(String osm, boolean parseUnknownAttributes) 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);
+            return OsmReader.parseDataSet(in, NullProgressMonitor.INSTANCE, Options.CONVERT_UNKNOWN_TO_TAGS);
         }
     }
 
     /**
+     * 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, Options.CONVERT_UNKNOWN_TO_TAGS, Options.SAVE_ORIGINAL_ID);
+        }
+    }
+
+    /**
      * Test invalid data.
      * @param osm OSM data without XML prefix
      * @param expectedError expected error message
@@ -182 +214 @@
             throws Exception {
         try (InputStream in = new ByteArrayInputStream(
                 ("<?xml version='1.0' encoding='UTF-8'?>" + osm).getBytes(StandardCharsets.UTF_8))) {
-            OsmReader.parseDataSet(in, NullProgressMonitor.INSTANCE, parseUnknownAttributes);
+            OsmReader.parseDataSet(in, NullProgressMonitor.INSTANCE, Options.CONVERT_UNKNOWN_TO_TAGS);
             fail("should throw exception");
         } catch (IllegalDataException e) {
             assertEquals(expectedError, e.getMessage());
@@ -326 +358 @@
         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 +441 @@
                 "<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 +458 @@
                 + "<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
+        }
+    }
 }