diff --git a/.github/workflows/ant.yml b/.github/workflows/ant.yml index d2fcbee3..413caaa1 100644 --- a/.github/workflows/ant.yml +++ b/.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' }} diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtils.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtils.java index c5977de3..4988da13 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtils.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtils.java @@ -172,6 +172,10 @@ public static DataSet getData(Collection bounds, int maximumDimensions) public static ForkJoinTask 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 { diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfo.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfo.java index efab8038..a93a14e5 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfo.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfo.java @@ -485,7 +485,7 @@ public List 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() { diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAILayerInfo.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAILayerInfo.java index 919fcfb2..94a20245 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAILayerInfo.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAILayerInfo.java @@ -369,14 +369,20 @@ private void updateEsriLayers(@Nonnull final Collection 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 layers) throws IOException { + private void updateOvertureLayers(@Nonnull final Collection layers) { final var overtureLayers = new ArrayList(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)); } diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIProvidersPanel.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIProvidersPanel.java index 7eef587f..1f7b1877 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIProvidersPanel.java +++ b/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(); diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/CommonSourceReader.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/CommonSourceReader.java index b7430017..8d040f29 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/CommonSourceReader.java +++ b/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 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 * diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/OvertureSourceReader.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/OvertureSourceReader.java index 6c51b50d..dcbf97ae 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/OvertureSourceReader.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/OvertureSourceReader.java @@ -44,9 +44,88 @@ public List 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"}). + *

+ * 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 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 " Overture Release" + final var releaseId = latest != null ? latest : release.getString("title", "").split(" ", 2)[0]; + final var info = new ArrayList(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 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 parseRoot(JsonObject jsonObject) { final var info = new ArrayList(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); + } + } } diff --git a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java index 0cee8929..56bc25a8 100644 --- a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java +++ b/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 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()); + } } diff --git a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/OvertureSourceReaderTest.java b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/OvertureSourceReaderTest.java new file mode 100644 index 00000000..8730f23c --- /dev/null +++ b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/OvertureSourceReaderTest.java @@ -0,0 +1,121 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.plugins.mapwithai.io.mapwithai; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.io.StringReader; +import java.net.URI; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAICategory; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIType; +import org.openstreetmap.josm.testutils.annotations.BasicPreferences; + +import jakarta.json.Json; +import jakarta.json.JsonObject; + +/** + * Test class for {@link OvertureSourceReader} + */ +@BasicPreferences +class OvertureSourceReaderTest { + private static final String STAC = "https://stac.overturemaps.org/"; + private static final String ROOT = """ + {"type":"Catalog","id":"Overture Releases","stac_version":"1.1.0","latest":"2026-09-23.1", + "links":[{"rel":"child","href":"https://stac.overturemaps.org/2026-08-19.0/catalog.json","title":"2026-08-19.0 Overture Release"}, + {"rel":"child","href":"https://stac.overturemaps.org/2026-09-23.1/catalog.json","title":"2026-09-23.1 Overture Release","latest":true}, + {"rel":"self","href":"https://stac.overturemaps.org/catalog.json"}]}"""; + private static final String RELEASE = """ + {"type":"Catalog","id":"2026-09-23.1","stac_version":"1.1.0", + "links":[{"rel":"root","href":"https://stac.overturemaps.org/catalog.json"}, + {"rel":"child","href":"https://stac.overturemaps.org/2026-09-23.1/addresses/catalog.json","title":"addresses"}, + {"rel":"child","href":"https://stac.overturemaps.org/2026-09-23.1/buildings/catalog.json","title":"buildings"}, + {"rel":"child","href":"https://stac.overturemaps.org/2026-09-23.1/divisions/catalog.json","title":"divisions"}, + {"rel":"child","href":"https://stac.overturemaps.org/2026-09-23.1/transportation/catalog.json","title":"transportation"}]}"""; + + private static String theme(String theme, boolean tiles) { + return "{\"type\":\"Catalog\",\"id\":\"" + theme + "\",\"links\":[{\"rel\":\"root\",\"href\":\"" + STAC + + "catalog.json\"}" + (tiles ? ",{\"rel\":\"pmtiles\",\"href\":\"https://tiles.overturemaps.org/2026-09-23.1/" + + theme + ".pmtiles\",\"type\":\"application/vnd.pmtiles\"}" : "") + + "]}"; + } + + private static final Map CATALOGS = Map.of(STAC + "2026-09-23.1/catalog.json", RELEASE, + STAC + "2026-09-23.1/addresses/catalog.json", theme("addresses", true), + STAC + "2026-09-23.1/buildings/catalog.json", theme("buildings", true), + STAC + "2026-09-23.1/divisions/catalog.json", theme("divisions", false), + STAC + "2026-09-23.1/transportation/catalog.json", theme("transportation", true)); + + private static List parse(String json) throws IOException { + final var source = new MapWithAIInfo("Overture", STAC + "catalog.json"); + source.setSourceType(MapWithAIType.OVERTURE); + try (var reader = new OvertureSourceReader(source) { + @Override + protected JsonObject readObject(String url) throws IOException { + // Serve the canned catalogs instead of going to the network + if (!CATALOGS.containsKey(url)) { + throw new IOException("Unexpected url: " + url); + } + try (var parser = Json.createParser(new StringReader(CATALOGS.get(url)))) { + parser.next(); + return parser.getObject(); + } + } + + @Override + void readTileInformation(MapWithAIInfo info, URI uri, String releaseId) { + // Don't read the (remote) pmtiles headers in tests + } + }; var sr = new StringReader(json); var parser = Json.createParser(sr)) { + parser.next(); + return reader.parseJson(parser); + } + } + + /** + * Non-regression test for #24875: the old pmtiles catalog no longer exists. Use + * the STAC catalog instead. + */ + @Test + void testStacCatalog() throws IOException { + final var infos = parse(ROOT); + // divisions has no tiles, transportation is skipped + assertEquals(2, infos.size(), infos.toString()); + for (var info : infos) { + assertEquals(MapWithAIType.PMTILES, info.getSourceType()); + assertTrue(info.hasValidUrl()); + assertTrue(info.getName().contains("2026-09-23.1"), info.getName()); + } + final var buildings = infos.stream().filter(i -> i.getUrl().endsWith("/buildings.pmtiles")).findFirst() + .orElseThrow(); + assertEquals("https://tiles.overturemaps.org/2026-09-23.1/buildings.pmtiles", buildings.getUrl()); + assertEquals(MapWithAICategory.BUILDING, buildings.getCategory()); + // The id must not change between releases + assertEquals("Overture: buildings", buildings.getId()); + final var addresses = infos.stream().filter(i -> i.getUrl().endsWith("/addresses.pmtiles")).findFirst() + .orElseThrow(); + assertEquals(MapWithAICategory.ADDRESS, addresses.getCategory()); + } + + /** + * The {@code latest} field is optional in STAC; fall back to the link marked as + * latest. + */ + @Test + void testStacCatalogWithoutLatest() throws IOException { + assertEquals(2, parse(ROOT.replace("\"latest\":\"2026-09-23.1\",", "")).size()); + } + + @Test + void testBadCatalog() throws IOException { + assertTrue(parse("{\"type\":\"Catalog\"}").isEmpty()); + assertTrue(parse("{\"type\":\"Catalog\",\"links\":[]}").isEmpty()); + assertTrue(parse("{\"type\":\"Catalog\",\"latest\":\"2000-01-01.0\",\"links\":[{\"rel\":\"child\"," + + "\"href\":\"https://stac.overturemaps.org/2000-01-01.0/catalog.json\"}]}").isEmpty()); + } +}