Changeset 2070 in josm for trunk/src/org/openstreetmap/josm/io

Timestamp:

2009-09-06T23:07:33+02:00 (15 years ago)

Author:

Gubaer

Message:

new: rewrite of CombineWay action
new: conflict resolution dialog for CombineWay, including conflicts for different relation memberships
cleanup: cleanup in OsmReader, reduces memory footprint and reduces parsing time
cleanup: made most of the public fields in OsmPrimitive @deprecated, added accessors and changed the code
cleanup: replaced usages of @deprecated constructors for ExtendedDialog
fixed #3208: Combine ways brokes relation order

WARNING: this changeset touches a lot of code all over the code base. "latest" might become slightly unstable in the next days. Also experience incompatibility issues with plugins in the next few days.

Location:

trunk/src/org/openstreetmap/josm/io

Files:

• DiffResultReader.java (modified) (3 diffs)
• FileImporter.java (modified) (1 diff)
• IllegalDataException.java (added)
• MultiFetchServerObjectReader.java (modified) (3 diffs)
• OsmApi.java (modified) (4 diffs)
• OsmBzip2Importer.java (modified) (2 diffs)
• OsmGzipImporter.java (modified) (2 diffs)
• OsmImporter.java (modified) (3 diffs)
• OsmReader.java (modified) (11 diffs)
• OsmServerBackreferenceReader.java (modified) (3 diffs)
• OsmServerObjectReader.java (modified) (1 diff)
• OsmWriter.java (modified) (2 diffs)

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

@@ -94 +94 @@
     public void visit(Node n) {
         String key = "node:" + (newIdMap.containsKey(n) ? newIdMap.get(n) : n.getId());
-        System.out.println("key: "+key);
         Long[] nv = versions.get(key);
         if (nv != null) {
             processed.add(n);
             if (!n.isDeleted()) {
-                n.id = nv[0]; n.version = nv[1].intValue();
+                n.setOsmId(nv[0], nv[1].intValue());
             }
         }
@@ -109 +108 @@
             processed.add(w);
             if (!w.isDeleted()) {
-                w.id = nv[0]; w.version = nv[1].intValue();
+                w.setOsmId(nv[0], nv[1].intValue());
             }
         }
@@ -119 +118 @@
             processed.add(r);
             if (!r.isDeleted()) {
-                r.id = nv[0]; r.version = nv[1].intValue();
+                r.setOsmId(nv[0], nv[1].intValue());
             }
         }

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

@@ -21 +21 @@
     }
 
-    public void importData(File file) throws IOException {
+    public void importData(File file) throws IOException, IllegalDataException {
         throw new IOException(tr("Could not read \"{0}\"", file.getName()));
     }

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

@@ -204 +204 @@
         remember(relation.getId(), OsmPrimitiveType.RELATION);
         for (RelationMember member : relation.getMembers()) {
-            if (OsmPrimitiveType.from(member.member).equals(OsmPrimitiveType.RELATION)) {
+            if (OsmPrimitiveType.from(member.getMember()).equals(OsmPrimitiveType.RELATION)) {
                 // avoid infinite recursion in case of cyclic dependencies in relations
                 //
-                if (relations.contains(member.member.getId())) {
+                if (relations.contains(member.getMember().getId())) {
                     continue;
                 }
@@ -325 +325 @@
         progressMonitor.subTask(tr("Downloading OSM data..."));
         try {
-            final OsmReader osm = OsmReader.parseDataSetOsm(in, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
-            merge(osm.getDs());
+
+            merge(
+                    OsmReader.parseDataSet(in, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false))
+            );
         } catch(Exception e) {
             throw new OsmTransferException(e);
@@ -348 +350 @@
         progressMonitor.subTask(tr("Downloading OSM data..."));
         try {
-            final OsmReader osm = OsmReader.parseDataSetOsm(in, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
-            merge(osm.getDs());
+            merge(
+                    OsmReader.parseDataSet(in, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false))
+            );
         } catch(Exception e) {
             throw new OsmTransferException(e);

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

@@ -233 +233 @@
         try {
             ret = sendRequest("PUT", OsmPrimitiveType.from(osm).getAPIName()+"/create", toXml(osm, true),monitor);
-            osm.id = Long.parseLong(ret.trim());
-            osm.version = 1;
+            osm.setOsmId(Long.parseLong(ret.trim()), 1);
         } catch(NumberFormatException e){
             throw new OsmTransferException(tr("unexpected format of id replied by the server, got ''{0}''", ret));
@@ -258 +257 @@
             try {
                 ret = sendRequest("PUT", OsmPrimitiveType.from(osm).getAPIName()+"/" + osm.getId(), toXml(osm, true), monitor);
-                osm.version = Integer.parseInt(ret.trim());
+                osm.setOsmId(osm.getId(), Integer.parseInt(ret.trim()));
             } catch(NumberFormatException e) {
                 throw new OsmTransferException(tr("unexpected format of new version of modified primitive ''{0}'', got ''{1}''", osm.getId(), ret));
@@ -341 +340 @@
                     return;
                 }
-                changeset.id = this.changeset.getId();
-                this.changeset.cloneFrom(changeset);
+                this.changeset.setKeys(changeset.getKeys());
                 progressMonitor.setCustomText(tr("Updating changeset {0}...", changeset.getId()));
                 sendRequest(
@@ -473 +471 @@
      */
     private String sendRequest(String requestMethod, String urlSuffix,String requestBody, ProgressMonitor monitor) throws OsmTransferException {
-
         StringBuffer responseBody = new StringBuffer();
 

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

@@ -11 +11 @@
 import org.apache.tools.bzip2.CBZip2InputStream;
 import org.openstreetmap.josm.actions.ExtensionFileFilter;
-import org.xml.sax.SAXException;
 
 public class OsmBzip2Importer extends OsmImporter {
@@ -21 +20 @@
 
     @Override
-    public void importData(File file) throws IOException {
+    public void importData(File file) throws IOException, IllegalDataException {
         BufferedInputStream bis = new BufferedInputStream(new FileInputStream(file));
         int b = bis.read();
-        if (b != 'B') {
+        if (b != 'B')
             throw new IOException(tr("Invalid bz2 file."));
-        }
         b = bis.read();
-        if (b != 'Z') {
+        if (b != 'Z')
             throw new IOException(tr("Invalid bz2 file."));
-        }
         CBZip2InputStream in = new CBZip2InputStream(bis);
-
-        try {
-            importData(in, file);
-        } catch (SAXException e) {
-            e.printStackTrace();
-            throw new IOException(tr("Could not read \"{0}\"", file.getName()));
-        }
+        importData(in, file);
     }
 }

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

@@ -10 +10 @@
 
 import org.openstreetmap.josm.actions.ExtensionFileFilter;
-import org.xml.sax.SAXException;
 
 public class OsmGzipImporter extends OsmImporter {
@@ -19 +18 @@
 
     @Override
-    public void importData(File file) throws IOException {
+    public void importData(File file) throws IOException, IllegalDataException {
         GZIPInputStream in = new GZIPInputStream(new FileInputStream(file));
-        try {
-            importData(in, file);
-        } catch (SAXException e) {
-            e.printStackTrace();
-            throw new IOException(tr("Could not read \"{0}\"", file.getName()));
-        }
+        importData(in, file);
     }
 }

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

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
-import java.awt.HeadlessException;
 import java.io.File;
 import java.io.FileInputStream;
@@ -18 +17 @@
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
-import org.xml.sax.SAXException;
 
 public class OsmImporter extends FileImporter {
@@ -30 +28 @@
     }
 
-    @Override public void importData(File file) throws IOException {
+    @Override public void importData(File file) throws IOException, IllegalDataException {
         try {
             FileInputStream in = new FileInputStream(file);
             importData(in, file);
-        } catch (HeadlessException e) {
-            e.printStackTrace();
-            throw new IOException(tr("Could not read \"{0}\"", file.getName()));
         } catch (FileNotFoundException e) {
             e.printStackTrace();
             throw new IOException(tr("File \"{0}\" does not exist", file.getName()));
-        } catch (SAXException e) {
-            e.printStackTrace();
-            throw new IOException(tr("Parsing file \"{0}\" failed", file.getName()));
         }
     }
 
-    protected void importData(InputStream in, File associatedFile) throws SAXException, IOException {
-        OsmReader osm = OsmReader.parseDataSetOsm(in, NullProgressMonitor.INSTANCE);
-        DataSet dataSet = osm.getDs();
+    protected void importData(InputStream in, File associatedFile) throws IllegalDataException {
+        DataSet dataSet = OsmReader.parseDataSet(in, NullProgressMonitor.INSTANCE);
         final OsmDataLayer layer = new OsmDataLayer(dataSet, associatedFile.getName(), associatedFile);
         // FIXME: remove UI stuff from IO subsystem

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

@@ -1 @@
-// License: GPL. Copyright 2007 by Immanuel Scholz and others
 package org.openstreetmap.josm.io;
 
 import static org.openstreetmap.josm.tools.I18n.tr;
 
-import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -14 +12 @@
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.logging.Logger;
 
@@ -34 +31 @@
 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;
@@ -40 +38 @@
  * Parser for the Osm Api. Read from an input stream and construct a dataset out of it.
  *
- * Reading process takes place in three phases. During the first phase (including xml parse),
- * all nodes are read and stored. Other information than nodes are stored in a raw list
- *
- * The second phase read all ways out of the remaining objects in the raw list.
- *
- * @author Imi
  */
 public class OsmReader {
@@ -54 +46 @@
      */
     private DataSet ds = new DataSet();
-    public DataSet getDs() { return ds; }
-
-    /**
-     * All read nodes after phase 1.
-     */
-    private Map<Long, Node> nodes = new HashMap<Long, Node>();
+
+    /**
+     * Replies the parsed data set
+     * 
+     * @return the parsed data set
+     */
+    public DataSet getDataSet() {
+        return ds;
+    }
+
+    /** the map from external ids to read OsmPrimitives. External ids are
+     * longs too, but in contrast to internal ids negative values are used
+     * to identify primitives unknown to the OSM server
+     * 
+     * The keys are strings composed as follows
+     * <ul>
+     *   <li>"n" + id  for nodes</li>
+     *   <li>"w" + id  for nodes</li>
+     *   <li>"r" + id  for nodes</li>
+     * </ul>
+     */
+    private Map<String, OsmPrimitive> externalIdMap = new HashMap<String, OsmPrimitive>();
 
 
@@ -69 +77 @@
      */
     private OsmReader() {
+        externalIdMap = new HashMap<String, OsmPrimitive>();
     }
 
     private static class OsmPrimitiveData {
         public long id = 0;
-        public Map<String,String> keys = new HashMap<String, String>();
         public boolean modified = false;
-        public boolean selected = false;
         public boolean deleted = false;
         public Date timestamp = new Date();
         public User user = null;
         public boolean visible = true;
-        public int version = -1;
+        public int version = 0;
         public LatLon latlon = new LatLon(0,0);
+        private OsmPrimitive primitive;
 
         public void copyTo(OsmPrimitive osm) {
-            osm.id = id;
-            osm.setKeys(keys);
-            osm.modified = modified;
-            osm.setSelected(selected);
-            osm.deleted = deleted;
+            osm.setModified(modified);
+            osm.setDeleted(deleted);
+            //  id < 0 possible if read from a file
+            if (id <= 0) {
+                osm.clearOsmId();
+            } else {
+                osm.setOsmId(id, version);
+            }
             osm.setTimestamp(timestamp);
             osm.user = user;
-            osm.visible = visible;
-            osm.version = version;
+            osm.setVisible(visible);
             osm.mappaintStyle = null;
         }
 
         public Node createNode() {
-            Node node = new Node(latlon);
+            Node node = new Node();
+            node.setCoor(latlon);
             copyTo(node);
+            primitive = node;
             return node;
         }
 
         public Way createWay() {
-            Way way = new Way(id);
+            Way way = new Way();
             copyTo(way);
+            primitive = way;
             return way;
         }
-
         public Relation createRelation() {
-            Relation rel = new Relation(id);
-            copyTo(rel);
-            return rel;
+            Relation relation= new Relation();
+            copyTo(relation);
+            primitive = relation;
+            return relation;
+        }
+
+        public void rememberTag(String key, String value) {
+            primitive.put(key, value);
         }
     }
@@ -128 +145 @@
      * Data structure for the remaining way objects
      */
-    private Map<OsmPrimitiveData, Collection<Long>> ways = new HashMap<OsmPrimitiveData, Collection<Long>>();
+    private Map<Long, Collection<Long>> ways = new HashMap<Long, Collection<Long>>();
 
     /**
      * Data structure for relation objects
      */
-    private Map<OsmPrimitiveData, Collection<RelationMemberData>> relations = new HashMap<OsmPrimitiveData, Collection<RelationMemberData>>();
+    private Map<Long, Collection<RelationMemberData>> relations = new HashMap<Long, Collection<RelationMemberData>>();
+
+    static public class OsmDataParsingException extends SAXException {
+        private int columnNumber;
+        private int lineNumber;
+
+        public OsmDataParsingException() {
+            super();
+        }
+
+        public OsmDataParsingException(Exception e) {
+            super(e);
+        }
+
+        public OsmDataParsingException(String message, Exception e) {
+            super(message, e);
+        }
+
+        public OsmDataParsingException(String message) {
+            super(message);
+        }
+
+        public OsmDataParsingException rememberLocation(Locator locator) {
+            if (locator == null) return this;
+            this.columnNumber = locator.getColumnNumber();
+            this.lineNumber = locator.getLineNumber();
+            return this;
+        }
+
+        @Override
+        public String getMessage() {
+            String msg = super.getMessage();
+            if (lineNumber == 0 && columnNumber == 0)
+                return msg;
+            if (msg == null) {
+                msg = getClass().getName();
+            }
+            msg = msg + " " + tr("(at line {0}, column {1})", lineNumber, columnNumber);
+            return msg;
+        }
+
+        public int getColumnNumber() {
+            return columnNumber;
+        }
+
+        public int getLineNumber() {
+            return lineNumber;
+        }
+    }
 
     private class Parser extends DefaultHandler {
+        private Locator locator;
+
+        @Override
+        public void setDocumentLocator(Locator locator) {
+            this.locator = locator;
+        }
+
+        protected void throwException(String msg) throws OsmDataParsingException{
+            throw new OsmDataParsingException(msg).rememberLocation(locator);
+        }
         /**
          * The current osm primitive to be read.
@@ -143 +218 @@
 
         @Override public void startElement(String namespaceURI, String localName, String qName, Attributes atts) throws SAXException {
-            try {
-                if (qName.equals("osm")) {
-                    if (atts == null)
-                        throw new SAXException(tr("Unknown version"));
-                    String v = atts.getValue("version");
-                    if (v == null)
-                        throw new SAXException(tr("Version number missing from OSM data"));
-                    if (!(v.equals("0.5") || v.equals("0.6")))
-                        throw new SAXException(tr("Unknown version: {0}", v));
-                    // save generator attribute for later use when creating DataSource objects
-                    generator = atts.getValue("generator");
-                    ds.version = v;
-
-                } else if (qName.equals("bounds")) {
-                    // new style bounds.
-                    String minlon = atts.getValue("minlon");
-                    String minlat = atts.getValue("minlat");
-                    String maxlon = atts.getValue("maxlon");
-                    String maxlat = atts.getValue("maxlat");
-                    String origin = atts.getValue("origin");
-                    if (minlon != null && maxlon != null && minlat != null && maxlat != null) {
-                        if (origin == null) {
-                            origin = generator;
-                        }
-                        Bounds bounds = new Bounds(
-                                new LatLon(Double.parseDouble(minlat), Double.parseDouble(minlon)),
-                                new LatLon(Double.parseDouble(maxlat), Double.parseDouble(maxlon)));
-                        DataSource src = new DataSource(bounds, origin);
-                        ds.dataSources.add(src);
+            if (qName.equals("osm")) {
+                if (atts == null) {
+                    throwException(tr("Missing mandatory attribute ''{0}'' of XML element {1}", "version", "osm"));
+                }
+                String v = atts.getValue("version");
+                if (v == null) {
+                    throwException(tr("Missing mandatory attribute ''{0}''", "version"));
+                }
+                if (!(v.equals("0.5") || v.equals("0.6"))) {
+                    throwException(tr("Unsupported version: {0}", v));
+                }
+                // save generator attribute for later use when creating DataSource objects
+                generator = atts.getValue("generator");
+                ds.version = v;
+
+            } else if (qName.equals("bounds")) {
+                // new style bounds.
+                String minlon = atts.getValue("minlon");
+                String minlat = atts.getValue("minlat");
+                String maxlon = atts.getValue("maxlon");
+                String maxlat = atts.getValue("maxlat");
+                String origin = atts.getValue("origin");
+                if (minlon != null && maxlon != null && minlat != null && maxlat != null) {
+                    if (origin == null) {
+                        origin = generator;
                     }
-
-                    // ---- PARSING NODES AND WAYS ----
-
-                } else if (qName.equals("node")) {
-                    current = new OsmPrimitiveData();
-                    current.latlon = new LatLon(getDouble(atts, "lat"), getDouble(atts, "lon"));
-                    readCommon(atts, current);
-                } else if (qName.equals("way")) {
-                    current = new OsmPrimitiveData();
-                    readCommon(atts, current);
-                    ways.put(current, new ArrayList<Long>());
-                } else if (qName.equals("nd")) {
-                    Collection<Long> list = ways.get(current);
-                    if (list == null)
-                        throw new SAXException(tr("Found <nd> element in non-way."));
-                    long id = getLong(atts, "ref");
-                    if (id == 0)
-                        throw new SAXException(tr("<nd> has zero ref"));
-                    list.add(id);
-
-                    // ---- PARSING RELATIONS ----
-
-                } else if (qName.equals("relation")) {
-                    current = new OsmPrimitiveData();
-                    readCommon(atts, current);
-                    relations.put(current, new LinkedList<RelationMemberData>());
-                } else if (qName.equals("member")) {
-                    Collection<RelationMemberData> list = relations.get(current);
-                    if (list == null)
-                        throw new SAXException(tr("Found <member> element in non-relation."));
-                    RelationMemberData emd = new RelationMemberData();
-                    String value = atts.getValue("ref");
-                    if (value == null)
-                        throw new SAXException(tr("Missing attribute \"ref\" on member in relation {0}",current.id));
-                    try {
-                        emd.id = Long.parseLong(value);
-                    } catch(NumberFormatException e) {
-                        throw new SAXException(tr("Illegal value for attribute \"ref\" on member in relation {0}, got {1}", Long.toString(current.id),value));
-                    }
-                    value = atts.getValue("type");
-                    if (value == null)
-                        throw new SAXException(tr("Missing attribute \"type\" on member {0} in relation {1}", Long.toString(emd.id), Long.toString(current.id)));
-                    if (! (value.equals("way") || value.equals("node") || value.equals("relation")))
-                        throw new SAXException(tr("Unexpected \"type\" on member {0} in relation {1}, got {2}.", Long.toString(emd.id), Long.toString(current.id), value));
-                    emd.type= value;
-                    value = atts.getValue("role");
-                    emd.role = value;
-
-                    if (emd.id == 0)
-                        throw new SAXException(tr("Incomplete <member> specification with ref=0"));
-
-                    list.add(emd);
-
-                    // ---- PARSING TAGS (applicable to all objects) ----
-
-                } else if (qName.equals("tag")) {
-                    String key = atts.getValue("k");
-                    String value = atts.getValue("v");
-                    current.keys.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);
-            }
-        }
-
-        @Override
-        public void endElement(String uri, String localName, String qName) throws SAXException {
-            if (qName.equals("node")) {
-                nodes.put(current.id, current.createNode());
+                    Bounds bounds = new Bounds(
+                            new LatLon(Double.parseDouble(minlat), Double.parseDouble(minlon)),
+                            new LatLon(Double.parseDouble(maxlat), Double.parseDouble(maxlon)));
+                    DataSource src = new DataSource(bounds, origin);
+                    ds.dataSources.add(src);
+                } else {
+                    throwException(tr(
+                            "Missing manadatory attributes on element ''bounds''. Got minlon=''{0}'',minlat=''{1}00,maxlon=''{3}'',maxlat=''{4}'', origin=''{5}''",
+                            minlon, minlat, maxlon, maxlat, origin
+                    ));
+                }
+
+                // ---- PARSING NODES AND WAYS ----
+
+            } else if (qName.equals("node")) {
+                current = new OsmPrimitiveData();
+                current.latlon = new LatLon(getDouble(atts, "lat"), getDouble(atts, "lon"));
+                readCommon(atts, current);
+                Node n = current.createNode();
+                externalIdMap.put("n"+current.id, n);
+            } else if (qName.equals("way")) {
+                current = new OsmPrimitiveData();
+                readCommon(atts, current);
+                Way w = current.createWay();
+                externalIdMap.put("w"+current.id, w);
+                ways.put(current.id, new ArrayList<Long>());
+            } else if (qName.equals("nd")) {
+                Collection<Long> list = ways.get(current.id);
+                if (list == null) {
+                    throwException(
+                            tr("found XML element <nd> element not as direct child of element <way>")
+                    );
+                }
+                if (atts.getValue("ref") == null) {
+                    throwException(
+                            tr("Missing mandatory attribute ''{0}'' on <nd> of way {1}", "ref", current.id)
+                    );
+                }
+                long id = getLong(atts, "ref");
+                if (id == 0) {
+                    throwException(
+                            tr("Illegal value of attribute ''ref'' of element <nd>. Got {0}", id)
+                    );
+                }
+                list.add(id);
+
+                // ---- PARSING RELATIONS ----
+
+            } else if (qName.equals("relation")) {
+                current = new OsmPrimitiveData();
+                readCommon(atts, current);
+                Relation r = current.createRelation();
+                externalIdMap.put("r"+current.id, r );
+                relations.put(current.id, new LinkedList<RelationMemberData>());
+            } else if (qName.equals("member")) {
+                Collection<RelationMemberData> list = relations.get(current.id);
+                if (list == null) {
+                    throwException(
+                            tr("Found XML element <member> not as direct child of element <relation>")
+                    );
+                }
+                RelationMemberData emd = new RelationMemberData();
+                String value = atts.getValue("ref");
+                if (value == null) {
+                    throwException(tr("Missing attribute ''ref'' on member in relation {0}",current.id));
+                }
+                try {
+                    emd.id = Long.parseLong(value);
+                } catch(NumberFormatException e) {
+                    throwException(tr("Illegal value for attribute ''ref'' on member in relation {0}. Got {1}", Long.toString(current.id),value));
+                }
+                value = atts.getValue("type");
+                if (value == null) {
+                    throwException(tr("Missing attribute ''type'' on member {0} in relation {1}", Long.toString(emd.id), Long.toString(current.id)));
+                }
+                if (! (value.equals("way") || value.equals("node") || value.equals("relation"))) {
+                    throwException(tr("Illegal value for attribute ''type'' on member {0} in relation {1}. Got {2}.", Long.toString(emd.id), Long.toString(current.id), value));
+                }
+                emd.type= value;
+                value = atts.getValue("role");
+                emd.role = value;
+
+                if (emd.id == 0) {
+                    throwException(tr("Incomplete <member> specification with ref=0"));
+                }
+
+                list.add(emd);
+
+                // ---- PARSING TAGS (applicable to all objects) ----
+
+            } else if (qName.equals("tag")) {
+                String key = atts.getValue("k");
+                String value = atts.getValue("v");
+                current.rememberTag(key, value);
             }
         }
@@ -252 +344 @@
             return Double.parseDouble(atts.getValue(value));
         }
-    }
-
-    /**
-     * Read out the common attributes from atts and put them into this.current.
-     */
-    void readCommon(Attributes atts, OsmPrimitiveData current) throws SAXException {
-        current.id = getLong(atts, "id");
-        if (current.id == 0)
-            throw new SAXException(tr("Illegal object with id=0"));
-
-        String time = atts.getValue("timestamp");
-        if (time != null && time.length() != 0) {
-            current.timestamp =  DateUtils.fromString(time);
-        }
-
-        // user attribute added in 0.4 API
-        String user = atts.getValue("user");
-        if (user != null) {
-            // do not store literally; get object reference for string
-            current.user = User.get(user);
-        }
-
-        // uid attribute added in 0.6 API
-        String uid = atts.getValue("uid");
-        if (uid != null) {
-            if (current.user != null) {
-                current.user.uid = uid;
-            }
-        }
-
-        // visible attribute added in 0.4 API
-        String visible = atts.getValue("visible");
-        if (visible != null) {
-            current.visible = Boolean.parseBoolean(visible);
-        }
-
-        String version = atts.getValue("version");
-        current.version = 0;
-        if (version != null) {
+
+        private User createUser(String uid, String name) throws SAXException {
+            if (uid == null) {
+                if (name == null)
+                    return null;
+                return User.createLocalUser(name);
+            }
             try {
-                current.version = Integer.parseInt(version);
+                long id = Long.parseLong(uid);
+                return User.createOsmUser(id, name);
             } catch(NumberFormatException e) {
-                throw new SAXException(tr("Illegal value for attribute \"version\" on OSM primitive with id {0}, got {1}", Long.toString(current.id), version));
-            }
-        } else {
-            // version expected for OSM primitives with an id assigned by the server (id > 0), since API 0.6
-            //
-            if (current.id > 0 && ds.version != null && ds.version.equals("0.6"))
-                throw new SAXException(tr("Missing attribute \"version\" on OSM primitive with id {0}", Long.toString(current.id)));
-        }
-
-        String action = atts.getValue("action");
-        if (action == null)
-            return;
-        if (action.equals("delete")) {
-            current.deleted = true;
-        } else if (action.startsWith("modify")) {
-            current.modified = true;
-        }
-    }
-    private long getLong(Attributes atts, String value) throws SAXException {
-        String s = atts.getValue(value);
-        if (s == null)
-            throw new SAXException(tr("Missing required attribute \"{0}\".",value));
-        return Long.parseLong(s);
-    }
-
-    private Node findNode(long id) {
-        Node n = nodes.get(id);
-        if (n != null)
-            return n;
-        return null;
-    }
-
-    protected void createWays() {
-        for (Entry<OsmPrimitiveData, Collection<Long>> e : ways.entrySet()) {
-            Way w = new Way(e.getKey().id);
+                throwException(tr("Illegal value for attribute ''uid''. Got ''{0}''", uid));
+            }
+            return null;
+        }
+        /**
+         * Read out the common attributes from atts and put them into this.current.
+         */
+        void readCommon(Attributes atts, OsmPrimitiveData current) throws SAXException {
+            current.id = getLong(atts, "id");
+            if (current.id == 0) {
+                throwException(tr("Illegal object with id=0"));
+            }
+
+            String time = atts.getValue("timestamp");
+            if (time != null && time.length() != 0) {
+                current.timestamp =  DateUtils.fromString(time);
+            }
+
+            // user attribute added in 0.4 API
+            String user = atts.getValue("user");
+            // uid attribute added in 0.6 API
+            String uid = atts.getValue("uid");
+            current.user = createUser(uid, user);
+
+            // visible attribute added in 0.4 API
+            String visible = atts.getValue("visible");
+            if (visible != null) {
+                current.visible = Boolean.parseBoolean(visible);
+            }
+
+            String version = atts.getValue("version");
+            current.version = 0;
+            if (version != null) {
+                try {
+                    current.version = Integer.parseInt(version);
+                } catch(NumberFormatException e) {
+                    throwException(tr("Illegal value for attribute ''version'' on OSM primitive with id {0}. Got {1}", Long.toString(current.id), version));
+                }
+                if (current.version <= 0) {
+                    throwException(tr("Illegal value for attribute ''version'' on OSM primitive with id {0}. Got {1}", Long.toString(current.id), version));
+                }
+            } else {
+                // version expected for OSM primitives with an id assigned by the server (id > 0), since API 0.6
+                //
+                if (current.id > 0 && ds.version != null && ds.version.equals("0.6")) {
+                    throwException(tr("Missing attribute ''version'' on OSM primitive with id {0}", Long.toString(current.id)));
+                }
+            }
+
+            String action = atts.getValue("action");
+            if (action == null)
+                return;
+            if (action.equals("delete")) {
+                current.deleted = true;
+            } else if (action.startsWith("modify")) {
+                current.modified = true;
+            }
+        }
+
+        private long getLong(Attributes atts, String name) throws SAXException {
+            String value = atts.getValue(name);
+            if (value == null) {
+                throwException(tr("Missing required attribute ''{0}''.",name));
+            }
+            try {
+                return Long.parseLong(value);
+            } catch(NumberFormatException e) {
+                throwException(tr("Illegal long value for attribute ''{0}''. Got ''{1}''",name, value));
+            }
+            return 0; // should not happen
+        }
+    }
+
+
+    /**
+     * Processes the ways after parsing. Rebuilds the list of nodes of each way and
+     * adds the way to the dataset
+     * 
+     * @throws IllegalDataException thrown if a data integrity problem is detected
+     */
+    protected void processWaysAfterParsing() throws IllegalDataException{
+        for (Long externalWayId: ways.keySet()) {
+            Way w = (Way)externalIdMap.get("w" + externalWayId);
             boolean incomplete = false;
             List<Node> wayNodes = new ArrayList<Node>();
-            for (long id : e.getValue()) {
-                Node n = findNode(id);
+            for (long id : ways.get(externalWayId)) {
+                Node n = (Node)externalIdMap.get("n" +id);
                 if (n == null) {
+                    if (id <= 0)
+                        throw new IllegalDataException (
+                                tr(
+                                        "way with external id ''{0}'' includes missing node with external id ''{1}''",
+                                        externalWayId,
+                                        id
+                                )
+                        );
                     n = new Node(id);
                     n.incomplete = true;
@@ -343 +460 @@
             if (incomplete) {
                 logger.warning(tr("marked way {0} with {1} nodes incomplete because at least one node was missing in the " +
-                                "loaded data and is therefore incomplete too", e.getKey().id, w.getNodesCount()));
-                e.getKey().copyTo(w);
+                        "loaded data and is therefore incomplete too", externalWayId, w.getNodesCount()));
                 w.incomplete = true;
                 ds.addPrimitive(w);
             } else {
-                e.getKey().copyTo(w);
                 w.incomplete = false;
                 ds.addPrimitive(w);
@@ -356 +471 @@
 
     /**
-     * Return the Way object with the given id, or null if it doesn't
-     * exist yet. This method only looks at ways stored in the already parsed
-     * ways.
-     *
-     * @param id
-     * @return way object or null
-     */
-    private Way findWay(long id) {
-        for (Way way : ds.ways)
-            if (way.id == id)
-                return way;
-        return null;
-    }
-
-    /**
-     * Return the Relation object with the given id, or null if it doesn't
-     * exist yet. This method only looks at relations in the already parsed
-     * relations.
-     *
-     * @param id
-     * @return relation object or null
-     */
-    private Relation findRelation(long id) {
-        for (Relation e : ds.relations)
-            if (e.id == id)
-                return e;
-        return null;
-    }
-
-    /**
-     * Create relations. This is slightly different than n/s/w because
-     * unlike other objects, relations may reference other relations; it
-     * is not guaranteed that a referenced relation will have been created
-     * before it is referenced. So we have to create all relations first,
-     * and populate them later.
-     */
-    private void createRelations() throws SAXException {
-
-        // pass 1 - create all relations
-        for (Entry<OsmPrimitiveData, Collection<RelationMemberData>> e : relations.entrySet()) {
-            Relation en = new Relation();
-            e.getKey().copyTo(en);
-            ds.addPrimitive(en);
-        }
-
-        // Cache the ways here for much better search performance
-        HashMap<Long, Way> hm = new HashMap<Long, Way>(10000);
-        for (Way wy : ds.ways) {
-            hm.put(wy.id, wy);
-        }
-
-        // pass 2 - sort out members
-        for (Entry<OsmPrimitiveData, Collection<RelationMemberData>> e : relations.entrySet()) {
-            Relation en = findRelation(e.getKey().id);
-            if (en == null) throw new Error("Failed to create relation " + e.getKey().id);
-
+     * Processes the parsed nodes after parsing. Just adds them to
+     * the dataset
+     * 
+     */
+    protected void processNodesAfterParsing() {
+        for (OsmPrimitive primitive: externalIdMap.values()) {
+            if (primitive instanceof Node) {
+                this.ds.addPrimitive(primitive);
+            }
+        }
+    }
+
+    /**
+     * Completes the parsed relations with its members.
+     * 
+     * @throws IllegalDataException thrown if a data integrity problem is detected, i.e. if a
+     * relation member refers to a local primitive which wasn't available in the data
+     * 
+     */
+    private void processRelationsAfterParsing() throws IllegalDataException {
+        for (Long externalRelationId : relations.keySet()) {
+            Relation relation = (Relation) externalIdMap.get("r" +externalRelationId);
             List<RelationMember> relationMembers = new ArrayList<RelationMember>();
-
-            for (RelationMemberData emd : e.getValue()) {
-                OsmPrimitive member;
-                if (emd.type.equals("node")) {
-                    member = findNode(emd.id);
-                    if (member == null) {
-                        member = new Node(emd.id);
-                        ds.addPrimitive(member);
+            for (RelationMemberData rm : relations.get(externalRelationId)) {
+                OsmPrimitive primitive = null;
+
+                // 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
+                    throw new IllegalDataException(
+                            tr("Unknown relation member type ''{0}'' in relation with external id ''{1}''", rm.type,externalRelationId)
+                    );
+
+                if (primitive == null) {
+                    if (rm.id <= 0)
+                        // relation member refers to a primitive with a negative id which was not
+                        // found in the data. This is always a data integrity problem and we abort
+                        // with an exception
+                        //
+                        throw new IllegalDataException(
+                                tr(
+                                        "Relation with external id ''{0}'' refers to missing primitive with external id ''{1}''",
+                                        externalRelationId,
+                                        rm.id
+                                )
+                        );
+
+                    // member refers to OSM primitive which was not present in the parsed data
+                    // -> create a new incomplete primitive and add it to the dataset
+                    //
+                    if (rm.type.equals("node")) {
+                        primitive = new Node(rm.id);
+                    } else if (rm.type.equals("way")) {
+                        primitive = new Way(rm.id);
+                    } else if (rm.type.equals("relation")) {
+                        primitive = new Relation(rm.id);
+                    } else {
+                        // can't happen, we've been testing for valid member types
+                        // at the beginning of this method
+                        //
                     }
-                } else if (emd.type.equals("way")) {
-                    member = hm.get(emd.id);
-                    if (member == null) {
-                        member = findWay(emd.id);
-                    }
-                    if (member == null) {
-                        member = new Way(emd.id);
-                        ds.addPrimitive(member);
-                    }
-                } else if (emd.type.equals("relation")) {
-                    member = findRelation(emd.id);
-                    if (member == null) {
-                        member = new Relation(emd.id);
-                        ds.addPrimitive(member);
-                    }
-                } else {
-                    throw new SAXException(tr("Unknown relation member type {0}", emd.type));
-                }
-                relationMembers.add(new RelationMember(emd.role, member));
-            }
-            en.setMembers(relationMembers);
-        }
-        hm = null;
+                    ds.addPrimitive(primitive);
+                }
+                relationMembers.add(new RelationMember(rm.role, primitive));
+            }
+            relation.setMembers(relationMembers);
+            ds.addPrimitive(relation);
+        }
     }
 
     /**
      * Parse the given input source and return the dataset.
-     * @param ref The dataset that is search in for references first. If
-     *      the Reference is not found here, Main.ds is searched and a copy of the
-     *  element found there is returned.
-     */
-    public static DataSet parseDataSet(InputStream source, ProgressMonitor progressMonitor) throws SAXException, IOException {
-        return parseDataSetOsm(source, progressMonitor).ds;
-    }
-
-    public static OsmReader parseDataSetOsm(InputStream source, ProgressMonitor progressMonitor) throws SAXException, IOException {
+     * 
+     * @param source the source input stream
+     * @param progressMonitor  the progress monitor
+     * 
+     * @return the dataset with the parsed data
+     * @throws IllegalDataException thrown if the an error was found while parsing the data from the source
+     */
+    public static DataSet parseDataSet(InputStream source, ProgressMonitor progressMonitor) throws IllegalDataException {
         OsmReader reader = new OsmReader();
-
-        // phase 1: Parse nodes and read in raw ways
-        InputSource inputSource = new InputSource(new InputStreamReader(source, "UTF-8"));
         try {
+            progressMonitor.beginTask(tr("Prepare OSM data...", 2));
+            progressMonitor.subTask(tr("Parsing OSM data..."));
+            InputSource inputSource = new InputSource(new InputStreamReader(source, "UTF-8"));
             SAXParserFactory.newInstance().newSAXParser().parse(inputSource, reader.new Parser());
-        } catch (ParserConfigurationException e1) {
-            e1.printStackTrace(); // broken SAXException chaining
-            throw new SAXException(e1);
-        }
-
-        progressMonitor.beginTask(tr("Prepare OSM data...", 2));
-        try {
-            for (Node n : reader.nodes.values()) {
-                reader.ds.addPrimitive(n);
-            }
-
             progressMonitor.worked(1);
 
-            try {
-                reader.createWays();
-                reader.createRelations();
-            } catch (NumberFormatException e) {
-                e.printStackTrace();
-                throw new SAXException(tr("Ill-formed node id"));
-            }
-
-            // clear all negative ids (new to this file)
-            for (OsmPrimitive o : reader.ds.allPrimitives())
-                if (o.id < 0) {
-                    o.id = 0;
-                }
-
-            return reader;
+            progressMonitor.subTask(tr("Preparing data set..."));
+            reader.processNodesAfterParsing();
+            reader.processWaysAfterParsing();
+            reader.processRelationsAfterParsing();
+            progressMonitor.worked(1);
+            return reader.getDataSet();
+        } catch(IllegalDataException e) {
+            throw e;
+        } catch(ParserConfigurationException e) {
+            throw new IllegalDataException(e.getMessage(), e);
+        } catch(SAXException e) {
+            throw new IllegalDataException(e.getMessage(), e);
+        } catch(Exception e) {
+            throw new IllegalDataException(e);
         } finally {
             progressMonitor.finishTask();

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

@@ -51 +51 @@
         if (primitive == null)
             throw new IllegalArgumentException(tr("parameter ''{0}'' must not be null", "primitive"));
-        if (primitive.id == 0)
-            throw new IllegalArgumentException(tr("id parameter ''{0}'' > 0 required. Got {1}", "primitive", primitive.id));
-        this.id = primitive.id;
+        if (primitive.getId() == 0)
+            throw new IllegalArgumentException(tr("id parameter ''{0}'' > 0 required. Got {1}", "primitive", primitive.getId()));
+        this.id = primitive.getId();
         this.primitiveType = OsmPrimitiveType.from(primitive);
         this.readFull = false;
@@ -222 +222 @@
             if (isReadFull() ||primitiveType.equals(OsmPrimitiveType.NODE)) {
                 for (Way way: waysToCheck) {
-                    if (way.id > 0 && way.incomplete) {
-                        OsmServerObjectReader reader = new OsmServerObjectReader(way.id, OsmPrimitiveType.from(way), true /* read full */);
+                    if (way.getId() > 0 && way.incomplete) {
+                        OsmServerObjectReader reader = new OsmServerObjectReader(way.getId(), OsmPrimitiveType.from(way), true /* read full */);
                         DataSet wayDs = reader.parseOsm(progressMonitor.createSubTaskMonitor(1, false));
                         MergeVisitor visitor = new MergeVisitor(ds, wayDs);
@@ -233 +233 @@
                 Collection<Relation> relationsToCheck  = new ArrayList<Relation>(ds.relations);
                 for (Relation relation: relationsToCheck) {
-                    if (relation.id > 0 && relation.incomplete) {
-                        OsmServerObjectReader reader = new OsmServerObjectReader(relation.id, OsmPrimitiveType.from(relation), true /* read full */);
+                    if (relation.getId() > 0 && relation.incomplete) {
+                        OsmServerObjectReader reader = new OsmServerObjectReader(relation.getId(), OsmPrimitiveType.from(relation), true /* read full */);
                         DataSet wayDs = reader.parseOsm(progressMonitor.createSubTaskMonitor(1, false));
                         MergeVisitor visitor = new MergeVisitor(ds, wayDs);

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

@@ -46 +46 @@
             if (in == null)
                 return null;
-            final OsmReader osm = OsmReader.parseDataSetOsm(in, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
-            final DataSet data = osm.getDs();
+            final DataSet data = OsmReader.parseDataSet(in, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
             return data;
         } catch(OsmTransferException e) {

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

@@ -14 +14 @@
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
+import org.openstreetmap.josm.data.osm.User;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.osm.visitor.Visitor;
@@ -208 +209 @@
         // user and visible added with 0.4 API
         if (osm.user != null) {
-            out.print(" user='"+XmlWriter.encode(osm.user.name)+"'");
+            if(osm.user.isLocalUser()) {
+                out.print(" user='"+XmlWriter.encode(osm.user.getName())+"'");
+            } else if (osm.user.isOsmUser()) {
+                // uid added with 0.6
+                out.print(" uid='"+ osm.user.getId()+"'");
+                out.print(" user='"+XmlWriter.encode(osm.user.getName())+"'");
+            }
         }
         out.print(" visible='"+osm.isVisible()+"'");
-        if (osm.version != -1) {
-            out.print(" version='"+osm.version+"'");
+        if (osm.getVersion() != 0) {
+            out.print(" version='"+osm.getVersion()+"'");
         }
         if (this.changeset != null && this.changeset.getId() != 0) {