Changeset 2604 in josm for trunk/src/org

Timestamp:

2009-12-10T18:42:41+01:00 (15 years ago)

Author:

Gubaer

Message:

New: JOSM reading, writing, merging changeset attribute
fixed #4090: Add changeset:* search option to JOSM's search engine

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/search/SearchAction.java (modified) (1 diff)
• actions/search/SearchCompiler.java (modified) (2 diffs)
• data/osm/Changeset.java (modified) (6 diffs)
• data/osm/OsmPrimitive.java (modified) (7 diffs)
• data/osm/PrimitiveData.java (modified) (2 diffs)
• io/DiffResultProcessor.java (moved) (moved from trunk/src/org/openstreetmap/josm/io/DiffResultReader.java ) (2 diffs)
• io/OsmApi.java (modified) (6 diffs)
• io/OsmChangeBuilder.java (moved) (moved from trunk/src/org/openstreetmap/josm/data/osm/visitor/CreateOsmChangeVisitor.java ) (5 diffs)
• io/OsmChangesetParser.java (modified) (1 diff)
• io/OsmReader.java (modified) (13 diffs)
• io/OsmWriter.java (modified) (7 diffs)

trunk/src/org/openstreetmap/josm/actions/search/SearchAction.java

@@ -126 +126 @@
                     + "<li>"+tr("<b>user:anonymous</b> - all objects changed by anonymous users")+"</li>"
                     + "<li>"+tr("<b>id:</b>... - object with given ID (0 for new objects)")+"</li>"
+                    + "<li>"+tr("<b>changeset:</b>... - object with given changeset id (0 objects without assigned changeset)")+"</li>"
                     + "<li>"+tr("<b>nodes:</b>... - object with given number of nodes")+"</li>"
                     + "<li>"+tr("<b>tags:</b>... - object with given number of tags (tags:count or tags:min-max)")+"</li>"

trunk/src/org/openstreetmap/josm/actions/search/SearchCompiler.java

@@ -85 +85 @@
         }
         @Override public String toString() {return "id="+id;}
+    }
+
+    private static class ChangesetId extends Match {
+        private long changesetid;
+        public ChangesetId(long changesetid) {this.changesetid = changesetid;}
+        @Override public boolean match(OsmPrimitive osm) {
+            return osm.getChangesetId() == changesetid;
+        }
+        @Override public String toString() {return "changeset="+changesetid;}
     }
 
@@ -693 +702 @@
                 throw new ParseError(tr("Incorrect value of id operator: {0}. Number is expected.", value));
             }
+        } else if (key.equals("changeset")) {
+            try {
+                return new ChangesetId(Integer.parseInt(value));
+            } catch (NumberFormatException x) {
+                throw new ParseError(tr("Incorrect value of changeset operator: {0}. Number is expected.", value));
+            }
         } else
             return new KeyValue(key, value, regexSearch, caseSensitive);

trunk/src/org/openstreetmap/josm/data/osm/Changeset.java

@@ -17 +17 @@
 public final class Changeset implements Tagged {
     /** the changeset id */
-    private long id;
+    private int id;
     /** the user who owns the changeset */
     private User user;
@@ -50 +50 @@
      * @param id the id
      */
-    public Changeset(long id) {
+    public Changeset(int id) {
         this.id = id;
         this.incomplete = id > 0;
@@ -90 +90 @@
 
     public int compareTo(Changeset other) {
-        return Long.valueOf(getId()).compareTo(other.getId());
+        return Integer.valueOf(getId()).compareTo(other.getId());
     }
 
@@ -102 +102 @@
     }
 
-    public long getId() {
+    public int getId() {
         return id;
     }
 
-    public void setId(long id) {
+    public void setId(int id) {
         this.id = id;
     }
@@ -234 +234 @@
         final int prime = 31;
         int result = 1;
-        result = prime * result + (int) (id ^ (id >>> 32));
+        result = prime * result + (id ^ (id >>> 32));
         if (id > 0)
             return prime * result + getClass().hashCode();
@@ -306 +306 @@
         return id <= 0;
     }
+
+    public void mergeFrom(Changeset other) {
+        if (other == null)
+            return;
+        if (id != other.id)
+            return;
+        this.user = other.user;
+        this.createdAt = other.createdAt;
+        this.closedAt = other.closedAt;
+        this.open  = other.open;
+        this.min = other.min;
+        this.max = other.max;
+        this.tags.clear();
+        this.tags.putAll(other.tags);
+    }
 }

trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java

@@ -238 +238 @@
 
     /**
+     * The id of the changeset this primitive was last uploaded to.
+     * 0 if it wasn't uploaded to a changeset yet of if the changeset
+     * id isn't known.
+     */
+    private int changesetId;
+
+    /**
      * Creates a new primitive for the given id. If the id > 0, the primitive is marked
      * as incomplete.
@@ -577 +584 @@
     public void setUser(User user) {
         this.user = user;
+    }
+
+
+    /**
+     * Replies the id of the changeset this primitive was last uploaded to.
+     * 0 if this primitive wasn't uploaded to a changeset yet or if the
+     * changeset isn't known.
+     * 
+     * @return the id of the changeset this primitive was last uploaded to.
+     */
+    public int getChangesetId() {
+        return changesetId;
+    }
+
+
+    /**
+     * Sets the changeset id of this primitive. Can't be set on a new
+     * primitive.
+     * 
+     * @param changesetId the id. >= 0 required.
+     * @throws IllegalStateException thrown if this primitive is new.
+     * @throws IllegalArgumentException thrown if id < 0
+     */
+    public void setChangesetId(int changesetId) throws IllegalStateException, IllegalArgumentException {
+        if (changesetId < 0)
+            throw new IllegalArgumentException(tr("Parameter ''{0}'' >= 0 expected, got {1}", "changesetId", changesetId));
+        if (isNew() && changesetId > 0)
+            throw new IllegalStateException(tr("Can''t assign a changesetId > 0 to a new primitive. Value of changesetId is {0}", changesetId));
+        this.changesetId = changesetId;
     }
 
@@ -916 +952 @@
      * use this only in the data initializing phase
      */
-    public void cloneFrom(OsmPrimitive osm) {
-        setKeys(osm.getKeys());
-        id = osm.id;
-        timestamp = osm.timestamp;
-        version = osm.version;
-        setIncomplete(osm.isIncomplete());
-        flags = osm.flags;
-        user= osm.user;
+    public void cloneFrom(OsmPrimitive other) {
+        setKeys(other.getKeys());
+        id = other.id;
+        timestamp = other.timestamp;
+        version = other.version;
+        setIncomplete(other.isIncomplete());
+        flags = other.flags;
+        user= other.user;
+        changesetId = other.changesetId;
         clearCached();
     }
@@ -951 +988 @@
         flags = other.flags;
         user= other.user;
+        changesetId = other.changesetId;
     }
 
@@ -1000 +1038 @@
         && version == other.version
         && isVisible() == other.isVisible()
-        && (user == null ? other.user==null : user==other.user);
+        && (user == null ? other.user==null : user==other.user)
+        && changesetId == other.changesetId;
     }
 
@@ -1098 +1137 @@
         timestamp = data.getTimestamp();
         user = data.getUser();
+        changesetId = data.getChangesetId();
         setDeleted(data.isDeleted());
         setModified(data.isModified());
@@ -1118 +1158 @@
         data.setModified(isModified());
         data.setVisible(isVisible());
+        data.setChangesetId(changesetId);
     }
 

trunk/src/org/openstreetmap/josm/data/osm/PrimitiveData.java

@@ -46 +46 @@
     private int version;
     private int timestamp;
+    private int changesetId;
 
     public boolean isModified() {
@@ -89 +90 @@
         this.timestamp = timestamp;
     }
+
+    public int getChangesetId() {
+        return changesetId;
+    }
+
+    public void setChangesetId(int changesetId) {
+        this.changesetId = changesetId;
+    }
+
     public Map<String, String> getKeys() {
         return keys;

trunk/src/org/openstreetmap/josm/io/DiffResultProcessor.java

@@ -7 +7 @@
 import java.io.StringReader;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParserFactory;
 
-import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.Changeset;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
-import org.openstreetmap.josm.data.osm.Relation;
-import org.openstreetmap.josm.data.osm.Way;
-import org.openstreetmap.josm.data.osm.visitor.AbstractVisitor;
+import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
+import org.openstreetmap.josm.data.osm.PrimitiveId;
+import org.openstreetmap.josm.data.osm.SimplePrimitiveId;
+import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.xml.sax.Attributes;
 import org.xml.sax.InputSource;
+import org.xml.sax.Locator;
 import org.xml.sax.SAXException;
 import org.xml.sax.helpers.DefaultHandler;
 
-/**
- */
-public class DiffResultReader extends AbstractVisitor {
+public class DiffResultProcessor  {
 
-    /**
-     * mapping from old id to new id/version
-     */
-    private Map<String, Long[]> versions = new HashMap<String, Long[]>();
-    private Collection<OsmPrimitive> processed;
-    private Map<OsmPrimitive,Long> newIdMap;
-
-    /**
-     * List of protocol versions that will be accepted on reading
-     */
-
-    private class Parser extends DefaultHandler {
-
-        @Override public void startElement(String namespaceURI, String localName, String qName, Attributes atts) throws SAXException {
-            try {
-                if (qName.equals("osm")) {
-                } else if (qName.equals("node") || qName.equals("way") || qName.equals("relation")) {
-                    String key = qName + ":" + atts.getValue("old_id");
-                    String newid = atts.getValue("new_id");
-                    String newver = atts.getValue("new_version");
-                    Long[] value = new Long[] { newid == null ? null : new Long(newid), newver == null ? null : new Long(newver) };
-                    versions.put(key, value);
-                }
-            } catch (NumberFormatException x) {
-                x.printStackTrace(); // SAXException does not chain correctly
-                throw new SAXException(x.getMessage(), x);
-            } catch (NullPointerException x) {
-                x.printStackTrace(); // SAXException does not chain correctly
-                throw new SAXException(tr("NullPointerException, possibly some missing tags."), x);
-            }
-        }
+    static private class DiffResultEntry {
+        public long new_id;
+        public int new_version;
     }
 
     /**
-     * Parse the given input source and return the dataset.
+     * mapping from old id to new id and version, the result of parsing the diff result
+     * replied by the server
      */
-    public static void parseDiffResult(String source, Collection<OsmPrimitive> osm, Collection<OsmPrimitive> processed, Map<OsmPrimitive,Long> newIdMap, ProgressMonitor progressMonitor)
-    throws SAXException, IOException {
+    private Map<PrimitiveId, DiffResultEntry> diffResults = new HashMap<PrimitiveId, DiffResultEntry>();
+    /**
+     * the set of processed primitives *after* the new id, the new version and the new changeset id
+     * is set
+     */
+    private Set<OsmPrimitive> processed;
+    /**
+     * the collection of primitives being uploaded
+     */
+    private Collection<OsmPrimitive> primitives;
 
-        progressMonitor.beginTask(tr("Preparing data..."));
+    /**
+     * Creates a diff result reader
+     * 
+     * @param primitives the collection of primitives which have been uploaded. If null,
+     * assumes an empty collection.
+     */
+    public DiffResultProcessor(Collection<OsmPrimitive> primitives) {
+        if (primitives == null) {
+            primitives = Collections.emptyList();
+        }
+        this.primitives = primitives;
+        this.processed = new HashSet<OsmPrimitive>();
+    }
+
+    /**
+     * Parse the response from a diff upload to the OSM API.
+     * 
+     * @param diffUploadResponse the response. Must not be null.
+     * @param progressMonitor a progress monitor. Defaults to {@see NullProgressMonitor#INSTANCE} if null
+     * @throws IllegalArgumentException thrown if diffUploadRequest is null
+     * @throws OsmDataParsingException thrown if the diffUploadRequest can't be parsed successfully
+     * 
+     */
+    public  void parse(String diffUploadResponse, ProgressMonitor progressMonitor) throws OsmDataParsingException {
+        if (progressMonitor == null) {
+            progressMonitor = NullProgressMonitor.INSTANCE;
+        }
+        if (diffUploadResponse == null)
+            throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "source"));
         try {
-
-            DiffResultReader drr = new DiffResultReader();
-            drr.processed = processed;
-            drr.newIdMap = newIdMap;
-            InputSource inputSource = new InputSource(new StringReader(source));
-            try {
-                SAXParserFactory.newInstance().newSAXParser().parse(inputSource, drr.new Parser());
-            } catch (ParserConfigurationException e1) {
-                e1.printStackTrace(); // broken SAXException chaining
-                throw new SAXException(e1);
-            }
-
-            for (OsmPrimitive p : osm) {
-                //System.out.println("old: "+ p);
-                p.visit(drr);
-                //System.out.println("new: "+ p);
-                //System.out.println("");
-            }
+            progressMonitor.beginTask(tr("Parsing response from server..."));
+            InputSource inputSource = new InputSource(new StringReader(diffUploadResponse));
+            SAXParserFactory.newInstance().newSAXParser().parse(inputSource, new Parser());
+        } catch(IOException e) {
+            throw new OsmDataParsingException(e);
+        } catch(ParserConfigurationException e) {
+            throw new OsmDataParsingException(e);
+        } catch(OsmDataParsingException e) {
+            throw e;
+        } catch(SAXException e) {
+            throw new OsmDataParsingException(e);
         } finally {
             progressMonitor.finishTask();
@@ -92 +97 @@
     }
 
-    public void visit(Node n) {
-        String key = "node:" + (newIdMap.containsKey(n) ? newIdMap.get(n) : n.getId());
-        Long[] nv = versions.get(key);
-        if (nv != null) {
-            processed.add(n);
-            if (!n.isDeleted()) {
-                n.setOsmId(nv[0], nv[1].intValue());
+    /**
+     * Postprocesses the diff result read and parsed from the server.
+     * 
+     * Uploaded objects are assigned their new id (if they got assigned a new
+     * id by the server), their new version (if the version was incremented),
+     * and the id of the changeset to which they were uploaded.
+     * 
+     * @param cs the current changeset. Ignored if null.
+     * @param monitor the progress monitor. Set to {@see NullProgressMonitor#INSTANCE} if null
+     * @return the collection of processed primitives
+     */
+    protected Set<OsmPrimitive> postProcess(Changeset cs,ProgressMonitor monitor) {
+        if (monitor == null) {
+            monitor = NullProgressMonitor.INSTANCE;
+        }
+        try {
+            monitor.beginTask("Postprocessing uploaded data ...");
+            monitor.setTicksCount(primitives.size());
+            monitor.setTicks(0);
+            for (OsmPrimitive p: primitives) {
+                monitor.worked(1);
+                DiffResultEntry entry = diffResults.get(p.getPrimitiveId());
+                if (entry == null) {
+                    continue;
+                }
+                processed.add(p);
+                if (!p.isDeleted()) {
+                    p.setOsmId(entry.new_id, entry.new_version);
+                }
+                if (cs != null && !cs.isNew()) {
+                    p.setChangesetId(cs.getId());
+                }
             }
+            return processed;
+        } finally {
+            monitor.finishTask();
         }
     }
-    public void visit(Way w) {
-        String key = "way:" + (newIdMap.containsKey(w) ? newIdMap.get(w) : w.getId());
-        Long[] nv = versions.get(key);
-        if (nv != null) {
-            processed.add(w);
-            if (!w.isDeleted()) {
-                w.setOsmId(nv[0], nv[1].intValue());
-            }
+
+    private class Parser extends DefaultHandler {
+        private Locator locator;
+
+        @Override
+        public void setDocumentLocator(Locator locator) {
+            this.locator = locator;
         }
-    }
-    public void visit(Relation r) {
-        String key = "relation:" + (newIdMap.containsKey(r) ? newIdMap.get(r) : r.getId());
-        Long[] nv = versions.get(key);
-        if (nv != null) {
-            processed.add(r);
-            if (!r.isDeleted()) {
-                r.setOsmId(nv[0], nv[1].intValue());
+
+        protected void throwException(String msg) throws OsmDataParsingException{
+            throw new OsmDataParsingException(msg).rememberLocation(locator);
+        }
+
+        @Override public void startElement(String namespaceURI, String localName, String qName, Attributes atts) throws SAXException {
+            try {
+                if (qName.equals("diffResult")) {
+                    // the root element, ignore
+                } else if (qName.equals("node") || qName.equals("way") || qName.equals("relation")) {
+
+                    PrimitiveId id  = new SimplePrimitiveId(
+                            Long.parseLong(atts.getValue("old_id")),
+                            OsmPrimitiveType.fromApiTypeName(qName)
+                    );
+                    DiffResultEntry entry = new DiffResultEntry();
+                    if (atts.getValue("new_id") != null) {
+                        entry.new_id = Long.parseLong(atts.getValue("new_id"));
+                    }
+                    if (atts.getValue("new_version") != null) {
+                        entry.new_version = Integer.parseInt(atts.getValue("new_version"));
+                    }
+                    diffResults.put(id, entry);
+                } else {
+                    throwException(tr("Unexpected XML element with name ''{0}''", qName));
+                }
+            } catch (NumberFormatException e) {
+                throw new OsmDataParsingException(e).rememberLocation(locator);
             }
         }

trunk/src/org/openstreetmap/josm/io/OsmApi.java

@@ -19 +19 @@
 import java.net.URL;
 import java.net.UnknownHostException;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -30 +29 @@
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
-import org.openstreetmap.josm.data.osm.visitor.CreateOsmChangeVisitor;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
@@ -240 +238 @@
             ret = sendRequest("PUT", OsmPrimitiveType.from(osm).getAPIName()+"/create", toXml(osm, true),monitor);
             osm.setOsmId(Long.parseLong(ret.trim()), 1);
+            osm.setChangesetId(getChangeset().getId());
         } catch(NumberFormatException e){
             throw new OsmTransferException(tr("Unexpected format of ID replied by the server. Got ''{0}''.", ret));
@@ -260 +259 @@
             ret = sendRequest("PUT", OsmPrimitiveType.from(osm).getAPIName()+"/" + osm.getId(), toXml(osm, true), monitor);
             osm.setOsmId(osm.getId(), Integer.parseInt(ret.trim()));
+            osm.setChangesetId(getChangeset().getId());
         } catch(NumberFormatException e) {
             throw new OsmTransferException(tr("Unexpected format of new version of modified primitive ''{0}''. Got ''{1}''.", osm.getId(), ret));
@@ -301 +301 @@
             try {
                 ret = sendRequest("PUT", "changeset/create", toXml(changeset),progressMonitor);
-                changeset.setId(Long.parseLong(ret.trim()));
+                changeset.setId(Integer.parseInt(ret.trim()));
                 changeset.setOpen(true);
             } catch(NumberFormatException e){
@@ -398 +398 @@
 
             initialize(monitor);
-            final ArrayList<OsmPrimitive> processed = new ArrayList<OsmPrimitive>();
-
-            CreateOsmChangeVisitor duv = new CreateOsmChangeVisitor(changeset, OsmApi.this);
-
-            monitor.subTask(tr("Preparing..."));
-            for (OsmPrimitive osm : list) {
-                osm.visit(duv);
-                monitor.worked(1);
-            }
-            monitor.indeterminateSubTask(tr("Uploading..."));
-
-            String diff = duv.getDocument();
-            String diffresult = sendRequest("POST", "changeset/" + changeset.getId() + "/upload", diff,monitor);
-            DiffResultReader.parseDiffResult(diffresult, list, processed, duv.getNewIdMap(),
-                    monitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
-            return processed;
+
+            // prepare upload request
+            //
+            OsmChangeBuilder changeBuilder = new OsmChangeBuilder(changeset);
+            monitor.subTask(tr("Preparing upload request..."));
+            changeBuilder.start();
+            changeBuilder.append(list);
+            changeBuilder.finish();
+            String diffUploadRequest = changeBuilder.getDocument();
+
+            // Upload to the server
+            //
+            monitor.indeterminateSubTask(tr("Uploading {0} objects...", list.size()));
+            String diffUploadResponse = sendRequest("POST", "changeset/" + changeset.getId() + "/upload", diffUploadRequest,monitor);
+
+            // Process the response from the server
+            //
+            DiffResultProcessor reader = new DiffResultProcessor(list);
+            reader.parse(diffUploadResponse, monitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
+            return reader.postProcess(
+                    getChangeset(),
+                    monitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)
+            );
         } catch(OsmTransferException e) {
             throw e;
-        } catch(Exception e) {
+        } catch(OsmDataParsingException e) {
             throw new OsmTransferException(e);
         } finally {

trunk/src/org/openstreetmap/josm/io/OsmChangeBuilder.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
-package org.openstreetmap.josm.data.osm.visitor;
+package org.openstreetmap.josm.io;
+
+import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
-import java.util.Map;
+import java.util.Collection;
 
 import org.openstreetmap.josm.data.osm.Changeset;
-import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
-import org.openstreetmap.josm.data.osm.Relation;
-import org.openstreetmap.josm.data.osm.Way;
-import org.openstreetmap.josm.io.OsmApi;
-import org.openstreetmap.josm.io.OsmWriter;
 
 /**
@@ -19 +16 @@
  * OsmChange format.
  *
- * @author fred
- *
  */
-public class CreateOsmChangeVisitor extends AbstractVisitor {
+public class OsmChangeBuilder {
+    static public final String DEFAULT_API_VERSION = "0.6";
 
     private String currentMode;
@@ -28 +24 @@
     private StringWriter swriter;
     private OsmWriter osmwriter;
-    private OsmApi api;
+    private String apiVersion = DEFAULT_API_VERSION;
+    private boolean prologWritten = false;
 
-    public CreateOsmChangeVisitor(Changeset changeset, OsmApi api) {
+    public OsmChangeBuilder(Changeset changeset) {
+        this(changeset, null /* default api version */);
+    }
+
+    public OsmChangeBuilder(Changeset changeset, String apiVersion) {
+        this.apiVersion = apiVersion == null ? DEFAULT_API_VERSION : apiVersion;
         writer = new PrintWriter(swriter = new StringWriter());
-        writer.write("<osmChange version=\"");
-        writer.write(api.getVersion());
-        writer.write("\" generator=\"JOSM\">\n");
-        this.api = api;
-        // need to set osmConform = false here so that negative IDs get transmitted.
-        // this also enables unnecessary and (if the API were more strict) potentially
-        // harmful action="..." attributes.
-        osmwriter = new OsmWriter(writer, false, api.getVersion());
+        osmwriter = new OsmWriter(writer, false, apiVersion);
         osmwriter.setChangeset(changeset);
     }
 
-    // FIXME: This should really NOT use a visitor pattern, it looks
-    // stupid. Just have one method named "write" instead of three "visit"s.
-
-    public void visit(Node n) {
-        if (n.isDeleted()) {
+    protected void write(OsmPrimitive p) {
+        if (p.isDeleted()) {
             switchMode("delete");
             osmwriter.setWithBody(false);
-            osmwriter.visit(n);
+            p.visit(osmwriter);
         } else {
-            switchMode(n.isNew() ? "create" : "modify");
+            switchMode(p.isNew() ? "create" : "modify");
             osmwriter.setWithBody(true);
-            osmwriter.visit(n);
-        }
-    }
-    public void visit(Way w) {
-        if (w.isDeleted()) {
-            switchMode("delete");
-            osmwriter.setWithBody(false);
-            osmwriter.visit(w);
-        } else {
-            switchMode(w.isNew() ? "create" : "modify");
-            osmwriter.setWithBody(true);
-            osmwriter.visit(w);
-        }
-    }
-    public void visit(Relation r) {
-        if (r.isDeleted()) {
-            switchMode("delete");
-            osmwriter.setWithBody(false);
-            osmwriter.visit(r);
-        } else {
-            switchMode(r.isNew() ? "create" : "modify");
-            osmwriter.setWithBody(true);
-            osmwriter.visit(r);
+            p.visit(osmwriter);
         }
     }
@@ -83 +53 @@
         if ((newMode != null && !newMode.equals(currentMode))||(newMode == null && currentMode != null)) {
             if (currentMode != null) {
-                writer.write("</");
-                writer.write(currentMode);
-                writer.write(">\n");
+                writer.print("</");
+                writer.print(currentMode);
+                writer.println(">");
             }
             if (newMode != null) {
-                writer.write("<");
-                writer.write(newMode);
-                writer.write(" version=\"");
-                writer.write(api.getVersion());
-                writer.write("\" generator=\"JOSM\">\n");
+                writer.print("<");
+                writer.print(newMode);
+                writer.println(">");
             }
             currentMode = newMode;
@@ -98 +66 @@
     }
 
-    public String getDocument() {
-        switchMode(null);
-        return swriter.toString() + "</osmChange>\n";
+    /**
+     * Writes the prolog of the OsmChange document
+     * 
+     * @throws IllegalStateException thrown if the prologs has already been written
+     */
+    public void start() throws IllegalStateException{
+        if (prologWritten)
+            throw new IllegalStateException(tr("Prolog of OsmChange document already written. Please write only once."));
+        writer.print("<osmChange version=\"");
+        writer.print(apiVersion);
+        writer.println("\" generator=\"JOSM\">");
+        prologWritten=true;
     }
 
-    public Map<OsmPrimitive,Long> getNewIdMap() {
-        return osmwriter.usedNewIds;
+    /**
+     * Appends a collection of {@see OsmPrimitive}s to the OsmChange document.
+     * 
+     * @param primitives the collection of primitives. Ignored if null.
+     * @throws IllegalStateException thrown if the prologs has not been written yet
+     * @see #start()
+     * @see #append(OsmPrimitive)
+     */
+    public void append(Collection<OsmPrimitive> primitives) throws IllegalStateException{
+        if (primitives == null) return;
+        if (!prologWritten)
+            throw new IllegalStateException(tr("Prolog of OsmChange document not written yet. Please write frst."));
+        for (OsmPrimitive p : primitives) {
+            write(p);
+        }
+    }
+
+    /**
+     * Appends an {@see OsmPrimitive} to the OsmChange document.
+     * 
+     * @param p the primitive. Ignored if null.
+     * @throws IllegalStateException thrown if the prologs has not been written yet
+     * @see #start()
+     * @see #append(Collection)
+
+     */
+    public void append(OsmPrimitive p) {
+        if (p == null) return;
+        if (!prologWritten)
+            throw new IllegalStateException(tr("Prolog of OsmChange document not written yet. Please write frst."));
+        write(p);
+    }
+
+    /**
+     * Writes the epilog of the OsmChange document
+     * 
+     * @throws IllegalStateException thrown if the prologs has not been written yet
+     */
+    public void finish() throws IllegalStateException {
+        if (!prologWritten)
+            throw new IllegalStateException(tr("Prolog of OsmChange document not written yet. Please write frst."));
+        if (currentMode != null) {
+            writer.print("</");
+            writer.print(currentMode);
+            writer.println(">");
+        }
+        writer.println("</osmChange>");
+    }
+
+    public String getDocument() {
+        return swriter.toString();
     }
 }

trunk/src/org/openstreetmap/josm/io/OsmChangesetParser.java

@@ -72 +72 @@
                 throwException(tr("Missing mandatory attribute ''{0}''.", "id"));
             }
-            long id = 0;
+            int id = 0;
             try {
-                id = Long.parseLong(value);
+                id = Integer.parseInt(value);
             } catch(NumberFormatException e) {
                 throwException(tr("Illegal value for attribute ''{0}''. Got ''{1}''.", "id", value));

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

@@ -25 +25 @@
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
+import org.openstreetmap.josm.data.osm.PrimitiveId;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
+import org.openstreetmap.josm.data.osm.SimplePrimitiveId;
 import org.openstreetmap.josm.data.osm.User;
 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.tools.DateUtils;
@@ -69 +72 @@
      * </ul>
      */
-    private Map<String, OsmPrimitive> externalIdMap = new HashMap<String, OsmPrimitive>();
+    private Map<PrimitiveId, OsmPrimitive> externalIdMap = new HashMap<PrimitiveId, OsmPrimitive>();
 
     /**
@@ -78 +81 @@
      */
     private OsmReader() {
-        externalIdMap = new HashMap<String, OsmPrimitive>();
+        externalIdMap = new HashMap<PrimitiveId, OsmPrimitive>();
     }
 
@@ -91 +94 @@
         public LatLon latlon = new LatLon(0,0);
         private OsmPrimitive primitive;
+        private int changesetId;
 
         public void copyTo(OsmPrimitive osm) {
@@ -101 +105 @@
             osm.setTimestamp(timestamp);
             osm.setUser(user);
-            if (id > 0) {
+            if (!osm.isNew() && changesetId > 0) {
+                osm.setChangesetId(changesetId);
+            } else if (osm.isNew() && changesetId > 0) {
+                System.out.println(tr("Warning: ignoring changeset id {0} for new object with external id {1} and internal id {2}",
+                        changesetId,
+                        id,
+                        osm.getUniqueId()
+                ));
+            }
+            if (! osm.isNew()) {
                 // ignore visible attribute for objects not yet known to the server
                 //
@@ -218 +231 @@
                 readCommon(atts, current);
                 Node n = current.createNode();
-                externalIdMap.put("n"+current.id, n);
+                externalIdMap.put(new SimplePrimitiveId(current.id, OsmPrimitiveType.NODE), n);
             } else if (qName.equals("way")) {
                 current = new OsmPrimitiveData();
                 readCommon(atts, current);
                 Way w = current.createWay();
-                externalIdMap.put("w"+current.id, w);
+                externalIdMap.put(new SimplePrimitiveId(current.id, OsmPrimitiveType.WAY), w);
                 ways.put(current.id, new ArrayList<Long>());
             } else if (qName.equals("nd")) {
@@ -251 +264 @@
                 readCommon(atts, current);
                 Relation r = current.createRelation();
-                externalIdMap.put("r"+current.id, r );
+                externalIdMap.put(new SimplePrimitiveId(current.id, OsmPrimitiveType.RELATION), r);
                 relations.put(current.id, new LinkedList<RelationMemberData>());
             } else if (qName.equals("member")) {
@@ -386 +399 @@
 
             String action = atts.getValue("action");
-            if (action == null)
-                return;
-            if (action.equals("delete")) {
+            if (action == null) {
+                // do nothing
+            } else if (action.equals("delete")) {
                 current.deleted = true;
-            } else if (action.startsWith("modify")) {
+            } else if (action.startsWith("modify")) { //FIXME: why startsWith()? why not equals()?
                 current.modified = true;
+            }
+
+            String v = atts.getValue("changeset");
+            if (v == null) {
+                current.changesetId = 0;
+            } else {
+                try {
+                    current.changesetId = Integer.parseInt(v);
+                } catch(NumberFormatException e) {
+                    if (current.id <= 0) {
+                        // for a new primitive we just log a warning
+                        System.out.println(tr("Illegal value for attribute ''changeset'' on new object {1}. Got {0}. Resetting to 0.", v, current.id));
+                        current.changesetId = 0;
+                    } else {
+                        // for an existing primitive this is a problem
+                        throwException(tr("Illegal value for attribute ''changeset''. Got {0}.", v));
+                    }
+                }
+                if (current.changesetId <=0) {
+                    if (current.id <= 0) {
+                        // for a new primitive we just log a warning
+                        System.out.println(tr("Illegal value for attribute ''changeset'' on new object {1}. Got {0}. Resetting to 0.", v, current.id));
+                        current.changesetId = 0;
+                    } else {
+                        // for an existing primitive this is a problem
+                        throwException(tr("Illegal value for attribute ''changeset''. Got {0}.", v));
+                    }
+                }
             }
         }
@@ -417 +458 @@
     protected void processWaysAfterParsing() throws IllegalDataException{
         for (Long externalWayId: ways.keySet()) {
-            Way w = (Way)externalIdMap.get("w" + externalWayId);
+            Way w = (Way)externalIdMap.get(new SimplePrimitiveId(externalWayId, OsmPrimitiveType.WAY));
             List<Node> wayNodes = new ArrayList<Node>();
             for (long id : ways.get(externalWayId)) {
-                Node n = (Node)externalIdMap.get("n" +id);
+                Node n = (Node)externalIdMap.get(new SimplePrimitiveId(id, OsmPrimitiveType.NODE));
                 if (n == null) {
                     if (id <= 0)
@@ -476 +517 @@
     private void processRelationsAfterParsing() throws IllegalDataException {
         for (Long externalRelationId : relations.keySet()) {
-            Relation relation = (Relation) externalIdMap.get("r" +externalRelationId);
+            Relation relation = (Relation) externalIdMap.get(
+                    new SimplePrimitiveId(externalRelationId, OsmPrimitiveType.RELATION)
+            );
             List<RelationMember> relationMembers = new ArrayList<RelationMember>();
             for (RelationMemberData rm : relations.get(externalRelationId)) {
@@ -483 +526 @@
                 // lookup the member from the map of already created primitives
                 //
-                if (rm.type.equals("node")) {
-                    primitive = externalIdMap.get("n" + rm.id);
-                } else if (rm.type.equals("way")) {
-                    primitive = externalIdMap.get("w" + rm.id);
-                } else if (rm.type.equals("relation")) {
-                    primitive = externalIdMap.get("r" + rm.id);
-                } else
+                try {
+                    OsmPrimitiveType type = OsmPrimitiveType.fromApiTypeName(rm.type);
+                    primitive = externalIdMap.get(new SimplePrimitiveId(rm.id, type));
+                } catch(IllegalArgumentException e) {
                     throw new IllegalDataException(
                             tr("Unknown relation member type ''{0}'' in relation with external id ''{1}''.", rm.type,externalRelationId)
                     );
+                }
 
                 if (primitive == null) {
@@ -523 +564 @@
                     }
                     ds.addPrimitive(primitive);
-
-                    if (rm.type.equals("node")) {
-                        externalIdMap.put("n" + rm.id, primitive);
-                    } else if (rm.type.equals("way")) {
-                        externalIdMap.put("w" + rm.id, primitive);
-                    } else if (rm.type.equals("relation")) {
-                        externalIdMap.put("r" + rm.id, primitive);
-                    }
-
+                    externalIdMap.put(new SimplePrimitiveId(rm.id, OsmPrimitiveType.fromApiTypeName(rm.type)), primitive);
                 }
                 relationMembers.add(new RelationMember(rm.role, primitive));
@@ -543 +576 @@
      * Parse the given input source and return the dataset.
      *
-     * @param source the source input stream
-     * @param progressMonitor  the progress monitor
+     * @param source the source input stream. Must not be null.
+     * @param progressMonitor  the progress monitor. If null, {@see NullProgressMonitor#INSTANCE} is assumed
      *
      * @return the dataset with the parsed data
      * @throws IllegalDataException thrown if the an error was found while parsing the data from the source
+     * @throws IllegalArgumentException thrown if source is null
      */
     public static DataSet parseDataSet(InputStream source, ProgressMonitor progressMonitor) throws IllegalDataException {
+        if (progressMonitor == null) {
+            progressMonitor = NullProgressMonitor.INSTANCE;
+        }
+        if (source == null)
+            throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "source"));
         OsmReader reader = new OsmReader();
         try {

trunk/src/org/openstreetmap/josm/io/OsmWriter.java

@@ -2 +2 @@
 package org.openstreetmap.josm.io;
 
+import static org.openstreetmap.josm.tools.I18n.tr;
+
 import java.io.PrintWriter;
-import java.util.HashMap;
 import java.util.Map.Entry;
 
@@ -29 +30 @@
     public final String DEFAULT_API_VERSION = "0.6";
 
-    /**
-     * The counter for newly created objects. Starts at -1 and goes down.
-     */
-    private long newIdCounter = -1;
-
-    /**
-     * All newly created ids and their primitive that uses it. This is a back reference
-     * map to allow references to use the correnct primitives.
-     */
-    public HashMap<OsmPrimitive, Long> usedNewIds = new HashMap<OsmPrimitive, Long>();
-
     private boolean osmConform;
     private boolean withBody = true;
@@ -123 +113 @@
             out.println(">");
             for (Node n : w.getNodes()) {
-                out.println("    <nd ref='"+getUsedId(n)+"' />");
+                out.println("    <nd ref='"+n.getUniqueId()+"' />");
             }
             addTags(w, "way", false);
@@ -139 +129 @@
                 out.print("    <member type='");
                 out.print(OsmPrimitiveType.from(em.getMember()).getAPIName());
-                out.println("' ref='"+getUsedId(em.getMember())+"' role='" +
+                out.println("' ref='"+em.getMember().getUniqueId()+"' role='" +
                         XmlWriter.encode(em.getRole()) + "' />");
             }
@@ -172 +162 @@
     }
 
-    /**
-     * Return the id for the given osm primitive (may access the usedId map)
-     */
-    private long getUsedId(OsmPrimitive osm) {
-        if (!osm.isNew())
-            return osm.getId();
-        if (usedNewIds.containsKey(osm))
-            return usedNewIds.get(osm);
-        usedNewIds.put(osm, newIdCounter);
-        return osmConform ? 0 : newIdCounter--;
-    }
-
     private void addTags(Tagged osm, String tagname, boolean tagOpen) {
         if (osm.hasKeys()) {
@@ -208 +186 @@
      */
     private void addCommon(OsmPrimitive osm, String tagname) {
-        long id = getUsedId(osm);
         out.print("  <"+tagname);
-        if (id != 0) {
-            out.print(" id='"+getUsedId(osm)+"'");
-        }
+        if (osm.getUniqueId() != 0) {
+            out.print(" id='"+ osm.getUniqueId()+"'");
+        } else
+            throw new IllegalStateException(tr("Unexpected id 0 for osm primitive found"));
         if (!osmConform) {
             String action = null;
@@ -243 +221 @@
         if (this.changeset != null && this.changeset.getId() != 0) {
             out.print(" changeset='"+this.changeset.getId()+"'" );
+        } else if (osm.getChangesetId() > 0 && !osm.isNew()) {
+            out.print(" changeset='"+osm.getChangesetId()+"'" );
         }
     }