Skip to content

Commit

Permalink
Add a matcher API for filters in the transit service used for service…
Browse files Browse the repository at this point in the history
…Journey lookup (opentripplanner#6207)

* Removes some unused functions from TransitService and TripOnServiceDate

* Adds filtering logic to the serviceJourneys Transmodel GraphQL query.

* Fixes a typo that carried over from a documentation copy.

* Resolving merge conflict.

* Addresses comments in review - updates naming to reflect clear division between netex and OTP internals.

* Applies formatting.

* Fixes formatting and some residual merge issues.

* Addresses comments in PR.

Importantly this creates the CriteriaCollection type which allows for checking features about collections of criteria independently of the collection type.

* Applies results from prettier to CriteriaCollectionTest.

* Addresses comments from review.

Adds formatting to API and md documentation, and adds an exception thrown for invalid calls to values in CriteriaCollection.

* Renames methods in CriteriaCollection as discussed in code review.

* Addresses comments from code review.

* Changes name of CriteriaCollection to FilterValueCollection.
* Adds documentation to clarify usage and intent of the FilterValueCollection.
* Fixes potential null values in request builders and exception throwing in get() method.

* Addresses comments from code review.

* Changes name of FilterValueCollection to FilterValues.
* Creates the RequiredFilterValues and FilterValuesEmptyIsEverything subclasses of the now abstract FilterValues class.

* Apply suggestions from code review

Co-authored-by: Thomas Gran <[email protected]>

* Addresses comments from code review.

* Makes a method name change.
* asserts that exception strings in the RequiredFilterValuesTest are as expected.

* Fixes some issues left over from resolving merge conflicts.

---------

Co-authored-by: Thomas Gran <[email protected]>
  • Loading branch information
eibakke and t2gran authored Nov 28, 2024
1 parent ace254e commit db171d2
Show file tree
Hide file tree
Showing 22 changed files with 783 additions and 184 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@
import org.opentripplanner.routing.graphfinder.PlaceAtDistance;
import org.opentripplanner.routing.graphfinder.PlaceType;
import org.opentripplanner.service.vehiclerental.model.VehicleRentalPlace;
import org.opentripplanner.transit.api.model.FilterValues;
import org.opentripplanner.transit.api.request.TripRequest;
import org.opentripplanner.transit.model.basic.TransitMode;
import org.opentripplanner.transit.model.framework.FeedScopedId;
import org.opentripplanner.transit.model.network.Route;
Expand Down Expand Up @@ -1261,76 +1263,68 @@ private GraphQLSchema create() {
GraphQLFieldDefinition
.newFieldDefinition()
.name("serviceJourneys")
.description("Get all service journeys")
.description("Get all _service journeys_")
.withDirective(TransmodelDirectives.TIMING_DATA)
.type(new GraphQLNonNull(new GraphQLList(serviceJourneyType)))
.argument(
GraphQLArgument
.newArgument()
.name("lines")
.description("Set of ids of lines to fetch serviceJourneys for.")
.description("Set of ids of _lines_ to fetch _service journeys_ for.")
.type(new GraphQLList(Scalars.GraphQLID))
.build()
)
.argument(
GraphQLArgument
.newArgument()
.name("privateCodes")
.description("Set of ids of private codes to fetch serviceJourneys for.")
.description("Set of ids of _private codes_ to fetch _service journeys_ for.")
.type(new GraphQLList(Scalars.GraphQLString))
.build()
)
.argument(
GraphQLArgument
.newArgument()
.name("activeDates")
.description("Set of ids of active dates to fetch serviceJourneys for.")
.description("Set of _operating days_ to fetch _service journeys_ for.")
.type(new GraphQLList(TransmodelScalars.DATE_SCALAR))
.build()
)
.argument(
GraphQLArgument
.newArgument()
.name("authorities")
.description("Set of ids of authorities to fetch serviceJourneys for.")
.description("Set of ids of _authorities_ to fetch _service journeys_ for.")
.type(new GraphQLList(Scalars.GraphQLString))
.build()
)
.dataFetcher(environment -> {
List<FeedScopedId> lineIds = mapIDsToDomainNullSafe(
environment.getArgumentOrDefault("lines", List.of())
var authorities = FilterValues.ofEmptyIsEverything(
"authorities",
mapIDsToDomainNullSafe(environment.getArgument("authorities"))
);
List<String> privateCodes = environment.getArgumentOrDefault("privateCodes", List.of());
List<LocalDate> activeServiceDates = environment.getArgumentOrDefault(
var lineIds = FilterValues.ofEmptyIsEverything(
"lines",
mapIDsToDomainNullSafe(environment.getArgument("lines"))
);
var privateCodes = FilterValues.ofEmptyIsEverything(
"privateCodes",
environment.<List<String>>getArgument("privateCodes")
);
var activeServiceDates = FilterValues.ofEmptyIsEverything(
"activeDates",
List.of()
environment.<List<LocalDate>>getArgument("activeDates")
);

// TODO OTP2 - Use FeedScoped ID
List<String> authorities = environment.getArgumentOrDefault("authorities", List.of());
TransitService transitService = GqlUtil.getTransitService(environment);
return transitService
.listTrips()
.stream()
.filter(t -> lineIds.isEmpty() || lineIds.contains(t.getRoute().getId()))
.filter(t ->
privateCodes.isEmpty() || privateCodes.contains(t.getNetexInternalPlanningCode())
)
.filter(t ->
authorities.isEmpty() ||
authorities.contains(t.getRoute().getAgency().getId().getId())
)
.filter(t ->
(
activeServiceDates.isEmpty() ||
transitService
.getCalendarService()
.getServiceDatesForServiceId(t.getServiceId())
.stream()
.anyMatch(activeServiceDates::contains)
)
)
.collect(Collectors.toList());
TripRequest tripRequest = TripRequest
.of()
.withAgencies(authorities)
.withRoutes(lineIds)
.withNetexInternalPlanningCodes(privateCodes)
.withServiceDates(activeServiceDates)
.build();

return GqlUtil.getTransitService(environment).getTrips(tripRequest);
})
.build()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.opentripplanner.apis.transmodel.model.EnumTypes;
import org.opentripplanner.apis.transmodel.model.framework.TransmodelScalars;
import org.opentripplanner.apis.transmodel.support.GqlUtil;
import org.opentripplanner.transit.api.model.FilterValues;
import org.opentripplanner.transit.api.request.TripOnServiceDateRequest;
import org.opentripplanner.transit.api.request.TripOnServiceDateRequestBuilder;
import org.opentripplanner.transit.model.framework.FeedScopedId;
Expand Down Expand Up @@ -94,26 +95,46 @@ public static GraphQLFieldDefinition createQuery(GraphQLOutputType datedServiceJ
)
.dataFetcher(environment -> {
// The null safety checks are not needed here - they are taken care of by the request
// object, but reuse let's use the mapping method and leave this improvement until all APIs
// object, but let's use the mapping method and leave this improvement until all APIs
// are pushing this check into the domain request.
var authorities = mapIDsToDomainNullSafe(environment.getArgument("authorities"));
var lines = mapIDsToDomainNullSafe(environment.getArgument("lines"));
var serviceJourneys = mapIDsToDomainNullSafe(environment.getArgument("serviceJourneys"));
var replacementFor = mapIDsToDomainNullSafe(environment.getArgument("replacementFor"));
var privateCodes = environment.<List<String>>getArgument("privateCodes");
var operatingDays = environment.<List<LocalDate>>getArgument("operatingDays");
var alterations = environment.<List<TripAlteration>>getArgument("alterations");
var authorities = FilterValues.ofEmptyIsEverything(
"authorities",
mapIDsToDomainNullSafe(environment.getArgument("authorities"))
);
var lines = FilterValues.ofEmptyIsEverything(
"lines",
mapIDsToDomainNullSafe(environment.getArgument("lines"))
);
var serviceJourneys = FilterValues.ofEmptyIsEverything(
"serviceJourneys",
mapIDsToDomainNullSafe(environment.getArgument("serviceJourneys"))
);
var replacementFor = FilterValues.ofEmptyIsEverything(
"replacementFor",
mapIDsToDomainNullSafe(environment.getArgument("replacementFor"))
);
var privateCodes = FilterValues.ofEmptyIsEverything(
"privateCodes",
environment.<List<String>>getArgument("privateCodes")
);
var operatingDays = FilterValues.ofRequired(
"operatingDays",
environment.<List<LocalDate>>getArgument("operatingDays")
);
var alterations = FilterValues.ofEmptyIsEverything(
"alterations",
environment.<List<TripAlteration>>getArgument("alterations")
);

TripOnServiceDateRequestBuilder tripOnServiceDateRequestBuilder = TripOnServiceDateRequest
.of()
.withOperatingDays(operatingDays)
.withAuthorities(authorities)
.withLines(lines)
.of(operatingDays)
.withAgencies(authorities)
.withRoutes(lines)
.withServiceJourneys(serviceJourneys)
.withReplacementFor(replacementFor);

tripOnServiceDateRequestBuilder =
tripOnServiceDateRequestBuilder.withPrivateCodes(privateCodes);
tripOnServiceDateRequestBuilder.withNetexInternalPlanningCodes(privateCodes);

tripOnServiceDateRequestBuilder =
tripOnServiceDateRequestBuilder.withAlterations(alterations);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package org.opentripplanner.transit.api.model;

import com.beust.jcommander.internal.Nullable;
import java.util.Collection;
import java.util.NoSuchElementException;
import org.opentripplanner.transit.model.framework.FeedScopedId;
import org.opentripplanner.transit.service.TransitService;

/**
* {@link FilterValues} is meant to be used when filtering results from {@link TransitService}.
* </p>
* This abstraction over the Collection type lets us keep filter specific functionality separate
* from interpretation of various states of a collection. For instance in which case the filter values
* should match all entities they are meant to filter.
* </p>
* @param <E> - The type of the filter values. Typically, String or {@link FeedScopedId}.
*/
public abstract class FilterValues<E> {

@Nullable
protected final Collection<E> values;

private final String name;

FilterValues(String name, @Nullable Collection<E> values) {
this.name = name;
this.values = values;
}

/**
* Returns a {@link FilterValues} that matches everything if there are no filter values.
* </p>
* @param name - The name of the filter.
* @param <E> - The type of the filter values. Typically, String or {@link FeedScopedId}.
* @param values - The {@link Collection} of filter values.
* @return FilterValues
*/
public static <E> FilterValues<E> ofEmptyIsEverything(
String name,
@Nullable Collection<E> values
) {
return new FilterValuesEmptyIsEverything<>(name, values);
}

/**
* Returns a {@link RequiredFilterValues} that throws an exception at creation time if the filter
* values is null or empty.
* </p>
* @param name - The name of the filter.
* @param <E> - The type of the filter values. Typically, String or {@link FeedScopedId}.
* @param values - The {@link Collection} of filter values.
* @return RequiredFilterValues
*/
public static <E> RequiredFilterValues<E> ofRequired(
String name,
@Nullable Collection<E> values
) {
return new RequiredFilterValues<>(name, values);
}

/**
* Returns True if the collection of filter values matches everything that it could filter. If this
* is the case, then the filter values should not be used to filter anything and filtering logic can
* safely ignore it.
* </p>
* @return boolean
*/
public abstract boolean includeEverything();

/**
* Returns the collection of filter values. If the filter values effectively don't filter anything,
* an exception is thrown.
* </p>
* @return Collection<E> - The values of the filter.
*/
public Collection<E> get() {
if (includeEverything()) {
throw new NoSuchElementException(
"Filter values for filter %s effectively don't filter, use includeEverything() before calling this method.".formatted(
name
)
);
}
return values;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package org.opentripplanner.transit.api.model;

import java.util.Collection;

/**
* {@link FilterValuesEmptyIsEverything} is a subclass of {@link FilterValues} that includes
* everything if the values are null or empty.
*/
public class FilterValuesEmptyIsEverything<E> extends FilterValues<E> {

FilterValuesEmptyIsEverything(String name, Collection<E> values) {
super(name, values);
}

@Override
public boolean includeEverything() {
return values == null || values.isEmpty();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package org.opentripplanner.transit.api.model;

import java.util.Collection;

/**
* {@link RequiredFilterValues} is a subclass of {@link FilterValues} that requires at least one
* value to be included.
*/
public class RequiredFilterValues<E> extends FilterValues<E> {

RequiredFilterValues(String name, Collection<E> values) {
super(name, values);
if (values == null || values.isEmpty()) {
throw new IllegalArgumentException("Filter %s values must not be empty.".formatted(name));
}
}

@Override
public boolean includeEverything() {
// RequiredFilterValues should never include everything. In theory the filter values could be
// exhaustive, but there is no check for that currently.
return false;
}
}
Loading

0 comments on commit db171d2

Please sign in to comment.