Index: trunk/src/org/openstreetmap/josm/actions/DownloadReferrersAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/DownloadReferrersAction.java (revision 2302) +++ trunk/src/org/openstreetmap/josm/actions/DownloadReferrersAction.java (revision 2303) @@ -8,4 +8,7 @@ import java.io.IOException; import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import java.util.Map.Entry; import javax.swing.JOptionPane; @@ -15,12 +18,12 @@ import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.visitor.MergeVisitor; -import org.openstreetmap.josm.gui.DefaultNameFormatter; import org.openstreetmap.josm.gui.PleaseWaitRunnable; import org.openstreetmap.josm.gui.layer.OsmDataLayer; import org.openstreetmap.josm.gui.progress.ProgressMonitor; -import org.openstreetmap.josm.io.OsmApi; import org.openstreetmap.josm.io.OsmServerBackreferenceReader; import org.openstreetmap.josm.io.OsmTransferException; +import org.openstreetmap.josm.tools.ExceptionUtil; import org.openstreetmap.josm.tools.Shortcut; import org.xml.sax.SAXException; @@ -30,5 +33,4 @@ * server. * - * */ public class DownloadReferrersAction extends JosmAction{ @@ -40,14 +42,50 @@ /** - * Downloads the primitives referring to the primitives in primitives. + * Downloads the primitives referring to the primitives in primitives + * into the target layer targetLayer. * Does nothing if primitives is null or empty. * - * @param primitives the collection of primitives. - */ - public void downloadReferrers(Collection primitives) { - if (primitives == null || primitives.isEmpty()) return; - Main.worker.submit(new DownloadReferrersTask(primitives)); - } - + * @param targetLayer the target layer. Must not be null. + * @param children the collection of child primitives. + * @exception IllegalArgumentException thrown if targetLayer is null + */ + static public void downloadReferrers(OsmDataLayer targetLayer, Collection children) throws IllegalArgumentException { + if (children == null || children.isEmpty()) return; + Main.worker.submit(new DownloadReferrersTask(targetLayer, children)); + } + + /** + * Downloads the primitives referring to the primitives in primitives + * into the target layer targetLayer. + * Does nothing if primitives is null or empty. + * + * @param targetLayer the target layer. Must not be null. + * @param children the collection of primitives, given as map of ids and types + * @exception IllegalArgumentException thrown if targetLayer is null + */ + static public void downloadReferrers(OsmDataLayer targetLayer, Map children) throws IllegalArgumentException { + if (children == null || children.isEmpty()) return; + Main.worker.submit(new DownloadReferrersTask(targetLayer, children)); + } + + /** + * Downloads the primitives referring to the primitive given by id and + * type. + * + * + * @param targetLayer the target layer. Must not be null. + * @param id the primitive id. id > 0 required. + * @param type the primitive type. type != null required + * @exception IllegalArgumentException thrown if targetLayer is null + * @exception IllegalArgumentException thrown if id <= 0 + * @exception IllegalArgumentException thrown if type == null + */ + static public void downloadReferrers(OsmDataLayer targetLayer, long id, OsmPrimitiveType type) throws IllegalArgumentException { + if (id <= 0) + throw new IllegalArgumentException(tr("Id > 0 required, got {0}", id)); + if (type == null) + throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "type")); + Main.worker.submit(new DownloadReferrersTask(targetLayer, id, type)); + } public void actionPerformed(ActionEvent e) { @@ -58,5 +96,5 @@ return; Collection primitives = layer.data.getSelected(); - downloadReferrers(primitives); + downloadReferrers(layer,primitives); } @@ -65,29 +103,88 @@ * */ - class DownloadReferrersTask extends PleaseWaitRunnable { - private DataSet ds; + public static class DownloadReferrersTask extends PleaseWaitRunnable { private boolean cancelled; - Exception lastException; - private Collection primitives; + private Exception lastException; + private OsmServerBackreferenceReader reader; + /** the target layer */ + private OsmDataLayer targetLayer; + /** the collection of child primitives */ + private Map children; + /** the parents */ private DataSet parents; - public DownloadReferrersTask(Collection primitives) { + /** + * constructor + * + * @param targetLayer the target layer for the downloaded primitives. Must not be null. + * @param children the collection of child primitives for which parents are to be downloaded + * + */ + public DownloadReferrersTask(OsmDataLayer targetLayer, Collection children) { super("Download referrers", false /* don't ignore exception*/); + if (targetLayer == null) + throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "targetLayer")); cancelled = false; - this.primitives = primitives; + this.children = new HashMap(); + if (children != null) { + for (OsmPrimitive p: children) { + if (! p.isNew()) { + this.children.put(p.getId(), OsmPrimitiveType.from(p)); + } + } + } + this.targetLayer = targetLayer; parents = new DataSet(); } - protected void showLastException() { - String msg = lastException.getMessage(); - if (msg == null) { - msg = lastException.toString(); - } - JOptionPane.showMessageDialog( - Main.map, - msg, - tr("Error"), - JOptionPane.ERROR_MESSAGE - ); + /** + * constructor + * + * @param targetLayer the target layer for the downloaded primitives. Must not be null. + * @param primitives the collection of children for which parents are to be downloaded. Children + * are specified by their id and their type. + * + */ + public DownloadReferrersTask(OsmDataLayer targetLayer, Map children) { + super("Download referrers", false /* don't ignore exception*/); + if (targetLayer == null) + throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "targetLayer")); + cancelled = false; + this.children = new HashMap(); + if (children != null) { + for (Entry entry : children.entrySet()) { + if (entry.getKey() > 0 && entry.getValue() != null) { + children.put(entry.getKey(), entry.getValue()); + } + } + } + this.targetLayer = targetLayer; + parents = new DataSet(); + } + + /** + * constructor + * + * @param targetLayer the target layer. Must not be null. + * @param id the primitive id. id > 0 required. + * @param type the primitive type. type != null required + * @exception IllegalArgumentException thrown if id <= 0 + * @exception IllegalArgumentException thrown if type == null + * @exception IllegalArgumentException thrown if targetLayer == null + * + */ + public DownloadReferrersTask(OsmDataLayer targetLayer, long id, OsmPrimitiveType type) throws IllegalArgumentException { + super("Download referrers", false /* don't ignore exception*/); + if (targetLayer == null) + throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "targetLayer")); + if (id <= 0) + throw new IllegalArgumentException(tr("Id > 0 required, got {0}", id)); + if (type == null) + throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "type")); + cancelled = false; + this.children = new HashMap(); + this.children.put(id, type); + this.targetLayer = targetLayer; + parents = new DataSet(); } @@ -95,5 +192,9 @@ protected void cancel() { cancelled = true; - OsmApi.getOsmApi().cancel(); + synchronized(this) { + if (reader != null) { + reader.cancel(); + } + } } @@ -103,14 +204,14 @@ return; if (lastException != null) { - showLastException(); + ExceptionUtil.explainException(lastException); return; } - MergeVisitor visitor = new MergeVisitor(Main.map.mapView.getEditLayer().data, parents); + MergeVisitor visitor = new MergeVisitor(targetLayer.data, parents); visitor.merge(); SwingUtilities.invokeLater( new Runnable() { public void run() { - Main.map.mapView.getEditLayer().fireDataChange(); + targetLayer.fireDataChange(); Main.map.mapView.repaint(); } @@ -119,5 +220,5 @@ if (visitor.getConflicts().isEmpty()) return; - Main.map.mapView.getEditLayer().getConflicts().add(visitor.getConflicts()); + targetLayer.getConflicts().add(visitor.getConflicts()); JOptionPane.showMessageDialog( Main.parent, @@ -130,7 +231,10 @@ } - protected void downloadParents(OsmPrimitive primitive, ProgressMonitor progressMonitor) throws OsmTransferException{ - OsmServerBackreferenceReader reader = new OsmServerBackreferenceReader(primitive); + protected void downloadParents(long id, OsmPrimitiveType type, ProgressMonitor progressMonitor) throws OsmTransferException{ + reader = new OsmServerBackreferenceReader(id, type); DataSet ds = reader.parseOsm(progressMonitor); + synchronized(this) { // avoid race condition in cancel() + reader = null; + } MergeVisitor visitor = new MergeVisitor(parents, ds); visitor.merge(); @@ -140,11 +244,17 @@ protected void realRun() throws SAXException, IOException, OsmTransferException { try { - progressMonitor.setTicksCount(primitives.size()); + progressMonitor.setTicksCount(children.size()); int i=1; - for (OsmPrimitive primitive: primitives) { + for (Entry entry: children.entrySet()) { if (cancelled) return; - progressMonitor.subTask(tr("({0}/{1}) Loading parents of primitive {2}", i+1,primitives.size(), primitive.getDisplayName(DefaultNameFormatter.getInstance()))); - downloadParents(primitive, progressMonitor.createSubTaskMonitor(1, false)); + String msg = ""; + switch(entry.getValue()) { + case NODE: msg = tr("({0}/{1}) Loading parents of node {2}", i+1,children.size(), entry.getKey()); break; + case WAY: msg = tr("({0}/{1}) Loading parents of way {2}", i+1,children.size(), entry.getKey()); break; + case RELATION: msg = tr("({0}/{1}) Loading parents of relation {2}", i+1,children.size(), entry.getKey()); break; + } + progressMonitor.subTask(msg); + downloadParents(entry.getKey(), entry.getValue(), progressMonitor.createSubTaskMonitor(1, false)); i++; } Index: trunk/src/org/openstreetmap/josm/actions/UploadAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/UploadAction.java (revision 2302) +++ trunk/src/org/openstreetmap/josm/actions/UploadAction.java (revision 2303) @@ -228,7 +228,7 @@ String lbl = ""; switch(primitiveType) { - case NODE: lbl = tr("Synchronize node {0} only", id); break; - case WAY: lbl = tr("Synchronize way {0} only", id); break; - case RELATION: lbl = tr("Synchronize relation {0} only", id); break; + case NODE: lbl = tr("Synchronize node {0} only", id); break; + case WAY: lbl = tr("Synchronize way {0} only", id); break; + case RELATION: lbl = tr("Synchronize relation {0} only", id); break; } ButtonSpec[] spec = new ButtonSpec[] { @@ -271,10 +271,10 @@ spec, spec[0], - "Concepts/Conflict" + "/Concepts/Conflict" ); switch(ret) { - case 0: synchronizePrimitive(primitiveType, id); break; - case 1: synchronizeDataSet(); break; - default: return; + case 0: synchronizePrimitive(primitiveType, id); break; + case 1: synchronizeDataSet(); break; + default: return; } } @@ -341,4 +341,46 @@ } + + /** + * Handles the case where deleting a node failed because it is still in use in + * a non-deleted way on the server. + */ + protected void handleUploadConflictForNodeStillInUse(long nodeId, long wayId) { + ButtonSpec[] options = new ButtonSpec[] { + new ButtonSpec( + tr("Prepare conflict resolution"), + ImageProvider.get("ok"), + tr("Click to download all parent ways for node {0}", nodeId), + null /* no specific help context */ + ), + new ButtonSpec( + tr("Cancel"), + ImageProvider.get("cancel"), + tr("Click to cancel and to resume editing the map", nodeId), + null /* no specific help context */ + ) + }; + String msg = tr("Uploading failed because you tried " + + "to delete node {0} which is still in use in way {1}.

" + + "Click {2} to download all parent ways of node {0}.
" + + "If necessary JOSM will create conflicts which you can resolve in the Conflict Resolution Dialog." + + "", + nodeId, wayId, options[0].text + ); + + int ret = HelpAwareOptionPane.showOptionDialog( + Main.parent, + msg, + tr("Node still in use"), + JOptionPane.ERROR_MESSAGE, + null, + options, + options[0], + "/Action/Upload#NodeStillInUseInWay" + ); + if (ret != 0) return; + DownloadReferrersAction.downloadReferrers(Main.map.mapView.getEditLayer(), nodeId, OsmPrimitiveType.NODE); + } + /** * handles an upload conflict, i.e. an error indicated by a HTTP return code 409. @@ -361,6 +403,30 @@ return; } - logger.warning(tr("Warning: error header \"{0}\" did not match expected pattern \"{1}\"", e.getErrorHeader(),pattern)); + pattern = "Node (\\d+) is still used by way (\\d+)."; + p = Pattern.compile(pattern); + m = p.matcher(e.getErrorHeader()); + if (m.matches()) { + handleUploadConflictForNodeStillInUse(Long.parseLong(m.group(1)), Long.parseLong(m.group(2))); + return; + } + logger.warning(tr("Warning: error header \"{0}\" did not match with an expected pattern", e.getErrorHeader())); handleUploadConflictForUnknownConflict(); + } + + /** + * handles an precondition failed conflict, i.e. an error indicated by a HTTP return code 412. + * + * @param e the exception + */ + protected void handlePreconditionFailed(OsmApiException e) { + String pattern = "Precondition failed: Node (\\d+) is still used by way (\\d+)."; + Pattern p = Pattern.compile(pattern); + Matcher m = p.matcher(e.getErrorHeader()); + if (m.matches()) { + handleUploadConflictForNodeStillInUse(Long.parseLong(m.group(1)), Long.parseLong(m.group(2))); + return; + } + logger.warning(tr("Warning: error header \"{0}\" did not match with an expected pattern", e.getErrorHeader())); + ExceptionDialogUtil.explainPreconditionFailed(e); } @@ -433,5 +499,5 @@ // else if (ex.getResponseCode() == HttpURLConnection.HTTP_PRECON_FAILED) { - ExceptionDialogUtil.explainPreconditionFailed(ex); + handlePreconditionFailed(ex); return; } Index: trunk/src/org/openstreetmap/josm/data/osm/visitor/MergeVisitor.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/osm/visitor/MergeVisitor.java (revision 2302) +++ trunk/src/org/openstreetmap/josm/data/osm/visitor/MergeVisitor.java (revision 2303) @@ -23,6 +23,4 @@ * into it. * - * @author imi - * @author Gubaer */ public class MergeVisitor extends AbstractVisitor { @@ -188,4 +186,9 @@ if (!mergedNode.isDeleted()) { newNodes.add(mergedNode); + } else { + // we've removed a node from a way during merging. + // Flag the way as being modified. + // + w.setModified(true); } replacedSomething = true; Index: trunk/src/org/openstreetmap/josm/gui/HelpAwareOptionPane.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/HelpAwareOptionPane.java (revision 2302) +++ trunk/src/org/openstreetmap/josm/gui/HelpAwareOptionPane.java (revision 2303) @@ -15,4 +15,5 @@ import javax.swing.JButton; import javax.swing.JDialog; +import javax.swing.JLabel; import javax.swing.JOptionPane; @@ -148,4 +149,9 @@ } } + + if (msg instanceof String) { + msg = new JLabel((String)msg); + } + final JOptionPane pane = new JOptionPane( msg, Index: trunk/src/org/openstreetmap/josm/io/OsmServerBackreferenceReader.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmServerBackreferenceReader.java (revision 2302) +++ trunk/src/org/openstreetmap/josm/io/OsmServerBackreferenceReader.java (revision 2303) @@ -14,4 +14,5 @@ import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.osm.visitor.MergeVisitor; +import org.openstreetmap.josm.gui.progress.NullProgressMonitor; import org.openstreetmap.josm.gui.progress.ProgressMonitor; @@ -134,5 +135,5 @@ progressMonitor.beginTask(null, 2); try { - progressMonitor.indeterminateSubTask(tr("Contacting OSM Server...")); + progressMonitor.indeterminateSubTask(tr("Downloading from OSM Server...")); StringBuffer sb = new StringBuffer(); sb.append(primitiveType.getAPIName()) @@ -160,8 +161,9 @@ } } - /** - + + /** * Reads referring relations from the API server and replies them in a {@see DataSet} * + * @param progressMonitor the progress monitor * @return the data set * @throws OsmTransferException @@ -213,4 +215,5 @@ * * @param ds the original dataset + * @param progressMonitor the progress monitor * @return the modified dataset * @throws OsmTransferException thrown if an exception occurs. @@ -251,4 +254,5 @@ * replies them as {@see DataSet} * + * @param progressMonitor the progress monitor. Set to {@see NullProgressMonitor#INSTANCE} if null. * @return the dataset with the referring primitives * @exception OsmTransferException thrown if an error occurs while communicating with the server @@ -256,6 +260,9 @@ @Override public DataSet parseOsm(ProgressMonitor progressMonitor) throws OsmTransferException { - progressMonitor.beginTask(null, 3); - try { + if (progressMonitor == null) { + progressMonitor = NullProgressMonitor.INSTANCE; + } + try { + progressMonitor.beginTask(null, 3); DataSet ret = new DataSet(); if (primitiveType.equals(OsmPrimitiveType.NODE)) {