Index: trunk/src/org/openstreetmap/josm/Main.java =================================================================== --- trunk/src/org/openstreetmap/josm/Main.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/Main.java (revision 2322) @@ -20,4 +20,5 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -32,4 +33,6 @@ import org.openstreetmap.josm.actions.downloadtasks.DownloadGpsTask; import org.openstreetmap.josm.actions.downloadtasks.DownloadOsmTask; +import org.openstreetmap.josm.actions.downloadtasks.DownloadTask; +import org.openstreetmap.josm.actions.downloadtasks.PostDownloadHandler; import org.openstreetmap.josm.actions.mapmode.MapMode; import org.openstreetmap.josm.actions.search.SearchAction; @@ -46,5 +49,4 @@ import org.openstreetmap.josm.gui.SplashScreen; import org.openstreetmap.josm.gui.dialogs.LayerListDialog; -import org.openstreetmap.josm.gui.download.DownloadDialog.DownloadTask; import org.openstreetmap.josm.gui.help.HelpBrowserProxy; import org.openstreetmap.josm.gui.io.SaveLayersDialog; @@ -421,5 +423,6 @@ //DownloadTask osmTask = main.menu.download.downloadTasks.get(0); DownloadTask osmTask = new DownloadOsmTask(); - osmTask.download(main.menu.download, b.min.lat(), b.min.lon(), b.max.lat(), b.max.lon(), null); + Future future = osmTask.download(main.menu.download, b.min.lat(), b.min.lon(), b.max.lat(), b.max.lon(), null); + Main.worker.submit(new PostDownloadHandler(osmTask, future)); } return; @@ -466,5 +469,8 @@ try { DownloadTask task = rawGps ? new DownloadGpsTask() : new DownloadOsmTask(); - task.download(main.menu.download, Double.parseDouble(st.nextToken()), Double.parseDouble(st.nextToken()), Double.parseDouble(st.nextToken()), Double.parseDouble(st.nextToken()), null); + // asynchronously launch the download task ... + Future future = task.download(main.menu.download, Double.parseDouble(st.nextToken()), Double.parseDouble(st.nextToken()), Double.parseDouble(st.nextToken()), Double.parseDouble(st.nextToken()), null); + // ... and the continuation when the download is finished (this will wait for the download to finish) + Main.worker.execute(new PostDownloadHandler(task, future)); return; } catch (final NumberFormatException e) { Index: trunk/src/org/openstreetmap/josm/actions/DownloadAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/DownloadAction.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/actions/DownloadAction.java (revision 2322) @@ -8,4 +8,6 @@ import java.awt.event.ActionEvent; import java.awt.event.KeyEvent; +import java.util.List; +import java.util.concurrent.Future; import javax.swing.JOptionPane; @@ -13,7 +15,10 @@ import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.actions.downloadtasks.DownloadTask; +import org.openstreetmap.josm.actions.downloadtasks.PostDownloadHandler; +import org.openstreetmap.josm.gui.ExceptionDialogUtil; import org.openstreetmap.josm.gui.ExtendedDialog; import org.openstreetmap.josm.gui.download.DownloadDialog; -import org.openstreetmap.josm.gui.download.DownloadDialog.DownloadTask; +import org.openstreetmap.josm.tools.ExceptionUtil; import org.openstreetmap.josm.tools.Shortcut; import org.openstreetmap.josm.tools.WindowGeometry; @@ -69,5 +74,8 @@ Main.pref.put("download."+task.getPreferencesSuffix(), task.getCheckBox().isSelected()); if (task.getCheckBox().isSelected()) { - task.download(this, dialog.minlat, dialog.minlon, dialog.maxlat, dialog.maxlon, null); + // asynchronously launch the download task ... + Future future = task.download(this, dialog.minlat, dialog.minlon, dialog.maxlat, dialog.maxlon, null); + // ... and the continuation when the download task is finished + Main.worker.submit(new PostDownloadHandler(task, future)); finish = true; } Index: trunk/src/org/openstreetmap/josm/actions/OpenLocationAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/OpenLocationAction.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/actions/OpenLocationAction.java (revision 2322) @@ -11,4 +11,5 @@ import java.util.LinkedList; import java.util.List; +import java.util.concurrent.Future; import javax.swing.JCheckBox; @@ -18,5 +19,7 @@ import org.openstreetmap.josm.Main; import org.openstreetmap.josm.actions.downloadtasks.DownloadOsmTask; +import org.openstreetmap.josm.actions.downloadtasks.PostDownloadHandler; import org.openstreetmap.josm.gui.ExtendedDialog; +import org.openstreetmap.josm.gui.progress.PleaseWaitProgressMonitor; import org.openstreetmap.josm.gui.widgets.HistoryComboBox; import org.openstreetmap.josm.tools.GBC; @@ -102,5 +105,8 @@ */ public void openUrl(boolean new_layer, String url) { - new DownloadOsmTask().loadUrl(new_layer, url, null); + DownloadOsmTask task = new DownloadOsmTask(); + PleaseWaitProgressMonitor monitor = new PleaseWaitProgressMonitor(tr("Download Data")); + Future future = task.loadUrl(new_layer, url, monitor); + Main.worker.submit(new PostDownloadHandler(task, future)); } } Index: trunk/src/org/openstreetmap/josm/actions/downloadtasks/AbstractDownloadTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/downloadtasks/AbstractDownloadTask.java (revision 2322) +++ trunk/src/org/openstreetmap/josm/actions/downloadtasks/AbstractDownloadTask.java (revision 2322) @@ -0,0 +1,45 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.actions.downloadtasks; + +import java.util.ArrayList; +import java.util.List; + +public abstract class AbstractDownloadTask implements DownloadTask { + private List errorMessages; + private boolean canceled = false; + private boolean failed = false; + + public AbstractDownloadTask() { + errorMessages = new ArrayList(); + } + + public boolean isCanceled() { + return canceled; + } + + public void setCanceled(boolean canceled) { + this.canceled = canceled; + } + + public boolean isFailed() { + return failed; + } + + public void setFailed(boolean failed) { + this.failed = failed; + } + + + protected void rememberErrorMessage(String message) { + errorMessages.add(message); + } + + protected void rememberException(Exception exception) { + errorMessages.add(exception); + } + + + public List getErrorObjects() { + return errorMessages; + } +} Index: trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadGpsTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadGpsTask.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadGpsTask.java (revision 2322) @@ -12,7 +12,5 @@ import org.openstreetmap.josm.actions.DownloadAction; import org.openstreetmap.josm.data.gpx.GpxData; -import org.openstreetmap.josm.gui.ExceptionDialogUtil; import org.openstreetmap.josm.gui.PleaseWaitRunnable; -import org.openstreetmap.josm.gui.download.DownloadDialog.DownloadTask; import org.openstreetmap.josm.gui.layer.GpxLayer; import org.openstreetmap.josm.gui.layer.Layer; @@ -22,14 +20,44 @@ import org.xml.sax.SAXException; -public class DownloadGpsTask implements DownloadTask { - private Future task = null; +public class DownloadGpsTask extends AbstractDownloadTask { - private static class Task extends PleaseWaitRunnable { + private JCheckBox checkBox = new JCheckBox(tr("Raw GPS data")); + private DownloadTask downloadTask; + + public Future download(DownloadAction action, double minlat, double minlon, + double maxlat, double maxlon, ProgressMonitor progressMonitor) { + downloadTask = new DownloadTask(action.dialog.newLayer.isSelected(), + new BoundingBoxDownloader(minlat, minlon, maxlat, maxlon), progressMonitor); + // We need submit instead of execute so we can wait for it to finish and get the error + // message if necessary. If no one calls getErrorMessage() it just behaves like execute. + return Main.worker.submit(downloadTask); + } + + public JCheckBox getCheckBox() { + return checkBox; + } + + public String getPreferencesSuffix() { + return "gps"; + } + + public Future loadUrl(boolean a,java.lang.String b, ProgressMonitor progressMonitor) { + return null; + // FIXME this is not currently used + } + + public void cancel() { + if (downloadTask != null) { + downloadTask.cancel(); + } + } + + + class DownloadTask extends PleaseWaitRunnable { private BoundingBoxDownloader reader; private GpxData rawData; private final boolean newLayer; - private OsmTransferException lastException; - public Task(boolean newLayer, BoundingBoxDownloader reader, ProgressMonitor progressMonitor) { + public DownloadTask(boolean newLayer, BoundingBoxDownloader reader, ProgressMonitor progressMonitor) { super(tr("Downloading GPS data")); this.reader = reader; @@ -39,17 +67,21 @@ @Override public void realRun() throws IOException, SAXException, OsmTransferException { try { + if (isCanceled()) + return; rawData = reader.parseRawGps(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); - } catch(OsmTransferException e) { - lastException = e; } catch(Exception e) { - lastException = new OsmTransferException(e); + if (isCanceled()) + return; + if (e instanceof OsmTransferException) { + rememberException(e); + } else { + rememberException(new OsmTransferException(e)); + } } } @Override protected void finish() { - if (lastException != null) { - ExceptionDialogUtil.explainException(lastException); + if (isCanceled() || isFailed()) return; - } if (rawData == null) return; @@ -79,4 +111,5 @@ @Override protected void cancel() { + setCanceled(true); if (reader != null) { reader.cancel(); @@ -84,44 +117,3 @@ } } - - private JCheckBox checkBox = new JCheckBox(tr("Raw GPS data")); - - public void download(DownloadAction action, double minlat, double minlon, - double maxlat, double maxlon, ProgressMonitor progressMonitor) { - Task t = new Task(action.dialog.newLayer.isSelected(), - new BoundingBoxDownloader(minlat, minlon, maxlat, maxlon), progressMonitor); - // We need submit instead of execute so we can wait for it to finish and get the error - // message if necessary. If no one calls getErrorMessage() it just behaves like execute. - task = Main.worker.submit(t, t); - } - - public JCheckBox getCheckBox() { - return checkBox; - } - - public String getPreferencesSuffix() { - return "gps"; - } - - public void loadUrl(boolean a,java.lang.String b, ProgressMonitor progressMonitor) { - // FIXME this is not currently used - } - - /* - * (non-Javadoc) - * @see org.openstreetmap.josm.gui.download.DownloadDialog.DownloadTask#getErrorMessage() - */ - public String getErrorMessage() { - if(task == null) - return ""; - - try { - Task t = task.get(); - return t.getProgressMonitor().getErrorMessage() == null - ? "" - : t.getProgressMonitor().getErrorMessage(); - } catch (Exception e) { - return ""; - } - } } Index: trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTask.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTask.java (revision 2322) @@ -18,5 +18,4 @@ import org.openstreetmap.josm.data.osm.DataSource; import org.openstreetmap.josm.gui.PleaseWaitRunnable; -import org.openstreetmap.josm.gui.download.DownloadDialog.DownloadTask; import org.openstreetmap.josm.gui.layer.Layer; import org.openstreetmap.josm.gui.layer.OsmDataLayer; @@ -26,5 +25,4 @@ import org.openstreetmap.josm.io.OsmServerReader; import org.openstreetmap.josm.io.OsmTransferException; -import org.openstreetmap.josm.tools.ExceptionUtil; import org.xml.sax.SAXException; @@ -33,21 +31,77 @@ * Run in the worker thread. */ -public class DownloadOsmTask implements DownloadTask { +public class DownloadOsmTask extends AbstractDownloadTask { private static final Logger logger = Logger.getLogger(DownloadOsmTask.class.getName()); private static Bounds currentBounds; - private Future task = null; private DataSet downloadedData; - private boolean canceled = false; - private boolean failed = false; + private DownloadTask downloadTask; - private class Task extends PleaseWaitRunnable { + private JCheckBox checkBox = new JCheckBox(tr("OpenStreetMap data"), true); + + private void rememberDownloadedData(DataSet ds) { + this.downloadedData = ds; + } + + public DataSet getDownloadedData() { + return downloadedData; + } + + public Future download(DownloadAction action, double minlat, double minlon, + double maxlat, double maxlon, ProgressMonitor progressMonitor) { + // Swap min and max if user has specified them the wrong way round + // (easy to do if you are crossing 0, for example) + // FIXME should perhaps be done in download dialog? + if (minlat > maxlat) { + double t = minlat; minlat = maxlat; maxlat = t; + } + if (minlon > maxlon) { + double t = minlon; minlon = maxlon; maxlon = t; + } + + boolean newLayer = action != null + && (action.dialog == null || action.dialog.newLayer.isSelected()); + + downloadTask = new DownloadTask(newLayer, + new BoundingBoxDownloader(minlat, minlon, maxlat, maxlon), progressMonitor); + currentBounds = new Bounds(new LatLon(minlat, minlon), new LatLon(maxlat, maxlon)); + // We need submit instead of execute so we can wait for it to finish and get the error + // message if necessary. If no one calls getErrorMessage() it just behaves like execute. + return Main.worker.submit(downloadTask); + } + + /** + * Loads a given URL from the OSM Server + * @param True if the data should be saved to a new layer + * @param The URL as String + */ + public Future loadUrl(boolean new_layer, String url, ProgressMonitor progressMonitor) { + downloadTask = new DownloadTask(new_layer, + new OsmServerLocationReader(url), + progressMonitor); + currentBounds = new Bounds(new LatLon(0,0), new LatLon(0,0)); + return Main.worker.submit(downloadTask); + } + + public JCheckBox getCheckBox() { + return checkBox; + } + + public String getPreferencesSuffix() { + return "osm"; + } + + public void cancel() { + if (downloadTask != null) { + downloadTask.cancel(); + } + } + + private class DownloadTask extends PleaseWaitRunnable { private OsmServerReader reader; private DataSet dataSet; private boolean newLayer; - private boolean canceled; - private Exception lastException; - public Task(boolean newLayer, OsmServerReader reader, ProgressMonitor progressMonitor) { + public DownloadTask(boolean newLayer, OsmServerReader reader, ProgressMonitor progressMonitor) { super(tr("Downloading data"), progressMonitor, false); this.reader = reader; @@ -57,15 +111,18 @@ @Override public void realRun() throws IOException, SAXException, OsmTransferException { try { + if (isCanceled()) + return; dataSet = reader.parseOsm(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); } catch(Exception e) { - if (canceled) { + if (isCanceled()) { logger.warning(tr("Ignoring exception because download has been cancelled. Exception was: {0}" + e.toString())); return; } if (e instanceof OsmTransferException) { - lastException = e; + rememberException(e); } else { - lastException = new OsmTransferException(e); + rememberException(new OsmTransferException(e)); } + DownloadOsmTask.this.setFailed(true); } } @@ -102,11 +159,6 @@ @Override protected void finish() { - if (canceled) + if (isFailed() || isCanceled()) return; - if (lastException != null) { - getProgressMonitor().setErrorMessage(ExceptionUtil.explainException(lastException)); - DownloadOsmTask.this.setFailed(true); - return; - } if (dataSet == null) return; // user canceled download or error occurred @@ -114,5 +166,5 @@ return; // no data retrieved if (dataSet.allPrimitives().isEmpty()) { - progressMonitor.setErrorMessage(tr("No data found in this area.")); + rememberErrorMessage(tr("No data found in this area.")); // need to synthesize a download bounds lest the visual indication of downloaded // area doesn't work @@ -138,96 +190,8 @@ @Override protected void cancel() { - this.canceled = true; + setCanceled(true); if (reader != null) { reader.cancel(); } - DownloadOsmTask.this.setCanceled(true); - } - } - private JCheckBox checkBox = new JCheckBox(tr("OpenStreetMap data"), true); - - private void rememberDownloadedData(DataSet ds) { - this.downloadedData = ds; - } - - public boolean isCanceled() { - return canceled; - } - - public void setCanceled(boolean canceled) { - this.canceled = canceled; - } - - public boolean isFailed() { - return failed; - } - - public void setFailed(boolean failed) { - this.failed = failed; - } - - public DataSet getDownloadedData() { - return downloadedData; - } - - public void download(DownloadAction action, double minlat, double minlon, - double maxlat, double maxlon, ProgressMonitor progressMonitor) { - // Swap min and max if user has specified them the wrong way round - // (easy to do if you are crossing 0, for example) - // FIXME should perhaps be done in download dialog? - if (minlat > maxlat) { - double t = minlat; minlat = maxlat; maxlat = t; - } - if (minlon > maxlon) { - double t = minlon; minlon = maxlon; maxlon = t; - } - - boolean newLayer = action != null - && (action.dialog == null || action.dialog.newLayer.isSelected()); - - Task t = new Task(newLayer, - new BoundingBoxDownloader(minlat, minlon, maxlat, maxlon), progressMonitor); - currentBounds = new Bounds(new LatLon(minlat, minlon), new LatLon(maxlat, maxlon)); - // We need submit instead of execute so we can wait for it to finish and get the error - // message if necessary. If no one calls getErrorMessage() it just behaves like execute. - task = Main.worker.submit(t, t); - } - - /** - * Loads a given URL from the OSM Server - * @param True if the data should be saved to a new layer - * @param The URL as String - */ - public void loadUrl(boolean new_layer, String url, ProgressMonitor progressMonitor) { - Task t = new Task(new_layer, - new OsmServerLocationReader(url), - progressMonitor); - currentBounds = new Bounds(new LatLon(0,0), new LatLon(0,0)); - task = Main.worker.submit(t, t); - } - - public JCheckBox getCheckBox() { - return checkBox; - } - - public String getPreferencesSuffix() { - return "osm"; - } - - /* - * (non-Javadoc) - * @see org.openstreetmap.josm.gui.download.DownloadDialog.DownloadTask#getErrorMessage() - */ - public String getErrorMessage() { - if(task == null) - return ""; - - try { - Task t = task.get(); - return t.getProgressMonitor().getErrorMessage() == null - ? "" - : t.getProgressMonitor().getErrorMessage(); - } catch (Exception e) { - return ""; } } Index: trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTaskList.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTaskList.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTaskList.java (revision 2322) @@ -5,4 +5,6 @@ import java.awt.EventQueue; +import java.awt.event.ActionEvent; +import java.awt.event.ActionListener; import java.awt.geom.Area; import java.awt.geom.Rectangle2D; @@ -10,7 +12,9 @@ import java.util.Collection; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Set; +import java.util.concurrent.Future; import javax.swing.JOptionPane; @@ -20,8 +24,9 @@ import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.gui.download.DownloadDialog.DownloadTask; import org.openstreetmap.josm.gui.layer.Layer; import org.openstreetmap.josm.gui.layer.OsmDataLayer; import org.openstreetmap.josm.gui.progress.ProgressMonitor; +import org.openstreetmap.josm.gui.progress.ProgressMonitor.CancelListener; +import org.openstreetmap.josm.tools.ExceptionUtil; /** @@ -34,4 +39,5 @@ public class DownloadOsmTaskList implements Runnable { private List osmTasks = new LinkedList(); + private List> osmTaskFutures = new LinkedList>(); private ProgressMonitor progressMonitor; @@ -41,5 +47,5 @@ * @param The List of Rectangle2D to download */ - public void download(boolean newLayer, List rects, ProgressMonitor progressMonitor) { + public Future download(boolean newLayer, List rects, ProgressMonitor progressMonitor) { this.progressMonitor = progressMonitor; if(newLayer) { @@ -50,21 +56,25 @@ progressMonitor.beginTask(null, rects.size()); - try { - int i = 0; - 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)); - dt.download(null, td.getMinY(), td.getMinX(), td.getMaxY(), td.getMaxX(), childProgress); - osmTasks.add(dt); - } - } finally { - // If we try to get the error message now the download task will never have been started - // and we'd be stuck in a classical dead lock. Instead attach this to the worker and once - // run() gets called all downloadTasks have finished and we can grab the error messages. - Main.worker.execute(this); - } + int i = 0; + 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)); + 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); } @@ -93,21 +103,34 @@ public void run() { progressMonitor.finishTask(); - String errors = ""; - - LinkedList shown = new LinkedList(); + + // wait for all tasks to finish + // + for (Future future: osmTaskFutures) { + try { + future.get(); + } catch(Exception e) { + e.printStackTrace(); + return; + } + } + LinkedHashSet errors = new LinkedHashSet(); for(DownloadTask dt : osmTasks) { - String err = dt.getErrorMessage(); - // avoid display of identical messages - if (err.equals("") || shown.contains(err.hashCode())) { - continue; - } - shown.add(err.hashCode()); - errors += "
* " + err; - } - - if(! errors.equals("")) { + errors.addAll(dt.getErrorObjects()); + } + if (!errors.isEmpty()) { + StringBuffer sb = new StringBuffer(); + for (Object error:errors) { + if (error instanceof String) { + sb.append("
  • ").append(error).append("
    • ").append("
    "); + } else if (error instanceof Exception) { + sb.append("
  • ").append(ExceptionUtil.explainException((Exception)error)).append("
    • ").append("
    "); + } + } + sb.insert(0, "
      "); + sb.append("
    "); + JOptionPane.showMessageDialog( Main.parent, - ""+tr("The following errors occurred during mass download:{0}", errors) + ""+tr("The following errors occurred during mass download: {0}", sb.toString()) +"", tr("Errors during Download"), Index: trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTask.java (revision 2322) +++ trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTask.java (revision 2322) @@ -0,0 +1,52 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.actions.downloadtasks; + +import java.util.List; +import java.util.concurrent.Future; + +import javax.swing.JCheckBox; + +import org.openstreetmap.josm.actions.DownloadAction; +import org.openstreetmap.josm.gui.progress.ProgressMonitor; + + +public interface DownloadTask { + /** + * Execute the download using the given bounding box. Set silent on progressMonitor + * if no error messages should be popped up. + */ + Future download(DownloadAction action, double minlat, double minlon, + double maxlat, double maxlon, ProgressMonitor progressMonitor); + + /** + * Execute the download using the given URL + * @param newLayer + * @param url + */ + Future loadUrl(boolean newLayer, String url, ProgressMonitor progressMonitor); + + /** + * @return The checkbox presented to the user + */ + JCheckBox getCheckBox(); + + /** + * @return The name of the preferences suffix to use for storing the + * selection state. + */ + String getPreferencesSuffix(); + + /** + * Replies the error objects of the task. Empty list, if no error messages are available. + * + * 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 getErrorObjects(); + + public void cancel(); +} Index: trunk/src/org/openstreetmap/josm/actions/downloadtasks/PostDownloadHandler.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/downloadtasks/PostDownloadHandler.java (revision 2322) +++ trunk/src/org/openstreetmap/josm/actions/downloadtasks/PostDownloadHandler.java (revision 2322) @@ -0,0 +1,86 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.actions.downloadtasks; + +import static org.openstreetmap.josm.tools.I18n.tr; + +import java.util.LinkedHashSet; +import java.util.concurrent.Future; + +import javax.swing.JOptionPane; + +import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.gui.ExceptionDialogUtil; +import org.openstreetmap.josm.tools.ExceptionUtil; + + +public class PostDownloadHandler implements Runnable { + private DownloadTask task; + private Future future; + + /** + * constructor + * @param task the asynchronous download task + * @param future the future on which the completion of the download task can be synchronized + */ + public PostDownloadHandler(DownloadTask task, Future future) { + this.task = task; + this.future = future; + } + + public void run() { + // wait for the download task to complete + // + try { + future.get(); + } catch(Exception e) { + e.printStackTrace(); + return; + } + + // make sure errors are reported only once + // + LinkedHashSet errors = new LinkedHashSet(); + errors.addAll(task.getErrorObjects()); + if (errors.isEmpty()) + return; + + // just one error object? + // + if (errors.size() == 1) { + Object error = errors.iterator().next(); + if (error instanceof Exception) { + ExceptionDialogUtil.explainException((Exception)error); + return; + } + JOptionPane.showMessageDialog( + Main.parent, + error.toString(), + tr("Errors during Download"), + JOptionPane.ERROR_MESSAGE); + return; + + } + + // multiple error object? prepare a HTML list + // + if (!errors.isEmpty()) { + StringBuffer sb = new StringBuffer(); + for (Object error:errors) { + if (error instanceof String) { + sb.append("
  • ").append(error).append("
    • ").append("
    "); + } else if (error instanceof Exception) { + sb.append("
  • ").append(ExceptionUtil.explainException((Exception)error)).append("
    • ").append("
    "); + } + } + sb.insert(0, "
      "); + sb.append("
    "); + + JOptionPane.showMessageDialog( + Main.parent, + sb.toString(), + tr("Errors during Download"), + JOptionPane.ERROR_MESSAGE); + return; + } + } +} Index: trunk/src/org/openstreetmap/josm/gui/PleaseWaitRunnable.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/PleaseWaitRunnable.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/gui/PleaseWaitRunnable.java (revision 2322) @@ -73,20 +73,4 @@ } } - } catch (SAXException x) { - x.printStackTrace(); - progressMonitor.setErrorMessage(tr("Error while parsing")+": "+x.getMessage()); - } catch (FileNotFoundException x) { - x.printStackTrace(); - progressMonitor.setErrorMessage(tr("File not found")+": "+x.getMessage()); - } catch (IOException x) { - x.printStackTrace(); - progressMonitor.setErrorMessage(x.getMessage()); - } catch(OsmTransferException x) { - x.printStackTrace(); - if (x.getCause() != null) { - progressMonitor.setErrorMessage(x.getCause().getMessage()); - } else { - progressMonitor.setErrorMessage(x.getMessage()); - } } finally { progressMonitor.finishTask(); @@ -96,10 +80,10 @@ } } - } catch (final Throwable e) { + } catch (final Exception e) { if (!ignoreException) { // Exception has to thrown in EDT to be shown to user SwingUtilities.invokeLater(new Runnable() { public void run() { - throw new RuntimeException(e); + ExceptionDialogUtil.explainException(e); } }); Index: trunk/src/org/openstreetmap/josm/gui/download/DownloadDialog.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/download/DownloadDialog.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/gui/download/DownloadDialog.java (revision 2322) @@ -27,4 +27,5 @@ import org.openstreetmap.josm.actions.downloadtasks.DownloadGpsTask; import org.openstreetmap.josm.actions.downloadtasks.DownloadOsmTask; +import org.openstreetmap.josm.actions.downloadtasks.DownloadTask; import org.openstreetmap.josm.data.Bounds; import org.openstreetmap.josm.gui.MapView; @@ -46,42 +47,4 @@ public class DownloadDialog extends JPanel { - public interface DownloadTask { - /** - * Execute the download using the given bounding box. Set silent on progressMonitor - * if no error messages should be popped up. - */ - void download(DownloadAction action, double minlat, double minlon, - double maxlat, double maxlon, ProgressMonitor progressMonitor); - - /** - * Execute the download using the given URL - * @param newLayer - * @param url - */ - void loadUrl(boolean newLayer, String url, ProgressMonitor progressMonitor); - - /** - * @return The checkbox presented to the user - */ - JCheckBox getCheckBox(); - - /** - * @return The name of the preferences suffix to use for storing the - * selection state. - */ - String getPreferencesSuffix(); - - /** - * Gets the error message of the task once it executed. If there is no error message, an empty - * string is returned. - * - * 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 Error message or empty String - */ - String getErrorMessage(); - } - /** * The list of download tasks. First entry should be the osm data entry Index: trunk/src/org/openstreetmap/josm/gui/layer/GeoImageLayer.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/layer/GeoImageLayer.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/gui/layer/GeoImageLayer.java (revision 2322) @@ -278,4 +278,6 @@ private final Collection files; private final GpxLayer gpxLayer; + private LinkedList gps; + public Loader(Collection files, GpxLayer gpxLayer) { super(tr("Images for {0}", gpxLayer.getName())); @@ -286,5 +288,5 @@ progressMonitor.subTask(tr("Read GPX...")); progressMonitor.setTicksCount(10 + files.size()); - LinkedList gps = new LinkedList(); + gps = new LinkedList(); // Extract dates and locations from GPX input @@ -322,8 +324,6 @@ - if (gps.isEmpty()) { - progressMonitor.setErrorMessage(tr("No images with readable timestamps found.")); + if (gps.isEmpty()) return; - } // read the image files @@ -352,8 +352,18 @@ } @Override protected void finish() { + if (gps.isEmpty()) { + JOptionPane.showMessageDialog( + Main.parent, + tr("No images with readable timestamps found."), + tr("Warning"), + JOptionPane.WARNING_MESSAGE + ); + return; + } if (layer != null) { Main.main.addLayer(layer); } } + @Override protected void cancel() { Index: trunk/src/org/openstreetmap/josm/gui/layer/GpxLayer.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/layer/GpxLayer.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/gui/layer/GpxLayer.java (revision 2322) @@ -29,4 +29,5 @@ import java.util.LinkedList; import java.util.List; +import java.util.concurrent.Future; import javax.swing.AbstractAction; @@ -346,21 +347,23 @@ if (data.tracks.size() > 0) { - info.append(""); + info.append("
    " - + trn("{0} track", "{0} tracks", data.tracks.size(), data.tracks.size()) - + "
    " + tr("Name") + "" - + tr("Description") + "" + tr("Timespan") - + "" + tr("Length") + "" + tr("URL") - + "
    "); for (GpxTrack trk : data.tracks) { - WayPoint earliest = null, latest = null; - - info.append(""); - } - - info.append("
    " + + trn("{0} track", "{0} tracks", data.tracks.size(), data.tracks.size()) + + "
    " + tr("Name") + "" + + tr("Description") + "" + tr("Timespan") + + "" + tr("Length") + "" + tr("URL") + + "
    "); - if (trk.attr.containsKey("name")) - info.append(trk.attr.get("name")); - info.append(""); - if (trk.attr.containsKey("desc")) - info.append(" ").append(trk.attr.get("desc")); - info.append(""); + WayPoint earliest = null, latest = null; + + info.append("
    "); + if (trk.attr.containsKey("name")) { + info.append(trk.attr.get("name")); + } + info.append(""); + if (trk.attr.containsKey("desc")) { + info.append(" ").append(trk.attr.get("desc")); + } + info.append(""); for (Collection seg : trk.trackSegs) { @@ -378,21 +381,22 @@ } - if (earliest != null && latest != null) { - DateFormat df = DateFormat.getDateTimeInstance(DateFormat.MEDIUM, DateFormat.SHORT); - info.append(df.format(new Date((long) (earliest.time * 1000))) + " - " - + df.format(new Date((long) (latest.time * 1000)))); - int diff = (int) (latest.time - earliest.time); - info.append(" (" + (diff / 3600) + ":" + ((diff % 3600) / 60) + ")"); - } - - info.append(""); - info.append(new DecimalFormat("#0.00").format(trk.length() / 1000) + "km"); - info.append(""); - if (trk.attr.containsKey("url")) - info.append(trk.attr.get("url")); - info.append("


    "); + if (earliest != null && latest != null) { + DateFormat df = DateFormat.getDateTimeInstance(DateFormat.MEDIUM, DateFormat.SHORT); + info.append(df.format(new Date((long) (earliest.time * 1000))) + " - " + + df.format(new Date((long) (latest.time * 1000)))); + int diff = (int) (latest.time - earliest.time); + info.append(" (" + (diff / 3600) + ":" + ((diff % 3600) / 60) + ")"); + } + + info.append(""); + info.append(new DecimalFormat("#0.00").format(trk.length() / 1000) + "km"); + info.append(""); + if (trk.attr.containsKey("url")) { + info.append(trk.attr.get("url")); + } + info.append(""); + } + + info.append("

    "); } @@ -401,6 +405,6 @@ info.append("
    "); - info.append(trn("{0} route, ", "{0} routes, ", data.routes.size(), data.routes.size())).append( - trn("{0} waypoint", "{0} waypoints", data.waypoints.size(), data.waypoints.size())).append("
    "); + info.append(trn("{0} route, ", "{0} routes, ", data.routes.size(), data.routes.size())).append( + trn("{0} waypoint", "{0} waypoints", data.waypoints.size(), data.waypoints.size())).append("
    "); return info.append("").toString(); @@ -694,5 +698,5 @@ } // end for segment } // end for trk - } // end if large || hdopcircle + } // end if large || hdopcircle /**************************************************************** @@ -939,5 +943,19 @@ } } - new DownloadOsmTaskList().download(false, toDownload, new PleaseWaitProgressMonitor(tr("Download data"))); + final PleaseWaitProgressMonitor monitor = new PleaseWaitProgressMonitor(tr("Download data")); + final Future future = new DownloadOsmTaskList().download(false, toDownload, monitor); + Main.worker.submit( + new Runnable() { + public void run() { + try { + future.get(); + } catch(Exception e) { + e.printStackTrace(); + return; + } + monitor.close(); + } + } + ); } } Index: trunk/src/org/openstreetmap/josm/gui/progress/PleaseWaitProgressMonitor.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/progress/PleaseWaitProgressMonitor.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/gui/progress/PleaseWaitProgressMonitor.java (revision 2322) @@ -16,4 +16,5 @@ import org.openstreetmap.josm.Main; import org.openstreetmap.josm.gui.PleaseWaitDialog; +import static org.openstreetmap.josm.tools.I18n.tr; Index: trunk/src/org/openstreetmap/josm/gui/progress/ProgressMonitor.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/progress/ProgressMonitor.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/gui/progress/ProgressMonitor.java (revision 2322) @@ -121,6 +121,4 @@ void setSilent(boolean value); - void setErrorMessage(String message); - String getErrorMessage(); /** Index: trunk/src/org/openstreetmap/josm/io/OsmConnection.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmConnection.java (revision 2321) +++ trunk/src/org/openstreetmap/josm/io/OsmConnection.java (revision 2322) @@ -97,11 +97,19 @@ public void cancel() { cancel = true; - if (activeConnection != null) { - activeConnection.setConnectTimeout(100); - activeConnection.setReadTimeout(100); - try { - Thread.sleep(100); - } catch (InterruptedException ex) {} - activeConnection.disconnect(); + synchronized (this) { + if (activeConnection != null) { + activeConnection.setConnectTimeout(100); + activeConnection.setReadTimeout(100); + } + } + try { + Thread.sleep(100); + } catch (InterruptedException ex) { + } + + synchronized (this) { + if (activeConnection != null) { + activeConnection.disconnect(); + } } }