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

Make ScheduledTransitLeg, FlexibleTransitLeg fully immutable #6386

Merged
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
ef53fe0
When using copy builder, also copy alerts
leonardehrenfried Jan 8, 2025
ff4bd69
WIP
leonardehrenfried Jan 9, 2025
6085bef
Add more transit leg mapper
leonardehrenfried Jan 14, 2025
5f36f51
Rename method
leonardehrenfried Jan 14, 2025
e4914bc
Reformat file again
leonardehrenfried Jan 14, 2025
d84de49
Fix mapping of legs
leonardehrenfried Jan 14, 2025
0fa210c
Rename distanceMeters to overrideDistanceMeters
leonardehrenfried Jan 14, 2025
6085ea6
Update leg
leonardehrenfried Jan 14, 2025
08e3415
Make fare products immutable
leonardehrenfried Jan 14, 2025
721c991
Make methods static
leonardehrenfried Jan 14, 2025
2a7a2b1
Use TransitLeg in AlertMapper
leonardehrenfried Jan 14, 2025
a4f9b11
Add another test
leonardehrenfried Jan 14, 2025
069387d
Add and rename tests
leonardehrenfried Jan 14, 2025
64e5976
Add test for builder
leonardehrenfried Jan 15, 2025
a9fcf8c
Take into account the complexities of the ConsolidatedStopLeg
leonardehrenfried Jan 15, 2025
624af76
Fix time shift
leonardehrenfried Jan 15, 2025
6abeac6
Convert stopPosInPattern to int
leonardehrenfried Jan 15, 2025
2b315a1
Merge remote-tracking branch 'upstream/dev-2.x' into immutable-leg
leonardehrenfried Jan 16, 2025
f6fa9ca
Update application/src/ext-test/java/org/opentripplanner/ext/flex/Fle…
leonardehrenfried Jan 20, 2025
4871f0f
Use separate times in test
leonardehrenfried Jan 20, 2025
2d7c8bc
Remove duplicate transfer of alerts
leonardehrenfried Jan 20, 2025
3c68f37
Make collection handling more consistent
leonardehrenfried Jan 20, 2025
ec2d61f
Simplify ConsolidatedStopLeg
leonardehrenfried Jan 20, 2025
c206b82
Merge remote-tracking branch 'upstream/dev-2.x' into immutable-leg
leonardehrenfried Jan 20, 2025
aa84c15
Apply some review feedback
leonardehrenfried Jan 22, 2025
5b5937d
Apply review feedback
leonardehrenfried Jan 22, 2025
98fd66d
Apply more review feedback about handling alerts
leonardehrenfried Jan 23, 2025
ad6054f
Introduce FaresAware interface
leonardehrenfried Jan 23, 2025
1b8f317
Add assertion
leonardehrenfried Jan 23, 2025
5bf1072
Small clean up of FlexibleTransitLeg
leonardehrenfried Jan 23, 2025
05ff5ee
Extract computation of distance meters
leonardehrenfried Jan 23, 2025
f5b70c3
Round distance to 2 digits
leonardehrenfried Jan 23, 2025
9f6b523
Extract utils class for distance computation
leonardehrenfried Jan 24, 2025
e195caf
Apply suggestions from code review
leonardehrenfried Jan 28, 2025
cee79d5
Rename class
leonardehrenfried Jan 30, 2025
71cac1a
Apply review feedback
leonardehrenfried Jan 30, 2025
14f3509
Use Box instead of AtomicBoolean
leonardehrenfried Jan 30, 2025
b2ac81f
Revert order of i and i-1
leonardehrenfried Jan 30, 2025
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 @@ -17,6 +17,7 @@
import org.opentripplanner.framework.model.Grams;
import org.opentripplanner.model.StopTime;
import org.opentripplanner.model.plan.Itinerary;
import org.opentripplanner.model.plan.LegUtils;
import org.opentripplanner.model.plan.ScheduledTransitLeg;
import org.opentripplanner.model.plan.ScheduledTransitLegBuilder;
import org.opentripplanner.model.plan.StreetLeg;
Expand Down Expand Up @@ -146,6 +147,7 @@ private ScheduledTransitLeg createTransitLeg(Route route) {
.withEndTime(TIME.plusMinutes(10))
.withServiceDate(TIME.toLocalDate())
.withZoneId(ZoneIds.BERLIN)
.withDistanceMeters(LegUtils.computeDistanceMeters(pattern, 0,2))
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@
import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.OTHER_FEED_AGENCY;
import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.id;

import java.util.List;
import org.opentripplanner.ext.fares.model.FareAttribute;
import org.opentripplanner.ext.fares.model.FareRuleSet;
import org.opentripplanner.framework.geometry.WgsCoordinate;
import org.opentripplanner.framework.i18n.I18NString;
import org.opentripplanner.model.fare.FareProduct;
import org.opentripplanner.model.fare.FareProductUse;
import org.opentripplanner.transit.model.basic.Money;
import org.opentripplanner.transit.model.basic.TransitMode;
import org.opentripplanner.transit.model.framework.FeedScopedId;
Expand Down Expand Up @@ -82,6 +85,16 @@ public class FareModelForTest {
.setTransfers(1)
.setAgency(OTHER_FEED_AGENCY.getId())
.build();
public static final FareProduct FARE_PRODUCT = new FareProduct(
id("fp"),
"fare product",
Money.euros(10.00f),
null,
null,
null
);
public static final FareProductUse FARE_PRODUCT_USE =
new FareProductUse("c1a04702-1fb6-32d4-ba02-483bf68111ed", FARE_PRODUCT);

// Fare rule sets
static FareRuleSet AIRPORT_TO_CITY_CENTER_SET = new FareRuleSet(TEN_DOLLARS);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package org.opentripplanner.ext.flex;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.opentripplanner._support.time.DateTimes.ANY_ZONED_DATE_TIME_1;
import static org.opentripplanner._support.time.DateTimes.ANY_ZONED_DATE_TIME_2;
import static org.opentripplanner.ext.fares.impl.FareModelForTest.FARE_PRODUCT_USE;
import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.id;

import java.time.Duration;
import java.time.LocalDate;
import java.util.List;
import java.util.Set;
import org.junit.jupiter.api.Test;
import org.opentripplanner.ext.flex.edgetype.FlexTripEdge;
import org.opentripplanner.ext.flex.flexpathcalculator.FlexPath;
import org.opentripplanner.framework.geometry.GeometryUtils;
import org.opentripplanner.framework.i18n.I18NString;
import org.opentripplanner.model.plan.PlanTestConstants;
import org.opentripplanner.routing.alertpatch.TransitAlert;
import org.opentripplanner.street.model._data.StreetModelForTest;

class FlexibleTransitLegBuilderTest implements PlanTestConstants {

private static final FlexTripEdge EDGE = new FlexTripEdge(
StreetModelForTest.intersectionVertex(1,1),
StreetModelForTest.intersectionVertex(2,2),
A.stop,
B.stop,
null,
1,
2,
LocalDate.of(2025, 1, 15),
new FlexPath(1000, 600, () -> GeometryUtils.makeLineString(1,1,2,2))
);
private static final TransitAlert ALERT = TransitAlert.of(id("alert")).withHeaderText(I18NString.of("alert 1")).build();
private static final Duration TIME_SHIFT = Duration.ofHours(5);

@Test
void listsAreInitialized(){
var leg = new FlexibleTransitLegBuilder().withStartTime(ANY_ZONED_DATE_TIME_1).withEndTime(ANY_ZONED_DATE_TIME_2).withFlexTripEdge(EDGE).build();
assertNotNull(leg.fareProducts());
assertNotNull(leg.getTransitAlerts());
}

@Test
void everythingIsNonNull(){
var expectedType = RuntimeException.class;
assertThrows(expectedType,()-> new FlexibleTransitLegBuilder().withStartTime(null).build());
assertThrows(expectedType,()-> new FlexibleTransitLegBuilder().withEndTime(null).build());
assertThrows(expectedType,()-> new FlexibleTransitLegBuilder().withFlexTripEdge(null).build());
assertThrows(expectedType,()-> new FlexibleTransitLegBuilder().withAlerts(null).build());
assertThrows(expectedType,()-> new FlexibleTransitLegBuilder().withFareProducts(null).build());
}

@Test
void copy(){
var leg = new FlexibleTransitLegBuilder().withStartTime(ANY_ZONED_DATE_TIME_1).withEndTime(ANY_ZONED_DATE_TIME_2).withFlexTripEdge(EDGE)
.withFareProducts(List.of(FARE_PRODUCT_USE)).withAlerts(Set.of(ALERT)).build();

var copy = leg.copy().build();

assertEquals(copy.flexTripEdge(), EDGE);
assertEquals(copy.getStartTime(), ANY_ZONED_DATE_TIME_1);
assertEquals(copy.getEndTime(), ANY_ZONED_DATE_TIME_2);
assertEquals(copy.getTransitAlerts(), Set.of(ALERT));
assertEquals(copy.fareProducts(), List.of(FARE_PRODUCT_USE));
}

@Test
void timeShift(){
var leg = new FlexibleTransitLegBuilder().withStartTime(ANY_ZONED_DATE_TIME_1).withEndTime(ANY_ZONED_DATE_TIME_2).withFlexTripEdge(EDGE).withFareProducts(List.of(FARE_PRODUCT_USE)).withAlerts(Set.of(ALERT)).build();
leonardehrenfried marked this conversation as resolved.
Show resolved Hide resolved

var shifted = leg.withTimeShift(TIME_SHIFT);

assertEquals(ANY_ZONED_DATE_TIME_1.plus(TIME_SHIFT), shifted.getStartTime());
assertEquals(ANY_ZONED_DATE_TIME_2.plus(TIME_SHIFT), shifted.getEndTime());
assertEquals(List.of(FARE_PRODUCT_USE), shifted.fareProducts());
assertEquals(Set.of(ALERT), shifted.getTransitAlerts());
}
}
Original file line number Diff line number Diff line change
@@ -1,41 +1,29 @@
package org.opentripplanner.ext.stopconsolidation;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.opentripplanner.ext.fares.impl.FareModelForTest.FARE_PRODUCT_USE;
import static org.opentripplanner.ext.stopconsolidation.TestStopConsolidationModel.STOP_C;
import static org.opentripplanner.ext.stopconsolidation.TestStopConsolidationModel.STOP_D;
import static org.opentripplanner.model.plan.PlanTestConstants.T11_05;
import static org.opentripplanner.model.plan.PlanTestConstants.T11_12;
import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.id;

import java.util.ArrayList;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.opentripplanner.ext.fares.impl.FareModelForTest;
import org.opentripplanner.ext.stopconsolidation.internal.DefaultStopConsolidationRepository;
import org.opentripplanner.ext.stopconsolidation.internal.DefaultStopConsolidationService;
import org.opentripplanner.ext.stopconsolidation.model.ConsolidatedStopGroup;
import org.opentripplanner.ext.stopconsolidation.model.ConsolidatedStopLeg;
import org.opentripplanner.model.fare.FareProduct;
import org.opentripplanner.model.fare.FareProductUse;
import org.opentripplanner.model.plan.Leg;
import org.opentripplanner.model.plan.Place;
import org.opentripplanner.model.plan.PlanTestConstants;
import org.opentripplanner.model.plan.ScheduledTransitLeg;
import org.opentripplanner.model.plan.StreetLeg;
import org.opentripplanner.model.plan.TestItineraryBuilder;
import org.opentripplanner.transit.model.basic.Money;

class DecorateConsolidatedStopNamesTest {

private static final FareProduct fp = new FareProduct(
id("fp"),
"fare product",
Money.euros(10.00f),
null,
null,
null
);
private static final List<FareProductUse> fpu = List.of(
new FareProductUse("c1a04702-1fb6-32d4-ba02-483bf68111ed", fp)
);
private static final List<ConsolidatedStopGroup> GROUPS = List.of(
new ConsolidatedStopGroup(STOP_C.getId(), List.of(STOP_D.getId()))
);
Expand All @@ -52,7 +40,12 @@ void changeNames() {
.bus(1, T11_05, T11_12, PlanTestConstants.F)
.build();

itinerary.getLegs().getFirst().setFareProducts(fpu);
var first = (ScheduledTransitLeg)itinerary.getLegs().getFirst();
var withFp = first.copy().withFareProducts(List.of(FARE_PRODUCT_USE)).build();
var legs = new ArrayList<>(itinerary.getLegs());
legs.set(0, withFp);

itinerary.setLegs(legs);

filter.decorate(itinerary);

Expand All @@ -61,7 +54,7 @@ void changeNames() {
assertEquals(STOP_D.getName(), updatedLeg.getTo().name);

// Check that the fares were carried over
assertEquals(fpu, updatedLeg.fareProducts());
assertEquals(List.of(FARE_PRODUCT_USE), updatedLeg.fareProducts());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package org.opentripplanner.ext.stopconsolidation.model;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.opentripplanner._support.time.DateTimes.ANY_LOCAL_DATE;
import static org.opentripplanner.ext.fares.impl.FareModelForTest.FARE_PRODUCT_USE;
import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.id;

import java.util.List;
import java.util.Set;
import org.junit.jupiter.api.Test;
import org.opentripplanner._support.time.ZoneIds;
import org.opentripplanner.framework.i18n.I18NString;
import org.opentripplanner.model.fare.FareProductUse;
import org.opentripplanner.model.plan.PlanTestConstants;
import org.opentripplanner.model.plan.ScheduledTransitLeg;
import org.opentripplanner.model.plan.ScheduledTransitLegBuilder;
import org.opentripplanner.routing.alertpatch.TransitAlert;
import org.opentripplanner.transit.model._data.TimetableRepositoryForTest;
import org.opentripplanner.transit.model.basic.TransitMode;
import org.opentripplanner.transit.model.network.TripPattern;

class ConsolidatedStopLegBuilderTest implements PlanTestConstants{
private static final Set<TransitAlert> ALERTS = Set.of(TransitAlert
.of(id("alert"))
.withDescriptionText(I18NString.of("alert"))
.build());
private static final TripPattern PATTERN = TimetableRepositoryForTest
.of()
.pattern(TransitMode.BUS)
.build();
private static final ScheduledTransitLeg SCHEDULED_TRANSIT_LEG = new ScheduledTransitLegBuilder<>()
.withZoneId(ZoneIds.BERLIN)
.withServiceDate(ANY_LOCAL_DATE)
.withTripPattern(PATTERN)
.withBoardStopIndexInPattern(0)
.withDistanceMeters(1000)
.withAlightStopIndexInPattern(1).build();
private static final List<FareProductUse> FARES = List.of(FARE_PRODUCT_USE);

@Test
void build(){
var leg = new ConsolidatedStopLegBuilder(SCHEDULED_TRANSIT_LEG).withFrom(E.stop).withTo(F.stop).build();
assertEquals(E.stop, leg.getFrom().stop);
assertEquals(F.stop, leg.getTo().stop);
}

@Test
void copyAttributesFromConsolidatedStopLeg(){
var leg = new ConsolidatedStopLegBuilder(SCHEDULED_TRANSIT_LEG).withFrom(E.stop).withTo(F.stop).build();

var copy = leg.copy().withAccessibilityScore(4f).withFareProducts(FARES).withAlerts(Set.of(ALERTS)).build();

assertEquals(leg.getFrom().stop, copy.getFrom().stop);
assertEquals(leg.getTo().stop, copy.getTo().stop);
assertEquals(Set.of(ALERTS), copy.getTransitAlerts());
assertEquals(FARES, copy.fareProducts());
assertEquals(ZoneIds.BERLIN, copy.getZoneId());

}

@Test
void copyConsolidatedLeg(){
var leg = new ConsolidatedStopLegBuilder(SCHEDULED_TRANSIT_LEG).withFrom(E.stop).withTo(F.stop).withAlerts(ALERTS).build();

var copy = leg.copy().build();

assertEquals(E.stop, copy.getFrom().stop);
assertEquals(F.stop, copy.getTo().stop);
assertEquals(ALERTS, copy.getTransitAlerts());
}

@Test
void copyAttributesFromScheduledLeg(){
var leg = SCHEDULED_TRANSIT_LEG.copy().withFareProducts(FARES).withAlerts(Set.of(ALERTS)).build();

var copy = new ConsolidatedStopLegBuilder(leg).withFrom(C.stop).withTo(G.stop).build();

assertEquals(C.stop, copy.getFrom().stop);
assertEquals(G.stop, copy.getTo().stop);
assertEquals(Set.of(ALERTS), copy.getTransitAlerts());
assertEquals(FARES, copy.fareProducts());
assertEquals(ZoneIds.BERLIN, copy.getZoneId());

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ private Itinerary addAccessibilityScore(Itinerary i) {
.stream()
.map(leg -> {
if (leg instanceof ScheduledTransitLeg transitLeg) {
return transitLeg.withAccessibilityScore(compute(transitLeg));
return transitLeg.copy().withAccessibilityScore(compute(transitLeg)).build();
} else if (leg instanceof StreetLeg streetLeg && leg.isWalkingLeg()) {
return streetLeg.withAccessibilityScore(compute(streetLeg));
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import org.opentripplanner.model.fare.FareProductUse;
import org.opentripplanner.model.fare.ItineraryFares;
import org.opentripplanner.model.plan.FareProductAware;
import org.opentripplanner.model.plan.Itinerary;
import org.opentripplanner.model.plan.TransitLeg;
import org.opentripplanner.utils.collection.ListUtils;

/**
Expand All @@ -22,8 +24,12 @@ public static void addFaresToLegs(ItineraryFares fares, Itinerary i) {

i.transformTransitLegs(leg -> {
var legUses = fares.getLegProducts().get(leg);
leg.setFareProducts(ListUtils.combine(itineraryFareUses, legUses));
return leg;
var allUses = ListUtils.combine(itineraryFareUses, legUses);
leonardehrenfried marked this conversation as resolved.
Show resolved Hide resolved
if(leg instanceof FareProductAware<TransitLeg> fpa) {
return fpa.decorateWithFareProducts(allUses);
} else {
return leg;
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
import org.opentripplanner.model.plan.Leg;
import org.opentripplanner.model.plan.LegCallTime;
import org.opentripplanner.model.plan.Place;
import org.opentripplanner.model.plan.ScheduledTransitLegBuilder;
import org.opentripplanner.model.plan.StopArrival;
import org.opentripplanner.model.plan.TransitLeg;
import org.opentripplanner.routing.alertpatch.TransitAlert;
import org.opentripplanner.transit.model.basic.TransitMode;
import org.opentripplanner.transit.model.network.Route;
import org.opentripplanner.transit.model.organization.Agency;
Expand Down Expand Up @@ -100,6 +102,11 @@ public LineString getLegGeometry() {
return null;
}

@Override
public Set<TransitAlert> getTransitAlerts() {
return Set.of();
}

@Override
public int getGeneralizedCost() {
if (first.getGeneralizedCost() == UNKNOWN) {
Expand All @@ -119,9 +126,6 @@ public Set<FareZone> getFareZones() {
return fareZones;
}

@Override
public void setFareProducts(List<FareProductUse> products) {}

@Override
public List<FareProductUse> fareProducts() {
return List.of();
Expand All @@ -133,4 +137,14 @@ public List<FareProductUse> fareProducts() {
public List<Leg> originalLegs() {
return List.of(first, second);
}

@Override
public TransitLeg decorateWithAlerts(Set<TransitAlert> alerts) {
throw new UnsupportedOperationException();
}

@Override
public TransitLeg decorateWithFareProducts(List<FareProductUse> fares) {
throw new UnsupportedOperationException();
}
}
Loading
Loading