bugfix(#55): ImportService and StandaloneBoundaryImporter to use …

src/main/java/com/dedicatedcode/paikka/service/importer/ImportService.java

@@ -35,6 +35,7 @@
 import org.locationtech.jts.algorithm.construct.MaximumInscribedCircle;
 import org.locationtech.jts.geom.*;
 import org.locationtech.jts.io.WKBWriter;
+import org.locationtech.jts.operation.polygonize.Polygonizer;
 import org.rocksdb.*;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Service;
@@ -1524,39 +1525,46 @@ private org.locationtech.jts.geom.Geometry buildGeometryFromRelRec(RelRec rec, R
         List<List<Coordinate>> outerRings = buildConnectedRings(toList(rec.outer), nodeCache, wayIndexDb);
         List<List<Coordinate>> innerRings = buildConnectedRings(toList(rec.inner), nodeCache, wayIndexDb);
         if (outerRings.isEmpty()) return null;
-        List<Polygon> validPolygons = new ArrayList<>();
+
+        Polygonizer polygonizer = new Polygonizer();
         for (List<Coordinate> outerRing : outerRings) {
             try {
-                LinearRing shell = GEOMETRY_FACTORY.createLinearRing(outerRing.toArray(new Coordinate[0]));
-                List<LinearRing> holes = new ArrayList<>();
-                for (List<Coordinate> innerRing : innerRings)
-                    try {
-                        holes.add(GEOMETRY_FACTORY.createLinearRing(innerRing.toArray(new Coordinate[0])));
-                    } catch (Exception e) {
-                        stats.recordError(ImportStatistics.Stage.PROCESSING_ADMIN_BOUNDARIES, Kind.READ, rec.osmId, "rocks-get:way_index", e);
-                    }
-                Polygon polygon = GEOMETRY_FACTORY.createPolygon(shell, holes.toArray(new LinearRing[0]));
-                if (polygon.isValid()) {
-                    validPolygons.add(polygon);
-                } else {
-                    try {
-                        org.locationtech.jts.geom.Geometry repaired = polygon.buffer(0);
-                        if (repaired != null && repaired.isValid() && !repaired.isEmpty()) {
-                            for (int i = 0; i < repaired.getNumGeometries(); i++) {
-                                org.locationtech.jts.geom.Geometry part = repaired.getGeometryN(i);
-                                if (part instanceof Polygon && part.isValid()) {
-                                    validPolygons.add((Polygon) part);
-                                }
+                polygonizer.add(GEOMETRY_FACTORY.createLinearRing(outerRing.toArray(new Coordinate[0])));
+            } catch (Exception e) {
+                stats.recordError(ImportStatistics.Stage.PROCESSING_ADMIN_BOUNDARIES, Kind.GEOMETRY, rec.osmId, "createLinearRing-outer", e);
+            }
+        }
+        for (List<Coordinate> innerRing : innerRings) {
+            try {
+                polygonizer.add(GEOMETRY_FACTORY.createLinearRing(innerRing.toArray(new Coordinate[0])));
+            } catch (Exception e) {
+                stats.recordError(ImportStatistics.Stage.PROCESSING_ADMIN_BOUNDARIES, Kind.READ, rec.osmId, "buildConnectedRings-inner", e);
+            }
+        }
+
+        @SuppressWarnings("unchecked")
+        Collection<Polygon> polygons = polygonizer.getPolygons();
+        List<Polygon> validPolygons = new ArrayList<>();
+        for (Polygon p : polygons) {
+            if (p.isValid()) {
+                validPolygons.add(p);
+            } else {
+                try {
+                    org.locationtech.jts.geom.Geometry repaired = p.buffer(0);
+                    if (repaired != null && repaired.isValid() && !repaired.isEmpty()) {
+                        for (int i = 0; i < repaired.getNumGeometries(); i++) {
+                            org.locationtech.jts.geom.Geometry part = repaired.getGeometryN(i);
+                            if (part instanceof Polygon polygon && polygon.isValid()) {
+                                validPolygons.add(polygon);
                             }
                         }
-                    } catch (Exception repairEx) {
-                        stats.recordError(ImportStatistics.Stage.PROCESSING_ADMIN_BOUNDARIES, Kind.GEOMETRY, rec.osmId, "repair-polygon", repairEx);
                     }
+                } catch (Exception repairEx) {
+                    stats.recordError(ImportStatistics.Stage.PROCESSING_ADMIN_BOUNDARIES, Kind.GEOMETRY, rec.osmId, "repair-polygon", repairEx);
                 }
-            } catch (Exception e) {
-                stats.recordError(ImportStatistics.Stage.PROCESSING_ADMIN_BOUNDARIES, Kind.GEOMETRY, null, "build-boundary-geometry", e);
             }
         }
+
         if (validPolygons.isEmpty()) return null;
         return validPolygons.size() == 1 ? validPolygons.getFirst() : GEOMETRY_FACTORY.createMultiPolygon(validPolygons.toArray(new Polygon[0]));
     }

src/main/java/com/dedicatedcode/paikka/service/importer/StandaloneBoundaryImporter.java

@@ -23,6 +23,7 @@
 import de.topobyte.osm4j.pbf.seq.PbfIterator;
 import org.locationtech.jts.geom.*;
 import org.locationtech.jts.io.WKBWriter;
+import org.locationtech.jts.operation.polygonize.Polygonizer;
 import org.rocksdb.*;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -395,26 +396,26 @@ private Geometry buildMultiPolygon(RelationStub stub, RocksDB nodeCache, RocksDB
         List<List<Coordinate>> innerRings = stitchRings(stub.innerWays(), nodeCache, wayCache);
         if (outerRings.isEmpty()) return null;
 
-        List<Polygon> polygons = new ArrayList<>();
+        Polygonizer polygonizer = new Polygonizer();
         for (List<Coordinate> outer : outerRings) {
             try {
-                LinearRing shell = GEOMETRY_FACTORY.createLinearRing(outer.toArray(new Coordinate[0]));
-                List<LinearRing> holes = new ArrayList<>();
-                for (List<Coordinate> inner : innerRings) {
-                    try {
-                        holes.add(GEOMETRY_FACTORY.createLinearRing(inner.toArray(new Coordinate[0])));
-                    } catch (Exception e) {
-                        stats.recordError(BoundaryImportStatistics.Stage.PROCESSING_RELATIONS, BoundaryImportStatistics.Kind.GEOMETRY, stub.osmId(), "createLinearRing-inner", e);
-                    }
-                }
-                Polygon p = GEOMETRY_FACTORY.createPolygon(shell, holes.toArray(new LinearRing[0]));
-                if (p.isValid()) polygons.add(p);
+                polygonizer.add(GEOMETRY_FACTORY.createLinearRing(outer.toArray(new Coordinate[0])));
+            } catch (Exception e) {
+                stats.recordError(BoundaryImportStatistics.Stage.PROCESSING_RELATIONS, BoundaryImportStatistics.Kind.GEOMETRY, stub.osmId(), "createLinearRing-outer", e);
+            }
+        }
+        for (List<Coordinate> inner : innerRings) {
+            try {
+                polygonizer.add(GEOMETRY_FACTORY.createLinearRing(inner.toArray(new Coordinate[0])));
             } catch (Exception e) {
-                stats.recordError(BoundaryImportStatistics.Stage.PROCESSING_RELATIONS, BoundaryImportStatistics.Kind.GEOMETRY, stub.osmId(), "buildMultiPolygon", e);
+                stats.recordError(BoundaryImportStatistics.Stage.PROCESSING_RELATIONS, BoundaryImportStatistics.Kind.GEOMETRY, stub.osmId(), "createLinearRing-inner", e);
             }
         }
+
+        @SuppressWarnings("unchecked")
+        Collection<Polygon> polygons = polygonizer.getPolygons();
         if (polygons.isEmpty()) return null;
-        return polygons.size() == 1 ? polygons.getFirst() : GEOMETRY_FACTORY.createMultiPolygon(polygons.toArray(new Polygon[0]));
+        return polygons.size() == 1 ? polygons.iterator().next() : GEOMETRY_FACTORY.createMultiPolygon(polygons.toArray(new Polygon[0]));
     }
 
     private List<List<Coordinate>> stitchRings(List<Long> wayIds, RocksDB nodeCache, RocksDB wayCache) {

src/test/java/com/dedicatedcode/paikka/service/importer/ImportServiceTest.java

@@ -217,7 +217,7 @@ void testImportPoiHasNamesAndBoundary() throws Exception {
         POI poiById = findPoiById(tempDataDir, 432751852);
         assertEquals(1, poiById.namesLength(), "POI should have no");
         assertEquals("Jardin des Boulingrins", poiById.names(0).text(), "POI should have no");
-        assertEquals(6, poiById.hierarchyLength());
+        assertEquals(3, poiById.hierarchyLength());
     }
 
     @Test

src/test/java/com/dedicatedcode/paikka/service/importer/StandaloneBoundaryImporterThuringiaTest.java

@@ -0,0 +1,253 @@
+/*
+ *  This file is part of paikka.
+ *
+ *  Paikka is free software: you can redistribute it and/or
+ *  modify it under the terms of the GNU Affero General Public License
+ *  as published by the Free Software Foundation, either version 3 or
+ *  any later version.
+ *
+ *  Paikka is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied
+ *  warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *  See the GNU Affero General Public License for more details.
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Paikka. If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package com.dedicatedcode.paikka.service.importer;
+
+import com.dedicatedcode.paikka.config.PaikkaConfiguration;
+import org.junit.jupiter.api.*;
+import org.locationtech.jts.geom.*;
+import org.locationtech.jts.io.WKBReader;
+import org.rocksdb.Options;
+import org.rocksdb.RocksDB;
+
+import java.io.InputStream;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.util.*;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+/**
+ * Tests for proper hole-in-polygon assignment in the standalone boundary importer.
+ * Uses thueringen-260804.boundaries.osm.pbf which has multipolygon relations
+ * with disjoint outer rings and inner rings (holes). This verifies the fix
+ * for the bug where holes were incorrectly assigned to all outer rings,
+ * causing rendering artifacts (large open triangles / lines).
+ */
+class StandaloneBoundaryImporterThuringiaTest {
+
+    private static Path tempOutputDir;
+    private static Path tempPbfFile;
+
+    @BeforeAll
+    static void setUp() throws Exception {
+        tempOutputDir = Files.createTempDirectory("paikka-boundary-thueringen-test");
+        tempPbfFile = Files.createTempFile("paikka-boundary-thueringen-test", ".pbf");
+
+        try (InputStream is = StandaloneBoundaryImporter.class.getClassLoader()
+                .getResourceAsStream("thueringen-260804.boundaries.osm.pbf")) {
+            assertNotNull(is, "thueringen-260804.boundaries.osm.pbf not found in test resources");
+            Files.copy(is, tempPbfFile, StandardCopyOption.REPLACE_EXISTING);
+        }
+
+        PaikkaConfiguration config = new PaikkaConfiguration();
+
+        PaikkaConfiguration.ImportConfiguration importCfg = new PaikkaConfiguration.ImportConfiguration();
+        importCfg.setThreads(4);
+        config.setImportConfiguration(importCfg);
+
+        PaikkaConfiguration.SimplificationConfiguration simplCfg = new PaikkaConfiguration.SimplificationConfiguration();
+        simplCfg.setContinentTolerance(0.005);
+        simplCfg.setCountryTolerance(0.00045);
+        simplCfg.setStateTolerance(0.00009);
+        simplCfg.setPoiTolerance(0.000018);
+        simplCfg.setDefaultTolerance(0.000045);
+        config.setSimplificationConfiguration(simplCfg);
+
+        GeometrySimplificationService simplService = new GeometrySimplificationService(config);
+        StandaloneBoundaryImporter importer = new StandaloneBoundaryImporter(simplService, config);
+        importer.importBoundaries(Collections.singletonList(tempPbfFile.toString()), tempOutputDir.toString());
+    }
+
+    @AfterAll
+    static void tearDown() {
+        if (tempOutputDir != null && Files.exists(tempOutputDir)) {
+            deleteDirectory(tempOutputDir.toFile());
+        }
+        if (tempPbfFile != null && Files.exists(tempPbfFile)) {
+            tempPbfFile.toFile().delete();
+        }
+    }
+
+    // ===== infrastructure =====
+
+    @Test
+    void testOutputDatabasesExist() {
+        assertTrue(Files.exists(tempOutputDir.resolve("h3_to_osm")), "h3_to_osm directory should exist");
+        assertTrue(Files.exists(tempOutputDir.resolve("region_metadata")), "region_metadata directory should exist");
+        assertTrue(Files.exists(tempOutputDir.resolve("region_geometry")), "region_geometry directory should exist");
+        assertTrue(Files.exists(tempOutputDir.resolve("osm_names.tsv")), "osm_names.tsv should exist");
+    }
+
+    @Test
+    void testAllGeometryValid() throws Exception {
+        Path geomDbPath = tempOutputDir.resolve("region_geometry");
+        WKBReader wkbReader = new WKBReader();
+
+        try (Options opts = new Options().setCreateIfMissing(false);
+             RocksDB db = RocksDB.open(opts, geomDbPath.toString())) {
+
+            var it = db.newIterator();
+            it.seekToFirst();
+            int checked = 0;
+            List<Long> invalidOsmIds = new ArrayList<>();
+
+            while (it.isValid()) {
+                long osmId = ByteBuffer.wrap(it.key()).order(ByteOrder.BIG_ENDIAN).getLong();
+                byte[] wkb = it.value();
+                Geometry geom = wkbReader.read(wkb);
+
+                if (!geom.isValid()) {
+                    invalidOsmIds.add(osmId);
+                }
+                checked++;
+                it.next();
+            }
+
+            System.out.println("Checked " + checked + " geometries for validity");
+            assertTrue(checked > 0, "Should have at least one geometry entry");
+
+            if (!invalidOsmIds.isEmpty()) {
+                System.out.println("Invalid geometries (OSM IDs): " + invalidOsmIds);
+            }
+            assertTrue(invalidOsmIds.isEmpty(),
+                    "All stored geometries should be valid. Invalid OSM IDs: " + invalidOsmIds);
+        }
+    }
+
+    @Test
+    void testHolesAreContainedInOuterPolygons() throws Exception {
+        Path geomDbPath = tempOutputDir.resolve("region_geometry");
+        WKBReader wkbReader = new WKBReader();
+
+        try (Options opts = new Options().setCreateIfMissing(false);
+             RocksDB db = RocksDB.open(opts, geomDbPath.toString())) {
+
+            var it = db.newIterator();
+            it.seekToFirst();
+            int checked = 0;
+            List<String> violations = new ArrayList<>();
+
+            while (it.isValid()) {
+                long osmId = ByteBuffer.wrap(it.key()).order(ByteOrder.BIG_ENDIAN).getLong();
+                byte[] wkb = it.value();
+                Geometry geom = wkbReader.read(wkb);
+
+                violations.addAll(checkHoleContainment(osmId, geom));
+                checked++;
+                it.next();
+            }
+
+            System.out.println("Checked " + checked + " geometries for hole containment");
+            assertTrue(checked > 0, "Should have at least one geometry entry");
+
+            if (!violations.isEmpty()) {
+                for (String v : violations) {
+                    System.out.println("VIOLATION: " + v);
+                }
+            }
+            assertTrue(violations.isEmpty(),
+                    "All inner rings (holes) must be contained within their outer polygon shell. Violations:\n"
+                    + String.join("\n", violations));
+        }
+    }
+
+    @Test
+    void testGeometryHasExpectedTopology() throws Exception {
+        /*
+         * Thuringia (OSM relation 62366) is a multipolygon: one main body plus
+         * exclaves. The stored WKB should be a Polygon or MultiPolygon where
+         * every inner ring (hole) is properly contained within an outer ring.
+         *
+         * Assertion to implement:
+         * 1. Load geometry for Thuringia relation by OSM ID
+         * 2. Verify it's a Polygon or MultiPolygon
+         * 3. For each polygon, verify all interior rings are within the exterior ring
+         * 4. Verify there are no "floating" holes (holes with no containing shell)
+         */
+
+        Path geomDbPath = tempOutputDir.resolve("region_geometry");
+        long thueringenOsmId = 62366L;
+        WKBReader wkbReader = new WKBReader();
+
+        try (Options opts = new Options().setCreateIfMissing(false);
+             RocksDB db = RocksDB.open(opts, geomDbPath.toString())) {
+
+            byte[] key = ByteBuffer.allocate(8).order(ByteOrder.BIG_ENDIAN).putLong(thueringenOsmId).array();
+            byte[] wkb = db.get(key);
+
+            assertNotNull(wkb, "Thuringia (OSM ID " + thueringenOsmId + ") should have stored geometry");
+            assertTrue(wkb.length > 0, "Thuringia WKB should not be empty");
+
+            Geometry geom = wkbReader.read(wkb);
+            System.out.println("Thuringia geometry: type=" + geom.getGeometryType()
+                    + " valid=" + geom.isValid()
+                    + " numGeometries=" + geom.getNumGeometries()
+                    + " numPoints=" + geom.getNumPoints());
+
+            assertTrue(geom.isValid(), "Thuringia geometry should be valid");
+            assertTrue(geom instanceof Polygon || geom instanceof MultiPolygon,
+                    "Thuringia geometry should be a Polygon or MultiPolygon, got: " + geom.getGeometryType());
+        }
+    }
+
+    // ===== helpers =====
+
+    private static final GeometryFactory GEOM_CHECK_FACTORY = new GeometryFactory();
+
+    /**
+     * Checks that every interior ring of every polygon is properly contained
+     * within that polygon's exterior ring. Returns a list of violation messages,
+     * empty if all is well.
+     */
+    private static List<String> checkHoleContainment(long osmId, Geometry geom) {
+        List<String> violations = new ArrayList<>();
+        int numParts = geom.getNumGeometries();
+        for (int g = 0; g < numParts; g++) {
+            Geometry part = geom.getGeometryN(g);
+            if (!(part instanceof Polygon polygon))
+                continue;
+
+            LinearRing shell = polygon.getExteriorRing();
+            Polygon shellPoly = GEOM_CHECK_FACTORY.createPolygon(shell);
+            for (int h = 0; h < polygon.getNumInteriorRing(); h++) {
+                LinearRing hole = polygon.getInteriorRingN(h);
+                Polygon holePoly = GEOM_CHECK_FACTORY.createPolygon(hole);
+                if (!shellPoly.contains(holePoly)) {
+                    violations.add(String.format(
+                            "OSM %d: interior ring %d is not contained within its exterior ring (hole centroid: %.6f,%.6f)",
+                            osmId, h, hole.getCentroid().getX(), hole.getCentroid().getY()));
+                }
+            }
+        }
+        return violations;
+    }
+
+    private static void deleteDirectory(java.io.File dir) {
+        if (dir.isDirectory()) {
+            java.io.File[] children = dir.listFiles();
+            if (children != null) {
+                for (java.io.File child : children) {
+                    deleteDirectory(child);
+                }
+            }
+        }
+        dir.delete();
+    }
+}