Fix/24875 empty source url

.github/workflows/ant.yml

@@ -20,6 +20,8 @@ jobs:
     uses: JOSM/JOSMPluginAction/.github/workflows/ant.yml@v3
     with:
       java-version: 17
+      # The workflow default (1.10.15) is no longer on downloads.apache.org
+      ant-version: apache-ant-1.10.18
       josm-revision: ${{ matrix.josm-revision }}
       plugin-jar-name: 'mapwithai'
       perform-revision-tagging: ${{ matrix.josm-revision == 'r19067' && github.repository == 'JOSM/MapWithAI' && github.ref_type == 'branch' && github.ref_name == 'master' && github.event_name != 'schedule' && github.event_name != 'pull_request' }}

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

@@ -172,6 +172,10 @@
     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 {
@@ -248,7 +252,7 @@
      */
     public static ForkJoinPool getForkJoinPool() {
         if (requiresForkJoinPool == null) {
             requiresForkJoinPool = Utils.isRunningWebStart() || System.getSecurityManager() != null;
         }
         if (requiresForkJoinPool) {
             synchronized (MapWithAIDataUtils.class) {

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

@@ -188,7 +188,7 @@
         }
         // Ensure that the cache is initialized prior to running in the fork join pool
         // on webstart
         if (System.getSecurityManager() != null) {
             Logging.trace("MapWithAI loaded: {0}", ESRISourceReader.SOURCE_CACHE.getClass());
         }
         loadDefaults(false, MapWithAIDataUtils.getForkJoinPool(), fastFail, listener);
@@ -369,14 +369,20 @@
         /**
          * 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 @@
      * @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/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/CommonSourceReader.java

@@ -10,6 +10,7 @@
 import org.openstreetmap.josm.tools.Utils;
 
 import jakarta.json.Json;
+import jakarta.json.JsonObject;
 import jakarta.json.stream.JsonParser;
 import jakarta.json.stream.JsonParsingException;
 
@@ -54,6 +55,31 @@ public Optional<T> parse() throws IOException {
         return Optional.empty();
     }
 
+    /**
+     * Read a JSON object from another URL, using the same cache settings as the
+     * main source.
+     *
+     * @param url The url to read
+     * @return The object, or {@code null} if the document is not a JSON object
+     * @throws IOException if any I/O error occurs
+     */
+    protected JsonObject readObject(String url) throws IOException {
+        final var file = new CachedFile(url).setMaxAge(CachedFile.DAYS)
+                .setCachingStrategy(CachedFile.CachingStrategy.IfModifiedSince);
+        if (this.clearCache) {
+            file.clear();
+        }
+        file.setFastFail(this.fastFail);
+        try (file; JsonParser reader = Json.createParser(file.getContentReader())) {
+            if (reader.hasNext() && reader.next() == JsonParser.Event.START_OBJECT) {
+                return reader.getObject();
+            }
+        } catch (JsonParsingException jsonParsingException) {
+            Logging.error(jsonParsingException);
+        }
+        return null;
+    }
+
     /**
      * Parses MapWithAI entry sources
      *

src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/OvertureSourceReader.java

@@ -44,9 +44,88 @@ public List<MapWithAIInfo> parseJson(JsonParser jsonParser) {
         if (jsonObject.containsKey("releases")) {
             return parseRoot(jsonObject);
         }
+        // The STAC catalog (https://stac.overturemaps.org/catalog.json), see #24875
+        if ("Catalog".equals(jsonObject.getString("type", null)) && jsonObject.containsKey("links")) {
+            return parseStacRoot(jsonObject);
+        }
         return Collections.emptyList();
     }
 
+    /**
+     * Create the sources for the latest release from the STAC catalog. The root
+     * catalog links to the releases, a release links to its themes, and a theme
+     * links to its tiles ({@code "rel": "pmtiles"}).
+     * <p>
+     * Overture only keeps the last two releases, so the sources keep the same id
+     * between releases. This means that user entries are updated to the new
+     * release instead of being dropped.
+     *
+     * @param root The root catalog
+     * @return The sources for the latest release
+     */
+    private List<MapWithAIInfo> parseStacRoot(JsonObject root) {
+        final var latest = root.getString("latest", null);
+        final var releases = getLinks(root, "child").toList();
+        var release = latest == null ? null
+                : releases.stream().filter(link -> link.getString("href").contains('/' + latest + '/')).findFirst()
+                        .orElse(null);
+        if (release == null) {
+            release = releases.stream().filter(link -> link.getBoolean("latest", false)).findFirst().orElse(null);
+        }
+        if (release == null) {
+            Logging.warn("MapWithAI: No overture release found in {0}", this.source.getUrl());
+            return Collections.emptyList();
+        }
+        // The title is "<release> Overture Release"
+        final var releaseId = latest != null ? latest : release.getString("title", "").split(" ", 2)[0];
+        final var info = new ArrayList<MapWithAIInfo>(6);
+        try {
+            final var releaseCatalog = readObject(release.getString("href"));
+            if (releaseCatalog == null) {
+                return info;
+            }
+            for (var theme : getLinks(releaseCatalog, "child").toList()) {
+                final var themeCatalog = readObject(theme.getString("href"));
+                if (themeCatalog == null) {
+                    continue;
+                }
+                final var themeId = themeCatalog.getString("id", theme.getString("title", ""));
+                final var tiles = getLinks(themeCatalog, "pmtiles").findFirst();
+                if (tiles.isEmpty()) {
+                    Logging.warn("MapWithAI: Overture theme {0} has no tiles in release {1}", themeId, releaseId);
+                    continue;
+                }
+                final var themeInfo = buildSource(URI.create(tiles.get().getString("href")), releaseId, themeId);
+                if (themeInfo != null) {
+                    themeInfo.setId(this.source.getName() + ": " + themeId);
+                    info.add(themeInfo);
+                }
+            }
+        } catch (IOException | IllegalArgumentException e) {
+            Logging.warn("MapWithAI: Could not read the overture catalog for release {0}: {1}", releaseId,
+                    e.getMessage());
+            Logging.trace(e);
+        }
+        return info;
+    }
+
+    /**
+     * Get the links of a STAC object with a specific relation
+     *
+     * @param stac The STAC object (catalog or collection)
+     * @param rel  The relation to look for
+     * @return The links with an {@code href}
+     */
+    private static Stream<JsonObject> getLinks(JsonObject stac, String rel) {
+        final var links = stac.get("links");
+        if (links instanceof JsonArray array) {
+            return array.stream().filter(JsonObject.class::isInstance).map(JsonObject.class::cast)
+                    .filter(link -> rel.equals(link.getString("rel", null)) && link.containsKey("href")
+                            && link.get("href").getValueType() == JsonValue.ValueType.STRING);
+        }
+        return Stream.empty();
+    }
+
     private List<MapWithAIInfo> parseRoot(JsonObject jsonObject) {
         final var info = new ArrayList<MapWithAIInfo>(6 * 4);
         final var releases = jsonObject.get("releases");
@@ -123,24 +202,34 @@ private MapWithAIInfo buildSource(URI uri, String releaseId, String theme) {
         info.setId(info.getName());
         if (uri.getPath().endsWith(".pmtiles")) {
             info.setSourceType(MapWithAIType.PMTILES);
-            // Set additional information
-            try {
-                final var header = PMTiles.readHeader(uri);
-                final var metadata = PMTiles.readMetadata(header);
-                final var bounds = new Bounds(header.minLatitude(), header.minLongitude(), header.maxLatitude(),
-                        header.maxLongitude());
-                info.setBounds(new ImageryInfo.ImageryBounds(bounds.encodeAsString(","), ","));
-                if (metadata.containsKey("name") && metadata.get("name")instanceof JsonString name) {
-                    info.setName(name.getString() + " - " + releaseId);
-                }
-                if (metadata.containsKey("description")
-                        && metadata.get("description")instanceof JsonString description) {
-                    info.setDescription(description.getString());
-                }
-            } catch (IOException ioException) {
-                Logging.error(ioException);
-            }
+            readTileInformation(info, uri, releaseId);
         }
         return info;
     }
+
+    /**
+     * Read additional information (bounds, name, description) from the tiles
+     *
+     * @param info      The info to update
+     * @param uri       The location of the tiles
+     * @param releaseId The release id
+     */
+    void readTileInformation(MapWithAIInfo info, URI uri, String releaseId) {
+        try {
+            final var header = PMTiles.readHeader(uri);
+            final var metadata = PMTiles.readMetadata(header);
+            final var bounds = new Bounds(header.minLatitude(), header.minLongitude(), header.maxLatitude(),
+                    header.maxLongitude());
+            info.setBounds(new ImageryInfo.ImageryBounds(bounds.encodeAsString(","), ","));
+            if (metadata.containsKey("name") && metadata.get("name")instanceof JsonString name) {
+                info.setName(name.getString() + " - " + releaseId);
+            }
+            if (metadata.containsKey("description")
+                    && metadata.get("description")instanceof JsonString description) {
+                info.setDescription(description.getString());
+            }
+        } catch (IOException ioException) {
+            Logging.error(ioException);
+        }
+    }
 }

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());
+    }
 }