Index: src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java =================================================================== --- src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java (revision 16569) +++ src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java (working copy) @@ -734,6 +734,7 @@ Arrays.asList( "created_by", "converted_by", + "current_id", "geobase:datasetName", "geobase:uuid", "KSJ2:ADS", Index: src/org/openstreetmap/josm/io/OsmReader.java =================================================================== --- src/org/openstreetmap/josm/io/OsmReader.java (revision 16569) +++ src/org/openstreetmap/josm/io/OsmReader.java (working copy) @@ -4,7 +4,9 @@ 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,9 +41,22 @@ */ 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; private static final Set COMMON_XML_ATTRIBUTES = new TreeSet<>(); @@ -64,19 +79,19 @@ * @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,7 +440,10 @@ 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,7 +514,7 @@ * @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,15 +522,15 @@ * * @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); } } Index: test/unit/org/openstreetmap/josm/io/OsmReaderTest.java =================================================================== --- test/unit/org/openstreetmap/josm/io/OsmReaderTest.java (revision 16569) +++ test/unit/org/openstreetmap/josm/io/OsmReaderTest.java (working copy) @@ -6,6 +6,7 @@ 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,6 +23,7 @@ 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,11 +84,22 @@ private static void testUnknown(String osm, boolean parseUnknownAttributes) throws Exception { try (InputStream in = new ByteArrayInputStream( ("" + 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( + ("" + 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,11 +160,30 @@ private static DataSet testValidData(String osm, boolean parseUnknownAttributes) throws Exception { try (InputStream in = new ByteArrayInputStream( ("" + 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( + ("" + 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,7 +214,7 @@ throws Exception { try (InputStream in = new ByteArrayInputStream( ("" + 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,6 +358,10 @@ 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,7 +441,9 @@ "\r\n" + "runtime error: Query ran out of memory in \"query\" at line 5.\r\n" + ""; - 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,13 +458,47 @@ + "" + ""; 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 = "" + + "" + ""; + 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 + } + } }