Skip to content

Commit

Permalink
Fix NPE in Itinerary mapper when casting access/egress to DefaultAcce…
Browse files Browse the repository at this point in the history
…ssEgress.

The code did not work because the access/egress is can be wrapped inside Raptor.
  • Loading branch information
t2gran committed Feb 11, 2025
1 parent 0d22337 commit b866b5e
Show file tree
Hide file tree
Showing 7 changed files with 198 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.opentripplanner.framework.application.OTPFeature;
import org.opentripplanner.framework.geometry.GeometryUtils;
import org.opentripplanner.framework.i18n.NonLocalizedString;
import org.opentripplanner.framework.model.TimeAndCost;
import org.opentripplanner.model.GenericLocation;
import org.opentripplanner.model.plan.FrequencyTransitLegBuilder;
import org.opentripplanner.model.plan.Itinerary;
Expand All @@ -20,6 +21,7 @@
import org.opentripplanner.model.plan.StreetLeg;
import org.opentripplanner.model.plan.UnknownTransitPathLeg;
import org.opentripplanner.model.transfer.ConstrainedTransfer;
import org.opentripplanner.raptor.api.model.RaptorAccessEgress;
import org.opentripplanner.raptor.api.path.AccessPathLeg;
import org.opentripplanner.raptor.api.path.EgressPathLeg;
import org.opentripplanner.raptor.api.path.PathLeg;
Expand Down Expand Up @@ -156,15 +158,15 @@ else if (pathLeg.isTransferLeg()) {
}

var penaltyCost = 0;
if (accessPathLeg.access() instanceof RoutingAccessEgress ae) {
itinerary.setAccessPenalty(ae.penalty());
penaltyCost += ae.penalty().cost().toSeconds();
}

if (egressPathLeg.egress() instanceof RoutingAccessEgress ae) {
itinerary.setEgressPenalty(ae.penalty());
penaltyCost += ae.penalty().cost().toSeconds();
}
var accessPenalty = mapAccessEgressPenalty(accessPathLeg.access());
itinerary.setAccessPenalty(accessPenalty);
penaltyCost += accessPenalty.cost().toSeconds();

var egressPenalty = mapAccessEgressPenalty(egressPathLeg.egress());
itinerary.setEgressPenalty(egressPenalty);
penaltyCost += egressPenalty.cost().toSeconds();

if (path.isC2Set()) {
itinerary.setGeneralizedCost2(path.c2());
}
Expand Down Expand Up @@ -192,13 +194,9 @@ private List<Leg> mapAccessLeg(AccessPathLeg<T> accessPathLeg) {
return List.of();
}

RoutingAccessEgress accessPath = (RoutingAccessEgress) accessPathLeg.access();

var graphPath = new GraphPath<>(accessPath.getLastState());

Itinerary subItinerary = graphPathToItineraryMapper.generateItinerary(graphPath);
var subItinerary = mapAccessEgressPathLeg(accessPathLeg.access());

if (subItinerary.getLegs().isEmpty()) {
if (subItinerary == null || subItinerary.getLegs().isEmpty()) {
return List.of();
}

Expand Down Expand Up @@ -329,13 +327,9 @@ private Itinerary mapEgressLeg(EgressPathLeg<T> egressPathLeg) {
return null;
}

RoutingAccessEgress egressPath = (RoutingAccessEgress) egressPathLeg.egress();
var subItinerary = mapAccessEgressPathLeg(egressPathLeg.egress());

var graphPath = new GraphPath<>(egressPath.getLastState());

Itinerary subItinerary = graphPathToItineraryMapper.generateItinerary(graphPath);

if (subItinerary.getLegs().isEmpty()) {
if (subItinerary == null || subItinerary.getLegs().isEmpty()) {
return null;
}

Expand Down Expand Up @@ -432,4 +426,20 @@ private boolean includeTransferInItinerary(Leg transitLegBeforeTransfer) {
!transitLegBeforeTransfer.getTransferToNextLeg().getTransferConstraint().isStaySeated()
);
}

private Itinerary mapAccessEgressPathLeg(RaptorAccessEgress accessEgress) {
return accessEgress
.findOriginal(RoutingAccessEgress.class)
.map(RoutingAccessEgress::getLastState)
.map(GraphPath::new)
.map(graphPathToItineraryMapper::generateItinerary)
.orElseThrow();
}

private TimeAndCost mapAccessEgressPenalty(RaptorAccessEgress accessEgress) {
return accessEgress
.findOriginal(RoutingAccessEgress.class)
.map(RoutingAccessEgress::penalty)
.orElseThrow();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public DefaultAccessEgress(
this.generalizedCost = generalizedCost;
this.timePenalty = penalty.isZero() ? RaptorConstants.TIME_NOT_SET : penalty.timeInSeconds();
this.penalty = penalty;
this.lastState = lastState;
this.lastState = Objects.requireNonNull(lastState);
}

public DefaultAccessEgress(int stop, State lastState) {
Expand All @@ -63,7 +63,7 @@ protected DefaultAccessEgress(RoutingAccessEgress other, TimeAndCost penalty) {
penalty,
other.getLastState()
);
if (other.hasPenalty()) {
if (other.penalty() != TimeAndCost.ZERO) {
throw new IllegalStateException("Can not add penalty twice...");
}
}
Expand Down Expand Up @@ -103,11 +103,6 @@ public boolean isWalkOnly() {
return lastState.containsOnlyWalkMode();
}

@Override
public boolean hasPenalty() {
return !penalty.isZero();
}

@Override
public TimeAndCost penalty() {
return penalty;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,5 @@ public interface RoutingAccessEgress extends RaptorAccessEgress {
*/
boolean isWalkOnly();

boolean hasPenalty();

TimeAndCost penalty();
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ void canNotAddPenaltyTwice() {

@Test
void durationInSeconds() {
// TODO - The value is ?
int expected = 118215;
int expected = (int) LAST_STATE.getElapsedTimeSeconds();
assertEquals(expected, subject.durationInSeconds());
assertEquals(expected, subjectWithPenalty.durationInSeconds());
}
Expand Down Expand Up @@ -102,13 +101,6 @@ void containsModeWalkOnly() {
assertFalse(subject.isWalkOnly());
}

@Test
void hasPenalty() {
assertFalse(subject.hasPenalty());
assertFalse(subject.withPenalty(TimeAndCost.ZERO).hasPenalty());
assertTrue(subjectWithPenalty.hasPenalty());
}

@Test
void penalty() {
assertEquals(TimeAndCost.ZERO, subject.penalty());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.opentripplanner.raptor.api.model;

import java.time.Duration;
import java.util.Objects;
import java.util.Optional;
import javax.annotation.Nullable;

/**
Expand All @@ -16,6 +18,13 @@ public AbstractAccessEgressDecorator(RaptorAccessEgress delegate) {
this.delegate = delegate;
}

public static RaptorAccessEgress accessEgressWithExtraSlack(
RaptorAccessEgress delegate,
Duration slack
) {
return accessEgressWithExtraSlack(delegate, (int) slack.toSeconds());
}

public static RaptorAccessEgress accessEgressWithExtraSlack(
RaptorAccessEgress delegate,
int slack
Expand All @@ -28,6 +37,18 @@ public int durationInSeconds() {
};
}

@SuppressWarnings({ "ReassignedVariable", "unchecked" })
static <T extends RaptorAccessEgress> Optional<T> findType(RaptorAccessEgress it, Class<T> type) {
while (!type.isAssignableFrom(it.getClass())) {
if (it instanceof AbstractAccessEgressDecorator d) {
it = d.delegate();
} else {
throw new IllegalStateException("Unexpected type: " + type + ". Type not found in:" + it);
}
}
return Optional.of((T) it);
}

protected RaptorAccessEgress delegate() {
return delegate;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.opentripplanner.raptor.api.model.RaptorConstants.TIME_NOT_SET;

import java.time.temporal.ChronoUnit;
import java.util.Optional;
import javax.annotation.Nullable;
import org.opentripplanner.utils.time.DurationUtils;
import org.opentripplanner.utils.time.TimeUtils;
Expand All @@ -13,6 +14,26 @@
* to Raptor - all these are the same thing.
*/
public interface RaptorAccessEgress {
/**
* Raptor may decorate access/egress passed into Raptor. Use this method to get the original
* instance of a given {@code type} type passed into Raptor. The first element matching the the
* given {@code type} in the chain of delegates (see {@link AbstractAccessEgressDecorator}) is
* returned.
* <p>
* This method is primarily for use outside Raptor to get the base access-egress instant to
* access the EXTENDED state of the base type - state not part of the Raptor interface. This is
* useful in the caller to get access to additional information stored in the base type.
* <p>
* Be careful, calling methods part of the {@link RaptorAccessEgress} interface on the returned
* value will no longer be decorated - not be the value used by Raptor.
*
* @throws IllegalStateException if the given {@code parentType} does not exist in the chain
* of delegates including the first and last element.
*/
default <T extends RaptorAccessEgress> Optional<T> findOriginal(Class<T> type) {
return AbstractAccessEgressDecorator.findType(this, type);
}

/**
* <ul>
* <li>Access: The first stop in the journey, where the access path just arrived at.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
package org.opentripplanner.raptor.api.model;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.time.Duration;
import org.junit.jupiter.api.Test;
import org.opentripplanner.raptor._data.transit.TestAccessEgress;

class AbstractAccessEgressDecoratorTest {

private static final int DURATION = 600;
private static final int SLACK = 60;
private static final int STOP = 7;
private static final TestAccessEgress DELEGATE = TestAccessEgress.walk(STOP, DURATION);
private static final TestAccessEgress DELEGATE_W_OPENING_HOURS = DELEGATE.openingHours(
"10:00",
"10:35"
);

private final RaptorAccessEgress subject = AbstractAccessEgressDecorator.accessEgressWithExtraSlack(
DELEGATE,
SLACK
);
private final RaptorAccessEgress subjectWOpeningHours = AbstractAccessEgressDecorator.accessEgressWithExtraSlack(
DELEGATE_W_OPENING_HOURS,
SLACK
);

private AbstractAccessEgressDecorator subjectCast() {
return (AbstractAccessEgressDecorator) subject;
}

@Test
void resolveDelegate() {
assertEquals(
DELEGATE,
AbstractAccessEgressDecorator.findType(subject, TestAccessEgress.class).orElseThrow()
);
assertEquals(
DELEGATE,
AbstractAccessEgressDecorator.findType(DELEGATE, TestAccessEgress.class).orElseThrow()
);
}

@Test
void createAccessEgressWithExtraSlackFromDuration() {
var fromDuration = AbstractAccessEgressDecorator.accessEgressWithExtraSlack(
DELEGATE,
Duration.ofSeconds(SLACK)
);
assertEquals(subject.durationInSeconds(), fromDuration.durationInSeconds());
}

@Test
void delegate() {
assertEquals(DELEGATE, subjectCast().delegate());
}

@Test
void stop() {
assertEquals(STOP, subject.stop());
assertEquals(DELEGATE.stop(), subject.stop());
}

@Test
void c1() {
assertEquals(DELEGATE.c1(), subject.c1());
}

@Test
void durationInSeconds() {
assertEquals(DELEGATE.durationInSeconds() + SLACK, subject.durationInSeconds());
}

@Test
void timePenalty() {}

@Test
void hasTimePenalty() {
assertFalse(subject.hasTimePenalty());
}

@Test
void earliestDepartureTime() {
assertEquals(100, subject.earliestDepartureTime(100));
}

@Test
void latestArrivalTime() {
assertEquals(100, subject.latestArrivalTime(100));
}

@Test
void hasOpeningHours() {
assertFalse(subject.hasOpeningHours());
assertTrue(subjectWOpeningHours.hasOpeningHours());
}

@Test
void openingHoursToString() {
assertNull(subject.openingHoursToString());
assertEquals("Open(10:00 10:35)", subjectWOpeningHours.openingHoursToString());
}

@Test
void numberOfRides() {
assertEquals(0, subject.numberOfRides());
}

@Test
void hasRides() {
assertFalse(subject.hasRides());
}

@Test
void testToString() {
assertEquals("Walk 10m C₁1_200 ~ 7", subject.toString());
assertEquals("Walk 10m C₁1_200 Open(10:00 10:35) ~ 7", subjectWOpeningHours.toString());
}
}

0 comments on commit b866b5e

Please sign in to comment.