Changeset 2325 in josm for trunk

Timestamp:

2009-10-26T16:51:21+01:00 (15 years ago)

Author:

Gubaer

Message:

fixed #3783: does not finish download API
Also improved help

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/UpdateDataAction.java (modified) (2 diffs)
• actions/downloadtasks/DownloadOsmTaskList.java (modified) (11 diffs)
• gui/layer/OsmDataLayer.java (modified) (4 diffs)

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

@@ -10 +10 @@
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.Future;
 
+import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.actions.downloadtasks.DownloadOsmTaskList;
 import org.openstreetmap.josm.data.osm.DataSource;
@@ -81 +83 @@
             // bounds defined? => use the bbox downloader
             //
-            new DownloadOsmTaskList().download(false, areas, new PleaseWaitProgressMonitor(tr("Updating data")));
+            final PleaseWaitProgressMonitor monitor = new PleaseWaitProgressMonitor(tr("Download data"));
+            final Future<?> future = new DownloadOsmTaskList().download(false /* no new layer */, areas, monitor);
+            Main.worker.submit(
+                    new Runnable() {
+                        public void run() {
+                            try {
+                                future.get();
+                            } catch(Exception e) {
+                                e.printStackTrace();
+                                return;
+                            }
+                            monitor.close();
+                        }
+                    }
+            );
         }
     }

trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTaskList.java

@@ -3 +3 @@
 
 import static org.openstreetmap.josm.tools.I18n.tr;
+import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
 
 import java.awt.EventQueue;
-import java.awt.event.ActionEvent;
-import java.awt.event.ActionListener;
 import java.awt.geom.Area;
 import java.awt.geom.Rectangle2D;
@@ -24 +23 @@
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
+import org.openstreetmap.josm.gui.HelpAwareOptionPane;
+import org.openstreetmap.josm.gui.HelpAwareOptionPane.ButtonSpec;
 import org.openstreetmap.josm.gui.layer.Layer;
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
@@ -29 +30 @@
 import org.openstreetmap.josm.gui.progress.ProgressMonitor.CancelListener;
 import org.openstreetmap.josm.tools.ExceptionUtil;
+import org.openstreetmap.josm.tools.ImageProvider;
 
 /**
  * This class encapsulates the downloading of several bounding boxes that would otherwise be too
- * large to download in one go. Error messages will be collected for all downloads and displayed
- * as a list in the end.
+ * large to download in one go. Error messages will be collected for all downloads and displayed as
+ * a list in the end.
  * @author xeen
- *
+ * 
  */
-public class DownloadOsmTaskList implements Runnable {
+public class DownloadOsmTaskList {
     private List<DownloadTask> osmTasks = new LinkedList<DownloadTask>();
     private List<Future<?>> osmTaskFutures = new LinkedList<Future<?>>();
@@ -49 +51 @@
     public Future<?> download(boolean newLayer, List<Rectangle2D> rects, ProgressMonitor progressMonitor) {
         this.progressMonitor = progressMonitor;
-        if(newLayer) {
+        if (newLayer) {
             Layer l = new OsmDataLayer(new DataSet(), OsmDataLayer.createNewName(), null);
             Main.main.addLayer(l);
@@ -57 +59 @@
         progressMonitor.beginTask(null, rects.size());
         int i = 0;
-        for(Rectangle2D td : rects) {
+        for (Rectangle2D td : rects) {
             i++;
             DownloadTask dt = new DownloadOsmTask();
             ProgressMonitor childProgress = progressMonitor.createSubTaskMonitor(1, false);
             childProgress.setSilent(true);
-            childProgress.setCustomText(tr("Download {0} of {1} ({2} left)", i, rects.size(), rects.size()-i));
+            childProgress.setCustomText(tr("Download {0} of {1} ({2} left)", i, rects.size(), rects.size() - i));
             Future<?> future = dt.download(null, td.getMinY(), td.getMinX(), td.getMaxY(), td.getMaxX(), childProgress);
             osmTaskFutures.add(future);
             osmTasks.add(dt);
         }
-        progressMonitor.addCancelListener(
-                new CancelListener() {
-                    public void operationCanceled() {
-                        for (DownloadTask dt: osmTasks) {
-                            dt.cancel();
-                        }
-                    }
-                }
-        );
-        return Main.worker.submit(this);
+        progressMonitor.addCancelListener(new CancelListener() {
+            public void operationCanceled() {
+                for (DownloadTask dt : osmTasks) {
+                    dt.cancel();
+                }
+            }
+        });
+        return Main.worker.submit(new PostDownloadProcessor());
     }
 
@@ -84 +84 @@
      * @param The Collection of Areas to download
      */
-    public void download(boolean newLayer, Collection<Area> areas, ProgressMonitor progressMonitor) {
+    public Future<?> download(boolean newLayer, Collection<Area> areas, ProgressMonitor progressMonitor) {
         progressMonitor.beginTask(tr("Updating data"));
         try {
             List<Rectangle2D> rects = new LinkedList<Rectangle2D>();
-            for(Area a : areas) {
+            for (Area a : areas) {
                 rects.add(a.getBounds2D());
             }
 
-            download(newLayer, rects, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
+            return download(newLayer, rects, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
         } finally {
             progressMonitor.finishTask();
@@ -99 +99 @@
 
     /**
-     * Grabs and displays the error messages after all download threads have finished.
-     */
-    public void run() {
-        progressMonitor.finishTask();
-
-        // wait for all tasks to finish
-        //
-        for (Future<?> future: osmTaskFutures) {
-            try {
-                future.get();
-            } catch(Exception e) {
-                e.printStackTrace();
-                return;
-            }
-        }
-        LinkedHashSet<Object> errors = new LinkedHashSet<Object>();
-        for(DownloadTask dt : osmTasks) {
-            errors.addAll(dt.getErrorObjects());
-        }
-        if (!errors.isEmpty()) {
-            StringBuffer sb = new StringBuffer();
-            for (Object error:errors) {
-                if (error instanceof String) {
-                    sb.append("<li>").append(error).append("</li>").append("<br>");
-                } else if (error instanceof Exception) {
-                    sb.append("<li>").append(ExceptionUtil.explainException((Exception)error)).append("</li>").append("<br>");
-                }
-            }
-            sb.insert(0, "<ul>");
-            sb.append("</ul>");
-
-            JOptionPane.showMessageDialog(
-                    Main.parent,
-                    "<html>"+tr("The following errors occurred during mass download: {0}", sb.toString())
-                    +"</html>",
-                    tr("Errors during Download"),
-                    JOptionPane.ERROR_MESSAGE);
-            return;
-        }
-
-        // FIXME: this is a hack. We assume that the user canceled the whole download if at least
-        // one task was canceled or if it failed
-        //
-        for (DownloadTask task: osmTasks) {
-            if (task instanceof DownloadOsmTask) {
-                DownloadOsmTask osmTask = (DownloadOsmTask)task;
-                if (osmTask.isCanceled() || osmTask.isFailed())
-                    return;
-            }
-        }
-        final OsmDataLayer editLayer = Main.map.mapView.getEditLayer();
-        if (editLayer != null) {
-            Set<OsmPrimitive> myPrimitives = getCompletePrimitives(editLayer.data);
-            for (DownloadTask task : osmTasks) {
-                if(task instanceof DownloadOsmTask) {
-                    DataSet ds = ((DownloadOsmTask)task).getDownloadedData();
-                    if (ds != null) {
-                        myPrimitives.removeAll(ds.nodes);
-                        myPrimitives.removeAll(ds.ways);
-                        myPrimitives.removeAll(ds.relations);
-                    }
-                }
-            }
-            if (! myPrimitives.isEmpty()) {
-                handlePotentiallyDeletedPrimitives(myPrimitives);
-            }
-        }
-    }
-
-
-    /**
-     * Replies the set of ids of all complete primitives (i.e. those with
-     * ! primitive.incomplete)
-     *
-     * @return the set of ids of all complete primitives
+     * Replies the set of ids of all complete, non-new primitives (i.e. those with !
+     * primitive.incomplete)
+     * 
+     * @return the set of ids of all complete, non-new primitives
      */
     protected Set<OsmPrimitive> getCompletePrimitives(DataSet ds) {
         HashSet<OsmPrimitive> ret = new HashSet<OsmPrimitive>();
         for (OsmPrimitive primitive : ds.nodes) {
-            if (!primitive.incomplete && primitive.isNew()) {
+            if (!primitive.incomplete && !primitive.isNew()) {
                 ret.add(primitive);
             }
         }
         for (OsmPrimitive primitive : ds.ways) {
-            if (! primitive.incomplete && primitive.isNew()) {
+            if (!primitive.incomplete && !primitive.isNew()) {
                 ret.add(primitive);
             }
         }
         for (OsmPrimitive primitive : ds.relations) {
-            if (! primitive.incomplete && primitive.isNew()) {
+            if (!primitive.incomplete && !primitive.isNew()) {
                 ret.add(primitive);
             }
@@ -196 +125 @@
 
     /**
-     * Updates the local state of a set of primitives (given by a set of primitive
-     * ids) with the state currently held on the server.
-     *
+     * Updates the local state of a set of primitives (given by a set of primitive ids) with the
+     * state currently held on the server.
+     * 
      * @param potentiallyDeleted a set of ids to check update from the server
      */
@@ -208 +137 @@
             }
         }
-        EventQueue.invokeLater(
-                new Runnable() {
-                    public void run() {
-                        new UpdateSelectionAction().updatePrimitives(toSelect);
-                    }
-                }
+        EventQueue.invokeLater(new Runnable() {
+            public void run() {
+                new UpdateSelectionAction().updatePrimitives(toSelect);
+            }
+        });
+    }
+
+    /**
+     * Processes a set of primitives (given by a set of their ids) which might be deleted on the
+     * server. First prompts the user whether he wants to check the current state on the server. If
+     * yes, retrieves the current state on the server and checks whether the primitives are indeed
+     * deleted on the server.
+     * 
+     * @param potentiallyDeleted a set of primitives (given by their ids)
+     */
+    protected void handlePotentiallyDeletedPrimitives(Set<OsmPrimitive> potentiallyDeleted) {
+        ButtonSpec[] options = new ButtonSpec[] {
+                new ButtonSpec(
+                        tr("Check on the server"), 
+                        ImageProvider.get("ok"),
+                        tr("Click to check whether objects in your local dataset are deleted on the server"), 
+                        null  /* no specific help topic */
+                ), 
+                new ButtonSpec(
+                        tr("Ignore"), 
+                        ImageProvider.get("cancel"), 
+                        tr("Click to abort and to resume editing"), 
+                        null /* no specific help topic */
+                ),
+        };
+
+        String message = tr("<html>" + "There are {0} primitives in your local dataset which<br>"
+                + "might be deleted on the server. If you later try to delete or<br>"
+                + "update them the server is likely to report a<br>" + "conflict.<br>" + "<br>"
+                + "Click <strong>{1}</strong> to check the state of these primitives<br>" + "on the server.<br>"
+                + "Click <strong>{2}</strong> to ignore.<br>" + "</html>", 
+                potentiallyDeleted.size(), 
+                options[0].text,
+                options[1].text
+                );
+
+        int ret = HelpAwareOptionPane.showOptionDialog(
+                Main.parent, 
+                message, 
+                tr("Deleted or moved primitives"),
+                JOptionPane.WARNING_MESSAGE, 
+                null, 
+                options, 
+                options[0],
+                ht("/Action/UpdateData#SyncPotentiallyDeletedObjects")
         );
-    }
-
-    /**
-     * Processes a set of primitives (given by a set of their ids) which might be
-     * deleted on the server. First prompts the user whether he wants to check
-     * the current state on the server. If yes, retrieves the current state on the server
-     * and checks whether the primitives are indeed deleted on the server.
-     *
-     * @param potentiallyDeleted a set of primitives (given by their ids)
-     */
-    protected void handlePotentiallyDeletedPrimitives(Set<OsmPrimitive> potentiallyDeleted) {
-        String [] options = {
-                "Check on the server",
-                "Ignore"
-        };
-
-        String message = tr("<html>"
-                +  "There are {0} primitives in your local dataset which<br>"
-                + "might be deleted on the server. If you later try to delete or<br>"
-                + "update them the server is likely to report a<br>"
-                + "conflict.<br>"
-                + "<br>"
-                + "Click <strong>{1}</strong> to check the state of these primitives<br>"
-                + "on the server.<br>"
-                + "Click <strong>{2}</strong> to ignore.<br>"
-                + "</html>",
-                potentiallyDeleted.size(), options[0], options[1]
-        );
-
-        int ret =JOptionPane.showOptionDialog(
-                Main.parent,
-                message,
-                tr("Deleted or moved primitives"),
-                JOptionPane.YES_NO_OPTION,
-                JOptionPane.WARNING_MESSAGE,
-                null,
-                options,
-                options[0]
-        );
-        switch(ret) {
-            case JOptionPane.CLOSED_OPTION: return;
-            case JOptionPane.NO_OPTION: return;
-            case JOptionPane.YES_OPTION: updatePotentiallyDeletedPrimitives(potentiallyDeleted); break;
-        }
+        if (ret != 0 /* OK */)
+            return;
+        
+        updatePotentiallyDeletedPrimitives(potentiallyDeleted);        
     }
 
     /**
      * Replies the set of primitive ids which have been downloaded by this task list
-     *
+     * 
      * @return the set of primitive ids which have been downloaded by this task list
      */
@@ -269 +202 @@
         HashSet<OsmPrimitive> ret = new HashSet<OsmPrimitive>();
         for (DownloadTask task : osmTasks) {
-            if(task instanceof DownloadOsmTask) {
-                DataSet ds = ((DownloadOsmTask)task).getDownloadedData();
+            if (task instanceof DownloadOsmTask) {
+                DataSet ds = ((DownloadOsmTask) task).getDownloadedData();
                 if (ds != null) {
                     ret.addAll(ds.nodes);
@@ -280 +213 @@
         return ret;
     }
+
+    class PostDownloadProcessor implements Runnable {
+        /**
+         * Grabs and displays the error messages after all download threads have finished.
+         */
+        public void run() {
+            progressMonitor.finishTask();
+
+            // wait for all download tasks to finish
+            //
+            for (Future<?> future : osmTaskFutures) {
+                try {
+                    future.get();
+                } catch (Exception e) {
+                    e.printStackTrace();
+                    return;
+                }
+            }
+            LinkedHashSet<Object> errors = new LinkedHashSet<Object>();
+            for (DownloadTask dt : osmTasks) {
+                errors.addAll(dt.getErrorObjects());
+            }
+            if (!errors.isEmpty()) {
+                StringBuffer sb = new StringBuffer();
+                for (Object error : errors) {
+                    if (error instanceof String) {
+                        sb.append("<li>").append(error).append("</li>").append("<br>");
+                    } else if (error instanceof Exception) {
+                        sb.append("<li>").append(ExceptionUtil.explainException((Exception) error)).append("</li>")
+                                .append("<br>");
+                    }
+                }
+                sb.insert(0, "<ul>");
+                sb.append("</ul>");
+
+                JOptionPane.showMessageDialog(Main.parent, "<html>"
+                        + tr("The following errors occurred during mass download: {0}", sb.toString()) + "</html>",
+                        tr("Errors during Download"), JOptionPane.ERROR_MESSAGE);
+                return;
+            }
+
+            // FIXME: this is a hack. We assume that the user canceled the whole download if at
+            // least
+            // one task was canceled or if it failed
+            //
+            for (DownloadTask task : osmTasks) {
+                if (task instanceof DownloadOsmTask) {
+                    DownloadOsmTask osmTask = (DownloadOsmTask) task;
+                    if (osmTask.isCanceled() || osmTask.isFailed())
+                        return;
+                }
+            }
+            final OsmDataLayer editLayer = Main.map.mapView.getEditLayer();
+            if (editLayer != null) {
+                Set<OsmPrimitive> myPrimitives = getCompletePrimitives(editLayer.data);
+                for (DownloadTask task : osmTasks) {
+                    if (task instanceof DownloadOsmTask) {
+                        DataSet ds = ((DownloadOsmTask) task).getDownloadedData();
+                        if (ds != null) {
+                            myPrimitives.removeAll(ds.nodes);
+                            myPrimitives.removeAll(ds.ways);
+                            myPrimitives.removeAll(ds.relations);
+                        }
+                    }
+                }
+                if (!myPrimitives.isEmpty()) {
+                    handlePotentiallyDeletedPrimitives(myPrimitives);
+                }
+            }
+        }
+    }
 }

trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java

@@ -4 +4 @@
 
 import static org.openstreetmap.josm.tools.I18n.marktr;
+import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
 import static org.openstreetmap.josm.tools.I18n.tr;
 import static org.openstreetmap.josm.tools.I18n.trn;
@@ -62 +63 @@
 import org.openstreetmap.josm.data.osm.visitor.MergeVisitor;
 import org.openstreetmap.josm.data.osm.visitor.SimplePaintVisitor;
+import org.openstreetmap.josm.gui.HelpAwareOptionPane;
 import org.openstreetmap.josm.gui.MapView;
+import org.openstreetmap.josm.gui.HelpAwareOptionPane.ButtonSpec;
 import org.openstreetmap.josm.gui.dialogs.LayerListDialog;
 import org.openstreetmap.josm.gui.dialogs.LayerListPopup;
@@ -334 +337 @@
         );
         String msg2 = trn(
-                "{0} object has been purged from the local dataset because it is deleted on the server.",
-                "{0} objects have been purged from the local dataset because they are deleted on the server.",
+                "{0} conflict has been <strong>resolved automatically</strong> by purging {0} object<br>from the local dataset because it is deleted on the server.",
+                "{0} conflicts have been <strong>resolved automatically</strong> by purging {0} objects<br> from the local dataset because they are deleted on the server.",
                 numPurgedPrimitives,
                 numPurgedPrimitives
         );
+        int numRemainingConflicts = numNewConflicts - numPurgedPrimitives;
+        String msg3 = "";
+        if (numRemainingConflicts >0) {
+            msg3 = trn(
+                    "{0} conflict remains to be resolved.<br><br>Please open the Conflict List Dialog and manually resolve it.",
+                    "{0} conflicts remain to be resolved.<br><br>Please open the Conflict List Dialog and manually resolve them.",
+                    numRemainingConflicts,
+                    numRemainingConflicts
+            );            
+        }
+        
         StringBuffer sb = new StringBuffer();
         sb.append("<html>").append(msg1);
@@ -344 +358 @@
             sb.append("<br>").append(msg2);
         }
-        sb.append("<br>").append(tr("Please consult the Conflict List Dialog<br>and manually resolve them."));
+        if (numRemainingConflicts > 0) {
+            sb.append("<br>").append(msg3);
+        }
         sb.append("</html>");
         if (numNewConflicts > 0) {
-            JButton[] options = new JButton[] {
-                    new JButton(tr("OK")),
-                    new JButton(tr("Help"))
+            ButtonSpec[] options = new ButtonSpec[] {
+                    new ButtonSpec(
+                            tr("OK"),
+                            ImageProvider.get("ok"),
+                            tr("Click to close this dialog and continue editing"),
+                            null /* no specific help */
+                            )
             };
-            options[0].setIcon(ImageProvider.get("ok"));
-            options[1].setIcon(ImageProvider.get("help"));
-            final JOptionPane pane = new JOptionPane(
+            HelpAwareOptionPane.showOptionDialog(
+                    Main.parent,
                     sb.toString(),
+                    tr("Conflicts detected"),
                     JOptionPane.WARNING_MESSAGE,
-                    JOptionPane.DEFAULT_OPTION,
-                    null,
+                    null, /* no icon */
                     options,
-                    options[0]
-            );
-            final JDialog dialog = new JDialog(
-                    JOptionPane.getFrameForComponent(Main.parent),
-                    tr("Conflicts detected"),
-                    true);
-            options[0].addActionListener(
-                    new ActionListener() {
-                        public void actionPerformed(ActionEvent e) {
-                            dialog.setVisible(false);
-                        }
-                    }
-            );
-            options[1].addActionListener(
-                    new ActionListener() {
-                        public void actionPerformed(ActionEvent e) {
-                            HelpBrowser b = new HelpBrowser();
-                            b.openHelpTopic("Help/Concepts/Conflict");
-                            b.setVisible(true);
-                        }
-                    }
-            );
-            dialog.setContentPane(pane);
-            dialog.pack();
-            HelpUtil.setHelpContext(dialog.getRootPane(), "/Concepts/Conflict");
-            WindowGeometry.centerOnScreen(dialog.getSize()).applySafe(dialog);
-            dialog.setVisible(true);
+                    options[0],
+                    ht("/Concepts/Conflict#WarningAboutDetectedConflicts")
+             );            
         }
     }