From c45bce79a8d6f6e3ae0660470f78de063249e93d Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Thu, 16 Jan 2025 12:08:45 +0000 Subject: [PATCH 1/2] fix null exception --- .../graph_builder/module/osm/OsmModule.java | 10 ++++++---- .../module/osm/SafetyValueNormalizer.java | 5 +++-- .../graph_builder/module/osm/StreetEdgePair.java | 3 ++- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java index 195d36b9ed1..39d27a5739b 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java @@ -9,6 +9,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import javax.annotation.Nullable; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.LineString; @@ -425,8 +426,9 @@ private void buildBasicGraph() { normalizer.applyWayProperties(street, backStreet, wayData, way); platform.ifPresent(plat -> { - osmInfoGraphBuildRepository.addPlatform(street, plat); - osmInfoGraphBuildRepository.addPlatform(backStreet, plat); + for (var s : streets.asIterable()) { + osmInfoGraphBuildRepository.addPlatform(s, plat); + } }); applyEdgesToTurnRestrictions(way, startNode, endNode, street, backStreet); @@ -488,8 +490,8 @@ private void applyEdgesToTurnRestrictions( OsmWay way, long startNode, long endNode, - StreetEdge street, - StreetEdge backStreet + @Nullable StreetEdge street, + @Nullable StreetEdge backStreet ) { /* Check if there are turn restrictions starting on this segment */ Collection restrictionTags = osmdb.getFromWayTurnRestrictions(way.getId()); diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/SafetyValueNormalizer.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/SafetyValueNormalizer.java index 2cb5b01dcc6..82c14c27cbf 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/SafetyValueNormalizer.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/SafetyValueNormalizer.java @@ -2,6 +2,7 @@ import java.util.HashSet; import java.util.Set; +import javax.annotation.Nullable; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; import org.opentripplanner.graph_builder.issues.Graphwide; import org.opentripplanner.osm.model.OsmWithTags; @@ -74,8 +75,8 @@ void applySafetyFactors() { } void applyWayProperties( - StreetEdge street, - StreetEdge backStreet, + @Nullable StreetEdge street, + @Nullable StreetEdge backStreet, WayProperties wayData, OsmWithTags way ) { diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/StreetEdgePair.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/StreetEdgePair.java index 7e87838c4d5..76ce28617fb 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/StreetEdgePair.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/StreetEdgePair.java @@ -1,9 +1,10 @@ package org.opentripplanner.graph_builder.module.osm; import java.util.ArrayList; +import javax.annotation.Nullable; import org.opentripplanner.street.model.edge.StreetEdge; -public record StreetEdgePair(StreetEdge main, StreetEdge back) { +public record StreetEdgePair(@Nullable StreetEdge main, @Nullable StreetEdge back) { /** * Return the non-null elements of this pair as an Iterable. */ From 4849d5bf2fc602858a5e9d8aa87f2fbfd29b22d5 Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Thu, 16 Jan 2025 13:59:56 +0000 Subject: [PATCH 2/2] add test case for the OSM which triggered this bug --- .../module/osm/OsmModuleTest.java | 21 ++++++++++++++++++ .../module/osm/sky_campus.osm.pbf | Bin 0 -> 9820 bytes 2 files changed, 21 insertions(+) create mode 100644 application/src/test/resources/org/opentripplanner/graph_builder/module/osm/sky_campus.osm.pbf diff --git a/application/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmModuleTest.java b/application/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmModuleTest.java index 6de345ddd9c..a25bc6ef033 100644 --- a/application/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmModuleTest.java +++ b/application/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmModuleTest.java @@ -164,6 +164,27 @@ public void testBuildGraphDetailed() { } } + /** + * There is a one-way road which is also marked as a platform in Sky Campus which crashed OSM + */ + @Test + void testCrappyOsmPlatform() { + var deduplicator = new Deduplicator(); + var graph = new Graph(deduplicator); + var osmInfoRepository = new DefaultOsmInfoGraphBuildRepository(); + var osmModule = OsmModule + .of( + new OsmProvider(RESOURCE_LOADER.file("sky_campus.osm.pbf"), false), + graph, + osmInfoRepository, + new DefaultVehicleParkingRepository() + ) + .withBoardingAreaRefTags(Set.of("naptan:AtcoCode")) + .build(); + osmModule.buildGraph(); + assertTrue(true); + } + @Test public void testBuildAreaWithoutVisibility() { testBuildingAreas(true); diff --git a/application/src/test/resources/org/opentripplanner/graph_builder/module/osm/sky_campus.osm.pbf b/application/src/test/resources/org/opentripplanner/graph_builder/module/osm/sky_campus.osm.pbf new file mode 100644 index 0000000000000000000000000000000000000000..bb484d571b2c2a600ec8be2d9542659f5548a11b GIT binary patch literal 9820 zcmV-iCZpK^000gO2~Sf^NM&JUWpWsl0T6}(8jb;Yoa2(>nErmk4km%)8(X$9Ni?ip zcJ4ou!jG?W&i-do;`1-g4NlHT%}vw|Gte_r;&MsND^B&xPf0D-5)95SD$xt6EJ%$n z$wpF3OTx94FZ=u~J$}td=k3 zI5UM*qLiAOnC|89#?4zcah!7Z+m@1DyJS||Io3=ex3=bIjHN`eQ)

OlVfitSux~ z<{Y$=bAlbR?K}2J_HvAsbhpG@&aoRMd%w0(SSdJxsmyGznT3=?d!^Q#hEgHfwoPVc zVlCTlC9$yPw4TE8_(WQB*j%o(=0p<)6NPje$0+4CW)dq7Z0)g3%QlK+=}A}u*_-X^}b?@*d3N&eP+n4CajDYcR*_i~!M#*#FVS`mjOAI{~}Xz zw1srH)Pe)X1aoW$C$hA$P|Du6R>~F9y?jmN3Xa)9ETjkQ)8q7}KYhJqDJQg$ZBv?T z=VPIF$Q9O7PHZVBCrx%scgR%5H?^_mG|i<1orEQ|HZhg+DG8M_OQ}*~N_MbhhY9hv zi^EnVTyFA~oqG+pEHxx8Hr%qzaEp@E$8a5wvdhNQf>er+z~Q7e&+m6@F6A-Av~X8LETsY1dD71G^u z3sqCGr9?*BWNj%|RZK#zDM^EezOOBP3j!H=0+@dXJa-~vc zy<;qXxs7s%RAFVf-hwn$Au}VrF#J-g+#^@)Bv)|a)apI5oicNYl7#Wc<;#p$8o#&9 zn4@>e_mF5gYa3H*r9^cjv@K;;q_*ZVYl3BJqta=r%C+RVI*GrKE6hKWDE4l#*)CB$ zyZL4k3rjvfftii<4*n>+MQSRsmh9Q7eR-93`6{lDF1#?H10(9=F$}qR3kPTWpsKpVg@cF>GQXPj!H0;PF^-en~wYl2QYS1w+WXp4E>@aTc){ zGMIrGF84(#L%(k+TGw0Y_E#7k`uMCM5X`gswBzoLB2bpob^$=U>aJXv;@y(M;{Fqb zQM_Y+r{n=5eL6fIplhX#jVysqI~=MBZ?&h^!JKQ;VeLaFPY7%8CeMP59@dVS26N!~ z>HrvFUm@V5W9$ITDba@h0c~?a?sU4Uf`iQ->zEk{*dAze5PF9BTfzs`9$qvaqkZ1D|wv(P=foE62^oaDsBqKVjkGxFm~_qbKIgga0^@ za>1m6z9HW8nL7X{9rP6?ISgKhP;Vmxzxhcs}k6vcJ;y#|K`dAQ{4}@feowEi$uNbX-ggB9VDW2GVKK-%}2m`^bCxp)Bb6MtXZaIIo3?+4Xmw|-zN2}hT2+xT5Exr2dA{f26jO!W>L3;XlF=XKy zUZK6{fdLnQfq)c-L44R0ILhU2z$3#sWg%jp+q^0Do50U?C*3VWuv#2x#bqS_B!#Z+lD85*lArt(hh2=fqU$Q~ZIHlal8p<}65 zvTu`4rhf^bctDEUm+NNHn{i0vuTGJ0mr>8Cdi1u?C-Iz`|F^2zQNE)KsObF4WqsY{%j)t10riH&*MNwtE{D~w5$5IT z#=@HD83lxi)90yy^jOvB*QC19wAHWPkv-hFFo*E733c+pmBmy#KM>+u@t80b z;L~h4aY)tNB-^(f?MU_`ZPla~Smr?V^rTO;|pUMjg{Sl_D zK>NXySY>}D=>#T|=UTt`5Pv zE`+JvOmdo{JhGjPno6UVS9*qMp9p%S)so&fr`}!K;4V!K+OOl?S+sznqP=|}l^aNg z%(x>W-eWsq9w3eE;ZdW{WU(g(ve>S=qr&?Kr4)}wQEA>$E3dhn5?mdrUQ94Ek}pl6 z%EB5!Kvc9Qk3u=b`OJwM&0NjHjs^w2Ls9(==hsXGK63l0Fol3VDNKE9yx1k@`;AnI zo*%rv=vjgkAM4nRkEknsKk5#s)~6b~GN(M(D$j~yToIN}yRTN2jmjC&rOtF`PAT=t zn39z8<(pm!z5qlNlqb-Ign60an?R2l?MIW<_Y-!1@hwzN`l{;fYf|sEA%j=%XpHIA zX(Ie=Lb~8LiK^lULaIg(rXnNipWa|qXJg8Blm3@H;QE6=V9ffM>}dWX`e)r>^0QiC zt*zDT#`vEhK93g^M*g(jWY<%zEJZn7y34vIHsPUg2mIoxyT!Lz%F6@vS2eTzn%}81 zIFbp!a2!)SNor*unS?muR&szp#LIDvK62K!77Zk|PH2uqpJhV`Q=abo1RjJ{+El+h zkVg0=!Phk%W^WLFWrqBwp6<|zEbn&he#xe%u3t*8w!R)sVEikv{srW9vln|4=I7@} ze8U_#cYL;B zKDB6}YG(ZIOFVqJ>6HO|sXsp1{t_dBekuKy{`F|V{D1EJzkvSw3qN`T=FtDMFAp!$ zx-!;oXx}{)(g2$up&A!(xf`GILJ@`N%+?dr&u$I)B4>@?%cl*uF~H2q*^ z$9-|DFxd&TIKwWw8g*x(yRq=t@}f28bwa0S`y$V8svJRr=$hWqDIOdA#S!Wyde@JG zO!1k?&i&B#G#J*-(YUIW41=JP;Ht5+EiA=@OA7D} z9EU{Lskj$AqTS>-RM zbZzOBuLlj|K6-4F;{+Wi#m_vtu?e^YftqQ4bnP@p@R;^u`|0v=*(2Q%uJ4F~!H zU4(<^4QpGcADkS=9K-Q+A8cg0aI03VtWZ3RLKYo2aGw)^_M^)NeHz|>z43mu!4dgI zVE_K8slQ&T`UOzUrm9z5husCx%M0hVfvidC2F+hoe-Svh2R71=V53dsA$a;a>|`tT zLl~b~*=#8&2D$JA3PlB|3tk6X3K?hM3U;L;&813ne^5eRjfB^FU>9xqd&m!1f*r48?vMue$3kr#sa>$r@tr{3 zduHW%AVS9x#SY-$qsVTzoh)0uL2Z9MYFeLi8b!>^Usdv9awbTh6*$L#)g5Q%K9e{- z^3w(%T)Q&+gIlZYS2!*`2yB-g@P#)(?~Y_pwJmq1Evl^qi3Xu}=-^YJGX%ta+#~Q+ zADvsN*~i>p+dT}5ze-*byL?EuhK~Cn1En1YUh|{&<1BHT{P+lZaucPu;j<`jek!_w zB2mY;g{TDGLgicovQ=w99b5-0*D6Ckf;yCgD$v6pFQBWa3f(6<+jk359+!fWB0v=y z=Bm(L^)mEOlm=^1BB+sC_yhly2lW0E(-iNd&cvFnN^?UO%y z!%mpLwOHg~l$3?i9W!6CMi4yFBY6nqFy>TT6EJuGe}jVmV) zf+pmHy1_N@r{z;O@5}}cz?sC-uj|h^(!DsGix<=jYM4QR3v-ITh}-ZmdJNzDs%+(< z_Y?F7^&dcYp&vKMR<5XC~C9p)S?x!+5yIGupime(YORf!yolGhl1On4;G_D@|3!P zU2r1GL>FKbIt$B~oImH|HoXQx_q40?YK;PDSJ(@p793uc11$76pH$o90K!4`-*TsY zV6T%n0ImpY$l2yzuum;s;J_JVJHS5$Fc48RBVdL6FJ3YiiD`8KtD&i-ts@rFy5hh8 z;seoB`K!MP)A;H)NT{y=+>hbuGiDl!SRL{BVwTZt@tnEy=BuXTlLZSG&6~r2qox@R z64QkEI|MRWESjOizRkY#{C~&Z6&o)RE?u_#z4up)dEwJlA-v%GAEc)>dlm+PVpoY* z8@>tL^T;0vSgkd`u01Kkhl|##$}sD9ABk7IV4P(%Z~R;P%#Z&`?=Pk2dO+e^N6JTX z*M0h%k~sZj{imP(VZ(-v6V?9wX|*@K@qcFit@QM=%pjNLg~bH@AD$?J5C8xG32G>!xthEQ;FlPO>L=IiwkZ@d)pe^m+IB_ zwY|6R@`hxPxn|*J5-7gSx}vNCg5ZX@3nB=jps3(dcda{a6*sJ+R>ie0_nbix3cR1q z@5eurZ@#m9=lst(ALM)+@^6H=BK>WAM1*LcKvcZhEJzYjM>xeu-X#zq2onTC+Z{w{2U*g7wA<q&7 zr*SOkF$NIU5g|BvSSG6R6Qhk^jU?Lol^|Digg`vdV2D$+2*lHdI{fRD5VhoXhVf>B zuwdWp6u_(^T9Am=#7pRjahSs`2}C>CDNdA(e-*5PeIifP;o^kwC?3Sa91a&gkWl}B z@RKI_hdNw@5p29@CGA9enB#Mc#cbszNi<8{%?=yU3>st%ezib!jeZzMxh&lY6A|vP z3q;qgPak7IPh((U|3G8!-hKNLr4;UnAPO_@BgcQ`HM^sc)_ojAw zKxBDWxM+u?s3pM}DdG`r{P$9XAXtd1S5KoYBAj@R4;KZi#W)BOBoGyt#FNPNG@Zr(;-R5;SI}jYCwgKQEhzUIu%D;3B$VywfWD(=FPK10!J@oJW$yiB~Tjli+d*PNS)w?pnK}@p04;rwHC2O|&)#5HloF2p7#(sF+aE z9E}eeiy)buVgz!MbpHKOU)RE5yUWSj!FZ?tXV3Sdi?HK`P+sDrCK6A#y^cCzB2a@7 zZTtj*^h9pGqE?tyG*5KOv;um%98wRbTarQ?lC4L;fWW}O-n|0?dk|GX9jDa16cNr_ z!Gaknfrn)#387A&C_*9gaMDP@35Ez1s7-_oz&xErfC6>7oq`k+>foIg;^7i)f{PD@ zqbsc7t}enI3T}kp>7)pU6Vyjoc~_XjX@iW|M62kC6eoz*z<_>4JIdj-4CS5CqN)^ z?{tcgA|_10xDxp*q^_$(iB2KJCP?8}TaaI*IfH&%3Dt*qH}K9T+CyaS5vmu_HZ0?r ziPq_Ggay48pWuXC5Zwg9{;Gk2zHoyA7%Y80u9|;2=@L#s@R1e!*5Zza-Me#lyt1 z3LqXSo3jccj1*`-Ks}3KC!Vhy3E`oc5+)Sb6AqOi+QG$e!3semotm-IIKHlB8^6U` zV61$!;PhVeAvckYzc*Rqt)Z1FwTDLQrDMD~je=oV;@L()+P3SgXNm6T=`~AKhM7-t zKR3=;S)}x%Onr7PICP7e7r*=57jqGg6x@5(tupQYT!y}IqwpMkY28I(+un z&z%qBdu}+p8jUHz7{e*SN?TB`*1b41b*{>AVbP+XdiBOVOF@0bijA4Q4jrqh(ZxSq zI@3gNd3k!?&=LfO8_e#Q@`J{kR%tj|{}!lG>Q={QC8`YhcenI!P*Z&Ir&|;~Eo)uT zASJDsir?A3ljmiDvXy0kTrc+IG*CnNaFn5QJ65HOtKGDjF=XXrwlgO0p#9pLykcUe zzrAPE!6Ou{jLSXwE*Hq2nhqjr6{pd5GIr+F`i_5k`<-{+10Phnge4DFs|@M)R}C_5 zuGy~i>tOOI-Fp;#plOB+W>2G}(&qzBS5E2M#ZNE$ol%wYkTDdUtMW5Gy>LhA_r8fk zo2BJob5G^=)%}$1lzK0{S2;d&-PpJp^1-{o!9VKUmZfwvuFl-5G^|?eY25Ld--jlT zto>k-RncCYnVr3C9c=c8D0S}v))-yhk=$h}!}O}{1M8#o>81R_ht!;Mj`dlzsj^~Tur*hAeV;y$EUurqW&8OM(nfG2< z>DddcwAZb?dA&qsn3ni>WWALK)a=Wfr_jkeHyj=?ucUT%`?J-%b3RBtn;&b)T3fM~ z(G{G$d21MT`}yuE0WtHhFKIh2=stxxVd=bP;(G(BgUFw9WYTto#EQbqUR{%KX}8`!t5@!h zoqwU*)5OioJN|~SepBXM?d-~|B5f?fwKEU$j4Z90JN3jW$)>O*{0ps&R!m~v?}Mi_RyBiF-&3wM34KNElJ(r>8+pxd^iPB z7-X6>XMBg#+aYO3)1KD8eeLP27uw{V$4iu&hw-LPTpq=y9(*>=+mr6oFHouaSo7zg z&rO`CqHq5JLHbYne)`$KU{I>nWoOoa(x2}h2{P{Am9F&bZF2uQWpjdJ-?fc90vVd# zh2KX@4y6q=P(GBAdWZ6(x_ku8>>Wt;XH4{975p({sWH?y)Ynz_Qg-yavH1BNeUDd`H zF^r+)Oq$wQRoTvOkjdjx&8kFt7M4>OJNMj=*_1Z}r95m1$0&vj)endBc(lNwpof-Jz z5k4az5e9`^4b2mwNxixCUPtBR>h#-fxzE^_MKd;2(6L5B`jkW==Z|fnA-;8Eb?Uyc z8FASsJasQ(0e?f_Ozh6v%i?KJFv!5AyYNF$dR&{jPvhm!cyr9yapMPk32HobTh|v9s0_J#uMRb49sZ5+8)Ndnz4zBk{@InqOFPDwExSU; zAnZDodv92N{L_?gayBN$3|o5P;7hOLdutzPxKG(+S#{GOsapt=UXE={PR+{pG;=%N zbmN!mb2^Yuk>yqN$gctnKAegu{$24^Yk}4+NXy9#sLq*oR!Qd;?%&X!`!hR#Z*mFB zfBlWmh^`>7C%)g-_13c3yxA(l>RB0m>*c+Y4sE~TUvao#cKb2}ZX~-v7T19IUwytC z(N+5&Fan8Q|2JR#xB51J|56Ti)$z;~Dnm{|fvH|myHr$k<5tcFI2wI zeZnr3l`-_!=n6yhb)G16S5hxLV+_v^6ntbn`sk6;FT|uvLj{WC^Ve5v&ttk^s4aRW z4Uzl~q#=}gIhM_9p~F0su01t*ai`Q9D^Gi_x%+&PBByXgT6eAwd+>4!sA)QADBY1g z_b)Pr(wJ56)GHy`W!vsk85>GZQrzFzIpxVG>s9v107M}6t(HBx>cOQ@Y(?R;N(CGn%MN1d3uPy}5_@b52w$m5 z``lQOT%qzanOl^vuq~#4+o097JASJil05^&A z+ZH`ncq_uf|G_CHh!bT--CKgmJN?7!&v2SPLD+>8R#67Swz@`}gu{0m zj^Wo=JGval^d%hF(CIQ`v(U)LHV3D$+UscBoFN@w6zo50P+Y({GzTYA5(0Gse{-Si zf;uIQJf&N>_5{^rubeUb!!C8CDa2b&F~To+>8U}eMmPr7i;7jtejW3&cJ4*?8G@So zkX@BVbTOv!- zjtIE#{j(K$kK`3-3on`d*-}|R1^@g;MgJG4lJt?^Px|lH1Tz~6ZvG>J*^LCZ{1L%5 zjRed7h~WAL!5-|^MnUa=GfLL9;@gd~JA?CZq{g$P>_H0uy)x(2`Y z<1YGA9CZzUoyT8$5OyK#L?}ntE-ORWf>46631I_55kdh%KEhgrT!bGHb|dUSs6g1( zRLkq9VuV73^$6<_9>-Bv!8d#ylY$8Msa0~+em~$8Ekvi?#TDhVM#Ypq9l2)R+BnZU zzrYb;)~}Hju&7`*HNux;2_e3O#8Prno8O5v$8j_qjX9jk(%dLEuSNUH7VH~OqkWow zjD|}vz;XDr>pH+r1gPx$o6FZIo80XcE$~Xe>0;u(v6r$=6>F;Rz?8m|UzyHwJ=s}R zDBW~OQ7w!W)fv27(lFO&zo7)VfD+|1Q!;$H0c@-+#nRsd_>Az4qc{(|ASq(0c&asn zS5Tbdy0dYzDDC6RDSTsF+pfNd+f@`dkd2r3YhhX@Qb`m{Oz$}qT&5HW6d^D-za*`x zxGq~%!Wk3Z^o$SFPQM7gm%L@aCp3h#owv)BO&UV_`7J9|hL?w?1vfN=Gm8@v$IA_& zH+%I+&Xwjwpjse;dQlw_;12!E8w>fzq0q9Mz!Pc~H5ciViu9R-Kqnx^%8yAhuSc^t zWYu&s#nCBLD@wbpO8lkj8@kEo3ROilU7VVv0<&PKlclQ1ps0gfmvTG@a?l>rte*;}#jT`pP}*F1CQsf!eyBa@EG$0O9ukRpT) z4KqZZHM0?78)nU1gjBgn2e1!*AKbyM26xRIIk^4Uhi{w^BFij9FTvKMH&0|P%BkR% z`1lR7@b+=stpv8rEP!^Kj8Gq@wjR)ZV^?f{)>KLJ&P^(&R8Rg6Mb!TS!BgFP0000C z3I|V9O+;aIVHl4A5P<<2i2-<=K zE=nyfDauSP$pp&r=H4Y5iiwR?NtlC8 z;Q!8rk69(&E$CEG_