From dc0a6c6ea86df04daa0cc4542eb2fe55410bc536 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 18 Jun 2020 18:40:16 +0200 Subject: [PATCH] Transmodel GraphQL API - Clean code, cleanup TransmodelMappingUtil. - Rename mapping methods to mapToApi, mapToDomain... - Move `getMonoOrMultiModalStation` method to Graph where this kind of functionality should be. --- .../legacygraphqlapi/LegacyGraphQLAPI.java | 2 +- .../TransmodelGraphQLPlanner.java | 4 +- .../TransmodelIndexGraphQLSchema.java | 78 +++++++++---------- .../mapping/TransmodelMappingUtil.java | 52 ++++++------- .../opentripplanner/routing/graph/Graph.java | 16 +++- 5 files changed, 77 insertions(+), 75 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/legacygraphqlapi/LegacyGraphQLAPI.java b/src/ext/java/org/opentripplanner/ext/legacygraphqlapi/LegacyGraphQLAPI.java index c925450eb3f..a85a22326d5 100644 --- a/src/ext/java/org/opentripplanner/ext/legacygraphqlapi/LegacyGraphQLAPI.java +++ b/src/ext/java/org/opentripplanner/ext/legacygraphqlapi/LegacyGraphQLAPI.java @@ -38,7 +38,7 @@ public class LegacyGraphQLAPI { @SuppressWarnings("unused") - private static final Logger LOG = LoggerFactory.getLogger(org.opentripplanner.ext.transmodelapi.TransmodelIndexAPI.class); + private static final Logger LOG = LoggerFactory.getLogger(LegacyGraphQLAPI.class); private final Router router; private final ObjectMapper deserializer = new ObjectMapper(); diff --git a/src/ext/java/org/opentripplanner/ext/transmodelapi/TransmodelGraphQLPlanner.java b/src/ext/java/org/opentripplanner/ext/transmodelapi/TransmodelGraphQLPlanner.java index a2668eee8eb..220279e3022 100644 --- a/src/ext/java/org/opentripplanner/ext/transmodelapi/TransmodelGraphQLPlanner.java +++ b/src/ext/java/org/opentripplanner/ext/transmodelapi/TransmodelGraphQLPlanner.java @@ -127,7 +127,7 @@ private GenericLocation toGenericLocation(Map m) { } String placeRef = (String) m.get("place"); - FeedScopedId stopId = placeRef == null ? null : mappingUtil.fromIdString(placeRef); + FeedScopedId stopId = placeRef == null ? null : mappingUtil.mapIdToDomain(placeRef); String name = (String) m.get("name"); name = name == null ? "" : name; @@ -302,7 +302,7 @@ private RoutingRequest createRequest(DataFetchingEnvironment environment) { } private HashMap toBannedTrips(Collection serviceJourneyIds) { - Map bannedTrips = serviceJourneyIds.stream().map(mappingUtil::fromIdString).collect(Collectors.toMap(Function.identity(), id -> BannedStopSet.ALL)); + Map bannedTrips = serviceJourneyIds.stream().map(mappingUtil::mapIdToDomain).collect(Collectors.toMap(Function.identity(), id -> BannedStopSet.ALL)); return new HashMap<>(bannedTrips); } diff --git a/src/ext/java/org/opentripplanner/ext/transmodelapi/TransmodelIndexGraphQLSchema.java b/src/ext/java/org/opentripplanner/ext/transmodelapi/TransmodelIndexGraphQLSchema.java index 2912d889652..1bfd96ad777 100644 --- a/src/ext/java/org/opentripplanner/ext/transmodelapi/TransmodelIndexGraphQLSchema.java +++ b/src/ext/java/org/opentripplanner/ext/transmodelapi/TransmodelIndexGraphQLSchema.java @@ -174,7 +174,7 @@ public class TransmodelIndexGraphQLSchema { private GraphQLInputObjectType allowedModesType; - private TransmodelMappingUtil mappingUtil; + private TransmodelMappingUtil mapper; private TripTimeShortHelper tripTimeShortHelper; private String fixedAgencyId; @@ -231,8 +231,8 @@ private Agency getAgency(FeedScopedId agencyId, RoutingService routingService) { } @SuppressWarnings("unchecked") - public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, TransmodelMappingUtil mappingUtil) { - this.mappingUtil = mappingUtil; + public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, TransmodelMappingUtil mapper) { + this.mapper = mapper; this.routing = new DefaultRoutingRequest(defaultRequest); tripTimeShortHelper = new TripTimeShortHelper(); @@ -964,7 +964,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .defaultValue(routing.request.debugItineraryFilter) .build()) - .dataFetcher(environment -> new TransmodelGraphQLPlanner(mappingUtil).plan(environment)) + .dataFetcher(environment -> new TransmodelGraphQLPlanner(mapper).plan(environment)) .build(); noticeType = GraphQLObjectType.newObject() @@ -1213,8 +1213,8 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .name("id") .type(new GraphQLNonNull(Scalars.GraphQLID)) .dataFetcher(environment -> relay.toGlobalId(quayAtDistance.getName(), - Integer.toString((int) ((StopAtDistance) environment.getSource()).distance) + ";" + - mappingUtil.toIdString(((StopAtDistance) environment.getSource()).stop.getId()))) + ((StopAtDistance) environment.getSource()).distance + ";" + + mapper.mapToApi(((StopAtDistance) environment.getSource()).stop.getId()))) .build()) .field(GraphQLFieldDefinition.newFieldDefinition() .name("quay") @@ -1258,7 +1258,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .name("id") .type(new GraphQLNonNull(Scalars.GraphQLID)) .dataFetcher(environment -> - mappingUtil.toIdString(((MonoOrMultiModalStation) environment.getSource()).getId())) + mapper.mapToApi(((MonoOrMultiModalStation) environment.getSource()).getId())) .build()) .field(GraphQLFieldDefinition.newFieldDefinition() .name("name") @@ -1407,7 +1407,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, List lineIdList = whiteList.get("lines"); if (lineIdList != null) { - lineIds.addAll(lineIdList.stream().map(id -> mappingUtil.fromIdString(id)).collect(Collectors.toSet())); + lineIds.addAll(lineIdList.stream().map(id -> mapper.mapIdToDomain(id)).collect(Collectors.toSet())); } } @@ -1445,7 +1445,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .name("id") .type(new GraphQLNonNull(Scalars.GraphQLID)) .dataFetcher(environment -> - mappingUtil.toIdString(((Stop) environment.getSource()).getId())) + mapper.mapToApi(((Stop) environment.getSource()).getId())) .build()) .field(GraphQLFieldDefinition.newFieldDefinition() .name("name") @@ -1577,7 +1577,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, List lineIdList = whiteList.get("lines"); if (lineIdList != null) { - lineIds.addAll(lineIdList.stream().map(id -> mappingUtil.fromIdString(id)).collect(Collectors.toSet())); + lineIds.addAll(lineIdList.stream().map(id -> mapper.mapIdToDomain(id)).collect(Collectors.toSet())); } } @@ -1887,7 +1887,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .name("id") .type(new GraphQLNonNull(Scalars.GraphQLID)) .dataFetcher(environment -> - mappingUtil.toIdString(((Trip) environment.getSource()).getId())) + mapper.mapToApi(((Trip) environment.getSource()).getId())) .build()) .field(GraphQLFieldDefinition.newFieldDefinition() .name("line") @@ -1899,7 +1899,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .type(new GraphQLNonNull(new GraphQLList(dateScalar))) .dataFetcher(environment -> getRoutingService(environment).getCalendarService() .getServiceDatesForServiceId((((Trip) environment.getSource()).getServiceId())) - .stream().map(serviceDate -> mappingUtil.serviceDateToSecondsSinceEpoch(serviceDate)).sorted().collect(Collectors.toList()) + .stream().map(serviceDate -> mapper.serviceDateToSecondsSinceEpoch(serviceDate)).sorted().collect(Collectors.toList()) ) .build()) /* @@ -1986,7 +1986,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .dataFetcher(environment -> { final Trip trip = environment.getSource(); - final ServiceDate serviceDate = mappingUtil.secondsSinceEpochToServiceDate(environment.getArgument("date")); + final ServiceDate serviceDate = mapper.secondsSinceEpochToServiceDate(environment.getArgument("date")); return getRoutingService(environment).getTripTimesShort(trip, serviceDate); }) .build()) @@ -2046,7 +2046,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .field(GraphQLFieldDefinition.newFieldDefinition() .name("id") .type(new GraphQLNonNull(Scalars.GraphQLID)) - .dataFetcher(environment -> mappingUtil.toIdString(((TripPattern) environment.getSource()).getId())) + .dataFetcher(environment -> mapper.mapToApi(((TripPattern) environment.getSource()).getId())) .build()) .field(GraphQLFieldDefinition.newFieldDefinition() .name("line") @@ -2078,7 +2078,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .type(new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(serviceJourneyType)))) .dataFetcher(environment -> { - BitSet services = getRoutingService(environment).getServicesRunningForDate(mappingUtil.secondsSinceEpochToServiceDate(environment.getArgument("date"))); + BitSet services = getRoutingService(environment).getServicesRunningForDate(mapper.secondsSinceEpochToServiceDate(environment.getArgument("date"))); return ((TripPattern) environment.getSource()).scheduledTimetable.tripTimes .stream() .filter(times -> services.get(times.serviceCode)) @@ -2143,7 +2143,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .name("id") .type(new GraphQLNonNull(Scalars.GraphQLID)) .dataFetcher(environment -> - mappingUtil.toIdString(((Route) environment.getSource()).getId())) + mapper.mapToApi(((Route) environment.getSource()).getId())) .build()) .field(GraphQLFieldDefinition.newFieldDefinition() .name("authority") @@ -2309,7 +2309,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .name("id") .description("Operator id") .type(new GraphQLNonNull(Scalars.GraphQLID)) - .dataFetcher(environment -> mappingUtil.toIdString(((Operator) environment.getSource()).getId())) + .dataFetcher(environment -> mapper.mapToApi(((Operator) environment.getSource()).getId())) .build()) .field(GraphQLFieldDefinition.newFieldDefinition() .name("name") @@ -2523,10 +2523,9 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .name("id") .type(new GraphQLNonNull(Scalars.GraphQLString)) .build()) - .dataFetcher(environment -> - mappingUtil.getMonoOrMultiModalStation( - environment.getArgument("id"), - getRoutingService(environment) + .dataFetcher(env -> + getRoutingService(env).getMonoOrMultiModalStation( + mapper.mapIdToDomain(env.getArgument("id")) ) ) .build()) @@ -2538,17 +2537,16 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .name("ids") .type(new GraphQLList(Scalars.GraphQLString)) .build()) - .dataFetcher(environment -> { - if ((environment.getArgument("ids") instanceof List)) { - return ((List) environment.getArgument("ids")) + .dataFetcher(env -> { + if ((env.getArgument("ids") instanceof List)) { + return ((List) env.getArgument("ids")) .stream() - .map(id -> mappingUtil.getMonoOrMultiModalStation( - id, - getRoutingService(environment) - )) + .map(id -> + getRoutingService(env).getMonoOrMultiModalStation(mapper.mapIdToDomain(id)) + ) .collect(Collectors.toList()); } - return new ArrayList<>(getRoutingService(environment).getStations()); + return new ArrayList<>(getRoutingService(env).getStations()); }) .build()) .field(GraphQLFieldDefinition.newFieldDefinition() @@ -2630,7 +2628,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .type(new GraphQLNonNull(Scalars.GraphQLString)) .build()) .dataFetcher(environment -> getRoutingService(environment).getStopForId( - mappingUtil.fromIdString(environment.getArgument("id"))) + mapper.mapIdToDomain(environment.getArgument("id"))) ).build()) .field(GraphQLFieldDefinition.newFieldDefinition() .name("quays") @@ -2656,7 +2654,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, RoutingService routingService = getRoutingService(environment); return ((List) environment.getArgument("ids")) .stream() - .map(id -> routingService.getStopForId(mappingUtil.fromIdString(id))) + .map(id -> routingService.getStopForId(mapper.mapIdToDomain(id))) .collect(Collectors.toList()); } if (environment.getArgument("name") == null) { @@ -2666,7 +2664,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, else { return index.getLuceneIndex().query(environment.getArgument("name"), true, true, false) .stream() - .map(result -> index.stopForId.get(mappingUtil.fromIdString(result.id))); + .map(result -> index.stopForId.get(mapper.fromIdString(result.id))); } */ return emptyList(); @@ -2845,7 +2843,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, if (CollectionUtils.isEmpty(placeTypes)) { placeTypes = Arrays.asList(TransmodelPlaceType.values()); } - List filterByPlaceTypes = mappingUtil.mapPlaceTypes(placeTypes); + List filterByPlaceTypes = mapper.mapPlaceTypes(placeTypes); // Need to fetch more than requested no of places if stopPlaces are allowed, as this requires fetching potentially multiple quays for the same stop place and mapping them to unique stop places. int orgMaxResults = environment.getArgument("maximumResults"); @@ -2893,7 +2891,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .build()) .dataFetcher(environment -> getRoutingService(environment).getAgencyForId( - mappingUtil.fromIdString(environment.getArgument("id")) + mapper.mapIdToDomain(environment.getArgument("id")) )) .build()) .field(GraphQLFieldDefinition.newFieldDefinition() @@ -2912,7 +2910,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .build()) .dataFetcher(environment -> getRoutingService(environment).getOperatorForId().get( - mappingUtil.fromIdString(environment.getArgument("id")) + mapper.mapIdToDomain(environment.getArgument("id")) )) .build()) .field(GraphQLFieldDefinition.newFieldDefinition() @@ -2930,7 +2928,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .type(new GraphQLNonNull(Scalars.GraphQLString)) .build()) .dataFetcher(environment -> getRoutingService(environment).getRouteForId( - mappingUtil.fromIdString(environment.getArgument("id"))) + mapper.mapIdToDomain(environment.getArgument("id"))) ).build()) .field(GraphQLFieldDefinition.newFieldDefinition() .name("lines") @@ -2973,7 +2971,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, return ((List) environment.getArgument("ids")) .stream() .map(id -> getRoutingService(environment).getRouteForId( - mappingUtil.fromIdString(id)) + mapper.mapIdToDomain(id)) ).collect(Collectors.toList()); } Stream stream = getRoutingService(environment).getAllRoutes().stream(); @@ -3021,7 +3019,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .type(new GraphQLNonNull(Scalars.GraphQLString)) .build()) .dataFetcher(environment -> getRoutingService(environment).getTripForId() - .get(mappingUtil.fromIdString(environment.getArgument("id")))) + .get(mapper.mapIdToDomain(environment.getArgument("id")))) .build()) .field(GraphQLFieldDefinition.newFieldDefinition() .name("serviceJourneys") @@ -3056,7 +3054,7 @@ public TransmodelIndexGraphQLSchema(Graph graph, RoutingRequest defaultRequest, .filter(t -> CollectionUtils.isEmpty(lineIds) || lineIds.contains(t.getRoute().getId().getId())) //.filter(t -> CollectionUtils.isEmpty(privateCodes) || privateCodes.contains(t.getTripPrivateCode())) .filter(t -> CollectionUtils.isEmpty(authorities) || authorities.contains(t.getRoute().getAgency().getId())) - .filter(t -> CollectionUtils.isEmpty(activeDates) || getRoutingService(environment).getCalendarService().getServiceDatesForServiceId(t.getServiceId()).stream().anyMatch(sd -> activeDates.contains(mappingUtil.serviceDateToSecondsSinceEpoch(sd)))) + .filter(t -> CollectionUtils.isEmpty(activeDates) || getRoutingService(environment).getCalendarService().getServiceDatesForServiceId(t.getServiceId()).stream().anyMatch(sd -> activeDates.contains(mapper.serviceDateToSecondsSinceEpoch(sd)))) .collect(Collectors.toList()); }) .build()) @@ -3368,7 +3366,7 @@ private List convertQuaysToStopPlaces(List toIdList(List ids) { if (ids == null) return Collections.emptyList(); - return ids.stream().map(id -> mappingUtil.fromIdString(id)).collect(Collectors.toList()); + return ids.stream().map(id -> mapper.mapIdToDomain(id)).collect(Collectors.toList()); } private void createPlanType() { diff --git a/src/ext/java/org/opentripplanner/ext/transmodelapi/mapping/TransmodelMappingUtil.java b/src/ext/java/org/opentripplanner/ext/transmodelapi/mapping/TransmodelMappingUtil.java index a0ce6b25c85..331c088e090 100644 --- a/src/ext/java/org/opentripplanner/ext/transmodelapi/mapping/TransmodelMappingUtil.java +++ b/src/ext/java/org/opentripplanner/ext/transmodelapi/mapping/TransmodelMappingUtil.java @@ -2,19 +2,16 @@ import com.google.common.base.Function; import com.google.common.base.Joiner; -import org.opentripplanner.ext.transmodelapi.model.MonoOrMultiModalStation; import org.opentripplanner.ext.transmodelapi.model.PlaceType; import org.opentripplanner.ext.transmodelapi.model.TransmodelPlaceType; import org.opentripplanner.model.FeedScopedId; -import org.opentripplanner.model.MultiModalStation; -import org.opentripplanner.model.Station; import org.opentripplanner.model.TransitEntity; import org.opentripplanner.model.calendar.ServiceDate; -import org.opentripplanner.routing.RoutingService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.time.LocalDate; +import java.util.ArrayList; import java.util.Collection; import java.util.Date; import java.util.List; @@ -82,20 +79,33 @@ else if(feedIds.size() == 1) { return fixedFeedId; } - public String toIdString(FeedScopedId id) { + public String mapEntityToApiId(TransitEntity entity) { + return mapToApi(entity.getId()); + } + + public String mapToApi(FeedScopedId id) { if (fixedFeedId != null) { return id.getId(); } return id.toString(); } - public FeedScopedId fromIdString(String id) { + public FeedScopedId mapIdToDomain(String id) { + if(id == null) { return null; } if (fixedFeedId != null) { return new FeedScopedId(fixedFeedId, id); } return FeedScopedId.parseId(id); } + public List mapToDomain(Collection ids) { + if(ids == null) { return null; } + List list = new ArrayList<>(); + for (String id : ids) { + list.add(mapIdToDomain(id)); + } + return list; + } /** * Add agency id prefix to vertexIds if fixed agency is set. @@ -162,29 +172,13 @@ public ServiceDate secondsSinceEpochToServiceDate(Long secondsSinceEpoch) { } - public List mapPlaceTypes(List inputTypes) { - if (inputTypes == null) { - return null; - } - - return inputTypes.stream().map(pt -> mapPlaceType(pt)).distinct().collect(Collectors.toList()); - } - - public MonoOrMultiModalStation getMonoOrMultiModalStation( - String idString, - RoutingService routingService - ) { - FeedScopedId id = fromIdString(idString); - Station station = routingService.getStationById(id); - if (station != null) { - return new MonoOrMultiModalStation(station, routingService.getMultiModalStationForStations().get(station)); - } - MultiModalStation multiModalStation = routingService.getMultiModalStationById(id); - if (multiModalStation != null) { - return new MonoOrMultiModalStation(multiModalStation); - } - return null; - } + // public List mapPlaceTypes(List inputTypes) { + // if (inputTypes == null) { + // return null; + // } + // + // return inputTypes.stream().map(pt -> mapPlaceType(pt)).distinct().collect(Collectors.toList()); + // } private PlaceType mapPlaceType(TransmodelPlaceType transmodelType){ if (transmodelType!=null) { diff --git a/src/main/java/org/opentripplanner/routing/graph/Graph.java b/src/main/java/org/opentripplanner/routing/graph/Graph.java index 30017f495de..1f4b4f6c67e 100644 --- a/src/main/java/org/opentripplanner/routing/graph/Graph.java +++ b/src/main/java/org/opentripplanner/routing/graph/Graph.java @@ -22,6 +22,7 @@ import org.opentripplanner.common.geometry.SphericalDistanceLibrary; import org.opentripplanner.common.model.T2; import org.opentripplanner.ext.siri.updater.SiriSXUpdater; +import org.opentripplanner.ext.transmodelapi.model.MonoOrMultiModalStation; import org.opentripplanner.graph_builder.DataImportIssueStore; import org.opentripplanner.graph_builder.issues.NoFutureDates; import org.opentripplanner.model.Agency; @@ -38,6 +39,7 @@ import org.opentripplanner.model.TimetableSnapshot; import org.opentripplanner.model.TimetableSnapshotProvider; import org.opentripplanner.model.TransitEntity; +import org.opentripplanner.model.TransitMode; import org.opentripplanner.model.Trip; import org.opentripplanner.model.TripPattern; import org.opentripplanner.model.WgsCoordinate; @@ -54,7 +56,6 @@ import org.opentripplanner.routing.edgetype.StreetEdge; import org.opentripplanner.routing.impl.AlertPatchServiceImpl; import org.opentripplanner.routing.impl.StreetVertexIndex; -import org.opentripplanner.model.TransitMode; import org.opentripplanner.routing.services.AlertPatchService; import org.opentripplanner.routing.services.notes.StreetNotesService; import org.opentripplanner.routing.trippattern.Deduplicator; @@ -1036,8 +1037,17 @@ public Collection getStations() { return stationById.values(); } - public MultiModalStation getMultiModalStationById(FeedScopedId feedScopedId) { - return multiModalStationById.get(feedScopedId); + @SuppressWarnings({"unused", "UsedBy @Delegate"}) + public MonoOrMultiModalStation getMonoOrMultiModalStation(FeedScopedId id) { + Station station = getStationById(id); + if (station != null) { + return new MonoOrMultiModalStation(station, index.getMultiModalStationForStations().get(station)); + } + MultiModalStation multiModalStation = multiModalStationById.get(id); + if (multiModalStation != null) { + return new MonoOrMultiModalStation(multiModalStation); + } + return null; } public Map getServiceCodes() {