From: Frederik Ramm Don-vip Subject: Handle deleted nodes without coordinates after recent OSM API change Origin: upstream, http://josm.openstreetmap.de/changeset/5326/josm upstream, http://josm.openstreetmap.de/changeset/5328/josm upstream, http://josm.openstreetmap.de/changeset/5332/josm upstream, http://josm.openstreetmap.de/changeset/5333/josm upstream, http://josm.openstreetmap.de/changeset/5334/josm upstream, http://josm.openstreetmap.de/changeset/5346/josm upstream, http://josm.openstreetmap.de/changeset/5349/josm upstream, http://josm.openstreetmap.de/changeset/5350/josm upstream, http://josm.openstreetmap.de/changeset/5351/josm upstream, http://josm.openstreetmap.de/changeset/5356/josm Bug-Debian: http://bugs.debian.org/682315 --- src/org/openstreetmap/josm/data/osm/DataSetMerger.java | 17 +++-- src/org/openstreetmap/josm/data/osm/DatasetConsistencyTest.java | 10 +-- src/org/openstreetmap/josm/data/osm/Node.java | 17 ++--- src/org/openstreetmap/josm/data/osm/visitor/paint/WireframeMapRenderer.java | 14 ++-- src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java | 27 +++++--- src/org/openstreetmap/josm/gui/DefaultNameFormatter.java | 18 +++-- src/org/openstreetmap/josm/gui/dialogs/InspectPrimitiveDialog.java | 14 ++-- src/org/openstreetmap/josm/gui/history/CoordinateInfoViewer.java | 32 +++++++--- src/org/openstreetmap/josm/io/GeoJSONWriter.java | 4 - src/org/openstreetmap/josm/io/OsmChangesetContentParser.java | 13 ++-- src/org/openstreetmap/josm/io/OsmHistoryReader.java | 13 ++-- src/org/openstreetmap/josm/io/OsmReader.java | 6 + src/org/openstreetmap/josm/io/OsmWriter.java | 4 - 13 files changed, 118 insertions(+), 71 deletions(-) --- josm.orig/src/org/openstreetmap/josm/data/osm/Node.java +++ josm/src/org/openstreetmap/josm/data/osm/Node.java @@ -33,16 +33,12 @@ public final class Node extends OsmPrimi @Override public final void setCoor(LatLon coor) { - if(coor != null){ - updateCoor(coor, null); - } + updateCoor(coor, null); } @Override public final void setEastNorth(EastNorth eastNorth) { - if(eastNorth != null) { - updateCoor(null, eastNorth); - } + updateCoor(null, eastNorth); } private void updateCoor(LatLon coor, EastNorth eastNorth) { @@ -112,8 +108,11 @@ public final class Node extends OsmPrimi this.lon = ll.lon(); this.east = eastNorth.east(); this.north = eastNorth.north(); - } else - throw new IllegalArgumentException(); + } else { + this.lat = Double.NaN; + this.lon = Double.NaN; + invalidateEastNorthCache(); + } } protected Node(long id, boolean allowNegative) { @@ -177,7 +176,7 @@ public final class Node extends OsmPrimi @Override void setDataset(DataSet dataSet) { super.setDataset(dataSet); - if (!isIncomplete() && (getCoor() == null || getEastNorth() == null)) + if (!isIncomplete() && isVisible() && (getCoor() == null || getEastNorth() == null)) throw new DataIntegrityProblemException("Complete node with null coordinates: " + toString() + get3892DebugInfo()); } --- josm.orig/src/org/openstreetmap/josm/io/OsmReader.java +++ josm/src/org/openstreetmap/josm/io/OsmReader.java @@ -177,7 +177,11 @@ public class OsmReader extends AbstractR protected Node parseNode() throws XMLStreamException { NodeData nd = new NodeData(); - nd.setCoor(new LatLon(Double.parseDouble(parser.getAttributeValue(null, "lat")), Double.parseDouble(parser.getAttributeValue(null, "lon")))); + String lat = parser.getAttributeValue(null, "lat"); + String lon = parser.getAttributeValue(null, "lon"); + if (lat != null && lon != null) { + nd.setCoor(new LatLon(Double.parseDouble(lat), Double.parseDouble(lon))); + } readCommon(nd); Node n = new Node(nd.getId(), nd.getVersion()); n.setVisible(nd.isVisible()); --- josm.orig/src/org/openstreetmap/josm/io/OsmWriter.java +++ josm/src/org/openstreetmap/josm/io/OsmWriter.java @@ -135,7 +135,9 @@ public class OsmWriter extends XmlWriter public void visit(INode n) { if (n.isIncomplete()) return; addCommon(n, "node"); - out.print(" lat='"+n.getCoor().lat()+"' lon='"+n.getCoor().lon()+"'"); + if (n.getCoor() != null) { + out.print(" lat='"+n.getCoor().lat()+"' lon='"+n.getCoor().lon()+"'"); + } if (!withBody) { out.println("/>"); } else { --- josm.orig/src/org/openstreetmap/josm/gui/DefaultNameFormatter.java +++ josm/src/org/openstreetmap/josm/gui/DefaultNameFormatter.java @@ -18,6 +18,7 @@ import java.util.Set; import org.openstreetmap.josm.Main; import org.openstreetmap.josm.data.coor.CoordinateFormat; +import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.data.osm.Changeset; import org.openstreetmap.josm.data.osm.IPrimitive; import org.openstreetmap.josm.data.osm.IRelation; @@ -178,7 +179,9 @@ public class DefaultNameFormatter implem } else { preset.nameTemplate.appendText(name, node); } - name.append(" \u200E(").append(node.getCoor().latToString(CoordinateFormat.getDefaultFormat())).append(", ").append(node.getCoor().lonToString(CoordinateFormat.getDefaultFormat())).append(")"); + if (node.getCoor() != null) { + name.append(" \u200E(").append(node.getCoor().latToString(CoordinateFormat.getDefaultFormat())).append(", ").append(node.getCoor().lonToString(CoordinateFormat.getDefaultFormat())).append(")"); + } } decorateNameWithId(name, node); @@ -553,11 +556,14 @@ public class DefaultNameFormatter implem } else { sb.append(name); } - sb.append(" (") - .append(node.getCoords().latToString(CoordinateFormat.getDefaultFormat())) - .append(", ") - .append(node.getCoords().lonToString(CoordinateFormat.getDefaultFormat())) - .append(")"); + LatLon coord = node.getCoords(); + if (coord != null) { + sb.append(" (") + .append(coord.latToString(CoordinateFormat.getDefaultFormat())) + .append(", ") + .append(coord.lonToString(CoordinateFormat.getDefaultFormat())) + .append(")"); + } decorateNameWithId(sb, node); return sb.toString(); } --- josm.orig/src/org/openstreetmap/josm/gui/dialogs/InspectPrimitiveDialog.java +++ josm/src/org/openstreetmap/josm/gui/dialogs/InspectPrimitiveDialog.java @@ -272,12 +272,14 @@ public class InspectPrimitiveDialog exte } void addCoordinates(Node n) { - add(tr("Coordinates: "), - Double.toString(n.getCoor().lat()), ", ", - Double.toString(n.getCoor().lon())); - add(tr("Coordinates (projected): "), - Double.toString(n.getEastNorth().east()), ", ", - Double.toString(n.getEastNorth().north())); + if (n.getCoor() != null) { + add(tr("Coordinates: "), + Double.toString(n.getCoor().lat()), ", ", + Double.toString(n.getCoor().lon())); + add(tr("Coordinates (projected): "), + Double.toString(n.getEastNorth().east()), ", ", + Double.toString(n.getEastNorth().north())); + } } void addReferrers(StringBuilder s, OsmPrimitive o) { --- josm.orig/src/org/openstreetmap/josm/data/osm/DatasetConsistencyTest.java +++ josm/src/org/openstreetmap/josm/data/osm/DatasetConsistencyTest.java @@ -69,7 +69,7 @@ public class DatasetConsistencyTest { public void checkCompleteNodesWithoutCoordinates() { for (Node node:dataSet.getNodes()) { - if (!node.isIncomplete() && (node.getCoor() == null || node.getEastNorth() == null)) { + if (!node.isIncomplete() && node.isVisible() && (node.getCoor() == null || node.getEastNorth() == null)) { printError("COMPLETE WITHOUT COORDINATES", "%s is not incomplete but has null coordinates", node); } } @@ -79,9 +79,11 @@ public class DatasetConsistencyTest { for (Node n:dataSet.getNodes()) { if (!n.isIncomplete() && !n.isDeleted()) { LatLon c = n.getCoor(); - BBox box = new BBox(new LatLon(c.lat() - 0.0001, c.lon() - 0.0001), new LatLon(c.lat() + 0.0001, c.lon() + 0.0001)); - if (!dataSet.searchNodes(box).contains(n)) { - printError("SEARCH NODES", "%s not found using Dataset.searchNodes()", n); + if (c != null) { + BBox box = new BBox(new LatLon(c.lat() - 0.0001, c.lon() - 0.0001), new LatLon(c.lat() + 0.0001, c.lon() + 0.0001)); + if (!dataSet.searchNodes(box).contains(n)) { + printError("SEARCH NODES", "%s not found using Dataset.searchNodes()", n); + } } } } --- josm.orig/src/org/openstreetmap/josm/gui/history/CoordinateInfoViewer.java +++ josm/src/org/openstreetmap/josm/gui/history/CoordinateInfoViewer.java @@ -14,6 +14,7 @@ import javax.swing.JLabel; import javax.swing.JPanel; import org.openstreetmap.josm.data.coor.CoordinateFormat; +import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.data.osm.history.HistoryNode; import org.openstreetmap.josm.data.osm.history.HistoryOsmPrimitive; import org.openstreetmap.josm.gui.NavigatableComponent; @@ -255,19 +256,24 @@ public class CoordinateInfoViewer extend HistoryNode node = (HistoryNode)p; HistoryNode oppositeNode = (HistoryNode) opposite; + LatLon coord = node.getCoords(); + LatLon oppositeCoord = oppositeNode.getCoords(); + // display the coordinates // - lblLat.setText(node.getCoords().latToString(CoordinateFormat.DECIMAL_DEGREES)); - lblLon.setText(node.getCoords().lonToString(CoordinateFormat.DECIMAL_DEGREES)); + lblLat.setText(coord != null ? coord.latToString(CoordinateFormat.DECIMAL_DEGREES) : tr("(none)")); + lblLon.setText(coord != null ? coord.lonToString(CoordinateFormat.DECIMAL_DEGREES) : tr("(none)")); // update background color to reflect differences in the coordinates // - if (node.getCoords().lat() == oppositeNode.getCoords().lat()) { + if (coord == oppositeCoord || + (coord != null && oppositeCoord != null && coord.lat() == oppositeCoord.lat())) { lblLat.setBackground(Color.WHITE); } else { lblLat.setBackground(BGCOLOR_DIFFERENCE); } - if (node.getCoords().lon() == oppositeNode.getCoords().lon()) { + if (coord == oppositeCoord || + (coord != null && oppositeCoord != null && coord.lon() == oppositeCoord.lon())) { lblLon.setBackground(Color.WHITE); } else { lblLon.setBackground(BGCOLOR_DIFFERENCE); @@ -321,15 +327,23 @@ public class CoordinateInfoViewer extend HistoryNode node = (HistoryNode) p; HistoryNode oppositeNode = (HistoryNode) opposite; + LatLon coord = node.getCoords(); + LatLon oppositeCoord = oppositeNode.getCoords(); + // update distance // - double distance = node.getCoords().greatCircleDistance(oppositeNode.getCoords()); - if (distance > 0) { - lblDistance.setBackground(BGCOLOR_DIFFERENCE); + if (coord != null && oppositeCoord != null) { + double distance = coord.greatCircleDistance(oppositeNode.getCoords()); + if (distance > 0) { + lblDistance.setBackground(BGCOLOR_DIFFERENCE); + } else { + lblDistance.setBackground(Color.WHITE); + } + lblDistance.setText(NavigatableComponent.getDistText(distance)); } else { - lblDistance.setBackground(Color.WHITE); + lblDistance.setBackground(coord != oppositeCoord ? BGCOLOR_DIFFERENCE : Color.WHITE); + lblDistance.setText(tr("(none)")); } - lblDistance.setText(NavigatableComponent.getDistText(distance)); } } } --- josm.orig/src/org/openstreetmap/josm/io/GeoJSONWriter.java +++ josm/src/org/openstreetmap/josm/io/GeoJSONWriter.java @@ -103,6 +103,8 @@ public class GeoJSONWriter implements Vi } protected void appendCoord(LatLon c) { - out.append("[").append(c.lon()).append(", ").append(c.lat()).append("]"); + if (c != null) { + out.append("[").append(c.lon()).append(", ").append(c.lat()).append("]"); + } } } --- josm.orig/src/org/openstreetmap/josm/io/OsmChangesetContentParser.java +++ josm/src/org/openstreetmap/josm/io/OsmChangesetContentParser.java @@ -102,16 +102,16 @@ public class OsmChangesetContentParser { return l; } - protected Double getMandatoryAttributeDouble(Attributes attr, String name) throws SAXException{ + protected Double getAttributeDouble(Attributes attr, String name) throws SAXException{ String v = attr.getValue(name); if (v == null) { - throwException(tr("Missing mandatory attribute ''{0}''.", name)); + return null; } double d = 0.0; try { d = Double.parseDouble(v); } catch(NumberFormatException e) { - throwException(tr("Illegal value for mandatory attribute ''{0}'' of type double. Got ''{1}''.", name, v)); + throwException(tr("Illegal value for attribute ''{0}'' of type double. Got ''{1}''.", name, v)); } return d; } @@ -159,10 +159,11 @@ public class OsmChangesetContentParser { Date timestamp = DateUtils.fromString(v); HistoryOsmPrimitive primitive = null; if (type.equals(OsmPrimitiveType.NODE)) { - double lat = getMandatoryAttributeDouble(atts, "lat"); - double lon = getMandatoryAttributeDouble(atts, "lon"); + Double lat = getAttributeDouble(atts, "lat"); + Double lon = getAttributeDouble(atts, "lon"); + LatLon coor = (lat != null && lon != null) ? new LatLon(lat,lon) : null; primitive = new HistoryNode( - id,version,visible,user,changesetId,timestamp, new LatLon(lat,lon) + id,version,visible,user,changesetId,timestamp,coor ); } else if (type.equals(OsmPrimitiveType.WAY)) { --- josm.orig/src/org/openstreetmap/josm/io/OsmHistoryReader.java +++ josm/src/org/openstreetmap/josm/io/OsmHistoryReader.java @@ -98,16 +98,16 @@ public class OsmHistoryReader { return l; } - protected Double getMandatoryAttributeDouble(Attributes attr, String name) throws SAXException{ + protected Double getAttributeDouble(Attributes attr, String name) throws SAXException{ String v = attr.getValue(name); if (v == null) { - throwException(tr("Missing mandatory attribute ''{0}''.", name)); + return null; } double d = 0.0; try { d = Double.parseDouble(v); } catch(NumberFormatException e) { - throwException(tr("Illegal value for mandatory attribute ''{0}'' of type double. Got ''{1}''.", name, v)); + throwException(tr("Illegal value for attribute ''{0}'' of type double. Got ''{1}''.", name, v)); } return d; } @@ -153,10 +153,11 @@ public class OsmHistoryReader { Date timestamp = DateUtils.fromString(v); HistoryOsmPrimitive primitive = null; if (type.equals(OsmPrimitiveType.NODE)) { - double lat = getMandatoryAttributeDouble(atts, "lat"); - double lon = getMandatoryAttributeDouble(atts, "lon"); + Double lat = getAttributeDouble(atts, "lat"); + Double lon = getAttributeDouble(atts, "lon"); + LatLon coord = (lat != null && lon != null) ? new LatLon(lat,lon) : null; primitive = new HistoryNode( - id,version,visible,user,changesetId,timestamp, new LatLon(lat,lon) + id,version,visible,user,changesetId,timestamp,coord ); } else if (type.equals(OsmPrimitiveType.WAY)) { --- josm.orig/src/org/openstreetmap/josm/data/osm/DataSetMerger.java +++ josm/src/org/openstreetmap/josm/data/osm/DataSetMerger.java @@ -178,8 +178,9 @@ public class DataSetMerger { List referrers = target.getReferrers(); if (referrers.isEmpty()) { - target.setDeleted(true); + resetPrimitive(target); target.mergeFrom(source); + target.setDeleted(true); it.remove(); flag = true; } else { @@ -202,11 +203,7 @@ public class DataSetMerger { // There are some more objects rest in the objectsToDelete set // This can be because of cross-referenced relations. for (OsmPrimitive osm: objectsToDelete) { - if (osm instanceof Way) { - ((Way) osm).setNodes(null); - } else if (osm instanceof Relation) { - ((Relation) osm).setMembers(null); - } + resetPrimitive(osm); } for (OsmPrimitive osm: objectsToDelete) { osm.setDeleted(true); @@ -215,6 +212,14 @@ public class DataSetMerger { } } + private final void resetPrimitive(OsmPrimitive osm) { + if (osm instanceof Way) { + ((Way) osm).setNodes(null); + } else if (osm instanceof Relation) { + ((Relation) osm).setMembers(null); + } + } + /** * Merges the node list of a source way onto its target way. * --- josm.orig/src/org/openstreetmap/josm/data/osm/visitor/paint/WireframeMapRenderer.java +++ josm/src/org/openstreetmap/josm/data/osm/visitor/paint/WireframeMapRenderer.java @@ -137,33 +137,33 @@ public class WireframeMapRenderer extend time to iterate through list twice, OTOH does not require changing the colour while painting... */ for (final OsmPrimitive osm: data.searchRelations(bbox)) { - if (!osm.isDeleted() && !ds.isSelected(osm) && !osm.isDisabledAndHidden()) { + if (osm.isDrawable() && !ds.isSelected(osm) && !osm.isDisabledAndHidden()) { osm.visit(this); } } for (final OsmPrimitive osm:data.searchWays(bbox)){ - if (!osm.isDeleted() && !ds.isSelected(osm) && !osm.isDisabledAndHidden() && osm.isTagged()) { + if (osm.isDrawable() && !ds.isSelected(osm) && !osm.isDisabledAndHidden() && osm.isTagged()) { osm.visit(this); } } displaySegments(); for (final OsmPrimitive osm:data.searchWays(bbox)){ - if (!osm.isDeleted() && !ds.isSelected(osm) && !osm.isDisabledAndHidden() && !osm.isTagged()) { + if (osm.isDrawable() && !ds.isSelected(osm) && !osm.isDisabledAndHidden() && !osm.isTagged()) { osm.visit(this); } } displaySegments(); for (final OsmPrimitive osm : data.getSelected()) { - if (!osm.isDeleted()) { + if (osm.isDrawable()) { osm.visit(this); } } displaySegments(); for (final OsmPrimitive osm: data.searchNodes(bbox)) { - if (!osm.isDeleted() && !ds.isSelected(osm) && !osm.isDisabledAndHidden()) + if (osm.isDrawable() && !ds.isSelected(osm) && !osm.isDisabledAndHidden()) { osm.visit(this); } @@ -343,7 +343,7 @@ public class WireframeMapRenderer extend g.setColor(col); for (RelationMember m : r.getMembers()) { - if (m.getMember().isIncomplete() || m.getMember().isDeleted()) { + if (m.getMember().isIncomplete() || !m.getMember().isDrawable()) { continue; } @@ -360,7 +360,7 @@ public class WireframeMapRenderer extend boolean first = true; for (Node n : m.getWay().getNodes()) { - if (n.isIncomplete() || n.isDeleted()) { + if (!n.isDrawable()) { continue; } Point p = nc.getPoint(n); --- josm.orig/src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java +++ josm/src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java @@ -43,35 +43,44 @@ public class DuplicateNode extends Test double precision = Main.pref.getDouble("validator.duplicatenodes.precision", 0.); - private LatLon RoundCoord(Node o) { + private LatLon roundCoord(LatLon coor) { return new LatLon( - Math.round(o.getCoor().lat() / precision) * precision, - Math.round(o.getCoor().lon() / precision) * precision + Math.round(coor.lat() / precision) * precision, + Math.round(coor.lon() / precision) * precision ); } @SuppressWarnings("unchecked") private LatLon getLatLon(Object o) { if (o instanceof Node) { + LatLon coor = ((Node) o).getCoor(); + if (coor == null) + return null; if (precision==0) - return ((Node) o).getCoor().getRoundedToOsmPrecision(); - return RoundCoord((Node) o); + return coor.getRoundedToOsmPrecision(); + return roundCoord(coor); } else if (o instanceof List) { + LatLon coor = ((List) o).get(0).getCoor(); + if (coor == null) + return null; if (precision==0) - return ((List) o).get(0).getCoor().getRoundedToOsmPrecision(); - return RoundCoord(((List) o).get(0)); + return coor.getRoundedToOsmPrecision(); + return roundCoord(coor); } else throw new AssertionError(); } @Override public boolean equals(Object k, Object t) { - return getLatLon(k).equals(getLatLon(t)); + LatLon coorK = getLatLon(k); + LatLon coorT = getLatLon(t); + return coorK == coorT || (coorK != null && coorT != null && coorK.equals(coorT)); } @Override public int getHashCode(Object k) { - return getLatLon(k).hashCode(); + LatLon coorK = getLatLon(k); + return coorK == null ? 0 : coorK.hashCode(); } }