From 738e1816570905bd112985217cf4b5928de4480f Mon Sep 17 00:00:00 2001
From: Michael Barry <msbarry@users.noreply.github.com>
Date: Mon, 24 Apr 2023 06:43:03 -0400
Subject: [PATCH] handle empty geopackage geometries (#561)

---
 .../planetiler/reader/GeoPackageReader.java   |  13 ++++++--
 .../reader/GeoPackageReaderTest.java          |  31 +++++++++++++++---
 .../src/test/resources/empty-geom.gpkg        | Bin 0 -> 102400 bytes
 3 files changed, 37 insertions(+), 7 deletions(-)
 create mode 100644 planetiler-core/src/test/resources/empty-geom.gpkg

diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/GeoPackageReader.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/GeoPackageReader.java
index e4568a9b..e326436a 100644
--- a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/GeoPackageReader.java
+++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/GeoPackageReader.java
@@ -25,11 +25,14 @@ import org.geotools.referencing.CRS;
 import org.locationtech.jts.geom.Geometry;
 import org.opengis.referencing.FactoryException;
 import org.opengis.referencing.operation.MathTransform;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Utility that reads {@link SourceFeature SourceFeatures} from the vector geometries contained in a GeoPackage file.
  */
 public class GeoPackageReader extends SimpleReader<SimpleFeature> {
+  private static final Logger LOGGER = LoggerFactory.getLogger(GeoPackageReader.class);
 
   private final boolean keepUnzipped;
   private Path extractedPath = null;
@@ -122,6 +125,7 @@ public class GeoPackageReader extends SimpleReader<SimpleFeature> {
   public void readFeatures(Consumer<SimpleFeature> next) throws Exception {
     var latLonCRS = CRS.decode("EPSG:4326");
     long id = 0;
+    boolean loggedMissingGeometry = false;
 
     for (var featureName : geoPackage.getFeatureTables()) {
       FeatureDao features = geoPackage.getFeatureDao(featureName);
@@ -136,11 +140,16 @@ public class GeoPackageReader extends SimpleReader<SimpleFeature> {
 
       for (var feature : features.queryForAll()) {
         GeoPackageGeometryData geometryData = feature.getGeometry();
-        if (geometryData == null) {
+        byte[] wkb;
+        if (geometryData == null || (wkb = geometryData.getWkb()).length == 0) {
+          if (!loggedMissingGeometry) {
+            loggedMissingGeometry = true;
+            LOGGER.warn("Geopackage file contains empty geometry: {}", geoPackage.getPath());
+          }
           continue;
         }
 
-        Geometry featureGeom = (new WKBReader()).read(geometryData.getWkb());
+        Geometry featureGeom = (new WKBReader()).read(wkb);
         Geometry latLonGeom = (transform.isIdentity()) ? featureGeom : JTS.transform(featureGeom, transform);
 
         FeatureColumns columns = feature.getColumns();
diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/reader/GeoPackageReaderTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/reader/GeoPackageReaderTest.java
index 1957a262..ea0675be 100644
--- a/planetiler-core/src/test/java/com/onthegomap/planetiler/reader/GeoPackageReaderTest.java
+++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/reader/GeoPackageReaderTest.java
@@ -4,7 +4,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.onthegomap.planetiler.TestUtils;
-import com.onthegomap.planetiler.collection.IterableOnce;
 import com.onthegomap.planetiler.geo.GeoUtils;
 import com.onthegomap.planetiler.stats.Stats;
 import com.onthegomap.planetiler.util.FileUtils;
@@ -13,7 +12,8 @@ import java.io.IOException;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.function.Consumer;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Timeout;
 import org.junit.jupiter.api.io.TempDir;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -45,9 +45,7 @@ class GeoPackageReaderTest {
             List<Geometry> points = new ArrayList<>();
             List<String> names = new ArrayList<>();
             WorkerPipeline.start("test", Stats.inMemory())
-              .readFromTiny("files", List.of(Path.of("dummy-path")))
-              .addWorker("geopackage", 1,
-                (IterableOnce<Path> p, Consumer<SimpleFeature> next) -> reader.readFeatures(next))
+              .fromGenerator("geopackage", reader::readFeatures, 1)
               .addBuffer("reader_queue", 100, 1)
               .sinkToConsumer("counter", 1, elem -> {
                 assertTrue(elem.getTag("name") instanceof String);
@@ -67,4 +65,27 @@ class GeoPackageReaderTest {
       }
     }
   }
+
+  @Test
+  @Timeout(30)
+  void testReadEmptyGeoPackage() throws IOException {
+    Path path = TestUtils.pathToResource("empty-geom.gpkg");
+
+    try (
+      var reader = new GeoPackageReader(null, "test", path, tmpDir, false)
+    ) {
+      for (int iter = 0; iter < 2; iter++) {
+        String id = "iter=" + iter;
+        assertEquals(1, reader.getFeatureCount(), id);
+        AtomicInteger found = new AtomicInteger(0);
+        WorkerPipeline.start("test", Stats.inMemory())
+          .fromGenerator("geopackage", reader::readFeatures, 1)
+          .addBuffer("reader_queue", 100, 1)
+          .sinkToConsumer("counter", 1, elem -> {
+            found.incrementAndGet();
+          }).await();
+        assertEquals(0, found.get());
+      }
+    }
+  }
 }
diff --git a/planetiler-core/src/test/resources/empty-geom.gpkg b/planetiler-core/src/test/resources/empty-geom.gpkg
new file mode 100644
index 0000000000000000000000000000000000000000..c8294df1a48d481bf81d80d2dbf178b3aaeeeb17
GIT binary patch
literal 102400
zcmeI5%TpW49mhxJA;1RgU3;|~cGqp~5}~pLlEB9Jkwk_8YZpmakM**SYf39MVq$3q
z&kP1!d6dA9Y~`4$JvgUSCFfKQd0g^G<d#dWdq{Fg4tq#awdr|C8a*HwTM~xvSVlA5
zJ^lFne%(FYt^Q5&_M)g!vLY+Rj7BE8)11S>eUA{1;|AFOci8{(@jg50E?=;32giB-
zqF*m8eY{|Ex1SDh<99ylVJ=<6|J2>z8UD}k*XF4`7|uZe1V8`;KmY_l00ck)1V8`;
zKmY_@HGy+(7USs=_GkFaAp6@j_-k&6{TW())hQu95C8!X009sH0T2KI5C8!X0D%`G
z@O{pCgFDxEzL=JBbVZbCj^wGFS2Am>VwNNm$yhwbub2HQC-y*DSypnQlwoVV3SFTJ
zm9mtmTdGEjcfTng&THH`$NBr!RAw`ZMpZE*l@s}<5_#o>R0Hji$j;ecuJm8z&YgHH
zmfpLwkR(^8^z7xRCT97i<ihE%7WnwWZ1S$#^mO~?!l`uBxD<jjSEk&)<kBrZ5s%DO
zgIu1Rz7m|A_JwAyjE62wheDH=E>HTx>C~-wB9dCZ>(*m-``puk$$<Oby(T`FCqtKY
zpQS`3%CpoK6iTHHF}v#a1)2oEG7|{u!PBuw>aIIS^DH<EIUWd3PhFY}&4exmXJ)1*
zo8@697z!DA_%y-_xtP&JZ9T_sj>QwHTP$m)vn=H;cRrj<HOujond3q^<V3-Wid5;V
zM3hL5X2oKrz`pfdszjEEwn~Y`UR_(IiY(^z@xNnuh+`idAOHd&00JNY0w4eaAOHd&
z00JOzObC=@$6zV?UMV_wLoe|OJ{StbrUJrJB7QUciLf{u3<QLHLH;~b5FSpB2gZX!
zC=i_SONu5WC3(X?D?g;lypqXkqAd9fnJuaa`Xq#3AGVMGKxK{!Rpr9E;Xk!PGuk?H
zXmvWFKxon*nDU1Lso>Prz|7T4Q{&U2na@strT@s^G5iaTeQ<yP2!H?xfB*=900@8p
z2!H?xfB*=zpFpm=6zy;I%mBv!?H4Rk1OX5L0T2KI5C8!X009sH0T2KI8-ccr|IOe3
zKj(&@+fv3U2!H?xfB*=900@8p2!H?xfB*=9K${5U99aKv6KW&_0w4eaAOHd&00JNY
z0w4eaAOHfLBhbe6|Hs_$<IWWnDT4qAfB*=900@8p2!H?xfB*=9z;Ph($kA7do*O;U
z*VpRZ{_Kql#b|ml6<LZeE-%DmW&3|$a>HL92hu1b2!H?xfB*=900@8p2!H?xfB*;_
zCIVs)n-FNZ?LU1s0QUb5liZOP5C8!X009sH0T2KI5C8!X0D)sjpl!eZ=T09(x+pRT
zfB*=900@8p2!H?xfB*=900<l-0{Z$t#{b7i;3z5xfB*=900@8p2!H?xfB*=9z_BEt
zzyA;8|6?h76dD9T00ck)1V8`;KmY_l00cnb7!knw|1lCaiV6ZC00JNY0w4eaAOHd&
z00JOzYzYhw{fj%pMY*AWpZe?2ABKK-%5my>|LV!#pS<t<asT<AKlW^NUpeu2$FEqB
z|9@<!&NxTX{=UaSO%d~Xs^r)1=LJnHP@$M%PMhVg4Kb&!3hQgRj7ICj%qIA7iYKW=
zWMP3%5Kp~-vp78DCO;of@Fcx7r{5f(Csu++N+cd@jK@RR9fS|h-Xe+k9TvetBu0|_
zB0rlViEt#zkA-i>6Dc3@nE52KM6}H31xh>(dGdJ3Ls2fU(x@aWORA=1L`l0^n~7{j
zl4Om1PDz2PD$!Om5(#>|<jyUg<$Ns0-x;sT^mP*SUL$;LZhP*uiS^9Bu!@pO6|ElI
zi(xh6s9=r6l6)fdk{Hc6JBab4Vduzgrkxjd!}v&+i$Z}uq=kd(W<v}WgqCK?E2J9c
zC4nAXFKt<9rj>zOt+a){P6C!rULP`H_Ur@Z0X4Fku5h^Yv6?Y*Kz!dF>~$sm4o;ME
zbW?p$5H%`f)-~C<7V7J+5Uh6@+L=A&bou>`r!z*IRPSGZJy@>(;hT#*srMygMkIoi
zDN>T+KTVNXoc&KPF8XXmjl@#C-Xd50YFew^d{?QMU)4y1x6vz@t#fm~-ELeHH(CAM
z6jbpMC3Eq#UZ6^rOwX<6p0n{-GR2zqSc<IO7c{L%mJ*R@II&DV=9ldZe6~93_1SJ)
zVYd~Ff{m8m>qHh}^)Yx!f}dy2dTf?Ynz&iFpixOvZK%E8o%aWvu1l93Ps}Lm<szuG
zp&i%VPVOm9J%qh9wGy?L*{3S1Am$o{Qxqj(vjMK|y4C2K*=!b;xi*HaMVZ%%>MLVe
zCnh6qRy@6vuL{U4Z=-P3wTvca3W7pc1a(U#V^%=#!pUCO-Antfr;L1KUDf4r>M`eZ
zx~8Wc&(2rtt8RapYRZ<7l?&@dNj1b@@7UKU>-r$O3)^SL+^k|(BGI<iXtMyj>Bb|H
zio}-nN>r(J#YQKaq*-0l6Vpqn+O^M~Z6AAV(is>Tsi|`7?Cz{xOVY8(?KFE{s#e09
z@T`jJ;iX!^+XC;aX4Y#{R(N?`iuCuo?oIEhOBJD58X<(5>cl{#@5Q4s`;ht+`b%&1
zI9(GHj?&$#V#;}iNy}`nj11TvdK(qe?roFH9u3b5&1mZiWvQ%7)y`p#pAWOQlNmja
zH{9FnN>A*G%bu=P@9a*_op(<-T^^6)$wswoHRUQ_bTyW#>}6N9JqpA|G8sXyO0{8p
zVvaTX;)+NW)5FwWUyiC-MO@QFHmEQ<+;Y)99$z7&YC@Kc4)UyjW22c=BwFp^NIBpb
z8)U3#Vv&w{Mwk7gMgM3nHG1pnX!Ppn%4Z&*N0K)@Ua!yIlKY6g1@{$g4L9?$HQ06i
zLSto6^y;AZ90zG#D~;<+cdskz*;B%0DOeSueAzS8wf|d9#V_C5&4#gk`cQVe%VG4r
zJKm~t)b#^~?$^1UXjGND5N7Yu<XLK$7S-M+`eY@ak;F%=J1T2NrIodNv5F%rIo4G*
z?Y?t#MU+G%iaK?y*E_HN`~Ly%H{8(g2A59_4*a&C_VxGvq}SgiF~?Q*@0A_Bu2&G%
zlj|e>BiqhWwC~wPYv5sh2{~Gz>?yjYWf5BaLd90cUsg72GQ{1{!n#R#KE(zAb8M1C
zpJa&<cdN_qw&rE5(Q`v*#3pP~q(<cAM%}fa1g$BSg1kW$TmR%`VNEwrtIOdrN6k;J
ze``1OHAlF|M14C_*s}UoC$nA$^}TqA)OYEfUf0Eoj%B06v}P?-`e2<(Sz34bM!DrO
zCLHQK$Mnw8$L{p*IN$Dfm7=AnWBW%&tgWtyt&aapO}X2kU}MC+BIewcF}!_BBb-jf
zBQchvC~qIG5w|`Ha1;BRfW6zxE_0Ik+HMni_PLdV&W|>Uqke{z&E4BWb!?8bCKJVV
z)tI@@D{SDxp5=|n`|32Q{)}c!;Md&royufs<Smo!$iCPfaJtl<_92@vwP%gIB+KP<
z`_fZ$g7D?^v?h*S_0*b*?9>{Q$=1rmi?OwCvgzsBed(#i<)!qrCXNc*YTw+r#<uVE
zIx}AfS!jBdESKMOj@)6ZO*6aId29OIoaa8MqBe`9LbG8t)S6T8l&aQbw&~JI^Yu_G
z+tmfpTW^?bUHfBuKt-*|?jdVYYx4FIDE0BbWB6|z```cp5C8!X009sH0T2KI5C8!X
z009tq?FpQ9baCSgOWc{S*z5iry;gb5@&9vf`1xyJKokH3KmY_l00ck)1V8`;KmY_l
z00dq`0y#%_Dcaxa-z%Eq|9NhB{xw8|EQ0_DfB*=900@8p2!H?xfB*=9KwAlvrX6fg
z`Z@oD3#}fn>pS<1UC@fAP|7wXF#A!fnznXzVq#-sW1M-fWwQ4(c{<L_1x%=GG#d;B
zCO)8>%=nPXYzhd*eH#ihIOUh6!j^9T-!c4zV;>wK00JNY0w4eaAOHd&00JNY0w4ea
ztt9ZMSD!p+`C5OoxdVOu-`TUmo&4{~KMnZ$zI1l?tepD##B?i*i@1*ffv$-oPzV$O
z1lmDhXXFAibN8V1z_ESLFmq?l-nM$O*tEO7mQk1u;`XQbcC9JA_GiF7CdthFvfRuz
zp3kJB2k`C;6DFNvsp!~_wp}`U4%&%EPtVmk$>_=NDHq+4ws+^v^GqUgNp@`CZo5Pj
znZ1^xogy(3e5-RZG17OB8DCB7iY_!s_U;V5&!mzqW&5_7kl34$+6bwQNm+vUB$7y_
z!;2(kSTJ?BF#~RrPDWx2#7J^X%o%puHw_c*`d?AnGS0WWH{R=Y-M+Y|0rkqlpK2M%
zuR5G9ZM^H$?YZuZ8qzeD6|3HLmv7eSXPsw*k^F`iPxvYkH+$EF&0u-$>u1!g++b4Q
z_G*bd6|^iRjjx2Ld7p%j)F$rrK6SeLM_7Lx?fdB+%Y<AT<!hOeYjy8m7FgMKc>7D(
z5sS<0b!ss(Cee*?%gk-rfIbPw=8SiiBvV2}<%?_DmSMBi4CeJ(rpC)AC=={OAxn$l
zSzd1v<D|U}z$f9wG<$Q9nLpjWua9LqDoUT)bL=C!V`j6~(G0lUEMSeJX^~tX|8vYB
z01glU0T2KI5C8!X009sH0T2KI5CDOrOTgIwf6~vf4-OCj0T2KI5C8!X009sH0Xu;&
zmK@#O&p)&~;T!}&00drF0$BflT_uNng8&GC00@8p2!H?xfB*=900@A<{t4*g{~qoi
z+^IhgboEBN9<kFSdptSmv3C7D6*{o%Cr1lZqepqyk6FU<uAe!6kx%g?9wY8n7urpX
zT|aUm*Rbj5CchAg5i2kAiFmZlJ9${f5<C%e<OYc^&W*eE%{<o#ADer!^lhe%D%*qh
zEZ+fbTwh~5ZRu32CDX}GXjT!+`j}pt3%6GvD*`7MZjcy%$CSRY56!kci$s#K^x`6k
zC-nVXd+ba*1WL@R(paTbzG|7w&1BCeM=h29nZIKyJ$dw1dh*b#G~07&==A=!Tpj|&
zu4$c**A~zxAAeZxi0*Z))`pILtqs-J+IeE_P;TQRQl;@`&m%{oN2;l~?6K;p?*mJl
Zzc}A9Z4MrNZ4SOZ+Wcaj(eY~h{|7Fii(UW#

literal 0
HcmV?d00001