Changeset 2303 in josm for trunk

Timestamp:

2009-10-24T10:20:45+02:00 (15 years ago)

Author:

Gubaer

Message:

see #3761: Precondition failed: Node is still used by way
Also added help for this use case, see help

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/DownloadReferrersAction.java (modified) (11 diffs)
• actions/UploadAction.java (modified) (5 diffs)
• data/osm/visitor/MergeVisitor.java (modified) (2 diffs)
• gui/HelpAwareOptionPane.java (modified) (2 diffs)
• io/OsmServerBackreferenceReader.java (modified) (6 diffs)

trunk/src/org/openstreetmap/josm/actions/DownloadReferrersAction.java

@@ -8 +8 @@
 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 +18 @@
 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 +33 @@
  * server.
  *
- *
  */
 public class DownloadReferrersAction extends JosmAction{
@@ -40 +42 @@
 
     /**
-     * Downloads the primitives referring to the primitives in <code>primitives</code>.
+     * Downloads the primitives referring to the primitives in <code>primitives</code>
+     * into the target layer <code>targetLayer</code>.
      * Does nothing if primitives is null or empty.
      *
-     * @param primitives the collection of primitives.
-     */
-    public void downloadReferrers(Collection<OsmPrimitive> 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<OsmPrimitive> children) throws IllegalArgumentException {
+        if (children == null || children.isEmpty()) return;
+        Main.worker.submit(new DownloadReferrersTask(targetLayer, children));
+    }
+
+    /**
+     * Downloads the primitives referring to the primitives in <code>primitives</code>
+     * into the target layer <code>targetLayer</code>.
+     * 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<Long, OsmPrimitiveType> 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 <code>id</code> and
+     * <code>type</code>.
+     * 
+     *
+     * @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 +96 @@
             return;
         Collection<OsmPrimitive> primitives = layer.data.getSelected();
-        downloadReferrers(primitives);
+        downloadReferrers(layer,primitives);
     }
 
@@ -65 +103 @@
      *
      */
-    class DownloadReferrersTask extends PleaseWaitRunnable {
-        private DataSet ds;
+    public static class DownloadReferrersTask extends PleaseWaitRunnable {
         private boolean cancelled;
-        Exception lastException;
-        private Collection<OsmPrimitive> primitives;
+        private Exception lastException;
+        private OsmServerBackreferenceReader reader;
+        /** the target layer */
+        private OsmDataLayer targetLayer;
+        /** the collection of child primitives */
+        private Map<Long, OsmPrimitiveType> children;
+        /** the parents */
         private DataSet parents;
 
-        public DownloadReferrersTask(Collection<OsmPrimitive> 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<OsmPrimitive> 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<Long, OsmPrimitiveType>();
+            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<Long, OsmPrimitiveType> 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<Long, OsmPrimitiveType>();
+            if (children != null) {
+                for (Entry<Long, OsmPrimitiveType> 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<Long, OsmPrimitiveType>();
+            this.children.put(id, type);
+            this.targetLayer = targetLayer;
+            parents = new DataSet();
         }
 
@@ -95 +192 @@
         protected void cancel() {
             cancelled = true;
-            OsmApi.getOsmApi().cancel();
+            synchronized(this) {
+                if (reader != null) {
+                    reader.cancel();
+                }
+            }
         }
 
@@ -103 +204 @@
                 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 +220 @@
             if (visitor.getConflicts().isEmpty())
                 return;
-            Main.map.mapView.getEditLayer().getConflicts().add(visitor.getConflicts());
+            targetLayer.getConflicts().add(visitor.getConflicts());
             JOptionPane.showMessageDialog(
                     Main.parent,
@@ -130 +231 @@
         }
 
-        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 +244 @@
         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<Long, OsmPrimitiveType> 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++;
                 }

trunk/src/org/openstreetmap/josm/actions/UploadAction.java

@@ -228 +228 @@
         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 +271 @@
                 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 +341 @@
     }
 
+
+    /**
+     * 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("<html>Uploading <strong>failed</strong> because you tried "
+                + "to delete node {0} which is still in use in way {1}.<br><br>"
+                + "Click <strong>{2}</strong> to download all parent ways of node {0}.<br>"
+                + "If necessary JOSM will create conflicts which you can resolve in the Conflict Resolution Dialog."
+                + "</html>",
+                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 +403 @@
             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 +499 @@
             //
             else if (ex.getResponseCode() == HttpURLConnection.HTTP_PRECON_FAILED) {
-                ExceptionDialogUtil.explainPreconditionFailed(ex);
+                handlePreconditionFailed(ex);
                 return;
             }

trunk/src/org/openstreetmap/josm/data/osm/visitor/MergeVisitor.java

@@ -23 +23 @@
  * into it.
  *
- * @author imi
- * @author Gubaer
  */
 public class MergeVisitor extends AbstractVisitor {
@@ -188 +186 @@
                 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;

trunk/src/org/openstreetmap/josm/gui/HelpAwareOptionPane.java

@@ -15 +15 @@
 import javax.swing.JButton;
 import javax.swing.JDialog;
+import javax.swing.JLabel;
 import javax.swing.JOptionPane;
 
@@ -148 +149 @@
             }
         }
+
+        if (msg instanceof String) {
+            msg = new JLabel((String)msg);
+        }
+
         final JOptionPane pane = new JOptionPane(
                 msg,

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

@@ -14 +14 @@
 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 +135 @@
         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 +161 @@
         }
     }
-    /**
-
+
+    /**
      * 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 +215 @@
      *
      * @param ds the original dataset
+     * @param progressMonitor  the progress monitor
      * @return the modified dataset
      * @throws OsmTransferException thrown if an exception occurs.
@@ -251 +254 @@
      * 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 +260 @@
     @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)) {