Changeset 36318 in osm

Timestamp:

2024-08-21T22:32:24+02:00 (5 months ago)

Author:

taylor.smock

Message:

Clean up some lint issues in reverter

Location:

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

Files:

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

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

@@ -14 +14 @@
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -56 +57 @@
 public class ChangesetReverter {
 
+    /** The type of reversion */
     public enum RevertType {
         FULL,
@@ -67 +69 @@
             ));
 
+    /** The id of the changeset */
     public final int changesetId;
+    /** The changeset to be reverted */
     public final Changeset changeset;
+    /** The reversion type */
     public final RevertType revertType;
 
-    private final OsmDataLayer layer; // data layer associated with reverter
-    private final DataSet ds; // DataSet associated with reverter
-    private final ChangesetDataSet cds; // Current changeset data
+    /** data layer associated with reverter */
+    private final OsmDataLayer layer;
+    /** DataSet associated with reverter */
+    private final DataSet ds;
+    /** Current changeset data */
+    private final ChangesetDataSet cds;
     /** original DataSet, used if a new layer is requested */
     private final DataSet ods;
-    private DataSet nds; // Dataset that contains new objects downloaded by reverter
+    /** Dataset that contains new objects downloaded by reverter */
+    private DataSet nds;
 
     private final HashSet<PrimitiveId> missing = new HashSet<>();
@@ -87 +96 @@
     //// Handling missing objects
     ////////////////////////////////////////
+
+    /**
+     * Add a primitive that is not in {@link #ds} to {@link #missing}
+     * @param id The primitive to add if missing
+     */
     private void addIfMissing(PrimitiveId id) {
         OsmPrimitive p = ds.getPrimitiveById(id);
@@ -94 +108 @@
     }
 
+    /**
+     * Add missing primitives to {@link #missing}
+     * @param primitives The primitives to iterate through
+     */
     private void addMissingHistoryIds(Iterable<HistoryOsmPrimitive> primitives) {
         for (HistoryOsmPrimitive p : primitives) {
@@ -100 +118 @@
     }
 
+    /**
+     * Add missing necessary primitives to {@link #missing}
+     * @param p The primitive to look at
+     */
     private void addMissingId(OsmPrimitive p) {
         addIfMissing(p);
@@ -171 +193 @@
         }
 
-        // Build our own lists of created/updated/modified objects for better performance
+        buildCreatedUpdatedModifiedObjectLists();
+    }
+
+    /**
+     * Build our own lists of created/updated/modified objects for better performance
+     */
+    private void buildCreatedUpdatedModifiedObjectLists() {
         for (PrimitiveId id : cds.getIds()) {
             ChangesetDataSetEntry first = cds.getFirstEntry(id);
@@ -189 +217 @@
     }
 
+    /**
+     * Add missing primitives that have been created
+     */
     public void checkMissingCreated() {
         addMissingHistoryIds(created);
     }
 
+    /**
+     * Add missing primitives that have been updated
+     */
     public void checkMissingUpdated() {
         addMissingHistoryIds(updated);
     }
 
+    /**
+     * Add missing primitives that have been deleted
+     */
     public void checkMissingDeleted() {
         addMissingHistoryIds(deleted);
@@ -237 +274 @@
             if (progressMonitor.isCanceled()) return;
             nds = rdr.parseOsm(progressMonitor.createSubTaskMonitor(1, true));
-            ds.update(() -> {
-                for (OsmPrimitive p : nds.allPrimitives()) {
-                    if (!p.isIncomplete()) {
-                        addMissingId(p);
-                    } else {
-                        if (ds.getPrimitiveById(p.getPrimitiveId()) == null) {
-                            switch (p.getType()) {
-                            case NODE: ds.addPrimitive(new Node(p.getUniqueId())); break;
-                            case CLOSEDWAY:
-                            case WAY: ds.addPrimitive(new Way(p.getUniqueId())); break;
-                            case MULTIPOLYGON:
-                            case RELATION: ds.addPrimitive(new Relation(p.getUniqueId())); break;
-                            default: throw new AssertionError();
-                            }
-                        }
-                    }
-                }
-            });
+            ds.update(this::addPartialPrimitives);
         } finally {
             progressMonitor.finishTask();
@@ -260 +280 @@
     }
 
+    /**
+     * Add partial primitives if they are incomplete in {@link #nds}
+     */
+    private void addPartialPrimitives() {
+        for (OsmPrimitive p : nds.allPrimitives()) {
+            if (!p.isIncomplete()) {
+                addMissingId(p);
+            } else {
+                if (ds.getPrimitiveById(p.getPrimitiveId()) == null) {
+                    switch (p.getType()) {
+                        case NODE: ds.addPrimitive(new Node(p.getUniqueId())); break;
+                        case CLOSEDWAY:
+                        case WAY: ds.addPrimitive(new Way(p.getUniqueId())); break;
+                        case MULTIPOLYGON:
+                        case RELATION: ds.addPrimitive(new Relation(p.getUniqueId())); break;
+                        default: throw new AssertionError();
+                    }
+                }
+            }
+        }
+    }
+
+    /**
+     * Download any missing primitives we have
+     * @param monitor The monitor to update with our progress
+     * @throws OsmTransferException If there is an issue transferring data from OSM
+     */
     public void downloadMissingPrimitives(ProgressMonitor monitor) throws OsmTransferException {
         if (!hasMissingObjects()) return;
@@ -308 +355 @@
         switch (current.getType()) {
         case NODE:
-            LatLon currentCoor = ((Node) current).getCoor();
-            LatLon historyCoor = ((HistoryNode) history).getCoords();
-            if (currentCoor == historyCoor || (currentCoor != null && historyCoor != null && currentCoor.equals(historyCoor)))
-                return true;
-            // Handle case where a deleted note has been restored to avoid false conflicts (fix #josm8660)
-            if (currentCoor != null && historyCoor == null) {
-                LatLon previousCoor = ((Node) nds.getPrimitiveById(history.getPrimitiveId())).getCoor();
-                return previousCoor != null && previousCoor.equals(currentCoor);
-            }
-            return false;
+            return hasEqualSemanticAttributesNode(nds, (HistoryNode) history, (Node) current);
         case CLOSEDWAY:
         case WAY:
-            List<Node> currentNodes = ((Way) current).getNodes();
-            List<Long> historyNodes = ((HistoryWay) history).getNodes();
-            if (currentNodes.size() != historyNodes.size()) return false;
-            for (int i = 0; i < currentNodes.size(); i++) {
-                if (currentNodes.get(i).getId() != historyNodes.get(i)) return false;
-            }
-            return true;
+            return hasEqualSemanticAttributesWay((HistoryWay) history, (Way) current);
         case MULTIPOLYGON:
         case RELATION:
-            List<org.openstreetmap.josm.data.osm.RelationMember> currentMembers =
-                ((Relation) current).getMembers();
-            List<RelationMemberData> historyMembers = ((HistoryRelation) history).getMembers();
-            if (currentMembers.size() != historyMembers.size()) return false;
-            for (int i = 0; i < currentMembers.size(); i++) {
-                org.openstreetmap.josm.data.osm.RelationMember currentMember =
+            return hasEqualSemanticAttributesRelation((HistoryRelation) history, (Relation) current);
+        default: throw new AssertionError();
+        }
+    }
+
+    /**
+     * Check if two nodes with the same tags are otherwise semantically similar
+     * @param nds The dataset with downloaded data from OSM (see {@link #nds})
+     * @param history The node history
+     * @param current The current node
+     * @return {@code true} if they are semantically similar
+     */
+    private static boolean hasEqualSemanticAttributesNode(DataSet nds, HistoryNode history, Node current) {
+        LatLon currentCoor = current.getCoor();
+        LatLon historyCoor = history.getCoords();
+        if (Objects.equals(currentCoor, historyCoor))
+            return true;
+        // Handle case where a deleted note has been restored to avoid false conflicts (fix #josm8660)
+        if (currentCoor != null && historyCoor == null) {
+            LatLon previousCoor = ((Node) nds.getPrimitiveById(history.getPrimitiveId())).getCoor();
+            return previousCoor != null && previousCoor.equals(currentCoor);
+        }
+        return false;
+    }
+
+    /**
+     * Check if two ways with the same tags are otherwise semantically similar
+     * @param history The way history
+     * @param current The current way
+     * @return {@code true} if they are semantically similar
+     */
+    private static boolean hasEqualSemanticAttributesWay(HistoryWay history, Way current) {
+        List<Node> currentNodes = current.getNodes();
+        List<Long> historyNodes = history.getNodes();
+        if (currentNodes.size() != historyNodes.size()) return false;
+        for (int i = 0; i < currentNodes.size(); i++) {
+            if (currentNodes.get(i).getId() != historyNodes.get(i)) return false;
+        }
+        return true;
+    }
+
+    /**
+     * Check if two relations with the same tags are otherwise semantically similar
+     * @param history The relation history
+     * @param current The current relation
+     * @return {@code true} if they are semantically similar
+     */
+    private static boolean hasEqualSemanticAttributesRelation(HistoryRelation history, Relation current) {
+        List<org.openstreetmap.josm.data.osm.RelationMember> currentMembers = current.getMembers();
+        List<RelationMemberData> historyMembers = history.getMembers();
+        if (currentMembers.size() != historyMembers.size()) return false;
+        for (int i = 0; i < currentMembers.size(); i++) {
+            org.openstreetmap.josm.data.osm.RelationMember currentMember =
                     currentMembers.get(i);
-                RelationMemberData historyMember = historyMembers.get(i);
-                if (!currentMember.getRole().equals(historyMember.getRole())) return false;
-                if (!currentMember.getMember().getPrimitiveId().equals(new SimplePrimitiveId(
-                        historyMember.getMemberId(), historyMember.getMemberType()))) return false;
-            }
-            return true;
-        default: throw new AssertionError();
-        }
+            RelationMemberData historyMember = historyMembers.get(i);
+            if (!currentMember.getRole().equals(historyMember.getRole())) return false;
+            if (!currentMember.getMember().getPrimitiveId().equals(new SimplePrimitiveId(
+                    historyMember.getMemberId(), historyMember.getMemberType()))) return false;
+        }
+        return true;
     }
 
@@ -386 +463 @@
         }
 
-        // Check objects versions
+        checkObjectVersions(cmds, conflicted, toDelete);
+        checkForDeletedReferrers(cmds, conflicted, toDelete);
+        return cmds;
+    }
+
+    /**
+     * Check objects versions
+     * @param cmds The command list to add to
+     * @param conflicted The primitives with conflicts
+     * @param toDelete The primitives that will be deleted
+     */
+    private void checkObjectVersions(List<Command> cmds, HashSet<OsmPrimitive> conflicted, HashSet<OsmPrimitive> toDelete) {
         for (Iterator<ChangesetDataSetEntry> it = cds.iterator(); it.hasNext();) {
             ChangesetDataSetEntry entry = it.next();
-            if (!checkOsmChangeEntry(entry)) continue;
-            if (entry.getModificationType() == ChangesetModificationType.DELETED
-                    && revertType == RevertType.SELECTION_WITH_UNDELETE) {
-                // see #22520: missing merge target when object is first created and then
-                // deleted in the same changeset
-                ChangesetDataSetEntry first = cds.getFirstEntry(entry.getPrimitive().getPrimitiveId());
-                if (first.getModificationType() == ChangesetModificationType.CREATED)
-                    continue;
-            }
-            HistoryOsmPrimitive hp = entry.getPrimitive();
-            OsmPrimitive dp = ds.getPrimitiveById(hp.getPrimitiveId());
-            if (dp == null || dp.isIncomplete()) {
-                throw new IllegalStateException(addChangesetIdPrefix(tr("Missing merge target for {0}", hp.getPrimitiveId())));
-            }
-
-            if (hp.getVersion() != dp.getVersion()
-                    && (hp.isVisible() || dp.isVisible()) &&
-                    /* Don't create conflict if changeset object and dataset object
-                     * has same semantic attributes (but different versions) */
-                    !hasEqualSemanticAttributes(dp, hp)
-                    /* Don't create conflict if the object has to be deleted but has already been deleted */
-                    && !(toDelete.contains(dp) && dp.isDeleted())) {
-                cmds.add(new ConflictAddCommand(layer.data, createConflict(dp,
-                        entry.getModificationType() == ChangesetModificationType.CREATED)));
-                conflicted.add(dp);
-            }
-        }
-
-        /* Check referrers for deleted objects: if object is referred by another object that
-         * isn't going to be deleted or modified, create a conflict.
-         */
+            if (checkOsmChangeEntry(entry) && checkModificationType(cds, entry, revertType)) {
+                HistoryOsmPrimitive hp = entry.getPrimitive();
+                OsmPrimitive dp = ds.getPrimitiveById(hp.getPrimitiveId());
+                if (dp == null || dp.isIncomplete()) {
+                    throw new IllegalStateException(addChangesetIdPrefix(tr("Missing merge target for {0}", hp.getPrimitiveId())));
+                }
+
+                if (checkObjectVersionsNotSemanticallySame(toDelete, hp, dp)) {
+                    cmds.add(new ConflictAddCommand(layer.data, createConflict(dp,
+                            entry.getModificationType() == ChangesetModificationType.CREATED)));
+                    conflicted.add(dp);
+                }
+            }
+        }
+    }
+
+    /**
+     * Check that the modification type is something that we are looking at
+     * @param cds {@link #cds} (current changeset data)
+     * @param entry The entry in the changeset data
+     * @param revertType The type of revert we are doing
+     * @return {@code true} if the entry is something we are interested in for this revert
+     */
+    private static boolean checkModificationType(ChangesetDataSet cds, ChangesetDataSetEntry entry, RevertType revertType) {
+        if (entry.getModificationType() == ChangesetModificationType.DELETED
+                && revertType == RevertType.SELECTION_WITH_UNDELETE) {
+            // see #22520: missing merge target when object is first created and then
+            // deleted in the same changeset
+            ChangesetDataSetEntry first = cds.getFirstEntry(entry.getPrimitive().getPrimitiveId());
+            return first.getModificationType() != ChangesetModificationType.CREATED;
+        }
+        return true;
+    }
+
+    /**
+     * Check that versions are not the same prior to creating a conflict
+     * @param toDelete The deleted objects
+     * @param hp The history primitive
+     * @param dp The current primitive
+     * @return {@code true} if the objects are not semantically the same
+     */
+    private boolean checkObjectVersionsNotSemanticallySame(HashSet<OsmPrimitive> toDelete, HistoryOsmPrimitive hp, OsmPrimitive dp) {
+        return hp.getVersion() != dp.getVersion()
+                && (hp.isVisible() || dp.isVisible()) &&
+                /* Don't create conflict if changeset object and dataset object
+                 * has same semantic attributes (but different versions) */
+                !hasEqualSemanticAttributes(dp, hp)
+                /* Don't create conflict if the object has to be deleted but has already been deleted */
+                && !(toDelete.contains(dp) && dp.isDeleted());
+    }
+
+    /**
+     * Check referrers for deleted objects: if object is referred by another object that
+     * isn't going to be deleted or modified, create a conflict.
+     * @param cmds The command list to add to
+     * @param conflicted The primitives with conflicts
+     * @param toDelete The primitives that will be deleted
+     */
+    private void checkForDeletedReferrers(List<Command> cmds, HashSet<OsmPrimitive> conflicted, HashSet<OsmPrimitive> toDelete) {
         List<OsmPrimitive> delSorted = toDelete.stream()
                 .filter(p -> !p.isDeleted())
@@ -429 +545 @@
             for (int i = 0; i < delSorted.size() && !restartNeeded; i++) {
                 OsmPrimitive p = delSorted.get(i);
-                for (OsmPrimitive referrer : p.getReferrers()) {
-                    if (toDelete.contains(referrer)) continue; // object is going to be deleted
-                    if (nds.getPrimitiveById(referrer) != null)
-                        continue; /* object is going to be modified so it cannot refer to
-                         * objects created in changeset to be reverted
-                         */
-                    if (conflicted.add(p)) {
-                        cmds.add(new ConflictAddCommand(layer.data, createConflict(p, true)));
-                        if (p instanceof Relation) {
-                            // handle possible special case with nested relations
-                            for (int j = 0; j < i; j++) {
-                                if (delSorted.get(j).getReferrers().contains(p)) {
-                                    // we have to create a conflict for a previously processed relation
-                                    restartNeeded = true;
-                                    break;
-                                }
-                            }
-                        }
-                    }
-                    toDelete.remove(p);
-                    break;
-                }
+                restartNeeded = checkForDeletedReferrersPrimitive(cmds, conflicted, toDelete, delSorted, p, i);
             }
         } while (restartNeeded);
@@ -458 +553 @@
             cmds.add(new DeleteCommand(delSorted));
         }
-        return cmds;
-    }
-
+    }
+
+    /**
+     *
+     * @param cmds The command list to add to
+     * @param conflicted The primitives with conflicts
+     * @param toDelete The primitives that will be deleted
+     * @param delSorted The list of sorted deleted objects we need to check against
+     * @param p The primitive to check
+     * @param i The current index in {@code delSorted}
+     * @return {@code true} if we need to restart processing
+     */
+    private boolean checkForDeletedReferrersPrimitive(List<Command> cmds, HashSet<OsmPrimitive> conflicted,
+                                                      HashSet<OsmPrimitive> toDelete, List<OsmPrimitive> delSorted,
+                                                      OsmPrimitive p, int i) {
+        for (OsmPrimitive referrer : p.getReferrers()) {
+            if (toDelete.contains(referrer) || // object is going to be deleted
+                    nds.getPrimitiveById(referrer) != null)
+                continue; /* object is going to be modified so it cannot refer to
+                 * objects created in changeset to be reverted
+                 */
+            if (conflicted.add(p)) {
+                cmds.add(new ConflictAddCommand(layer.data, createConflict(p, true)));
+                if (p instanceof Relation) {
+                    // handle possible special case with nested relations
+                    for (int j = 0; j < i; j++) {
+                        if (delSorted.get(j).getReferrers().contains(p)) {
+                            // we have to create a conflict for a previously processed relation
+                            return true;
+                        }
+                    }
+                }
+            }
+            toDelete.remove(p);
+            return false;
+        }
+        return false;
+    }
+
+    /**
+     * Check if there are missing objects
+     * @return {@code true} if we need to handle missing objects
+     */
     public boolean hasMissingObjects() {
         return !missing.isEmpty();
     }
 
+    /**
+     * Get coordinates for nodes that have no coordinates
+     * @param progressMonitor The monitor to update our progress with
+     * @throws OsmTransferException If we have an issue getting data from the API
+     */
     public void fixNodesWithoutCoordinates(ProgressMonitor progressMonitor) throws OsmTransferException {
         Collection<Node> nodes = new ArrayList<>(nds.getNodes());

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

@@ -2 +2 @@
 package reverter;
 
+import static org.openstreetmap.josm.tools.I18n.marktr;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
@@ -14 +15 @@
 import reverter.ChangesetReverter.RevertType;
 
+/**
+ * An action for reverting changesets
+ */
 public class RevertChangesetAction extends JosmAction {
+    private static final String REVERT_CHANGESET = marktr("Revert changeset");
 
+    /**
+     * Create a new action for reverting changesets
+     */
     public RevertChangesetAction() {
-        super(tr("Revert changeset"), "revert-changeset", tr("Revert changeset"),
-            Shortcut.registerShortcut("tool:revert", tr("Data: {0}", tr("Revert changeset")), KeyEvent.VK_T, Shortcut.CTRL_SHIFT),
+        super(tr(REVERT_CHANGESET), "revert-changeset", tr(REVERT_CHANGESET),
+            Shortcut.registerShortcut("tool:revert", tr("Data: {0}", tr(REVERT_CHANGESET)), KeyEvent.VK_T, Shortcut.CTRL_SHIFT),
                 true, false);
     }

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

@@ -9 +9 @@
 import java.util.Collections;
 import java.util.List;
-import java.util.concurrent.Callable;
 import java.util.stream.Collectors;
 
@@ -30 +29 @@
 import reverter.ChangesetReverter.RevertType;
 
+/**
+ * The task for reverting changesets
+ */
 public class RevertChangesetTask extends PleaseWaitRunnable {
     private final Collection<Integer> changesetIds;
@@ -40 +42 @@
     private int numberOfConflicts;
 
+    /**
+     * Create a new task for reverting a changeset with a default progress monitor and no new layer with user confirmation of the download
+     * @param changesetId The changeset id to revert
+     * @param revertType The type of revert to do
+     */
     public RevertChangesetTask(int changesetId, RevertType revertType) {
         this(changesetId, revertType, false);
     }
 
+    /**
+     * Create a new task for reverting a changeset with a default progress monitor and no new layer
+     * @param changesetId The changeset id to revert
+     * @param revertType The type of revert to do
+     * @param autoConfirmDownload {@code true} if the user has already indicated that they want to download missing data
+     */
     public RevertChangesetTask(int changesetId, RevertType revertType, boolean autoConfirmDownload) {
         this(changesetId, revertType, autoConfirmDownload, false);
     }
 
+    /**
+     * Create a new task for reverting a changeset with a default progress monitor
+     * @param changesetId The changeset id to revert
+     * @param revertType The type of revert to do
+            * @param autoConfirmDownload {@code true} if the user has already indicated that they want to download missing data
+     * @param newLayer {@code true} if the user wants the reversion to be on a new layer
+     */
     public RevertChangesetTask(int changesetId, RevertType revertType, boolean autoConfirmDownload, boolean newLayer) {
         this(Collections.singleton(changesetId), revertType, autoConfirmDownload, newLayer);
     }
 
+    /**
+     * Create a new task for reverting a changeset with a default progress monitor
+     * @param changesetIds The changeset ids to revert
+     * @param revertType The type of revert to do
+     * @param autoConfirmDownload {@code true} if the user has already indicated that they want to download missing data
+     * @param newLayer {@code true} if the user wants the reversion to be on a new layer
+     */
     public RevertChangesetTask(Collection<Integer> changesetIds, RevertType revertType, boolean autoConfirmDownload, boolean newLayer) {
         this(null, changesetIds, revertType, autoConfirmDownload, newLayer);
@@ -64 +91 @@
      * @param newLayer {@code true} if the user wants the reversion to be on a new layer
      */
-    public RevertChangesetTask(ProgressMonitor progressMonitor, Collection<Integer> changesetIds, RevertType revertType, boolean autoConfirmDownload, boolean newLayer) {
+    public RevertChangesetTask(ProgressMonitor progressMonitor, Collection<Integer> changesetIds, RevertType revertType,
+                               boolean autoConfirmDownload, boolean newLayer) {
         super(tr("Reverting..."), progressMonitor, false);
         this.changesetIds = new ArrayList<>(changesetIds);
@@ -77 +105 @@
         if (!rev.hasMissingObjects()) return true;
         if (!downloadConfirmed) {
-            final Integer selectedOption = GuiHelper.runInEDTAndWaitAndReturn(new Callable<Integer>() {
-                @Override
-                public Integer call() throws Exception {
-                    return JOptionPane.showConfirmDialog(MainApplication.getMainFrame(),
-                            tr("This changeset has objects that are not present in current dataset.\n" +
-                                    "It is needed to download them before reverting. Do you want to continue?"),
-                            tr("Confirm"), JOptionPane.YES_NO_OPTION);
-                }
-            });
+            final Integer selectedOption = GuiHelper.runInEDTAndWaitAndReturn(() -> JOptionPane.showConfirmDialog(MainApplication.getMainFrame(),
+                    tr("This changeset has objects that are not present in current dataset.\n" +
+                            "It is needed to download them before reverting. Do you want to continue?"),
+                    tr("Confirm"), JOptionPane.YES_NO_OPTION));
             downloadConfirmed = selectedOption != null && selectedOption == JOptionPane.YES_OPTION;
             if (!downloadConfirmed) return false;
@@ -142 +165 @@
             rev = new ChangesetReverter(changesetId, revertType, newLayer, oldDataSet, progressMonitor.createSubTaskMonitor(0, true));
         } catch (final RevertRedactedChangesetException e) {
+            Logging.debug(e);
             GuiHelper.runInEDT(() -> new Notification(
                     e.getMessage()+"<br>"+
@@ -191 +215 @@
             return null;
         }
-        GuiHelper.runInEDT(() -> cmds.get(0).getAffectedDataSet().update(() -> {
+        GuiHelper.runInEDT(() -> executeCommands(cmds));
+        final String desc = getRevertDescription(revertType, changesetId);
+        return new RevertChangesetCommand(desc, cmds);
+    }
+
+    /**
+     * Execute a list of commands
+     * @param cmds The commands to execute
+     */
+    private void executeCommands(List<Command> cmds) {
+        cmds.get(0).getAffectedDataSet().update(() -> {
             for (Command c : cmds) {
                 if (c instanceof ConflictAddCommand) {
@@ -198 +232 @@
                 c.executeCommand();
             }
-        }));
-        final String desc;
+        });
+    }
+
+    /**
+     * Get the description to show the user for this reversion
+     * @param revertType The type of revert
+     * @param changesetId The changeset id
+     * @return The string to show the user
+     */
+    private static String getRevertDescription(RevertType revertType, int changesetId) {
         if (revertType == RevertType.FULL) {
-            desc = tr("Revert changeset {0}", String.valueOf(changesetId));
+            return tr("Revert changeset {0}", String.valueOf(changesetId));
         } else {
-            desc = tr("Partially revert changeset {0}", String.valueOf(changesetId));
-        }
-        return new RevertChangesetCommand(desc, cmds);
+            return tr("Partially revert changeset {0}", String.valueOf(changesetId));
+        }
     }