Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove fare service from Graph #6292

Draft
wants to merge 8 commits into
base: dev-2.x
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public void testBasic() {

var feedId = timetableRepository.getFeedIds().iterator().next();

var serverContext = TestServerContext.createServerContext(graph, timetableRepository);
var serverContext = TestServerContext.createServerContext(graph, timetableRepository, model.fareServiceFactory().makeFareService());

var start = LocalDateTime
.of(2009, Month.AUGUST, 7, 12, 0, 0)
Expand All @@ -56,7 +56,7 @@ public void testPortland() {
TimetableRepository timetableRepository = model.timetableRepository();
var portlandId = timetableRepository.getFeedIds().iterator().next();

var serverContext = TestServerContext.createServerContext(graph, timetableRepository);
var serverContext = TestServerContext.createServerContext(graph, timetableRepository, model.fareServiceFactory().makeFareService());

// from zone 3 to zone 2
var from = GenericLocation.fromStopId(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.junit.jupiter.api.Test;
import org.opentripplanner.TestOtpModel;
import org.opentripplanner.TestServerContext;
import org.opentripplanner.ext.fares.impl.DefaultFareServiceFactory;
import org.opentripplanner.framework.application.OTPFeature;
import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
import org.opentripplanner.graph_builder.module.DirectTransferGenerator;
Expand All @@ -32,6 +33,7 @@
import org.opentripplanner.routing.api.request.RouteRequest;
import org.opentripplanner.routing.api.request.framework.TimeAndCostPenalty;
import org.opentripplanner.routing.api.request.request.filter.AllowAllTransitFilter;
import org.opentripplanner.routing.fares.FareServiceFactory;
import org.opentripplanner.routing.graph.Graph;
import org.opentripplanner.transit.service.TimetableRepository;

Expand All @@ -58,17 +60,17 @@ static void setup() {
TestOtpModel model = FlexIntegrationTestData.cobbOsm();
graph = model.graph();
timetableRepository = model.timetableRepository();

addGtfsToGraph(
graph,
timetableRepository,
List.of(
FlexIntegrationTestData.COBB_BUS_30_GTFS,
FlexIntegrationTestData.MARTA_BUS_856_GTFS,
FlexIntegrationTestData.COBB_FLEX_GTFS
)
),
new DefaultFareServiceFactory()
);
service = TestServerContext.createServerContext(graph, timetableRepository).routingService();
service = TestServerContext.createServerContext(graph, timetableRepository, model.fareServiceFactory().makeFareService()).routingService();
}

@Test
Expand Down Expand Up @@ -179,15 +181,18 @@ static void teardown() {
private static void addGtfsToGraph(
Graph graph,
TimetableRepository timetableRepository,
List<File> gtfsFiles
List<File> gtfsFiles,
FareServiceFactory fareServiceFactory
) {
// GTFS
var gtfsBundles = gtfsFiles.stream().map(GtfsBundle::new).toList();
GtfsModule gtfsModule = new GtfsModule(
gtfsBundles,
timetableRepository,
graph,
ServiceDateInterval.unbounded()
DataImportIssueStore.NOOP,
ServiceDateInterval.unbounded(),
fareServiceFactory
);
gtfsModule.buildGraph();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
import java.util.Map;
import org.opentripplanner.ConstantsForTests;
import org.opentripplanner.TestOtpModel;
import org.opentripplanner.ext.fares.impl.DefaultFareServiceFactory;
import org.opentripplanner.framework.application.OTPFeature;
import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
import org.opentripplanner.gtfs.graphbuilder.GtfsBundle;
import org.opentripplanner.gtfs.graphbuilder.GtfsModule;
import org.opentripplanner.model.calendar.ServiceDateInterval;
Expand Down Expand Up @@ -49,7 +51,9 @@ private static TestOtpModel buildFlexGraph(File file) {
List.of(gtfsBundle),
timetableRepository,
graph,
new ServiceDateInterval(LocalDate.of(2021, 1, 1), LocalDate.of(2022, 1, 1))
DataImportIssueStore.NOOP,
new ServiceDateInterval(LocalDate.of(2021, 1, 1), LocalDate.of(2022, 1, 1)),
new DefaultFareServiceFactory()
);
OTPFeature.enableFeatures(Map.of(OTPFeature.FlexRouting, true));
module.buildGraph();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,17 @@

import java.time.LocalDateTime;
import java.time.Month;
import java.time.OffsetDateTime;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.locationtech.jts.geom.Coordinate;
import org.opentripplanner.TestOtpModel;
import org.opentripplanner.TestServerContext;
import org.opentripplanner._support.time.ZoneIds;
import org.opentripplanner.ext.fares.DecorateWithFare;
import org.opentripplanner.ext.fares.impl.DefaultFareService;
import org.opentripplanner.ext.flex.FlexIntegrationTestData;
import org.opentripplanner.ext.flex.FlexParameters;
import org.opentripplanner.ext.flex.FlexRouter;
import org.opentripplanner.framework.application.OTPFeature;
import org.opentripplanner.framework.geometry.EncodedPolyline;
import org.opentripplanner.framework.i18n.I18NString;
import org.opentripplanner.graph_builder.module.ValidateAndInterpolateStopTimesForEachTrip;
import org.opentripplanner.model.GenericLocation;
import org.opentripplanner.model.StopTime;
Expand All @@ -34,15 +27,9 @@
import org.opentripplanner.routing.api.request.request.filter.AllowAllTransitFilter;
import org.opentripplanner.routing.framework.DebugTimingAggregator;
import org.opentripplanner.routing.graph.Graph;
import org.opentripplanner.routing.graphfinder.NearbyStop;
import org.opentripplanner.standalone.api.OtpServerRequestContext;
import org.opentripplanner.street.model.vertex.StreetLocation;
import org.opentripplanner.street.search.request.StreetSearchRequest;
import org.opentripplanner.street.search.state.State;
import org.opentripplanner.transit.model.framework.FeedScopedId;
import org.opentripplanner.transit.model.network.grouppriority.TransitGroupPriorityService;
import org.opentripplanner.transit.model.site.AreaStop;
import org.opentripplanner.transit.service.DefaultTransitService;
import org.opentripplanner.transit.service.TimetableRepository;
import org.opentripplanner.utils.time.ServiceDateUtils;

Expand Down Expand Up @@ -92,41 +79,6 @@ void parseCobbCountyAsScheduledDeviatedTrip() {
assertEquals(-84.63430143459385, flexZone.getLon(), delta);
}

@Test
void calculateDirectFare() {
OTPFeature.enableFeatures(Map.of(OTPFeature.FlexRouting, true));
var trip = getFlexTrip();

var from = getNearbyStop(trip, "from-stop");
var to = getNearbyStop(trip, "to-stop");

var router = new FlexRouter(
graph,
new DefaultTransitService(timetableRepository),
FlexParameters.defaultValues(),
OffsetDateTime.parse("2021-11-12T10:15:24-05:00").toInstant(),
null,
1,
1,
List.of(from),
List.of(to)
);

var filter = new DecorateWithFare(graph.getFareService());

var itineraries = router
.createFlexOnlyItineraries(false)
.stream()
.peek(filter::decorate)
.toList();

var itinerary = itineraries.getFirst();

assertFalse(itinerary.getFares().getLegProducts().isEmpty());

OTPFeature.enableFeatures(Map.of(OTPFeature.FlexRouting, false));
}

/**
* Trips which consist of flex and fixed-schedule stops should work in transit mode.
* <p>
Expand All @@ -137,7 +89,7 @@ void calculateDirectFare() {
void flexTripInTransitMode() {
var feedId = timetableRepository.getFeedIds().iterator().next();

var serverContext = TestServerContext.createServerContext(graph, timetableRepository);
var serverContext = TestServerContext.createServerContext(graph, timetableRepository, new DefaultFareService());

// from zone 3 to zone 2
var from = GenericLocation.fromStopId("Transfer Point for Route 30", feedId, "cujv");
Expand Down Expand Up @@ -223,27 +175,6 @@ private static List<Itinerary> getItineraries(
return result.getItineraries();
}

private static NearbyStop getNearbyStop(FlexTrip<?, ?> trip, String id) {
// getStops() returns a set of stops and the order doesn't correspond to the stop times
// of the trip
var stopLocation = trip
.getStops()
.stream()
.filter(s -> s instanceof AreaStop)
.findFirst()
.orElseThrow();

return new NearbyStop(
stopLocation,
0,
List.of(),
new State(
new StreetLocation(id, new Coordinate(0, 0), I18NString.of(id)),
StreetSearchRequest.of().build()
)
);
}

private static FlexTrip<?, ?> getFlexTrip() {
var feedId = timetableRepository.getFeedIds().iterator().next();
var tripId = new FeedScopedId(feedId, "a326c618-d42c-4bd1-9624-c314fbf8ecd8");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.opentripplanner.TestServerContext;
import org.opentripplanner.ext.fares.impl.DefaultFareService;
import org.opentripplanner.routing.graph.Graph;
import org.opentripplanner.test.support.HttpForTest;
import org.opentripplanner.transit.service.TimetableRepository;
Expand All @@ -17,7 +18,7 @@ void tileJson() {
// the Grizzly request is awful to instantiate, using Mockito
var grizzlyRequest = Mockito.mock(Request.class);
var resource = new VectorTilesResource(
TestServerContext.createServerContext(new Graph(), new TimetableRepository()),
TestServerContext.createServerContext(new Graph(), new TimetableRepository(), new DefaultFareService()),
grizzlyRequest,
"default"
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@

/**
* Computes the fares of an itinerary and adds them.
* <p>
* TODO: Convert to a class - exposing a service in a DTO is a risk.
*/
public record DecorateWithFare(FareService fareService) implements ItineraryDecorator {
public final class DecorateWithFare implements ItineraryDecorator {
private final FareService fareService;

public DecorateWithFare(FareService fareService) {
this.fareService = fareService;
}

@Override
public void decorate(Itinerary itinerary) {
var fare = fareService.calculateFares(itinerary);
Expand All @@ -18,4 +22,5 @@ public void decorate(Itinerary itinerary) {
FaresToItineraryMapper.addFaresToLegs(fare, itinerary);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public static GraphQLRequestContext ofServerContext(OtpServerRequestContext cont
return new GraphQLRequestContext(
context.routingService(),
context.transitService(),
context.graph().getFareService(),
context.fareService(),
context.vehicleRentalService(),
context.vehicleParkingService(),
context.realtimeVehicleService(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.onebusaway.gtfs.serialization.GtfsReader;
import org.onebusaway.gtfs.services.GenericMutableDao;
import org.onebusaway.gtfs.services.GtfsMutableRelationalDao;
import org.opentripplanner.ext.fares.impl.DefaultFareServiceFactory;
import org.opentripplanner.ext.flex.FlexTripsMapper;
import org.opentripplanner.framework.application.OTPFeature;
import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
Expand Down Expand Up @@ -98,22 +97,6 @@ public GtfsModule(
this.fareServiceFactory = fareServiceFactory;
}

public GtfsModule(
List<GtfsBundle> bundles,
TimetableRepository timetableRepository,
Graph graph,
ServiceDateInterval transitPeriodLimit
) {
this(
bundles,
timetableRepository,
graph,
DataImportIssueStore.NOOP,
transitPeriodLimit,
new DefaultFareServiceFactory()
);
}

@Override
public void buildGraph() {
CalendarServiceData calendarServiceData = new CalendarServiceData();
Expand Down Expand Up @@ -142,7 +125,7 @@ public void buildGraph() {
mapper.mapStopTripAndRouteDataIntoBuilder();

OtpTransitServiceBuilder builder = mapper.getBuilder();
var fareRulesService = mapper.getFareRulesService();
var fareRulesData = mapper.fareRulesData();

builder.limitServiceDays(transitPeriodLimit);

Expand Down Expand Up @@ -195,8 +178,7 @@ public void buildGraph() {
.run(otpTransitService.getTripPatterns());
}

fareServiceFactory.processGtfs(fareRulesService, otpTransitService);
graph.setFareService(fareServiceFactory.makeFareService());
fareServiceFactory.processGtfs(fareRulesData, otpTransitService);
}
} catch (IOException e) {
throw new RuntimeException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,11 @@ public OtpTransitServiceBuilder getBuilder() {
return builder;
}

public FareRulesData getFareRulesService() {
public FareRulesData fareRulesData() {
return fareRulesBuilder;
}

public void mapStopTripAndRouteDataIntoBuilder() {
var siteRepository = builder.siteRepository();
translationHelper.importTranslations(data.getAllTranslations(), data.getAllFeedInfos());

builder.getAgenciesById().addAll(agencyMapper.map(data.getAllAgencies()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public static ItineraryListFilterChain createFilterChain(
builder.withTransitGroupPriority();
}

var fareService = context.graph().getFareService();
var fareService = context.fareService();
if (fareService != null) {
builder.withFareDecorator(new DecorateWithFare(fareService));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import org.opentripplanner.model.plan.Itinerary;

/**
* Computes a fare for a given sequence of Rides. The FareService is serialized as part of the
* Computes a fare for a given sequence of legs. The FareService is serialized as part of the
* Graph; Hence it should be {@link Serializable}.
*/
public interface FareService extends Serializable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.opentripplanner.framework.geometry.CompactElevationProfile;
import org.opentripplanner.framework.geometry.GeometryUtils;
import org.opentripplanner.model.calendar.openinghours.OpeningHoursCalendarService;
import org.opentripplanner.routing.fares.FareService;
import org.opentripplanner.routing.graph.index.StreetIndex;
import org.opentripplanner.routing.linking.VertexLinker;
import org.opentripplanner.routing.services.notes.StreetNotesService;
Expand Down Expand Up @@ -110,8 +109,6 @@ public class Graph implements Serializable {
// static variable in CompactElevationProfile in SerializedGraphObject
private double distanceBetweenElevationSamples;

private FareService fareService;

/**
* Hack. I've tried three different ways of generating unique labels. Previously we were just
* tolerating edge label collisions. For some reason we're repeatedly generating splits on the
Expand Down Expand Up @@ -360,14 +357,6 @@ public void setDistanceBetweenElevationSamples(double distanceBetweenElevationSa
CompactElevationProfile.setDistanceBetweenSamplesM(distanceBetweenElevationSamples);
}

public FareService getFareService() {
return fareService;
}

public void setFareService(FareService fareService) {
this.fareService = fareService;
}

private void indexIfNotIndexed(SiteRepository siteRepository) {
if (streetIndex == null) {
index(siteRepository);
Expand Down
Loading
Loading