Index: trunk/src/org/openstreetmap/josm/actions/SaveActionBase.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/SaveActionBase.java (revision 1893) +++ trunk/src/org/openstreetmap/josm/actions/SaveActionBase.java (revision 1894) @@ -216,5 +216,5 @@ return; } - layer.cleanData(null, false); + layer.cleanupAfterSaveToDisk(); } catch (IOException e) { e.printStackTrace(); Index: trunk/src/org/openstreetmap/josm/actions/UploadAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/UploadAction.java (revision 1893) +++ trunk/src/org/openstreetmap/josm/actions/UploadAction.java (revision 1894) @@ -235,11 +235,11 @@ ); switch(ret) { - case JOptionPane.CLOSED_OPTION: return; - case JOptionPane.CANCEL_OPTION: return; - case 0: synchronizePrimitive(id); break; - case 1: synchronizeDataSet(); break; - default: - // should not happen - throw new IllegalStateException(tr("unexpected return value. Got {0}", ret)); + case JOptionPane.CLOSED_OPTION: return; + case JOptionPane.CANCEL_OPTION: return; + case 0: synchronizePrimitive(id); break; + case 1: synchronizeDataSet(); break; + default: + // should not happen + throw new IllegalStateException(tr("unexpected return value. Got {0}", ret)); } } @@ -274,10 +274,10 @@ ); switch(ret) { - case JOptionPane.CLOSED_OPTION: return; - case 1: return; - case 0: synchronizeDataSet(); break; - default: - // should not happen - throw new IllegalStateException(tr("unexpected return value. Got {0}", ret)); + case JOptionPane.CLOSED_OPTION: return; + case 1: return; + case 0: synchronizeDataSet(); break; + default: + // should not happen + throw new IllegalStateException(tr("unexpected return value. Got {0}", ret)); } } @@ -537,5 +537,4 @@ try { writer.uploadOsm(getCurrentDataSet().version, toUpload, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); - getEditLayer().cleanData(writer.processed, !toUpload.isEmpty()); } catch (Exception sxe) { if (uploadCancelled) { @@ -550,4 +549,9 @@ if (uploadCancelled) return; + + // we always clean the data, even in case of errors. It's possible the data was + // partially uploaded + // + getEditLayer().cleanupAfterUpload(writer.getProcessedPrimitives()); if (lastException != null) { handleFailedUpload(lastException); Index: trunk/src/org/openstreetmap/josm/command/Command.java =================================================================== --- trunk/src/org/openstreetmap/josm/command/Command.java (revision 1893) +++ trunk/src/org/openstreetmap/josm/command/Command.java (revision 1894) @@ -89,4 +89,5 @@ e.getKey().cloneFrom(e.getValue()); } + getLayer().setModified(true); } Index: trunk/src/org/openstreetmap/josm/data/UndoRedoHandler.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/UndoRedoHandler.java (revision 1893) +++ trunk/src/org/openstreetmap/josm/data/UndoRedoHandler.java (revision 1894) @@ -99,8 +99,10 @@ } - public void layerRemoved(Layer oldLayer) { + public void clean(Layer layer) { + if (layer == null) + return; boolean changed = false; for (Iterator it = commands.iterator(); it.hasNext();) { - if (it.next().invalidBecauselayerRemoved(oldLayer)) { + if (it.next().invalidBecauselayerRemoved(layer)) { it.remove(); changed = true; @@ -108,5 +110,5 @@ } for (Iterator it = redoCommands.iterator(); it.hasNext();) { - if (it.next().invalidBecauselayerRemoved(oldLayer)) { + if (it.next().invalidBecauselayerRemoved(layer)) { it.remove(); changed = true; @@ -118,4 +120,8 @@ } + public void layerRemoved(Layer oldLayer) { + clean(oldLayer); + } + public void layerAdded(Layer newLayer) {} public void activeLayerChange(Layer oldLayer, Layer newLayer) {} Index: trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (revision 1893) +++ trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (revision 1894) @@ -65,4 +65,6 @@ import org.openstreetmap.josm.tools.ImageProvider; +import sun.security.action.GetLongAction; + /** * A layer holding data from a specific dataset. @@ -317,43 +319,41 @@ /** + * Cleanup the layer after save to disk. Just marks the layer as unmodified. + * Leaves the undo/redo stack unchanged. + * + */ + public void cleanupAfterSaveToDisk() { + setModified(false); + } + + /** * Clean out the data behind the layer. This means clearing the redo/undo lists, - * really deleting all deleted objects and reset the modified flags. This is done - * after a successfull upload. + * really deleting all deleted objects and reset the modified flags. This should + * be done after an upload, even after a partial upload. * * @param processed A list of all objects that were actually uploaded. - * May be null, which means nothing has been uploaded but - * saved to disk instead. Note that an empty collection for "processed" - * means that an upload has been attempted but failed. - */ - public void cleanData(final Collection processed, boolean dataAdded) { + * May be null, which means nothing has been uploaded + */ + public void cleanupAfterUpload(final Collection processed) { // return immediately if an upload attempt failed - if (processed != null && processed.isEmpty() && !dataAdded) + if (processed == null || processed.isEmpty()) return; - Main.main.undoRedo.clean(); + Main.main.undoRedo.clean(this); // if uploaded, clean the modified flags as well - if (processed != null) { - final Set processedSet = new HashSet(processed); - for (final Iterator it = data.nodes.iterator(); it.hasNext();) { - cleanIterator(it, processedSet); - } - for (final Iterator it = data.ways.iterator(); it.hasNext();) { - cleanIterator(it, processedSet); - } - for (final Iterator it = data.relations.iterator(); it.hasNext();) { - cleanIterator(it, processedSet); - } - } - - // update the modified flag - boolean b = (getAssociatedFile() != null && processed != null); - if (b && !dataAdded) - return; // do nothing when uploading non-harmful changes. - - // modified if server changed the data (esp. the id). - uploadedModified = b && dataAdded; - setModified(uploadedModified); + final Set processedSet = new HashSet(processed); + for (final Iterator it = data.nodes.iterator(); it.hasNext();) { + cleanIterator(it, processedSet); + } + for (final Iterator it = data.ways.iterator(); it.hasNext();) { + cleanIterator(it, processedSet); + } + for (final Iterator it = data.relations.iterator(); it.hasNext();) { + cleanIterator(it, processedSet); + } + + setModified(true); } Index: trunk/src/org/openstreetmap/josm/io/OsmApi.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmApi.java (revision 1893) +++ trunk/src/org/openstreetmap/josm/io/OsmApi.java (revision 1894) @@ -507,3 +507,11 @@ } + /** + * Replies the current changeset + * + * @return the current changeset + */ + public Changeset getCurrentChangeset() { + return changeset; + } } Index: trunk/src/org/openstreetmap/josm/io/OsmServerWriter.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmServerWriter.java (revision 1893) +++ trunk/src/org/openstreetmap/josm/io/OsmServerWriter.java (revision 1894) @@ -5,4 +5,5 @@ import java.util.Collection; +import java.util.Collections; import java.util.LinkedList; import java.util.List; @@ -11,4 +12,5 @@ import org.openstreetmap.josm.Main; import org.openstreetmap.josm.actions.UploadAction; +import org.openstreetmap.josm.data.osm.Changeset; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; @@ -34,5 +36,5 @@ * than where passed in the list to upload*. */ - public Collection processed; + private Collection processed; private OsmApi api = OsmApi.getOsmApi(); @@ -79,4 +81,75 @@ /** + * Uploads the changes individually. Invokes one API call per uploaded primitmive. + * + * @param primitives the collection of primitives to upload + * @param progressMonitor the progress monitor + * @throws OsmTransferException thrown if an exception occurs + */ + protected void uploadChangesIndividually(Collection primitives, ProgressMonitor progressMonitor) throws OsmTransferException { + try { + progressMonitor.setTicksCount(primitives.size()); + api.createChangeset(getChangesetComment(), progressMonitor.createSubTaskMonitor(0, false)); + uploadStartTime = System.currentTimeMillis(); + for (OsmPrimitive osm : primitives) { + int progress = progressMonitor.getTicks(); + String time_left_str = timeLeft(progress, primitives.size()); + progressMonitor.subTask( + tr("{0}% ({1}/{2}), {3} left. Uploading {4}: {5} (id: {6})", + Math.round(100.0*progress/primitives.size()), progress, + primitives.size(), time_left_str, + OsmPrimitiveType.from(osm).getLocalizedDisplayNameSingular(), + NAME_FORMATTER.getName(osm), + osm.id)); + makeApiRequest(osm,progressMonitor); + processed.add(osm); + progressMonitor.worked(1); + } + } catch(OsmTransferException e) { + throw e; + } catch(Exception e) { + throw new OsmTransferException(e); + } finally { + try { + api.stopChangeset(progressMonitor.createSubTaskMonitor(0, false)); + } catch(Exception e) { + Changeset changeset = api.getCurrentChangeset(); + String changesetId = (changeset == null ? tr("unknown") : Long.toString(changeset.id)); + logger.warning(tr("Failed to close changeset {0}, will be closed by server after timeout. Exception was: {1}", + changesetId, e.toString())); + } + } + } + + /** + * Upload all changes in one diff upload + * + * @param primitives the collection of primitives to upload + * @param progressMonitor the progress monitor + * @throws OsmTransferException thrown if an exception occurs + */ + protected void uploadChangesAsDiffUpload(Collection primitives, ProgressMonitor progressMonitor) throws OsmTransferException { + // upload everything in one changeset + // + try { + api.createChangeset(getChangesetComment(), progressMonitor.createSubTaskMonitor(0, false)); + processed.addAll(api.uploadDiff(primitives, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false))); + } catch(OsmTransferException e) { + throw e; + } catch(Exception e) { + throw new OsmTransferException(e); + } finally { + try { + api.stopChangeset(progressMonitor.createSubTaskMonitor(0, false)); + } catch (Exception ee) { + Changeset changeset = api.getCurrentChangeset(); + String changesetId = (changeset == null ? tr("unknown") : Long.toString(changeset.id)); + logger.warning(tr("Failed to close changeset {0}, will be closed by server after timeout. Exception was: {1}", + changesetId, ee.toString())); + } + } + } + + /** * Send the dataset to the server. * @@ -92,5 +165,4 @@ try { - // check whether we can use changeset // @@ -103,44 +175,8 @@ if (useChangeset) { - // upload everything in one changeset - // - try { - api.createChangeset(getChangesetComment(), progressMonitor.createSubTaskMonitor(0, false)); - processed.addAll(api.uploadDiff(primitives, progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false))); - } catch(OsmTransferException e) { - throw e; - } finally { - try { - if (canUseChangeset) { - api.stopChangeset(progressMonitor.createSubTaskMonitor(0, false)); - } - } catch (Exception ee) { - ee.printStackTrace(); - // ignore nested exception - } - } + uploadChangesAsDiffUpload(primitives, progressMonitor); } else { - // upload changes individually (90% of code is for the status display...) - // - progressMonitor.setTicksCount(primitives.size()); - api.createChangeset(getChangesetComment(), progressMonitor.createSubTaskMonitor(0, false)); - uploadStartTime = System.currentTimeMillis(); - for (OsmPrimitive osm : primitives) { - int progress = progressMonitor.getTicks(); - String time_left_str = timeLeft(progress, primitives.size()); - progressMonitor.subTask( - tr("{0}% ({1}/{2}), {3} left. Uploading {4}: {5} (id: {6})", - Math.round(100.0*progress/primitives.size()), progress, - primitives.size(), time_left_str, - OsmPrimitiveType.from(osm).getLocalizedDisplayNameSingular(), - NAME_FORMATTER.getName(osm), - osm.id)); - makeApiRequest(osm,progressMonitor); - processed.add(osm); - progressMonitor.worked(1); - } - api.stopChangeset(progressMonitor.createSubTaskMonitor(0, false)); - } - + uploadChangesIndividually(primitives, progressMonitor); + } } finally { progressMonitor.finishTask(); @@ -163,3 +199,12 @@ } } + + /** + * Replies the collection of successfully processed primitives + * + * @return the collection of successfully processed primitives + */ + public Collection getProcessedPrimitives() { + return processed; + } }