Merge pull request #5638 from entur/fix_pass_through

Pass-through should override transit-group-priority

Author: t2gran

src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java

@@ -121,7 +121,7 @@ private TransitRouterResult route() {
     );
 
     // Prepare transit search
-    var raptorRequest = RaptorRequestMapper.mapRequest(
+    var raptorRequest = RaptorRequestMapper.<TripSchedule>mapRequest(
       request,
       transitSearchTimeZero,
       serverContext.raptorConfig().isMultiThreaded(),

src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapper.java

@@ -12,6 +12,7 @@
 import org.opentripplanner.raptor.api.model.GeneralizedCostRelaxFunction;
 import org.opentripplanner.raptor.api.model.RaptorAccessEgress;
 import org.opentripplanner.raptor.api.model.RaptorConstants;
+import org.opentripplanner.raptor.api.model.RaptorTripSchedule;
 import org.opentripplanner.raptor.api.model.RelaxFunction;
 import org.opentripplanner.raptor.api.request.DebugRequestBuilder;
 import org.opentripplanner.raptor.api.request.Optimization;
@@ -28,7 +29,7 @@
 import org.opentripplanner.routing.api.request.framework.CostLinearFunction;
 import org.opentripplanner.transit.model.site.StopLocation;
 
-public class RaptorRequestMapper {
+public class RaptorRequestMapper<T extends RaptorTripSchedule> {
 
   private final RouteRequest request;
   private final Collection<? extends RaptorAccessEgress> accessPaths;
@@ -56,7 +57,7 @@ private RaptorRequestMapper(
     this.meterRegistry = meterRegistry;
   }
 
-  public static RaptorRequest<TripSchedule> mapRequest(
+  public static <T extends RaptorTripSchedule> RaptorRequest<T> mapRequest(
     RouteRequest request,
     ZonedDateTime transitSearchTimeZero,
     boolean isMultiThreaded,
@@ -65,7 +66,7 @@ public static RaptorRequest<TripSchedule> mapRequest(
     Duration searchWindowAccessSlack,
     MeterRegistry meterRegistry
   ) {
-    return new RaptorRequestMapper(
+    return new RaptorRequestMapper<T>(
       request,
       isMultiThreaded,
       accessPaths,
@@ -77,8 +78,8 @@ public static RaptorRequest<TripSchedule> mapRequest(
       .doMap();
   }
 
-  private RaptorRequest<TripSchedule> doMap() {
-    var builder = new RaptorRequestBuilder<TripSchedule>();
+  private RaptorRequest<T> doMap() {
+    var builder = new RaptorRequestBuilder<T>();
     var searchParams = builder.searchParams();
 
     var preferences = request.preferences();
@@ -119,12 +120,14 @@ private RaptorRequest<TripSchedule> doMap() {
     builder.withMultiCriteria(mcBuilder -> {
       var pt = preferences.transit();
       var r = pt.raptor();
-      if (!pt.relaxTransitGroupPriority().isNormal()) {
-        mcBuilder.withTransitPriorityCalculator(TransitGroupPriority32n.priorityCalculator());
-        mcBuilder.withRelaxC1(mapRelaxCost(pt.relaxTransitGroupPriority()));
-      } else {
+
+      // Note! If a pass-through-point exists, then the transit-group-priority feature is disabled
+      if (!request.getPassThroughPoints().isEmpty()) {
         mcBuilder.withPassThroughPoints(mapPassThroughPoints());
         r.relaxGeneralizedCostAtDestination().ifPresent(mcBuilder::withRelaxCostAtDestination);
+      } else if (!pt.relaxTransitGroupPriority().isNormal()) {
+        mcBuilder.withTransitPriorityCalculator(TransitGroupPriority32n.priorityCalculator());
+        mcBuilder.withRelaxC1(mapRelaxCost(pt.relaxTransitGroupPriority()));
       }
     });
 
@@ -174,13 +177,15 @@ private RaptorRequest<TripSchedule> doMap() {
     }
 
     // Add this last, it depends on generating an alias from the set values
-    builder.performanceTimers(
-      new PerformanceTimersForRaptor(
-        builder.generateAlias(),
-        preferences.system().tags(),
-        meterRegistry
-      )
-    );
+    if (meterRegistry != null) {
+      builder.performanceTimers(
+        new PerformanceTimersForRaptor(
+          builder.generateAlias(),
+          preferences.system().tags(),
+          meterRegistry
+        )
+      );
+    }
 
     return builder.build();
   }

src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapperTest.java

@@ -1,15 +1,33 @@
 package org.opentripplanner.routing.algorithm.raptoradapter.transit.mappers;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.time.Duration;
+import java.time.ZonedDateTime;
 import java.util.List;
+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.raptor._data.transit.TestAccessEgress;
+import org.opentripplanner.raptor._data.transit.TestTripSchedule;
+import org.opentripplanner.raptor.api.model.RaptorAccessEgress;
+import org.opentripplanner.raptor.api.request.RaptorRequest;
+import org.opentripplanner.routing.api.request.PassThroughPoint;
+import org.opentripplanner.routing.api.request.RouteRequest;
 import org.opentripplanner.routing.api.request.framework.CostLinearFunction;
+import org.opentripplanner.transit.model._data.TransitModelForTest;
+import org.opentripplanner.transit.model.site.StopLocation;
 
 class RaptorRequestMapperTest {
 
+  private static final TransitModelForTest TEST_MODEL = TransitModelForTest.of();
+  private static final StopLocation STOP_A = TEST_MODEL.stop("Stop:A").build();
+  private static final List<RaptorAccessEgress> ACCESS = List.of(TestAccessEgress.walk(12, 45));
+  private static final List<RaptorAccessEgress> EGRESS = List.of(TestAccessEgress.walk(144, 54));
+  private static final Duration D0s = Duration.ofSeconds(0);
+
   private static final CostLinearFunction R1 = CostLinearFunction.of("50 + 1.0x");
   private static final CostLinearFunction R2 = CostLinearFunction.of("0 + 1.5x");
   private static final CostLinearFunction R3 = CostLinearFunction.of("30 + 2.0x");
@@ -33,4 +51,47 @@ void mapRelaxCost(CostLinearFunction input, int cost, int expected) {
     var calcCost = RaptorRequestMapper.mapRelaxCost(input);
     assertEquals(expected, calcCost.relax(cost));
   }
+
+  @Test
+  void testPassThroughPoints() {
+    var req = new RouteRequest();
+
+    req.setPassThroughPoints(List.of(new PassThroughPoint(List.of(STOP_A), "Via A")));
+
+    var result = map(req);
+
+    assertTrue(result.multiCriteria().hasPassThroughPoints());
+    assertEquals(
+      "[(Via A, stops: " + STOP_A.getIndex() + ")]",
+      result.multiCriteria().passThroughPoints().toString()
+    );
+  }
+
+  @Test
+  void testPassThroughPointsTurnTransitGroupPriorityOff() {
+    var req = new RouteRequest();
+
+    // Set pass-through and relax transit-group-priority
+    req.setPassThroughPoints(List.of(new PassThroughPoint(List.of(STOP_A), "Via A")));
+    req.withPreferences(p ->
+      p.withTransit(t -> t.withRelaxTransitGroupPriority(CostLinearFunction.of("30m + 1.2t")))
+    );
+
+    var result = map(req);
+
+    //  transit-group-priority CANNOT be used with pass-through and is turned off...
+    assertTrue(result.multiCriteria().transitPriorityCalculator().isEmpty());
+  }
+
+  private static RaptorRequest<TestTripSchedule> map(RouteRequest request) {
+    return RaptorRequestMapper.mapRequest(
+      request,
+      ZonedDateTime.now(),
+      false,
+      ACCESS,
+      EGRESS,
+      D0s,
+      null
+    );
+  }
 }