From 19ccf18472593b6e653888e370a0313134e441b1 Mon Sep 17 00:00:00 2001 From: Robert Scott Date: Sun, 30 Sep 2018 12:33:27 +0100 Subject: [PATCH v1 2/4] SlippyMapBBoxChooser: redesign getAllTileSources to perform deduplication of provided tile sources this also enables us to easily ensure currently active TileSource remains present & selected in the source menu even when it's no longer supplied by TileSourceProviders --- .../josm/gui/bbox/SlippyMapBBoxChooser.java | 45 +++++++++++++--------- .../josm/gui/dialogs/MinimapDialogTest.java | 35 +++++++++++++++++ 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/org/openstreetmap/josm/gui/bbox/SlippyMapBBoxChooser.java b/src/org/openstreetmap/josm/gui/bbox/SlippyMapBBoxChooser.java index 907a5152f..4b322854d 100644 --- a/src/org/openstreetmap/josm/gui/bbox/SlippyMapBBoxChooser.java +++ b/src/org/openstreetmap/josm/gui/bbox/SlippyMapBBoxChooser.java @@ -16,11 +16,13 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import javax.swing.ButtonModel; import javax.swing.JOptionPane; @@ -78,20 +80,12 @@ public class SlippyMapBBoxChooser extends JMapViewer implements BBoxChooser, Cha * TMS TileSource provider for the slippymap chooser */ public static class TMSTileSourceProvider implements TileSourceProvider { - private static final Set existingSlippyMapUrls = new HashSet<>(); - static { - // Urls that already exist in the slippymap chooser and shouldn't be copied from TMS layer list - existingSlippyMapUrls.add("https://{switch:a,b,c}.tile.openstreetmap.org/{zoom}/{x}/{y}.png"); // Mapnik - } @Override public List getTileSources() { if (!TMSLayer.PROP_ADD_TO_SLIPPYMAP_CHOOSER.get()) return Collections.emptyList(); List sources = new ArrayList<>(); for (ImageryInfo info : ImageryLayerInfo.instance.getLayers()) { - if (existingSlippyMapUrls.contains(info.getUrl())) { - continue; - } try { TileSource source = TMSLayer.getTileSourceStatic(info); if (source != null) { @@ -177,7 +171,7 @@ public class SlippyMapBBoxChooser extends JMapViewer implements BBoxChooser, Cha } setMaxTilesInMemory(Config.getPref().getInt("slippy_map_chooser.max_tiles", 1000)); - List tileSources = getAllTileSources(); + List tileSources = new ArrayList<>(getAllTileSources().values()); this.showDownloadAreaButtonModel = new JToggleButton.ToggleButtonModel(); this.showDownloadAreaButtonModel.setSelected(PROP_SHOWDLAREA.get()); @@ -210,12 +204,16 @@ public class SlippyMapBBoxChooser extends JMapViewer implements BBoxChooser, Cha new SlippyMapControler(this, this); } - private static List getAllTileSources() { - List tileSources = new ArrayList<>(); - for (TileSourceProvider provider: providers) { - tileSources.addAll(provider.getTileSources()); - } - return tileSources; + private static LinkedHashMap getAllTileSources() { + // using a LinkedHashMap of to retain ordering but provide deduplication + return providers.stream().flatMap( + provider -> provider.getTileSources().stream() + ).collect(Collectors.toMap( + TileSource::getId, + ts -> ts, + (old, _new) -> old, + () -> new LinkedHashMap() + )); } /** @@ -358,9 +356,12 @@ public class SlippyMapBBoxChooser extends JMapViewer implements BBoxChooser, Cha this.tileController.setTileCache(new MemoryTileCache()); this.setTileSource(tileSource); PROP_MAPSTYLE.put(tileSource.getName()); // TODO Is name really unique? - if (this.iSourceButton.getCurrentSource() != tileSource) { // prevent infinite recursion - this.iSourceButton.setCurrentMap(tileSource); - } + + // we need to refresh the tile sources in case the deselected source should no longer be present + // (and only remained there because its removal was deferred while the source was still the + // selected one). this should also have the effect of propagating the new selection to the + // iSourceButton & menu: it attempts to re-select the current source when rebuilding its menu. + this.refreshTileSources(); } @Override @@ -416,6 +417,12 @@ public class SlippyMapBBoxChooser extends JMapViewer implements BBoxChooser, Cha * @since 6364 */ public final void refreshTileSources() { - iSourceButton.setSources(getAllTileSources()); + final LinkedHashMap newTileSources = getAllTileSources(); + final TileSource currentTileSource = this.getTileController().getTileSource(); + + // re-add the currently active TileSource to prevent inconsistent display of menu + newTileSources.putIfAbsent(currentTileSource.getId(), currentTileSource); + + this.iSourceButton.setSources(new ArrayList<>(newTileSources.values())); } } diff --git a/test/unit/org/openstreetmap/josm/gui/dialogs/MinimapDialogTest.java b/test/unit/org/openstreetmap/josm/gui/dialogs/MinimapDialogTest.java index f8a61a41b..9d9fa63e3 100644 --- a/test/unit/org/openstreetmap/josm/gui/dialogs/MinimapDialogTest.java +++ b/test/unit/org/openstreetmap/josm/gui/dialogs/MinimapDialogTest.java @@ -30,6 +30,7 @@ import org.openstreetmap.josm.TestUtils; import org.openstreetmap.josm.data.Bounds; import org.openstreetmap.josm.data.DataSource; import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.imagery.ImageryLayerInfo; import org.openstreetmap.josm.data.projection.ProjectionRegistry; import org.openstreetmap.josm.data.projection.Projections; import org.openstreetmap.josm.gui.MainApplication; @@ -257,6 +258,40 @@ public class MinimapDialogTest { } /** + * Tests that the currently selected source being removed from ImageryLayerInfo will remain present and + * selected in the source menu even after the tile sources have been refreshed. + * @throws Exception if any error occurs + */ + @Test + public void testRemovedSourceStillSelected() throws Exception { + // relevant prefs starting out empty, should choose the first source and have shown download area enabled + // (not that there's a data layer for it to use) + + this.setUpMiniMap(); + + this.clickSourceMenuItemByLabel("Green Tiles"); + + ImageryLayerInfo.instance.remove( + ImageryLayerInfo.instance.getLayers().stream().filter(i -> i.getName().equals("Green Tiles")).findAny().get() + ); + + this.assertSingleSelectedSourceLabel("Green Tiles"); + + this.slippyMap.refreshTileSources(); + + this.assertSingleSelectedSourceLabel("Green Tiles"); + + // call paint to trigger new tile fetch + this.paintSlippyMap(); + + Awaitility.await().atMost(1000, MILLISECONDS).until(this.slippyMapTasksFinished); + + this.paintSlippyMap(); + + assertEquals(0xff00ff00, paintedSlippyMap.getRGB(0, 0)); + } + + /** * Tests minimap obeys a saved "mapstyle" preference on startup. * @throws Exception if any error occurs */ -- 2.11.0