Ticket #23584: 23584-2.patch

src/reverter/ChangesetReverter.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 import static org.openstreetmap.josm.tools.I18n.trn;
 
-import java.net.HttpURLConnection;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -46 +45 @@
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.io.MultiFetchServerObjectReader;
-import org.openstreetmap.josm.io.OsmApiException;
 import org.openstreetmap.josm.io.OsmServerChangesetReader;
 import org.openstreetmap.josm.io.OsmTransferException;
-import org.openstreetmap.josm.tools.Logging;
 
 /**
  * Fetches and stores data for reverting of specific changeset.
@@ -203 +200 @@
         addMissingHistoryIds(deleted);
     }
 
-    private static void readObjectVersion(OsmServerMultiObjectReader rdr, PrimitiveId id, int version, ProgressMonitor progressMonitor)
-            throws OsmTransferException {
-        boolean readOK = false;
-        while (!readOK && version >= 1) {
-            try {
-                rdr.readObject(id, version, progressMonitor.createSubTaskMonitor(1, true));
-                readOK = true;
-            } catch (OsmApiException e) {
-                if (e.getResponseCode() != HttpURLConnection.HTTP_FORBIDDEN) {
-                    throw e;
-                }
-                String message = "Version " + version + " of " + id + " is unauthorized";
-                Logging.info(version <= 1 ? message : message + ", requesting previous one");
-                version--;
-            }
-        }
-        if (!readOK) {
-            Logging.warn("Cannot retrieve any previous version of "+id);
-        }
-    }
-
     /**
      * fetch objects that were updated or deleted by changeset
      * @param progressMonitor progress monitor
@@ -237 +213 @@
                 addChangesetIdPrefix(
                         trn("Downloading history for {0} object", "Downloading history for {0} objects", num, num)), num + 1);
         try {
-            HashMap<Long, Integer> nodeList = new HashMap<>(),
-                    wayList = new HashMap<>(),
-                    relationList = new HashMap<>();
+            HashMap<Long, Integer> nodeList = new HashMap<>();
+            HashMap<Long, Integer> wayList = new HashMap<>();
+            HashMap<Long, Integer> relationList = new HashMap<>();
 
             for (HashSet<HistoryOsmPrimitive> collection : Arrays.asList(updated, deleted)) {
                 for (HistoryOsmPrimitive entry : collection) {
@@ -255 +231 @@
                     }
                 }
             }
-            rdr.readMultiObjects(OsmPrimitiveType.NODE, nodeList, progressMonitor);
-            rdr.readMultiObjects(OsmPrimitiveType.WAY, wayList, progressMonitor);
-            rdr.readMultiObjects(OsmPrimitiveType.RELATION, relationList, progressMonitor);
+            rdr.readMultiObjectsOrNextOlder(OsmPrimitiveType.NODE, nodeList, progressMonitor);
+            rdr.readMultiObjectsOrNextOlder(OsmPrimitiveType.WAY, wayList, progressMonitor);
+            rdr.readMultiObjectsOrNextOlder(OsmPrimitiveType.RELATION, relationList, progressMonitor);
             if (progressMonitor.isCanceled()) return;
-            // If multi-read failed, retry with regular read
-            for (Map.Entry<Long, Integer> entry : nodeList.entrySet()) {
-                if (progressMonitor.isCanceled()) return;
-                readObjectVersion(rdr, new SimplePrimitiveId(entry.getKey(), OsmPrimitiveType.NODE), entry.getValue(), progressMonitor);
-            }
-            for (Map.Entry<Long, Integer> entry : wayList.entrySet()) {
-                if (progressMonitor.isCanceled()) return;
-                readObjectVersion(rdr, new SimplePrimitiveId(entry.getKey(), OsmPrimitiveType.WAY), entry.getValue(), progressMonitor);
-            }
-            for (Map.Entry<Long, Integer> entry : relationList.entrySet()) {
-                if (progressMonitor.isCanceled()) return;
-                readObjectVersion(rdr, new SimplePrimitiveId(entry.getKey(), OsmPrimitiveType.RELATION), entry.getValue(), progressMonitor);
-            }
-            if (progressMonitor.isCanceled()) return;
             nds = rdr.parseOsm(progressMonitor.createSubTaskMonitor(1, true));
             ds.update(() -> {
                 for (OsmPrimitive p : nds.allPrimitives()) {
@@ -512 +474 @@
 
         // Remove primitives where we already know the LatLon.
         nodes.removeIf(n -> {
+            if (n.isDeleted() || n.isLatLonKnown())
+                return true;
             PrimitiveId id = n.getPrimitiveId();
             OsmPrimitive p = ds.getPrimitiveById(id);
-            return !(p instanceof Node) || ((Node) p).isLatLonKnown();
+            return !(p instanceof Node) || ((Node) p).isLatLonKnown() || p.getVersion() <= 1;
         });
+
         progressMonitor.worked(num - nodes.size());
 
         // Do bulk version fetches first
@@ -523 +488 @@
         final Map<Long, Integer> versionMap = nodes.stream()
                 .collect(Collectors.toMap(AbstractPrimitive::getUniqueId,
                         id -> Math.max(1, Optional.ofNullable(ds.getPrimitiveById(id)).orElse(id).getVersion() - 1)));
+
         try {
             while (!versionMap.isEmpty()) {
                 final OsmServerMultiObjectReader rds = new OsmServerMultiObjectReader();
                 final ProgressMonitor subMonitor = progressMonitor.createSubTaskMonitor(num, false);
                 subMonitor.beginTask(tr("Fetching multi-objects"), versionMap.size());
-                rds.readMultiObjects(OsmPrimitiveType.NODE, versionMap, subMonitor);
+                rds.readMultiObjectsOrNextOlder(OsmPrimitiveType.NODE, versionMap, subMonitor);
                 subMonitor.finishTask();
                 final DataSet history = rds.parseOsm(progressMonitor.createSubTaskMonitor(0, false));
+                ds.update(() -> updateNodes(progressMonitor, nodes, versionMap, history));
+                versionMap.values().removeIf(i -> i <= 1);
                 versionMap.replaceAll((key, value) -> value - 1);
-                versionMap.values().removeIf(i -> i <= 0);
-                ds.update(() -> updateNodes(progressMonitor, nodes, versionMap, history));
                 if (progressMonitor.isCanceled()) {
                     break;
                 }
@@ -554 +520 @@
         for (Node n : nodes) {
             if (!n.isDeleted() && !n.isLatLonKnown()) {
                 final Node historyNode = (Node) history.getPrimitiveById(n);
-                if (historyNode != null && historyNode.isLatLonKnown()
-                        && changeset.getClosedAt().isAfter(historyNode.getInstant())) {
-                    n.load(historyNode.save());
-                    versionMap.remove(n.getUniqueId());
-                    progressMonitor.worked(1);
+                if (historyNode != null) {
+                    if (historyNode.isLatLonKnown()
+                            && changeset.getClosedAt().isAfter(historyNode.getInstant())) {
+                        n.load(historyNode.save());
+                        progressMonitor.worked(1);
+                    } else {
+                        versionMap.put(n.getId(), historyNode.getVersion());
+                    }
                 }
             }
             if (progressMonitor.isCanceled()) {

src/reverter/OsmServerMultiObjectReader.java

@@ -5 +5 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.HttpURLConnection;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -15 +16 @@
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.io.IllegalDataException;
+import org.openstreetmap.josm.io.OsmApiException;
 import org.openstreetmap.josm.io.OsmServerReader;
 import org.openstreetmap.josm.io.OsmTransferException;
 import org.openstreetmap.josm.tools.Logging;
@@ -49 +51 @@
      * @param list The map of ids to versions
      * @return The queries to make
      */
-    private static List<String> makeQueryStrings(OsmPrimitiveType type, Map<Long,Integer> list) {
+    private static List<String> makeQueryStrings(OsmPrimitiveType type, Map<Long, Integer> list) {
         // This is a "worst-case" calculation. Keep it fast (and err higher rather than lower), not accurate.
         final int expectedSize = (int) (list.entrySet().stream().mapToLong(entry ->
                 // Keep in mind that 0-3 is 0, 3-32 is 1, 32-316 is 2, and so on when rounding log10.
@@ -62 +64 @@
                 ).sum() / MAX_QUERY_LENGTH);
         List<String> result = new ArrayList<>(expectedSize + 1);
         StringBuilder sb = new StringBuilder();
-        int cnt=0;
-        for (Map.Entry<Long,Integer> entry : list.entrySet()) {
+        int cnt = 0;
+        for (Map.Entry<Long, Integer> entry : list.entrySet()) {
             if (cnt == 0) {
                 sb.append(type.getAPIName());
                 sb.append("s?");
@@ -82 +84 @@
                 cnt = 0;
             }
         }
-        if (cnt>0) {
+        if (cnt > 0) {
             result.add(sb.toString());
         }
         return result;
@@ -108 +110 @@
     protected static final int MAX_QUERY_LENGTH = 8000;
 
     /**
-     * Parse many objects
+     * Parse many objects.
      * @param type The object type (<i>must</i> be common between all objects)
-     * @param list The map of object id to object version
+     * @param list The map of object id to object version. Successfully retrieved objects are removed.
      * @param progressMonitor The progress monitor to update
      * @throws OsmTransferException If there is an issue getting the data
      */
-    public void readMultiObjects(OsmPrimitiveType type, Map<Long,Integer> list, ProgressMonitor progressMonitor) throws OsmTransferException {
-        for (String query : makeQueryStrings(type,list)) {
+    public void readMultiObjects(OsmPrimitiveType type, Map<Long, Integer> list, ProgressMonitor progressMonitor) throws OsmTransferException {
+        for (String query : makeQueryStrings(type, list)) {
             if (progressMonitor.isCanceled()) {
                 return;
             }
@@ -129 +131 @@
             } catch (IOException | IllegalDataException e) {
                 Logging.warn(e);
                 throw new OsmTransferException(e);
+            } catch (OsmApiException e) {
+                Logging.warn(e);
+                // allow to continue further bulk requests
+                if (e.getResponseCode() != HttpURLConnection.HTTP_FORBIDDEN
+                        && e.getResponseCode() != HttpURLConnection.HTTP_NOT_FOUND) {
+                    throw e;
+                }
             } finally {
                 rdr.callback = null;
             }
@@ -135 +144 @@
         }
     }
 
+    /**
+     * Parse many objects. If redacted elements are requested the method tries to retrieve the next older version.
+     * @param type The object type (<i>must</i> be common between all objects)
+     * @param list The map of object id to object version. Successfully retrieved objects are removed.
+     * @param progressMonitor The progress monitor to update
+     * @throws OsmTransferException If there is an issue getting the data
+     */
+    public void readMultiObjectsOrNextOlder(OsmPrimitiveType type, Map<Long, Integer> list,
+            ProgressMonitor progressMonitor) throws OsmTransferException {
+        readMultiObjects(type, list, progressMonitor);
+        // If multi-read failed, retry with regular read
+        for (Map.Entry<Long, Integer> entry : list.entrySet()) {
+            if (progressMonitor.isCanceled()) return;
+            readObjectVersion(type, entry.getKey(), entry.getValue(), progressMonitor);
+        }
+    }
 
+    private void readObjectVersion(OsmPrimitiveType type, long id, int version, ProgressMonitor progressMonitor)
+            throws OsmTransferException {
+        boolean readOK = false;
+        while (!readOK && version >= 1) {
+            try {
+                readObject(id, version, type, progressMonitor.createSubTaskMonitor(1, true));
+                readOK = true;
+            } catch (OsmApiException e) {
+                if (e.getResponseCode() != HttpURLConnection.HTTP_FORBIDDEN) {
+                    throw e;
+                }
+                String message = "Version " + version + " of " + id + " is unauthorized";
+                Logging.info(version <= 1 ? message : message + ", requesting previous one");
+                version--;
+            }
+        }
+        if (!readOK) {
+            Logging.warn("Cannot retrieve any previous version of {1} {2}", type, id);
+        }
+    }
+
+
     /**
      * Method to parse downloaded objects
      * @return the data requested