From 16b037ebcf3a8e1b13e4629ddcc44cee2a4cf425 Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Tue, 22 Jun 2021 09:48:27 -0400 Subject: [PATCH 01/17] refactor(PdxFareServiceImpl): WIP add special fares --- .../opentripplanner/routing/impl/PdxFareServiceImpl.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main/java/org/opentripplanner/routing/impl/PdxFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/PdxFareServiceImpl.java index 99984327372..a029b51a607 100644 --- a/src/main/java/org/opentripplanner/routing/impl/PdxFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/PdxFareServiceImpl.java @@ -10,6 +10,7 @@ import org.opentripplanner.routing.core.FareComponent; import org.opentripplanner.routing.core.FareRuleSet; import org.opentripplanner.routing.core.Fare.FareType; +import org.opentripplanner.routing.core.WrappedCurrency; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -17,6 +18,9 @@ public class PdxFareServiceImpl extends DefaultFareServiceImpl { public PdxFareServiceImpl(Collection regularFareRules) { addFareRules(FareType.regular, regularFareRules); + addFareRules(FareType.senior, regularFareRules); + addFareRules(FareType.special, regularFareRules); + addFareRules(FareType.youth, regularFareRules); } private static final long serialVersionUID = 20120229L; @@ -32,6 +36,10 @@ public PdxFareServiceImpl(Collection regularFareRules) { @Override protected boolean populateFare(Fare fare, Currency currency, FareType fareType, List rides, Collection fareRules) { + fare.addFare(FareType.regular, new WrappedCurrency(currency),0); + fare.addFare(FareType.senior, new WrappedCurrency(currency),0); + fare.addFare(FareType.special, new WrappedCurrency(currency),0); + fare.addFare(FareType.youth, new WrappedCurrency(currency),0); float lowestCost = getLowestCost(fareType, rides, fareRules); float cost = 0; for (Ride ride : rides) { From a63b60c46a914055f5519eebc75fd8c02256ef92 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 30 Jun 2021 14:20:36 +0100 Subject: [PATCH 02/17] refactor(Orca fare enhancement): Initital work on Orca fare discount. --- .../opentripplanner/routing/core/Fare.java | 2 +- .../impl/DefaultFareServiceFactory.java | 3 + .../routing/impl/OrcaFareServiceFactory.java | 35 ++++++++ .../routing/impl/OrcaFareServiceImpl.java | 80 +++++++++++++++++++ .../routing/fares/OrcaFareServiceTest.java | 51 ++++++++++++ 5 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceFactory.java create mode 100644 src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java create mode 100644 src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java diff --git a/src/main/java/org/opentripplanner/routing/core/Fare.java b/src/main/java/org/opentripplanner/routing/core/Fare.java index b36ca0ebcda..c43fb79dc7c 100644 --- a/src/main/java/org/opentripplanner/routing/core/Fare.java +++ b/src/main/java/org/opentripplanner/routing/core/Fare.java @@ -15,7 +15,7 @@ public class Fare { public static enum FareType implements Serializable { - regular, student, senior, tram, special, youth + regular, student, senior, tram, special, youth, orcaRegular, orcaYouth, orcaReduced } /** diff --git a/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceFactory.java b/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceFactory.java index 5ba20983c50..bf6963ef457 100644 --- a/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceFactory.java +++ b/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceFactory.java @@ -162,6 +162,9 @@ public static FareServiceFactory fromConfig(JsonNode config) { case "dutch": retval = new DutchFareServiceFactory(); break; + case "ocra": + retval = new OrcaFareServiceFactory(); + break; case "pdx": retval = new PdxFareServiceFactory(); break; diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceFactory.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceFactory.java new file mode 100644 index 00000000000..801f3112082 --- /dev/null +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceFactory.java @@ -0,0 +1,35 @@ +package org.opentripplanner.routing.impl; + +import com.fasterxml.jackson.databind.JsonNode; +import org.opentripplanner.model.FeedScopedId; +import org.opentripplanner.model.OtpTransitService; +import org.opentripplanner.routing.core.FareRuleSet; +import org.opentripplanner.routing.services.FareService; + +import java.util.HashMap; +import java.util.Map; + +public class OrcaFareServiceFactory extends DefaultFareServiceFactory { + protected Map regularFareRules = new HashMap<>(); + + @Override + public FareService makeFareService() { + return new OrcaFareServiceImpl(regularFareRules.values()); + } + + /** + * This step ensures that the fares in the source GTFS data are accounted for correctly. + */ + @Override + public void processGtfs(OtpTransitService transitService) { + fillFareRules(null, transitService.getAllFareAttributes(), transitService.getAllFareRules(), regularFareRules); + } + + /** + * There is no configuration code in DefaultFareServiceFactory. We override the super class's method just in case it changes. + */ + @Override + public void configure(JsonNode config) { + // No configuration at the moment. + } +} diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java new file mode 100644 index 00000000000..0ddd3f752df --- /dev/null +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -0,0 +1,80 @@ +package org.opentripplanner.routing.impl; + +import org.opentripplanner.routing.core.Fare; +import org.opentripplanner.routing.core.FareRuleSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Currency; +import java.util.List; + +public class OrcaFareServiceImpl extends DefaultFareServiceImpl { + private static final String WASHINGTON_STATE_FERRIES_AGENCY = "Washington State Ferries"; + +// TODO: Do we need to consider paper transfers? King County Metro and Kitsap Transit offer paper transfers that +// are good within each system. Community Transit, Everett Transit, Pierce Transit and Sound Transit do not accept paper transfers. + +// TODO: Do we need to consider expiration? Transfer value expires two hours after tapping the card. If the initial +// trip is less than the amount of fare required for a transfer, the difference must be paid with cash or E-purse. + + public OrcaFareServiceImpl(Collection regularFareRules) { + addFareRules(Fare.FareType.regular, regularFareRules); + addFareRules(Fare.FareType.youth, regularFareRules); + addFareRules(Fare.FareType.orcaReduced, regularFareRules); + addFareRules(Fare.FareType.orcaRegular, regularFareRules); + addFareRules(Fare.FareType.orcaYouth, regularFareRules); + } + + private static final long serialVersionUID = 20210625L; + @SuppressWarnings("unused") + private static final Logger LOG = LoggerFactory.getLogger(OrcaFareServiceImpl.class); + + @Override + protected boolean populateFare(Fare fare, + Currency currency, + Fare.FareType fareType, + List rides, + Collection fareRules + ) { + // Get full youth fare. + saveFare(calculateCost(Fare.FareType.youth, rides, fareRules), fare, Fare.FareType.youth, currency); + // Get full regular fare. + float regularCost = calculateCost(Fare.FareType.regular, rides, fareRules); + saveFare(regularCost, fare, Fare.FareType.regular, currency); + + if (regularCost > 0 && regularCost < Float.POSITIVE_INFINITY) { + float ocraRegularCost = getOrcaRegularFare(regularCost, fareType, rides, fareRules); + // Apply ORCA discounts to regular cost. + saveFare(ocraRegularCost, fare, Fare.FareType.orcaRegular, currency); + // TODO: Reduced and Youth. + } + return fare.fare.size() > 0; + } + + private void saveFare(float cost, Fare fare,Fare.FareType fareType, Currency currency) { + if (cost < Float.POSITIVE_INFINITY) { + fare.addFare(fareType, getMoney(currency, cost)); + } + } + + private float getOrcaRegularFare(float regularCost, + Fare.FareType fareType, + List rides, + Collection fareRules + ) { + if (rides.size() == 1 || + rides.size() == 2 && rides.get(1).agency.equalsIgnoreCase(WASHINGTON_STATE_FERRIES_AGENCY) + ) { + // An orca discount can not be applied to a single leg journey or Washington state ferry transfers. + return regularCost; + } + + // Orca regular cost is equal to the cash regular costs minus the cost of the first leg. + List leg = new ArrayList<>(); + Ride firstTransfer = rides.get(0); + leg.add(firstTransfer); + return regularCost - calculateCost(fareType, leg, fareRules); + } +} diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java new file mode 100644 index 00000000000..b8913483fb6 --- /dev/null +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -0,0 +1,51 @@ +package org.opentripplanner.routing.fares; + +import junit.framework.TestCase; +import org.opentripplanner.ConstantsForTests; +import org.opentripplanner.gtfs.GtfsContext; +import org.opentripplanner.gtfs.GtfsLibrary; +import org.opentripplanner.model.calendar.CalendarServiceData; +import org.opentripplanner.routing.algorithm.AStar; +import org.opentripplanner.routing.core.Fare; +import org.opentripplanner.routing.core.RoutingRequest; +import org.opentripplanner.routing.graph.Graph; +import org.opentripplanner.routing.impl.OrcaFareServiceFactory; +import org.opentripplanner.routing.services.FareService; +import org.opentripplanner.routing.spt.GraphPath; +import org.opentripplanner.routing.spt.ShortestPathTree; +import org.opentripplanner.util.TestUtils; + +import java.io.File; + +import static org.opentripplanner.calendar.impl.CalendarServiceDataFactoryImpl.createCalendarServiceData; + +public class OrcaFareServiceTest extends TestCase { + + private AStar aStar = new AStar(); + + public void test() throws Exception { + Graph gg = new Graph(); + GtfsContext context = GtfsLibrary.readGtfs(new File(ConstantsForTests.KCM_GTFS)); + + OrcaFareServiceFactory orcaFareServiceFactory = new OrcaFareServiceFactory(); + orcaFareServiceFactory.processGtfs(context.getOtpTransitService()); + + gg.putService( + CalendarServiceData.class, + createCalendarServiceData(context.getOtpTransitService()) + ); + + RoutingRequest options = new RoutingRequest(); + String vertex0 = context.getFeedId().getId() + ":2010"; + String vertex1 = context.getFeedId().getId() + ":2140"; + + options.dateTime = TestUtils.dateInSeconds("America/Los_Angeles", 2016, 5, 24, 5, 0, 0); + options.setRoutingContext(gg, vertex0, vertex1); + ShortestPathTree spt = aStar.getShortestPathTree(options); + GraphPath path = spt.getPath(gg.getVertex(vertex1), true); + + FareService f = orcaFareServiceFactory.makeFareService(); + Fare fare = f.getCost(path); + assertEquals(5, fare.fare.size()); + } +} From d7c2fd96926ac3bace483d953a526c73f70abd7f Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Mon, 5 Jul 2021 17:15:13 +0100 Subject: [PATCH 03/17] refactor(Added orca fare and ride type definitions): Defined orca fare and ride type along with unit --- pom.xml | 6 + .../opentripplanner/routing/core/Fare.java | 2 +- .../routing/impl/OrcaFareServiceImpl.java | 264 +++++++++++++++--- .../routing/fares/OrcaFareServiceTest.java | 123 +++++++- 4 files changed, 338 insertions(+), 57 deletions(-) diff --git a/pom.xml b/pom.xml index 6d064584143..d50466b344e 100644 --- a/pom.xml +++ b/pom.xml @@ -765,5 +765,11 @@ bsf 2.4.0 + + org.junit.jupiter + junit-jupiter-params + 5.5.2 + test + diff --git a/src/main/java/org/opentripplanner/routing/core/Fare.java b/src/main/java/org/opentripplanner/routing/core/Fare.java index c43fb79dc7c..82f35682b85 100644 --- a/src/main/java/org/opentripplanner/routing/core/Fare.java +++ b/src/main/java/org/opentripplanner/routing/core/Fare.java @@ -15,7 +15,7 @@ public class Fare { public static enum FareType implements Serializable { - regular, student, senior, tram, special, youth, orcaRegular, orcaYouth, orcaReduced + regular, student, senior, tram, special, youth, orcaRegular, orcaYouth, orcaLift, orcaReduced } /** diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index 0ddd3f752df..596c133ff5e 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -1,9 +1,8 @@ package org.opentripplanner.routing.impl; +import org.opentripplanner.model.Route; import org.opentripplanner.routing.core.Fare; import org.opentripplanner.routing.core.FareRuleSet; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.Collection; @@ -11,25 +10,203 @@ import java.util.List; public class OrcaFareServiceImpl extends DefaultFareServiceImpl { - private static final String WASHINGTON_STATE_FERRIES_AGENCY = "Washington State Ferries"; -// TODO: Do we need to consider paper transfers? King County Metro and Kitsap Transit offer paper transfers that -// are good within each system. Community Transit, Everett Transit, Pierce Transit and Sound Transit do not accept paper transfers. + enum RideType { + COMM_TRANS_LOCAL_SWIFT, + COMM_TRANS_COMMUTER_EXPRESS, + EVERETT_TRANSIT, + INTERCITY_TRANSIT, + KC_WATER_TAXI_VASHON_ISLAND, + KC_WATER_TAXI_WEST_SEATTLE, + KC_METRO, + KITSAP_TRANSIT, + PIERCE_COUNTY_TRANSIT, + SEATTLE_STREET_CAR, + SOUTH_TRANSIT, + WASHINGTON_STATE_FERRIES + } + + // Testing only. + public OrcaFareServiceImpl() {} -// TODO: Do we need to consider expiration? Transfer value expires two hours after tapping the card. If the initial -// trip is less than the amount of fare required for a transfer, the difference must be paid with cash or E-purse. public OrcaFareServiceImpl(Collection regularFareRules) { addFareRules(Fare.FareType.regular, regularFareRules); addFareRules(Fare.FareType.youth, regularFareRules); - addFareRules(Fare.FareType.orcaReduced, regularFareRules); addFareRules(Fare.FareType.orcaRegular, regularFareRules); addFareRules(Fare.FareType.orcaYouth, regularFareRules); + addFareRules(Fare.FareType.orcaLift, regularFareRules); + addFareRules(Fare.FareType.orcaReduced, regularFareRules); } private static final long serialVersionUID = 20210625L; - @SuppressWarnings("unused") - private static final Logger LOG = LoggerFactory.getLogger(OrcaFareServiceImpl.class); + + /** + * Classify the ride type based on the route information provided. In most cases the agency name is sufficient. Above + * this the route type and description is checked. + */ + private static RideType classify(Route route) { + if (route == null) { + return null; + } + String agency = route.getAgency().getName(); + if ("Community Transit".equals(agency)) { + if (route.getType() == 3) { + return RideType.COMM_TRANS_LOCAL_SWIFT; + } else { + // FIXME: The only route type used is 3 (Bus) so assuming for now that anything else is comm express. + return RideType.COMM_TRANS_COMMUTER_EXPRESS; + } + } else if ("Metro Transit".equals(agency)) { + if (route.getType() == 4 && route.getDesc().contains("Water Taxi: West Seattle")) { + return RideType.KC_WATER_TAXI_WEST_SEATTLE; + } else if (route.getType() == 4 && route.getDesc().contains("Water Taxi: Vashon Island")) { + return RideType.KC_WATER_TAXI_VASHON_ISLAND; + } + return RideType.KC_METRO; + } else if ("Sound Transit".equals(agency)) { + return RideType.SOUTH_TRANSIT; + } else if ("Everett Transit".equals(agency)) { + return RideType.EVERETT_TRANSIT; + } else if ("Pierce Transit".equals(agency)) { + return RideType.PIERCE_COUNTY_TRANSIT; + } else if ("City of Seattle".equals(agency)) { + return RideType.SEATTLE_STREET_CAR; + } else if ("Washington State Ferries".equals(agency)) { + return RideType.WASHINGTON_STATE_FERRIES; + } else if ("Kitsap Transit".equals(agency)) { + return RideType.KITSAP_TRANSIT; + } + return null; + } + + /** + * Return the Orca discount if applicable. In most cases the orca fare for regular and youth is the same as the + * cash fare with the following exceptions: + * 1) KC water taxi where the orca regular and youth fares are less. + * 2) WSF where Orca can not be used. + * 3) Pierce County where orca Lift can not be used. + * + * In cases where Orca is not applicable a null value is returned. No assumptions are therefore made regarding the + * correct cash value to be applied. + */ + private Float getOrcaFare(float regularCashFare, float youthCashFare, Fare.FareType fareType, RideType rideType) { + + if (rideType == null) { + return null; + } + + // Apart from KC water taxi and WSF, orca regular and orca youth match their cash equivalents. + switch (fareType) { + case orcaRegular: + if (rideType == RideType.KC_WATER_TAXI_VASHON_ISLAND) { + return 5.75f; + } else if (rideType == RideType.KC_WATER_TAXI_WEST_SEATTLE) { + return 5f; + } else if (rideType == RideType.WASHINGTON_STATE_FERRIES) { + return null; + } + return regularCashFare; + case orcaYouth: + if (rideType == RideType.KC_WATER_TAXI_VASHON_ISLAND) { + return 4.5f; + } else if (rideType == RideType.KC_WATER_TAXI_WEST_SEATTLE) { + return 3.75f; + } else if (rideType == RideType.WASHINGTON_STATE_FERRIES) { + return null; + } + return youthCashFare; + } + + switch (rideType) { + case COMM_TRANS_LOCAL_SWIFT: + case COMM_TRANS_COMMUTER_EXPRESS: + switch (fareType) { + case orcaLift: + case orcaReduced: + if (rideType == RideType.COMM_TRANS_LOCAL_SWIFT) { + return 1.25f; + } else { + return 2f; + } + } + break; + case EVERETT_TRANSIT: + switch (fareType) { + case orcaLift: + return youthCashFare; + case orcaReduced: + return 0.5f; + } + break; + case KC_METRO: + case SOUTH_TRANSIT: + switch (fareType) { + case orcaLift: + return youthCashFare; + case orcaReduced: + return 1f; + } + break; + case PIERCE_COUNTY_TRANSIT: + switch (fareType) { + case orcaLift: + return null; + case orcaReduced: + return 1f; + } + break; + case KC_WATER_TAXI_VASHON_ISLAND: + switch (fareType) { + case orcaLift: + return youthCashFare; + case orcaReduced: + return 3f; + } + break; + case KC_WATER_TAXI_WEST_SEATTLE: + switch (fareType) { + case orcaLift: + return youthCashFare; + case orcaReduced: + return 2.5f; + } + break; + case WASHINGTON_STATE_FERRIES: + switch (fareType) { + case orcaLift: + case orcaReduced: + return null; + } + break; + case SEATTLE_STREET_CAR: + switch (fareType) { + case orcaLift: + return youthCashFare; + case orcaReduced: + return 1f; + } + break; + case KITSAP_TRANSIT: + switch (fareType) { + case orcaLift: + case orcaReduced: + return 1f; + } + break; + } + return regularCashFare; + } + + //TODO: merge with populateFare once Route issue has been resolved. + public Float calcFare(Route route, + float regularCashFare, + float youthCashFare, + Fare.FareType fareType + ) { + RideType rideType = classify(route); + return getOrcaFare(regularCashFare, youthCashFare, fareType, rideType); + } @Override protected boolean populateFare(Fare fare, @@ -38,43 +215,42 @@ protected boolean populateFare(Fare fare, List rides, Collection fareRules ) { - // Get full youth fare. - saveFare(calculateCost(Fare.FareType.youth, rides, fareRules), fare, Fare.FareType.youth, currency); - // Get full regular fare. - float regularCost = calculateCost(Fare.FareType.regular, rides, fareRules); - saveFare(regularCost, fare, Fare.FareType.regular, currency); - - if (regularCost > 0 && regularCost < Float.POSITIVE_INFINITY) { - float ocraRegularCost = getOrcaRegularFare(regularCost, fareType, rides, fareRules); - // Apply ORCA discounts to regular cost. - saveFare(ocraRegularCost, fare, Fare.FareType.orcaRegular, currency); - // TODO: Reduced and Youth. + float cost = 0; + float WSFCost = 0; + if ( + fareType == Fare.FareType.orcaRegular || + fareType == Fare.FareType.orcaYouth || + fareType == Fare.FareType.orcaLift || + fareType == Fare.FareType.orcaReduced + ) { + for (Ride ride : rides) { + List leg = new ArrayList<>(); + leg.add(ride); + //TODO: get Route obj from Ride obj, DB call? + Route route = null; + float regularCashFare = calculateCost(Fare.FareType.regular, leg, fareRules); + float youthCashFare = calculateCost(Fare.FareType.youth, leg, fareRules); + RideType rideType = classify(route); + Float legCost = calcFare(null, regularCashFare, youthCashFare, fareType); + if (legCost == null) { + // FIXME: Orca not valid for this ride type. Apply regular cash value. + legCost = regularCashFare; + } + if (rideType == RideType.WASHINGTON_STATE_FERRIES) { + //WSF do not except transfers. + WSFCost += legCost; + } else { + cost = Float.max(cost, legCost); + } + } + cost += WSFCost; + } else { + // Get cash fare for complete journey. + cost = calculateCost(fareType, rides, fareRules); } - return fare.fare.size() > 0; - } - - private void saveFare(float cost, Fare fare,Fare.FareType fareType, Currency currency) { if (cost < Float.POSITIVE_INFINITY) { fare.addFare(fareType, getMoney(currency, cost)); } - } - - private float getOrcaRegularFare(float regularCost, - Fare.FareType fareType, - List rides, - Collection fareRules - ) { - if (rides.size() == 1 || - rides.size() == 2 && rides.get(1).agency.equalsIgnoreCase(WASHINGTON_STATE_FERRIES_AGENCY) - ) { - // An orca discount can not be applied to a single leg journey or Washington state ferry transfers. - return regularCost; - } - - // Orca regular cost is equal to the cash regular costs minus the cost of the first leg. - List leg = new ArrayList<>(); - Ride firstTransfer = rides.get(0); - leg.add(firstTransfer); - return regularCost - calculateCost(fareType, leg, fareRules); + return cost > 0 && cost < Float.POSITIVE_INFINITY; } } diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index b8913483fb6..e9cda5d4a9f 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -1,51 +1,150 @@ package org.opentripplanner.routing.fares; -import junit.framework.TestCase; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.opentripplanner.ConstantsForTests; import org.opentripplanner.gtfs.GtfsContext; import org.opentripplanner.gtfs.GtfsLibrary; +import org.opentripplanner.model.Agency; +import org.opentripplanner.model.Route; import org.opentripplanner.model.calendar.CalendarServiceData; import org.opentripplanner.routing.algorithm.AStar; import org.opentripplanner.routing.core.Fare; import org.opentripplanner.routing.core.RoutingRequest; +import org.opentripplanner.routing.edgetype.factory.PatternHopFactory; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.impl.OrcaFareServiceFactory; +import org.opentripplanner.routing.impl.OrcaFareServiceImpl; import org.opentripplanner.routing.services.FareService; import org.opentripplanner.routing.spt.GraphPath; import org.opentripplanner.routing.spt.ShortestPathTree; import org.opentripplanner.util.TestUtils; import java.io.File; +import java.util.stream.Stream; +import static org.junit.Assert.assertEquals; import static org.opentripplanner.calendar.impl.CalendarServiceDataFactoryImpl.createCalendarServiceData; -public class OrcaFareServiceTest extends TestCase { +public class OrcaFareServiceTest { - private AStar aStar = new AStar(); + private static final float regularCashFare = 3.50f; + private static final float youthCashFare = 2.50f; + private static OrcaFareServiceImpl orcaFareService; + private static Agency agency; + private static Route route; + @BeforeAll + public static void setUpClass() { + orcaFareService = new OrcaFareServiceImpl(); + agency = new Agency(); + route = new Route(); + } + + @ParameterizedTest + @MethodSource("createRoutes") + public void correctOrcaLegFareIsAppliedTests(String agencyName, + int routeType, + String routeDesc, + Fare.FareType fareType, + Float expectedFare + ) { + agency.setName(agencyName); + route.setAgency(agency); + route.setType(routeType); + route.setDesc(routeDesc); + Float orcaFare = orcaFareService.calcFare(route, regularCashFare, youthCashFare, fareType); + Assertions.assertEquals(orcaFare, expectedFare); + } + + private static Stream createRoutes() { + return Stream.of( + Arguments.of("Community Transit", 3, "", Fare.FareType.orcaLift, 1.25f), + Arguments.of("Community Transit", 1, "", Fare.FareType.orcaLift, 2f), + Arguments.of("Community Transit", 3, "", Fare.FareType.orcaReduced, 1.25f), + Arguments.of("Community Transit", 1, "", Fare.FareType.orcaReduced, 2f), + Arguments.of("Community Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), + Arguments.of("Community Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), + Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaLift, youthCashFare), + Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaReduced, 2.5f), + Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaRegular, 5f), + Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaYouth, 3.75f), + Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaLift, youthCashFare), + Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaReduced, 3f), + Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaRegular, 5.75f), + Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaYouth, 4.5f), + Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaLift, youthCashFare), + Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaReduced, 1f), + Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), + Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), + Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaLift, youthCashFare), + Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaReduced, 0.5f), + Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), + Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), + Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaLift, null), + Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaReduced, 1f), + Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), + Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), + Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaLift, youthCashFare), + Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaReduced, 1f), + Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaRegular, regularCashFare), + Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaYouth, youthCashFare), + Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaLift, null), + Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaReduced, null), + Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaRegular, null), + Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaYouth, null), + Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaLift, 1f), + Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaReduced, 1f), + Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), + Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare) + ); + } + + // TODO: Define test routes and read in required GTFS feeds. + @Test public void test() throws Exception { + AStar aStar = new AStar(); Graph gg = new Graph(); - GtfsContext context = GtfsLibrary.readGtfs(new File(ConstantsForTests.KCM_GTFS)); + GtfsContext KCcontext = GtfsLibrary.readGtfs(new File(ConstantsForTests.KCM_GTFS)); + GtfsContext CalContext = GtfsLibrary.readGtfs(new File(ConstantsForTests.CALTRAIN_GTFS)); + // Add - wash state and commtrans GTFS feeds. - OrcaFareServiceFactory orcaFareServiceFactory = new OrcaFareServiceFactory(); - orcaFareServiceFactory.processGtfs(context.getOtpTransitService()); + // create one of these for each GTFS feed. + PatternHopFactory KCfactory = new PatternHopFactory(KCcontext); + KCfactory.setFareServiceFactory(new OrcaFareServiceFactory()); + PatternHopFactory Calfactory = new PatternHopFactory(KCcontext); + Calfactory.setFareServiceFactory(new OrcaFareServiceFactory()); + KCfactory.run(gg); + Calfactory.run(gg); gg.putService( CalendarServiceData.class, - createCalendarServiceData(context.getOtpTransitService()) + createCalendarServiceData(KCcontext.getOtpTransitService()) ); +// gg.putService( +// CalendarServiceData.class, +// createCalendarServiceData(CalContext.getOtpTransitService()) +// ); + FareService fareService = gg.getService(FareService.class); RoutingRequest options = new RoutingRequest(); - String vertex0 = context.getFeedId().getId() + ":2010"; - String vertex1 = context.getFeedId().getId() + ":2140"; + String feedId = gg.getFeedIds().iterator().next(); + + String vertex0 = feedId + ":2010"; + String vertex1 = feedId + ":2140"; options.dateTime = TestUtils.dateInSeconds("America/Los_Angeles", 2016, 5, 24, 5, 0, 0); options.setRoutingContext(gg, vertex0, vertex1); ShortestPathTree spt = aStar.getShortestPathTree(options); GraphPath path = spt.getPath(gg.getVertex(vertex1), true); - FareService f = orcaFareServiceFactory.makeFareService(); - Fare fare = f.getCost(path); - assertEquals(5, fare.fare.size()); + Fare fare = fareService.getCost(path); + Assertions.assertEquals(6, fare.fare.size()); } + + } From 6d295c0620e2f6cacddc7d382fdd19c2a9b52581 Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Tue, 6 Jul 2021 17:30:30 -0400 Subject: [PATCH 04/17] refactor(OrcaFareServiceImpl): reorganize orca fare service --- .../opentripplanner/routing/core/Fare.java | 2 +- .../routing/impl/OrcaFareServiceImpl.java | 244 +++++++----------- .../routing/fares/OrcaFareServiceTest.java | 20 +- 3 files changed, 106 insertions(+), 160 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/core/Fare.java b/src/main/java/org/opentripplanner/routing/core/Fare.java index 82f35682b85..5cfa69cc4b5 100644 --- a/src/main/java/org/opentripplanner/routing/core/Fare.java +++ b/src/main/java/org/opentripplanner/routing/core/Fare.java @@ -15,7 +15,7 @@ public class Fare { public static enum FareType implements Serializable { - regular, student, senior, tram, special, youth, orcaRegular, orcaYouth, orcaLift, orcaReduced + regular, student, senior, tram, special, youth, orcaRegular, orcaYouth, orcaLift, orcaSenior } /** diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index 596c133ff5e..7dd45733678 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -1,6 +1,5 @@ package org.opentripplanner.routing.impl; -import org.opentripplanner.model.Route; import org.opentripplanner.routing.core.Fare; import org.opentripplanner.routing.core.FareRuleSet; @@ -22,7 +21,7 @@ enum RideType { KITSAP_TRANSIT, PIERCE_COUNTY_TRANSIT, SEATTLE_STREET_CAR, - SOUTH_TRANSIT, + SOUND_TRANSIT, WASHINGTON_STATE_FERRIES } @@ -32,11 +31,12 @@ public OrcaFareServiceImpl() {} public OrcaFareServiceImpl(Collection regularFareRules) { addFareRules(Fare.FareType.regular, regularFareRules); + addFareRules(Fare.FareType.senior, regularFareRules); addFareRules(Fare.FareType.youth, regularFareRules); addFareRules(Fare.FareType.orcaRegular, regularFareRules); addFareRules(Fare.FareType.orcaYouth, regularFareRules); addFareRules(Fare.FareType.orcaLift, regularFareRules); - addFareRules(Fare.FareType.orcaReduced, regularFareRules); + addFareRules(Fare.FareType.orcaSenior, regularFareRules); } private static final long serialVersionUID = 20210625L; @@ -45,36 +45,32 @@ public OrcaFareServiceImpl(Collection regularFareRules) { * Classify the ride type based on the route information provided. In most cases the agency name is sufficient. Above * this the route type and description is checked. */ - private static RideType classify(Route route) { - if (route == null) { - return null; - } - String agency = route.getAgency().getName(); - if ("Community Transit".equals(agency)) { + private static RideType classify(String agencyId, String routeId) { + if ("Community Transit".equals(agencyId)) { if (route.getType() == 3) { return RideType.COMM_TRANS_LOCAL_SWIFT; } else { // FIXME: The only route type used is 3 (Bus) so assuming for now that anything else is comm express. return RideType.COMM_TRANS_COMMUTER_EXPRESS; } - } else if ("Metro Transit".equals(agency)) { + } else if ("Metro Transit".equals(agencyId)) { if (route.getType() == 4 && route.getDesc().contains("Water Taxi: West Seattle")) { return RideType.KC_WATER_TAXI_WEST_SEATTLE; } else if (route.getType() == 4 && route.getDesc().contains("Water Taxi: Vashon Island")) { return RideType.KC_WATER_TAXI_VASHON_ISLAND; } return RideType.KC_METRO; - } else if ("Sound Transit".equals(agency)) { - return RideType.SOUTH_TRANSIT; - } else if ("Everett Transit".equals(agency)) { + } else if ("Sound Transit".equals(agencyId)) { + return RideType.SOUND_TRANSIT; + } else if ("Everett Transit".equals(agencyId)) { return RideType.EVERETT_TRANSIT; - } else if ("Pierce Transit".equals(agency)) { + } else if ("Pierce Transit".equals(agencyId)) { return RideType.PIERCE_COUNTY_TRANSIT; - } else if ("City of Seattle".equals(agency)) { + } else if ("City of Seattle".equals(agencyId)) { return RideType.SEATTLE_STREET_CAR; - } else if ("Washington State Ferries".equals(agency)) { + } else if ("Washington State Ferries".equals(agencyId)) { return RideType.WASHINGTON_STATE_FERRIES; - } else if ("Kitsap Transit".equals(agency)) { + } else if ("Kitsap Transit".equals(agencyId)) { return RideType.KITSAP_TRANSIT; } return null; @@ -90,122 +86,77 @@ private static RideType classify(Route route) { * In cases where Orca is not applicable a null value is returned. No assumptions are therefore made regarding the * correct cash value to be applied. */ - private Float getOrcaFare(float regularCashFare, float youthCashFare, Fare.FareType fareType, RideType rideType) { - + private float getDiscountedFare(Fare.FareType fareType, RideType rideType, float defaultFare) { if (rideType == null) { - return null; + return defaultFare; } - - // Apart from KC water taxi and WSF, orca regular and orca youth match their cash equivalents. switch (fareType) { - case orcaRegular: - if (rideType == RideType.KC_WATER_TAXI_VASHON_ISLAND) { - return 5.75f; - } else if (rideType == RideType.KC_WATER_TAXI_WEST_SEATTLE) { - return 5f; - } else if (rideType == RideType.WASHINGTON_STATE_FERRIES) { - return null; - } - return regularCashFare; + case youth: case orcaYouth: - if (rideType == RideType.KC_WATER_TAXI_VASHON_ISLAND) { - return 4.5f; - } else if (rideType == RideType.KC_WATER_TAXI_WEST_SEATTLE) { - return 3.75f; - } else if (rideType == RideType.WASHINGTON_STATE_FERRIES) { - return null; - } - return youthCashFare; + return getYouthFare(rideType); + case orcaLift: + return getLiftFare(rideType, defaultFare); + case orcaSenior: + case senior: + return getSeniorFare(fareType, rideType, defaultFare); + case orcaRegular: + case regular: + default: + return defaultFare; } + } + + private float getLiftFare(RideType rideType, float defaultFare) { + switch (rideType) { + case COMM_TRANS_LOCAL_SWIFT: return 1.25f; + case COMM_TRANS_COMMUTER_EXPRESS: return 2.00f; + case PIERCE_COUNTY_TRANSIT: return defaultFare; + case KC_WATER_TAXI_VASHON_ISLAND: return 4.50f; + case KC_WATER_TAXI_WEST_SEATTLE: return 3.75f; + case INTERCITY_TRANSIT: return 0.00f; + case KITSAP_TRANSIT: return 1.00f; + // KCM, Sound Transit, Everett, Seattle Streetcar. + default: return 1.50f; + } + } + private float getSeniorFare(Fare.FareType fareType, RideType rideType, float defaultFare) { switch (rideType) { - case COMM_TRANS_LOCAL_SWIFT: - case COMM_TRANS_COMMUTER_EXPRESS: - switch (fareType) { - case orcaLift: - case orcaReduced: - if (rideType == RideType.COMM_TRANS_LOCAL_SWIFT) { - return 1.25f; - } else { - return 2f; - } - } - break; + case COMM_TRANS_LOCAL_SWIFT: return 1.25f; + case COMM_TRANS_COMMUTER_EXPRESS: return 2.00f; + case INTERCITY_TRANSIT: return 0.00f; case EVERETT_TRANSIT: - switch (fareType) { - case orcaLift: - return youthCashFare; - case orcaReduced: - return 0.5f; - } - break; - case KC_METRO: - case SOUTH_TRANSIT: - switch (fareType) { - case orcaLift: - return youthCashFare; - case orcaReduced: - return 1f; - } - break; + return fareType.equals(Fare.FareType.orcaSenior) ? 0.50f : defaultFare; case PIERCE_COUNTY_TRANSIT: - switch (fareType) { - case orcaLift: - return null; - case orcaReduced: - return 1f; - } - break; - case KC_WATER_TAXI_VASHON_ISLAND: - switch (fareType) { - case orcaLift: - return youthCashFare; - case orcaReduced: - return 3f; - } - break; - case KC_WATER_TAXI_WEST_SEATTLE: - switch (fareType) { - case orcaLift: - return youthCashFare; - case orcaReduced: - return 2.5f; - } - break; - case WASHINGTON_STATE_FERRIES: - switch (fareType) { - case orcaLift: - case orcaReduced: - return null; - } - break; case SEATTLE_STREET_CAR: - switch (fareType) { - case orcaLift: - return youthCashFare; - case orcaReduced: - return 1f; - } - break; case KITSAP_TRANSIT: - switch (fareType) { - case orcaLift: - case orcaReduced: - return 1f; - } - break; + // Pierce, Seattle Streetcar, and Kitsap only provide discounted senior fare for orca. + return fareType.equals(Fare.FareType.orcaSenior) ? 1.00f : defaultFare; + case KC_WATER_TAXI_VASHON_ISLAND: return 3.00f; + case KC_WATER_TAXI_WEST_SEATTLE: return 2.50f; + // KC Metro, Sound Transit + default: return 1.00f; } - return regularCashFare; } - //TODO: merge with populateFare once Route issue has been resolved. - public Float calcFare(Route route, - float regularCashFare, - float youthCashFare, - Fare.FareType fareType - ) { - RideType rideType = classify(route); - return getOrcaFare(regularCashFare, youthCashFare, fareType, rideType); + private float getYouthFare(RideType rideType) { + switch (rideType) { + case COMM_TRANS_LOCAL_SWIFT: return 1.75f; + case COMM_TRANS_COMMUTER_EXPRESS: return 3.00f; + case PIERCE_COUNTY_TRANSIT: return 1.00f; + case KC_WATER_TAXI_VASHON_ISLAND: return 4.50f; + case KC_WATER_TAXI_WEST_SEATTLE: return 3.75f; + case KITSAP_TRANSIT: return 2.00f; + case INTERCITY_TRANSIT: return 0.00f; + // Default case accounts for KC_METRO, Sound Transit, Everett, and Seattle Streetcar. + default: return 1.50f; + } + } + + private float getRidePrice(Ride ride, Fare.FareType fareType, Collection fareRules) { + List ridesSingleton = new ArrayList<>(); + ridesSingleton.add(ride); + return calculateCost(fareType, ridesSingleton, fareRules); } @Override @@ -216,41 +167,36 @@ protected boolean populateFare(Fare fare, Collection fareRules ) { float cost = 0; - float WSFCost = 0; - if ( - fareType == Fare.FareType.orcaRegular || - fareType == Fare.FareType.orcaYouth || - fareType == Fare.FareType.orcaLift || - fareType == Fare.FareType.orcaReduced - ) { - for (Ride ride : rides) { - List leg = new ArrayList<>(); - leg.add(ride); - //TODO: get Route obj from Ride obj, DB call? - Route route = null; - float regularCashFare = calculateCost(Fare.FareType.regular, leg, fareRules); - float youthCashFare = calculateCost(Fare.FareType.youth, leg, fareRules); - RideType rideType = classify(route); - Float legCost = calcFare(null, regularCashFare, youthCashFare, fareType); - if (legCost == null) { - // FIXME: Orca not valid for this ride type. Apply regular cash value. - legCost = regularCashFare; - } - if (rideType == RideType.WASHINGTON_STATE_FERRIES) { - //WSF do not except transfers. - WSFCost += legCost; - } else { - cost = Float.max(cost, legCost); - } + for (Ride ride : rides) { + String routeId = ride.route.getId(); + String agencyId = ride.agency; + RideType rideType = classify(agencyId, routeId); + float singleLegPrice = getRidePrice(ride, Fare.FareType.regular, fareRules); + float discountedFare = getDiscountedFare(fareType, rideType, singleLegPrice); + if (hasFreeTransfers(fareType, rideType)) { + // If using Orca (free transfers), the total fare should be equivalent to the + // most expensive leg of the journey. + cost = Float.max(cost, discountedFare); + } else { + // If free transfers not permitted (i.e., paying with cash), accumulate each + // leg as we go. + cost += discountedFare; } - cost += WSFCost; - } else { - // Get cash fare for complete journey. - cost = calculateCost(fareType, rides, fareRules); } if (cost < Float.POSITIVE_INFINITY) { fare.addFare(fareType, getMoney(currency, cost)); } return cost > 0 && cost < Float.POSITIVE_INFINITY; } + + private boolean hasFreeTransfers(Fare.FareType fareType, RideType rideType) { + return rideType != RideType.WASHINGTON_STATE_FERRIES && usesOrca(fareType); + } + + private boolean usesOrca(Fare.FareType fareType) { + return fareType.equals(Fare.FareType.orcaLift) || + fareType.equals(Fare.FareType.orcaSenior) || + fareType.equals(Fare.FareType.orcaRegular) || + fareType.equals(Fare.FareType.orcaYouth); + } } diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index e9cda5d4a9f..a8e9cfd861b 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -65,40 +65,40 @@ private static Stream createRoutes() { return Stream.of( Arguments.of("Community Transit", 3, "", Fare.FareType.orcaLift, 1.25f), Arguments.of("Community Transit", 1, "", Fare.FareType.orcaLift, 2f), - Arguments.of("Community Transit", 3, "", Fare.FareType.orcaReduced, 1.25f), - Arguments.of("Community Transit", 1, "", Fare.FareType.orcaReduced, 2f), + Arguments.of("Community Transit", 3, "", Fare.FareType.orcaSenior, 1.25f), + Arguments.of("Community Transit", 1, "", Fare.FareType.orcaSenior, 2f), Arguments.of("Community Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), Arguments.of("Community Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaLift, youthCashFare), - Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaReduced, 2.5f), + Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaSenior, 2.5f), Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaRegular, 5f), Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaYouth, 3.75f), Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaLift, youthCashFare), - Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaReduced, 3f), + Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaSenior, 3f), Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaRegular, 5.75f), Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaYouth, 4.5f), Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaLift, youthCashFare), - Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaReduced, 1f), + Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaSenior, 1f), Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaLift, youthCashFare), - Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaReduced, 0.5f), + Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaSenior, 0.5f), Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaLift, null), - Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaReduced, 1f), + Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaSenior, 1f), Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaLift, youthCashFare), - Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaReduced, 1f), + Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaSenior, 1f), Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaRegular, regularCashFare), Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaYouth, youthCashFare), Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaLift, null), - Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaReduced, null), + Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaSenior, null), Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaRegular, null), Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaYouth, null), Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaLift, 1f), - Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaReduced, 1f), + Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaSenior, 1f), Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare) ); From 771ef597ab290ce50724b7861094de13b091e6ed Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Mon, 12 Jul 2021 17:19:06 +0100 Subject: [PATCH 05/17] refactor(Bug fixes and unit tests): Bug fixes and unit tests --- .../impl/DefaultFareServiceFactory.java | 2 +- .../routing/impl/DefaultFareServiceImpl.java | 71 +---- .../routing/impl/OrcaFareServiceImpl.java | 101 ++++--- .../opentripplanner/routing/impl/Ride.java | 79 ++++++ .../opentripplanner/ConstantsForTests.java | 2 +- .../routing/fares/OrcaFareServiceTest.java | 263 ++++++++++-------- 6 files changed, 292 insertions(+), 226 deletions(-) create mode 100644 src/main/java/org/opentripplanner/routing/impl/Ride.java diff --git a/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceFactory.java b/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceFactory.java index bf6963ef457..82f5f4ce38f 100644 --- a/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceFactory.java +++ b/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceFactory.java @@ -162,7 +162,7 @@ public static FareServiceFactory fromConfig(JsonNode config) { case "dutch": retval = new DutchFareServiceFactory(); break; - case "ocra": + case "orca": retval = new OrcaFareServiceFactory(); break; case "pdx": diff --git a/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceImpl.java index 6005b51c085..e2a9d594cbf 100644 --- a/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/DefaultFareServiceImpl.java @@ -14,6 +14,7 @@ import org.opentripplanner.model.FeedScopedId; import org.opentripplanner.model.FareAttribute; +import org.opentripplanner.model.Route; import org.opentripplanner.model.Stop; import org.opentripplanner.routing.core.Fare; import org.opentripplanner.routing.core.Fare.FareType; @@ -29,75 +30,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** A set of edges on a single route, with associated information for calculating fares */ -class Ride { - - String feedId; - - String agency; // route agency - - FeedScopedId route; - - FeedScopedId trip; - - Set zones; - - String startZone; - - String endZone; - - long startTime; - - long endTime; - - // in DefaultFareServiceImpl classifier is just the TraverseMode - // it can be used differently in custom fare services - public Object classifier; - - public Stop firstStop; - - public Stop lastStop; - - public Ride() { - zones = new HashSet(); - } - - public String toString() { - StringBuilder builder = new StringBuilder(); - builder.append("Ride"); - if (startZone != null) { - builder.append("(from zone "); - builder.append(startZone); - } - if (endZone != null) { - builder.append(" to zone "); - builder.append(endZone); - } - builder.append(" on route "); - builder.append(route); - if (zones.size() > 0) { - builder.append(" through zones "); - boolean first = true; - for (String zone : zones) { - if (first) { - first = false; - } else { - builder.append(","); - } - builder.append(zone); - } - } - builder.append(" at "); - builder.append(startTime); - if (classifier != null) { - builder.append(", classified by "); - builder.append(classifier.toString()); - } - builder.append(")"); - return builder.toString(); - } -} - /** Holds information for doing the graph search on fares */ class FareSearch { // Cell [i,j] holds the best (lowest) cost for a trip from rides[i] to rides[j] @@ -175,6 +107,7 @@ protected List createRides(GraphPath path) { ride.startZone = hEdge.getBeginStop().getZoneId(); ride.zones.add(ride.startZone); ride.agency = state.getBackTrip().getRoute().getAgency().getId(); + ride.routeData = state.getBackTrip().getRoute(); ride.route = state.getRoute(); ride.startTime = state.getBackState().getTimeSeconds(); ride.firstStop = hEdge.getBeginStop(); diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index 7dd45733678..e8b6ac4990a 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -1,7 +1,10 @@ package org.opentripplanner.routing.impl; +import org.opentripplanner.model.Route; import org.opentripplanner.routing.core.Fare; import org.opentripplanner.routing.core.FareRuleSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.Collection; @@ -10,11 +13,12 @@ public class OrcaFareServiceImpl extends DefaultFareServiceImpl { - enum RideType { + private static final Logger LOG = LoggerFactory.getLogger(OrcaFareServiceImpl.class); + + public enum RideType { COMM_TRANS_LOCAL_SWIFT, COMM_TRANS_COMMUTER_EXPRESS, EVERETT_TRANSIT, - INTERCITY_TRANSIT, KC_WATER_TAXI_VASHON_ISLAND, KC_WATER_TAXI_WEST_SEATTLE, KC_METRO, @@ -25,9 +29,8 @@ enum RideType { WASHINGTON_STATE_FERRIES } - // Testing only. - public OrcaFareServiceImpl() {} - + public boolean IS_TEST; + public static final float DEFAULT_TEST_RIDE_PRICE = 3.49f; public OrcaFareServiceImpl(Collection regularFareRules) { addFareRules(Fare.FareType.regular, regularFareRules); @@ -42,50 +45,44 @@ public OrcaFareServiceImpl(Collection regularFareRules) { private static final long serialVersionUID = 20210625L; /** - * Classify the ride type based on the route information provided. In most cases the agency name is sufficient. Above - * this the route type and description is checked. + * Classify the ride type based on the route information provided. In most cases the agency name is sufficient. In + * some cases the route description and short name are needed to define inner agency ride types. */ - private static RideType classify(String agencyId, String routeId) { - if ("Community Transit".equals(agencyId)) { - if (route.getType() == 3) { + private static RideType classify(Route routeData) { + if ("29".equals(routeData.getAgency().getId())) { + try { + int routeId = Integer.parseInt(routeData.getShortName()); + if (routeId >= 400 && routeId <= 899) { + return RideType.COMM_TRANS_COMMUTER_EXPRESS; + } + return RideType.COMM_TRANS_LOCAL_SWIFT; + } catch (NumberFormatException e) { + LOG.warn("Unable to determine comm trans route id from {}.", routeData.getShortName(), e); return RideType.COMM_TRANS_LOCAL_SWIFT; - } else { - // FIXME: The only route type used is 3 (Bus) so assuming for now that anything else is comm express. - return RideType.COMM_TRANS_COMMUTER_EXPRESS; } - } else if ("Metro Transit".equals(agencyId)) { - if (route.getType() == 4 && route.getDesc().contains("Water Taxi: West Seattle")) { + } else if ("1".equals(routeData.getAgency().getId())) { + if (routeData.getType() == 4 && routeData.getDesc().contains("Water Taxi: West Seattle")) { return RideType.KC_WATER_TAXI_WEST_SEATTLE; - } else if (route.getType() == 4 && route.getDesc().contains("Water Taxi: Vashon Island")) { + } else if (routeData.getType() == 4 && routeData.getDesc().contains("Water Taxi: Vashon Island")) { return RideType.KC_WATER_TAXI_VASHON_ISLAND; } return RideType.KC_METRO; - } else if ("Sound Transit".equals(agencyId)) { + } else if ("40".equals(routeData.getAgency().getId())) { return RideType.SOUND_TRANSIT; - } else if ("Everett Transit".equals(agencyId)) { + } else if ("97".equals(routeData.getAgency().getId())) { return RideType.EVERETT_TRANSIT; - } else if ("Pierce Transit".equals(agencyId)) { + } else if ("3".equals(routeData.getAgency().getId())) { return RideType.PIERCE_COUNTY_TRANSIT; - } else if ("City of Seattle".equals(agencyId)) { + } else if ("23".equals(routeData.getAgency().getId())) { return RideType.SEATTLE_STREET_CAR; - } else if ("Washington State Ferries".equals(agencyId)) { + } else if ("wsf".equalsIgnoreCase(routeData.getAgency().getId())) { return RideType.WASHINGTON_STATE_FERRIES; - } else if ("Kitsap Transit".equals(agencyId)) { + } else if ("kt".equalsIgnoreCase(routeData.getAgency().getId())) { return RideType.KITSAP_TRANSIT; } return null; } - /** - * Return the Orca discount if applicable. In most cases the orca fare for regular and youth is the same as the - * cash fare with the following exceptions: - * 1) KC water taxi where the orca regular and youth fares are less. - * 2) WSF where Orca can not be used. - * 3) Pierce County where orca Lift can not be used. - * - * In cases where Orca is not applicable a null value is returned. No assumptions are therefore made regarding the - * correct cash value to be applied. - */ private float getDiscountedFare(Fare.FareType fareType, RideType rideType, float defaultFare) { if (rideType == null) { return defaultFare; @@ -100,12 +97,21 @@ private float getDiscountedFare(Fare.FareType fareType, RideType rideType, float case senior: return getSeniorFare(fareType, rideType, defaultFare); case orcaRegular: + return getOrcaRegularFare(rideType, defaultFare); case regular: default: return defaultFare; } } + private float getOrcaRegularFare(RideType rideType, float defaultFare) { + switch (rideType) { + case KC_WATER_TAXI_VASHON_ISLAND: return 5.75f; + case KC_WATER_TAXI_WEST_SEATTLE: return 5.00f; + default: return defaultFare; + } + } + private float getLiftFare(RideType rideType, float defaultFare) { switch (rideType) { case COMM_TRANS_LOCAL_SWIFT: return 1.25f; @@ -113,7 +119,6 @@ private float getLiftFare(RideType rideType, float defaultFare) { case PIERCE_COUNTY_TRANSIT: return defaultFare; case KC_WATER_TAXI_VASHON_ISLAND: return 4.50f; case KC_WATER_TAXI_WEST_SEATTLE: return 3.75f; - case INTERCITY_TRANSIT: return 0.00f; case KITSAP_TRANSIT: return 1.00f; // KCM, Sound Transit, Everett, Seattle Streetcar. default: return 1.50f; @@ -124,7 +129,6 @@ private float getSeniorFare(Fare.FareType fareType, RideType rideType, float def switch (rideType) { case COMM_TRANS_LOCAL_SWIFT: return 1.25f; case COMM_TRANS_COMMUTER_EXPRESS: return 2.00f; - case INTERCITY_TRANSIT: return 0.00f; case EVERETT_TRANSIT: return fareType.equals(Fare.FareType.orcaSenior) ? 0.50f : defaultFare; case PIERCE_COUNTY_TRANSIT: @@ -147,13 +151,16 @@ private float getYouthFare(RideType rideType) { case KC_WATER_TAXI_VASHON_ISLAND: return 4.50f; case KC_WATER_TAXI_WEST_SEATTLE: return 3.75f; case KITSAP_TRANSIT: return 2.00f; - case INTERCITY_TRANSIT: return 0.00f; // Default case accounts for KC_METRO, Sound Transit, Everett, and Seattle Streetcar. default: return 1.50f; } } private float getRidePrice(Ride ride, Fare.FareType fareType, Collection fareRules) { + if (IS_TEST) { + // Testing, return default test ride price. + return DEFAULT_TEST_RIDE_PRICE; + } List ridesSingleton = new ArrayList<>(); ridesSingleton.add(ride); return calculateCost(fareType, ridesSingleton, fareRules); @@ -165,24 +172,36 @@ protected boolean populateFare(Fare fare, Fare.FareType fareType, List rides, Collection fareRules + ) { + return calculateFare(fare, currency, fareType, rides, fareRules); + } + + /** + * Public access to allow unit testing. + */ + public boolean calculateFare(Fare fare, + Currency currency, + Fare.FareType fareType, + List rides, + Collection fareRules ) { float cost = 0; + float mostExpensiveDiscountedFare = 0; for (Ride ride : rides) { - String routeId = ride.route.getId(); - String agencyId = ride.agency; - RideType rideType = classify(agencyId, routeId); - float singleLegPrice = getRidePrice(ride, Fare.FareType.regular, fareRules); + RideType rideType = classify(ride.routeData); + float singleLegPrice = getRidePrice(ride, fareType, fareRules); float discountedFare = getDiscountedFare(fareType, rideType, singleLegPrice); if (hasFreeTransfers(fareType, rideType)) { // If using Orca (free transfers), the total fare should be equivalent to the // most expensive leg of the journey. - cost = Float.max(cost, discountedFare); + mostExpensiveDiscountedFare = Float.max(mostExpensiveDiscountedFare, discountedFare); } else { // If free transfers not permitted (i.e., paying with cash), accumulate each // leg as we go. - cost += discountedFare; + cost += singleLegPrice; } } + cost += mostExpensiveDiscountedFare; if (cost < Float.POSITIVE_INFINITY) { fare.addFare(fareType, getMoney(currency, cost)); } diff --git a/src/main/java/org/opentripplanner/routing/impl/Ride.java b/src/main/java/org/opentripplanner/routing/impl/Ride.java new file mode 100644 index 00000000000..e8b713333fa --- /dev/null +++ b/src/main/java/org/opentripplanner/routing/impl/Ride.java @@ -0,0 +1,79 @@ +package org.opentripplanner.routing.impl; + +import org.opentripplanner.model.FeedScopedId; +import org.opentripplanner.model.Route; +import org.opentripplanner.model.Stop; + +import java.util.HashSet; +import java.util.Set; + +/** A set of edges on a single route, with associated information for calculating fares */ +public class Ride { + + String feedId; + + public String agency; // route agency + + FeedScopedId route; + + public Route routeData; + + FeedScopedId trip; + + Set zones; + + String startZone; + + String endZone; + + long startTime; + + long endTime; + + // in DefaultFareServiceImpl classifier is just the TraverseMode + // it can be used differently in custom fare services + public Object classifier; + + public Stop firstStop; + + public Stop lastStop; + + public Ride() { + zones = new HashSet(); + } + + public String toString() { + StringBuilder builder = new StringBuilder(); + builder.append("Ride"); + if (startZone != null) { + builder.append("(from zone "); + builder.append(startZone); + } + if (endZone != null) { + builder.append(" to zone "); + builder.append(endZone); + } + builder.append(" on route "); + builder.append(route); + if (zones.size() > 0) { + builder.append(" through zones "); + boolean first = true; + for (String zone : zones) { + if (first) { + first = false; + } else { + builder.append(","); + } + builder.append(zone); + } + } + builder.append(" at "); + builder.append(startTime); + if (classifier != null) { + builder.append(", classified by "); + builder.append(classifier.toString()); + } + builder.append(")"); + return builder.toString(); + } +} diff --git a/src/test/java/org/opentripplanner/ConstantsForTests.java b/src/test/java/org/opentripplanner/ConstantsForTests.java index 56ba9f38e4d..6d6b2690eca 100644 --- a/src/test/java/org/opentripplanner/ConstantsForTests.java +++ b/src/test/java/org/opentripplanner/ConstantsForTests.java @@ -27,7 +27,7 @@ public class ConstantsForTests { public static final String PORTLAND_GTFS = "src/test/resources/google_transit.zip"; public static final String KCM_GTFS = "src/test/resources/kcm_gtfs.zip"; - + public static final String FAKE_GTFS = "src/test/resources/testagency.zip"; public static final String FARE_COMPONENT_GTFS = "src/test/resources/farecomponent_gtfs.zip"; diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index a8e9cfd861b..0a904338d15 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -2,149 +2,184 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import org.opentripplanner.ConstantsForTests; -import org.opentripplanner.gtfs.GtfsContext; -import org.opentripplanner.gtfs.GtfsLibrary; import org.opentripplanner.model.Agency; +import org.opentripplanner.model.FeedScopedId; import org.opentripplanner.model.Route; -import org.opentripplanner.model.calendar.CalendarServiceData; -import org.opentripplanner.routing.algorithm.AStar; import org.opentripplanner.routing.core.Fare; -import org.opentripplanner.routing.core.RoutingRequest; -import org.opentripplanner.routing.edgetype.factory.PatternHopFactory; -import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.OrcaFareServiceFactory; +import org.opentripplanner.routing.core.FareRuleSet; import org.opentripplanner.routing.impl.OrcaFareServiceImpl; -import org.opentripplanner.routing.services.FareService; -import org.opentripplanner.routing.spt.GraphPath; -import org.opentripplanner.routing.spt.ShortestPathTree; -import org.opentripplanner.util.TestUtils; +import org.opentripplanner.routing.impl.Ride; -import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.stream.Stream; -import static org.junit.Assert.assertEquals; -import static org.opentripplanner.calendar.impl.CalendarServiceDataFactoryImpl.createCalendarServiceData; - public class OrcaFareServiceTest { - private static final float regularCashFare = 3.50f; - private static final float youthCashFare = 2.50f; private static OrcaFareServiceImpl orcaFareService; - private static Agency agency; - private static Route route; + private static float DEFAULT_RIDE_PRICE_IN_CENTS; @BeforeAll public static void setUpClass() { - orcaFareService = new OrcaFareServiceImpl(); - agency = new Agency(); - route = new Route(); + Map regularFareRules = new HashMap<>(); + orcaFareService = new OrcaFareServiceImpl(regularFareRules.values()); + orcaFareService.IS_TEST = true; + DEFAULT_RIDE_PRICE_IN_CENTS = OrcaFareServiceImpl.DEFAULT_TEST_RIDE_PRICE * 100; } + /** + * These test are design to specifically validate Orca fares. Since these fares are hard-coded, it is acceptable to + * make direct calls to the Orca fare service with predefined routes. Where the default fare is applied a test + * substitute {@link OrcaFareServiceImpl#DEFAULT_TEST_RIDE_PRICE} is used. This will be the same for all cash fare types. + */ @ParameterizedTest - @MethodSource("createRoutes") - public void correctOrcaLegFareIsAppliedTests(String agencyName, - int routeType, - String routeDesc, - Fare.FareType fareType, - Float expectedFare + @MethodSource("trips") + public void calculateFareTest(List rides, + Fare.FareType fareType, + float expectedFareInCents ) { - agency.setName(agencyName); - route.setAgency(agency); - route.setType(routeType); - route.setDesc(routeDesc); - Float orcaFare = orcaFareService.calcFare(route, regularCashFare, youthCashFare, fareType); - Assertions.assertEquals(orcaFare, expectedFare); + Fare fare = new Fare(); + orcaFareService.calculateFare(fare, null, fareType, rides, null); + Assertions.assertEquals(expectedFareInCents, fare.getFare(fareType).getCents()); } - private static Stream createRoutes() { + private static Stream trips() { + List trip1 = getTrip( + OrcaFareServiceImpl.RideType.COMM_TRANS_COMMUTER_EXPRESS + ); + List trip2 = getTrip( + OrcaFareServiceImpl.RideType.KITSAP_TRANSIT, + OrcaFareServiceImpl.RideType.WASHINGTON_STATE_FERRIES, + OrcaFareServiceImpl.RideType.COMM_TRANS_LOCAL_SWIFT + ); + List trip3 = getTrip( + OrcaFareServiceImpl.RideType.PIERCE_COUNTY_TRANSIT, + OrcaFareServiceImpl.RideType.SOUND_TRANSIT, + OrcaFareServiceImpl.RideType.COMM_TRANS_COMMUTER_EXPRESS + ); + List trip4 = getTrip( + OrcaFareServiceImpl.RideType.EVERETT_TRANSIT, + OrcaFareServiceImpl.RideType.COMM_TRANS_COMMUTER_EXPRESS, + OrcaFareServiceImpl.RideType.SOUND_TRANSIT + ); + List trip5 = getTrip( + OrcaFareServiceImpl.RideType.KC_WATER_TAXI_VASHON_ISLAND, + OrcaFareServiceImpl.RideType.KC_METRO + ); + List trip6 = getTrip( + OrcaFareServiceImpl.RideType.SEATTLE_STREET_CAR, + OrcaFareServiceImpl.RideType.KC_METRO, + OrcaFareServiceImpl.RideType.COMM_TRANS_COMMUTER_EXPRESS + ); return Stream.of( - Arguments.of("Community Transit", 3, "", Fare.FareType.orcaLift, 1.25f), - Arguments.of("Community Transit", 1, "", Fare.FareType.orcaLift, 2f), - Arguments.of("Community Transit", 3, "", Fare.FareType.orcaSenior, 1.25f), - Arguments.of("Community Transit", 1, "", Fare.FareType.orcaSenior, 2f), - Arguments.of("Community Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), - Arguments.of("Community Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), - Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaLift, youthCashFare), - Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaSenior, 2.5f), - Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaRegular, 5f), - Arguments.of("Metro Transit", 4, "Water Taxi: West Seattle", Fare.FareType.orcaYouth, 3.75f), - Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaLift, youthCashFare), - Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaSenior, 3f), - Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaRegular, 5.75f), - Arguments.of("Metro Transit", 4, "Water Taxi: Vashon Island", Fare.FareType.orcaYouth, 4.5f), - Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaLift, youthCashFare), - Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaSenior, 1f), - Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), - Arguments.of("Sound Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), - Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaLift, youthCashFare), - Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaSenior, 0.5f), - Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), - Arguments.of("Everett Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), - Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaLift, null), - Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaSenior, 1f), - Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), - Arguments.of("Pierce Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare), - Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaLift, youthCashFare), - Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaSenior, 1f), - Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaRegular, regularCashFare), - Arguments.of("City of Seattle", -1, "", Fare.FareType.orcaYouth, youthCashFare), - Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaLift, null), - Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaSenior, null), - Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaRegular, null), - Arguments.of("Washington State Ferries", -1, "", Fare.FareType.orcaYouth, null), - Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaLift, 1f), - Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaSenior, 1f), - Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaRegular, regularCashFare), - Arguments.of("Kitsap Transit", -1, "", Fare.FareType.orcaYouth, youthCashFare) + Arguments.of(trip1, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS), + Arguments.of(trip1, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS), + Arguments.of(trip1, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS), + Arguments.of(trip1, Fare.FareType.orcaLift, 200f), + Arguments.of(trip1, Fare.FareType.orcaRegular, DEFAULT_RIDE_PRICE_IN_CENTS), + Arguments.of(trip1, Fare.FareType.orcaSenior, 200f), + Arguments.of(trip1, Fare.FareType.orcaYouth, 300f), + Arguments.of(trip2, Fare.FareType.orcaLift, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f), + Arguments.of(trip2, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), + Arguments.of(trip2, Fare.FareType.orcaSenior, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f), + Arguments.of(trip2, Fare.FareType.orcaYouth, 200f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), + Arguments.of(trip3, Fare.FareType.orcaLift, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f), + Arguments.of(trip3, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), + Arguments.of(trip3, Fare.FareType.orcaSenior, 0f + 0f + 200f), + Arguments.of(trip3, Fare.FareType.orcaYouth, 0f + 0f + 300f), + Arguments.of(trip4, Fare.FareType.orcaLift, 0f + 200f + 0f), + Arguments.of(trip4, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), + Arguments.of(trip4, Fare.FareType.orcaSenior, 0f + 0f + 200f), + Arguments.of(trip4, Fare.FareType.orcaYouth, 0f + 0f + 300f), + Arguments.of(trip5, Fare.FareType.orcaLift, 450f + 0f), + Arguments.of(trip5, Fare.FareType.orcaRegular, 575f + 0f), + Arguments.of(trip5, Fare.FareType.orcaSenior, 300f + 0f), + Arguments.of(trip5, Fare.FareType.orcaYouth, 450f + 0f), + Arguments.of(trip6, Fare.FareType.orcaLift, 0f + 0f + 200f), + Arguments.of(trip6, Fare.FareType.orcaRegular, 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS), + Arguments.of(trip6, Fare.FareType.orcaSenior, 0f + 0f + 200f), + Arguments.of(trip6, Fare.FareType.orcaYouth, 0f + 0f + 300f) ); } - // TODO: Define test routes and read in required GTFS feeds. - @Test - public void test() throws Exception { - AStar aStar = new AStar(); - Graph gg = new Graph(); - GtfsContext KCcontext = GtfsLibrary.readGtfs(new File(ConstantsForTests.KCM_GTFS)); - GtfsContext CalContext = GtfsLibrary.readGtfs(new File(ConstantsForTests.CALTRAIN_GTFS)); - // Add - wash state and commtrans GTFS feeds. - - // create one of these for each GTFS feed. - PatternHopFactory KCfactory = new PatternHopFactory(KCcontext); - KCfactory.setFareServiceFactory(new OrcaFareServiceFactory()); - PatternHopFactory Calfactory = new PatternHopFactory(KCcontext); - Calfactory.setFareServiceFactory(new OrcaFareServiceFactory()); - - KCfactory.run(gg); - Calfactory.run(gg); - gg.putService( - CalendarServiceData.class, - createCalendarServiceData(KCcontext.getOtpTransitService()) - ); -// gg.putService( -// CalendarServiceData.class, -// createCalendarServiceData(CalContext.getOtpTransitService()) -// ); - FareService fareService = gg.getService(FareService.class); - - RoutingRequest options = new RoutingRequest(); - String feedId = gg.getFeedIds().iterator().next(); - - String vertex0 = feedId + ":2010"; - String vertex1 = feedId + ":2140"; + /** + * Build a list of {@link Ride)s from the ride types provided. The values used here to produce a {@link Ride} match + * the values used in {@link OrcaFareServiceImpl} to determine the correct ride type. + */ + private static List getTrip(OrcaFareServiceImpl.RideType... rideTypes) { + List rides = new ArrayList<>(); + for (OrcaFareServiceImpl.RideType rideType : rideTypes) { + switch (rideType) { + case COMM_TRANS_LOCAL_SWIFT: + rides.add(getRide("29")); + break; + case COMM_TRANS_COMMUTER_EXPRESS: + rides.add(getRide("29", "400")); + break; + case EVERETT_TRANSIT: + rides.add(getRide("97")); + break; + case PIERCE_COUNTY_TRANSIT: + rides.add(getRide("3")); + break; + case SEATTLE_STREET_CAR: + rides.add(getRide("23")); + break; + case KITSAP_TRANSIT: + rides.add(getRide("kt")); + break; + case KC_WATER_TAXI_VASHON_ISLAND: + rides.add(getRide("1", 4, "Water Taxi: Vashon Island")); + break; + case KC_WATER_TAXI_WEST_SEATTLE: + rides.add(getRide("1", 4, "Water Taxi: West Seattle")); + break; + case KC_METRO: + rides.add(getRide("1")); + break; + case SOUND_TRANSIT: + rides.add(getRide("40")); + break; + case WASHINGTON_STATE_FERRIES: + rides.add(getRide("wsf")); + break; + } + + } + return rides; + } - options.dateTime = TestUtils.dateInSeconds("America/Los_Angeles", 2016, 5, 24, 5, 0, 0); - options.setRoutingContext(gg, vertex0, vertex1); - ShortestPathTree spt = aStar.getShortestPathTree(options); - GraphPath path = spt.getPath(gg.getVertex(vertex1), true); + private static Ride getRide(String agencyId) { + return getRide(agencyId, "-1", -1, null); + } - Fare fare = fareService.getCost(path); - Assertions.assertEquals(6, fare.fare.size()); + private static Ride getRide(String agencyId, int rideType, String desc) { + return getRide(agencyId, "-1", rideType, desc); } + private static Ride getRide(String agencyId, String shortName) { + return getRide(agencyId, shortName, -1, null); + } + /** + * Create a {@link Ride} containing route data that will be used by {@link OrcaFareServiceImpl} to determine the + * correct ride type. + */ + private static Ride getRide(String agencyId, String shortName, int rideType, String desc) { + Ride ride = new Ride(); + Agency agency = new Agency(); + agency.setId(agencyId); + Route route = new Route(); + route.setAgency(agency); + route.setShortName(shortName); + route.setType(rideType); + route.setDesc(desc); + ride.routeData = route; + return ride; + } } From e8681e9c8eee3ea3b3c500c0a15a5431c81b02c6 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Mon, 12 Jul 2021 18:20:24 +0100 Subject: [PATCH 06/17] refactor(Reverted change related to most expensive discount fare.): Free transfer check is now back --- .../opentripplanner/routing/impl/OrcaFareServiceImpl.java | 4 +--- .../opentripplanner/routing/fares/OrcaFareServiceTest.java | 5 +++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index e8b6ac4990a..207ee6e87d0 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -186,7 +186,6 @@ public boolean calculateFare(Fare fare, Collection fareRules ) { float cost = 0; - float mostExpensiveDiscountedFare = 0; for (Ride ride : rides) { RideType rideType = classify(ride.routeData); float singleLegPrice = getRidePrice(ride, fareType, fareRules); @@ -194,14 +193,13 @@ public boolean calculateFare(Fare fare, if (hasFreeTransfers(fareType, rideType)) { // If using Orca (free transfers), the total fare should be equivalent to the // most expensive leg of the journey. - mostExpensiveDiscountedFare = Float.max(mostExpensiveDiscountedFare, discountedFare); + cost = Float.max(cost, discountedFare); } else { // If free transfers not permitted (i.e., paying with cash), accumulate each // leg as we go. cost += singleLegPrice; } } - cost += mostExpensiveDiscountedFare; if (cost < Float.POSITIVE_INFINITY) { fare.addFare(fareType, getMoney(currency, cost)); } diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index 0a904338d15..5248697f899 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -84,9 +84,10 @@ private static Stream trips() { Arguments.of(trip1, Fare.FareType.orcaRegular, DEFAULT_RIDE_PRICE_IN_CENTS), Arguments.of(trip1, Fare.FareType.orcaSenior, 200f), Arguments.of(trip1, Fare.FareType.orcaYouth, 300f), - Arguments.of(trip2, Fare.FareType.orcaLift, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f), + // The costs are made up of the expected cost to be applied for each agency. + Arguments.of(trip2, Fare.FareType.orcaLift, 100f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), Arguments.of(trip2, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), - Arguments.of(trip2, Fare.FareType.orcaSenior, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f), + Arguments.of(trip2, Fare.FareType.orcaSenior, 100f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), Arguments.of(trip2, Fare.FareType.orcaYouth, 200f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), Arguments.of(trip3, Fare.FareType.orcaLift, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f), Arguments.of(trip3, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), From d8fb6145f6150d9bb2a6c6f87a28f6a96b1429d6 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 13 Jul 2021 09:22:36 +0100 Subject: [PATCH 07/17] refactor(Added method comments and static imports): Added method comments and updated static imports --- .../routing/impl/OrcaFareServiceImpl.java | 40 ++++++++++++++-- .../routing/fares/OrcaFareServiceTest.java | 47 ++++++++++++------- 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index 207ee6e87d0..556c52929f5 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -11,6 +11,9 @@ import java.util.Currency; import java.util.List; +/** + * Calculate Orca discount fares based on the fare type and the agencies traversed within a trip. + */ public class OrcaFareServiceImpl extends DefaultFareServiceImpl { private static final Logger LOG = LoggerFactory.getLogger(OrcaFareServiceImpl.class); @@ -29,6 +32,7 @@ public enum RideType { WASHINGTON_STATE_FERRIES } + // If set to true, the test ride price is used instead of the actual agency cash fare. public boolean IS_TEST; public static final float DEFAULT_TEST_RIDE_PRICE = 3.49f; @@ -42,8 +46,6 @@ public OrcaFareServiceImpl(Collection regularFareRules) { addFareRules(Fare.FareType.orcaSenior, regularFareRules); } - private static final long serialVersionUID = 20210625L; - /** * Classify the ride type based on the route information provided. In most cases the agency name is sufficient. In * some cases the route description and short name are needed to define inner agency ride types. @@ -83,6 +85,10 @@ private static RideType classify(Route routeData) { return null; } + /** + * Define which discount fare should be applied based on the fare type. If the ride type is unknown the discount + * fare can not be applied, use the default fare. + */ private float getDiscountedFare(Fare.FareType fareType, RideType rideType, float defaultFare) { if (rideType == null) { return defaultFare; @@ -97,14 +103,17 @@ private float getDiscountedFare(Fare.FareType fareType, RideType rideType, float case senior: return getSeniorFare(fareType, rideType, defaultFare); case orcaRegular: - return getOrcaRegularFare(rideType, defaultFare); + return getRegularFare(rideType, defaultFare); case regular: default: return defaultFare; } } - private float getOrcaRegularFare(RideType rideType, float defaultFare) { + /** + * Apply Orca regular discount fares. If the ride type can not be matched the default fare is used. + */ + private float getRegularFare(RideType rideType, float defaultFare) { switch (rideType) { case KC_WATER_TAXI_VASHON_ISLAND: return 5.75f; case KC_WATER_TAXI_WEST_SEATTLE: return 5.00f; @@ -112,6 +121,9 @@ private float getOrcaRegularFare(RideType rideType, float defaultFare) { } } + /** + * Apply Orca lift discount fares based on the ride type. + */ private float getLiftFare(RideType rideType, float defaultFare) { switch (rideType) { case COMM_TRANS_LOCAL_SWIFT: return 1.25f; @@ -125,6 +137,9 @@ private float getLiftFare(RideType rideType, float defaultFare) { } } + /** + * Apply Orca senior discount fares based on the fare and ride types. + */ private float getSeniorFare(Fare.FareType fareType, RideType rideType, float defaultFare) { switch (rideType) { case COMM_TRANS_LOCAL_SWIFT: return 1.25f; @@ -143,6 +158,9 @@ private float getSeniorFare(Fare.FareType fareType, RideType rideType, float def } } + /** + * Apply Orca youth discount fares based on the ride type. + */ private float getYouthFare(RideType rideType) { switch (rideType) { case COMM_TRANS_LOCAL_SWIFT: return 1.75f; @@ -156,6 +174,10 @@ private float getYouthFare(RideType rideType) { } } + /** + * Get the ride price for a single leg. If testing, this class is being called directly so the required agency cash + * values are not available therefore the default test price is used instead. + */ private float getRidePrice(Ride ride, Fare.FareType fareType, Collection fareRules) { if (IS_TEST) { // Testing, return default test ride price. @@ -177,7 +199,8 @@ protected boolean populateFare(Fare fare, } /** - * Public access to allow unit testing. + * Accumulate the cost of a trip by working through each leg and applying the appropriate cost based on either the + * cash price or discount price. This method has public access to allow for unit testing. */ public boolean calculateFare(Fare fare, Currency currency, @@ -206,10 +229,17 @@ public boolean calculateFare(Fare fare, return cost > 0 && cost < Float.POSITIVE_INFINITY; } + /** + * If using Orca and the ride type is not Washington state ferries (they do no allow free transfers) a free transfer + * can be applied. + */ private boolean hasFreeTransfers(Fare.FareType fareType, RideType rideType) { return rideType != RideType.WASHINGTON_STATE_FERRIES && usesOrca(fareType); } + /** + * Define Orca fare types. + */ private boolean usesOrca(Fare.FareType fareType) { return fareType.equals(Fare.FareType.orcaLift) || fareType.equals(Fare.FareType.orcaSenior) || diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index 5248697f899..ffd0c76177f 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -19,6 +19,17 @@ import java.util.Map; import java.util.stream.Stream; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.COMM_TRANS_COMMUTER_EXPRESS; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.COMM_TRANS_LOCAL_SWIFT; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.EVERETT_TRANSIT; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.KC_METRO; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.KC_WATER_TAXI_VASHON_ISLAND; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.KITSAP_TRANSIT; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.PIERCE_COUNTY_TRANSIT; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.SEATTLE_STREET_CAR; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.SOUND_TRANSIT; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.WASHINGTON_STATE_FERRIES; + public class OrcaFareServiceTest { private static OrcaFareServiceImpl orcaFareService; @@ -35,7 +46,8 @@ public static void setUpClass() { /** * These test are design to specifically validate Orca fares. Since these fares are hard-coded, it is acceptable to * make direct calls to the Orca fare service with predefined routes. Where the default fare is applied a test - * substitute {@link OrcaFareServiceImpl#DEFAULT_TEST_RIDE_PRICE} is used. This will be the same for all cash fare types. + * substitute {@link OrcaFareServiceImpl#DEFAULT_TEST_RIDE_PRICE} is used. This will be the same for all cash fare + * types. */ @ParameterizedTest @MethodSource("trips") @@ -50,31 +62,31 @@ public void calculateFareTest(List rides, private static Stream trips() { List trip1 = getTrip( - OrcaFareServiceImpl.RideType.COMM_TRANS_COMMUTER_EXPRESS + COMM_TRANS_COMMUTER_EXPRESS ); List trip2 = getTrip( - OrcaFareServiceImpl.RideType.KITSAP_TRANSIT, - OrcaFareServiceImpl.RideType.WASHINGTON_STATE_FERRIES, - OrcaFareServiceImpl.RideType.COMM_TRANS_LOCAL_SWIFT + KITSAP_TRANSIT, + WASHINGTON_STATE_FERRIES, + COMM_TRANS_LOCAL_SWIFT ); List trip3 = getTrip( - OrcaFareServiceImpl.RideType.PIERCE_COUNTY_TRANSIT, - OrcaFareServiceImpl.RideType.SOUND_TRANSIT, - OrcaFareServiceImpl.RideType.COMM_TRANS_COMMUTER_EXPRESS + PIERCE_COUNTY_TRANSIT, + SOUND_TRANSIT, + COMM_TRANS_COMMUTER_EXPRESS ); List trip4 = getTrip( - OrcaFareServiceImpl.RideType.EVERETT_TRANSIT, - OrcaFareServiceImpl.RideType.COMM_TRANS_COMMUTER_EXPRESS, - OrcaFareServiceImpl.RideType.SOUND_TRANSIT + EVERETT_TRANSIT, + COMM_TRANS_COMMUTER_EXPRESS, + SOUND_TRANSIT ); List trip5 = getTrip( - OrcaFareServiceImpl.RideType.KC_WATER_TAXI_VASHON_ISLAND, - OrcaFareServiceImpl.RideType.KC_METRO + KC_WATER_TAXI_VASHON_ISLAND, + KC_METRO ); List trip6 = getTrip( - OrcaFareServiceImpl.RideType.SEATTLE_STREET_CAR, - OrcaFareServiceImpl.RideType.KC_METRO, - OrcaFareServiceImpl.RideType.COMM_TRANS_COMMUTER_EXPRESS + SEATTLE_STREET_CAR, + KC_METRO, + COMM_TRANS_COMMUTER_EXPRESS ); return Stream.of( Arguments.of(trip1, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS), @@ -84,7 +96,8 @@ private static Stream trips() { Arguments.of(trip1, Fare.FareType.orcaRegular, DEFAULT_RIDE_PRICE_IN_CENTS), Arguments.of(trip1, Fare.FareType.orcaSenior, 200f), Arguments.of(trip1, Fare.FareType.orcaYouth, 300f), - // The costs are made up of the expected cost to be applied for each agency. + // The following costs are made up of the expected cost to be applied for each agency. This is usually the + // most expensive leg of the journey. Arguments.of(trip2, Fare.FareType.orcaLift, 100f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), Arguments.of(trip2, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), Arguments.of(trip2, Fare.FareType.orcaSenior, 100f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), From c9b9297adb8e01b943d24284befffd686c1893f3 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 13 Jul 2021 09:42:54 +0100 Subject: [PATCH 08/17] refactor(PdxFareServiceImpl.java): Restored to previous set-up. A separate Orca fare class was creat --- .../routing/impl/PdxFareServiceImpl.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/impl/PdxFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/PdxFareServiceImpl.java index a029b51a607..e4eac712107 100644 --- a/src/main/java/org/opentripplanner/routing/impl/PdxFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/PdxFareServiceImpl.java @@ -5,12 +5,9 @@ import java.util.Currency; import java.util.List; -import com.google.common.collect.Iterables; import org.opentripplanner.routing.core.Fare; -import org.opentripplanner.routing.core.FareComponent; import org.opentripplanner.routing.core.FareRuleSet; import org.opentripplanner.routing.core.Fare.FareType; -import org.opentripplanner.routing.core.WrappedCurrency; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -18,9 +15,6 @@ public class PdxFareServiceImpl extends DefaultFareServiceImpl { public PdxFareServiceImpl(Collection regularFareRules) { addFareRules(FareType.regular, regularFareRules); - addFareRules(FareType.senior, regularFareRules); - addFareRules(FareType.special, regularFareRules); - addFareRules(FareType.youth, regularFareRules); } private static final long serialVersionUID = 20120229L; @@ -36,10 +30,6 @@ public PdxFareServiceImpl(Collection regularFareRules) { @Override protected boolean populateFare(Fare fare, Currency currency, FareType fareType, List rides, Collection fareRules) { - fare.addFare(FareType.regular, new WrappedCurrency(currency),0); - fare.addFare(FareType.senior, new WrappedCurrency(currency),0); - fare.addFare(FareType.special, new WrappedCurrency(currency),0); - fare.addFare(FareType.youth, new WrappedCurrency(currency),0); float lowestCost = getLowestCost(fareType, rides, fareRules); float cost = 0; for (Ride ride : rides) { From 231d7e8405b3e4774313e27f9916fce0ca09698f Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 13 Jul 2021 14:49:59 +0100 Subject: [PATCH 09/17] refactor(Bug fix): Updated Orca fare service to only use the most expensive discount fare when calcu --- .../opentripplanner/routing/impl/OrcaFareServiceImpl.java | 8 +++++--- .../routing/fares/OrcaFareServiceTest.java | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index 556c52929f5..633959b1441 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -199,8 +199,8 @@ protected boolean populateFare(Fare fare, } /** - * Accumulate the cost of a trip by working through each leg and applying the appropriate cost based on either the - * cash price or discount price. This method has public access to allow for unit testing. + * Calculate the cost of a journey. Where free transfers are not permitted the cash price is use. If free transfers + * are applicable, the most expensive discount fare across all legs is added to the final cumulative cash price. */ public boolean calculateFare(Fare fare, Currency currency, @@ -209,6 +209,7 @@ public boolean calculateFare(Fare fare, Collection fareRules ) { float cost = 0; + float orcaFareDiscount = 0; for (Ride ride : rides) { RideType rideType = classify(ride.routeData); float singleLegPrice = getRidePrice(ride, fareType, fareRules); @@ -216,13 +217,14 @@ public boolean calculateFare(Fare fare, if (hasFreeTransfers(fareType, rideType)) { // If using Orca (free transfers), the total fare should be equivalent to the // most expensive leg of the journey. - cost = Float.max(cost, discountedFare); + orcaFareDiscount = Float.max(orcaFareDiscount, discountedFare); } else { // If free transfers not permitted (i.e., paying with cash), accumulate each // leg as we go. cost += singleLegPrice; } } + cost += orcaFareDiscount; if (cost < Float.POSITIVE_INFINITY) { fare.addFare(fareType, getMoney(currency, cost)); } diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index ffd0c76177f..24cca9bed34 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -98,9 +98,9 @@ private static Stream trips() { Arguments.of(trip1, Fare.FareType.orcaYouth, 300f), // The following costs are made up of the expected cost to be applied for each agency. This is usually the // most expensive leg of the journey. - Arguments.of(trip2, Fare.FareType.orcaLift, 100f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), + Arguments.of(trip2, Fare.FareType.orcaLift, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f), Arguments.of(trip2, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), - Arguments.of(trip2, Fare.FareType.orcaSenior, 100f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), + Arguments.of(trip2, Fare.FareType.orcaSenior, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f), Arguments.of(trip2, Fare.FareType.orcaYouth, 200f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), Arguments.of(trip3, Fare.FareType.orcaLift, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f), Arguments.of(trip3, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), From 4cdb24e5cae50714d7af78fad09b952efdd143b5 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 14 Jul 2021 15:43:14 +0100 Subject: [PATCH 10/17] refactor(Addressed PR feedback): Addressed PR feedback --- .../routing/impl/OrcaFareServiceImpl.java | 132 ++++++++++-------- .../routing/fares/OrcaFareServiceTest.java | 88 ++++++------ 2 files changed, 126 insertions(+), 94 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index 633959b1441..a64f59ad319 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -9,7 +9,10 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Currency; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.function.Function; /** * Calculate Orca discount fares based on the fare type and the agencies traversed within a trip. @@ -18,6 +21,16 @@ public class OrcaFareServiceImpl extends DefaultFareServiceImpl { private static final Logger LOG = LoggerFactory.getLogger(OrcaFareServiceImpl.class); + public static final String COMM_TRANS_AGENCY_ID = "29"; + public static final String KC_METRO_AGENCY_ID = "1"; + public static final String SOUND_TRANSIT_AGENCY_ID = "40"; + public static final String EVERETT_TRANSIT_AGENCY_ID = "97"; + public static final String PIERCE_COUNTY_TRANSIT_AGENCY_ID = "3"; + public static final String SEATTLE_STREET_CAR_AGENCY_ID = "23"; + public static final String WASHINGTON_STATE_FERRIES_AGENCY_ID = "wsf"; + public static final String KITSAP_TRANSIT_AGENCY_ID = "kt"; + public static final int ROUTE_TYPE_FERRY = 4; + public enum RideType { COMM_TRANS_LOCAL_SWIFT, COMM_TRANS_COMMUTER_EXPRESS, @@ -32,6 +45,8 @@ public enum RideType { WASHINGTON_STATE_FERRIES } + private static final Map> classificationStrategy = new HashMap<>(); + // If set to true, the test ride price is used instead of the actual agency cash fare. public boolean IS_TEST; public static final float DEFAULT_TEST_RIDE_PRICE = 3.49f; @@ -44,6 +59,41 @@ public OrcaFareServiceImpl(Collection regularFareRules) { addFareRules(Fare.FareType.orcaYouth, regularFareRules); addFareRules(Fare.FareType.orcaLift, regularFareRules); addFareRules(Fare.FareType.orcaSenior, regularFareRules); + + classificationStrategy.put( + COMM_TRANS_AGENCY_ID, + routeData -> { + try { + int routeId = Integer.parseInt(routeData.getShortName()); + if (routeId >= 400 && routeId <= 899) { + return RideType.COMM_TRANS_COMMUTER_EXPRESS; + } + return RideType.COMM_TRANS_LOCAL_SWIFT; + } catch (NumberFormatException e) { + LOG.warn("Unable to determine comm trans route id from {}.", routeData.getShortName(), e); + return RideType.COMM_TRANS_LOCAL_SWIFT; + } + } + ); + classificationStrategy.put( + KC_METRO_AGENCY_ID, + routeData -> { + if (routeData.getType() == ROUTE_TYPE_FERRY && + routeData.getDesc().contains("Water Taxi: West Seattle")) { + return RideType.KC_WATER_TAXI_WEST_SEATTLE; + } else if (routeData.getType() == ROUTE_TYPE_FERRY && + routeData.getDesc().contains("Water Taxi: Vashon Island")) { + return RideType.KC_WATER_TAXI_VASHON_ISLAND; + } + return RideType.KC_METRO; + } + ); + classificationStrategy.put(SOUND_TRANSIT_AGENCY_ID, routeData -> RideType.SOUND_TRANSIT); + classificationStrategy.put(EVERETT_TRANSIT_AGENCY_ID, routeData -> RideType.EVERETT_TRANSIT); + classificationStrategy.put(PIERCE_COUNTY_TRANSIT_AGENCY_ID, routeData -> RideType.PIERCE_COUNTY_TRANSIT); + classificationStrategy.put(SEATTLE_STREET_CAR_AGENCY_ID, routeData -> RideType.SEATTLE_STREET_CAR); + classificationStrategy.put(WASHINGTON_STATE_FERRIES_AGENCY_ID, routeData -> RideType.WASHINGTON_STATE_FERRIES); + classificationStrategy.put(KITSAP_TRANSIT_AGENCY_ID, routeData -> RideType.KITSAP_TRANSIT); } /** @@ -51,38 +101,8 @@ public OrcaFareServiceImpl(Collection regularFareRules) { * some cases the route description and short name are needed to define inner agency ride types. */ private static RideType classify(Route routeData) { - if ("29".equals(routeData.getAgency().getId())) { - try { - int routeId = Integer.parseInt(routeData.getShortName()); - if (routeId >= 400 && routeId <= 899) { - return RideType.COMM_TRANS_COMMUTER_EXPRESS; - } - return RideType.COMM_TRANS_LOCAL_SWIFT; - } catch (NumberFormatException e) { - LOG.warn("Unable to determine comm trans route id from {}.", routeData.getShortName(), e); - return RideType.COMM_TRANS_LOCAL_SWIFT; - } - } else if ("1".equals(routeData.getAgency().getId())) { - if (routeData.getType() == 4 && routeData.getDesc().contains("Water Taxi: West Seattle")) { - return RideType.KC_WATER_TAXI_WEST_SEATTLE; - } else if (routeData.getType() == 4 && routeData.getDesc().contains("Water Taxi: Vashon Island")) { - return RideType.KC_WATER_TAXI_VASHON_ISLAND; - } - return RideType.KC_METRO; - } else if ("40".equals(routeData.getAgency().getId())) { - return RideType.SOUND_TRANSIT; - } else if ("97".equals(routeData.getAgency().getId())) { - return RideType.EVERETT_TRANSIT; - } else if ("3".equals(routeData.getAgency().getId())) { - return RideType.PIERCE_COUNTY_TRANSIT; - } else if ("23".equals(routeData.getAgency().getId())) { - return RideType.SEATTLE_STREET_CAR; - } else if ("wsf".equalsIgnoreCase(routeData.getAgency().getId())) { - return RideType.WASHINGTON_STATE_FERRIES; - } else if ("kt".equalsIgnoreCase(routeData.getAgency().getId())) { - return RideType.KITSAP_TRANSIT; - } - return null; + Function classifier = classificationStrategy.get(routeData.getAgency().getId()); + return classifier != null ? classifier.apply(routeData) : null; } /** @@ -96,7 +116,7 @@ private float getDiscountedFare(Fare.FareType fareType, RideType rideType, float switch (fareType) { case youth: case orcaYouth: - return getYouthFare(rideType); + return getYouthFare(rideType, defaultFare); case orcaLift: return getLiftFare(rideType, defaultFare); case orcaSenior: @@ -128,12 +148,17 @@ private float getLiftFare(RideType rideType, float defaultFare) { switch (rideType) { case COMM_TRANS_LOCAL_SWIFT: return 1.25f; case COMM_TRANS_COMMUTER_EXPRESS: return 2.00f; - case PIERCE_COUNTY_TRANSIT: return defaultFare; case KC_WATER_TAXI_VASHON_ISLAND: return 4.50f; case KC_WATER_TAXI_WEST_SEATTLE: return 3.75f; case KITSAP_TRANSIT: return 1.00f; - // KCM, Sound Transit, Everett, Seattle Streetcar. - default: return 1.50f; + case KC_METRO: + case SOUND_TRANSIT: + case EVERETT_TRANSIT: + case SEATTLE_STREET_CAR: + return 1.50f; + case PIERCE_COUNTY_TRANSIT: + default: + return defaultFare; } } @@ -153,15 +178,17 @@ private float getSeniorFare(Fare.FareType fareType, RideType rideType, float def return fareType.equals(Fare.FareType.orcaSenior) ? 1.00f : defaultFare; case KC_WATER_TAXI_VASHON_ISLAND: return 3.00f; case KC_WATER_TAXI_WEST_SEATTLE: return 2.50f; - // KC Metro, Sound Transit - default: return 1.00f; + case KC_METRO: + case SOUND_TRANSIT: + return 1.00f; + default: return defaultFare; } } /** * Apply Orca youth discount fares based on the ride type. */ - private float getYouthFare(RideType rideType) { + private float getYouthFare(RideType rideType, float defaultFare) { switch (rideType) { case COMM_TRANS_LOCAL_SWIFT: return 1.75f; case COMM_TRANS_COMMUTER_EXPRESS: return 3.00f; @@ -169,8 +196,12 @@ private float getYouthFare(RideType rideType) { case KC_WATER_TAXI_VASHON_ISLAND: return 4.50f; case KC_WATER_TAXI_WEST_SEATTLE: return 3.75f; case KITSAP_TRANSIT: return 2.00f; - // Default case accounts for KC_METRO, Sound Transit, Everett, and Seattle Streetcar. - default: return 1.50f; + case KC_METRO: + case SOUND_TRANSIT: + case EVERETT_TRANSIT: + case SEATTLE_STREET_CAR: + return 1.50f; + default: return defaultFare; } } @@ -188,25 +219,16 @@ private float getRidePrice(Ride ride, Fare.FareType fareType, Collection rides, Collection fareRules - ) { - return calculateFare(fare, currency, fareType, rides, fareRules); - } - - /** - * Calculate the cost of a journey. Where free transfers are not permitted the cash price is use. If free transfers - * are applicable, the most expensive discount fare across all legs is added to the final cumulative cash price. - */ - public boolean calculateFare(Fare fare, - Currency currency, - Fare.FareType fareType, - List rides, - Collection fareRules ) { float cost = 0; float orcaFareDiscount = 0; diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index 24cca9bed34..1eb6aa7ecd0 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -17,30 +17,61 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Function; import java.util.stream.Stream; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.COMM_TRANS_AGENCY_ID; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.EVERETT_TRANSIT_AGENCY_ID; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.KC_METRO_AGENCY_ID; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.KITSAP_TRANSIT_AGENCY_ID; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.PIERCE_COUNTY_TRANSIT_AGENCY_ID; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.ROUTE_TYPE_FERRY; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.COMM_TRANS_COMMUTER_EXPRESS; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.COMM_TRANS_LOCAL_SWIFT; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.EVERETT_TRANSIT; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.KC_METRO; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.KC_WATER_TAXI_VASHON_ISLAND; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.KC_WATER_TAXI_WEST_SEATTLE; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.KITSAP_TRANSIT; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.PIERCE_COUNTY_TRANSIT; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.SEATTLE_STREET_CAR; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.SOUND_TRANSIT; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.WASHINGTON_STATE_FERRIES; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.SEATTLE_STREET_CAR_AGENCY_ID; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.SOUND_TRANSIT_AGENCY_ID; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.WASHINGTON_STATE_FERRIES_AGENCY_ID; public class OrcaFareServiceTest { private static OrcaFareServiceImpl orcaFareService; private static float DEFAULT_RIDE_PRICE_IN_CENTS; + private static final Map tripStrategy = new HashMap<>(); + @BeforeAll public static void setUpClass() { Map regularFareRules = new HashMap<>(); orcaFareService = new OrcaFareServiceImpl(regularFareRules.values()); orcaFareService.IS_TEST = true; DEFAULT_RIDE_PRICE_IN_CENTS = OrcaFareServiceImpl.DEFAULT_TEST_RIDE_PRICE * 100; + + tripStrategy.put(COMM_TRANS_LOCAL_SWIFT, getRide(COMM_TRANS_AGENCY_ID)); + tripStrategy.put(COMM_TRANS_COMMUTER_EXPRESS, getRide(COMM_TRANS_AGENCY_ID, "400")); + tripStrategy.put(EVERETT_TRANSIT, getRide(EVERETT_TRANSIT_AGENCY_ID)); + tripStrategy.put(PIERCE_COUNTY_TRANSIT, getRide(PIERCE_COUNTY_TRANSIT_AGENCY_ID)); + tripStrategy.put(SEATTLE_STREET_CAR, getRide(SEATTLE_STREET_CAR_AGENCY_ID)); + tripStrategy.put(KITSAP_TRANSIT, getRide(KITSAP_TRANSIT_AGENCY_ID)); + tripStrategy.put( + KC_WATER_TAXI_VASHON_ISLAND, + getRide(KC_METRO_AGENCY_ID, ROUTE_TYPE_FERRY, "Water Taxi: Vashon Island") + ); + tripStrategy.put( + KC_WATER_TAXI_WEST_SEATTLE, + getRide(KC_METRO_AGENCY_ID, ROUTE_TYPE_FERRY, "Water Taxi: West Seattle") + ); + tripStrategy.put(KC_METRO, getRide(KC_METRO_AGENCY_ID)); + tripStrategy.put(SOUND_TRANSIT, getRide(SOUND_TRANSIT_AGENCY_ID)); + tripStrategy.put(WASHINGTON_STATE_FERRIES, getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID)); } /** @@ -56,7 +87,7 @@ public void calculateFareTest(List rides, float expectedFareInCents ) { Fare fare = new Fare(); - orcaFareService.calculateFare(fare, null, fareType, rides, null); + orcaFareService.populateFare(fare, null, fareType, rides, null); Assertions.assertEquals(expectedFareInCents, fare.getFare(fareType).getCents()); } @@ -88,6 +119,10 @@ private static Stream trips() { KC_METRO, COMM_TRANS_COMMUTER_EXPRESS ); + // The cost parameters are made up of the expected fare to be applied after each leg of a trip has been evaluated. + // E.g. if a trip covers three agencies a value of "0f + 0f + 200f" would represent no charge for the first two + // legs, but a charge of 200 cents for the third. This implies that the third leg is the most expensive transfer + // of the trip. return Stream.of( Arguments.of(trip1, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS), Arguments.of(trip1, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS), @@ -96,20 +131,30 @@ private static Stream trips() { Arguments.of(trip1, Fare.FareType.orcaRegular, DEFAULT_RIDE_PRICE_IN_CENTS), Arguments.of(trip1, Fare.FareType.orcaSenior, 200f), Arguments.of(trip1, Fare.FareType.orcaYouth, 300f), - // The following costs are made up of the expected cost to be applied for each agency. This is usually the - // most expensive leg of the journey. + Arguments.of(trip2, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 3), + Arguments.of(trip2, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3), + Arguments.of(trip2, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 3), Arguments.of(trip2, Fare.FareType.orcaLift, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f), Arguments.of(trip2, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), Arguments.of(trip2, Fare.FareType.orcaSenior, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f), Arguments.of(trip2, Fare.FareType.orcaYouth, 200f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), + Arguments.of(trip3, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 3), + Arguments.of(trip3, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3), + Arguments.of(trip3, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 3), Arguments.of(trip3, Fare.FareType.orcaLift, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f), Arguments.of(trip3, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), Arguments.of(trip3, Fare.FareType.orcaSenior, 0f + 0f + 200f), Arguments.of(trip3, Fare.FareType.orcaYouth, 0f + 0f + 300f), + Arguments.of(trip4, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 3), + Arguments.of(trip4, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3), + Arguments.of(trip4, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 3), Arguments.of(trip4, Fare.FareType.orcaLift, 0f + 200f + 0f), Arguments.of(trip4, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), Arguments.of(trip4, Fare.FareType.orcaSenior, 0f + 0f + 200f), Arguments.of(trip4, Fare.FareType.orcaYouth, 0f + 0f + 300f), + Arguments.of(trip5, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 2), + Arguments.of(trip5, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 2), + Arguments.of(trip5, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 2), Arguments.of(trip5, Fare.FareType.orcaLift, 450f + 0f), Arguments.of(trip5, Fare.FareType.orcaRegular, 575f + 0f), Arguments.of(trip5, Fare.FareType.orcaSenior, 300f + 0f), @@ -128,42 +173,7 @@ private static Stream trips() { private static List getTrip(OrcaFareServiceImpl.RideType... rideTypes) { List rides = new ArrayList<>(); for (OrcaFareServiceImpl.RideType rideType : rideTypes) { - switch (rideType) { - case COMM_TRANS_LOCAL_SWIFT: - rides.add(getRide("29")); - break; - case COMM_TRANS_COMMUTER_EXPRESS: - rides.add(getRide("29", "400")); - break; - case EVERETT_TRANSIT: - rides.add(getRide("97")); - break; - case PIERCE_COUNTY_TRANSIT: - rides.add(getRide("3")); - break; - case SEATTLE_STREET_CAR: - rides.add(getRide("23")); - break; - case KITSAP_TRANSIT: - rides.add(getRide("kt")); - break; - case KC_WATER_TAXI_VASHON_ISLAND: - rides.add(getRide("1", 4, "Water Taxi: Vashon Island")); - break; - case KC_WATER_TAXI_WEST_SEATTLE: - rides.add(getRide("1", 4, "Water Taxi: West Seattle")); - break; - case KC_METRO: - rides.add(getRide("1")); - break; - case SOUND_TRANSIT: - rides.add(getRide("40")); - break; - case WASHINGTON_STATE_FERRIES: - rides.add(getRide("wsf")); - break; - } - + rides.add(tripStrategy.get(rideType)); } return rides; } From 6360ad63b7b69d5b6a7a4c03ad3c3a478e5eb14d Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 15 Jul 2021 14:57:32 +0100 Subject: [PATCH 11/17] refactor(Addressed PR feedback and added free transfer trip time): PR feedback and refactored the fa --- .../routing/impl/OrcaFareServiceImpl.java | 35 +++++--- .../opentripplanner/routing/impl/Ride.java | 27 +++---- .../routing/fares/OrcaFareServiceTest.java | 79 ++++++++++++------- 3 files changed, 81 insertions(+), 60 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index a64f59ad319..89728e28dbe 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -1,12 +1,12 @@ package org.opentripplanner.routing.impl; +import com.google.common.collect.Lists; import org.opentripplanner.model.Route; import org.opentripplanner.routing.core.Fare; import org.opentripplanner.routing.core.FareRuleSet; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; import java.util.Collection; import java.util.Currency; import java.util.HashMap; @@ -21,6 +21,8 @@ public class OrcaFareServiceImpl extends DefaultFareServiceImpl { private static final Logger LOG = LoggerFactory.getLogger(OrcaFareServiceImpl.class); + private long FREE_TRANSFER_TIME_LIMIT = 7200; // 2 hours + public static final String COMM_TRANS_AGENCY_ID = "29"; public static final String KC_METRO_AGENCY_ID = "1"; public static final String SOUND_TRANSIT_AGENCY_ID = "40"; @@ -214,14 +216,12 @@ private float getRidePrice(Ride ride, Fare.FareType fareType, Collection ridesSingleton = new ArrayList<>(); - ridesSingleton.add(ride); - return calculateCost(fareType, ridesSingleton, fareRules); + return calculateCost(fareType, Lists.newArrayList(ride), fareRules); } /** - * Calculate the cost of a journey. Where free transfers are not permitted the cash price is use. If free transfers - * are applicable, the most expensive discount fare across all legs is added to the final cumulative cash price. + * Calculate the cost of a journey. Where free transfers are not permitted the cash price is used. If free transfers + * are applicable, the most expensive discount fare across all legs is added to the final cumulative price. */ @Override public boolean populateFare(Fare fare, @@ -230,21 +230,28 @@ public boolean populateFare(Fare fare, List rides, Collection fareRules ) { + long freeTransferTripTime = 0; float cost = 0; float orcaFareDiscount = 0; for (Ride ride : rides) { RideType rideType = classify(ride.routeData); + freeTransferTripTime += ride.endTime - ride.startTime; float singleLegPrice = getRidePrice(ride, fareType, fareRules); float discountedFare = getDiscountedFare(fareType, rideType, singleLegPrice); - if (hasFreeTransfers(fareType, rideType)) { + if (hasFreeTransfers(fareType, rideType, freeTransferTripTime)) { // If using Orca (free transfers), the total fare should be equivalent to the // most expensive leg of the journey. orcaFareDiscount = Float.max(orcaFareDiscount, discountedFare); } else { - // If free transfers not permitted (i.e., paying with cash), accumulate each - // leg as we go. + // If free transfers are not permitted, add the cash price of this leg to the total cost. + // This case is for Washington State Ferries, which do not offer any discounts. cost += singleLegPrice; } + if (freeTransferTripTime > FREE_TRANSFER_TIME_LIMIT) { + // If the trip time has exceeded the free transfer time limit of two hours the rider is required to + // purchase a new fare. This also resets the free transfer trip window. + freeTransferTripTime = 0; + } } cost += orcaFareDiscount; if (cost < Float.POSITIVE_INFINITY) { @@ -254,11 +261,13 @@ public boolean populateFare(Fare fare, } /** - * If using Orca and the ride type is not Washington state ferries (they do no allow free transfers) a free transfer - * can be applied. + * A free transfer can be applied if using Orca, the ride type is not Washington State Ferries (they do no allow + * free transfers) and the trip is still within the free 2 hour transfer window. */ - private boolean hasFreeTransfers(Fare.FareType fareType, RideType rideType) { - return rideType != RideType.WASHINGTON_STATE_FERRIES && usesOrca(fareType); + private boolean hasFreeTransfers(Fare.FareType fareType, RideType rideType, long freeTransferTripTime) { + return rideType != RideType.WASHINGTON_STATE_FERRIES && + usesOrca(fareType) && + freeTransferTripTime <= FREE_TRANSFER_TIME_LIMIT; } /** diff --git a/src/main/java/org/opentripplanner/routing/impl/Ride.java b/src/main/java/org/opentripplanner/routing/impl/Ride.java index e8b713333fa..6db1c48bda9 100644 --- a/src/main/java/org/opentripplanner/routing/impl/Ride.java +++ b/src/main/java/org/opentripplanner/routing/impl/Ride.java @@ -12,7 +12,7 @@ public class Ride { String feedId; - public String agency; // route agency + String agency; // route agency FeedScopedId route; @@ -28,25 +28,25 @@ public class Ride { long startTime; - long endTime; + public long endTime; // in DefaultFareServiceImpl classifier is just the TraverseMode // it can be used differently in custom fare services - public Object classifier; + Object classifier; - public Stop firstStop; + Stop firstStop; - public Stop lastStop; + Stop lastStop; public Ride() { - zones = new HashSet(); + zones = new HashSet<>(); } public String toString() { StringBuilder builder = new StringBuilder(); builder.append("Ride"); if (startZone != null) { - builder.append("(from zone "); + builder.append(" from zone "); builder.append(startZone); } if (endZone != null) { @@ -57,23 +57,14 @@ public String toString() { builder.append(route); if (zones.size() > 0) { builder.append(" through zones "); - boolean first = true; - for (String zone : zones) { - if (first) { - first = false; - } else { - builder.append(","); - } - builder.append(zone); - } + builder.append(String.join(",", zones)); } builder.append(" at "); builder.append(startTime); if (classifier != null) { builder.append(", classified by "); - builder.append(classifier.toString()); + builder.append(classifier); } - builder.append(")"); return builder.toString(); } } diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index 1eb6aa7ecd0..15639c5b0df 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -13,11 +13,11 @@ import org.opentripplanner.routing.impl.OrcaFareServiceImpl; import org.opentripplanner.routing.impl.Ride; -import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.function.Function; +import java.util.stream.Collectors; import java.util.stream.Stream; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.COMM_TRANS_AGENCY_ID; @@ -55,28 +55,28 @@ public static void setUpClass() { orcaFareService.IS_TEST = true; DEFAULT_RIDE_PRICE_IN_CENTS = OrcaFareServiceImpl.DEFAULT_TEST_RIDE_PRICE * 100; - tripStrategy.put(COMM_TRANS_LOCAL_SWIFT, getRide(COMM_TRANS_AGENCY_ID)); - tripStrategy.put(COMM_TRANS_COMMUTER_EXPRESS, getRide(COMM_TRANS_AGENCY_ID, "400")); - tripStrategy.put(EVERETT_TRANSIT, getRide(EVERETT_TRANSIT_AGENCY_ID)); - tripStrategy.put(PIERCE_COUNTY_TRANSIT, getRide(PIERCE_COUNTY_TRANSIT_AGENCY_ID)); - tripStrategy.put(SEATTLE_STREET_CAR, getRide(SEATTLE_STREET_CAR_AGENCY_ID)); - tripStrategy.put(KITSAP_TRANSIT, getRide(KITSAP_TRANSIT_AGENCY_ID)); + tripStrategy.put(COMM_TRANS_LOCAL_SWIFT, getRide(COMM_TRANS_AGENCY_ID, 30)); + tripStrategy.put(COMM_TRANS_COMMUTER_EXPRESS, getRide(COMM_TRANS_AGENCY_ID, "400", 7)); + tripStrategy.put(EVERETT_TRANSIT, getRide(EVERETT_TRANSIT_AGENCY_ID, 23)); + tripStrategy.put(PIERCE_COUNTY_TRANSIT, getRide(PIERCE_COUNTY_TRANSIT_AGENCY_ID, 12)); + tripStrategy.put(SEATTLE_STREET_CAR, getRide(SEATTLE_STREET_CAR_AGENCY_ID, 17)); + tripStrategy.put(KITSAP_TRANSIT, getRide(KITSAP_TRANSIT_AGENCY_ID, 15)); tripStrategy.put( KC_WATER_TAXI_VASHON_ISLAND, - getRide(KC_METRO_AGENCY_ID, ROUTE_TYPE_FERRY, "Water Taxi: Vashon Island") + getRide(KC_METRO_AGENCY_ID, ROUTE_TYPE_FERRY, "Water Taxi: Vashon Island", 11) ); tripStrategy.put( KC_WATER_TAXI_WEST_SEATTLE, - getRide(KC_METRO_AGENCY_ID, ROUTE_TYPE_FERRY, "Water Taxi: West Seattle") + getRide(KC_METRO_AGENCY_ID, ROUTE_TYPE_FERRY, "Water Taxi: West Seattle", 44) ); - tripStrategy.put(KC_METRO, getRide(KC_METRO_AGENCY_ID)); - tripStrategy.put(SOUND_TRANSIT, getRide(SOUND_TRANSIT_AGENCY_ID)); - tripStrategy.put(WASHINGTON_STATE_FERRIES, getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID)); + tripStrategy.put(KC_METRO, getRide(KC_METRO_AGENCY_ID, 13)); + tripStrategy.put(SOUND_TRANSIT, getRide(SOUND_TRANSIT_AGENCY_ID, 5)); + tripStrategy.put(WASHINGTON_STATE_FERRIES, getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 34)); } /** - * These test are design to specifically validate Orca fares. Since these fares are hard-coded, it is acceptable to - * make direct calls to the Orca fare service with predefined routes. Where the default fare is applied a test + * These tests are designed to specifically validate Orca fares. Since these fares are hard-coded, it is acceptable + * to make direct calls to the Orca fare service with predefined routes. Where the default fare is applied a test * substitute {@link OrcaFareServiceImpl#DEFAULT_TEST_RIDE_PRICE} is used. This will be the same for all cash fare * types. */ @@ -119,9 +119,18 @@ private static Stream trips() { KC_METRO, COMM_TRANS_COMMUTER_EXPRESS ); + // Total trip time = 2h 4m. The last leg is outside of the free transfer window so the cash price is paid. + List trip7 = getTrip( + KITSAP_TRANSIT, + KITSAP_TRANSIT, + KITSAP_TRANSIT, + KITSAP_TRANSIT, + WASHINGTON_STATE_FERRIES, + COMM_TRANS_LOCAL_SWIFT + ); // The cost parameters are made up of the expected fare to be applied after each leg of a trip has been evaluated. // E.g. if a trip covers three agencies a value of "0f + 0f + 200f" would represent no charge for the first two - // legs, but a charge of 200 cents for the third. This implies that the third leg is the most expensive transfer + // legs, but a charge of 200 cents for the third. This implies that the third leg is the most expensive portion // of the trip. return Stream.of( Arguments.of(trip1, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS), @@ -159,10 +168,20 @@ private static Stream trips() { Arguments.of(trip5, Fare.FareType.orcaRegular, 575f + 0f), Arguments.of(trip5, Fare.FareType.orcaSenior, 300f + 0f), Arguments.of(trip5, Fare.FareType.orcaYouth, 450f + 0f), + Arguments.of(trip6, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 3), + Arguments.of(trip6, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3), + Arguments.of(trip6, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 3), Arguments.of(trip6, Fare.FareType.orcaLift, 0f + 0f + 200f), Arguments.of(trip6, Fare.FareType.orcaRegular, 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS), Arguments.of(trip6, Fare.FareType.orcaSenior, 0f + 0f + 200f), - Arguments.of(trip6, Fare.FareType.orcaYouth, 0f + 0f + 300f) + Arguments.of(trip6, Fare.FareType.orcaYouth, 0f + 0f + 300f), + Arguments.of(trip7, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 6), + Arguments.of(trip7, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 6), + Arguments.of(trip7, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 6), + Arguments.of(trip7, Fare.FareType.orcaLift, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), + Arguments.of(trip7, Fare.FareType.orcaRegular, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), + Arguments.of(trip7, Fare.FareType.orcaSenior, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), + Arguments.of(trip7, Fare.FareType.orcaYouth, 200f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS) ); } @@ -171,30 +190,31 @@ private static Stream trips() { * the values used in {@link OrcaFareServiceImpl} to determine the correct ride type. */ private static List getTrip(OrcaFareServiceImpl.RideType... rideTypes) { - List rides = new ArrayList<>(); - for (OrcaFareServiceImpl.RideType rideType : rideTypes) { - rides.add(tripStrategy.get(rideType)); - } - return rides; + return Arrays.stream(rideTypes).map(tripStrategy::get).collect(Collectors.toList()); } - private static Ride getRide(String agencyId) { - return getRide(agencyId, "-1", -1, null); + private static Ride getRide(String agencyId, long durationMins) { + return getRide(agencyId, "-1", -1, null, durationMins); } - private static Ride getRide(String agencyId, int rideType, String desc) { - return getRide(agencyId, "-1", rideType, desc); + private static Ride getRide(String agencyId, int rideType, String desc, long durationMins) { + return getRide(agencyId, "-1", rideType, desc, durationMins); } - private static Ride getRide(String agencyId, String shortName) { - return getRide(agencyId, shortName, -1, null); + private static Ride getRide(String agencyId, String shortName, long durationMins) { + return getRide(agencyId, shortName, -1, null,0); } /** * Create a {@link Ride} containing route data that will be used by {@link OrcaFareServiceImpl} to determine the * correct ride type. */ - private static Ride getRide(String agencyId, String shortName, int rideType, String desc) { + private static Ride getRide(String agencyId, + String shortName, + int rideType, + String desc, + long durationMins + ) { Ride ride = new Ride(); Agency agency = new Agency(); agency.setId(agencyId); @@ -204,6 +224,7 @@ private static Ride getRide(String agencyId, String shortName, int rideType, Str route.setType(rideType); route.setDesc(desc); ride.routeData = route; + ride.endTime = durationMins * 60; return ride; } } From 9f5f4405a00e447345211fa10d0c8a9efa742daf Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 16 Jul 2021 17:13:25 +0100 Subject: [PATCH 12/17] refactor(Free transfer window updates): redefined the process to define transfer window. Updated uni --- .../opentripplanner/routing/core/Fare.java | 5 +- .../routing/impl/OrcaFareServiceImpl.java | 80 ++++-- .../opentripplanner/routing/impl/Ride.java | 4 +- .../routing/fares/OrcaFareServiceTest.java | 242 +++++++----------- 4 files changed, 144 insertions(+), 187 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/core/Fare.java b/src/main/java/org/opentripplanner/routing/core/Fare.java index 5cfa69cc4b5..d039793c3fe 100644 --- a/src/main/java/org/opentripplanner/routing/core/Fare.java +++ b/src/main/java/org/opentripplanner/routing/core/Fare.java @@ -14,8 +14,9 @@ */ public class Fare { - public static enum FareType implements Serializable { - regular, student, senior, tram, special, youth, orcaRegular, orcaYouth, orcaLift, orcaSenior + public enum FareType implements Serializable { + regular, student, senior, tram, special, youth, + electronicRegular, electronicYouth, electronicSpecial, electronicSenior } /** diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index 89728e28dbe..3206f2f48f9 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -21,13 +21,14 @@ public class OrcaFareServiceImpl extends DefaultFareServiceImpl { private static final Logger LOG = LoggerFactory.getLogger(OrcaFareServiceImpl.class); - private long FREE_TRANSFER_TIME_LIMIT = 7200; // 2 hours + private static final long FREE_TRANSFER_TIME_DURATION = 7200; // 2 hours public static final String COMM_TRANS_AGENCY_ID = "29"; public static final String KC_METRO_AGENCY_ID = "1"; public static final String SOUND_TRANSIT_AGENCY_ID = "40"; public static final String EVERETT_TRANSIT_AGENCY_ID = "97"; public static final String PIERCE_COUNTY_TRANSIT_AGENCY_ID = "3"; + public static final String SKAGIT_TRANSIT_AGENCY_ID = "e0e4541a-2714-487b-b30c-f5c6cb4a310f"; public static final String SEATTLE_STREET_CAR_AGENCY_ID = "23"; public static final String WASHINGTON_STATE_FERRIES_AGENCY_ID = "wsf"; public static final String KITSAP_TRANSIT_AGENCY_ID = "kt"; @@ -42,6 +43,7 @@ public enum RideType { KC_METRO, KITSAP_TRANSIT, PIERCE_COUNTY_TRANSIT, + SKAGIT_TRANSIT, SEATTLE_STREET_CAR, SOUND_TRANSIT, WASHINGTON_STATE_FERRIES @@ -57,10 +59,10 @@ public OrcaFareServiceImpl(Collection regularFareRules) { addFareRules(Fare.FareType.regular, regularFareRules); addFareRules(Fare.FareType.senior, regularFareRules); addFareRules(Fare.FareType.youth, regularFareRules); - addFareRules(Fare.FareType.orcaRegular, regularFareRules); - addFareRules(Fare.FareType.orcaYouth, regularFareRules); - addFareRules(Fare.FareType.orcaLift, regularFareRules); - addFareRules(Fare.FareType.orcaSenior, regularFareRules); + addFareRules(Fare.FareType.electronicRegular, regularFareRules); + addFareRules(Fare.FareType.electronicYouth, regularFareRules); + addFareRules(Fare.FareType.electronicSpecial, regularFareRules); + addFareRules(Fare.FareType.electronicSenior, regularFareRules); classificationStrategy.put( COMM_TRANS_AGENCY_ID, @@ -93,6 +95,7 @@ public OrcaFareServiceImpl(Collection regularFareRules) { classificationStrategy.put(SOUND_TRANSIT_AGENCY_ID, routeData -> RideType.SOUND_TRANSIT); classificationStrategy.put(EVERETT_TRANSIT_AGENCY_ID, routeData -> RideType.EVERETT_TRANSIT); classificationStrategy.put(PIERCE_COUNTY_TRANSIT_AGENCY_ID, routeData -> RideType.PIERCE_COUNTY_TRANSIT); + classificationStrategy.put(SKAGIT_TRANSIT_AGENCY_ID, routeData -> RideType.SKAGIT_TRANSIT); classificationStrategy.put(SEATTLE_STREET_CAR_AGENCY_ID, routeData -> RideType.SEATTLE_STREET_CAR); classificationStrategy.put(WASHINGTON_STATE_FERRIES_AGENCY_ID, routeData -> RideType.WASHINGTON_STATE_FERRIES); classificationStrategy.put(KITSAP_TRANSIT_AGENCY_ID, routeData -> RideType.KITSAP_TRANSIT); @@ -117,14 +120,14 @@ private float getDiscountedFare(Fare.FareType fareType, RideType rideType, float } switch (fareType) { case youth: - case orcaYouth: + case electronicYouth: return getYouthFare(rideType, defaultFare); - case orcaLift: + case electronicSpecial: return getLiftFare(rideType, defaultFare); - case orcaSenior: + case electronicSenior: case senior: return getSeniorFare(fareType, rideType, defaultFare); - case orcaRegular: + case electronicRegular: return getRegularFare(rideType, defaultFare); case regular: default: @@ -172,12 +175,12 @@ private float getSeniorFare(Fare.FareType fareType, RideType rideType, float def case COMM_TRANS_LOCAL_SWIFT: return 1.25f; case COMM_TRANS_COMMUTER_EXPRESS: return 2.00f; case EVERETT_TRANSIT: - return fareType.equals(Fare.FareType.orcaSenior) ? 0.50f : defaultFare; + return fareType.equals(Fare.FareType.electronicSenior) ? 0.50f : defaultFare; case PIERCE_COUNTY_TRANSIT: case SEATTLE_STREET_CAR: case KITSAP_TRANSIT: // Pierce, Seattle Streetcar, and Kitsap only provide discounted senior fare for orca. - return fareType.equals(Fare.FareType.orcaSenior) ? 1.00f : defaultFare; + return fareType.equals(Fare.FareType.electronicSenior) ? 1.00f : defaultFare; case KC_WATER_TAXI_VASHON_ISLAND: return 3.00f; case KC_WATER_TAXI_WEST_SEATTLE: return 2.50f; case KC_METRO: @@ -222,6 +225,10 @@ private float getRidePrice(Ride ride, Fare.FareType fareType, Collection rides, Collection fareRules ) { - long freeTransferTripTime = 0; + Long freeTransferStartTime = null; float cost = 0; float orcaFareDiscount = 0; for (Ride ride : rides) { RideType rideType = classify(ride.routeData); - freeTransferTripTime += ride.endTime - ride.startTime; + if (freeTransferStartTime == null && permitsFreeTransfers(rideType)) { + // The start of a free transfer must be with a transit agency that permits it! + freeTransferStartTime = ride.startTime; + } float singleLegPrice = getRidePrice(ride, fareType, fareRules); float discountedFare = getDiscountedFare(fareType, rideType, singleLegPrice); - if (hasFreeTransfers(fareType, rideType, freeTransferTripTime)) { + if (hasFreeTransfers(fareType, rideType) && inFreeTransferWindow(freeTransferStartTime, ride.startTime)) { // If using Orca (free transfers), the total fare should be equivalent to the // most expensive leg of the journey. orcaFareDiscount = Float.max(orcaFareDiscount, discountedFare); } else { // If free transfers are not permitted, add the cash price of this leg to the total cost. - // This case is for Washington State Ferries, which do not offer any discounts. cost += singleLegPrice; } - if (freeTransferTripTime > FREE_TRANSFER_TIME_LIMIT) { + if (!inFreeTransferWindow(freeTransferStartTime, ride.startTime)) { // If the trip time has exceeded the free transfer time limit of two hours the rider is required to - // purchase a new fare. This also resets the free transfer trip window. - freeTransferTripTime = 0; + // purchase a new fare. This also resets the free transfer trip window, applies the orca discount to the + // overall cost and then resets the orca fare discount ready for the next transfer window. + freeTransferStartTime = null; + cost += orcaFareDiscount; + orcaFareDiscount = 0; } } cost += orcaFareDiscount; @@ -261,22 +273,34 @@ public boolean populateFare(Fare fare, } /** - * A free transfer can be applied if using Orca, the ride type is not Washington State Ferries (they do no allow - * free transfers) and the trip is still within the free 2 hour transfer window. + * Trip within the free two hour transfer window. + */ + private boolean inFreeTransferWindow(Long freeTransferStartTime, long currentLegStartTime) { + return freeTransferStartTime != null && + currentLegStartTime < freeTransferStartTime + FREE_TRANSFER_TIME_DURATION; + } + + /** + * A free transfer can be applied if using Orca and the transit agency permits free transfers. + */ + private boolean hasFreeTransfers(Fare.FareType fareType, RideType rideType) { + return permitsFreeTransfers(rideType) && usesOrca(fareType); + } + + /** + * All transit agencies permit free transfers, apart from these. */ - private boolean hasFreeTransfers(Fare.FareType fareType, RideType rideType, long freeTransferTripTime) { - return rideType != RideType.WASHINGTON_STATE_FERRIES && - usesOrca(fareType) && - freeTransferTripTime <= FREE_TRANSFER_TIME_LIMIT; + private boolean permitsFreeTransfers(RideType rideType) { + return rideType != RideType.WASHINGTON_STATE_FERRIES && rideType != RideType.SKAGIT_TRANSIT; } /** * Define Orca fare types. */ private boolean usesOrca(Fare.FareType fareType) { - return fareType.equals(Fare.FareType.orcaLift) || - fareType.equals(Fare.FareType.orcaSenior) || - fareType.equals(Fare.FareType.orcaRegular) || - fareType.equals(Fare.FareType.orcaYouth); + return fareType.equals(Fare.FareType.electronicSpecial) || + fareType.equals(Fare.FareType.electronicSenior) || + fareType.equals(Fare.FareType.electronicRegular) || + fareType.equals(Fare.FareType.electronicYouth); } } diff --git a/src/main/java/org/opentripplanner/routing/impl/Ride.java b/src/main/java/org/opentripplanner/routing/impl/Ride.java index 6db1c48bda9..78e46629a68 100644 --- a/src/main/java/org/opentripplanner/routing/impl/Ride.java +++ b/src/main/java/org/opentripplanner/routing/impl/Ride.java @@ -26,9 +26,9 @@ public class Ride { String endZone; - long startTime; + public long startTime; - public long endTime; + long endTime; // in DefaultFareServiceImpl classifier is just the TraverseMode // it can be used differently in custom fare services diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index 15639c5b0df..185523b98a9 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -1,10 +1,9 @@ package org.opentripplanner.routing.fares; +import com.google.common.collect.Lists; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.api.Test; import org.opentripplanner.model.Agency; import org.opentripplanner.model.FeedScopedId; import org.opentripplanner.model.Route; @@ -13,65 +12,26 @@ import org.opentripplanner.routing.impl.OrcaFareServiceImpl; import org.opentripplanner.routing.impl.Ride; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; -import java.util.stream.Stream; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.COMM_TRANS_AGENCY_ID; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.EVERETT_TRANSIT_AGENCY_ID; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.KC_METRO_AGENCY_ID; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.KITSAP_TRANSIT_AGENCY_ID; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.PIERCE_COUNTY_TRANSIT_AGENCY_ID; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.ROUTE_TYPE_FERRY; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.COMM_TRANS_COMMUTER_EXPRESS; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.COMM_TRANS_LOCAL_SWIFT; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.EVERETT_TRANSIT; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.KC_METRO; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.KC_WATER_TAXI_VASHON_ISLAND; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.KC_WATER_TAXI_WEST_SEATTLE; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.KITSAP_TRANSIT; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.PIERCE_COUNTY_TRANSIT; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.SEATTLE_STREET_CAR; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.SOUND_TRANSIT; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.RideType.WASHINGTON_STATE_FERRIES; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.SEATTLE_STREET_CAR_AGENCY_ID; -import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.SOUND_TRANSIT_AGENCY_ID; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.WASHINGTON_STATE_FERRIES_AGENCY_ID; +//TODO: Add additional test routes once defined by Jon. public class OrcaFareServiceTest { private static OrcaFareServiceImpl orcaFareService; private static float DEFAULT_RIDE_PRICE_IN_CENTS; - private static final Map tripStrategy = new HashMap<>(); - @BeforeAll public static void setUpClass() { Map regularFareRules = new HashMap<>(); orcaFareService = new OrcaFareServiceImpl(regularFareRules.values()); orcaFareService.IS_TEST = true; DEFAULT_RIDE_PRICE_IN_CENTS = OrcaFareServiceImpl.DEFAULT_TEST_RIDE_PRICE * 100; - - tripStrategy.put(COMM_TRANS_LOCAL_SWIFT, getRide(COMM_TRANS_AGENCY_ID, 30)); - tripStrategy.put(COMM_TRANS_COMMUTER_EXPRESS, getRide(COMM_TRANS_AGENCY_ID, "400", 7)); - tripStrategy.put(EVERETT_TRANSIT, getRide(EVERETT_TRANSIT_AGENCY_ID, 23)); - tripStrategy.put(PIERCE_COUNTY_TRANSIT, getRide(PIERCE_COUNTY_TRANSIT_AGENCY_ID, 12)); - tripStrategy.put(SEATTLE_STREET_CAR, getRide(SEATTLE_STREET_CAR_AGENCY_ID, 17)); - tripStrategy.put(KITSAP_TRANSIT, getRide(KITSAP_TRANSIT_AGENCY_ID, 15)); - tripStrategy.put( - KC_WATER_TAXI_VASHON_ISLAND, - getRide(KC_METRO_AGENCY_ID, ROUTE_TYPE_FERRY, "Water Taxi: Vashon Island", 11) - ); - tripStrategy.put( - KC_WATER_TAXI_WEST_SEATTLE, - getRide(KC_METRO_AGENCY_ID, ROUTE_TYPE_FERRY, "Water Taxi: West Seattle", 44) - ); - tripStrategy.put(KC_METRO, getRide(KC_METRO_AGENCY_ID, 13)); - tripStrategy.put(SOUND_TRANSIT, getRide(SOUND_TRANSIT_AGENCY_ID, 5)); - tripStrategy.put(WASHINGTON_STATE_FERRIES, getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 34)); } /** @@ -80,9 +40,7 @@ public static void setUpClass() { * substitute {@link OrcaFareServiceImpl#DEFAULT_TEST_RIDE_PRICE} is used. This will be the same for all cash fare * types. */ - @ParameterizedTest - @MethodSource("trips") - public void calculateFareTest(List rides, + private static void calculateFare(List rides, Fare.FareType fareType, float expectedFareInCents ) { @@ -91,130 +49,104 @@ public void calculateFareTest(List rides, Assertions.assertEquals(expectedFareInCents, fare.getFare(fareType).getCents()); } - private static Stream trips() { - List trip1 = getTrip( - COMM_TRANS_COMMUTER_EXPRESS - ); - List trip2 = getTrip( - KITSAP_TRANSIT, - WASHINGTON_STATE_FERRIES, - COMM_TRANS_LOCAL_SWIFT - ); - List trip3 = getTrip( - PIERCE_COUNTY_TRANSIT, - SOUND_TRANSIT, - COMM_TRANS_COMMUTER_EXPRESS - ); - List trip4 = getTrip( - EVERETT_TRANSIT, - COMM_TRANS_COMMUTER_EXPRESS, - SOUND_TRANSIT - ); - List trip5 = getTrip( - KC_WATER_TAXI_VASHON_ISLAND, - KC_METRO - ); - List trip6 = getTrip( - SEATTLE_STREET_CAR, - KC_METRO, - COMM_TRANS_COMMUTER_EXPRESS - ); - // Total trip time = 2h 4m. The last leg is outside of the free transfer window so the cash price is paid. - List trip7 = getTrip( - KITSAP_TRANSIT, - KITSAP_TRANSIT, - KITSAP_TRANSIT, - KITSAP_TRANSIT, - WASHINGTON_STATE_FERRIES, - COMM_TRANS_LOCAL_SWIFT - ); - // The cost parameters are made up of the expected fare to be applied after each leg of a trip has been evaluated. - // E.g. if a trip covers three agencies a value of "0f + 0f + 200f" would represent no charge for the first two - // legs, but a charge of 200 cents for the third. This implies that the third leg is the most expensive portion - // of the trip. - return Stream.of( - Arguments.of(trip1, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS), - Arguments.of(trip1, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS), - Arguments.of(trip1, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS), - Arguments.of(trip1, Fare.FareType.orcaLift, 200f), - Arguments.of(trip1, Fare.FareType.orcaRegular, DEFAULT_RIDE_PRICE_IN_CENTS), - Arguments.of(trip1, Fare.FareType.orcaSenior, 200f), - Arguments.of(trip1, Fare.FareType.orcaYouth, 300f), - Arguments.of(trip2, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip2, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip2, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip2, Fare.FareType.orcaLift, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f), - Arguments.of(trip2, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), - Arguments.of(trip2, Fare.FareType.orcaSenior, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f), - Arguments.of(trip2, Fare.FareType.orcaYouth, 200f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), - Arguments.of(trip3, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip3, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip3, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip3, Fare.FareType.orcaLift, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f), - Arguments.of(trip3, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), - Arguments.of(trip3, Fare.FareType.orcaSenior, 0f + 0f + 200f), - Arguments.of(trip3, Fare.FareType.orcaYouth, 0f + 0f + 300f), - Arguments.of(trip4, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip4, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip4, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip4, Fare.FareType.orcaLift, 0f + 200f + 0f), - Arguments.of(trip4, Fare.FareType.orcaRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f), - Arguments.of(trip4, Fare.FareType.orcaSenior, 0f + 0f + 200f), - Arguments.of(trip4, Fare.FareType.orcaYouth, 0f + 0f + 300f), - Arguments.of(trip5, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 2), - Arguments.of(trip5, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 2), - Arguments.of(trip5, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 2), - Arguments.of(trip5, Fare.FareType.orcaLift, 450f + 0f), - Arguments.of(trip5, Fare.FareType.orcaRegular, 575f + 0f), - Arguments.of(trip5, Fare.FareType.orcaSenior, 300f + 0f), - Arguments.of(trip5, Fare.FareType.orcaYouth, 450f + 0f), - Arguments.of(trip6, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip6, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip6, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 3), - Arguments.of(trip6, Fare.FareType.orcaLift, 0f + 0f + 200f), - Arguments.of(trip6, Fare.FareType.orcaRegular, 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS), - Arguments.of(trip6, Fare.FareType.orcaSenior, 0f + 0f + 200f), - Arguments.of(trip6, Fare.FareType.orcaYouth, 0f + 0f + 300f), - Arguments.of(trip7, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 6), - Arguments.of(trip7, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 6), - Arguments.of(trip7, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 6), - Arguments.of(trip7, Fare.FareType.orcaLift, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), - Arguments.of(trip7, Fare.FareType.orcaRegular, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), - Arguments.of(trip7, Fare.FareType.orcaSenior, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS), - Arguments.of(trip7, Fare.FareType.orcaYouth, 200f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS) - ); + /** + * Test to confirm the correct transfer cost per fare type within a single agency. + */ + @Test + public void calFareForSingleAgency() { + Ride ride = getRide(COMM_TRANS_AGENCY_ID, "400", 0); + List rides = Lists.newArrayList(ride); + calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.electronicSpecial, 200f); + calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.electronicSenior, 200f); + calculateFare(rides, Fare.FareType.electronicYouth, 300f); } /** - * Build a list of {@link Ride)s from the ride types provided. The values used here to produce a {@link Ride} match - * the values used in {@link OrcaFareServiceImpl} to determine the correct ride type. + * WSF do not except free transfers. This test is to make sure the rider is charged the cash price for WSF as well + * as the highest fare where Orca can be used. */ - private static List getTrip(OrcaFareServiceImpl.RideType... rideTypes) { - return Arrays.stream(rideTypes).map(tripStrategy::get).collect(Collectors.toList()); + @Test + public void calFareWithNoFreeTransfer() { + List rides = Lists.newArrayList(); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 0)); + rides.add(getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 1)); + rides.add(getRide(COMM_TRANS_AGENCY_ID, 2)); + calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 3); + calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3); + calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 3); + calculateFare(rides, Fare.FareType.electronicSpecial, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f); + calculateFare(rides, Fare.FareType.electronicRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.electronicSenior, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f); + calculateFare(rides, Fare.FareType.electronicYouth, 200f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f); } - private static Ride getRide(String agencyId, long durationMins) { - return getRide(agencyId, "-1", -1, null, durationMins); + /** + * Total trip time is 2h 30m. The first four transfers are within the permitted two hour window. A single (highest) + * Orca fare will be charged for these transfers. The fifth transfer is outside of the two hour window and will be + * charged a cash rate. At this point, the two hour window will start again so the final transfer will be + * charged at a discount rate... if using Orca. + */ + @Test + public void calFareThatExceedsTwoHourFreeTransferWindow() { + List rides = Lists.newArrayList(); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 0)); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 30)); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 60)); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 90)); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 120)); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 150)); + + calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 6); + calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 6); + calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 6); + calculateFare(rides, Fare.FareType.electronicSpecial, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 100f); + calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f + 0f + + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.electronicSenior, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 100f); + calculateFare(rides, Fare.FareType.electronicYouth, 200f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 200f); } - private static Ride getRide(String agencyId, int rideType, String desc, long durationMins) { - return getRide(agencyId, "-1", rideType, desc, durationMins); + /** + * This trip starts with a cash fare so the free transfer window doesn't start until the second transfer. Therefore, + * all subsequent transfers will come under one transfer window and only one Orca discount charge will apply. + */ + @Test + public void calFareThatStartsWithACashFare() { + List rides = Lists.newArrayList(); + rides.add(getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 0)); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 30)); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 60)); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 90)); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 120)); + rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 149)); + + calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 6); + calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 6); + calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 6); + calculateFare(rides, Fare.FareType.electronicSpecial, DEFAULT_RIDE_PRICE_IN_CENTS + 100f + 0f + 0f + 0f + 0f); + calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS + + DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f + 0f); + calculateFare(rides, Fare.FareType.electronicSenior, DEFAULT_RIDE_PRICE_IN_CENTS + 100f + 0f + 0f + 0f + 0f); + calculateFare(rides, Fare.FareType.electronicYouth, DEFAULT_RIDE_PRICE_IN_CENTS + 200f + 0f + 0f + 0f + 0f); } - private static Ride getRide(String agencyId, String shortName, long durationMins) { - return getRide(agencyId, shortName, -1, null,0); + private static Ride getRide(String agencyId, long startTimeMins) { + return getRide(agencyId, "-1", -1, null, startTimeMins); + } + + private static Ride getRide(String agencyId, String shortName, long startTimeMins) { + return getRide(agencyId, shortName, -1, null,startTimeMins); } /** * Create a {@link Ride} containing route data that will be used by {@link OrcaFareServiceImpl} to determine the * correct ride type. */ - private static Ride getRide(String agencyId, - String shortName, - int rideType, - String desc, - long durationMins - ) { + private static Ride getRide(String agencyId, String shortName, int rideType, String desc, long startTimeMins) { Ride ride = new Ride(); Agency agency = new Agency(); agency.setId(agencyId); @@ -224,7 +156,7 @@ private static Ride getRide(String agencyId, route.setType(rideType); route.setDesc(desc); ride.routeData = route; - ride.endTime = durationMins * 60; + ride.startTime = startTimeMins * 60; return ride; } } From ffefd9c0cfaf7b8be095732def71e0360eeedf0c Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Mon, 19 Jul 2021 10:42:16 +0100 Subject: [PATCH 13/17] refactor(OrcaFareServiceTest.java): Renamed cal to calculate for each unit test. --- .../routing/fares/OrcaFareServiceTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index 185523b98a9..a23ca77aac4 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -53,7 +53,7 @@ private static void calculateFare(List rides, * Test to confirm the correct transfer cost per fare type within a single agency. */ @Test - public void calFareForSingleAgency() { + public void calculateFareForSingleAgency() { Ride ride = getRide(COMM_TRANS_AGENCY_ID, "400", 0); List rides = Lists.newArrayList(ride); calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS); @@ -70,7 +70,7 @@ public void calFareForSingleAgency() { * as the highest fare where Orca can be used. */ @Test - public void calFareWithNoFreeTransfer() { + public void calculateFareWithNoFreeTransfer() { List rides = Lists.newArrayList(); rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 0)); rides.add(getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 1)); @@ -91,7 +91,7 @@ public void calFareWithNoFreeTransfer() { * charged at a discount rate... if using Orca. */ @Test - public void calFareThatExceedsTwoHourFreeTransferWindow() { + public void calculateFareThatExceedsTwoHourFreeTransferWindow() { List rides = Lists.newArrayList(); rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 0)); rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 30)); @@ -115,7 +115,7 @@ public void calFareThatExceedsTwoHourFreeTransferWindow() { * all subsequent transfers will come under one transfer window and only one Orca discount charge will apply. */ @Test - public void calFareThatStartsWithACashFare() { + public void calculateFareThatStartsWithACashFare() { List rides = Lists.newArrayList(); rides.add(getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 0)); rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 30)); From b8badf265deb89a5633758b974595d076d44ca71 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 22 Jul 2021 18:11:53 +0100 Subject: [PATCH 14/17] refactor(Added WSF and Kitsap fast ferry fares): Added agency specific discount fares that apply out --- .../routing/impl/OrcaFareServiceImpl.java | 144 +++++++++++++++--- .../opentripplanner/routing/impl/Ride.java | 2 +- .../routing/fares/OrcaFareServiceTest.java | 121 +++++++++++---- 3 files changed, 212 insertions(+), 55 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index 3206f2f48f9..c5fac2b8eb6 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -1,5 +1,6 @@ package org.opentripplanner.routing.impl; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import org.opentripplanner.model.Route; import org.opentripplanner.routing.core.Fare; @@ -42,6 +43,8 @@ public enum RideType { KC_WATER_TAXI_WEST_SEATTLE, KC_METRO, KITSAP_TRANSIT, + KITSAP_TRANSIT_FAST_FERRY_EASTBOUND, + KITSAP_TRANSIT_FAST_FERRY_WESTBOUND, PIERCE_COUNTY_TRANSIT, SKAGIT_TRANSIT, SEATTLE_STREET_CAR, @@ -50,6 +53,7 @@ public enum RideType { } private static final Map> classificationStrategy = new HashMap<>(); + private static final Map> washingtonStateFerriesFares = new HashMap<>(); // If set to true, the test ride price is used instead of the actual agency cash fare. public boolean IS_TEST; @@ -83,7 +87,7 @@ public OrcaFareServiceImpl(Collection regularFareRules) { KC_METRO_AGENCY_ID, routeData -> { if (routeData.getType() == ROUTE_TYPE_FERRY && - routeData.getDesc().contains("Water Taxi: West Seattle")) { + routeData.getLongName().contains("Water Taxi: West Seattle")) { return RideType.KC_WATER_TAXI_WEST_SEATTLE; } else if (routeData.getType() == ROUTE_TYPE_FERRY && routeData.getDesc().contains("Water Taxi: Vashon Island")) { @@ -99,49 +103,99 @@ public OrcaFareServiceImpl(Collection regularFareRules) { classificationStrategy.put(SEATTLE_STREET_CAR_AGENCY_ID, routeData -> RideType.SEATTLE_STREET_CAR); classificationStrategy.put(WASHINGTON_STATE_FERRIES_AGENCY_ID, routeData -> RideType.WASHINGTON_STATE_FERRIES); classificationStrategy.put(KITSAP_TRANSIT_AGENCY_ID, routeData -> RideType.KITSAP_TRANSIT); + + washingtonStateFerriesFares.put( + "Seattle - Bainbridge Island", + ImmutableMap.of(Fare.FareType.regular, 9.05f, Fare.FareType.youth, 4.50f, Fare.FareType.senior, 4.50f) + ); + washingtonStateFerriesFares.put( + "Seattle - Bremerton", + ImmutableMap.of(Fare.FareType.regular, 9.05f, Fare.FareType.youth, 4.50f, Fare.FareType.senior, 4.50f) + ); + washingtonStateFerriesFares.put( + "Mukilteo - Clinton", + ImmutableMap.of(Fare.FareType.regular, 5.55f, Fare.FareType.youth, 2.75f, Fare.FareType.senior, 2.75f) + ); + washingtonStateFerriesFares.put( + "Fauntleroy - Vashon Island", + ImmutableMap.of(Fare.FareType.regular, 5.95f, Fare.FareType.youth, 2.95f, Fare.FareType.senior, 2.95f) + ); + washingtonStateFerriesFares.put( + "Fauntleroy - Southworth", + ImmutableMap.of(Fare.FareType.regular, 7.10f, Fare.FareType.youth, 3.55f, Fare.FareType.senior, 3.55f) + ); + washingtonStateFerriesFares.put( + "Edmonds - Kingston", + ImmutableMap.of(Fare.FareType.regular, 9.05f, Fare.FareType.youth, 4.50f, Fare.FareType.senior, 4.50f) + ); + washingtonStateFerriesFares.put( + "Point Defiance - Tahlequah", + ImmutableMap.of(Fare.FareType.regular, 5.95f, Fare.FareType.youth, 2.95f, Fare.FareType.senior, 2.95f) + ); } /** * Classify the ride type based on the route information provided. In most cases the agency name is sufficient. In - * some cases the route description and short name are needed to define inner agency ride types. + * some cases the route description and short name are needed to define inner agency ride types. For Kitsap, the + * route data is enough to define the agency, but addition trip id checks are needed to define the fast ferry direction. */ - private static RideType classify(Route routeData) { + private static RideType classify(Route routeData, String tripId) { Function classifier = classificationStrategy.get(routeData.getAgency().getId()); - return classifier != null ? classifier.apply(routeData) : null; + if (classifier == null) { + return null; + } + + RideType rideType = classifier.apply(routeData); + if (rideType == RideType.KITSAP_TRANSIT && + routeData.getId().getId().equalsIgnoreCase("Kitsap Fast Ferry") && + routeData.getType() == ROUTE_TYPE_FERRY + ) { + // Additional trip id checks are required to distinguish Kitsap fast ferry routes. + if (tripId.contains("east")) { + rideType = RideType.KITSAP_TRANSIT_FAST_FERRY_EASTBOUND; + } else if (tripId.contains("west")) { + rideType = RideType.KITSAP_TRANSIT_FAST_FERRY_WESTBOUND; + } + } + return rideType; } /** * Define which discount fare should be applied based on the fare type. If the ride type is unknown the discount * fare can not be applied, use the default fare. */ - private float getDiscountedFare(Fare.FareType fareType, RideType rideType, float defaultFare) { + private float getDiscountedFare(Fare.FareType fareType, RideType rideType, float defaultFare, Route route) { if (rideType == null) { return defaultFare; } switch (fareType) { case youth: case electronicYouth: - return getYouthFare(rideType, defaultFare); + return getYouthFare(fareType, rideType, defaultFare, route); case electronicSpecial: return getLiftFare(rideType, defaultFare); case electronicSenior: case senior: - return getSeniorFare(fareType, rideType, defaultFare); - case electronicRegular: - return getRegularFare(rideType, defaultFare); + return getSeniorFare(fareType, rideType, defaultFare, route); case regular: + case electronicRegular: + return getRegularFare(fareType, rideType, defaultFare, route); default: return defaultFare; } } /** - * Apply Orca regular discount fares. If the ride type can not be matched the default fare is used. + * Apply Orca regular discount fares. If the ride type cannot be matched the default fare is used. */ - private float getRegularFare(RideType rideType, float defaultFare) { + private float getRegularFare(Fare.FareType fareType, RideType rideType, float defaultFare, Route route) { switch (rideType) { case KC_WATER_TAXI_VASHON_ISLAND: return 5.75f; case KC_WATER_TAXI_WEST_SEATTLE: return 5.00f; + case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND: return 2.00f; + case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND: return 10.00f; + case WASHINGTON_STATE_FERRIES: + return getWashingtonStateFerriesFare(route.getLongName(), fareType, defaultFare); default: return defaultFare; } } @@ -168,9 +222,9 @@ private float getLiftFare(RideType rideType, float defaultFare) { } /** - * Apply Orca senior discount fares based on the fare and ride types. + * Apply Orca and Skagit senior discount fares based on the fare and ride types. */ - private float getSeniorFare(Fare.FareType fareType, RideType rideType, float defaultFare) { + private float getSeniorFare(Fare.FareType fareType, RideType rideType, float defaultFare, Route route) { switch (rideType) { case COMM_TRANS_LOCAL_SWIFT: return 1.25f; case COMM_TRANS_COMMUTER_EXPRESS: return 2.00f; @@ -181,35 +235,65 @@ private float getSeniorFare(Fare.FareType fareType, RideType rideType, float def case KITSAP_TRANSIT: // Pierce, Seattle Streetcar, and Kitsap only provide discounted senior fare for orca. return fareType.equals(Fare.FareType.electronicSenior) ? 1.00f : defaultFare; + case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND: + // Kitsap only provide discounted senior fare for orca. + return fareType.equals(Fare.FareType.electronicSenior) ? 1.00f : 2.00f; case KC_WATER_TAXI_VASHON_ISLAND: return 3.00f; case KC_WATER_TAXI_WEST_SEATTLE: return 2.50f; case KC_METRO: case SOUND_TRANSIT: return 1.00f; + case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND: + return fareType.equals(Fare.FareType.electronicSenior) ? 5.00f : 10.00f; + case SKAGIT_TRANSIT: + // Discount specific to Skagit transit and not Orca. + return 0.50f; + case WASHINGTON_STATE_FERRIES: + return getWashingtonStateFerriesFare(route.getLongName(), fareType, defaultFare); default: return defaultFare; } } /** - * Apply Orca youth discount fares based on the ride type. + * Apply Orca and Skagit youth discount fares based on the ride type. */ - private float getYouthFare(RideType rideType, float defaultFare) { + private float getYouthFare(Fare.FareType fareType, RideType rideType, float defaultFare, Route route) { switch (rideType) { case COMM_TRANS_LOCAL_SWIFT: return 1.75f; case COMM_TRANS_COMMUTER_EXPRESS: return 3.00f; + case KITSAP_TRANSIT: + case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND: + return fareType.equals(Fare.FareType.electronicYouth) ? 1.00f : 2.00f; case PIERCE_COUNTY_TRANSIT: return 1.00f; case KC_WATER_TAXI_VASHON_ISLAND: return 4.50f; case KC_WATER_TAXI_WEST_SEATTLE: return 3.75f; - case KITSAP_TRANSIT: return 2.00f; case KC_METRO: case SOUND_TRANSIT: case EVERETT_TRANSIT: - case SEATTLE_STREET_CAR: - return 1.50f; + case SEATTLE_STREET_CAR: return 1.50f; + case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND: + return fareType.equals(Fare.FareType.electronicYouth) ? 5.00f : 10.00f; + case SKAGIT_TRANSIT: + // Discount specific to Skagit transit and not Orca. + return 0.50f; + case WASHINGTON_STATE_FERRIES: + return getWashingtonStateFerriesFare(route.getLongName(), fareType, defaultFare); default: return defaultFare; } } + /** + * Get the washington state ferries fare matching the route long name and fare type. If no match is found, return + * the default fare. + */ + private float getWashingtonStateFerriesFare(String routeLongName, Fare.FareType fareType, float defaultFare) { + if (routeLongName == null || routeLongName.isEmpty()) { + return defaultFare; + } + Float fare = washingtonStateFerriesFares.get(routeLongName).get(fareType); + return (fare != null) ? fare : defaultFare; + } + /** * Get the ride price for a single leg. If testing, this class is being called directly so the required agency cash * values are not available therefore the default test price is used instead. @@ -241,26 +325,31 @@ public boolean populateFare(Fare fare, float cost = 0; float orcaFareDiscount = 0; for (Ride ride : rides) { - RideType rideType = classify(ride.routeData); - if (freeTransferStartTime == null && permitsFreeTransfers(rideType)) { + RideType rideType = classify(ride.routeData, ride.trip.getId()); + boolean ridePermitsFreeTransfers = permitsFreeTransfers(rideType); + if (freeTransferStartTime == null && ridePermitsFreeTransfers) { // The start of a free transfer must be with a transit agency that permits it! freeTransferStartTime = ride.startTime; } float singleLegPrice = getRidePrice(ride, fareType, fareRules); - float discountedFare = getDiscountedFare(fareType, rideType, singleLegPrice); + float discountedFare = getDiscountedFare(fareType, rideType, singleLegPrice, ride.routeData); if (hasFreeTransfers(fareType, rideType) && inFreeTransferWindow(freeTransferStartTime, ride.startTime)) { // If using Orca (free transfers), the total fare should be equivalent to the // most expensive leg of the journey. orcaFareDiscount = Float.max(orcaFareDiscount, discountedFare); + } else if (hasAgencyDiscount(rideType)) { + // If not using Orca and an agency discount is available, add this to the overall cost. + cost += discountedFare; } else { - // If free transfers are not permitted, add the cash price of this leg to the total cost. + // If free transfers are not permitted and there are no agency specific discounts, add the cash price of + // this leg to the total cost. cost += singleLegPrice; } if (!inFreeTransferWindow(freeTransferStartTime, ride.startTime)) { // If the trip time has exceeded the free transfer time limit of two hours the rider is required to // purchase a new fare. This also resets the free transfer trip window, applies the orca discount to the // overall cost and then resets the orca fare discount ready for the next transfer window. - freeTransferStartTime = null; + freeTransferStartTime = ridePermitsFreeTransfers ? ride.startTime : null; cost += orcaFareDiscount; orcaFareDiscount = 0; } @@ -287,6 +376,15 @@ private boolean hasFreeTransfers(Fare.FareType fareType, RideType rideType) { return permitsFreeTransfers(rideType) && usesOrca(fareType); } + /** + * Has agency specific discount. This is separate to Orca. + */ + private boolean hasAgencyDiscount(RideType rideType) { + return rideType == RideType.WASHINGTON_STATE_FERRIES || + rideType == RideType.KITSAP_TRANSIT_FAST_FERRY_WESTBOUND || + rideType == RideType.KITSAP_TRANSIT_FAST_FERRY_EASTBOUND ; + } + /** * All transit agencies permit free transfers, apart from these. */ diff --git a/src/main/java/org/opentripplanner/routing/impl/Ride.java b/src/main/java/org/opentripplanner/routing/impl/Ride.java index 78e46629a68..70d2d8bedf8 100644 --- a/src/main/java/org/opentripplanner/routing/impl/Ride.java +++ b/src/main/java/org/opentripplanner/routing/impl/Ride.java @@ -18,7 +18,7 @@ public class Ride { public Route routeData; - FeedScopedId trip; + public FeedScopedId trip; Set zones; diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index a23ca77aac4..7a716843192 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -1,6 +1,5 @@ package org.opentripplanner.routing.fares; -import com.google.common.collect.Lists; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -12,6 +11,8 @@ import org.opentripplanner.routing.impl.OrcaFareServiceImpl; import org.opentripplanner.routing.impl.Ride; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -20,7 +21,6 @@ import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.KITSAP_TRANSIT_AGENCY_ID; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.WASHINGTON_STATE_FERRIES_AGENCY_ID; -//TODO: Add additional test routes once defined by Jon. public class OrcaFareServiceTest { private static OrcaFareServiceImpl orcaFareService; @@ -54,8 +54,9 @@ private static void calculateFare(List rides, */ @Test public void calculateFareForSingleAgency() { - Ride ride = getRide(COMM_TRANS_AGENCY_ID, "400", 0); - List rides = Lists.newArrayList(ride); + List rides = Collections.singletonList( + getRide(COMM_TRANS_AGENCY_ID, "400", 0) + ); calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS); calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS); calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS); @@ -66,22 +67,23 @@ public void calculateFareForSingleAgency() { } /** - * WSF do not except free transfers. This test is to make sure the rider is charged the cash price for WSF as well + * WSF do not accept free transfers. This test is to make sure the rider is charged the cash price for WSF as well * as the highest fare where Orca can be used. */ @Test public void calculateFareWithNoFreeTransfer() { - List rides = Lists.newArrayList(); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 0)); - rides.add(getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 1)); - rides.add(getRide(COMM_TRANS_AGENCY_ID, 2)); + List rides = Arrays.asList( + getRide(KITSAP_TRANSIT_AGENCY_ID, 0), + getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 1), + getRide(COMM_TRANS_AGENCY_ID, 2) + ); calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 3); calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3); calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 3); calculateFare(rides, Fare.FareType.electronicSpecial, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f); calculateFare(rides, Fare.FareType.electronicRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS); calculateFare(rides, Fare.FareType.electronicSenior, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f); - calculateFare(rides, Fare.FareType.electronicYouth, 200f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f); + calculateFare(rides, Fare.FareType.electronicYouth, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 175f); } /** @@ -92,14 +94,14 @@ public void calculateFareWithNoFreeTransfer() { */ @Test public void calculateFareThatExceedsTwoHourFreeTransferWindow() { - List rides = Lists.newArrayList(); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 0)); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 30)); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 60)); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 90)); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 120)); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 150)); - + List rides = Arrays.asList( + getRide(KITSAP_TRANSIT_AGENCY_ID, 0), + getRide(KITSAP_TRANSIT_AGENCY_ID, 30), + getRide(KITSAP_TRANSIT_AGENCY_ID, 60), + getRide(KITSAP_TRANSIT_AGENCY_ID, 90), + getRide(KITSAP_TRANSIT_AGENCY_ID, 120), + getRide(KITSAP_TRANSIT_AGENCY_ID, 150) + ); calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 6); calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 6); calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 6); @@ -107,7 +109,7 @@ public void calculateFareThatExceedsTwoHourFreeTransferWindow() { calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS); calculateFare(rides, Fare.FareType.electronicSenior, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 100f); - calculateFare(rides, Fare.FareType.electronicYouth, 200f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 200f); + calculateFare(rides, Fare.FareType.electronicYouth, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 100f); } /** @@ -116,14 +118,14 @@ public void calculateFareThatExceedsTwoHourFreeTransferWindow() { */ @Test public void calculateFareThatStartsWithACashFare() { - List rides = Lists.newArrayList(); - rides.add(getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 0)); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 30)); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 60)); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 90)); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 120)); - rides.add(getRide(KITSAP_TRANSIT_AGENCY_ID, 149)); - + List rides = Arrays.asList( + getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 0), + getRide(KITSAP_TRANSIT_AGENCY_ID, 30), + getRide(KITSAP_TRANSIT_AGENCY_ID, 60), + getRide(KITSAP_TRANSIT_AGENCY_ID, 90), + getRide(KITSAP_TRANSIT_AGENCY_ID, 120), + getRide(KITSAP_TRANSIT_AGENCY_ID, 149) + ); calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 6); calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 6); calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 6); @@ -131,32 +133,89 @@ public void calculateFareThatStartsWithACashFare() { calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f + 0f); calculateFare(rides, Fare.FareType.electronicSenior, DEFAULT_RIDE_PRICE_IN_CENTS + 100f + 0f + 0f + 0f + 0f); - calculateFare(rides, Fare.FareType.electronicYouth, DEFAULT_RIDE_PRICE_IN_CENTS + 200f + 0f + 0f + 0f + 0f); + calculateFare(rides, Fare.FareType.electronicYouth, DEFAULT_RIDE_PRICE_IN_CENTS + 100f + 0f + 0f + 0f + 0f); + } + + /** + * Single trip with Kitsap transit fast ferry east to confirm correct non Orca fares are applied. + */ + @Test + public void calculateFareForKitsapFastFerryEastAgency() { + List rides = Collections.singletonList( + getRide(KITSAP_TRANSIT_AGENCY_ID, 0, 4, "Kitsap Fast Ferry", "east") + ); + calculateFare(rides, Fare.FareType.regular, 200f); + calculateFare(rides, Fare.FareType.senior, 200f); + calculateFare(rides, Fare.FareType.youth, 200f); + calculateFare(rides, Fare.FareType.electronicSpecial, DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.electronicRegular, 200f); + calculateFare(rides, Fare.FareType.electronicSenior, 100f); + calculateFare(rides, Fare.FareType.electronicYouth, 100f); + } + + /** + * Single trip (Point Defiance - Tahlequah) with WSF transit to confirm correct non Orca fares are applied. + */ + @Test + public void calculateFareForWSFPtToTahlequah() { + List rides = Collections.singletonList( + getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 0, "Point Defiance - Tahlequah") + ); + calculateFare(rides, Fare.FareType.regular, 595f); + calculateFare(rides, Fare.FareType.senior, 295f); + calculateFare(rides, Fare.FareType.youth, 295f); + calculateFare(rides, Fare.FareType.electronicSpecial, DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.electronicSenior, DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.electronicYouth, DEFAULT_RIDE_PRICE_IN_CENTS); } private static Ride getRide(String agencyId, long startTimeMins) { - return getRide(agencyId, "-1", -1, null, startTimeMins); + return createRide(agencyId, "-1", -1, null, startTimeMins, "", "", ""); + } + + private static Ride getRide(String agencyId, long startTimeMins, String routeLongName) { + return createRide(agencyId, "-1", -1, null, startTimeMins, "", "", routeLongName); + } + + private static Ride getRide(String agencyId, long startTimeMins, int rideType, String routeId, String tripId) { + return createRide(agencyId, "-1", rideType, null, startTimeMins, routeId, tripId, ""); } private static Ride getRide(String agencyId, String shortName, long startTimeMins) { - return getRide(agencyId, shortName, -1, null,startTimeMins); + return createRide(agencyId, shortName, -1, null,startTimeMins, "", "", ""); } /** * Create a {@link Ride} containing route data that will be used by {@link OrcaFareServiceImpl} to determine the * correct ride type. */ - private static Ride getRide(String agencyId, String shortName, int rideType, String desc, long startTimeMins) { + private static Ride createRide(String agencyId, + String shortName, + int rideType, + String desc, + long startTimeMins, + String routeId, + String tripId, + String routeLongName + ) { Ride ride = new Ride(); Agency agency = new Agency(); agency.setId(agencyId); Route route = new Route(); + FeedScopedId routeFeedScopeId = new FeedScopedId(); + routeFeedScopeId.setId(routeId); + route.setId(routeFeedScopeId); route.setAgency(agency); route.setShortName(shortName); route.setType(rideType); route.setDesc(desc); + route.setLongName(routeLongName); ride.routeData = route; ride.startTime = startTimeMins * 60; + FeedScopedId tripFeedScopeId = new FeedScopedId(); + tripFeedScopeId.setId(tripId); + ride.trip = tripFeedScopeId; return ride; } } From af773dfb83cc4115897f53dad319bd9c83aad6ea Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 29 Jul 2021 10:03:25 +0100 Subject: [PATCH 15/17] refactor(Update to accommodate agencies default fares): Updated logic to simplify applying agencies --- .../routing/impl/OrcaFareServiceImpl.java | 87 ++++++++++++------- .../routing/fares/OrcaFareServiceTest.java | 12 +-- 2 files changed, 61 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index c5fac2b8eb6..c0e03e26fbc 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -104,32 +104,61 @@ public OrcaFareServiceImpl(Collection regularFareRules) { classificationStrategy.put(WASHINGTON_STATE_FERRIES_AGENCY_ID, routeData -> RideType.WASHINGTON_STATE_FERRIES); classificationStrategy.put(KITSAP_TRANSIT_AGENCY_ID, routeData -> RideType.KITSAP_TRANSIT); + // Spaces have been removed from the route name because of inconsistencies in the WSF GTFS route dataset. washingtonStateFerriesFares.put( - "Seattle - Bainbridge Island", + "Seattle-BainbridgeIsland", ImmutableMap.of(Fare.FareType.regular, 9.05f, Fare.FareType.youth, 4.50f, Fare.FareType.senior, 4.50f) ); washingtonStateFerriesFares.put( - "Seattle - Bremerton", + "Seattle-Bremerton", ImmutableMap.of(Fare.FareType.regular, 9.05f, Fare.FareType.youth, 4.50f, Fare.FareType.senior, 4.50f) ); washingtonStateFerriesFares.put( - "Mukilteo - Clinton", + "Mukilteo-Clinton", ImmutableMap.of(Fare.FareType.regular, 5.55f, Fare.FareType.youth, 2.75f, Fare.FareType.senior, 2.75f) ); washingtonStateFerriesFares.put( - "Fauntleroy - Vashon Island", + "Fauntleroy-VashonIsland", ImmutableMap.of(Fare.FareType.regular, 5.95f, Fare.FareType.youth, 2.95f, Fare.FareType.senior, 2.95f) ); washingtonStateFerriesFares.put( - "Fauntleroy - Southworth", + "Fauntleroy-Southworth", ImmutableMap.of(Fare.FareType.regular, 7.10f, Fare.FareType.youth, 3.55f, Fare.FareType.senior, 3.55f) ); washingtonStateFerriesFares.put( - "Edmonds - Kingston", + "Edmonds-Kingston", ImmutableMap.of(Fare.FareType.regular, 9.05f, Fare.FareType.youth, 4.50f, Fare.FareType.senior, 4.50f) ); washingtonStateFerriesFares.put( - "Point Defiance - Tahlequah", + "PointDefiance-Tahlequah", + ImmutableMap.of(Fare.FareType.regular, 5.95f, Fare.FareType.youth, 2.95f, Fare.FareType.senior, 2.95f) + ); + washingtonStateFerriesFares.put( + "Anacortes-FridayHarbor", + ImmutableMap.of(Fare.FareType.regular, 14.50f, Fare.FareType.youth, 7.25f, Fare.FareType.senior, 7.25f) + ); + washingtonStateFerriesFares.put( + "Anacortes-LopezIsland", + ImmutableMap.of(Fare.FareType.regular, 14.50f, Fare.FareType.youth, 7.25f, Fare.FareType.senior, 7.25f) + ); + washingtonStateFerriesFares.put( + "Anacortes-OrcasIsland", + ImmutableMap.of(Fare.FareType.regular, 14.50f, Fare.FareType.youth, 7.25f, Fare.FareType.senior, 7.25f) + ); + washingtonStateFerriesFares.put( + "Anacortes-ShawIsland", + ImmutableMap.of(Fare.FareType.regular, 14.50f, Fare.FareType.youth, 7.25f, Fare.FareType.senior, 7.25f) + ); + washingtonStateFerriesFares.put( + "Coupeville-PortTownsend", + ImmutableMap.of(Fare.FareType.regular, 3.80f, Fare.FareType.youth, 1.80f, Fare.FareType.senior, 1.80f) + ); + washingtonStateFerriesFares.put( + "PortTownsend-Coupeville", + ImmutableMap.of(Fare.FareType.regular, 3.80f, Fare.FareType.youth, 1.80f, Fare.FareType.senior, 1.80f) + ); + washingtonStateFerriesFares.put( + "Southworth-VashonIsland", ImmutableMap.of(Fare.FareType.regular, 5.95f, Fare.FareType.youth, 2.95f, Fare.FareType.senior, 2.95f) ); } @@ -164,7 +193,7 @@ private static RideType classify(Route routeData, String tripId) { * Define which discount fare should be applied based on the fare type. If the ride type is unknown the discount * fare can not be applied, use the default fare. */ - private float getDiscountedFare(Fare.FareType fareType, RideType rideType, float defaultFare, Route route) { + private float getLegFare(Fare.FareType fareType, RideType rideType, float defaultFare, Route route) { if (rideType == null) { return defaultFare; } @@ -186,7 +215,7 @@ private float getDiscountedFare(Fare.FareType fareType, RideType rideType, float } /** - * Apply Orca regular discount fares. If the ride type cannot be matched the default fare is used. + * Apply regular discount fares. If the ride type cannot be matched the default fare is used. */ private float getRegularFare(Fare.FareType fareType, RideType rideType, float defaultFare, Route route) { switch (rideType) { @@ -222,7 +251,7 @@ private float getLiftFare(RideType rideType, float defaultFare) { } /** - * Apply Orca and Skagit senior discount fares based on the fare and ride types. + * Apply senior discount fares based on the fare and ride types. */ private float getSeniorFare(Fare.FareType fareType, RideType rideType, float defaultFare, Route route) { switch (rideType) { @@ -255,7 +284,7 @@ private float getSeniorFare(Fare.FareType fareType, RideType rideType, float def } /** - * Apply Orca and Skagit youth discount fares based on the ride type. + * Apply youth discount fares based on the ride type. */ private float getYouthFare(Fare.FareType fareType, RideType rideType, float defaultFare, Route route) { switch (rideType) { @@ -263,6 +292,7 @@ private float getYouthFare(Fare.FareType fareType, RideType rideType, float defa case COMM_TRANS_COMMUTER_EXPRESS: return 3.00f; case KITSAP_TRANSIT: case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND: + // Discount specific to Kitsap transit. return fareType.equals(Fare.FareType.electronicYouth) ? 1.00f : 2.00f; case PIERCE_COUNTY_TRANSIT: return 1.00f; case KC_WATER_TAXI_VASHON_ISLAND: return 4.50f; @@ -272,11 +302,13 @@ private float getYouthFare(Fare.FareType fareType, RideType rideType, float defa case EVERETT_TRANSIT: case SEATTLE_STREET_CAR: return 1.50f; case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND: + // Discount specific to Kitsap transit. return fareType.equals(Fare.FareType.electronicYouth) ? 5.00f : 10.00f; case SKAGIT_TRANSIT: - // Discount specific to Skagit transit and not Orca. + // Discount specific to Skagit transit. return 0.50f; case WASHINGTON_STATE_FERRIES: + // Discount specific to WSF. return getWashingtonStateFerriesFare(route.getLongName(), fareType, defaultFare); default: return defaultFare; } @@ -290,7 +322,7 @@ private float getWashingtonStateFerriesFare(String routeLongName, Fare.FareType if (routeLongName == null || routeLongName.isEmpty()) { return defaultFare; } - Float fare = washingtonStateFerriesFares.get(routeLongName).get(fareType); + Float fare = washingtonStateFerriesFares.get(routeLongName.replaceAll(" ", "")).get(fareType); return (fare != null) ? fare : defaultFare; } @@ -332,20 +364,20 @@ public boolean populateFare(Fare fare, freeTransferStartTime = ride.startTime; } float singleLegPrice = getRidePrice(ride, fareType, fareRules); - float discountedFare = getDiscountedFare(fareType, rideType, singleLegPrice, ride.routeData); - if (hasFreeTransfers(fareType, rideType) && inFreeTransferWindow(freeTransferStartTime, ride.startTime)) { + float legFare = getLegFare(fareType, rideType, singleLegPrice, ride.routeData); + boolean inFreeTransferWindow = inFreeTransferWindow(freeTransferStartTime, ride.startTime); + if (hasFreeTransfers(fareType, rideType) && inFreeTransferWindow) { // If using Orca (free transfers), the total fare should be equivalent to the // most expensive leg of the journey. - orcaFareDiscount = Float.max(orcaFareDiscount, discountedFare); - } else if (hasAgencyDiscount(rideType)) { - // If not using Orca and an agency discount is available, add this to the overall cost. - cost += discountedFare; - } else { - // If free transfers are not permitted and there are no agency specific discounts, add the cash price of - // this leg to the total cost. + orcaFareDiscount = Float.max(orcaFareDiscount, legFare); + } else if (usesOrca(fareType) && !inFreeTransferWindow) { + // If using Orca and outside of the free transfer window, add the single leg cash price. cost += singleLegPrice; + } else { + // If not using Orca, add the agencies default price for this leg. + cost += legFare; } - if (!inFreeTransferWindow(freeTransferStartTime, ride.startTime)) { + if (!inFreeTransferWindow) { // If the trip time has exceeded the free transfer time limit of two hours the rider is required to // purchase a new fare. This also resets the free transfer trip window, applies the orca discount to the // overall cost and then resets the orca fare discount ready for the next transfer window. @@ -376,15 +408,6 @@ private boolean hasFreeTransfers(Fare.FareType fareType, RideType rideType) { return permitsFreeTransfers(rideType) && usesOrca(fareType); } - /** - * Has agency specific discount. This is separate to Orca. - */ - private boolean hasAgencyDiscount(RideType rideType) { - return rideType == RideType.WASHINGTON_STATE_FERRIES || - rideType == RideType.KITSAP_TRANSIT_FAST_FERRY_WESTBOUND || - rideType == RideType.KITSAP_TRANSIT_FAST_FERRY_EASTBOUND ; - } - /** * All transit agencies permit free transfers, apart from these. */ diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index 7a716843192..21fce9a86fb 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -58,8 +58,8 @@ public void calculateFareForSingleAgency() { getRide(COMM_TRANS_AGENCY_ID, "400", 0) ); calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS); - calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS); - calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.senior, 200f); + calculateFare(rides, Fare.FareType.youth, 300f); calculateFare(rides, Fare.FareType.electronicSpecial, 200f); calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS); calculateFare(rides, Fare.FareType.electronicSenior, 200f); @@ -78,8 +78,8 @@ public void calculateFareWithNoFreeTransfer() { getRide(COMM_TRANS_AGENCY_ID, 2) ); calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 3); - calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3); - calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 3); + calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS + 125); + calculateFare(rides, Fare.FareType.youth, 200f + DEFAULT_RIDE_PRICE_IN_CENTS + 175f); calculateFare(rides, Fare.FareType.electronicSpecial, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f); calculateFare(rides, Fare.FareType.electronicRegular, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS); calculateFare(rides, Fare.FareType.electronicSenior, 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 125f); @@ -104,7 +104,7 @@ public void calculateFareThatExceedsTwoHourFreeTransferWindow() { ); calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 6); calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 6); - calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 6); + calculateFare(rides, Fare.FareType.youth, 200f + 200f + 200f + 200f + 200f + 200f); calculateFare(rides, Fare.FareType.electronicSpecial, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 100f); calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS); @@ -128,7 +128,7 @@ public void calculateFareThatStartsWithACashFare() { ); calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 6); calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 6); - calculateFare(rides, Fare.FareType.youth, DEFAULT_RIDE_PRICE_IN_CENTS * 6); + calculateFare(rides, Fare.FareType.youth, 349f + 200f + 200f + 200f + 200f + 200f); calculateFare(rides, Fare.FareType.electronicSpecial, DEFAULT_RIDE_PRICE_IN_CENTS + 100f + 0f + 0f + 0f + 0f); calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f + 0f); From 3135363569eebd5c14abfbf11c771dc27f98ba5e Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 30 Jul 2021 10:10:09 +0100 Subject: [PATCH 16/17] Update src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java Co-authored-by: Evan Siroky --- .../routing/impl/OrcaFareServiceImpl.java | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index c0e03e26fbc..42884504965 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -370,21 +370,35 @@ public boolean populateFare(Fare fare, // If using Orca (free transfers), the total fare should be equivalent to the // most expensive leg of the journey. orcaFareDiscount = Float.max(orcaFareDiscount, legFare); - } else if (usesOrca(fareType) && !inFreeTransferWindow) { - // If using Orca and outside of the free transfer window, add the single leg cash price. - cost += singleLegPrice; + } else if (usesOrca(fareType) && !inFreeTransferWindow) { + // If using Orca and outside of the free transfer window, add the cumulative Orca fare (the maximum leg + // fare encountered within the free transfer window). + cost += orcaFareDiscount; + + // Reset the free transfer start time and next Orca fare as needed. + if (ridePermitsFreeTransfers) { + // The leg is using a ride type that permits free transfers. + // The next free transfer window begins at the start time of this leg. + freeTransferStartTime = ride.startTime; + // Reset the Orca fare to be the fare of this leg. + orcaFareDiscount = legFare; + } else { + // The leg is not using a ride type that permits free transfers. + // Since there are no free transfers for this leg, increase the total cost by the fare for this leg. + cost += legFare; + // The current free transfer window has expired and won't start again until another leg is + // encountered that does have free transfers. + freeTransferStartTime = null; + // The previous Orca fare has been applied to the total cost. Also, the non-free transfer cost has + // also been applied to the total cost. Therefore, the next Orca cost for the next free-transfer + // window needs to be reset to 0 so that it is not applied after looping through all rides. + orcaFareDiscount = 0; + } } else { // If not using Orca, add the agencies default price for this leg. cost += legFare; } - if (!inFreeTransferWindow) { - // If the trip time has exceeded the free transfer time limit of two hours the rider is required to - // purchase a new fare. This also resets the free transfer trip window, applies the orca discount to the - // overall cost and then resets the orca fare discount ready for the next transfer window. - freeTransferStartTime = ridePermitsFreeTransfers ? ride.startTime : null; - cost += orcaFareDiscount; - orcaFareDiscount = 0; - } + } } cost += orcaFareDiscount; if (cost < Float.POSITIVE_INFINITY) { From 2939e3cbc27e4ef61b523684705e4bd2519bae7e Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 30 Jul 2021 14:16:33 +0100 Subject: [PATCH 17/17] refactor(Additional tests to confirm correct fare calculation): Tests to make sure the Ocra and agen --- .../routing/impl/OrcaFareServiceImpl.java | 1 - .../routing/fares/OrcaFareServiceTest.java | 69 +++++++++++++++++-- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java index 42884504965..dbf5e3a89c5 100644 --- a/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/OrcaFareServiceImpl.java @@ -399,7 +399,6 @@ public boolean populateFare(Fare fare, cost += legFare; } } - } cost += orcaFareDiscount; if (cost < Float.POSITIVE_INFINITY) { fare.addFare(fareType, getMoney(currency, cost)); diff --git a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java index 21fce9a86fb..312378c2b31 100644 --- a/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/fares/OrcaFareServiceTest.java @@ -19,6 +19,7 @@ import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.COMM_TRANS_AGENCY_ID; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.KITSAP_TRANSIT_AGENCY_ID; +import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.SKAGIT_TRANSIT_AGENCY_ID; import static org.opentripplanner.routing.impl.OrcaFareServiceImpl.WASHINGTON_STATE_FERRIES_AGENCY_ID; public class OrcaFareServiceTest { @@ -88,9 +89,9 @@ public void calculateFareWithNoFreeTransfer() { /** * Total trip time is 2h 30m. The first four transfers are within the permitted two hour window. A single (highest) - * Orca fare will be charged for these transfers. The fifth transfer is outside of the two hour window and will be - * charged a cash rate. At this point, the two hour window will start again so the final transfer will be - * charged at a discount rate... if using Orca. + * Orca fare will be charged for these transfers. The fifth transfer is outside of the original two hour window so + * a single Orca fare for this leg is applied and the two hour window will start again. The final transfer is within + * the new two hour window and will be free. */ @Test public void calculateFareThatExceedsTwoHourFreeTransferWindow() { @@ -105,11 +106,65 @@ public void calculateFareThatExceedsTwoHourFreeTransferWindow() { calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 6); calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 6); calculateFare(rides, Fare.FareType.youth, 200f + 200f + 200f + 200f + 200f + 200f); - calculateFare(rides, Fare.FareType.electronicSpecial, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 100f); + calculateFare(rides, Fare.FareType.electronicSpecial, 100f + 0f + 0f + 0f + 100f + 0f); calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f + 0f + - DEFAULT_RIDE_PRICE_IN_CENTS + DEFAULT_RIDE_PRICE_IN_CENTS); - calculateFare(rides, Fare.FareType.electronicSenior, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 100f); - calculateFare(rides, Fare.FareType.electronicYouth, 100f + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS + 100f); + DEFAULT_RIDE_PRICE_IN_CENTS + 0f); + calculateFare(rides, Fare.FareType.electronicSenior, 100f + 0f + 0f + 0f + 100f + 0f); + calculateFare(rides, Fare.FareType.electronicYouth, 100f + 0f + 0f + 0f + 100f + 0f); + } + /** + * Total trip time is 2h 30m. Calculate fare with two free transfer windows which include agencies which do not permit + * free transfers. The free transfers will be applied for Kitsap, but not for WSF nor Skagit. Note: Not a real world + * trip! + */ + @Test + public void calculateFareThatIncludesNoFreeTransfers() { + List rides = Arrays.asList( + getRide(KITSAP_TRANSIT_AGENCY_ID, 0), + getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 30), + getRide(KITSAP_TRANSIT_AGENCY_ID, 60), + getRide(SKAGIT_TRANSIT_AGENCY_ID, 90), + getRide(KITSAP_TRANSIT_AGENCY_ID, 120), + getRide(WASHINGTON_STATE_FERRIES_AGENCY_ID, 150, "Fauntleroy-VashonIsland") + ); + calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 5 + 595f); + calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 3 + 50f + + DEFAULT_RIDE_PRICE_IN_CENTS + 295f); + calculateFare(rides, Fare.FareType.youth, 200f + DEFAULT_RIDE_PRICE_IN_CENTS + 200f + 50f + 200f + 295f); + calculateFare(rides, Fare.FareType.electronicSpecial, 100f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f + + DEFAULT_RIDE_PRICE_IN_CENTS + 100f + DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS * 2 + 0f + + DEFAULT_RIDE_PRICE_IN_CENTS * 3); + calculateFare(rides, Fare.FareType.electronicSenior, 100f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f + + 50f + 100f + DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.electronicYouth, 100f + DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 50f + 100f + DEFAULT_RIDE_PRICE_IN_CENTS); + } + + /** + * Total trip time is 4h 30m. This is equivalent to three transfer windows and therefore three Orca fare charges. + */ + @Test + public void calculateFareThatExceedsTwoHourFreeTransferWindowTwice() { + List rides = Arrays.asList( + getRide(KITSAP_TRANSIT_AGENCY_ID, 0), + getRide(KITSAP_TRANSIT_AGENCY_ID, 30), + getRide(KITSAP_TRANSIT_AGENCY_ID, 60), + getRide(KITSAP_TRANSIT_AGENCY_ID, 90), + getRide(KITSAP_TRANSIT_AGENCY_ID, 120), + getRide(KITSAP_TRANSIT_AGENCY_ID, 150), + getRide(KITSAP_TRANSIT_AGENCY_ID, 180), + getRide(KITSAP_TRANSIT_AGENCY_ID, 210), + getRide(KITSAP_TRANSIT_AGENCY_ID, 240), + getRide(KITSAP_TRANSIT_AGENCY_ID, 270) + ); + calculateFare(rides, Fare.FareType.regular, DEFAULT_RIDE_PRICE_IN_CENTS * 10); + calculateFare(rides, Fare.FareType.senior, DEFAULT_RIDE_PRICE_IN_CENTS * 10); + calculateFare(rides, Fare.FareType.youth, 200f + 200f + 200f + 200f + 200f + 200f + 200f + 200f + 200f + 200f); + calculateFare(rides, Fare.FareType.electronicSpecial, 100f + 0f + 0f + 0f + 100f + 0f + 0f + 0f + 100f + 0f); + calculateFare(rides, Fare.FareType.electronicRegular, DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f + 0f + + DEFAULT_RIDE_PRICE_IN_CENTS + 0f + 0f + 0f + DEFAULT_RIDE_PRICE_IN_CENTS); + calculateFare(rides, Fare.FareType.electronicSenior, 100f + 0f + 0f + 0f + 100f + 0f + 0f + 0f + 100f + 0f); + calculateFare(rides, Fare.FareType.electronicYouth, 100f + 0f + 0f + 0f + 100f + 0f + 0f + 0f + 100f + 0f); } /**