diff --git a/onebusaway-api-core/src/main/java/org/onebusaway/api/model/transit/BeanFactoryV2.java b/onebusaway-api-core/src/main/java/org/onebusaway/api/model/transit/BeanFactoryV2.java index 635b641eb..1efb63f81 100644 --- a/onebusaway-api-core/src/main/java/org/onebusaway/api/model/transit/BeanFactoryV2.java +++ b/onebusaway-api-core/src/main/java/org/onebusaway/api/model/transit/BeanFactoryV2.java @@ -471,6 +471,8 @@ public StopV2Bean getStop(StopBean stop) { List routeIds = new ArrayList(); for (RouteBean route : stop.getRoutes()) { + if (route == null) + continue; routeIds.add(route.getId()); addToReferences(route); } diff --git a/onebusaway-api-core/src/test/java/org/onebusaway/api/model/transit/BeanFactoryV2Test.java b/onebusaway-api-core/src/test/java/org/onebusaway/api/model/transit/BeanFactoryV2Test.java new file mode 100644 index 000000000..8f7f6ab66 --- /dev/null +++ b/onebusaway-api-core/src/test/java/org/onebusaway/api/model/transit/BeanFactoryV2Test.java @@ -0,0 +1,47 @@ +/** + * Copyright (C) 2026 OneBusAway + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.onebusaway.api.model.transit; + +import static org.junit.Assert.assertTrue; + +import java.util.Collections; + +import org.junit.Test; +import org.onebusaway.transit_data.model.RouteBean; +import org.onebusaway.transit_data.model.StopBean; + +public class BeanFactoryV2Test { + + /** + * A {@link StopBean} can carry a null {@link RouteBean} in its routes list + * when the bundle's stop->route-collection index references a collection that + * no longer resolves (the production trigger for issue #461). The serializer + * must skip it rather than NPE, which would otherwise unwind out of the action + * and emit a bare "null" response body. + */ + @Test + public void getStopSkipsNullRoute() { + BeanFactoryV2 factory = new BeanFactoryV2(true); + + StopBean stop = new StopBean(); + stop.setId("1_stop"); + stop.setRoutes(Collections.singletonList((RouteBean) null)); + + StopV2Bean bean = factory.getStop(stop); + + assertTrue(bean.getRouteIds().isEmpty()); + } +} diff --git a/onebusaway-api-webapp/src/main/java/org/onebusaway/api/actions/api/where/ArrivalsAndDeparturesForStopAction.java b/onebusaway-api-webapp/src/main/java/org/onebusaway/api/actions/api/where/ArrivalsAndDeparturesForStopAction.java index 7dd6aa897..9d9f8bba5 100644 --- a/onebusaway-api-webapp/src/main/java/org/onebusaway/api/actions/api/where/ArrivalsAndDeparturesForStopAction.java +++ b/onebusaway-api-webapp/src/main/java/org/onebusaway/api/actions/api/where/ArrivalsAndDeparturesForStopAction.java @@ -141,18 +141,27 @@ public DefaultHttpHeaders show() throws ServiceException { return setResourceNotFoundResponse(); } - if (isVersion(V1)) { - // Convert data to v1 form - List arrivals = getArrivalsAsV1(result); - StopWithArrivalsAndDeparturesBeanV1 v1 = new StopWithArrivalsAndDeparturesBeanV1( - result.getStop(), arrivals, result.getNearbyStops()); - return setOkResponse(v1); - } else if (isVersion(V2)) { - BeanFactoryV2 factory = getBeanFactoryV2(); - factory.setCustomRouteSort(customRouteSort); - return setOkResponse(factory.getResponse(result)); - } else { - return setUnknownVersionResponse(); + // Bean conversion must stay inside a try/catch: an unexpected failure here + // (e.g. a corrupt bundle record) would otherwise unwind out of the action + // with _response unset, and the ModelDriven serializer would emit a bare + // "null" body with HTTP 200 instead of a proper error. See issue #461. + try { + if (isVersion(V1)) { + // Convert data to v1 form + List arrivals = getArrivalsAsV1(result); + StopWithArrivalsAndDeparturesBeanV1 v1 = new StopWithArrivalsAndDeparturesBeanV1( + result.getStop(), arrivals, result.getNearbyStops()); + return setOkResponse(v1); + } else if (isVersion(V2)) { + BeanFactoryV2 factory = getBeanFactoryV2(); + factory.setCustomRouteSort(customRouteSort); + return setOkResponse(factory.getResponse(result)); + } else { + return setUnknownVersionResponse(); + } + } catch (Exception any) { + _log.error("Failed to build arrivals-and-departures response for stop {}", _id, any); + return setExceptionResponse(); } } diff --git a/onebusaway-transit-data-federation/src/main/java/org/onebusaway/transit_data_federation/impl/beans/StopBeanServiceImpl.java b/onebusaway-transit-data-federation/src/main/java/org/onebusaway/transit_data_federation/impl/beans/StopBeanServiceImpl.java index 803c4ed84..12be0ddda 100644 --- a/onebusaway-transit-data-federation/src/main/java/org/onebusaway/transit_data_federation/impl/beans/StopBeanServiceImpl.java +++ b/onebusaway-transit-data-federation/src/main/java/org/onebusaway/transit_data_federation/impl/beans/StopBeanServiceImpl.java @@ -39,12 +39,16 @@ import org.onebusaway.transit_data_federation.services.transit_graph.TransitGraphDao; import org.onebusaway.utility.text.NaturalStringOrder; import org.onebusaway.utility.text.StringLibrary; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @Component class StopBeanServiceImpl implements StopBeanService { + private static final Logger _log = LoggerFactory.getLogger(StopBeanServiceImpl.class); + private static RouteBeanComparator _routeBeanComparator = new RouteBeanComparator(); private TransitGraphDao _transitGraphDao; @@ -143,6 +147,12 @@ private void fillTransfersForStopBean(StopEntry stop, StopNarrative narrative, S List routeBeans = new ArrayList<>(staticRoutes.size()); for (AgencyAndId routeId : staticRoutes) { RouteBean staticRouteBean = _routeBeanService.getRouteForId(routeId); + if (staticRouteBean == null) { + // Same orphan-route guard as fillRoutesForStopBean below; see issue #461. + _log.warn("stop {} references static route {} that does not resolve to a route; skipping", + stop.getId(), routeId); + continue; + } routeBeans.add(staticRouteBean); } bean.setStaticRoutes(routeBeans); @@ -162,6 +172,15 @@ private void fillRoutesForStopBean(StopEntry stop, StopBean sb, AgencyServiceInt for (AgencyAndId routeCollectionId : routeCollectionIds) { RouteBean bean = _routeBeanService.getRouteForId(routeCollectionId); + if (bean == null) { + // A corrupt stop->route-collection index entry can reference a + // collection that no longer resolves to a route. Dropping it here keeps + // a single bad entry from NPEing bean serialization and blanking the + // entire API response. See issue #461. + _log.warn("stop {} references route collection {} that does not resolve to a route; skipping", + stop.getId(), routeCollectionId); + continue; + } routeBeans.add(bean); } diff --git a/onebusaway-transit-data-federation/src/main/java/org/onebusaway/transit_data_federation/impl/beans/StopWithArrivalsAndDeparturesBeanServiceImpl.java b/onebusaway-transit-data-federation/src/main/java/org/onebusaway/transit_data_federation/impl/beans/StopWithArrivalsAndDeparturesBeanServiceImpl.java index 705685459..365c01f92 100644 --- a/onebusaway-transit-data-federation/src/main/java/org/onebusaway/transit_data_federation/impl/beans/StopWithArrivalsAndDeparturesBeanServiceImpl.java +++ b/onebusaway-transit-data-federation/src/main/java/org/onebusaway/transit_data_federation/impl/beans/StopWithArrivalsAndDeparturesBeanServiceImpl.java @@ -74,8 +74,14 @@ public StopWithArrivalsAndDeparturesBean getArrivalsAndDeparturesByStopId( List nearbyStopIds = _nearbyStopsBeanService.getNearbyStops( stop, 100); List nearbyStops = new ArrayList(); - for (AgencyAndId nearbyStopId : nearbyStopIds) - nearbyStops.add(_stopBeanService.getStopForId(nearbyStopId, serviceInterval)); + for (AgencyAndId nearbyStopId : nearbyStopIds) { + StopBean nearbyStop = _stopBeanService.getStopForId(nearbyStopId, serviceInterval); + if (nearbyStop == null) { + _log.warn("nearby stop {} did not resolve to a stop; skipping", nearbyStopId); + continue; + } + nearbyStops.add(nearbyStop); + } List situations = _serviceAlertsBeanService.getServiceAlertsForStopId( query.getTime(), id); diff --git a/onebusaway-transit-data-federation/src/test/java/org/onebusaway/transit_data_federation/impl/beans/StopBeanServiceImplTest.java b/onebusaway-transit-data-federation/src/test/java/org/onebusaway/transit_data_federation/impl/beans/StopBeanServiceImplTest.java index 288d24747..108033ad1 100644 --- a/onebusaway-transit-data-federation/src/test/java/org/onebusaway/transit_data_federation/impl/beans/StopBeanServiceImplTest.java +++ b/onebusaway-transit-data-federation/src/test/java/org/onebusaway/transit_data_federation/impl/beans/StopBeanServiceImplTest.java @@ -19,7 +19,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -116,4 +118,79 @@ public void testGetStopForId() { assertSame(route, routes.get(0)); } + + /** + * A stop's route-collection index can reference a collection that no longer + * resolves to a route bean (e.g. a corrupt service-date entry in the bundle). + * {@link RouteBeanService#getRouteForId} then returns null. We must not let a + * null {@link RouteBean} leak into the stop's routes list: in production a + * single such orphan route survives the sort (a lone element is never + * compared) and later NPEs in bean serialization, blanking the whole API + * response with a literal "null". + */ + @Test + public void testGetStopForId_skipsUnresolvableRoute() { + + AgencyAndId stopId = new AgencyAndId("29", "1109"); + + StopEntryImpl stopEntry = new StopEntryImpl(stopId, 47.1, -122.1); + Mockito.when(_transitGraphDao.getStopEntryForId(stopId)).thenReturn( + stopEntry); + + StopNarrative.Builder builder = StopNarrative.builder(); + builder.setName("stop name"); + StopNarrative stop = builder.create(); + Mockito.when(_narrativeService.getStopForId(stopId)).thenReturn(stop); + + AgencyAndId orphanRouteId = new AgencyAndId("1", "orphan"); + + Set routeIds = new HashSet(); + routeIds.add(orphanRouteId); + Mockito.when(_routeService.getRouteCollectionIdsForStop(stopId)).thenReturn( + routeIds); + + // the corrupt collection id resolves to null + Mockito.when(_routeBeanService.getRouteForId(orphanRouteId)).thenReturn(null); + + StopBean stopBean = _service.getStopForId(stopId, null); + + assertNotNull(stopBean); + List routes = stopBean.getRoutes(); + assertTrue("unresolvable route must not leak into routes list", routes.isEmpty()); + } + + /** + * The static-routes path resolves route beans the same way as the primary + * routes path, so an unresolvable static route must likewise be dropped + * rather than leak a null into the stop's static routes list. See issue #461. + */ + @Test + public void testGetStopForId_skipsUnresolvableStaticRoute() { + + AgencyAndId stopId = new AgencyAndId("29", "1109"); + + StopEntryImpl stopEntry = new StopEntryImpl(stopId, 47.1, -122.1); + Mockito.when(_transitGraphDao.getStopEntryForId(stopId)).thenReturn( + stopEntry); + + StopNarrative.Builder builder = StopNarrative.builder(); + builder.setName("stop name"); + Mockito.when(_narrativeService.getStopForId(stopId)).thenReturn(builder.create()); + + // no active routes, so only the static-route path populates the bean + Mockito.when(_routeService.getRouteCollectionIdsForStop(stopId)).thenReturn( + new HashSet()); + + AgencyAndId orphanStaticRouteId = new AgencyAndId("1", "orphan"); + Mockito.when(_narrativeService.getStaticRoutes(stopId)).thenReturn( + Collections.singletonList(orphanStaticRouteId)); + // the corrupt static route resolves to null + Mockito.when(_routeBeanService.getRouteForId(orphanStaticRouteId)).thenReturn(null); + + StopBean stopBean = _service.getStopForId(stopId, null); + + assertNotNull(stopBean); + assertTrue("unresolvable static route must not leak into static routes list", + stopBean.getStaticRoutes().isEmpty()); + } }