Fix deprecated natural earth source

CHANGELOG.md

@@ -1,3 +1,7 @@
+Tiles 4.15.1
+------
+- Change to generic Geopackage source for Natural Earth via @mxzinke and @wipfli [#626]
+
 Tiles 4.15.0
 ------
 - derive road shields from route relations instead of way `ref` tags

tiles/src/main/java/com/protomaps/basemap/Basemap.java

@@ -134,7 +134,7 @@ public String description() {
 
   @Override
   public String version() {
-    return "4.15.0";
+    return "4.15.1";
   }
 
   @Override
@@ -284,8 +284,8 @@ static void run(Arguments args) throws IOException {
     }
 
     var planetiler = Planetiler.create(args)
-      .addNaturalEarthSource("ne", sourcesDir.resolve("natural_earth_vector.sqlite.zip"),
-        "https://naciscdn.org/naturalearth/packages/natural_earth_vector.sqlite.zip");
+      .addGeoPackageSource("ne", sourcesDir.resolve("natural_earth_vector.gpkg.zip"),
+        "https://naciscdn.org/naturalearth/packages/natural_earth_vector.gpkg.zip");
 
     if (!overtureFile.isEmpty()) {
       Path base = args.inputFile("overture", "overture base directory", Path.of("data", "overture"));

tiles/src/main/java/com/protomaps/basemap/layers/Boundaries.java

@@ -12,6 +12,7 @@
 import com.protomaps.basemap.feature.FeatureId;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Locale;
 import java.util.OptionalInt;
 
 @SuppressWarnings("java:S1192")
@@ -25,25 +26,40 @@ public String name() {
     return LAYER_NAME;
   }
 
+  /**
+   * Reads a Natural Earth attribute, falling back to the upper case spelling of the column.
+   * <p>
+   * Column name casing is not consistent across the Natural Earth distributions: the sqlite package is entirely lower
+   * case, while the GeoPackage package preserves whatever the source shapefiles use. That is lower case for most
+   * physical layers but upper case for the admin layers this handler reads, and some tables mix both.
+   */
+  private static String getNeString(SourceFeature sf, String key) {
+    Object value = sf.getTag(key);
+    if (value == null) {
+      value = sf.getTag(key.toUpperCase(Locale.ROOT));
+    }
+    return value == null ? null : value.toString();
+  }
+
   public void processNe(SourceFeature sf, FeatureCollector features) {
     String sourceLayer = sf.getSourceLayer();
     String kind = "";
     int adminLevel = 2;
     boolean disputed = false;
-    int themeMinZoom = 0;
     int themeMaxZoom = 0;
 
     if (sourceLayer.equals("ne_10m_admin_0_boundary_lines_land") ||
       sourceLayer.equals("ne_10m_admin_0_boundary_lines_map_units") ||
       sourceLayer.equals("ne_10m_admin_0_boundary_lines_disputed_areas") ||
       sourceLayer.equals("ne_10m_admin_1_states_provinces_lines")) {
-      themeMinZoom = 4;
       themeMaxZoom = 5;
       kind = "tz_boundary";
     }
 
+    String featureClass = getNeString(sf, "featurecla");
+
     if (!kind.isEmpty()) {
-      switch (sf.getString("featurecla")) {
+      switch (featureClass == null ? "" : featureClass) {
         case "Disputed (please verify)" -> {
           kind = "country";
           disputed = true;
@@ -130,19 +146,19 @@ public void processNe(SourceFeature sf, FeatureCollector features) {
       }
     }
 
-    if (sf.canBeLine() && sf.getString("min_zoom") != null && (!kind.isEmpty() && !kind.equals("tz_boundary"))) {
-      var minZoom = Double.parseDouble(sf.getString("min_zoom")) - 1.0;
+    String minZoomString = getNeString(sf, "min_zoom");
+
+    if (sf.canBeLine() && minZoomString != null && (!kind.isEmpty() && !kind.equals("tz_boundary"))) {
+      var minZoom = Double.parseDouble(minZoomString) - 1.0;
       int sortRank = 289 - (disputed ? 1 : 0);
       features.line(this.name())
         .setAttr("kind", kind)
         .setAttr("kind_detail", adminLevel)
         .setAttr("sort_rank", sortRank)
         .setSortKey(sortRank)
         .setAttr("disputed", disputed ? true : null)
-        .setAttr("brk_a3", sf.getString("brk_a3"))
-        .setZoomRange(
-          sf.getString("min_zoom") == null ? themeMinZoom : (int) minZoom,
-          themeMaxZoom)
+        .setAttr("brk_a3", getNeString(sf, "brk_a3"))
+        .setZoomRange((int) minZoom, themeMaxZoom)
         .setMinPixelSize(0)
         .setBufferPixels(8);
     }

tiles/src/test/java/com/protomaps/basemap/layers/BoundariesTest.java

@@ -1,7 +1,9 @@
 package com.protomaps.basemap.layers;
 
 import static com.onthegomap.planetiler.TestUtils.newLineString;
+import static com.onthegomap.planetiler.TestUtils.newPolygon;
 
+import com.onthegomap.planetiler.FeatureCollector;
 import com.onthegomap.planetiler.reader.SimpleFeature;
 import com.onthegomap.planetiler.reader.osm.OsmElement;
 import com.onthegomap.planetiler.reader.osm.OsmReader;
@@ -11,6 +13,7 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
 
 class BoundariesTest extends LayerTest {
   @Test
@@ -103,4 +106,159 @@ void testDisputedOsmWay(String key, String value) {
       List.of(Map.of("disputed", true)),
       collector);
   }
+
+  private static final String NE_ADMIN_1_LINES = "ne_10m_admin_1_states_provinces_lines";
+
+  private FeatureCollector processNe(String sourceLayer, Map<String, Object> tags) {
+    return process(SimpleFeature.create(
+      newLineString(0, 0, 1, 1),
+      new HashMap<>(tags),
+      "ne",
+      sourceLayer,
+      1
+    ));
+  }
+
+  /** Attributes every Natural Earth boundary line is expected to carry. */
+  private static Map<String, Object> neLine(String kind, int kindDetail, boolean disputed) {
+    return Map.of(
+      "_layer", "boundaries",
+      "_type", "line",
+      "kind", kind,
+      "kind_detail", kindDetail,
+      "sort_rank", disputed ? 288 : 289,
+      "disputed", disputed ? true : "<null>"
+    );
+  }
+
+  // Every featurecla value the Natural Earth switch knows about, so that a mistyped or reordered
+  // case arm is caught rather than silently falling through to the "drop the feature" default.
+  @ParameterizedTest
+  @CsvSource(value = {
+    "Disputed (please verify),country,2,true",
+    "Indefinite (please verify),country,2,true",
+    "Indeterminant frontier,country,2,true",
+    "Line of control (please verify),country,2,true",
+    "International boundary (verify),country,2,false",
+    "Lease limit,lease_limit,3,false",
+    "Overlay limit,overlay_limit,3,false",
+    "Unrecognized,unrecognized_country,2,false",
+    "Map unit boundary,map_unit,3,false",
+    "Breakaway,unrecognized_country,3,false",
+    "Claim boundary,unrecognized_country,3,false",
+    "Elusive frontier,unrecognized_country,3,false",
+    "Reference line,unrecognized_country,3,false",
+    "Admin-1 region boundary,macroregion,3,false",
+    "Admin-1 boundary,region,4,false",
+    "Admin-1 statistical boundary,region,4,false",
+    "Admin-1 statistical meta bounds,region,4,false",
+    "1st Order Admin Lines,region,4,false",
+    "Unrecognized Admin-1 region boundary,unrecognized_macroregion,4,false",
+    "Unrecognized Admin-1 boundary,unrecognized_region,4,false",
+    "Unrecognized Admin-1 statistical boundary,unrecognized_region,4,false",
+    "Unrecognized Admin-1 statistical meta bounds,unrecognized_region,4,false"
+  })
+  void testNeFeatureClass(String featureClass, String kind, int kindDetail, boolean disputed) {
+    assertFeatures(5,
+      List.of(neLine(kind, kindDetail, disputed)),
+      processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", featureClass, "MIN_ZOOM", 2.0)));
+  }
+
+  @ParameterizedTest
+  @ValueSource(strings = {
+    "ne_10m_admin_0_boundary_lines_land",
+    "ne_10m_admin_0_boundary_lines_map_units",
+    "ne_10m_admin_0_boundary_lines_disputed_areas",
+    NE_ADMIN_1_LINES
+  })
+  void testNeSourceLayers(String sourceLayer) {
+    assertFeatures(5,
+      List.of(neLine("country", 2, false)),
+      processNe(sourceLayer, Map.of("FEATURECLA", "International boundary (verify)", "MIN_ZOOM", 2.0)));
+  }
+
+  // Natural Earth column casing is not consistent: the sqlite package lower cases every column, the GeoPackage
+  // package preserves the source shapefile spelling, and some GeoPackage tables mix both in one table.
+  // MIN_ZOOM also arrives as a REAL from the GeoPackage but as text from sqlite.
+  @ParameterizedTest
+  @CsvSource(value = {
+    "FEATURECLA,MIN_ZOOM,BRK_A3",
+    "featurecla,min_zoom,brk_a3",
+    "FEATURECLA,min_zoom,BRK_A3",
+    "featurecla,MIN_ZOOM,brk_a3"
+  })
+  void testNeColumnCasing(String featureClassKey, String minZoomKey, String brkA3Key) {
+    assertFeatures(5,
+      List.of(Map.of("kind", "region", "kind_detail", 4, "brk_a3", "USA", "_minzoom", 1, "_maxzoom", 5)),
+      processNe(NE_ADMIN_1_LINES,
+        Map.of(featureClassKey, "Admin-1 boundary", minZoomKey, 2.0, brkA3Key, "USA")));
+  }
+
+  // min_zoom drives the low end of the zoom range, and the Natural Earth boundaries always stop at z5
+  // where the OpenStreetMap ones take over. Note that min_zoom above 6 is left uncovered here: it produces
+  // an inverted zoom range, which trips an assertion inside planetiler's setZoomRange.
+  @ParameterizedTest
+  @CsvSource(value = {
+    "1.0,0",
+    "2.0,1",
+    "4.5,3",
+    "6.0,5"
+  })
+  void testNeZoomRange(String minZoom, int expectedMinZoom) {
+    assertFeatures(5,
+      List.of(Map.of("_minzoom", expectedMinZoom, "_maxzoom", 5)),
+      processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", minZoom)));
+  }
+
+  @Test
+  void testNeMissingBrkA3() {
+    assertFeatures(5,
+      List.of(Map.of("kind", "region", "brk_a3", "<null>")),
+      processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", 2.0)));
+  }
+
+  // Regression test for the NullPointerException from switching the "ne" source to the GeoPackage:
+  // FEATURECLA was present but featurecla was not, which blew up on a null switch expression.
+  @Test
+  void testNeMissingFeatureClass() {
+    assertFeatures(5,
+      List.of(),
+      processNe(NE_ADMIN_1_LINES, Map.of("MIN_ZOOM", 2.0)));
+  }
+
+  @Test
+  void testNeUnknownFeatureClass() {
+    assertFeatures(5,
+      List.of(),
+      processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Some new NE value", "MIN_ZOOM", 2.0)));
+  }
+
+  @Test
+  void testNeMissingMinZoom() {
+    assertFeatures(5,
+      List.of(),
+      processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Admin-1 boundary")));
+  }
+
+  // Natural Earth has many more boundary tables than the four this layer reads.
+  @Test
+  void testNeUnhandledSourceLayer() {
+    assertFeatures(5,
+      List.of(),
+      processNe("ne_10m_admin_0_boundary_lines_maritime_indicator",
+        Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", 2.0)));
+  }
+
+  @Test
+  void testNePolygonIsIgnored() {
+    assertFeatures(5,
+      List.of(),
+      process(SimpleFeature.create(
+        newPolygon(0, 0, 0, 1, 1, 1, 0, 0),
+        new HashMap<>(Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", 2.0)),
+        "ne",
+        NE_ADMIN_1_LINES,
+        1
+      )));
+  }
 }