Index: /trunk/src/org/openstreetmap/josm/actions/UploadAction.java =================================================================== --- /trunk/src/org/openstreetmap/josm/actions/UploadAction.java (revision 1662) +++ /trunk/src/org/openstreetmap/josm/actions/UploadAction.java (revision 1663) @@ -4,10 +4,17 @@ import static org.openstreetmap.josm.tools.I18n.tr; +import java.awt.EventQueue; import java.awt.GridBagLayout; import java.awt.event.ActionEvent; import java.awt.event.KeyEvent; +import java.io.IOException; +import java.net.HttpURLConnection; import java.util.Collection; import java.util.LinkedList; import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.FutureTask; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.swing.JLabel; @@ -16,15 +23,22 @@ import javax.swing.JPanel; import javax.swing.JScrollPane; +import javax.swing.SwingUtilities; import org.openstreetmap.josm.Main; import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.visitor.CreateOsmChangeVisitor; import org.openstreetmap.josm.gui.ExtendedDialog; import org.openstreetmap.josm.gui.OsmPrimitivRenderer; import org.openstreetmap.josm.gui.PleaseWaitRunnable; import org.openstreetmap.josm.gui.historycombobox.SuggestingJHistoryComboBox; +import org.openstreetmap.josm.io.DiffResultReader; +import org.openstreetmap.josm.io.OsmApi; +import org.openstreetmap.josm.io.OsmApiException; +import org.openstreetmap.josm.io.OsmApiInitializationException; import org.openstreetmap.josm.io.OsmServerWriter; import org.openstreetmap.josm.tools.GBC; import org.openstreetmap.josm.tools.Shortcut; import org.xml.sax.SAXException; + /** @@ -66,5 +80,5 @@ public UploadAction() { super(tr("Upload to OSM..."), "upload", tr("Upload all changes to the OSM server."), - Shortcut.registerShortcut("file:upload", tr("File: {0}", tr("Upload to OSM...")), KeyEvent.VK_U, Shortcut.GROUPS_ALT1+Shortcut.GROUP_HOTKEY), true); + Shortcut.registerShortcut("file:upload", tr("File: {0}", tr("Upload to OSM...")), KeyEvent.VK_U, Shortcut.GROUPS_ALT1+Shortcut.GROUP_HOTKEY), true); /** @@ -112,8 +126,8 @@ while(true) { int result = new ExtendedDialog(Main.parent, - tr("Upload these changes?"), - p, - new String[] {tr("Upload Changes"), tr("Cancel")}, - new String[] {"upload.png", "cancel.png"}).getValue(); + tr("Upload these changes?"), + p, + new String[] {tr("Upload Changes"), tr("Cancel")}, + new String[] {"upload.png", "cancel.png"}).getValue(); // cancel pressed @@ -121,5 +135,7 @@ // don't allow empty commit message - if (cmt.getText().trim().length() < 3) continue; + if (cmt.getText().trim().length() < 3) { + continue; + } // store the history of comments @@ -151,12 +167,14 @@ final LinkedList delete = new LinkedList(); for (OsmPrimitive osm : Main.ds.allPrimitives()) { - if (osm.get("josm/ignore") != null) + if (osm.get("josm/ignore") != null) { continue; - if (osm.id == 0 && !osm.deleted) + } + if (osm.id == 0 && !osm.deleted) { add.addLast(osm); - else if (osm.modified && !osm.deleted) + } else if (osm.modified && !osm.deleted) { update.addLast(osm); - else if (osm.deleted && osm.id != 0) + } else if (osm.deleted && osm.id != 0) { delete.addFirst(osm); + } } @@ -178,16 +196,101 @@ all.addAll(delete); - PleaseWaitRunnable uploadTask = new PleaseWaitRunnable(tr("Uploading data")){ - @Override protected void realRun() throws SAXException { - server.uploadOsm(Main.ds.version, all); - } + class UploadDiffTask extends PleaseWaitRunnable { + + private boolean uploadCancelled = false; + private boolean uploadFailed = false; + private Exception lastException = null; + + public UploadDiffTask() { + super(tr("Uploading"),false /* don't ignore exceptions */); + } + + @Override protected void realRun() throws SAXException, IOException { + try { + server.uploadOsm(Main.ds.version, all); + Main.main.editLayer().cleanData(server.processed, !add.isEmpty()); + } catch (Exception sxe) { + if (uploadCancelled) { + System.out.println("Ignoring exception caught because upload is cancelled. Exception is: " + sxe.toString()); + return; + } + System.out.println("got exception: " + sxe.toString()); + uploadFailed = true; + lastException = sxe; + } + } + @Override protected void finish() { - Main.main.editLayer().cleanData(server.processed, !add.isEmpty()); - } + if (uploadFailed) { + handleFailedUpload(lastException); + } + } + @Override protected void cancel() { - // FIXME server.cancel(); - } - }; - Main.worker.execute(uploadTask); + server.disconnectActiveConnection(); + uploadCancelled = true; + }; + + } + Main.worker.execute(new UploadDiffTask()); + } + + public void handleFailedUpload(Exception e) { + if (e instanceof OsmApiInitializationException) { + handleOsmApiInitializationException(e); + return; + } + if (e instanceof OsmApiException) { + OsmApiException ex = (OsmApiException)e; + if(ex.getResponseCode() == HttpURLConnection.HTTP_CONFLICT) { + Pattern p = Pattern.compile("Version mismatch: Provided (\\d+), server had: (\\d+) of (\\S+) (\\d+)"); + Matcher m = p.matcher(ex.getErrorHeader()); + String msg; + if (m.matches()) { + msg = tr("Uploading failed because the server has a newer version of one
" + + "of your nodes, ways or relations.
" + + "The conflict is cause by the {0} with id {1},
" + + "the server has version {2}, your version is {3}.
" + + "Please synchronize your local dataset using
" + + "File->Update Data, resolve
" + + "any conflicts and try to upload again.", + m.group(3),m.group(4), m.group(2), m.group(1) + ); + } else { + msg = tr("Uploading failed because the server has a newer version of one
" + + "of your nodes, ways or relations.
" + + "Please synchronize your local dataset using
" + + "File->Update Data, resolve
" + + "any conflicts and try to upload again." + ); + } + JOptionPane.showMessageDialog( + null, + msg, + tr("Upload to OSM API failed"), + JOptionPane.WARNING_MESSAGE + ); + return; + } + } + JOptionPane.showMessageDialog( + null, + e.getMessage(), + tr("Upload to OSM API failed"), + JOptionPane.ERROR_MESSAGE + ); + } + + protected void handleOsmApiInitializationException(Exception e) { + JOptionPane.showMessageDialog( + null, + tr( "Failed to initialize communication with the OSM server {0}.\n" + + "Check the server URL in your preferences and your internet connection.", + Main.pref.get("osm-server.url") + ), + tr("Error"), + JOptionPane.ERROR_MESSAGE + ); + e.printStackTrace(); } } Index: /trunk/src/org/openstreetmap/josm/io/OsmApi.java =================================================================== --- /trunk/src/org/openstreetmap/josm/io/OsmApi.java (revision 1662) +++ /trunk/src/org/openstreetmap/josm/io/OsmApi.java (revision 1663) @@ -23,5 +23,4 @@ import java.util.Properties; import java.util.StringTokenizer; -import java.util.concurrent.FutureTask; import javax.xml.parsers.SAXParserFactory; @@ -34,5 +33,4 @@ import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.osm.visitor.CreateOsmChangeVisitor; -import org.openstreetmap.josm.gui.PleaseWaitRunnable; import org.xml.sax.Attributes; import org.xml.sax.InputSource; @@ -50,4 +48,6 @@ */ public class OsmApi extends OsmConnection { + /** max number of retries to send a request in case of HTTP 500 errors or timeouts */ + static public final int DEFAULT_MAX_NUM_RETRIES = 5; /** @@ -93,6 +93,7 @@ minVersion = atts.getValue("minimum"); maxVersion = atts.getValue("maximum"); - } else if (qName.equals("area")) + } else if (qName.equals("area")) { maxArea = atts.getValue("maximum"); + } } } @@ -132,6 +133,6 @@ * @exception UnknownHostException thrown, if the API host is unknown * @exception SocketTimeoutException thrown, if the connection to the API host times out - * @exception ConnectException throw, if the connection to the API host fails - * @exception Exception any other exception + * @exception ConnectException throw, if the connection to the API host fails + * @exception Exception any other exception */ public void initialize() throws UnknownHostException,SocketTimeoutException, ConnectException,Exception { @@ -149,10 +150,10 @@ System.err.println(tr("This version of JOSM is incompatible with the configured server.")); System.err.println(tr("It supports protocol versions 0.5 and 0.6, while the server says it supports {0} to {1}.", - minVersion, maxVersion)); + minVersion, maxVersion)); initialized = false; } System.out.println(tr("Communications with {0} established using protocol version {1}", - Main.pref.get("osm-server.url"), - version)); + Main.pref.get("osm-server.url"), + version)); osmWriter.setVersion(version); } catch (Exception ex) { @@ -302,112 +303,27 @@ public Collection uploadDiff(final Collection list) throws OsmTransferException { - if (changeset == null) { + if (changeset == null) throw new OsmTransferException(tr("No changeset present for diff upload")); - } - final ArrayList processed = new ArrayList(); - // this is the asynchronous update task - // - class UploadDiffTask extends PleaseWaitRunnable { - - private boolean uploadCancelled = false; - private boolean uploadFailed = false; - private Throwable lastThrowable = null; - - public UploadDiffTask(String title) { - super(title,false /* don't ignore exceptions */); - } - - @Override protected void realRun() throws SAXException, IOException { - CreateOsmChangeVisitor duv = new CreateOsmChangeVisitor(changeset, OsmApi.this); - - for (OsmPrimitive osm : list) { - int progress = Main.pleaseWaitDlg.progress.getValue(); - Main.pleaseWaitDlg.currentAction.setText(tr("Preparing...")); - osm.visit(duv); - Main.pleaseWaitDlg.progress.setValue(progress+1); - } - - Main.pleaseWaitDlg.currentAction.setText(tr("Uploading...")); - - String diff = duv.getDocument(); - try { - String diffresult = sendRequest("POST", "changeset/" + changeset.id + "/upload", diff); - DiffResultReader.parseDiffResult(diffresult, list, processed, duv.getNewIdMap(), Main.pleaseWaitDlg); - } catch (Exception sxe) { - if (isUploadCancelled()) { - // ignore exceptions thrown because the connection is aborted, - // i.e. IOExceptions or SocketExceptions - // - System.out.println("Ignoring exception caught because upload is cancelled. Exception is: " + sxe.toString()); - return; - } - uploadFailed = true; - // remember last exception and don't throw it. If it was thrown again it would - // have to be encapsulated in a RuntimeException which would be nested in yet - // another RuntimeException by parent classes. - // Rather check isUploadFailed() and retrieve getLastThrowable() after the task - // is completed - // - lastThrowable = sxe; - } - } - - @Override protected void finish() { - // do nothing - } - - @Override protected void cancel() { - activeConnection.disconnect(); - uploadCancelled = true; - } - - public boolean isUploadCancelled() { - return uploadCancelled; - } - - public boolean isUploadFailed() { - return uploadFailed; - } - - public Throwable getLastThrowable() { - return lastThrowable; - } - } - - UploadDiffTask uploadTask = new UploadDiffTask(tr("Uploading data")); - - // run data upload as asynchronous task - // + + CreateOsmChangeVisitor duv = new CreateOsmChangeVisitor(changeset, OsmApi.this); + + for (OsmPrimitive osm : list) { + int progress = Main.pleaseWaitDlg.progress.getValue(); + Main.pleaseWaitDlg.currentAction.setText(tr("Preparing...")); + osm.visit(duv); + Main.pleaseWaitDlg.progress.setValue(progress+1); + } + + Main.pleaseWaitDlg.currentAction.setText(tr("Uploading...")); + + String diff = duv.getDocument(); + String diffresult = sendRequest("POST", "changeset/" + changeset.id + "/upload", diff); try { - Void result = null; - FutureTask task = new FutureTask(uploadTask, result); - task.run(); - task.get(); // wait for the task to complete, no return value expected, though - } catch(Throwable e) { - if (uploadTask.isUploadCancelled()) { - throw new OsmTransferCancelledException(); - } + DiffResultReader.parseDiffResult(diffresult, list, processed, duv.getNewIdMap(), Main.pleaseWaitDlg); + } catch(Exception e) { throw new OsmTransferException(e); - } - - // handle failed upload - // - if (uploadTask.isUploadFailed()) { - if (uploadTask.getLastThrowable() != null && uploadTask.getLastThrowable() instanceof OsmTransferException) { - OsmTransferException e = (OsmTransferException)uploadTask.getLastThrowable(); - throw e; - } - // shouldn't happen, but just in case - // - throw new OsmTransferException(tr("Data upload failed for unknown reason")); - } - - // handle cancelled upload - // - if (uploadTask.isUploadCancelled()) { - throw new OsmTransferCancelledException(); } @@ -420,7 +336,6 @@ // System.out.print("backing off for 10 seconds..."); for(int i=0; i < 10; i++) { - if (cancel || isAuthCancelled()) { + if (cancel || isAuthCancelled()) throw new OsmTransferCancelledException(); - } try { Thread.sleep(1000); @@ -450,7 +365,8 @@ StringBuffer responseBody = new StringBuffer(); - StringBuffer statusMessage = new StringBuffer(); - - int retries = 5; // configurable? + + int retries = Main.pref.getInteger("osm-server.max-num-retries", DEFAULT_MAX_NUM_RETRIES); + retries = Math.max(0,retries); + while(true) { // the retry loop @@ -511,33 +427,30 @@ responseBody.append("\n"); } - - statusMessage.setLength(0); + String errorHeader = null; // Look for a detailed error message from the server if (activeConnection.getHeaderField("Error") != null) { - String er = activeConnection.getHeaderField("Error"); - System.err.println("Error header: " + er); - statusMessage.append(tr(er)); + errorHeader = activeConnection.getHeaderField("Error"); + System.err.println("Error header: " + errorHeader); } else if (retCode != 200 && responseBody.length()>0) { System.err.println("Error body: " + responseBody); - statusMessage.append(tr(responseBody.toString())); - } else { - statusMessage.append(activeConnection.getResponseMessage()); } activeConnection.disconnect(); - if (retCode != 200) { - throw new OsmTransferException(statusMessage.toString()); - } + if (retCode != 200) + throw new OsmApiException(retCode,errorHeader,responseBody.toString()); + return responseBody.toString(); } catch (UnknownHostException e) { - throw new OsmTransferException(tr("Unknown host")+": "+e.getMessage(), e); + throw new OsmTransferException(e); } catch (SocketTimeoutException e) { - if (retries-- > 0) + if (retries-- > 0) { continue; - throw new OsmTransferException(e.getMessage()+ " " + e.getClass().getCanonicalName(), e); + } + throw new OsmTransferException(e); } catch (ConnectException e) { - if (retries-- > 0) + if (retries-- > 0) { continue; - throw new OsmTransferException(e.getMessage()+ " " + e.getClass().getCanonicalName(), e); + } + throw new OsmTransferException(e); } catch (Exception e) { if (e instanceof OsmTransferException) throw (OsmTransferException) e; Index: /trunk/src/org/openstreetmap/josm/io/OsmApiException.java =================================================================== --- /trunk/src/org/openstreetmap/josm/io/OsmApiException.java (revision 1663) +++ /trunk/src/org/openstreetmap/josm/io/OsmApiException.java (revision 1663) @@ -0,0 +1,73 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.io; + +public class OsmApiException extends OsmTransferException { + + private int responseCode; + private String errorHeader; + private String errorBody; + + public OsmApiException() { + super(); + } + + public OsmApiException(int responseCode, String errorHeader, String errorBody) { + this.responseCode = responseCode; + this.errorHeader = errorHeader; + this.errorBody = errorBody; + } + + public OsmApiException(String message, Throwable cause) { + super(message, cause); + } + + public OsmApiException(String message) { + super(message); + } + + public OsmApiException(Throwable cause) { + super(cause); + } + + public int getResponseCode() { + return responseCode; + } + + public void setResponseCode(int responseCode) { + this.responseCode = responseCode; + } + + public String getErrorHeader() { + return errorHeader; + } + + public void setErrorHeader(String errorHeader) { + this.errorHeader = errorHeader; + } + + public String getErrorBody() { + return errorBody; + } + + public void setErrorBody(String errorBody) { + this.errorBody = errorBody; + } + + @Override + public String getMessage() { + StringBuilder sb = new StringBuilder(); + sb.append("ResponseCode=") + .append(responseCode); + if (errorHeader != null) { + sb.append(", Error Header=<") + .append(errorHeader) + .append(">"); + } + if (errorBody != null) { + sb.append(",Error Body=<") + .append(errorBody) + .append(">"); + } + return sb.toString(); + } +} Index: /trunk/src/org/openstreetmap/josm/io/OsmApiInitializationException.java =================================================================== --- /trunk/src/org/openstreetmap/josm/io/OsmApiInitializationException.java (revision 1663) +++ /trunk/src/org/openstreetmap/josm/io/OsmApiInitializationException.java (revision 1663) @@ -0,0 +1,21 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.io; + +public class OsmApiInitializationException extends OsmTransferException { + + public OsmApiInitializationException() { + super(); + } + + public OsmApiInitializationException(String message, Throwable cause) { + super(message, cause); + } + + public OsmApiInitializationException(String message) { + super(message); + } + + public OsmApiInitializationException(Throwable cause) { + super(cause); + } +} Index: /trunk/src/org/openstreetmap/josm/io/OsmServerWriter.java =================================================================== --- /trunk/src/org/openstreetmap/josm/io/OsmServerWriter.java (revision 1662) +++ /trunk/src/org/openstreetmap/josm/io/OsmServerWriter.java (revision 1663) @@ -7,6 +7,5 @@ import java.util.LinkedList; import java.util.List; - -import javax.swing.JOptionPane; +import java.util.logging.Logger; import org.openstreetmap.josm.Main; @@ -14,5 +13,4 @@ import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.visitor.NameVisitor; -import org.openstreetmap.josm.gui.historycombobox.JHistoryComboBox; /** @@ -24,4 +22,5 @@ */ public class OsmServerWriter { + static private final Logger logger = Logger.getLogger(OsmServerWriter.class.getName()); /** @@ -45,6 +44,7 @@ long now = System.currentTimeMillis(); long elapsed = now - uploadStartTime; - if (elapsed == 0) + if (elapsed == 0) { elapsed = 1; + } float uploads_per_ms = (float)progress / elapsed; float uploads_left = list_size - progress; @@ -53,6 +53,7 @@ int seconds_left = (ms_left / MSECS_PER_SECOND) % SECONDS_PER_MINUTE ; String time_left_str = Integer.toString(minutes_left) + ":"; - if (seconds_left < 10) + if (seconds_left < 10) { time_left_str += "0"; + } time_left_str += Integer.toString(seconds_left); return time_left_str; @@ -60,9 +61,25 @@ /** + * retrieves the most recent changeset comment from the preferences + * + * @return the most recent changeset comment + */ + protected String getChangesetComment() { + String cmt = ""; + List history = new LinkedList( + Main.pref.getCollection(UploadAction.HISTORY_KEY, new LinkedList())); + if(history.size() > 0) { + cmt = history.get(0); + } + return cmt; + } + + /** * Send the dataset to the server. - * @param the_version version of the data set - * @param list list of objects to send + * + * @param apiVersion version of the data set + * @param primitives list of objects to send */ - public void uploadOsm(String the_version, Collection list) { + public void uploadOsm(String apiVersion, Collection primitives) throws OsmTransferException { processed = new LinkedList(); @@ -73,72 +90,53 @@ api.initialize(); } catch(Exception e) { - JOptionPane.showMessageDialog( - null, - tr( "Failed to initialize communication with the OSM server {0}.\n" - + "Check the server URL in your preferences and your internet connection.", - Main.pref.get("osm-server.url") - ), - tr("Error"), - JOptionPane.ERROR_MESSAGE - ); - e.printStackTrace(); - return; + throw new OsmApiInitializationException(e); } - Main.pleaseWaitDlg.progress.setMaximum(list.size()); + Main.pleaseWaitDlg.progress.setMaximum(primitives.size()); Main.pleaseWaitDlg.progress.setValue(0); - boolean useChangesets = api.hasChangesetSupport(); - - // controls whether or not we try and upload the whole bunch in one go - boolean useDiffUploads = Main.pref.getBoolean("osm-server.atomic-upload", - "0.6".equals(api.getVersion())); - - // create changeset if required - try { - if (useChangesets) { - // add the last entered comment to the changeset - String cmt = ""; - List history = new LinkedList( - Main.pref.getCollection(UploadAction.HISTORY_KEY, new LinkedList())); - if(history.size() > 0) { - cmt = history.get(0); - } - api.createChangeset(cmt); - } - } catch (OsmTransferException ex) { - dealWithTransferException(ex); - return; + // check whether we can use changeset + // + boolean canUseChangeset = api.hasChangesetSupport(); + boolean useChangeset = Main.pref.getBoolean("osm-server.atomic-upload", apiVersion.compareTo("0.6")>=0); + if (useChangeset && ! canUseChangeset) { + System.out.println(tr("WARNING: preference '{0}' or api version {1} of dataset requires to use changesets, but API is not handle them. Ignoring changesets.", "osm-server.atomic-upload", apiVersion)); + useChangeset = false; } - try { - if (useDiffUploads) { - // all in one go - processed.addAll(api.uploadDiff(list)); - } else { - // upload changes individually (90% of code is for the status display...) - NameVisitor v = new NameVisitor(); - uploadStartTime = System.currentTimeMillis(); - for (OsmPrimitive osm : list) { - osm.visit(v); - int progress = Main.pleaseWaitDlg.progress.getValue(); - String time_left_str = timeLeft(progress, list.size()); - Main.pleaseWaitDlg.currentAction.setText( - tr("{0}% ({1}/{2}), {3} left. Uploading {4}: {5} (id: {6})", - Math.round(100.0*progress/list.size()), progress, - list.size(), time_left_str, tr(v.className), v.name, osm.id)); - makeApiRequest(osm); - processed.add(osm); - Main.pleaseWaitDlg.progress.setValue(progress+1); + if (useChangeset) { + // upload everything in one changeset + // + try { + api.createChangeset(getChangesetComment()); + processed.addAll(api.uploadDiff(primitives)); + } catch(OsmTransferException e) { + throw e; + } finally { + try { + if (canUseChangeset) { + api.stopChangeset(); + } + } catch (Exception ee) { + // ignore nested exception } } - if (useChangesets) api.stopChangeset(); - } catch (OsmTransferException e) { - try { - if (useChangesets) api.stopChangeset(); - } catch (Exception ee) { - // ignore nested exception + } else { + // upload changes individually (90% of code is for the status display...) + // + NameVisitor v = new NameVisitor(); + uploadStartTime = System.currentTimeMillis(); + for (OsmPrimitive osm : primitives) { + osm.visit(v); + int progress = Main.pleaseWaitDlg.progress.getValue(); + String time_left_str = timeLeft(progress, primitives.size()); + Main.pleaseWaitDlg.currentAction.setText( + tr("{0}% ({1}/{2}), {3} left. Uploading {4}: {5} (id: {6})", + Math.round(100.0*progress/primitives.size()), progress, + primitives.size(), time_left_str, tr(v.className), v.name, osm.id)); + makeApiRequest(osm); + processed.add(osm); + Main.pleaseWaitDlg.progress.setValue(progress+1); } - dealWithTransferException(e); } } @@ -154,14 +152,8 @@ } - private void dealWithTransferException (OsmTransferException e) { - if (e instanceof OsmTransferCancelledException) { - // ignore - don't bother the user with yet another message that he - // has successfully cancelled the data upload - // - return; + public void disconnectActiveConnection() { + if (api != null && api.activeConnection != null) { + api.activeConnection.disconnect(); } - - JOptionPane.showMessageDialog(Main.parent, - /* tr("Error during upload: ") + */ e.getMessage()); } }