Changeset 2330 in josm

Timestamp:

Oct 27, 2009 1:04:39 PM (4 years ago)

Author:

Gubaer

Message:

fixed #3794: not download the correct area

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/DownloadAction.java (modified) (9 diffs)
• actions/downloadtasks/DownloadTask.java (modified) (2 diffs)
• gui/download/DownloadDialog.java (modified) (1 diff)

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

@@ -10 +10 @@
 import java.awt.event.KeyEvent;
 import java.util.concurrent.Future;
+import java.util.logging.Logger;
 
 import javax.swing.JOptionPane;
@@ -18 +19 @@
 import org.openstreetmap.josm.actions.downloadtasks.DownloadOsmTask;
 import org.openstreetmap.josm.actions.downloadtasks.PostDownloadHandler;
+import org.openstreetmap.josm.data.Bounds;
 import org.openstreetmap.josm.gui.ExtendedDialog;
 import org.openstreetmap.josm.gui.download.DownloadDialog;
@@ -32 +34 @@
  */
 public class DownloadAction extends JosmAction {
+    private static final Logger logger = Logger.getLogger(DownloadAction.class.getName());
     
-
     private DownloadDialog dialog;
     private ExtendedDialog downloadDialog;
@@ -45 +47 @@
     /**
      * Creates the download dialog
+     * 
      * @return the downlaod dialog
      */
-    protected ExtendedDialog createUploadDialog() {
-        if (dialog == null)
+    protected ExtendedDialog createDownloadDialog() {
+        if (dialog == null) 
             dialog = new DownloadDialog();
         dialog.restoreSettings();
-        JPanel downPanel = new JPanel(new BorderLayout());
-        downPanel.add(dialog, BorderLayout.CENTER);
 
         final String prefName = dialog.getClass().getName()+ ".geometry";
@@ -59 +60 @@
 
         if (downloadDialog == null) {
+            JPanel downPanel = new JPanel(new BorderLayout());
+            downPanel.add(dialog, BorderLayout.CENTER);
             downloadDialog= new ExtendedDialog(Main.parent,
                 tr("Download"),
@@ -70 +73 @@
 
     public void actionPerformed(ActionEvent e) {
-        ExtendedDialog dlg = createUploadDialog();
+        ExtendedDialog dlg = createDownloadDialog();
         boolean finish = false;
         while (!finish) {            
@@ -76 +79 @@
             if (dlg.getValue() == 1 /* OK */) {
                 dialog.rememberSettings();
+                Bounds area = dialog.getSelectedDownloadArea();                
                 if (dialog.isDownloadOsmData()) {
                     DownloadOsmTask task = new DownloadOsmTask();
-                    Future<?> future = task.download(dialog.isNewLayerRequired(), dialog.getSelectedDownloadArea(), null);
+                    Future<?> future = task.download(dialog.isNewLayerRequired(), area, null);
                     Main.worker.submit(new PostDownloadHandler(task, future));
                     finish = true;
@@ -84 +88 @@
                 if (dialog.isDownloadGpxData()) {
                     DownloadGpsTask task = new DownloadGpsTask();
-                    Future<?> future = task.download(dialog.isNewLayerRequired(),dialog.getSelectedDownloadArea(), null);
+                    Future<?> future = task.download(dialog.isNewLayerRequired(),area, null);
                     Main.worker.submit(new PostDownloadHandler(task, future));
                     finish = true;
@@ -100 +104 @@
             }
         }
-
-        dialog = null;
-        dlg.dispose();
     }
 }

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

@@ -11 +11 @@
 public interface DownloadTask {
     /**
-     * Execute the download using the given bounding box. Set silent on progressMonitor
-     * if no error messages should be popped up.
+     * Asynchronously launches the download task for a given bounding box.
+     *
+     * Set <code>progressMonitor</code> to null, if the task should create, open, and close a progress monitor.
+     * Set progressMonitor to {@see NullProgressMonitor#INSTANCE} if progress information is to
+     * be discarded.
+     * 
+     * You can wait for the asynchronous download task to finish by synchronizing on the returned
+     * {@see Future}, but make sure not to freeze up JOSM. Example:
+     * <pre>
+     *    Future<?> future = task.download(...);
+     *    // DON'T run this on the Swing EDT or JOSM will freeze 
+     *    future.get(); // waits for the dowload task to complete 
+     * </pre>
+     * 
+     * The following example uses a pattern which is better suited if a task is launched from
+     * the Swing EDT:
+     * <pre>
+     *    final Future<?> future = task.download(...);
+     *    Runnable runAfterTask = new Runnable() {
+     *       public void run() {
+     *           // this is not strictly necessary because of the type of executor service
+     *           // Main.worker is initialized with, but it doesn't harm either
+     *           //
+     *           future.get(); // wait for the download task to complete
+     *           doSomethingAfterTheTaskCompleted();
+     *       }
+     *    }
+     *    Main.worker.submit(runAfterTask);
+     * </pre>
+     * 
+     * @param newLayer true, if the data is to be downloaded into a new layer. If false, the task
+     * selects one of the existing layers as download layer, preferably the active layer.
+     * 
+     * @param downloadArea the area to download
+     * @param progressMonitor the progressMonitor
+     * @return the future representing the asynchronous task
      */
     Future<?> download(boolean newLayer, Bounds downloadArea, ProgressMonitor progressMonitor);
 
     /**
-     * Execute the download using the given URL
-     * @param newLayer
-     * @param url
+     * Asynchronously launches the download task for a given bounding URL.
+     *
+     * Set progressMonitor to null, if the task should create, open, and close a progress monitor.
+     * Set progressMonitor to {@see NullProgressMonitor#INSTANCE} if progress information is to
+     * be discarded.
+ 
+     * @param newLayer newLayer true, if the data is to be downloaded into a new layer. If false, the task
+     * selects one of the existing layers as download layer, preferably the active layer.
+     * @param url the url to download from
+     * @param progressMonitor the progressMonitor
+     * @return the future representing the asynchronous task
+     * 
+     * @see #download(boolean, Bounds, ProgressMonitor)
      */
     Future<?> loadUrl(boolean newLayer, String url, ProgressMonitor progressMonitor);
@@ -28 +72 @@
      * Error objects are either {@see String}s with error messages or {@see Exception}s.
      *
-     * WARNING: Never call this in the same thread you requested the download() or it will cause a
-     * dead lock. See actions/downloadTasks/DownloadOsmTaskList.java for a proper implementation.
-     *
      * @return the list of error objects
      */
     List<Object> getErrorObjects();
 
+    /**
+     * Cancels the asynchronous download task.
+     * 
+     */
     public void cancel();
 }

trunk/src/org/openstreetmap/josm/gui/download/DownloadDialog.java

@@ -148 +148 @@
      * @param eventSource - the DownloadSelection object that fired this notification.
      */
-    public void boundingBoxChanged(Bounds b, DownloadSelection eventSource) {
+    public void boundingBoxChanged(Bounds b, DownloadSelection eventSource) {        
         this.currentBounds = b;
         for (DownloadSelection s : downloadSelections) {