Merge pull request opentripplanner#5610 from entur/cleanup_filters

Cleanup filters
ibi-group · Jan 19, 2024 · 3f69e5e · 3f69e5e
2 parents 323df42 + 4e4af20
commit 3f69e5e
Show file tree

Hide file tree

Showing 90 changed files with 1,362 additions and 880 deletions.
diff --git a/...xt-test/java/org/opentripplanner/ext/accessibilityscore/AccessibilityScoreFilterTest.java b/...xt-test/java/org/opentripplanner/ext/accessibilityscore/AccessibilityScoreFilterTest.java
diff --git a/...t/java/org/opentripplanner/ext/accessibilityscore/DecorateWithAccessibilityScoreTest.java b/...t/java/org/opentripplanner/ext/accessibilityscore/DecorateWithAccessibilityScoreTest.java
@@ -0,0 +1,64 @@
+package org.opentripplanner.ext.accessibilityscore;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.opentripplanner.model.plan.TestItineraryBuilder.newItinerary;
+
+import java.util.List;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.opentripplanner.model.plan.Itinerary;
+import org.opentripplanner.model.plan.Place;
+import org.opentripplanner.model.plan.PlanTestConstants;
+import org.opentripplanner.routing.api.request.preference.WheelchairPreferences;
+
+public class DecorateWithAccessibilityScoreTest implements PlanTestConstants {
+
+  private static final int ID = 1;
+
+  static List<Arguments> accessibilityScoreTestCase() {
+    return List.of(
+      Arguments.of(
+        newItinerary(A, 0)
+          .walk(20, Place.forStop(TEST_MODEL.stop("1:stop", 1d, 1d).build()))
+          .bus(ID, 0, 50, B)
+          .bus(ID, 52, 100, C)
+          .build(),
+        0.5f
+      ),
+      Arguments.of(
+        newItinerary(A, 0)
+          .walk(20, Place.forStop(TEST_MODEL.stop("1:stop", 1d, 1d).build()))
+          .bus(ID, 0, 50, B)
+          .bus(ID, 52, 100, C)
+          .build(),
+        0.5f
+      ),
+      Arguments.of(
+        newItinerary(A, 0)
+          .walk(20, Place.forStop(TEST_MODEL.stop("1:stop", 1d, 1d).build()))
+          .bus(ID, 0, 50, B)
+          .bus(ID, 52, 100, C)
+          .build(),
+        0.5f
+      )
+    );
+  }
+
+  @ParameterizedTest
+  @MethodSource("accessibilityScoreTestCase")
+  public void accessibilityScoreTest(Itinerary itinerary, float expectedAccessibilityScore) {
+    var filter = new DecorateWithAccessibilityScore(WheelchairPreferences.DEFAULT.maxSlope());
+
+    filter.decorate(itinerary);
+
+    assertEquals(expectedAccessibilityScore, itinerary.getAccessibilityScore());
+
+    itinerary
+      .getLegs()
+      .forEach(l -> {
+        assertNotNull(l.accessibilityScore());
+      });
+  }
+}
diff --git a/src/ext-test/java/org/opentripplanner/ext/emissions/EmissionsTest.java b/src/ext-test/java/org/opentripplanner/ext/emissions/EmissionsTest.java
@@ -31,7 +31,7 @@
 class EmissionsTest {
 
   private static DefaultEmissionsService eService;
-  private static EmissionsFilter emissionsFilter;
+  private static DecorateWithEmission decorateWithEmission;
 
   static final ZonedDateTime TIME = OffsetDateTime
     .parse("2023-07-20T17:49:06+03:00")
@@ -58,58 +58,52 @@ static void SetUp() {
     emissions.put(new FeedScopedId("F", "2"), 0.0);
     EmissionsDataModel emissionsDataModel = new EmissionsDataModel(emissions, 0.131);
     eService = new DefaultEmissionsService(emissionsDataModel);
-    emissionsFilter = new EmissionsFilter(eService);
+    decorateWithEmission = new DecorateWithEmission(eService);
   }
 
   @Test
   void testGetEmissionsForItinerary() {
     Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITH_EMISSIONS)));
-    assertEquals(
-      new Grams(2223.902),
-      emissionsFilter.filter(List.of(i)).get(0).getEmissionsPerPerson().getCo2()
-    );
+    decorateWithEmission.decorate(i);
+    assertEquals(new Grams(2223.902), i.getEmissionsPerPerson().getCo2());
   }
 
   @Test
   void testGetEmissionsForCarRoute() {
     Itinerary i = new Itinerary(List.of(STREET_LEG));
-    assertEquals(
-      new Grams(28.0864),
-      emissionsFilter.filter(List.of(i)).get(0).getEmissionsPerPerson().getCo2()
-    );
+    decorateWithEmission.decorate(i);
+    assertEquals(new Grams(28.0864), i.getEmissionsPerPerson().getCo2());
   }
 
   @Test
   void testNoEmissionsForFeedWithoutEmissionsConfigured() {
     Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITHOUT_EMISSIONS_CONFIGURED)));
-    assertNull(emissionsFilter.filter(List.of(i)).get(0).getEmissionsPerPerson());
+    decorateWithEmission.decorate(i);
+    assertNull(i.getEmissionsPerPerson());
   }
 
   @Test
   void testZeroEmissionsForItineraryWithZeroEmissions() {
     Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITH_ZERO_EMISSIONS)));
-    assertEquals(
-      new Grams(0.0),
-      emissionsFilter.filter(List.of(i)).get(0).getEmissionsPerPerson().getCo2()
-    );
+    decorateWithEmission.decorate(i);
+    assertEquals(new Grams(0.0), i.getEmissionsPerPerson().getCo2());
   }
 
   @Test
   void testGetEmissionsForCombinedRoute() {
     Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITH_EMISSIONS), STREET_LEG));
-    assertEquals(
-      new Grams(2251.9884),
-      emissionsFilter.filter(List.of(i)).get(0).getEmissionsPerPerson().getCo2()
-    );
+    decorateWithEmission.decorate(i);
+    assertEquals(new Grams(2251.9884), i.getEmissionsPerPerson().getCo2());
   }
 
   @Test
   void testNoEmissionsForCombinedRouteWithoutTransitEmissions() {
     Itinerary i = new Itinerary(
       List.of(createTransitLeg(ROUTE_WITHOUT_EMISSIONS_CONFIGURED), STREET_LEG)
     );
-    var emissionsResult = emissionsFilter.filter(List.of(i)).get(0).getEmissionsPerPerson() != null
-      ? emissionsFilter.filter(List.of(i)).get(0).getEmissionsPerPerson().getCo2()
+    decorateWithEmission.decorate(i);
+    var emissionsResult = i.getEmissionsPerPerson() != null
+      ? i.getEmissionsPerPerson().getCo2()
       : null;
     assertNull(emissionsResult);
   }

diff --git a/src/ext-test/java/org/opentripplanner/ext/fares/FaresFilterTest.java b/src/ext-test/java/org/opentripplanner/ext/fares/FaresFilterTest.java
@@ -31,9 +31,7 @@ void shouldAddFare() {
       .bus(ID, 52, 100, C)
       .build();
 
-    List<Itinerary> input = List.of(i1, i1, i1);
-
-    input.forEach(i -> assertEquals(ItineraryFares.empty(), i.getFares()));
+    assertEquals(ItineraryFares.empty(), i1.getFares());
 
     var fares = new ItineraryFares();
     fares.addFare(FareType.regular, Money.euros(2.80f));
@@ -42,12 +40,13 @@ void shouldAddFare() {
     var fp = new FareProduct(id("fp"), "fare product", Money.euros(10.00f), null, null, null);
     fares.addFareProduct(leg, fp);
 
-    var filter = new FaresFilter((FareService) itinerary -> fares);
-    var filtered = filter.filter(input);
+    var filter = new DecorateWithFare((FareService) itinerary -> fares);
+
+    filter.decorate(i1);
 
-    filtered.forEach(i -> assertEquals(fares, i.getFares()));
+    assertEquals(fares, i1.getFares());
 
-    var busLeg = filtered.get(0).getTransitLeg(1);
+    var busLeg = i1.getTransitLeg(1);
 
     assertEquals(
       List.of(new FareProductUse("c1a04702-1fb6-32d4-ba02-483bf68111ed", fp)),

diff --git a/src/ext-test/java/org/opentripplanner/ext/flex/trip/ScheduledDeviatedTripTest.java b/src/ext-test/java/org/opentripplanner/ext/flex/trip/ScheduledDeviatedTripTest.java
@@ -18,7 +18,7 @@
 import org.opentripplanner.TestOtpModel;
 import org.opentripplanner.TestServerContext;
 import org.opentripplanner._support.time.ZoneIds;
-import org.opentripplanner.ext.fares.FaresFilter;
+import org.opentripplanner.ext.fares.DecorateWithFare;
 import org.opentripplanner.ext.flex.FlexRouter;
 import org.opentripplanner.ext.flex.FlexTest;
 import org.opentripplanner.framework.application.OTPFeature;
@@ -144,9 +144,9 @@ void calculateDirectFare() {
       List.of(to)
     );
 
-    var filter = new FaresFilter(graph.getFareService());
+    var filter = new DecorateWithFare(graph.getFareService());
 
-    var itineraries = filter.filter(router.createFlexOnlyItineraries().stream().toList());
+    var itineraries = router.createFlexOnlyItineraries().stream().peek(filter::decorate).toList();
 
     var itinerary = itineraries.iterator().next();
     assertFalse(itinerary.getFares().getFareTypes().isEmpty());

diff --git a/...xt/ridehailing/RideHailingFilterTest.java → ...ehailing/DecorateWithRideHailingTest.java b/...xt/ridehailing/RideHailingFilterTest.java → ...ehailing/DecorateWithRideHailingTest.java
@@ -16,7 +16,7 @@
 import org.opentripplanner.model.plan.TestItineraryBuilder;
 import org.opentripplanner.transit.model.basic.Money;
 
-class RideHailingFilterTest implements PlanTestConstants {
+class DecorateWithRideHailingTest implements PlanTestConstants {
 
   public static final RideEstimate RIDE_ESTIMATE = new RideEstimate(
     UBER,
@@ -39,7 +39,7 @@ class RideHailingFilterTest implements PlanTestConstants {
 
   @Test
   void noServices() {
-    var filter = new RideHailingFilter(List.of(), false);
+    var filter = new DecorateWithRideHailing(List.of(), false);
 
     var filtered = filter.filter(List.of(i));
 
@@ -48,7 +48,7 @@ void noServices() {
 
   @Test
   void addRideHailingInformation() {
-    var filter = new RideHailingFilter(List.of(mockService), false);
+    var filter = new DecorateWithRideHailing(List.of(mockService), false);
 
     var filtered = filter.filter(List.of(i));
 
@@ -63,7 +63,7 @@ void addRideHailingInformation() {
 
   @Test
   void failingService() {
-    var filter = new RideHailingFilter(List.of(failingService), false);
+    var filter = new DecorateWithRideHailing(List.of(failingService), false);
 
     var filtered = filter.filter(List.of(i));
 

diff --git a/...ation/ConsolidatedStopNameFilterTest.java → ...on/DecorateConsolidatedStopNamesTest.java b/...ation/ConsolidatedStopNameFilterTest.java → ...on/DecorateConsolidatedStopNamesTest.java
@@ -1,7 +1,6 @@
 package org.opentripplanner.ext.stopconsolidation;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 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;
@@ -16,7 +15,7 @@
 import org.opentripplanner.model.plan.PlanTestConstants;
 import org.opentripplanner.model.plan.TestItineraryBuilder;
 
-class ConsolidatedStopNameFilterTest {
+class DecorateConsolidatedStopNamesTest {
 
   @Test
   void changeNames() {
@@ -27,7 +26,7 @@ void changeNames() {
     repo.addGroups(groups);
 
     var service = new DefaultStopConsolidationService(repo, transitModel);
-    var filter = new ConsolidatedStopNameFilter(service);
+    var filter = new DecorateConsolidatedStopNames(service);
 
     var itinerary = TestItineraryBuilder
       .newItinerary(Place.forStop(STOP_C))
@@ -36,10 +35,9 @@ void changeNames() {
       .bus(1, T11_05, T11_12, PlanTestConstants.F)
       .build();
 
-    var filtered = filter.filter(List.of(itinerary));
-    assertFalse(filtered.isEmpty());
+    filter.decorate(itinerary);
 
-    var updatedLeg = filtered.get(0).getLegs().get(0);
+    var updatedLeg = itinerary.getLegs().get(0);
     assertEquals(STOP_D.getName(), updatedLeg.getFrom().name);
     assertEquals(STOP_D.getName(), updatedLeg.getTo().name);
   }

diff --git a/...bilityscore/AccessibilityScoreFilter.java → ...score/DecorateWithAccessibilityScore.java b/...bilityscore/AccessibilityScoreFilter.java → ...score/DecorateWithAccessibilityScore.java
@@ -8,7 +8,7 @@
 import org.opentripplanner.model.plan.ScheduledTransitLeg;
 import org.opentripplanner.model.plan.StreetLeg;
 import org.opentripplanner.model.plan.WalkStep;
-import org.opentripplanner.routing.algorithm.filterchain.ItineraryListFilter;
+import org.opentripplanner.routing.algorithm.filterchain.framework.spi.ItineraryDecorator;
 import org.opentripplanner.street.model.edge.StreetEdge;
 import org.opentripplanner.street.model.edge.WheelchairTraversalInformation;
 import org.opentripplanner.transit.model.basic.Accessibility;
@@ -25,7 +25,8 @@
  * calculating them on the backend makes life a little easier and changes are automatically applied
  * to all frontends.
  */
-public record AccessibilityScoreFilter(double wheelchairMaxSlope) implements ItineraryListFilter {
+public record DecorateWithAccessibilityScore(double wheelchairMaxSlope)
+  implements ItineraryDecorator {
   public static Float compute(List<Leg> legs) {
     return legs
       .stream()
@@ -43,14 +44,14 @@ public static float compute(ScheduledTransitLeg leg) {
     var values = List.of(trip, fromStop, toStop);
     var sum = (float) values
       .stream()
-      .mapToDouble(AccessibilityScoreFilter::accessibilityScore)
+      .mapToDouble(DecorateWithAccessibilityScore::accessibilityScore)
       .sum();
     return sum / values.size();
   }
 
   @Override
-  public List<Itinerary> filter(List<Itinerary> itineraries) {
-    return itineraries.stream().map(this::addAccessibilityScore).toList();
+  public void decorate(Itinerary itinerary) {
+    addAccessibilityScore(itinerary);
   }
 
   private static double accessibilityScore(Accessibility wheelchair) {