Index: src/reverter/ChangesetReverter.java =================================================================== --- src/reverter/ChangesetReverter.java (revision 36234) +++ src/reverter/ChangesetReverter.java (working copy) @@ -4,7 +4,6 @@ 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,10 +45,8 @@ 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,27 +200,6 @@ 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,9 +213,9 @@ addChangesetIdPrefix( trn("Downloading history for {0} object", "Downloading history for {0} objects", num, num)), num + 1); try { - HashMap nodeList = new HashMap<>(), - wayList = new HashMap<>(), - relationList = new HashMap<>(); + HashMap nodeList = new HashMap<>(); + HashMap wayList = new HashMap<>(); + HashMap relationList = new HashMap<>(); for (HashSet collection : Arrays.asList(updated, deleted)) { for (HistoryOsmPrimitive entry : collection) { @@ -255,24 +231,10 @@ } } } - 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 entry : nodeList.entrySet()) { - if (progressMonitor.isCanceled()) return; - readObjectVersion(rdr, new SimplePrimitiveId(entry.getKey(), OsmPrimitiveType.NODE), entry.getValue(), progressMonitor); - } - for (Map.Entry entry : wayList.entrySet()) { - if (progressMonitor.isCanceled()) return; - readObjectVersion(rdr, new SimplePrimitiveId(entry.getKey(), OsmPrimitiveType.WAY), entry.getValue(), progressMonitor); - } - for (Map.Entry 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,10 +474,13 @@ // 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,17 +488,18 @@ final Map 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,11 +520,14 @@ 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()) { Index: src/reverter/OsmServerMultiObjectReader.java =================================================================== --- src/reverter/OsmServerMultiObjectReader.java (revision 36234) +++ src/reverter/OsmServerMultiObjectReader.java (working copy) @@ -5,6 +5,7 @@ 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,6 +16,7 @@ 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,7 +51,7 @@ * @param list The map of ids to versions * @return The queries to make */ - private static List makeQueryStrings(OsmPrimitiveType type, Map list) { + private static List makeQueryStrings(OsmPrimitiveType type, Map 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,8 +64,8 @@ ).sum() / MAX_QUERY_LENGTH); List result = new ArrayList<>(expectedSize + 1); StringBuilder sb = new StringBuilder(); - int cnt=0; - for (Map.Entry entry : list.entrySet()) { + int cnt = 0; + for (Map.Entry entry : list.entrySet()) { if (cnt == 0) { sb.append(type.getAPIName()); sb.append("s?"); @@ -82,7 +84,7 @@ cnt = 0; } } - if (cnt>0) { + if (cnt > 0) { result.add(sb.toString()); } return result; @@ -108,14 +110,14 @@ protected static final int MAX_QUERY_LENGTH = 8000; /** - * Parse many objects + * Parse many objects. * @param type The object type (must 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 list, ProgressMonitor progressMonitor) throws OsmTransferException { - for (String query : makeQueryStrings(type,list)) { + public void readMultiObjects(OsmPrimitiveType type, Map list, ProgressMonitor progressMonitor) throws OsmTransferException { + for (String query : makeQueryStrings(type, list)) { if (progressMonitor.isCanceled()) { return; } @@ -129,6 +131,13 @@ } 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,7 +144,45 @@ } } + /** + * Parse many objects. If redacted elements are requested the method tries to retrieve the next older version. + * @param type The object type (must 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 list, + ProgressMonitor progressMonitor) throws OsmTransferException { + readMultiObjects(type, list, progressMonitor); + // If multi-read failed, retry with regular read + for (Map.Entry 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