Merge pull request #5408 from ibi-group/fix-septa-calculator

Fix fare calculation for combined interlined legs

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

@@ -13,6 +13,7 @@
 
 import java.util.List;
 import java.util.stream.Stream;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.opentripplanner.ext.fares.impl.CombinedInterlinedLegsFareService.CombinationMode;
@@ -54,7 +55,7 @@ class CombinedInterlinedLegsFareServiceTest implements PlanTestConstants {
     name = "Itinerary with {3} and combination mode {0} should lead to a fare of {2}"
   )
   @VariableSource("testCases")
-  void modeAlways(CombinationMode mode, Itinerary itinerary, Money expectedPrice, String hint) {
+  void modes(CombinationMode mode, Itinerary itinerary, Money totalPrice, String hint) {
     var service = new CombinedInterlinedLegsFareService(mode);
     service.addFareRules(
       FareType.regular,
@@ -65,7 +66,43 @@ void modeAlways(CombinationMode mode, Itinerary itinerary, Money expectedPrice,
     assertNotNull(fare);
 
     var price = fare.getFare(FareType.regular);
+    assertEquals(totalPrice, price);
 
-    assertEquals(expectedPrice, price);
+    var firstLeg = itinerary.getTransitLeg(0);
+    var uses = fare.legProductsFromComponents().get(firstLeg);
+    assertEquals(1, uses.size());
+
+    var secondLeg = itinerary.getTransitLeg(1);
+    uses = fare.legProductsFromComponents().get(secondLeg);
+    assertEquals(1, uses.size());
+  }
+
+  @Test
+  void legFares() {
+    var itinerary = interlinedWithSameRoute;
+    var service = new CombinedInterlinedLegsFareService(ALWAYS);
+    service.addFareRules(
+      FareType.regular,
+      List.of(AIRPORT_TO_CITY_CENTER_SET, INSIDE_CITY_CENTER_SET)
+    );
+
+    var fare = service.calculateFares(itinerary);
+
+    var firstLeg = itinerary.getTransitLeg(0);
+    var uses = List.copyOf(fare.legProductsFromComponents().get(firstLeg));
+    assertEquals(1, uses.size());
+
+    var firstLegUse = uses.get(0);
+    assertEquals(tenDollars, firstLegUse.product().price());
+
+    var secondLeg = itinerary.getTransitLeg(1);
+    uses = List.copyOf(fare.legProductsFromComponents().get(secondLeg));
+    assertEquals(1, uses.size());
+
+    var secondLegUse = uses.get(0);
+    assertEquals(tenDollars, secondLegUse.product().price());
+
+    // the same far product is used for both legs as you only need to buy one
+    assertEquals(secondLegUse, firstLegUse);
   }
 }

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

@@ -9,6 +9,7 @@
 import org.locationtech.jts.geom.LineString;
 import org.opentripplanner.framework.collection.ListUtils;
 import org.opentripplanner.model.fare.FareProductUse;
+import org.opentripplanner.model.plan.Leg;
 import org.opentripplanner.model.plan.Place;
 import org.opentripplanner.model.plan.StopArrival;
 import org.opentripplanner.model.plan.TransitLeg;
@@ -116,4 +117,11 @@ public void setFareProducts(List<FareProductUse> products) {}
   public List<FareProductUse> fareProducts() {
     return List.of();
   }
+
+  /**
+   * The two legs that this combined leg originally consisted of.
+   */
+  public List<Leg> originalLegs() {
+    return List.of(first, second);
+  }
 }

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

@@ -259,7 +259,17 @@ protected boolean populateFare(
 
       var componentLegs = new ArrayList<Leg>();
       for (int i = start; i <= via; ++i) {
-        componentLegs.add(legs.get(i));
+        final var leg = legs.get(i);
+        // if we have a leg that is combined for the purpose of fare calculation we need to
+        // retrieve the original legs so that the fare products are assigned back to the original
+        // legs that the combined one originally consisted of.
+        // (remember that the combined leg only exists during fare calculation and is thrown away
+        // afterwards to associating fare products with it will result in the API not showing any.)
+        if (leg instanceof CombinedInterlinedTransitLeg combinedLeg) {
+          componentLegs.addAll(combinedLeg.originalLegs());
+        } else {
+          componentLegs.add(leg);
+        }
       }
       components.add(
         new FareComponent(fareId, Money.ofFractionalAmount(currency, cost), componentLegs)
@@ -374,12 +384,13 @@ protected Money getFarePrice(FareAttribute fare, FareType type) {
 
   /**
    * Returns true if two interlined legs (those with a stay-seated transfer between them) should be
-   * treated as a single leg.
+   * treated as a single leg for the purposes of fare calculation.
    * <p>
    * By default it's disabled since this is unspecified in the GTFS fares spec.
    *
    * @see DefaultFareService#combineInterlinedLegs(List)
    * @see HighestFareInFreeTransferWindowFareService#shouldCombineInterlinedLegs(ScheduledTransitLeg, ScheduledTransitLeg)
+   * @see HSLFareService#shouldCombineInterlinedLegs(ScheduledTransitLeg, ScheduledTransitLeg)
    */
   protected boolean shouldCombineInterlinedLegs(
     ScheduledTransitLeg previousLeg,

src/main/java/org/opentripplanner/model/plan/Leg.java

@@ -19,7 +19,6 @@
 import org.opentripplanner.routing.alertpatch.TransitAlert;
 import org.opentripplanner.street.model.note.StreetNote;
 import org.opentripplanner.transit.model.basic.Accessibility;
-import org.opentripplanner.transit.model.framework.FeedScopedId;
 import org.opentripplanner.transit.model.network.Route;
 import org.opentripplanner.transit.model.organization.Agency;
 import org.opentripplanner.transit.model.organization.Operator;