Changeset 34953 in osm

Timestamp:

2019-03-30T09:39:01+01:00 (6 years ago)

Author:

gerdp

Message:

fix #17535:

• don't create new progress monitor in RevertChangesetTask.checkAndDownloadMissing() , instead use setTicks(0) in existing one
• add changeset id to progress bar (where possible)
• add changeset id to IllegalStateException "Missing merge target" created in ChangesetReverter
• react on Cancel button in method fixNodesWithoutCoordinates()
• fix / improve several I18N strings
• fix some javadoc/sonarLint issues
• improve code in RevertChangesetCommand (no functional change)

Location:

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

Files:

• ChangesetReverter.java (modified) (6 diffs)
• DataSetCommandMerger.java (modified) (1 diff)
• RevertChangesetCommand.java (modified) (2 diffs)
• RevertChangesetTask.java (modified) (7 diffs)

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

@@ -3 +3 @@
 
 import static org.openstreetmap.josm.tools.I18n.tr;
+import static org.openstreetmap.josm.tools.I18n.trn;
 
 import java.net.HttpURLConnection;
@@ -61 +62 @@
 
     public static final Collection<Long> MODERATOR_REDACTION_ACCOUNTS = Collections.unmodifiableCollection(Arrays.asList(
-            722137L, // OSMF Redaction Account
-            760215L  // pnorman redaction revert
+            722_137L, // OSMF Redaction Account
+            760_215L  // pnorman redaction revert
             ));
 
@@ -193 +194 @@
                     throw e;
                 }
-                String message = "Version "+version+" of "+id+" is unauthorized";
-                if (version > 1) {
-                    message += ", requesting previous one";
-                }
-                Logging.info(message);
+                String message = "Version " + version + " of " + id + " is unauthorized";
+                Logging.info(version <= 1 ? message : message + ", requesting previous one");
                 version--;
             }
@@ -214 +212 @@
         final OsmServerMultiObjectReader rdr = new OsmServerMultiObjectReader();
 
-        progressMonitor.beginTask(tr("Downloading objects history"), updated.size()+deleted.size()+1);
+        int num = updated.size() + deleted.size();
+        progressMonitor.beginTask(
+                addChangesetIdPrefix(
+                        trn("Downloading history for {0} object", "Downloading history for {0} objects", num, num)), num + 1);
         try {
             for (HashSet<HistoryOsmPrimitive> collection : Arrays.asList(updated, deleted)) {
@@ -377 +378 @@
             HistoryOsmPrimitive hp = entry.getPrimitive();
             OsmPrimitive dp = ds.getPrimitiveById(hp.getPrimitiveId());
-            if (dp == null || dp.isIncomplete())
-                throw new IllegalStateException(tr("Missing merge target for {0} with id {1}",
-                        hp.getType(), hp.getId()));
+            if (dp == null || dp.isIncomplete()) {
+                throw new IllegalStateException(addChangesetIdPrefix(tr("Missing merge target for {0}", hp.getPrimitiveId())));
+            }
 
             if (hp.getVersion() != dp.getVersion()
@@ -434 +435 @@
 
     public void fixNodesWithoutCoordinates(ProgressMonitor progressMonitor) throws OsmTransferException {
-        for (Node n : nds.getNodes()) {
-            if (!n.isDeleted() && n.getCoor() == null) {
-                PrimitiveId id = n.getPrimitiveId();
-                OsmPrimitive p = ds.getPrimitiveById(id);
-                if (p instanceof Node && !((Node) p).isLatLonKnown()) {
-                    int version = p.getVersion();
-                    while (version > 1) {
-                        // find the version that was in use when the current changeset was closed
-                        --version;
-                        final OsmServerMultiObjectReader rdr = new OsmServerMultiObjectReader();
-                        readObjectVersion(rdr, id, version, progressMonitor);
-                        DataSet history = rdr.parseOsm(progressMonitor.createSubTaskMonitor(1, true));
-                        if (!history.isEmpty()) {
-                            Node historyNode = (Node) history.allPrimitives().iterator().next();
-                            if (historyNode.isLatLonKnown() && changeset.getClosedAt().after(historyNode.getTimestamp())) {
-                                n.load(historyNode.save());
-                                break;
+        Collection<Node> nodes = nds.getNodes();
+        int num = nodes.size();
+        progressMonitor.beginTask(addChangesetIdPrefix(
+                trn("Checking coordinates of {0} node", "Checking coordinates of {0} nodes", num, num)), num);
+
+        try {
+            for (Node n : nodes) {
+                if (!n.isDeleted() && n.getCoor() == null) {
+                    PrimitiveId id = n.getPrimitiveId();
+                    OsmPrimitive p = ds.getPrimitiveById(id);
+                    if (p instanceof Node && !((Node) p).isLatLonKnown()) {
+                        int version = p.getVersion();
+                        while (version > 1) {
+                            // find the version that was in use when the current changeset was closed
+                            --version;
+                            final OsmServerMultiObjectReader rdr = new OsmServerMultiObjectReader();
+                            readObjectVersion(rdr, id, version, progressMonitor);
+                            DataSet history = rdr.parseOsm(progressMonitor.createSubTaskMonitor(1, true));
+                            if (!history.isEmpty()) {
+                                Node historyNode = (Node) history.allPrimitives().iterator().next();
+                                if (historyNode.isLatLonKnown() && changeset.getClosedAt().after(historyNode.getTimestamp())) {
+                                    n.load(historyNode.save());
+                                    break;
+                                }
                             }
                         }
                     }
                 }
-            }
-        }
+                if (progressMonitor.isCanceled())
+                    return;
+                progressMonitor.worked(1);
+            }
+        } finally {
+            progressMonitor.finishTask();
+        }
+    }
+
+    /**
+     * Add prefix with properly formatted changeset id to a message.
+     * @param msg the message string
+     * @return prefixed message
+     */
+    String addChangesetIdPrefix(String msg) {
+        return tr("Changeset {0}: {1}", Long.toString(changesetId), msg);
     }
 }

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

@@ -62 +62 @@
         OsmPrimitive p = targetDataSet.getPrimitiveById(mergeSource.getId(), mergeSource.getType());
         if (p == null)
-            throw new IllegalStateException(tr("Missing merge target of type {0} with id {1}",
-                    mergeSource.getType(), mergeSource.getUniqueId()));
+            throw new IllegalStateException(tr("Missing merge target for {0}", mergeSource.getPrimitiveId()));
         return p;
     }

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

@@ -10 +10 @@
 import org.openstreetmap.josm.tools.ImageProvider;
 
+/**
+ * Commands to revert a changeset.
+ *
+ */
 public class RevertChangesetCommand extends SequenceCommand {
-    protected String name;
 
+    /**
+     * Create the command by specifying the list of commands to execute.
+     * @param name The description text
+     * @param sequenz The sequence that should be executed.
+     */
     public RevertChangesetCommand(String name, Collection<Command> sequenz) {
         super(name, sequenz);
-        this.name = name;
         ReverterPlugin.reverterUsed = true;
     }
@@ -21 +28 @@
     @Override
     public String getDescriptionText() {
-        return name;
+        return getName();
     }
 

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

@@ -22 +22 @@
 import org.openstreetmap.josm.gui.PleaseWaitRunnable;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
-import org.openstreetmap.josm.gui.progress.swing.PleaseWaitProgressMonitor;
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.io.OsmTransferException;
@@ -74 +73 @@
             if (!downloadConfirmed) return false;
         }
-        final PleaseWaitProgressMonitor monitor =
-            new PleaseWaitProgressMonitor(tr("Fetching missing primitives"));
-        try {
-            rev.downloadMissingPrimitives(monitor);
-        } finally {
-            monitor.close();
-        }
-        return !monitor.isCanceled();
+        progressMonitor.setTicks(0);
+        rev.downloadMissingPrimitives(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
+        return !progressMonitor.isCanceled();
     }
 
@@ -109 +103 @@
         }
         if (!allcmds.isEmpty()) {
-            Command cmd = allcmds.size() == 1 ? allcmds.get(0) : new SequenceCommand(tr("Revert changeset"), allcmds);
+            Command cmd = allcmds.size() == 1 ? allcmds.get(0) : new SequenceCommand(tr("Revert changesets"), allcmds);
             GuiHelper.runInEDT(() -> {
                 UndoRedoHandler.getInstance().add(cmd);
@@ -120 +114 @@
 
     private RevertChangesetCommand revertChangeset(int changesetId) throws OsmTransferException, UserCancelException {
-        progressMonitor.indeterminateSubTask(tr("Downloading changeset"));
+        progressMonitor.indeterminateSubTask(tr("Reverting changeset {0}", Long.toString(changesetId)));
         try {
             rev = new ChangesetReverter(changesetId, revertType, newLayer, progressMonitor.createSubTaskMonitor(0, true));
@@ -146 +140 @@
             // Don't ask user to download primitives going to be undeleted
             rev.checkMissingDeleted();
-            rev.downloadMissingPrimitives(progressMonitor.createSubTaskMonitor(0, false));
+            rev.downloadMissingPrimitives(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
         }
 
         if (progressMonitor.isCanceled())
             throw new UserCancelException();
+        progressMonitor.setTicks(0);
         rev.downloadObjectsHistory(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
         if (progressMonitor.isCanceled())
@@ -156 +151 @@
         if (!checkAndDownloadMissing())
             throw new UserCancelException();
-        rev.fixNodesWithoutCoordinates(progressMonitor);
+        progressMonitor.setTicks(0);
+        rev.fixNodesWithoutCoordinates(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false));
+        if (progressMonitor.isCanceled())
+            throw new UserCancelException();
         List<Command> cmds = rev.getCommands();
         if (cmds.isEmpty()) {
@@ -167 +165 @@
             }
         }
-        return new RevertChangesetCommand(tr(revertType == RevertType.FULL ? "Revert changeset #{0}" :
-                "Partially revert changeset #{0}", changesetId), cmds);
+        final String desc;
+        if (revertType == RevertType.FULL) {
+            desc = tr("Revert changeset {0}", String.valueOf(changesetId));
+        } else {
+            desc = tr("Partially revert changeset {0}", String.valueOf(changesetId));
+        }
+        return new RevertChangesetCommand(desc, cmds);
     }
 
     @Override
     protected void cancel() {
+        // nothing to do
     }
 
     @Override
     protected void finish() {
+        // nothing to do
+
     }
 
+    /**
+     * @return number of conflicts for this changeset
+     */
     public final int getNumberOfConflicts() {
         return numberOfConflicts;