diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreator.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreator.java index 3980c65071d..2e612ae3f7f 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreator.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreator.java @@ -13,12 +13,10 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.function.Predicate; import org.opentripplanner.routing.algorithm.raptoradapter.transit.RaptorTransitData; import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripPatternForDate; import org.opentripplanner.transit.model.network.RoutingTripPattern; import org.opentripplanner.transit.model.network.grouppriority.TransitGroupPriorityService; -import org.opentripplanner.transit.model.timetable.TripTimes; import org.opentripplanner.utils.time.DurationUtils; import org.opentripplanner.utils.time.ServiceDateUtils; import org.slf4j.Logger; @@ -184,24 +182,19 @@ private static List filterActiveTripPatterns( // and any previous day, while on subsequent search days we only want to add the // TripPatternForDate objects that start on that particular day. This is to prevent duplicates. // This was previously a stream, but was unrolled for improved performance. - - Predicate tripTimesWithSubmodesPredicate = tripTimes -> - filter.tripTimesPredicate(tripTimes, filter.hasSubModeFilters()); - Predicate tripTimesWithoutSubmodesPredicate = tripTimes -> - filter.tripTimesPredicate(tripTimes, false); Collection tripPatternsForDate = raptorTransitData.getTripPatternsForRunningDate(date); + List result = new ArrayList<>(tripPatternsForDate.size()); for (TripPatternForDate p : tripPatternsForDate) { if (firstDay || p.getStartOfRunningPeriod().equals(date)) { - if (filter.tripPatternPredicate(p)) { - var tripTimesPredicate = p.getTripPattern().getPattern().getContainsMultipleModes() - ? tripTimesWithSubmodesPredicate - : tripTimesWithoutSubmodesPredicate; - TripPatternForDate tripPatternForDate = p.newWithFilteredTripTimes(tripTimesPredicate); - if (tripPatternForDate != null) { - result.add(tripPatternForDate); - } + var tripTimesFilter = filter.createTripFilter(p.getTripPattern().getPattern()); + if (tripTimesFilter == null) { + continue; + } + TripPatternForDate tripPatternForDate = p.newWithFilteredTripTimes(tripTimesFilter); + if (tripPatternForDate != null) { + result.add(tripPatternForDate); } } } diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RouteRequestTransitDataProviderFilter.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RouteRequestTransitDataProviderFilter.java index 11c365a4635..3e9da3d93b7 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RouteRequestTransitDataProviderFilter.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RouteRequestTransitDataProviderFilter.java @@ -3,8 +3,9 @@ import java.util.BitSet; import java.util.List; import java.util.Set; +import java.util.function.Predicate; +import javax.annotation.Nullable; import org.opentripplanner.model.PickDrop; -import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripPatternForDate; import org.opentripplanner.routing.api.request.RouteRequest; import org.opentripplanner.routing.api.request.StreetMode; import org.opentripplanner.routing.api.request.preference.WheelchairPreferences; @@ -14,6 +15,7 @@ import org.opentripplanner.transit.model.network.BikeAccess; import org.opentripplanner.transit.model.network.CarAccess; import org.opentripplanner.transit.model.network.RoutingTripPattern; +import org.opentripplanner.transit.model.network.TripPattern; import org.opentripplanner.transit.model.timetable.Trip; import org.opentripplanner.transit.model.timetable.TripTimes; @@ -76,11 +78,6 @@ public RouteRequestTransitDataProviderFilter( this.hasSubModeFilters = filters.stream().anyMatch(TransitFilter::isSubModePredicate); } - @Override - public boolean hasSubModeFilters() { - return hasSubModeFilters; - } - public static BikeAccess bikeAccessForTrip(Trip trip) { if (trip.getBikesAllowed() != BikeAccess.UNKNOWN) { return trip.getBikesAllowed(); @@ -90,17 +87,18 @@ public static BikeAccess bikeAccessForTrip(Trip trip) { } @Override - public boolean tripPatternPredicate(TripPatternForDate tripPatternForDate) { + @Nullable + public Predicate createTripFilter(TripPattern tripPattern) { for (TransitFilter filter : filters) { - if (filter.matchTripPattern(tripPatternForDate.getTripPattern().getPattern())) { - return true; + if (filter.matchTripPattern(tripPattern)) { + var applyTripTimesFilters = hasSubModeFilters && tripPattern.getContainsMultipleModes(); + return tripTimes -> tripTimesPredicate(tripTimes, applyTripTimesFilters); } } - return false; + return null; } - @Override - public boolean tripTimesPredicate(TripTimes tripTimes, boolean withFilters) { + private boolean tripTimesPredicate(TripTimes tripTimes, boolean applyTripTimesFilters) { final Trip trip = tripTimes.getTrip(); if (requireBikesAllowed && bikeAccessForTrip(trip) != BikeAccess.ALLOWED) { @@ -139,7 +137,7 @@ public boolean tripTimesPredicate(TripTimes tripTimes, boolean withFilters) { // Trip has to match with at least one predicate in order to be included in search. We only have // to this if we have mode specific filters, and not all trips on the pattern have the same // mode, since that's the only thing that is trip specific - if (withFilters) { + if (applyTripTimesFilters) { for (TransitFilter f : filters) { if (f.matchTripTimes(tripTimes)) { return true; diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/TransitDataProviderFilter.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/TransitDataProviderFilter.java index e341f5276b3..2441fd69c0a 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/TransitDataProviderFilter.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/TransitDataProviderFilter.java @@ -1,9 +1,12 @@ package org.opentripplanner.routing.algorithm.raptoradapter.transit.request; import java.util.BitSet; +import java.util.function.Predicate; +import javax.annotation.Nullable; import org.opentripplanner.routing.algorithm.raptoradapter.transit.RaptorTransitData; import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripPatternForDate; import org.opentripplanner.transit.model.network.RoutingTripPattern; +import org.opentripplanner.transit.model.network.TripPattern; import org.opentripplanner.transit.model.timetable.TripTimes; /** @@ -11,17 +14,19 @@ * when constructing a {@link RaptorRoutingRequestTransitData} for a request. *

* {@link TripPatternForDate} and {@link TripTimes} are filtered based on the request parameters to - * only included components which are allowed by the request. Such filters may included bike or + * only included components which are allowed by the request. Such filters may include bike or * wheelchair accessibility, banned routes and transit modes. * * @see RouteRequestTransitDataProviderFilter */ public interface TransitDataProviderFilter { - boolean tripPatternPredicate(TripPatternForDate tripPatternForDate); - - boolean hasSubModeFilters(); - - boolean tripTimesPredicate(TripTimes tripTimes, boolean withFilters); + /** + * For performance reasons filtering is done in a two-step process. First you apply the pattern. If it doesn't match you + * get a null value. If it does match you get a Predicate<TripTimes> that you use to match a TripTimes + * object. + */ + @Nullable + Predicate createTripFilter(TripPattern tripPattern); /** * Check if boarding/alighting is possible at each stop. If the values differ from the default diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/request/filter/SelectRequest.java b/application/src/main/java/org/opentripplanner/routing/api/request/request/filter/SelectRequest.java index 9ce7b4ce717..5c226a616c9 100644 --- a/application/src/main/java/org/opentripplanner/routing/api/request/request/filter/SelectRequest.java +++ b/application/src/main/java/org/opentripplanner/routing/api/request/request/filter/SelectRequest.java @@ -38,37 +38,19 @@ public SelectRequest(Builder builder) { this.routes = builder.routes; } - public boolean matches(TripPattern tripPattern) { - if ( - // If the pattern contains multiple modes, we will do the filtering in - // SelectRequest.matches(TripTimes) - !tripPattern.getContainsMultipleModes() && - this.transportModeFilter != null && - !this.transportModeFilter.match(tripPattern.getMode(), tripPattern.getNetexSubmode()) - ) { - return false; - } - - if (!agencies.isEmpty() && !agencies.contains(tripPattern.getRoute().getAgency().getId())) { - return false; - } - - if (!routes.isEmpty() && !routes.contains(tripPattern.getRoute().getId())) { - return false; - } - - if (!groupOfRoutes.isEmpty()) { - var ids = new ArrayList(); - for (var gor : tripPattern.getRoute().getGroupsOfRoutes()) { - ids.add(gor.getId()); - } - - if (Collections.disjoint(groupOfRoutes, ids)) { - return false; - } - } + /** + * Will return false if the pattern doesn't match the filter and true if it matches or needs to + * look at the Trip level to be sure decide. + */ + public boolean matchesPatternSelect(TripPattern tripPattern) { + return matchesPattern(tripPattern, true); + } - return true; + /** + * Will return true if the pattern matches the filter and false if it doesn't match or might not match. + */ + public boolean matchesPatternNot(TripPattern tripPattern) { + return matchesPattern(tripPattern, false); } /** @@ -119,6 +101,40 @@ public List routes() { return routes; } + private boolean matchesPattern(TripPattern tripPattern, boolean maybeValue) { + if (!agencies.isEmpty() && !agencies.contains(tripPattern.getRoute().getAgency().getId())) { + return false; + } + + if (!routes.isEmpty() && !routes.contains(tripPattern.getRoute().getId())) { + return false; + } + + if (!groupOfRoutes.isEmpty()) { + var ids = new ArrayList(); + for (var gor : tripPattern.getRoute().getGroupsOfRoutes()) { + ids.add(gor.getId()); + } + + if (Collections.disjoint(groupOfRoutes, ids)) { + return false; + } + } + + if (this.transportModeFilter != null) { + // If the pattern contains multiple modes, we will do the filtering in + // SelectRequest.matches(TripTimes) + if (tripPattern.getContainsMultipleModes()) { + return maybeValue; + } + if (!this.transportModeFilter.match(tripPattern.getMode(), tripPattern.getNetexSubmode())) { + return false; + } + } + + return true; + } + private String transportModesToString() { if (transportModes == null) { return null; diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/request/filter/TransitFilterRequest.java b/application/src/main/java/org/opentripplanner/routing/api/request/request/filter/TransitFilterRequest.java index 6385960e8ff..28228d6c106 100644 --- a/application/src/main/java/org/opentripplanner/routing/api/request/request/filter/TransitFilterRequest.java +++ b/application/src/main/java/org/opentripplanner/routing/api/request/request/filter/TransitFilterRequest.java @@ -66,7 +66,7 @@ public boolean matchTripPattern(TripPattern tripPattern) { if (select.length != 0) { var anyMatch = false; for (SelectRequest s : select) { - if (s.matches(tripPattern)) { + if (s.matchesPatternSelect(tripPattern)) { anyMatch = true; break; } @@ -77,7 +77,7 @@ public boolean matchTripPattern(TripPattern tripPattern) { } for (SelectRequest s : not) { - if (s.matches(tripPattern)) { + if (s.matchesPatternNot(tripPattern)) { return false; } } diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreatorTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreatorTest.java index 894eb0f6ab9..5a80e4a5a78 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreatorTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreatorTest.java @@ -6,12 +6,12 @@ import java.time.LocalDate; import java.time.ZonedDateTime; import java.util.ArrayList; -import java.util.BitSet; import java.util.List; import org.junit.jupiter.api.Test; import org.opentripplanner._support.time.ZoneIds; import org.opentripplanner.model.StopTime; import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripPatternForDate; +import org.opentripplanner.routing.api.request.RouteRequest; import org.opentripplanner.transit.model._data.TimetableRepositoryForTest; import org.opentripplanner.transit.model.basic.TransitMode; import org.opentripplanner.transit.model.framework.FeedScopedId; @@ -61,12 +61,14 @@ public void testMergeTripPatterns() { tripPatternsForDates.add(new TripPatternForDate(tripPattern1, tripTimes, List.of(), third)); tripPatternsForDates.add(new TripPatternForDate(tripPattern3, tripTimes, List.of(), third)); + var noOpFilter = new RouteRequestTransitDataProviderFilter(RouteRequest.defaultValue()); + // Patterns containing trip schedules for all 3 days. Trip schedules for later days are offset // in time when requested. List combinedTripPatterns = RaptorRoutingRequestTransitDataCreator.merge( startOfTime, tripPatternsForDates, - new TestTransitDataProviderFilter(), + noOpFilter, TransitGroupPriorityService.empty() ); @@ -123,34 +125,4 @@ private static RoutingTripPattern createTripPattern(FeedScopedId id) { .build() .getRoutingTripPattern(); } - - /** - * Utility class that does nothing, used just to avoid null value on filter - */ - private static class TestTransitDataProviderFilter implements TransitDataProviderFilter { - - @Override - public boolean tripPatternPredicate(TripPatternForDate tripPatternForDate) { - return false; - } - - @Override - public boolean tripTimesPredicate(TripTimes tripTimes, boolean withFilters) { - return false; - } - - @Override - public boolean hasSubModeFilters() { - return false; - } - - @Override - public BitSet filterAvailableStops( - RoutingTripPattern tripPattern, - BitSet boardingPossible, - BoardAlight boardAlight - ) { - return boardingPossible; - } - } } diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RouteRequestTransitDataProviderFilterTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RouteRequestTransitDataProviderFilterTest.java index fdddb4276e6..51d2661d851 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RouteRequestTransitDataProviderFilterTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RouteRequestTransitDataProviderFilterTest.java @@ -1,6 +1,5 @@ package org.opentripplanner.routing.algorithm.raptoradapter.transit.request; -import static com.google.common.truth.Truth.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -11,6 +10,7 @@ import java.util.List; import java.util.Set; import java.util.stream.Stream; +import org.junit.Ignore; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -42,8 +42,6 @@ import org.opentripplanner.transit.model.network.StopPattern; import org.opentripplanner.transit.model.network.TripPattern; import org.opentripplanner.transit.model.site.RegularStop; -import org.opentripplanner.transit.model.timetable.RealTimeTripTimes; -import org.opentripplanner.transit.model.timetable.ScheduledTripTimes; import org.opentripplanner.transit.model.timetable.Trip; import org.opentripplanner.transit.model.timetable.TripAlteration; import org.opentripplanner.transit.model.timetable.TripBuilder; @@ -212,7 +210,8 @@ void notFilteringExpectedTripPatternForDateTest() { filterForMode(TransitMode.BUS) ); - boolean valid = filter.tripPatternPredicate(tripPatternForDate); + boolean valid = + filter.createTripFilter(tripPatternForDate.getTripPattern().getPattern()) != null; assertTrue(valid); } @@ -236,14 +235,15 @@ void bannedRouteFilteringTest() { ) ); - boolean valid = filter.tripPatternPredicate(tripPatternForDate); + boolean valid = + filter.createTripFilter(tripPatternForDate.getTripPattern().getPattern()) != null; assertFalse(valid); } @Test void bannedTripFilteringTest() { - TripTimes tripTimes = createTestTripTimes( + var patternAndTimes = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -265,9 +265,7 @@ void bannedTripFilteringTest() { filterForMode(TransitMode.BUS) ); - boolean valid = filter.tripTimesPredicate(tripTimes, true); - - assertFalse(valid); + assertFalse(validate(filter, patternAndTimes)); } /** @@ -276,10 +274,10 @@ void bannedTripFilteringTest() { */ @Test void matchModeFilterAndBannedAgencyFilter() { - TripTimes tripTimesMatchingSubModeMatchingAgency = createTestTripTimesWithSubmode( + var ptMatchingSubModeMatchingAgency = createPatternAndTimesWithSubmode( TransmodelTransportSubmode.UNKNOWN.getValue() ); - TripTimes tripTimesFailingSubModeMatchingAgency = createTestTripTimesWithSubmode( + var ptFailingSubModeMatchingAgency = createPatternAndTimesWithSubmode( TransmodelTransportSubmode.LOCAL.getValue() ); @@ -302,8 +300,8 @@ void matchModeFilterAndBannedAgencyFilter() { ) ); - assertTrue(filter.tripTimesPredicate(tripTimesMatchingSubModeMatchingAgency, true)); - assertTrue(filter.tripTimesPredicate(tripTimesFailingSubModeMatchingAgency, true)); + assertTrue(validate(filter, ptMatchingSubModeMatchingAgency)); + assertTrue(validate(filter, ptFailingSubModeMatchingAgency)); } /** @@ -312,10 +310,10 @@ void matchModeFilterAndBannedAgencyFilter() { */ @Test void matchCombinedModesAndBannedAgencyFilter() { - TripTimes tripTimesMatchingSubModeMatchingAgency = createTestTripTimesWithSubmode( + var ptMatchingSubModeMatchingAgency = createPatternAndTimesWithSubmode( TransmodelTransportSubmode.UNKNOWN.getValue() ); - TripTimes tripTimesFailingSubModeMatchingAgency = createTestTripTimesWithSubmode( + var ptFailingSubModeMatchingAgency = createPatternAndTimesWithSubmode( TransmodelTransportSubmode.LOCAL.getValue() ); @@ -338,16 +336,16 @@ void matchCombinedModesAndBannedAgencyFilter() { ) ); - assertTrue(filter.tripTimesPredicate(tripTimesMatchingSubModeMatchingAgency, true)); - assertFalse(filter.tripTimesPredicate(tripTimesFailingSubModeMatchingAgency, true)); + assertTrue(validate(filter, ptMatchingSubModeMatchingAgency)); + assertFalse(validate(filter, ptFailingSubModeMatchingAgency)); } @Test void matchSelectedAgencyExcludedSubMode() { - TripTimes tripTimesMatchingSubModeMatchingAgency = createTestTripTimesWithSubmode( + var ptMatchingSubModeMatchingAgency = createPatternAndTimesWithSubmode( TransmodelTransportSubmode.UNKNOWN.getValue() ); - TripTimes tripTimesFailingSubModeMatchingAgency = createTestTripTimesWithSubmode( + var ptFailingSubModeMatchingAgency = createPatternAndTimesWithSubmode( TransmodelTransportSubmode.LOCAL.getValue() ); @@ -382,13 +380,13 @@ void matchSelectedAgencyExcludedSubMode() { ) ); - assertTrue(filter.tripTimesPredicate(tripTimesMatchingSubModeMatchingAgency, true)); - assertFalse(filter.tripTimesPredicate(tripTimesFailingSubModeMatchingAgency, true)); + assertTrue(validate(filter, ptMatchingSubModeMatchingAgency)); + assertFalse(validate(filter, ptFailingSubModeMatchingAgency)); } @Test void transitModeFilteringTest() { - TripTimes tripTimes = createTestTripTimes( + var patternTimes = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -404,18 +402,64 @@ void transitModeFilteringTest() { final var LOCAL_BUS = SubMode.of(TransmodelTransportSubmode.LOCAL_BUS.getValue()); final var REGIONAL_BUS = SubMode.of(TransmodelTransportSubmode.REGIONAL_BUS.getValue()); - assertFalse( - validateModesOnTripTimes(List.of(new MainAndSubMode(BUS, REGIONAL_BUS)), tripTimes) + assertFalse(validateModes(List.of(new MainAndSubMode(BUS, REGIONAL_BUS)), patternTimes)); + assertFalse(validateModes(List.of(new MainAndSubMode(RAIL, LOCAL_BUS)), patternTimes)); + + assertTrue(validateModes(List.of(new MainAndSubMode(BUS)), patternTimes)); + assertTrue(validateModes(List.of(new MainAndSubMode(BUS, LOCAL_BUS)), patternTimes)); + } + + @Ignore + void selectCombinationTest() { + // This test illustrates a bug in the filtering logic. + // We have a filter that corresponds to this boolean expression: + // (agency == AGENCY && mode == BUS) || (agency == OTHER_AGENCY && mode == RAIL) + // This results in a match with a trip { agency: OTHER_AGENCY, mode: BUS } even though it + // shouldn't. + var transitFilter = TransitFilterRequest.of() + .addSelect( + SelectRequest.of() + .withAgencies(List.of(TimetableRepositoryForTest.OTHER_AGENCY.getId())) + .withTransportModes(List.of(new MainAndSubMode(TransitMode.BUS))) + .build() + ) + .addSelect( + SelectRequest.of() + .withAgencies(List.of(TimetableRepositoryForTest.AGENCY.getId())) + .withTransportModes(List.of(new MainAndSubMode(TransitMode.RAIL))) + .build() + ) + .build(); + + var filter = new RouteRequestTransitDataProviderFilter( + false, + false, + false, + DEFAULT_ACCESSIBILITY, + false, + false, + Set.of(), + List.of(transitFilter) ); - assertFalse(validateModesOnTripTimes(List.of(new MainAndSubMode(RAIL, LOCAL_BUS)), tripTimes)); - assertTrue(validateModesOnTripTimes(List.of(new MainAndSubMode(BUS)), tripTimes)); - assertTrue(validateModesOnTripTimes(List.of(new MainAndSubMode(BUS, LOCAL_BUS)), tripTimes)); + var patternTimes = createPatternAndTimes( + TRIP_ID, + ROUTE, + BikeAccess.NOT_ALLOWED, + CarAccess.NOT_ALLOWED, + TransitMode.BUS, + "localBus", + Accessibility.NOT_POSSIBLE, + null, + true + ); + + assertFalse(validate(filter, patternTimes)); } @Test void notFilteringExpectedTripTimesTest() { - TripTimes tripTimes = createTestTripTimes( + var pt = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -437,14 +481,14 @@ void notFilteringExpectedTripTimesTest() { filterForMode(TransitMode.BUS) ); - boolean valid = filter.tripTimesPredicate(tripTimes, true); + boolean valid = validate(filter, pt); assertTrue(valid); } @Test void bikesAllowedFilteringTest() { - TripTimes tripTimes = createTestTripTimes( + var patternTimes = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -466,14 +510,14 @@ void bikesAllowedFilteringTest() { List.of(AllowAllTransitFilter.of()) ); - boolean valid = filter.tripTimesPredicate(tripTimes, true); + boolean valid = validate(filter, patternTimes); assertFalse(valid); } @Test void carsAllowedFilteringTest() { - TripTimes tripTimes = createTestTripTimes( + var patternTimes = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -495,14 +539,14 @@ void carsAllowedFilteringTest() { List.of(AllowAllTransitFilter.of()) ); - boolean valid = filter.tripTimesPredicate(tripTimes, true); + boolean valid = validate(filter, patternTimes); assertFalse(valid); } @Test void removeInaccessibleTrip() { - TripTimes tripTimes = createTestTripTimes( + var patternTimes = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -524,14 +568,14 @@ void removeInaccessibleTrip() { List.of(AllowAllTransitFilter.of()) ); - boolean valid = filter.tripTimesPredicate(tripTimes, true); + boolean valid = validate(filter, patternTimes); assertFalse(valid); } @Test void keepAccessibleTrip() { - TripTimes wheelchairAccessibleTrip = createTestTripTimes( + var wheelchairAccessibleTrip = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -553,14 +597,14 @@ void keepAccessibleTrip() { filterForMode(TransitMode.BUS) ); - boolean valid = filter.tripTimesPredicate(wheelchairAccessibleTrip, true); + boolean valid = validate(filter, wheelchairAccessibleTrip); assertTrue(valid); } @Test void keepRealTimeAccessibleTrip() { - var builder = createTestTripTimes( + var patternAndTimes = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -569,7 +613,8 @@ void keepRealTimeAccessibleTrip() { null, Accessibility.NOT_POSSIBLE, TripAlteration.PLANNED - ).createRealTimeFromScheduledTimes(); + ); + var builder = patternAndTimes.tripTimes().createRealTimeFromScheduledTimes(); var filter = new RouteRequestTransitDataProviderFilter( false, @@ -582,16 +627,16 @@ void keepRealTimeAccessibleTrip() { filterForMode(TransitMode.BUS) ); - assertFalse(filter.tripTimesPredicate(builder.build(), true)); + assertFalse(validate(filter, patternAndTimes.withTimes(builder.build()))); builder.withWheelchairAccessibility(Accessibility.POSSIBLE); - assertTrue(filter.tripTimesPredicate(builder.build(), true)); + assertTrue(validate(filter, patternAndTimes.withTimes(builder.build()))); } @Test void includePlannedCancellationsTest() { - TripTimes tripTimesWithCancellation = createTestTripTimes( + var patternTimesWithCancellation = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -601,7 +646,7 @@ void includePlannedCancellationsTest() { Accessibility.NOT_POSSIBLE, TripAlteration.CANCELLATION ); - TripTimes tripTimesWithReplaced = createTestTripTimes( + var patternTimesWithReplaced = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -625,12 +670,12 @@ void includePlannedCancellationsTest() { ); // When - boolean valid1 = filter1.tripTimesPredicate(tripTimesWithCancellation, true); + boolean valid1 = validate(filter1, patternTimesWithCancellation); // Then assertTrue(valid1); // When - boolean valid2 = filter1.tripTimesPredicate(tripTimesWithReplaced, true); + boolean valid2 = validate(filter1, patternTimesWithReplaced); // Then assertTrue(valid2); @@ -647,19 +692,19 @@ void includePlannedCancellationsTest() { ); // When - boolean valid3 = filter2.tripTimesPredicate(tripTimesWithCancellation, true); + boolean valid3 = validate(filter2, patternTimesWithCancellation); // Then assertFalse(valid3); // When - boolean valid4 = filter2.tripTimesPredicate(tripTimesWithReplaced, true); + boolean valid4 = validate(filter2, patternTimesWithReplaced); // Then assertFalse(valid4); } @Test void includeRealtimeCancellationsTest() { - TripTimes tripTimes = createTestTripTimes( + var patternTimes = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -670,20 +715,14 @@ void includeRealtimeCancellationsTest() { TripAlteration.PLANNED ); - RealTimeTripTimes tripTimesWithCancellation = createTestTripTimes( - TRIP_ID, - ROUTE, - BikeAccess.NOT_ALLOWED, - CarAccess.NOT_ALLOWED, - TransitMode.BUS, - null, - Accessibility.NOT_POSSIBLE, - TripAlteration.PLANNED - ) + var cancelled = patternTimes + .tripTimes() .createRealTimeFromScheduledTimes() .cancelTrip() .build(); + var patternTimesWithCancellation = patternTimes.withTimes(cancelled); + // Given var filter1 = new RouteRequestTransitDataProviderFilter( false, @@ -697,12 +736,12 @@ void includeRealtimeCancellationsTest() { ); // When - boolean valid1 = filter1.tripTimesPredicate(tripTimes, true); + boolean valid1 = validate(filter1, patternTimes); // Then assertTrue(valid1); // When - boolean valid2 = filter1.tripTimesPredicate(tripTimesWithCancellation, true); + boolean valid2 = validate(filter1, patternTimesWithCancellation); // Then assertTrue(valid2); @@ -719,12 +758,12 @@ void includeRealtimeCancellationsTest() { ); // When - boolean valid3 = filter2.tripTimesPredicate(tripTimes, true); + boolean valid3 = validate(filter2, patternTimes); // Then assertTrue(valid3); // When - boolean valid4 = filter2.tripTimesPredicate(tripTimesWithCancellation, true); + boolean valid4 = validate(filter2, patternTimesWithCancellation); // Then assertFalse(valid4); } @@ -767,7 +806,7 @@ void testBikesAllowed() { @Test void testCarsAllowed() { - TripTimes tripTimesCarsAllowed = createTestTripTimes( + var patternTimesCarsAllowed = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.UNKNOWN, @@ -778,7 +817,7 @@ void testCarsAllowed() { TripAlteration.PLANNED ); - TripTimes tripTimesCarsNotAllowed = createTestTripTimes( + var patternTimesCarsNotAllowed = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.UNKNOWN, @@ -789,7 +828,7 @@ void testCarsAllowed() { TripAlteration.PLANNED ); - TripTimes tripTimesCarsUnknown = createTestTripTimes( + var patternTimesCarsUnknown = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.UNKNOWN, @@ -811,14 +850,14 @@ void testCarsAllowed() { List.of(AllowAllTransitFilter.of()) ); - assertThat(filter.tripTimesPredicate(tripTimesCarsAllowed, false)).isTrue(); - assertThat(filter.tripTimesPredicate(tripTimesCarsNotAllowed, false)).isFalse(); - assertThat(filter.tripTimesPredicate(tripTimesCarsUnknown, false)).isFalse(); + assertTrue(validate(filter, patternTimesCarsAllowed)); + assertFalse(validate(filter, patternTimesCarsNotAllowed)); + assertFalse(validate(filter, patternTimesCarsUnknown)); } @Test void multipleFilteringTest() { - TripTimes matchingTripTimes = createTestTripTimes( + var matchingPatternTimes = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.ALLOWED, @@ -828,7 +867,7 @@ void multipleFilteringTest() { Accessibility.POSSIBLE, TripAlteration.PLANNED ); - TripTimes failingTripTimes1 = createTestTripTimes( + var failingPatternTimes1 = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.ALLOWED, @@ -838,7 +877,7 @@ void multipleFilteringTest() { Accessibility.POSSIBLE, TripAlteration.PLANNED ); - TripTimes failingTripTimes2 = createTestTripTimes( + var failingPatternTimes2 = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -848,7 +887,7 @@ void multipleFilteringTest() { Accessibility.POSSIBLE, TripAlteration.CANCELLATION ); - TripTimes failingTripTimes3 = createTestTripTimes( + var failingPatternTimes3 = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -858,7 +897,7 @@ void multipleFilteringTest() { Accessibility.NOT_POSSIBLE, TripAlteration.CANCELLATION ); - TripTimes failingTripTimes4 = createTestTripTimes( + var failingPatternTimes4 = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.ALLOWED, @@ -868,7 +907,7 @@ void multipleFilteringTest() { Accessibility.NOT_POSSIBLE, TripAlteration.PLANNED ); - TripTimes failingTripTimes5 = createTestTripTimes( + var failingPatternTimes5 = createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.ALLOWED, @@ -890,18 +929,18 @@ void multipleFilteringTest() { filterForMode(TransitMode.BUS) ); - assertTrue(filter.tripTimesPredicate(matchingTripTimes, true)); + assertTrue(validate(filter, matchingPatternTimes)); - assertFalse(filter.tripTimesPredicate(failingTripTimes1, true)); - assertFalse(filter.tripTimesPredicate(failingTripTimes2, true)); - assertFalse(filter.tripTimesPredicate(failingTripTimes3, true)); - assertFalse(filter.tripTimesPredicate(failingTripTimes4, true)); - assertFalse(filter.tripTimesPredicate(failingTripTimes5, true)); + assertFalse(validate(filter, failingPatternTimes1)); + assertFalse(validate(filter, failingPatternTimes2)); + assertFalse(validate(filter, failingPatternTimes3)); + assertFalse(validate(filter, failingPatternTimes4)); + assertFalse(validate(filter, failingPatternTimes5)); } - private boolean validateModesOnTripTimes( + private boolean validateModes( Collection allowedModes, - TripTimes tripTimes + PatternAndTimes patternAndTimes ) { var filter = new RouteRequestTransitDataProviderFilter( false, @@ -914,7 +953,11 @@ private boolean validateModesOnTripTimes( filterForModes(allowedModes) ); - return filter.tripTimesPredicate(tripTimes, true); + var timesFilter = filter.createTripFilter(patternAndTimes.pattern()); + if (timesFilter == null) { + return false; + } + return timesFilter.test(patternAndTimes.tripTimes()); } private TripPatternForDate createTestTripPatternForDate() { @@ -974,7 +1017,13 @@ private List combinedFilterForModesAndBannedAgencies( ); } - private ScheduledTripTimes createTestTripTimes( + private record PatternAndTimes(TripPattern pattern, TripTimes tripTimes) { + public PatternAndTimes withTimes(TripTimes tripTimes) { + return new PatternAndTimes(pattern, tripTimes); + } + } + + private PatternAndTimes createPatternAndTimes( FeedScopedId tripId, Route route, BikeAccess bikeAccess, @@ -983,6 +1032,30 @@ private ScheduledTripTimes createTestTripTimes( String submode, Accessibility wheelchairBoarding, TripAlteration tripAlteration + ) { + return createPatternAndTimes( + tripId, + route, + bikeAccess, + carAccess, + mode, + submode, + wheelchairBoarding, + tripAlteration, + false + ); + } + + private PatternAndTimes createPatternAndTimes( + FeedScopedId tripId, + Route route, + BikeAccess bikeAccess, + CarAccess carAccess, + TransitMode mode, + String submode, + Accessibility wheelchairBoarding, + TripAlteration tripAlteration, + boolean multipleSubmodes ) { Trip trip = Trip.of(tripId) .withRoute(route) @@ -999,12 +1072,28 @@ private ScheduledTripTimes createTestTripTimes( stopTime.setArrivalTime(60); stopTime.setDepartureTime(60); stopTime.setStopSequence(0); + stopTime.setStop(STOP_FOR_TEST); - return TripTimesFactory.tripTimes(trip, List.of(stopTime), new Deduplicator()); + StopPattern stopPattern = new StopPattern(List.of(stopTime)); + var tripPattern = TripPattern.of(TimetableRepositoryForTest.id("P1")) + .withRoute(route) + .withStopPattern(stopPattern) + .withMode(mode) + .withNetexSubmode(SubMode.of(submode)) + .withContainsMultipleModes(multipleSubmodes) + .build(); + + TripTimes tripTimes = TripTimesFactory.tripTimes( + trip, + List.of(new StopTime()), + new Deduplicator() + ); + + return new PatternAndTimes(tripPattern, tripTimes); } - private TripTimes createTestTripTimesWithSubmode(String submode) { - return createTestTripTimes( + private PatternAndTimes createPatternAndTimesWithSubmode(String submode) { + return createPatternAndTimes( TRIP_ID, ROUTE, BikeAccess.NOT_ALLOWED, @@ -1012,7 +1101,8 @@ private TripTimes createTestTripTimesWithSubmode(String submode) { TransitMode.BUS, submode, Accessibility.NOT_POSSIBLE, - null + null, + true ); } @@ -1035,4 +1125,15 @@ private static StopTime getStopTime(String idAndName, PickDrop scheduled) { stopTime1.setPickupType(scheduled); return stopTime1; } + + private boolean validate( + RouteRequestTransitDataProviderFilter filter, + PatternAndTimes patternAndTimes + ) { + var tripTimesFilter = filter.createTripFilter(patternAndTimes.pattern()); + if (tripTimesFilter == null) { + return false; + } + return tripTimesFilter.test(patternAndTimes.tripTimes()); + } }