See #24875: Don't use sources without a URL, survive a dead overture catalog

The default source list currently fails to load at all: the overture catalog
referenced from sources.json returns 403, and that IOException discarded every
default source. The preferences then only show the "Loading" placeholder row,
which has an empty URL but still counted as valid, so enabling it stored it as
a source and every download failed with
"MalformedURLException: no protocol:".

* A blank URL is no longer a valid URL (MapWithAIInfo#hasValidUrl)
* MapWithAILayerInfo#add ignores sources without a URL; this also drops
  already-stored broken entries on load
* The placeholder row can no longer be enabled/activated
* Downloads skip sources without a URL instead of throwing
* A failing overture catalog only drops the overture sources

Author: watmildon

src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtils.java

@@ -172,6 +172,10 @@ public static DataSet getData(Collection<Bounds> bounds, int maximumDimensions)
     public static ForkJoinTask<DataSet> download(ProgressMonitor monitor, Bounds bound, MapWithAIInfo mapWithAIInfo,
             int maximumDimensions) {
         return ForkJoinTask.adapt(() -> {
+            if (Utils.isStripEmpty(mapWithAIInfo.getUrlExpanded())) {
+                Logging.warn("MapWithAI: Skipping source without a URL: {0}", mapWithAIInfo.getName());
+                return new DataSet();
+            }
             final var downloader = new BoundingBoxMapWithAIDownloader(bound, mapWithAIInfo,
                     DetectTaskingManagerUtils.hasTaskingManagerLayer());
             try {

src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfo.java

@@ -485,7 +485,7 @@ public List<String> getConflationParameterString() {
      * @return {@code true} if this source will have a valid url
      */
     public boolean hasValidUrl() {
-        return this.url != null || (this.isConflated() && this.conflationUrl != null);
+        return !Utils.isStripEmpty(this.url) || (this.isConflated() && !Utils.isStripEmpty(this.conflationUrl));
     }
 
     public String getUrlExpanded() {

src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAILayerInfo.java

@@ -369,14 +369,20 @@ private void updateEsriLayers(@Nonnull final Collection<MapWithAIInfo> layers) {
         /**
          * Update the overture layers
          * @param layers The layers to iterate through and modify
-         * @throws IOException If something happens while parsing overture layers
          */
-        private void updateOvertureLayers(@Nonnull final Collection<MapWithAIInfo> layers) throws IOException {
+        private void updateOvertureLayers(@Nonnull final Collection<MapWithAIInfo> layers) {
             final var overtureLayers = new ArrayList<MapWithAIInfo>(4);
             for (var layer : layers) {
                 if (MapWithAIType.OVERTURE == layer.getSourceType()) {
                     try (var reader = new OvertureSourceReader(layer)) {
+                        reader.setFastFail(this.fastFail);
                         reader.parse().ifPresent(overtureLayers::addAll);
+                    } catch (IOException e) {
+                        // See #24875: an unavailable catalog should only drop its own layers, not
+                        // every default source.
+                        Logging.warn("MapWithAI: Could not load overture catalog {0}: {1}", layer.getUrl(),
+                                e.getMessage());
+                        Logging.trace(e);
                     }
                 }
             }
@@ -576,6 +582,12 @@ private static boolean isSimilar(String a, String b) {
      * @param info imagery entry to add
      */
     public void add(MapWithAIInfo info) {
+        if (info == null || !info.hasValidUrl()) {
+            // See #24875: the "Loading" placeholder (or a broken preference entry) must
+            // never become a source
+            Logging.warn("MapWithAI: Ignoring source without a URL: {0}", info == null ? null : info.getName());
+            return;
+        }
         layers.add(info);
         this.listeners.fireEvent(l -> l.changeEvent(info));
     }

src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIProvidersPanel.java

@@ -514,6 +514,10 @@ private static void clickListener(MouseEvent e) {
                 }
             } else if (tr("Enabled").equals(tableName)) {
                 final var info = MapWithAIDefaultLayerTableModel.getRow(realRow);
+                if (!info.hasValidUrl()) {
+                    // The "Loading" placeholder
+                    return;
+                }
                 final var instance = MapWithAILayerInfo.getInstance();
                 if (instance.getLayers().contains(info)) {
                     instance.remove(info);
@@ -823,7 +827,8 @@ public void actionPerformed(ActionEvent e) {
                 return;
             }
             final var selected = Arrays.stream(defaultTable.getSelectedRows()).map(defaultTable::convertRowIndexToModel)
-                    .mapToObj(MapWithAIDefaultLayerTableModel::getRow).collect(Collectors.toCollection(ArrayList::new));
+                    .mapToObj(MapWithAIDefaultLayerTableModel::getRow).filter(MapWithAIInfo::hasValidUrl)
+                    .collect(Collectors.toCollection(ArrayList::new));
             if (selected.stream().anyMatch(MapWithAILayerTableModel::doesNotContain)) {
                 final var toAdd = selected.stream().filter(MapWithAILayerTableModel::doesNotContain).toList();
                 activeTable.getSelectionModel().clearSelection();

src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java

@@ -3,6 +3,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -198,4 +199,22 @@ public List<MapWithAIInfo> getAllDefaultLayers() {
         assertSame(info, MapWithAILayerInfo.getInstance().getLayers().get(0));
         assertEquals("22", info.getId());
     }
+
+    /**
+     * Non-regression test for #24875: a source with an empty url (like the
+     * "Loading" placeholder in the preferences) must not be usable as a source.
+     */
+    @Test
+    void testTicket24875EmptyUrl() {
+        final var placeholder = new MapWithAIInfo("Loading", "");
+        assertFalse(placeholder.hasValidUrl());
+        assertFalse(new MapWithAIInfo("Blank", "  ").hasValidUrl());
+        assertFalse(new MapWithAIInfo("Null").hasValidUrl());
+        assertTrue(new MapWithAIInfo("Real", "https://test.example").hasValidUrl());
+
+        final var layerInfo = MapWithAILayerInfo.getInstance();
+        layerInfo.clear();
+        layerInfo.add(placeholder);
+        assertTrue(layerInfo.getLayers().isEmpty());
+    }
 }