Merge pull request #188 from ibi-group/upstream-merge-2023-10-13

Upstream merge 2023-10-13

docs/Changelog.md

@@ -17,6 +17,8 @@ based on merged pull requests. Search GitHub issues and pull requests for smalle
 - Fix check for OSM relation members not being present in the extract [#5379](https://github.com/opentripplanner/OpenTripPlanner/pull/5379)
 - Add a configurable limit for the search window [#5293](https://github.com/opentripplanner/OpenTripPlanner/pull/5293)
 - Fix fare calculation for combined interlined legs [#5408](https://github.com/opentripplanner/OpenTripPlanner/pull/5408)
+- Fix board slack list mapping in Transmodel API [#5420](https://github.com/opentripplanner/OpenTripPlanner/pull/5420)
+- Fix flexible quay querying in Transmodel API [#5417](https://github.com/opentripplanner/OpenTripPlanner/pull/5417)
 [](AUTOMATIC_CHANGELOG_PLACEHOLDER_DO_NOT_REMOVE)
 
 ## 2.4.0 (2023-09-13)

pom.xml

@@ -60,7 +60,7 @@
         <!-- Lib versions - keep list sorted on property name -->
         <geotools.version>29.2</geotools.version>
         <google.dagger.version>2.48.1</google.dagger.version>
-        <jackson.version>2.15.2</jackson.version>
+        <jackson.version>2.15.3</jackson.version>
         <jersey.version>3.1.3</jersey.version>
         <junit.version>5.10.0</junit.version>
         <micrometer.version>1.11.5</micrometer.version>

src/ext-test/java/org/opentripplanner/ext/fares/impl/OrcaFareServiceTest.java

@@ -75,9 +75,9 @@ public static void setUpClass() {
    * types.
    */
   private static void calculateFare(List<Leg> legs, FareType fareType, Money expectedPrice) {
-    ItineraryFares fare = new ItineraryFares();
-    orcaFareService.populateFare(fare, USD, fareType, legs, null);
-    assertEquals(expectedPrice, fare.getFare(fareType));
+    var itinerary = new Itinerary(legs);
+    var itineraryFares = orcaFareService.calculateFares(itinerary);
+    assertEquals(expectedPrice, itineraryFares.getFare(fareType));
   }
 
   private static void assertLegFareEquals(
@@ -642,25 +642,27 @@ private static Itinerary createItinerary(
     String firstStopName,
     String lastStopName
   ) {
+    // Use the agency ID as feed ID to make sure that we have a new feed ID for each different agency
+    // This tests to make sure we are calculating transfers across feeds correctly.
     Agency agency = Agency
-      .of(new FeedScopedId(FEED_ID, agencyId))
+      .of(new FeedScopedId(agencyId, agencyId))
       .withName(agencyId)
       .withTimezone(ZoneIds.NEW_YORK.getId())
       .build();
 
     // Set up stops
     RegularStop firstStop = RegularStop
-      .of(new FeedScopedId(FEED_ID, "1"))
+      .of(new FeedScopedId(agencyId, "1"))
       .withCoordinate(new WgsCoordinate(1, 1))
       .withName(new NonLocalizedString(firstStopName))
       .build();
     RegularStop lastStop = RegularStop
-      .of(new FeedScopedId(FEED_ID, "2"))
+      .of(new FeedScopedId(agencyId, "2"))
       .withCoordinate(new WgsCoordinate(1, 2))
       .withName(new NonLocalizedString(lastStopName))
       .build();
 
-    FeedScopedId routeFeedScopeId = new FeedScopedId(FEED_ID, routeId);
+    FeedScopedId routeFeedScopeId = new FeedScopedId(agencyId, routeId);
     NonLocalizedString longName = null;
     if (routeLongName != null) {
       longName = new NonLocalizedString(routeLongName);

src/ext/java/org/opentripplanner/ext/fares/impl/DefaultFareService.java

@@ -90,6 +90,15 @@ public Map<FareType, Collection<FareRuleSet>> getFareRulesPerType() {
     return fareRulesPerType;
   }
 
+  /**
+   * Takes a legs and returns a map of their agency's feed id and all corresponding legs.
+   */
+  protected Map<String, List<Leg>> fareLegsByFeed(List<Leg> fareLegs) {
+    return fareLegs
+      .stream()
+      .collect(Collectors.groupingBy(leg -> leg.getAgency().getId().getFeedId()));
+  }
+
   @Override
   public ItineraryFares calculateFares(Itinerary itinerary) {
     var fareLegs = itinerary
@@ -105,9 +114,7 @@ public ItineraryFares calculateFares(Itinerary itinerary) {
     if (fareLegs.isEmpty()) {
       return null;
     }
-    var fareLegsByFeed = fareLegs
-      .stream()
-      .collect(Collectors.groupingBy(leg -> leg.getAgency().getId().getFeedId()));
+    var fareLegsByFeed = fareLegsByFeed(fareLegs);
 
     ItineraryFares fare = ItineraryFares.empty();
     boolean hasFare = false;

src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java

@@ -613,6 +613,15 @@ protected Collection<FareRuleSet> fareRulesForFeed(FareType fareType, String fee
     return fareRulesPerType.get(fareType);
   }
 
+  /**
+   * Disables functionality grouping legs by their feed.
+   * This ensures we can calculate transfers between agencies/feeds.
+   */
+  @Override
+  protected Map<String, List<Leg>> fareLegsByFeed(List<Leg> fareLegs) {
+    return Map.of(FEED_ID, fareLegs);
+  }
+
   /**
    * Check if trip falls within the transfer time window.
    */

src/ext/java/org/opentripplanner/ext/transmodelapi/mapping/preferences/TransitPreferencesMapper.java

@@ -20,7 +20,7 @@ public static void mapTransitPreferences(
       callWith.argument("boardSlackDefault", builder::withDefaultSec);
       callWith.argument(
         "boardSlackList",
-        (Integer v) -> TransportModeSlack.mapIntoDomain(builder, v)
+        (Object v) -> TransportModeSlack.mapIntoDomain(builder, v)
       );
     });
     transit.withAlightSlack(builder -> {

src/ext/java/org/opentripplanner/ext/transmodelapi/model/stop/QuayType.java

@@ -15,20 +15,18 @@
 import java.time.Instant;
 import java.time.ZoneId;
 import java.util.Collection;
-import java.util.Locale;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.stream.Collectors;
 import org.opentripplanner.ext.transmodelapi.model.EnumTypes;
 import org.opentripplanner.ext.transmodelapi.model.plan.JourneyWhiteListed;
 import org.opentripplanner.ext.transmodelapi.model.scalars.GeoJSONCoordinatesScalar;
 import org.opentripplanner.ext.transmodelapi.support.GqlUtil;
 import org.opentripplanner.framework.graphql.GraphQLUtils;
-import org.opentripplanner.framework.i18n.I18NString;
 import org.opentripplanner.model.TripTimeOnDate;
 import org.opentripplanner.routing.stoptimes.ArrivalDeparture;
 import org.opentripplanner.transit.model.basic.Accessibility;
 import org.opentripplanner.transit.model.basic.TransitMode;
+import org.opentripplanner.transit.model.network.TripPattern;
 import org.opentripplanner.transit.model.site.AreaStop;
 import org.opentripplanner.transit.model.site.GroupStop;
 import org.opentripplanner.transit.model.site.RegularStop;
@@ -184,15 +182,15 @@ public static GraphQLObjectType create(
           .withDirective(gqlUtil.timingData)
           .description("List of lines servicing this quay")
           .type(new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(lineType))))
-          .dataFetcher(environment -> {
-            return GqlUtil
+          .dataFetcher(environment ->
+            GqlUtil
               .getTransitService(environment)
               .getPatternsForStop(environment.getSource(), true)
               .stream()
-              .map(pattern -> pattern.getRoute())
+              .map(TripPattern::getRoute)
               .distinct()
-              .collect(Collectors.toList());
-          })
+              .toList()
+          )
           .build()
       )
       .field(
@@ -202,11 +200,9 @@ public static GraphQLObjectType create(
           .withDirective(gqlUtil.timingData)
           .description("List of journey patterns servicing this quay")
           .type(new GraphQLNonNull(new GraphQLList(journeyPatternType)))
-          .dataFetcher(environment -> {
-            return GqlUtil
-              .getTransitService(environment)
-              .getPatternsForStop(environment.getSource(), true);
-          })
+          .dataFetcher(environment ->
+            GqlUtil.getTransitService(environment).getPatternsForStop(environment.getSource(), true)
+          )
           .build()
       )
       .field(
@@ -317,7 +313,7 @@ public static GraphQLObjectType create(
             );
             Integer timeRangeInput = environment.getArgument("timeRange");
             Duration timeRange = Duration.ofSeconds(timeRangeInput.longValue());
-            RegularStop stop = environment.getSource();
+            StopLocation stop = environment.getSource();
 
             JourneyWhiteListed whiteListed = new JourneyWhiteListed(environment);
             Collection<TransitMode> transitModes = environment.getArgument("whiteListedModes");
@@ -343,7 +339,7 @@ public static GraphQLObjectType create(
               .sorted(TripTimeOnDate.compareByDeparture())
               .distinct()
               .limit(numberOfDepartures)
-              .collect(Collectors.toList());
+              .toList();
           })
           .build()
       )
@@ -353,12 +349,12 @@ public static GraphQLObjectType create(
           .name("situations")
           .description("Get all situations active for the quay.")
           .type(new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(ptSituationElementType))))
-          .dataFetcher(env -> {
-            return GqlUtil
+          .dataFetcher(env ->
+            GqlUtil
               .getTransitService(env)
               .getTransitAlertService()
-              .getStopAlerts(((StopLocation) env.getSource()).getId());
-          })
+              .getStopAlerts(((StopLocation) env.getSource()).getId())
+          )
           .build()
       )
       .field(

src/main/java/org/opentripplanner/model/plan/legreference/LegReference.java

@@ -1,11 +1,13 @@
 package org.opentripplanner.model.plan.legreference;
 
+import javax.annotation.Nullable;
 import org.opentripplanner.model.plan.Leg;
 import org.opentripplanner.transit.service.TransitService;
 
 /**
  * Marker interface for various types of leg references
  */
 public interface LegReference {
+  @Nullable
   Leg getLeg(TransitService transitService);
 }

src/main/java/org/opentripplanner/model/plan/legreference/ScheduledTransitLegReference.java

@@ -2,6 +2,7 @@
 
 import java.time.LocalDate;
 import java.time.ZoneId;
+import javax.annotation.Nullable;
 import org.opentripplanner.framework.time.ServiceDateUtils;
 import org.opentripplanner.model.Timetable;
 import org.opentripplanner.model.plan.ScheduledTransitLeg;
@@ -11,10 +12,12 @@
 import org.opentripplanner.transit.model.timetable.Trip;
 import org.opentripplanner.transit.model.timetable.TripTimes;
 import org.opentripplanner.transit.service.TransitService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * A reference which can be used to rebuild an exact copy of a {@link ScheduledTransitLeg} using the
- * {@Link RoutingService}
+ * {@link org.opentripplanner.routing.api.RoutingService}
  */
 public record ScheduledTransitLegReference(
   FeedScopedId tripId,
@@ -23,30 +26,71 @@ public record ScheduledTransitLegReference(
   int toStopPositionInPattern
 )
   implements LegReference {
+  private static final Logger LOG = LoggerFactory.getLogger(ScheduledTransitLegReference.class);
+
+  /**
+   * Reconstruct a scheduled transit leg from this scheduled transit leg reference.
+   * Since the transit model could have been modified between the time the reference is created
+   * and the time the transit leg is reconstructed (either because new planned data have been
+   * rolled out, or because a realtime update has modified a trip),
+   * it may not be possible to reconstruct the leg.
+   * In this case the method returns null.
+   */
   @Override
+  @Nullable
   public ScheduledTransitLeg getLeg(TransitService transitService) {
     Trip trip = transitService.getTripForId(tripId);
-
     if (trip == null) {
+      LOG.info("Invalid transit leg reference: trip {} not found", tripId);
       return null;
     }
 
     TripPattern tripPattern = transitService.getPatternForTrip(trip, serviceDate);
-
-    // no matching pattern found anywhere
     if (tripPattern == null) {
+      LOG.info(
+        "Invalid transit leg reference: trip pattern not found for trip {} and service date {} ",
+        tripId,
+        serviceDate
+      );
       return null;
     }
 
-    Timetable timetable = transitService.getTimetableForTripPattern(tripPattern, serviceDate);
+    int numStops = tripPattern.numberOfStops();
+    if (fromStopPositionInPattern >= numStops || toStopPositionInPattern >= numStops) {
+      LOG.info(
+        "Invalid transit leg reference: boarding stop {} or alighting stop {} is out of range" +
+        " in trip {} and service date {} ({} stops in trip pattern) ",
+        fromStopPositionInPattern,
+        toStopPositionInPattern,
+        tripId,
+        serviceDate,
+        numStops
+      );
+      return null;
+    }
 
+    Timetable timetable = transitService.getTimetableForTripPattern(tripPattern, serviceDate);
     TripTimes tripTimes = timetable.getTripTimes(trip);
 
+    if (tripTimes == null) {
+      LOG.info(
+        "Invalid transit leg reference: trip times not found for trip {} and service date {} ",
+        tripId,
+        serviceDate
+      );
+      return null;
+    }
+
     if (
       !transitService
         .getServiceCodesRunningForDate(serviceDate)
         .contains(tripTimes.getServiceCode())
     ) {
+      LOG.info(
+        "Invalid transit leg reference: the trip {} does not run on service date {} ",
+        tripId,
+        serviceDate
+      );
       return null;
     }
 

src/main/java/org/opentripplanner/transit/service/StopModel.java

@@ -248,6 +248,9 @@ private void reindex() {
   @Nullable
   @SafeVarargs
   private static <V> V getById(FeedScopedId id, Map<FeedScopedId, ? extends V>... maps) {
+    if (id == null) {
+      return null;
+    }
     for (Map<FeedScopedId, ? extends V> map : maps) {
       V v = map.get(id);
       if (v != null) {