Merge pull request #462 from OneBusAway/fix/arrivals-null-route-bean-npe

Fix arrivals-and-departures returning bare "null" on unresolvable route refs

Author: aaronbrethorst

onebusaway-api-core/src/main/java/org/onebusaway/api/model/transit/BeanFactoryV2.java

@@ -471,6 +471,8 @@ public StopV2Bean getStop(StopBean stop) {
 
     List<String> routeIds = new ArrayList<String>();
     for (RouteBean route : stop.getRoutes()) {
+      if (route == null)
+        continue;
       routeIds.add(route.getId());
       addToReferences(route);
     }

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());
+  }
+}

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<ArrivalAndDepartureBeanV1> 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<ArrivalAndDepartureBeanV1> 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();
     }
   }
 

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<RouteBean> 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);
     }
 

onebusaway-transit-data-federation/src/main/java/org/onebusaway/transit_data_federation/impl/beans/StopWithArrivalsAndDeparturesBeanServiceImpl.java

@@ -74,8 +74,14 @@ public StopWithArrivalsAndDeparturesBean getArrivalsAndDeparturesByStopId(
     List<AgencyAndId> nearbyStopIds = _nearbyStopsBeanService.getNearbyStops(
         stop, 100);
     List<StopBean> nearbyStops = new ArrayList<StopBean>();
-    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<ServiceAlertBean> situations = _serviceAlertsBeanService.getServiceAlertsForStopId(
         query.getTime(), id);

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<AgencyAndId> routeIds = new HashSet<AgencyAndId>();
+    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<RouteBean> 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>());
+
+    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());
+  }
 }