From Mon Sep 17 00:00:00 2001 From: Matt McCutchen Date: Thu, 15 Oct 2026 16:21:55 -0400 In PostDownloadProcessor, decide whether to use the full-update algorithm for detecting potentially deleted primitives based on whether the "Update data" action was used rather than comparing the bounding boxes. Fixes a bug where area optimization would confuse the bounding box comparison (#24717). --- .../josm/actions/UpdateDataAction.java | 29 +----------- .../actions/downloadtasks/DownloadTaskList.java | 52 +++++++++++++++++++--- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/src/org/openstreetmap/josm/actions/UpdateDataAction.java b/src/org/openstreetmap/josm/actions/UpdateDataAction.java index 35f375054c..eabb643b5a 100644 --- a/src/org/openstreetmap/josm/actions/UpdateDataAction.java +++ b/src/org/openstreetmap/josm/actions/UpdateDataAction.java @@ -6,8 +6,6 @@ import static org.openstreetmap.josm.tools.I18n.tr; import java.awt.event.ActionEvent; import java.awt.event.KeyEvent; -import java.awt.geom.Area; -import java.util.List; import java.util.concurrent.Future; import java.util.stream.Collectors; @@ -61,30 +59,7 @@ public class UpdateDataAction extends JosmAction { if (!isEnabled() || editLayer == null || !editLayer.isDownloadable()) return; - List areas = editLayer.data.getDataSources().stream() - .map(ds -> new Area(ds.bounds.asRect())) - .collect(Collectors.toList()); - - // The next two blocks removes every intersection from every DataSource Area - // This prevents downloading the same data numerous times at intersections - // and also skips smaller bounding boxes that are contained within larger ones entirely. - for (int i = 0; i < areas.size(); i++) { - for (int j = i+1; j < areas.size(); j++) { - areas.get(i).subtract(areas.get(j)); - } - } - - for (int i = areas.size()-1; i > 0; i--) { - for (int j = i-1; j > 0; j--) { - areas.get(i).subtract(areas.get(j)); - } - } - - List areasToDownload = areas.stream() - .filter(a -> !a.isEmpty()) - .collect(Collectors.toList()); - - if (areasToDownload.isEmpty()) { + if (editLayer.data.getDataSources().isEmpty()) { // no bounds defined in the dataset? We update all but the incomplete primitives in the data set UpdateSelectionAction.updatePrimitives(editLayer.data.allPrimitives().stream() .filter(p -> !p.isNew() && !p.isIncomplete()).collect(Collectors.toList())); @@ -92,7 +67,7 @@ public class UpdateDataAction extends JosmAction { // bounds defined? => use the bbox downloader final PleaseWaitProgressMonitor monitor = new PleaseWaitProgressMonitor(tr("Download data")); final Future future = new DownloadTaskList(Config.getPref().getBoolean("update.data.zoom-after-download")) - .download(false /* no new layer */, areasToDownload, true, false, monitor); + .fullUpdate(monitor); waitFuture(future, monitor); } } diff --git a/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTaskList.java b/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTaskList.java index f8369409b1..73b56326fa 100644 --- a/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTaskList.java +++ b/src/org/openstreetmap/josm/actions/downloadtasks/DownloadTaskList.java @@ -47,6 +47,7 @@ import org.openstreetmap.josm.tools.Utils; public class DownloadTaskList { private final List tasks = new LinkedList<>(); private final List> taskFutures = new LinkedList<>(); + private boolean isFullUpdate; private final boolean zoomAfterDownload; private ProgressMonitor progressMonitor; @@ -131,6 +132,51 @@ public class DownloadTaskList { } } + /** + * Fully updates the current edit layer by re-downloading all the layer's + * declared bounding boxes from the OSM server. + * + * For full updates, this method should be preferred to the {@code download} + * methods because it uses a more effective algorithm to detect primitives that + * may have been deleted from the server; see #24717. + * + * @param progressMonitor The progress monitor + * @return The Future representing the asynchronous download task + */ + public Future fullUpdate(ProgressMonitor progressMonitor) { + OsmDataLayer editLayer = MainApplication.getLayerManager().getEditLayer(); + if (editLayer == null || !editLayer.isDownloadable()) + throw new IllegalStateException("The current edit layer is not downloadable."); + if (editLayer.data.getDataSources().isEmpty()) + throw new IllegalStateException("The current edit layer has no declared bounds."); + + List areas = editLayer.data.getDataSources().stream() + .map(ds -> new Area(ds.bounds.asRect())) + .collect(Collectors.toList()); + + // The next two blocks removes every intersection from every DataSource Area + // This prevents downloading the same data numerous times at intersections + // and also skips smaller bounding boxes that are contained within larger ones entirely. + for (int i = 0; i < areas.size(); i++) { + for (int j = i+1; j < areas.size(); j++) { + areas.get(i).subtract(areas.get(j)); + } + } + + for (int i = areas.size()-1; i > 0; i--) { + for (int j = i-1; j > 0; j--) { + areas.get(i).subtract(areas.get(j)); + } + } + + List areasToDownload = areas.stream() + .filter(a -> !a.isEmpty()) + .collect(Collectors.toList()); + + isFullUpdate = true; + return download(false /* no new layer */, areasToDownload, true, false, progressMonitor); + } + /** * Replies the set of ids of all complete, non-new primitives (i.e. those with !primitive.incomplete) * @param ds data set @@ -275,12 +321,8 @@ public class DownloadTaskList { .filter(DownloadOsmTask.class::isInstance).map(DownloadOsmTask.class::cast) .filter(t -> t.getDownloadedData() != null) .collect(Collectors.toList()); - final Set tasksBounds = osmTasks.stream() - .flatMap(t -> t.getDownloadedData().getDataSourceBounds().stream()) - .collect(Collectors.toSet()); - final Set layerBounds = new LinkedHashSet<>(editDataSet.getDataSourceBounds()); final Set myPrimitives = new LinkedHashSet<>(); - if (layerBounds.equals(tasksBounds)) { + if (isFullUpdate) { // the full edit layer is updated (we have downloaded again all its current bounds) myPrimitives.addAll(getCompletePrimitives(editDataSet)); for (DownloadOsmTask task : osmTasks) {