Index: trunk/src/org/openstreetmap/josm/actions/MergeSelectionAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/MergeSelectionAction.java (revision 2197) +++ trunk/src/org/openstreetmap/josm/actions/MergeSelectionAction.java (revision 2198) @@ -51,6 +51,3 @@ setEnabled(!getEditLayer().data.getSelected().isEmpty()); } - - - } Index: trunk/src/org/openstreetmap/josm/actions/UploadAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/UploadAction.java (revision 2197) +++ trunk/src/org/openstreetmap/josm/actions/UploadAction.java (revision 2198) @@ -11,4 +11,5 @@ import java.util.Collection; import java.util.Date; +import java.util.HashSet; import java.util.LinkedList; import java.util.logging.Logger; @@ -29,4 +30,5 @@ import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; +import org.openstreetmap.josm.gui.DefaultNameFormatter; import org.openstreetmap.josm.gui.ExceptionDialogUtil; import org.openstreetmap.josm.gui.PleaseWaitRunnable; @@ -37,6 +39,8 @@ import org.openstreetmap.josm.io.OsmApiException; import org.openstreetmap.josm.io.OsmApiInitializationException; +import org.openstreetmap.josm.io.OsmApiPrimitiveGoneException; import org.openstreetmap.josm.io.OsmChangesetCloseException; import org.openstreetmap.josm.io.OsmServerWriter; +import org.openstreetmap.josm.io.OsmTransferException; import org.openstreetmap.josm.tools.DateUtils; import org.openstreetmap.josm.tools.Shortcut; @@ -350,7 +354,7 @@ * @see UpdateSelectionAction#handlePrimitiveGoneException(long) */ - protected void handleGoneForKnownPrimitive(OsmPrimitiveType primitiveType, String id) { + protected void handleGoneForKnownPrimitive(OsmPrimitiveType primitiveType, long id) { UpdateSelectionAction act = new UpdateSelectionAction(); - act.handlePrimitiveGoneException(Long.parseLong(id),primitiveType); + act.handlePrimitiveGoneException(id,primitiveType); } @@ -363,12 +367,8 @@ * @param e the exception */ - protected void handleGone(OsmApiException e) { - String pattern = "The (\\S+) with the id (\\d+) has already been deleted"; - Pattern p = Pattern.compile(pattern); - Matcher m = p.matcher(e.getErrorHeader()); - if (m.matches()) { - handleGoneForKnownPrimitive(OsmPrimitiveType.from(m.group(1)), m.group(2)); + protected void handleGone(OsmApiPrimitiveGoneException e) { + if (e.isKnownPrimitive()) { + handleGoneForKnownPrimitive(e.getPrimitiveType(), e.getPrimitiveId()); } else { - logger.warning(tr("Error header \"{0}\" does not match expected pattern \"{1}\"",e.getErrorHeader(), pattern)); ExceptionDialogUtil.explainGoneForUnknownPrimitive(e); } @@ -391,4 +391,8 @@ if (e instanceof OsmChangesetCloseException) { ExceptionDialogUtil.explainOsmChangesetCloseException((OsmChangesetCloseException)e); + return; + } + if (e instanceof OsmApiPrimitiveGoneException) { + handleGone((OsmApiPrimitiveGoneException)e); return; } @@ -406,11 +410,4 @@ else if (ex.getResponseCode() == HttpURLConnection.HTTP_PRECON_FAILED) { ExceptionDialogUtil.explainPreconditionFailed(ex); - return; - } - // Tried to delete an already deleted primitive? Let the user - // decide whether and how to resolve this conflict. - // - else if (ex.getResponseCode() == HttpURLConnection.HTTP_GONE) { - handleGone(ex); return; } @@ -489,6 +486,6 @@ } - public UploadDiffTask createUploadTask(OsmDataLayer layer, Collection toUpload, Changeset changeset, boolean closeChangesetAfterUpload) { - return new UploadDiffTask(layer, toUpload, changeset, closeChangesetAfterUpload); + public UploadPrimitivesTask createUploadTask(OsmDataLayer layer, Collection toUpload, Changeset changeset, boolean closeChangesetAfterUpload) { + return new UploadPrimitivesTask(layer, toUpload, changeset, closeChangesetAfterUpload); } @@ -497,5 +494,5 @@ * */ - public class UploadDiffTask extends PleaseWaitRunnable { + public class UploadPrimitivesTask extends PleaseWaitRunnable { private boolean uploadCancelled = false; private Exception lastException = null; @@ -505,4 +502,5 @@ private Changeset changeset; private boolean closeChangesetAfterUpload; + private HashSet processedPrimitives; /** @@ -513,5 +511,5 @@ * @param closeChangesetAfterUpload true, if the changeset is to be closed after uploading */ - private UploadDiffTask(OsmDataLayer layer, Collection toUpload, Changeset changeset, boolean closeChangesetAfterUpload) { + private UploadPrimitivesTask(OsmDataLayer layer, Collection toUpload, Changeset changeset, boolean closeChangesetAfterUpload) { super(tr("Uploading data for layer ''{0}''", layer.getName()),false /* don't ignore exceptions */); this.toUpload = toUpload; @@ -519,4 +517,40 @@ this.changeset = changeset; this.closeChangesetAfterUpload = closeChangesetAfterUpload; + this.processedPrimitives = new HashSet(); + } + + protected OsmPrimitive getPrimitive(OsmPrimitiveType type, long id) { + for (OsmPrimitive p: toUpload) { + if (OsmPrimitiveType.from(p).equals(type) && p.getId() == id) + return p; + } + return null; + } + + /** + * Retries to recover the upload operation from an exception which was thrown because + * an uploaded primitive was already deleted on the server. + * + * @param e the exception throw by the API + * @param monitor a progress monitor + * @throws OsmTransferException thrown if we can't recover from the exception + */ + protected void recoverFromGoneOnServer(OsmApiPrimitiveGoneException e, ProgressMonitor monitor) throws OsmTransferException{ + if (!e.isKnownPrimitive()) throw e; + OsmPrimitive p = getPrimitive(e.getPrimitiveType(), e.getPrimitiveId()); + if (p == null) throw e; + if (p.isDeleted()) { + // we tried to delete an already deleted primitive. + // + System.out.println(tr("Warning: primitive ''{0}'' is already deleted on the server. Skipping this primitive and retrying to upload.", p.getDisplayName(DefaultNameFormatter.getInstance()))); + processedPrimitives.addAll(writer.getProcessedPrimitives()); + processedPrimitives.add(p); + toUpload.removeAll(processedPrimitives); + return; + } + // exception was thrown because we tried to *update* an already deleted + // primitive. We can't resolve this automatically. Re-throw exception, + // a conflict is going to be created later. + throw e; } @@ -524,12 +558,32 @@ writer = new OsmServerWriter(); try { - ProgressMonitor monitor = progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false); - writer.uploadOsm(layer.data.version, toUpload, changeset,closeChangesetAfterUpload, monitor); - } catch (Exception sxe) { + // + while(true) { + try { + ProgressMonitor monitor = progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false); + writer.uploadOsm(layer.data.version, toUpload, changeset, monitor); + processedPrimitives.addAll(writer.getProcessedPrimitives()); + // if we get here we've successfully uploaded the data. We + // can exit the loop. + // + break; + } catch(OsmApiPrimitiveGoneException e) { + // try to recover from the 410 Gone + recoverFromGoneOnServer(e, getProgressMonitor()); + } + } + // if required close the changeset + // + if (closeChangesetAfterUpload) { + if (changeset != null && changeset.getId() > 0) { + OsmApi.getOsmApi().closeChangeset(changeset, progressMonitor.createSubTaskMonitor(0,false)); + } + } + } catch (Exception e) { if (uploadCancelled) { - System.out.println("Ignoring exception caught because upload is cancelled. Exception is: " + sxe.toString()); + System.out.println("Ignoring exception caught because upload is cancelled. Exception is: " + e.toString()); return; } - lastException = sxe; + lastException = e; } } @@ -539,20 +593,15 @@ return; - // we always clean the data, even in case of errors. It's possible the data was + // we always clean up the data, even in case of errors. It's possible the data was // partially uploaded // - layer.cleanupAfterUpload(writer.getProcessedPrimitives()); + layer.cleanupAfterUpload(processedPrimitives); DataSet.fireSelectionChanged(layer.data.getSelected()); layer.fireDataChange(); if (lastException != null) { handleFailedUpload(lastException); - } else { - // run post upload action on the layer - // - layer.onPostUploadToServer(); - // refresh changeset dialog with the updated changeset - // - UploadDialog.getUploadDialog().setOrUpdateChangeset(changeset); - } + } + layer.onPostUploadToServer(); + UploadDialog.getUploadDialog().setOrUpdateChangeset(changeset); } @@ -563,16 +612,4 @@ } } - - public boolean isSuccessful() { - return !isCancelled() && !isFailed(); - } - - public boolean isCancelled() { - return uploadCancelled; - } - - public boolean isFailed() { - return lastException != null; - } } } Index: trunk/src/org/openstreetmap/josm/actions/upload/RelationUploadOrderHook.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/upload/RelationUploadOrderHook.java (revision 2197) +++ trunk/src/org/openstreetmap/josm/actions/upload/RelationUploadOrderHook.java (revision 2198) @@ -33,5 +33,5 @@ public class RelationUploadOrderHook implements UploadHook { - /** the data to be analysed */ + /** the data to be analyzed */ private APIDataSet data; Index: trunk/src/org/openstreetmap/josm/command/ConflictResolveCommand.java =================================================================== --- trunk/src/org/openstreetmap/josm/command/ConflictResolveCommand.java (revision 2197) +++ trunk/src/org/openstreetmap/josm/command/ConflictResolveCommand.java (revision 2198) @@ -12,9 +12,9 @@ /** - * This is the common basse class for {@see Command}s which manipulate {@see Conflict}s in + * This is the common base class for {@see Command}s which manipulate {@see Conflict}s in * addition to {@see OsmPrimitive}s. * * A ConflictResolverCommand can remember a collection of conflicts it resolves. Upon undoing - * it reconstitutes these conflicts. + * it reconstitutes them. * */ @@ -30,4 +30,9 @@ } + public ConflictResolveCommand(OsmDataLayer layer) { + super(layer); + resolvedConflicts = new ConflictCollection(); + } + /** * remembers a conflict in the internal list of remembered conflicts @@ -35,5 +40,5 @@ * @param c the remembered conflict */ - protected void rememberConflict(Conflict c) { + protected void rememberConflict(Conflict c) { if (! resolvedConflicts.hasConflictForMy(c.getMy())) { resolvedConflicts.add(c); @@ -48,5 +53,5 @@ protected void reconstituteConflicts() { OsmDataLayer editLayer = getLayer(); - for(Conflict c : resolvedConflicts) { + for(Conflict c : resolvedConflicts) { if (!editLayer.getConflicts().hasConflictForMy(c.getMy())) { editLayer.getConflicts().add(c); @@ -60,5 +65,5 @@ if (! Main.map.mapView.hasLayer(getLayer())) { - logger.warning(tr("Can't undo command ''{0}'' because layer ''{1}'' is not present any more", + logger.warning(tr("Can''t undo command ''{0}'' because layer ''{1}'' is not present any more", this.toString(), getLayer().toString() @@ -70,6 +75,3 @@ reconstituteConflicts(); } - - - } Index: trunk/src/org/openstreetmap/josm/command/PurgePrimitivesCommand.java =================================================================== --- trunk/src/org/openstreetmap/josm/command/PurgePrimitivesCommand.java (revision 2197) +++ trunk/src/org/openstreetmap/josm/command/PurgePrimitivesCommand.java (revision 2198) @@ -4,7 +4,9 @@ import static org.openstreetmap.josm.tools.I18n.tr; -import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.logging.Logger; @@ -15,10 +17,11 @@ import org.openstreetmap.josm.Main; import org.openstreetmap.josm.data.conflict.ConflictCollection; -import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.BackreferencedDataSet; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.Relation; -import org.openstreetmap.josm.data.osm.RelationMember; import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.gui.DefaultNameFormatter; +import org.openstreetmap.josm.gui.layer.OsmDataLayer; import org.openstreetmap.josm.tools.ImageProvider; @@ -38,127 +41,69 @@ static private final Logger logger = Logger.getLogger(PurgePrimitivesCommand.class.getName()); - /** - * Represents a pair of {@see OsmPrimitive} where the parent referrs to - * the child, either because a {@see Way} includes a {@see Node} or - * because a {@see Relation} refers to any other {@see OsmPrimitive} - * via a relation member. - * - */ - static class OsmParentChildPair { - private OsmPrimitive parent; - private OsmPrimitive child; - - - public OsmParentChildPair(OsmPrimitive parent, OsmPrimitive child) { - this.parent = parent; - this.child = child; - } - - public OsmPrimitive getParent() { - return parent; - } - - public OsmPrimitive getChild() { - return child; - } - - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((child == null) ? 0 : child.hashCode()); - result = prime * result + ((parent == null) ? 0 : parent.hashCode()); - return result; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - OsmParentChildPair other = (OsmParentChildPair) obj; - if (child == null) { - if (other.child != null) - return false; - } else if (child != other.child) - return false; - if (parent == null) { - if (other.parent != null) - return false; - } else if (parent != other.parent) - return false; - return true; - } - } - - /** - * creates a list of all {@see OsmParentChildPair}s for a given {@see OsmPrimitive} - * as child and given set of parents. We don't use {@see CollectBackReferencesVisitor} - * because it seems quite inefficient. - * - * @param parents the set of potential parents - * @param child the child - * @return the list of {@see OsmParentChildPair} - */ - protected List getParentChildPairs(List parents, OsmPrimitive child) { - ArrayList pairs = new ArrayList(); - for (OsmPrimitive parent : parents) { - if (parent instanceof Way) { - Way w = (Way)parent; - for (OsmPrimitive node : w.getNodes()) { - if (node == child) { - OsmParentChildPair pair = new OsmParentChildPair(parent, node); - if (! pairs.contains(pair)) { - pairs.add(pair); - } - } - } - } else if (parent instanceof Relation) { - Relation r = (Relation)parent; - for (RelationMember member : r.getMembers()) { - if (member.getMember() == child) { - OsmParentChildPair pair = new OsmParentChildPair(parent, member.getMember()); - if (! pairs.contains(pair)) { - pairs.add(pair); - } - } - } - } - } - return pairs; - } - - /** the primitive to purge */ - private OsmPrimitive primitive; + /** the primitives to purge */ + private Collection toPurge; /** the set of primitives to purge as consequence of purging * {@see #primitive}, including {@see #primitive} */ - private ArrayList purgedPrimitives; - - /** the set of {@see OsmParentChildPair}. We keep a reference - * to this set for the {@see #fillModifiedData(Collection, Collection, Collection)} operation - */ - private ArrayList pairs; - + private Set purgedPrimitives; + + private Set origVersionsOfTouchedPrimitives; + + /** + * the data structure with child->parent references + */ + private BackreferencedDataSet backreferenceDataSet; + + protected void init(Collection toPurge) { + this.toPurge = toPurge; + this.purgedPrimitives = new HashSet(); + this.origVersionsOfTouchedPrimitives = new HashSet(); + } /** * constructor - * @param node the node to undelete + * @param primitive the primitive to purge + * */ public PurgePrimitivesCommand(OsmPrimitive primitive) { - this.primitive = primitive; - purgedPrimitives = new ArrayList(); - pairs = new ArrayList(); - } - - @Override - public MutableTreeNode description() { + init(Collections.singleton(primitive)); + } + + /** + * constructor + * @param layer the OSM data layer + * @param primitive the primitive to purge + * + */ + public PurgePrimitivesCommand(OsmDataLayer layer, OsmPrimitive primitive) { + super(layer); + init(Collections.singleton(primitive)); + } + + /** + * constructor + * @param layer the OSM data layer + * @param primitives the primitives to purge + * + */ + public PurgePrimitivesCommand(OsmDataLayer layer, Collection primitives) { + super(layer); + init(primitives); + } + + /** + * Replies a collection with the purged primitives + * + * @return a collection with the purged primitives + */ + public Collection getPurgedPrimitives() { + return purgedPrimitives; + } + + protected MutableTreeNode getDescription(OsmPrimitive primitive) { return new DefaultMutableTreeNode( new JLabel( - tr("Purging 1 primitive"), + tr("Purged object ''{0}''", primitive.getDisplayName(DefaultNameFormatter.getInstance())), ImageProvider.get("data", "object"), JLabel.HORIZONTAL @@ -167,27 +112,47 @@ } - /** - * Purges an {@see OsmPrimitive} toPurge from a {@see DataSet}. + protected MutableTreeNode getDescription(Collection primitives) { + + DefaultMutableTreeNode root = new DefaultMutableTreeNode( + tr("Purged {0} objects", primitives.size()) + ); + for (OsmPrimitive p : primitives) { + root.add(getDescription(p)); + } + return root; + } + + @Override + public MutableTreeNode description() { + if (purgedPrimitives.size() == 1) + return getDescription(purgedPrimitives.iterator().next()); + else + return getDescription(purgedPrimitives); + } + + /** + * Purges an {@see OsmPrimitive} child from a {@see DataSet}. * - * @param toPurge the primitive to purge - * @param ds the dataset to purge from + * @param child the primitive to purge * @param hive the hive of {@see OsmPrimitive}s we remember other {@see OsmPrimitive} - * we have to purge because we purge toPurge. + * we have to purge because we purge child. * */ - protected void purge(OsmPrimitive toPurge, DataSet ds, ArrayList hive) { - ArrayList parents = new ArrayList(); - parents.addAll(getLayer().data.ways); - parents.addAll(getLayer().data.relations); - List pairs = getParentChildPairs(parents, primitive); - hive.remove(toPurge); - for (OsmParentChildPair pair: pairs) { - if (pair.getParent() instanceof Way) { - Way w = (Way)pair.getParent(); - System.out.println(tr("removing reference from way {0}",w.getId())); + protected void removeReferecesToPrimitive(OsmPrimitive child, Set hive) { + hive.remove(child); + for (OsmPrimitive parent: this.backreferenceDataSet.getParents(child)) { + if (toPurge.contains(parent)) + // parent itself is to be purged. This method is going to be + // invoked for parent later + return; + if (parent instanceof Way) { + Way w = (Way)parent; + if (!origVersionsOfTouchedPrimitives.contains(w)) { + origVersionsOfTouchedPrimitives.add(w); + } List wayNodes = w.getNodes(); - wayNodes.remove(primitive); + wayNodes.remove(child); w.setNodes(wayNodes); - // if a way ends up with less than two node we + // if a way ends up with less than two nodes we // remember it on the "hive" // @@ -195,12 +160,15 @@ System.out.println(tr("Warning: Purging way {0} because number of nodes dropped below 2. Current is {1}", w.getId(),w.getNodesCount())); - if (!hive.contains(w)) { - hive.add(w); - } + hive.add(w); } - } else if (pair.getParent() instanceof Relation) { - Relation r = (Relation)pair.getParent(); - System.out.println(tr("removing reference from relation {0}",r.getId())); - r.removeMembersFor(primitive); + } else if (parent instanceof Relation) { + Relation r = (Relation)parent; + if (!origVersionsOfTouchedPrimitives.contains(r)) { + origVersionsOfTouchedPrimitives.add(r); + } + System.out.println(tr("Removing reference from relation {0}",r.getId())); + r.removeMembersFor(child); + } else { + // should not happen. parent can't be a node } } @@ -209,28 +177,28 @@ @Override public boolean executeCommand() { - ArrayList hive = new ArrayList(); + if (backreferenceDataSet == null) { + backreferenceDataSet = new BackreferencedDataSet(getLayer().data); + backreferenceDataSet.build(); + } + HashSet hive = new HashSet(); // iteratively purge the primitive and all primitives - // which violate invariants after they loose a reference to + // which violate invariants after they lose a reference to // the primitive (i.e. ways which end up with less than two // nodes) - hive.add(primitive); + hive.addAll(toPurge); while(! hive.isEmpty()) { - OsmPrimitive toPurge = hive.get(0); - purge(toPurge, getLayer().data, hive); - if (toPurge instanceof Node) { - getLayer().data.nodes.remove(toPurge); - } else if (primitive instanceof Way) { - getLayer().data.ways.remove(toPurge); - } else if (primitive instanceof Relation) { - getLayer().data.relations.remove(toPurge); + OsmPrimitive p = hive.iterator().next(); + removeReferecesToPrimitive(p, hive); + getLayer().data.removePrimitive(p); + purgedPrimitives.add(p); + ConflictCollection conflicts = getLayer().getConflicts(); + if (conflicts.hasConflictForMy(p)) { + rememberConflict(conflicts.getConflictForMy(p)); + conflicts.remove(p); } - purgedPrimitives.add(toPurge); - ConflictCollection conflicts = getLayer().getConflicts(); - if (conflicts.hasConflictForMy(toPurge)) { - rememberConflict(conflicts.getConflictForMy(toPurge)); - conflicts.remove(toPurge); - } - } + } + // we don't need this any more + backreferenceDataSet = null; return super.executeCommand(); } @@ -239,9 +207,5 @@ public void fillModifiedData(Collection modified, Collection deleted, Collection added) { - for (OsmParentChildPair pair : pairs) { - modified.add(pair.getParent()); - } - // we don't need pairs anymore - pairs = null; + modified.addAll(origVersionsOfTouchedPrimitives); } @@ -249,5 +213,5 @@ public void undoCommand() { if (! Main.map.mapView.hasLayer(getLayer())) { - logger.warning(tr("Can't undo command ''{0}'' because layer ''{1}'' is not present any more", + logger.warning(tr("Can''t undo command ''{0}'' because layer ''{1}'' is not present any more", this.toString(), getLayer().toString() @@ -263,7 +227,18 @@ } reconstituteConflicts(); - // will restore the former references to the purged nodes - // + + // will restore the primitives referring to one + // of the purged primitives super.undoCommand(); } + + /** + * Use to inject a backreference data set used when the command + * is executed. + * + * @param ds the backreference data set + */ + public void setBackreferenceDataSet(BackreferencedDataSet ds) { + this.backreferenceDataSet = ds; + } } Index: trunk/src/org/openstreetmap/josm/data/conflict/Conflict.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/conflict/Conflict.java (revision 2197) +++ trunk/src/org/openstreetmap/josm/data/conflict/Conflict.java (revision 2198) @@ -6,5 +6,5 @@ /** * Represents a conflict between two {@see OsmPrimitive}s. It is represented as - * a pair if {@see OsmPrimitive} where one element of the pair has the role my + * a pair of {@see OsmPrimitive}s where one element of the pair has the role my * and the other has the role their. *
    Index: trunk/src/org/openstreetmap/josm/gui/io/UploadLayerTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/io/UploadLayerTask.java (revision 2197) +++ trunk/src/org/openstreetmap/josm/gui/io/UploadLayerTask.java (revision 2198) @@ -5,14 +5,20 @@ import java.util.Collection; +import java.util.HashSet; +import org.openstreetmap.josm.actions.upload.CyclicUploadDependencyException; import org.openstreetmap.josm.data.APIDataSet; import org.openstreetmap.josm.data.osm.Changeset; import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.OsmPrimitiveType; +import org.openstreetmap.josm.gui.DefaultNameFormatter; import org.openstreetmap.josm.gui.layer.OsmDataLayer; import org.openstreetmap.josm.gui.progress.NullProgressMonitor; import org.openstreetmap.josm.gui.progress.ProgressMonitor; -import org.openstreetmap.josm.io.ChangesetProcessingType; +import org.openstreetmap.josm.io.OsmApi; +import org.openstreetmap.josm.io.OsmApiPrimitiveGoneException; import org.openstreetmap.josm.io.OsmServerWriter; +import org.openstreetmap.josm.io.OsmTransferException; /** @@ -37,4 +43,6 @@ private Changeset changeset; private boolean closeChangesetAfterUpload; + private Collection toUpload; + private HashSet processedPrimitives; /** @@ -42,6 +50,5 @@ * @param layer the layer. Must not be null. * @param monitor a progress monitor. If monitor is null, uses {@see NullProgressMonitor#INSTANCE} - * @param changeset the changeset to be used if changesetProcessingType indicates that a new - * changeset is to be used + * @param changeset the changeset to be used * @param closeChangesetAfterUpload true, if the changeset should be closed after the upload * @throws IllegalArgumentException thrown, if layer is null @@ -57,4 +64,40 @@ this.changeset = changeset; this.closeChangesetAfterUpload = closeChangesetAfterUpload; + processedPrimitives = new HashSet(); + } + + protected OsmPrimitive getPrimitive(OsmPrimitiveType type, long id) { + for (OsmPrimitive p: toUpload) { + if (OsmPrimitiveType.from(p).equals(type) && p.getId() == id) + return p; + } + return null; + } + + /** + * Retries to recover the upload operation from an exception which was thrown because + * an uploaded primitive was already deleted on the server. + * + * @param e the exception throw by the API + * @param monitor a progress monitor + * @throws OsmTransferException thrown if we can't recover from the exception + */ + protected void recoverFromGoneOnServer(OsmApiPrimitiveGoneException e, ProgressMonitor monitor) throws OsmTransferException{ + if (!e.isKnownPrimitive()) throw e; + OsmPrimitive p = getPrimitive(e.getPrimitiveType(), e.getPrimitiveId()); + if (p == null) throw e; + if (p.isDeleted()) { + // we tried to delete an already deleted primitive. + // + System.out.println(tr("Warning: primitive ''{0}'' is already deleted on the server. Skipping this primitive and retrying to upload.", p.getDisplayName(DefaultNameFormatter.getInstance()))); + processedPrimitives.addAll(writer.getProcessedPrimitives()); + processedPrimitives.add(p); + toUpload.removeAll(processedPrimitives); + return; + } + // exception was thrown because we tried to *update* an already deleted + // primitive. We can't resolve this automatically. Re-throw exception, + // a conflict is going to be created later. + throw e; } @@ -62,12 +105,32 @@ public void run() { monitor.subTask(tr("Preparing primitives to upload ...")); - Collection toUpload = new APIDataSet(layer.data).getPrimitives(); + APIDataSet ds = new APIDataSet(layer.data); + try { + ds.adjustRelationUploadOrder(); + } catch(CyclicUploadDependencyException e) { + setLastException(e); + return; + } + toUpload = ds.getPrimitives(); if (toUpload.isEmpty()) return; writer = new OsmServerWriter(); try { - ProgressMonitor m = monitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false); - if (isCancelled()) return; - writer.uploadOsm(layer.data.version, toUpload, changeset, closeChangesetAfterUpload, m); + while(true) { + try { + ProgressMonitor m = monitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false); + if (isCancelled()) return; + writer.uploadOsm(layer.data.version, toUpload, changeset, m); + processedPrimitives.addAll(writer.getProcessedPrimitives()); + break; + } catch(OsmApiPrimitiveGoneException e) { + recoverFromGoneOnServer(e, monitor); + } + } + if (closeChangesetAfterUpload) { + if (changeset != null && changeset.getId() > 0) { + OsmApi.getOsmApi().closeChangeset(changeset, monitor.createSubTaskMonitor(0, false)); + } + } } catch (Exception sxe) { if (isCancelled()) { @@ -80,14 +143,15 @@ if (isCancelled()) return; - layer.cleanupAfterUpload(writer.getProcessedPrimitives()); + layer.cleanupAfterUpload(processedPrimitives); DataSet.fireSelectionChanged(layer.data.getSelected()); layer.fireDataChange(); layer.onPostUploadToServer(); + + // don't process exceptions remembered with setLastException(). + // Caller is supposed to deal with them. } @Override public void cancel() { - // make sure the the softCancel operation is serialized with - // blocks which can be interrupted. setCancelled(true); if (writer != null) { Index: trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (revision 2197) +++ trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (revision 2198) @@ -38,4 +38,5 @@ import org.openstreetmap.josm.Main; import org.openstreetmap.josm.actions.RenameLayerAction; +import org.openstreetmap.josm.command.PurgePrimitivesCommand; import org.openstreetmap.josm.data.conflict.Conflict; import org.openstreetmap.josm.data.conflict.ConflictCollection; @@ -45,4 +46,5 @@ import org.openstreetmap.josm.data.gpx.GpxTrack; import org.openstreetmap.josm.data.gpx.WayPoint; +import org.openstreetmap.josm.data.osm.BackreferencedDataSet; import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.DataSource; @@ -285,29 +287,110 @@ if (data.version == null) { data.version = from.version; - } else { - if ("0.5".equals(data.version) ^ "0.5".equals(from.version)) { - System.err.println(tr("Warning: mixing 0.6 and 0.5 data results in version 0.5")); - data.version = "0.5"; - } + } else if ("0.5".equals(data.version) ^ "0.5".equals(from.version)) { + System.err.println(tr("Warning: mixing 0.6 and 0.5 data results in version 0.5")); + data.version = "0.5"; + } + + int numNewConflicts = 0; + for (Conflict c : visitor.getConflicts()) { + if (!conflicts.hasConflict(c)) { + numNewConflicts++; + conflicts.add(c); + } + } + PurgePrimitivesCommand cmd = buildPurgeCommand(); + if (cmd != null) { + Main.main.undoRedo.add(cmd); } fireDataChange(); // repaint to make sure new data is displayed properly. Main.map.mapView.repaint(); - - int numNewConflicts = 0; - for (Conflict c : visitor.getConflicts()) { - if (!conflicts.hasConflict(c)) { - numNewConflicts++; - conflicts.add(c); - } - } + warnNumNewConflicts( + numNewConflicts, + cmd == null ? 0 : cmd.getPurgedPrimitives().size() + ); + } + + /** + * Warns the user about the number of detected conflicts + * + * @param numNewConflicts the number of detected conflicts + * @param numPurgedPrimitives the number of automatically purged objects + */ + protected void warnNumNewConflicts(int numNewConflicts, int numPurgedPrimitives) { + if (numNewConflicts == 0 && numPurgedPrimitives == 0) return; + + String msg1 = trn( + "There was {0} conflict detected.", + "There were {0} conflicts detected.", + numNewConflicts, + numNewConflicts + ); + 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.", + numPurgedPrimitives, + numPurgedPrimitives + ); + StringBuffer sb = new StringBuffer(); + sb.append("").append(msg1); + if (numPurgedPrimitives > 0) { + sb.append("
    ").append(msg2); + } + sb.append(""); if (numNewConflicts > 0) { JOptionPane.showMessageDialog( Main.parent, - tr("There were {0} conflicts during import.", numNewConflicts), - tr("Warning"), + sb.toString(), + tr("Conflicts detected"), JOptionPane.WARNING_MESSAGE ); } + } + + /** + * Builds the purge command for primitives which can be purged automatically + * from the local dataset because they've been deleted on the + * server. + * + * @return the purge command. null if no primitives have to + * be purged + */ + protected PurgePrimitivesCommand buildPurgeCommand() { + BackreferencedDataSet ds = new BackreferencedDataSet(data); + ds.build(); + ArrayList toPurge = new ArrayList(); + conflictLoop: for (Conflict c: conflicts) { + if (c.getMy().isDeleted() && !c.getTheir().isVisible()) { + // Local and server version of the primitive are deleted. We + // can purge it from the local dataset. + // + toPurge.add(c.getMy()); + } else if (!c.getMy().isModified() && ! c.getTheir().isVisible()) { + // We purge deleted *ways* and *relations* automatically if they are + // deleted on the server and if they aren't modified in the local + // dataset. + // + if (c.getMy() instanceof Way || c.getMy() instanceof Relation) { + toPurge.add(c.getMy()); + continue conflictLoop; + } + // We only purge nodes if they aren't part of a modified way. + // Otherwise the number of nodes of a modified way could drop + // below 2 and we would lose the modified data when the way + // gets purged. + // + for (OsmPrimitive parent: ds.getParents(c.getMy())) { + if (parent.isModified() && parent instanceof Way) { + continue conflictLoop; + } + } + toPurge.add(c.getMy()); + } + } + if (toPurge.isEmpty()) return null; + PurgePrimitivesCommand cmd = new PurgePrimitivesCommand(this, toPurge); + cmd.setBackreferenceDataSet(ds); + return cmd; } @@ -580,5 +663,5 @@ */ public void onPostUploadToServer() { - setRequiresUploadToServer(false); + setRequiresUploadToServer(data.isModified()); // keep requiresSaveToDisk unchanged } Index: trunk/src/org/openstreetmap/josm/io/OsmApi.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmApi.java (revision 2197) +++ trunk/src/org/openstreetmap/josm/io/OsmApi.java (revision 2198) @@ -398,29 +398,30 @@ * * @param list the list of changed OSM Primitives + * @param monitor the progress monitor * @return list of processed primitives * @throws OsmTransferException if something is wrong */ - public Collection uploadDiff(Collection list, ProgressMonitor progressMonitor) throws OsmTransferException { + public Collection uploadDiff(Collection list, ProgressMonitor monitor) throws OsmTransferException { try { - progressMonitor.beginTask("", list.size() * 2); + monitor.beginTask("", list.size() * 2); if (changeset == null) throw new OsmTransferException(tr("No changeset present for diff upload.")); - initialize(progressMonitor); + initialize(monitor); final ArrayList processed = new ArrayList(); CreateOsmChangeVisitor duv = new CreateOsmChangeVisitor(changeset, OsmApi.this); - progressMonitor.subTask(tr("Preparing...")); + monitor.subTask(tr("Preparing...")); for (OsmPrimitive osm : list) { osm.visit(duv); - progressMonitor.worked(1); + monitor.worked(1); } - progressMonitor.indeterminateSubTask(tr("Uploading...")); + monitor.indeterminateSubTask(tr("Uploading...")); String diff = duv.getDocument(); - String diffresult = sendRequest("POST", "changeset/" + changeset.getId() + "/upload", diff,progressMonitor); + String diffresult = sendRequest("POST", "changeset/" + changeset.getId() + "/upload", diff,monitor); DiffResultReader.parseDiffResult(diffresult, list, processed, duv.getNewIdMap(), - progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); + monitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); return processed; } catch(OsmTransferException e) { @@ -429,9 +430,7 @@ throw new OsmTransferException(e); } finally { - progressMonitor.finishTask(); - } - } - - + monitor.finishTask(); + } + } private void sleepAndListen(int retry, ProgressMonitor monitor) throws OsmTransferCancelledException { @@ -548,11 +547,15 @@ activeConnection.disconnect(); - if (retCode != 200) - throw new OsmApiException( - retCode, - errorHeader == null? null : errorHeader.trim(), - responseBody == null ? null : responseBody.toString().trim() - ); - + errorHeader = errorHeader == null? null : errorHeader.trim(); + String errorBody = responseBody == null ? null : responseBody.toString().trim(); + switch(retCode) { + case HttpURLConnection.HTTP_OK: + break; // do nothing + case HttpURLConnection.HTTP_GONE: + throw new OsmApiPrimitiveGoneException(errorHeader, errorBody); + default: + throw new OsmApiException(retCode, errorHeader, errorBody); + + } return responseBody.toString(); } catch (UnknownHostException e) { Index: trunk/src/org/openstreetmap/josm/io/OsmApiPrimitiveGoneException.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmApiPrimitiveGoneException.java (revision 2198) +++ trunk/src/org/openstreetmap/josm/io/OsmApiPrimitiveGoneException.java (revision 2198) @@ -0,0 +1,65 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.io; + +import java.net.HttpURLConnection; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.openstreetmap.josm.data.osm.OsmPrimitiveType; + +/** + * Represents an exception thrown by the OSM API if JOSM tries to update or delete a primitive + * which is already deleted on the server. + * + */ +public class OsmApiPrimitiveGoneException extends OsmApiException{ + /** + * The regexp pattern for the error header replied by the OSM API + */ + static public final String ERROR_HEADER_PATTERN = "The (\\S+) with the id (\\d+) has already been deleted"; + /** the type of the primitive which is gone on the server */ + private OsmPrimitiveType type; + /** the id of the primitive */ + private long id; + + + public OsmApiPrimitiveGoneException(String errorHeader, String errorBody) { + super(HttpURLConnection.HTTP_GONE, errorHeader, errorBody); + if (errorHeader == null) return; + Pattern p = Pattern.compile(ERROR_HEADER_PATTERN); + Matcher m = p.matcher(errorHeader); + if (m.matches()) { + type = OsmPrimitiveType.from(m.group(1)); + id = Long.parseLong(m.group(2)); + } + } + + /** + * Replies true if we know what primitive this exception was thrown for + * + * @return true if we know what primitive this exception was thrown for + */ + public boolean isKnownPrimitive() { + return id > 0 && type != null; + } + + /** + * Replies the type of the primitive this exception was thrown for. null, + * if the type is not known. + * + * @return the type of the primitive this exception was thrown for + */ + public OsmPrimitiveType getPrimitiveType() { + return type; + } + + /** + * Replies the id of the primitive this exception was thrown for. 0, if + * the id is not known. + * + * @return the id of the primitive this exception was thrown for + */ + public long getPrimitiveId() { + return id; + } +} Index: trunk/src/org/openstreetmap/josm/io/OsmServerWriter.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmServerWriter.java (revision 2197) +++ trunk/src/org/openstreetmap/josm/io/OsmServerWriter.java (revision 2198) @@ -13,4 +13,5 @@ import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; +import org.openstreetmap.josm.gui.progress.NullProgressMonitor; import org.openstreetmap.josm.gui.progress.ProgressMonitor; @@ -111,6 +112,4 @@ */ protected void uploadChangesAsDiffUpload(Collection primitives, ProgressMonitor progressMonitor) throws OsmTransferException { - // upload everything in one changeset - // try { progressMonitor.beginTask(tr("Starting to upload in one request ...")); @@ -130,13 +129,20 @@ * @param apiVersion version of the data set * @param primitives list of objects to send - */ - public void uploadOsm(String apiVersion, Collection primitives, Changeset changeset, boolean closeChangesetAfterUpload, ProgressMonitor progressMonitor) throws OsmTransferException { + * @param changeset the changeset the data is uploaded to. Must not be null. + * @param monitor the progress monitor. If null, assumes {@see NullProgressMonitor#INSTANCE} + * @throws IllegalArgumentException thrown if changeset is null + * @throws OsmTransferException thrown if something goes wrong + */ + public void uploadOsm(String apiVersion, Collection primitives, Changeset changeset, ProgressMonitor monitor) throws OsmTransferException { + if (changeset == null) + throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "changeset")); processed = new LinkedList(); - progressMonitor.beginTask(tr("Uploading data ...")); - api.initialize(progressMonitor); + monitor = monitor == null ? NullProgressMonitor.INSTANCE : monitor; + monitor.beginTask(tr("Uploading data ...")); try { + api.initialize(monitor); // check whether we can use diff upload // - boolean casUseDiffUploads = api.hasSupportForDiffUploads(); + boolean canUseDiffUpload = api.hasSupportForDiffUploads(); if (apiVersion == null) { System.out.println(tr("WARNING: no API version defined for data to upload. Falling back to version 0.6")); @@ -144,21 +150,18 @@ } boolean useDiffUpload = Main.pref.getBoolean("osm-server.atomic-upload", apiVersion.compareTo("0.6")>=0); - if (useDiffUpload && ! casUseDiffUploads) { + if (useDiffUpload && ! canUseDiffUpload) { System.out.println(tr("WARNING: preference ''{0}'' or API version ''{1}'' of dataset requires to use diff uploads, but API is not able to handle them. Ignoring diff upload.", "osm-server.atomic-upload", apiVersion)); useDiffUpload = false; } - if (changeset == null) { - changeset = new Changeset(); - } if (changeset.getId() == 0) { - api.openChangeset(changeset,progressMonitor.createSubTaskMonitor(0, false)); + api.openChangeset(changeset,monitor.createSubTaskMonitor(0, false)); } else { - api.updateChangeset(changeset,progressMonitor.createSubTaskMonitor(0, false)); + api.updateChangeset(changeset,monitor.createSubTaskMonitor(0, false)); } api.setChangeset(changeset); if (useDiffUpload) { - uploadChangesAsDiffUpload(primitives,progressMonitor.createSubTaskMonitor(0,false)); + uploadChangesAsDiffUpload(primitives,monitor.createSubTaskMonitor(0,false)); } else { - uploadChangesIndividually(primitives,progressMonitor.createSubTaskMonitor(0,false)); + uploadChangesIndividually(primitives,monitor.createSubTaskMonitor(0,false)); } } catch(OsmTransferException e) { @@ -167,16 +170,6 @@ throw new OsmTransferException(e); } finally { - try { - if (closeChangesetAfterUpload && api.getChangeset() != null && api.getChangeset().getId() > 0) { - api.closeChangeset(changeset,progressMonitor.createSubTaskMonitor(0, false)); - api.setChangeset(null); - } - } catch (Exception ee) { - OsmChangesetCloseException closeException = new OsmChangesetCloseException(ee); - closeException.setChangeset(api.getChangeset()); - throw closeException; - } finally { - progressMonitor.finishTask(); - } + monitor.finishTask(); + api.setChangeset(null); } }