Skip to content

Commit 5a1687e

Browse files
committed
Workaround for inconsistent casing in Geopackage column names
Changeset assisted by Claude Opus 4.8.
1 parent 3e61f42 commit 5a1687e

3 files changed

Lines changed: 184 additions & 9 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ static void run(Arguments args) throws IOException {
313313
.addGeoPackageSource("landcover", sourcesDir.resolve("daylight-landcover.gpkg"),
314314
"https://r2-public.protomaps.com/datasets/daylight-landcover.gpkg");
315315
}
316+
316317
Path pgfEncodingZip = sourcesDir.resolve("pgf-encoding.zip");
317318
Path qrankCsv = sourcesDir.resolve("qrank.csv.gz");
318319

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

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import com.protomaps.basemap.feature.FeatureId;
1313
import java.util.ArrayList;
1414
import java.util.List;
15+
import java.util.Locale;
1516
import java.util.OptionalInt;
1617

1718
@SuppressWarnings("java:S1192")
@@ -25,25 +26,40 @@ public String name() {
2526
return LAYER_NAME;
2627
}
2728

29+
/**
30+
* Reads a Natural Earth attribute, falling back to the upper case spelling of the column.
31+
* <p>
32+
* Column name casing is not consistent across the Natural Earth distributions: the sqlite package is entirely lower
33+
* case, while the GeoPackage package preserves whatever the source shapefiles use. That is lower case for most
34+
* physical layers but upper case for the admin layers this handler reads, and some tables mix both.
35+
*/
36+
private static String getNeString(SourceFeature sf, String key) {
37+
Object value = sf.getTag(key);
38+
if (value == null) {
39+
value = sf.getTag(key.toUpperCase(Locale.ROOT));
40+
}
41+
return value == null ? null : value.toString();
42+
}
43+
2844
public void processNe(SourceFeature sf, FeatureCollector features) {
2945
String sourceLayer = sf.getSourceLayer();
3046
String kind = "";
3147
int adminLevel = 2;
3248
boolean disputed = false;
33-
int themeMinZoom = 0;
3449
int themeMaxZoom = 0;
3550

3651
if (sourceLayer.equals("ne_10m_admin_0_boundary_lines_land") ||
3752
sourceLayer.equals("ne_10m_admin_0_boundary_lines_map_units") ||
3853
sourceLayer.equals("ne_10m_admin_0_boundary_lines_disputed_areas") ||
3954
sourceLayer.equals("ne_10m_admin_1_states_provinces_lines")) {
40-
themeMinZoom = 4;
4155
themeMaxZoom = 5;
4256
kind = "tz_boundary";
4357
}
4458

59+
String featureClass = getNeString(sf, "featurecla");
60+
4561
if (!kind.isEmpty()) {
46-
switch (sf.getString("featurecla", "")) {
62+
switch (featureClass == null ? "" : featureClass) {
4763
case "Disputed (please verify)" -> {
4864
kind = "country";
4965
disputed = true;
@@ -130,19 +146,19 @@ public void processNe(SourceFeature sf, FeatureCollector features) {
130146
}
131147
}
132148

133-
if (sf.canBeLine() && sf.getString("min_zoom") != null && (!kind.isEmpty() && !kind.equals("tz_boundary"))) {
134-
var minZoom = Double.parseDouble(sf.getString("min_zoom")) - 1.0;
149+
String minZoomString = getNeString(sf, "min_zoom");
150+
151+
if (sf.canBeLine() && minZoomString != null && (!kind.isEmpty() && !kind.equals("tz_boundary"))) {
152+
var minZoom = Double.parseDouble(minZoomString) - 1.0;
135153
int sortRank = 289 - (disputed ? 1 : 0);
136154
features.line(this.name())
137155
.setAttr("kind", kind)
138156
.setAttr("kind_detail", adminLevel)
139157
.setAttr("sort_rank", sortRank)
140158
.setSortKey(sortRank)
141159
.setAttr("disputed", disputed ? true : null)
142-
.setAttr("brk_a3", sf.getString("brk_a3"))
143-
.setZoomRange(
144-
sf.getString("min_zoom") == null ? themeMinZoom : (int) minZoom,
145-
themeMaxZoom)
160+
.setAttr("brk_a3", getNeString(sf, "brk_a3"))
161+
.setZoomRange((int) minZoom, themeMaxZoom)
146162
.setMinPixelSize(0)
147163
.setBufferPixels(8);
148164
}

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

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package com.protomaps.basemap.layers;
22

33
import static com.onthegomap.planetiler.TestUtils.newLineString;
4+
import static com.onthegomap.planetiler.TestUtils.newPolygon;
45

6+
import com.onthegomap.planetiler.FeatureCollector;
57
import com.onthegomap.planetiler.reader.SimpleFeature;
68
import com.onthegomap.planetiler.reader.osm.OsmElement;
79
import com.onthegomap.planetiler.reader.osm.OsmReader;
@@ -11,6 +13,7 @@
1113
import org.junit.jupiter.api.Test;
1214
import org.junit.jupiter.params.ParameterizedTest;
1315
import org.junit.jupiter.params.provider.CsvSource;
16+
import org.junit.jupiter.params.provider.ValueSource;
1417

1518
class BoundariesTest extends LayerTest {
1619
@Test
@@ -103,4 +106,159 @@ void testDisputedOsmWay(String key, String value) {
103106
List.of(Map.of("disputed", true)),
104107
collector);
105108
}
109+
110+
private static final String NE_ADMIN_1_LINES = "ne_10m_admin_1_states_provinces_lines";
111+
112+
private FeatureCollector processNe(String sourceLayer, Map<String, Object> tags) {
113+
return process(SimpleFeature.create(
114+
newLineString(0, 0, 1, 1),
115+
new HashMap<>(tags),
116+
"ne",
117+
sourceLayer,
118+
1
119+
));
120+
}
121+
122+
/** Attributes every Natural Earth boundary line is expected to carry. */
123+
private static Map<String, Object> neLine(String kind, int kindDetail, boolean disputed) {
124+
return Map.of(
125+
"_layer", "boundaries",
126+
"_type", "line",
127+
"kind", kind,
128+
"kind_detail", kindDetail,
129+
"sort_rank", disputed ? 288 : 289,
130+
"disputed", disputed ? true : "<null>"
131+
);
132+
}
133+
134+
// Every featurecla value the Natural Earth switch knows about, so that a mistyped or reordered
135+
// case arm is caught rather than silently falling through to the "drop the feature" default.
136+
@ParameterizedTest
137+
@CsvSource(value = {
138+
"Disputed (please verify),country,2,true",
139+
"Indefinite (please verify),country,2,true",
140+
"Indeterminant frontier,country,2,true",
141+
"Line of control (please verify),country,2,true",
142+
"International boundary (verify),country,2,false",
143+
"Lease limit,lease_limit,3,false",
144+
"Overlay limit,overlay_limit,3,false",
145+
"Unrecognized,unrecognized_country,2,false",
146+
"Map unit boundary,map_unit,3,false",
147+
"Breakaway,unrecognized_country,3,false",
148+
"Claim boundary,unrecognized_country,3,false",
149+
"Elusive frontier,unrecognized_country,3,false",
150+
"Reference line,unrecognized_country,3,false",
151+
"Admin-1 region boundary,macroregion,3,false",
152+
"Admin-1 boundary,region,4,false",
153+
"Admin-1 statistical boundary,region,4,false",
154+
"Admin-1 statistical meta bounds,region,4,false",
155+
"1st Order Admin Lines,region,4,false",
156+
"Unrecognized Admin-1 region boundary,unrecognized_macroregion,4,false",
157+
"Unrecognized Admin-1 boundary,unrecognized_region,4,false",
158+
"Unrecognized Admin-1 statistical boundary,unrecognized_region,4,false",
159+
"Unrecognized Admin-1 statistical meta bounds,unrecognized_region,4,false"
160+
})
161+
void testNeFeatureClass(String featureClass, String kind, int kindDetail, boolean disputed) {
162+
assertFeatures(5,
163+
List.of(neLine(kind, kindDetail, disputed)),
164+
processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", featureClass, "MIN_ZOOM", 2.0)));
165+
}
166+
167+
@ParameterizedTest
168+
@ValueSource(strings = {
169+
"ne_10m_admin_0_boundary_lines_land",
170+
"ne_10m_admin_0_boundary_lines_map_units",
171+
"ne_10m_admin_0_boundary_lines_disputed_areas",
172+
NE_ADMIN_1_LINES
173+
})
174+
void testNeSourceLayers(String sourceLayer) {
175+
assertFeatures(5,
176+
List.of(neLine("country", 2, false)),
177+
processNe(sourceLayer, Map.of("FEATURECLA", "International boundary (verify)", "MIN_ZOOM", 2.0)));
178+
}
179+
180+
// Natural Earth column casing is not consistent: the sqlite package lower cases every column, the GeoPackage
181+
// package preserves the source shapefile spelling, and some GeoPackage tables mix both in one table.
182+
// MIN_ZOOM also arrives as a REAL from the GeoPackage but as text from sqlite.
183+
@ParameterizedTest
184+
@CsvSource(value = {
185+
"FEATURECLA,MIN_ZOOM,BRK_A3",
186+
"featurecla,min_zoom,brk_a3",
187+
"FEATURECLA,min_zoom,BRK_A3",
188+
"featurecla,MIN_ZOOM,brk_a3"
189+
})
190+
void testNeColumnCasing(String featureClassKey, String minZoomKey, String brkA3Key) {
191+
assertFeatures(5,
192+
List.of(Map.of("kind", "region", "kind_detail", 4, "brk_a3", "USA", "_minzoom", 1, "_maxzoom", 5)),
193+
processNe(NE_ADMIN_1_LINES,
194+
Map.of(featureClassKey, "Admin-1 boundary", minZoomKey, 2.0, brkA3Key, "USA")));
195+
}
196+
197+
// min_zoom drives the low end of the zoom range, and the Natural Earth boundaries always stop at z5
198+
// where the OpenStreetMap ones take over. Note that min_zoom above 6 is left uncovered here: it produces
199+
// an inverted zoom range, which trips an assertion inside planetiler's setZoomRange.
200+
@ParameterizedTest
201+
@CsvSource(value = {
202+
"1.0,0",
203+
"2.0,1",
204+
"4.5,3",
205+
"6.0,5"
206+
})
207+
void testNeZoomRange(String minZoom, int expectedMinZoom) {
208+
assertFeatures(5,
209+
List.of(Map.of("_minzoom", expectedMinZoom, "_maxzoom", 5)),
210+
processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", minZoom)));
211+
}
212+
213+
@Test
214+
void testNeMissingBrkA3() {
215+
assertFeatures(5,
216+
List.of(Map.of("kind", "region", "brk_a3", "<null>")),
217+
processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", 2.0)));
218+
}
219+
220+
// Regression test for the NullPointerException from switching the "ne" source to the GeoPackage:
221+
// FEATURECLA was present but featurecla was not, which blew up on a null switch expression.
222+
@Test
223+
void testNeMissingFeatureClass() {
224+
assertFeatures(5,
225+
List.of(),
226+
processNe(NE_ADMIN_1_LINES, Map.of("MIN_ZOOM", 2.0)));
227+
}
228+
229+
@Test
230+
void testNeUnknownFeatureClass() {
231+
assertFeatures(5,
232+
List.of(),
233+
processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Some new NE value", "MIN_ZOOM", 2.0)));
234+
}
235+
236+
@Test
237+
void testNeMissingMinZoom() {
238+
assertFeatures(5,
239+
List.of(),
240+
processNe(NE_ADMIN_1_LINES, Map.of("FEATURECLA", "Admin-1 boundary")));
241+
}
242+
243+
// Natural Earth has many more boundary tables than the four this layer reads.
244+
@Test
245+
void testNeUnhandledSourceLayer() {
246+
assertFeatures(5,
247+
List.of(),
248+
processNe("ne_10m_admin_0_boundary_lines_maritime_indicator",
249+
Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", 2.0)));
250+
}
251+
252+
@Test
253+
void testNePolygonIsIgnored() {
254+
assertFeatures(5,
255+
List.of(),
256+
process(SimpleFeature.create(
257+
newPolygon(0, 0, 0, 1, 1, 1, 0, 0),
258+
new HashMap<>(Map.of("FEATURECLA", "Admin-1 boundary", "MIN_ZOOM", 2.0)),
259+
"ne",
260+
NE_ADMIN_1_LINES,
261+
1
262+
)));
263+
}
106264
}

0 commit comments

Comments
 (0)