Skip to content

Commit

Permalink
Fix #22839: NPE at AbstractDownloadStrategy.getExisting()
Browse files Browse the repository at this point in the history
This is likely due to a race condition where a user deletes a layer just before
the download tasks start. This removes the possibility of getting an NPE with
`MainApplication.getMap().mapView` and `mapView.getLayerManager()`.
  • Loading branch information
tsmock committed May 16, 2023
1 parent e5933f1 commit 83fccdd
Showing 1 changed file with 12 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Locale;
import java.util.Optional;
import java.util.concurrent.Future;

import org.openstreetmap.josm.actions.downloadtasks.AbstractDownloadTask;
Expand All @@ -14,8 +15,10 @@
import org.openstreetmap.josm.data.Bounds;
import org.openstreetmap.josm.data.coor.LatLon;
import org.openstreetmap.josm.gui.MainApplication;
import org.openstreetmap.josm.gui.MapView;
import org.openstreetmap.josm.gui.layer.GpxLayer;
import org.openstreetmap.josm.gui.layer.Layer;
import org.openstreetmap.josm.gui.layer.MainLayerManager;
import org.openstreetmap.josm.gui.layer.OsmDataLayer;
import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
import org.openstreetmap.josm.gui.progress.ProgressMonitor;
Expand Down Expand Up @@ -109,12 +112,17 @@ private static Bounds intersection(Bounds box1, Bounds box2) {
* @return The collection of bounds that have already been downloaded.
*/
private static Collection<Bounds> getExisting(Class<?> klass) {
// The code used to use MainApplication.getMap().mapView.getLayerManager()
// That layer manager is almost always the same as MainApplication.getLayerManager()
// Regardless, keep the original code just in case.
final MainLayerManager layerManager = Optional.ofNullable(MainApplication.getMap()).map(map -> map.mapView)
.map(MapView::getLayerManager).orElseGet(MainApplication::getLayerManager);
if (klass.isAssignableFrom(OsmDataLayer.class)) {
if (!MainApplication.isDisplayingMapView())
return Collections.emptyList();
OsmDataLayer layer = MainApplication.getMap().mapView.getLayerManager().getEditLayer();
OsmDataLayer layer = layerManager.getEditLayer();
if (layer == null) {
Collection<Layer> layers = MainApplication.getMap().mapView.getLayerManager().getLayers();
Collection<Layer> layers = layerManager.getLayers();
for (Layer layer1 : layers) {
if (layer1 instanceof OsmDataLayer)
return ((OsmDataLayer) layer1).data.getDataSourceBounds();
Expand All @@ -127,10 +135,10 @@ private static Collection<Bounds> getExisting(Class<?> klass) {
if (!MainApplication.isDisplayingMapView())
return Collections.emptyList();
boolean merge = Config.getPref().getBoolean("download.gps.mergeWithLocal", false);
Layer active = MainApplication.getMap().mapView.getLayerManager().getActiveLayer();
Layer active = layerManager.getActiveLayer();
if (active instanceof GpxLayer && (merge || ((GpxLayer) active).data.fromServer))
return ((GpxLayer) active).data.getDataSourceBounds();
for (GpxLayer l : MainApplication.getMap().mapView.getLayerManager().getLayersOfType(GpxLayer.class)) {
for (GpxLayer l : layerManager.getLayersOfType(GpxLayer.class)) {
if (merge || l.data.fromServer)
return l.data.getDataSourceBounds();
}
Expand Down

0 comments on commit 83fccdd

Please sign in to comment.