Changeset 34028 in osm for applications/editors

Timestamp:

2018-01-23T21:36:34+01:00 (7 years ago)

Author:

donvip

Message:

fix #josm15821 - better handling of batch reverts

Location:

applications/editors/josm/plugins/reverter/src/reverter

Files:

• RevertChangesetAction.java (modified) (2 diffs)
• RevertChangesetTask.java (modified) (8 diffs)

applications/editors/josm/plugins/reverter/src/reverter/RevertChangesetAction.java

@@ -14 +14 @@
 import reverter.ChangesetReverter.RevertType;
 
-@SuppressWarnings("serial")
 public class RevertChangesetAction extends JosmAction {
 
     public RevertChangesetAction() {
         super(tr("Revert changeset"), "revert-changeset", tr("Revert changeset"),
-            Shortcut.registerShortcut("tool:revert",
-                tr("Tool: {0}", tr("Revert changeset")),
-                KeyEvent.VK_T, Shortcut.CTRL_SHIFT),
+            Shortcut.registerShortcut("tool:revert", tr("Tool: {0}", tr("Revert changeset")), KeyEvent.VK_T, Shortcut.CTRL_SHIFT),
                 true);
     }
 
     @Override
-    public void actionPerformed(ActionEvent arg0) {
+    public void actionPerformed(ActionEvent e) {
         final ChangesetIdQuery dlg = new ChangesetIdQuery();
         if (dlg.showDialog().getValue() != 1) return;
@@ -35 +32 @@
         boolean newLayer = dlg.isNewLayerRequired();
         final boolean autoConfirmDownload = newLayer || changesetIds.size() > 1;
-        for (Integer changesetId : changesetIds) {
-            MainApplication.worker.submit(new RevertChangesetTask(changesetId, revertType, autoConfirmDownload, newLayer));
-            newLayer = false; // reuse layer for subsequent reverts
-        }
+        MainApplication.worker.submit(new RevertChangesetTask(changesetIds, revertType, autoConfirmDownload, newLayer));
     }
 }

applications/editors/josm/plugins/reverter/src/reverter/RevertChangesetTask.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+import java.text.MessageFormat;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Callable;
@@ -11 +15 @@
 import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.command.Command;
+import org.openstreetmap.josm.command.SequenceCommand;
 import org.openstreetmap.josm.command.conflict.ConflictAddCommand;
 import org.openstreetmap.josm.gui.MainApplication;
@@ -19 +24 @@
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.io.OsmTransferException;
+import org.openstreetmap.josm.tools.Logging;
+import org.openstreetmap.josm.tools.UserCancelException;
 
 import reverter.ChangesetReverter.RevertType;
 
 public class RevertChangesetTask extends PleaseWaitRunnable {
-    private final int changesetId;
+    private final Collection<Integer> changesetIds;
     private final RevertType revertType;
-    private final boolean newLayer;
+    private boolean newLayer;
 
     private ChangesetReverter rev;
     private boolean downloadConfirmed;
+    private int numberOfConflicts;
 
     public RevertChangesetTask(int changesetId, RevertType revertType) {
@@ -39 +47 @@
 
     public RevertChangesetTask(int changesetId, RevertType revertType, boolean autoConfirmDownload, boolean newLayer) {
+        this(Collections.singleton(changesetId), revertType, autoConfirmDownload, newLayer);
+    }
+
+    public RevertChangesetTask(Collection<Integer> changesetIds, RevertType revertType, boolean autoConfirmDownload, boolean newLayer) {
         super(tr("Reverting..."));
-        this.changesetId = changesetId;
+        this.changesetIds = new ArrayList<>(changesetIds);
         this.revertType = revertType;
         this.downloadConfirmed = autoConfirmDownload;
@@ -73 +85 @@
     @Override
     protected void realRun() throws OsmTransferException {
+        numberOfConflicts = 0;
+        final List<Command> allcmds = new ArrayList<>();
+        Logging.info("Reverting {0} changeset(s): {1}", changesetIds.size(), changesetIds);
+        for (int changesetId : changesetIds) {
+            try {
+                Logging.info("Reverting changeset {0}", changesetId);
+                allcmds.add(revertChangeset(changesetId));
+                Logging.info("Reverted changeset {0}", changesetId);
+                newLayer = false; // reuse layer for subsequent reverts
+            } catch (OsmTransferException e) {
+                Logging.error(e);
+                throw e;
+            } catch (UserCancelException e) {
+                Logging.warn("Revert canceled");
+                Logging.trace(e);
+                return;
+            }
+        }
+        if (!allcmds.isEmpty()) {
+            Command cmd = allcmds.size() == 1 ? allcmds.get(0) : new SequenceCommand(tr("Revert changeset"), allcmds);
+            GuiHelper.runInEDT(() -> {
+                MainApplication.undoRedo.add(cmd);
+                if (numberOfConflicts > 0) {
+                    MainApplication.getMap().conflictDialog.warnNumNewConflicts(numberOfConflicts);
+                }
+            });
+        }
+    }
+
+    private RevertChangesetCommand revertChangeset(int changesetId) throws OsmTransferException, UserCancelException {
         progressMonitor.indeterminateSubTask(tr("Downloading changeset"));
         try {
-            rev = new ChangesetReverter(changesetId, revertType, newLayer,
-                    progressMonitor.createSubTaskMonitor(0, true));
+            rev = new ChangesetReverter(changesetId, revertType, newLayer, progressMonitor.createSubTaskMonitor(0, true));
         } catch (final RevertRedactedChangesetException e) {
-            GuiHelper.runInEDT(new Runnable() {
-                @Override
-                public void run() {
-                    new Notification(
-                            e.getMessage()+"<br>"+
-                            tr("See {0}", "<a href=\"https://www.openstreetmap.org/redactions\">https://www.openstreetmap.org/redactions</a>"))
-                    .setIcon(JOptionPane.ERROR_MESSAGE)
-                    .setDuration(Notification.TIME_LONG)
-                    .show();
-                }
-            });
+            GuiHelper.runInEDT(() -> new Notification(
+                    e.getMessage()+"<br>"+
+                    tr("See {0}", "<a href=\"https://www.openstreetmap.org/redactions\">https://www.openstreetmap.org/redactions</a>"))
+            .setIcon(JOptionPane.ERROR_MESSAGE)
+            .setDuration(Notification.TIME_LONG)
+            .show());
             progressMonitor.cancel();
         }
-        if (progressMonitor.isCanceled()) return;
+        if (progressMonitor.isCanceled())
+            throw new UserCancelException();
 
         // Check missing objects
@@ -99 +136 @@
             // If missing created or updated objects, ask user
             rev.checkMissingDeleted();
-            if (!checkAndDownloadMissing()) return;
+            if (!checkAndDownloadMissing())
+                throw new UserCancelException();
         } else {
             // Don't ask user to download primitives going to be undeleted
@@ -106 +144 @@
         }
 
-        if (progressMonitor.isCanceled()) return;
+        if (progressMonitor.isCanceled())
+            throw new UserCancelException();
         rev.downloadObjectsHistory(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
-        if (progressMonitor.isCanceled()) return;
-        if (!checkAndDownloadMissing()) return;
+        if (progressMonitor.isCanceled())
+            throw new UserCancelException();
+        if (!checkAndDownloadMissing())
+            throw new UserCancelException();
         rev.fixNodesWithoutCoordinates(progressMonitor);
         List<Command> cmds = rev.getCommands();
-        final Command cmd = new RevertChangesetCommand(tr(revertType == RevertType.FULL ? "Revert changeset #{0}" :
-                "Partially revert changeset #{0}", changesetId), cmds);
-        int n = 0;
+        if (cmds.isEmpty()) {
+            String msg = MessageFormat.format("No revert commands found for changeset {0}", changesetId);
+            Logging.warn(msg);
+            throw new OsmTransferException(msg);
+        }
         for (Command c : cmds) {
             if (c instanceof ConflictAddCommand) {
-                n++;
+                numberOfConflicts++;
             }
         }
-        final int newConflicts = n;
-        GuiHelper.runInEDT(new Runnable() {
-            @Override
-            public void run() {
-                MainApplication.undoRedo.add(cmd);
-                if (newConflicts > 0) {
-                    MainApplication.getMap().conflictDialog.warnNumNewConflicts(newConflicts);
-                }
-            }
-        });
+        return new RevertChangesetCommand(tr(revertType == RevertType.FULL ? "Revert changeset #{0}" :
+                "Partially revert changeset #{0}", changesetId), cmds);
     }
 
@@ -139 +174 @@
     protected void finish() {
     }
+
+    public final int getNumberOfConflicts() {
+        return numberOfConflicts;
+    }
 }