Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fare calculation for combined interlined legs #5408

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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));
leonardehrenfried marked this conversation as resolved.
Show resolved Hide resolved
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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion src/main/java/org/opentripplanner/model/plan/Leg.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down