Skip to content

Commit b45d54d

Browse files
t2grant2gran
authored
Merge pull request opentripplanner#6961 from entur/fix_npe_in_trip_plan_query
Fix NPE in TransModel trip plan query
2 parents 6b65772 + 9db9e2c commit b45d54d

File tree

10 files changed

+396
-67
lines changed

10 files changed

+396
-67
lines changed

‎application/src/main/java/org/opentripplanner/apis/transmodel/TransmodelGraphQLPlanner.java‎

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@
1010
import org.opentripplanner.apis.transmodel.mapping.ViaRequestMapper;
1111
import org.opentripplanner.apis.transmodel.model.PlanResponse;
1212
import org.opentripplanner.routing.algorithm.mapping.TripPlanMapper;
13-
import org.opentripplanner.routing.api.request.RouteRequest;
14-
import org.opentripplanner.routing.api.request.RouteViaRequest;
13+
import org.opentripplanner.routing.api.request.RouteRequestBuilder;
1514
import org.opentripplanner.routing.api.response.RoutingResponse;
1615
import org.opentripplanner.routing.api.response.ViaRoutingResponse;
1716
import org.opentripplanner.routing.error.RoutingValidationException;
@@ -31,47 +30,54 @@ public TransmodelGraphQLPlanner(FeedScopedIdMapper idMapper) {
3130
}
3231

3332
public DataFetcherResult<PlanResponse> plan(DataFetchingEnvironment environment) {
34-
PlanResponse response = new PlanResponse();
3533
TransmodelRequestContext ctx = environment.getContext();
36-
RouteRequest request = null;
34+
Locale locale;
35+
PlanResponse response;
36+
RouteRequestBuilder requestBuilder = tripRequestMapper.createRequestBuilder(environment);
3737
try {
38-
request = tripRequestMapper.createRequest(environment);
38+
var request = requestBuilder.buildRequest();
3939
RoutingResponse res = ctx.getRoutingService().route(request);
40-
41-
response.plan = res.getTripPlan();
42-
response.metadata = res.getMetadata();
43-
response.messages = res.getRoutingErrors();
44-
response.debugOutput = res.getDebugTimingAggregator().finishedRendering();
45-
response.previousPageCursor = res.getPreviousPageCursor();
46-
response.nextPageCursor = res.getNextPageCursor();
40+
response = PlanResponse.of()
41+
.withPlan(res.getTripPlan())
42+
.withMetadata(res.getMetadata())
43+
.withMessages(res.getRoutingErrors())
44+
.withDebugOutput(res.getDebugTimingAggregator().finishedRendering())
45+
.withPreviousPageCursor(res.getPreviousPageCursor())
46+
.withNextPageCursor(res.getNextPageCursor())
47+
.build();
48+
locale = request.preferences().locale();
4749
} catch (RoutingValidationException e) {
48-
response.plan = TripPlanMapper.mapTripPlan(request, List.of());
49-
response.messages.addAll(e.getRoutingErrors());
50+
response = PlanResponse.of()
51+
.withPlan(TripPlanMapper.mapEmptyTripPlan(requestBuilder))
52+
.withMessages(e.getRoutingErrors())
53+
.build();
54+
locale = defaultLocale(ctx);
5055
}
51-
5256
return DataFetcherResult.<PlanResponse>newResult()
5357
.data(response)
54-
.localContext(Map.of("locale", request.preferences().locale()))
58+
.localContext(Map.of("locale", locale))
5559
.build();
5660
}
5761

5862
public DataFetcherResult<ViaRoutingResponse> planVia(DataFetchingEnvironment environment) {
5963
ViaRoutingResponse response;
6064
TransmodelRequestContext ctx = environment.getContext();
61-
RouteViaRequest request = null;
65+
Locale locale;
6266
try {
63-
request = viaRequestMapper.createRouteViaRequest(environment);
67+
var request = viaRequestMapper.createRouteViaRequest(environment);
6468
response = ctx.getRoutingService().route(request);
69+
locale = request.locale();
6570
} catch (RoutingValidationException e) {
6671
response = new ViaRoutingResponse(Map.of(), List.of(), e.getRoutingErrors());
72+
locale = defaultLocale(ctx);
6773
}
68-
69-
Locale defaultLocale = ctx.getServerContext().defaultRouteRequest().preferences().locale();
70-
// This is strange, the `request` can not be null here ?
71-
Locale locale = request == null ? defaultLocale : request.locale();
7274
return DataFetcherResult.<ViaRoutingResponse>newResult()
7375
.data(response)
7476
.localContext(Map.of("locale", locale))
7577
.build();
7678
}
79+
80+
private static Locale defaultLocale(TransmodelRequestContext ctx) {
81+
return ctx.getServerContext().defaultRouteRequest().preferences().locale();
82+
}
7783
}

‎application/src/main/java/org/opentripplanner/apis/transmodel/mapping/TripRequestMapper.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.opentripplanner.apis.transmodel.support.DataFetcherDecorator;
1313
import org.opentripplanner.apis.transmodel.support.GqlUtil;
1414
import org.opentripplanner.routing.api.request.RouteRequest;
15+
import org.opentripplanner.routing.api.request.RouteRequestBuilder;
1516

1617
public class TripRequestMapper {
1718

@@ -29,10 +30,17 @@ public TripRequestMapper(FeedScopedIdMapper idMapper) {
2930
this.idMapper = idMapper;
3031
}
3132

33+
/**
34+
* Create a RouteRequestBuilder from the input fields of the trip query arguments.
35+
*/
36+
public RouteRequest createRequest(DataFetchingEnvironment dataFetchingEnvironment) {
37+
return createRequestBuilder(dataFetchingEnvironment).buildRequest();
38+
}
39+
3240
/**
3341
* Create a RouteRequest from the input fields of the trip query arguments.
3442
*/
35-
public RouteRequest createRequest(DataFetchingEnvironment environment) {
43+
public RouteRequestBuilder createRequestBuilder(DataFetchingEnvironment environment) {
3644
TransmodelRequestContext context = environment.getContext();
3745
var serverContext = context.getServerContext();
3846
var requestBuilder = serverContext.defaultRouteRequest().copyOf();
@@ -103,6 +111,6 @@ public RouteRequest createRequest(DataFetchingEnvironment environment) {
103111
PreferencesMapper.mapPreferences(environment, callWith, preferences)
104112
);
105113

106-
return requestBuilder.buildRequest();
114+
return requestBuilder;
107115
}
108116
}

‎application/src/main/java/org/opentripplanner/apis/transmodel/model/PlanResponse.java‎

Lines changed: 125 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,71 @@
11
package org.opentripplanner.apis.transmodel.model;
22

3+
import java.time.Instant;
34
import java.util.ArrayList;
45
import java.util.List;
6+
import java.util.Objects;
7+
import javax.annotation.Nullable;
58
import org.opentripplanner.api.resource.DebugOutput;
9+
import org.opentripplanner.model.plan.Itinerary;
10+
import org.opentripplanner.model.plan.Place;
611
import org.opentripplanner.model.plan.TripPlan;
712
import org.opentripplanner.model.plan.paging.cursor.PageCursor;
813
import org.opentripplanner.routing.api.response.RoutingError;
914
import org.opentripplanner.routing.api.response.TripSearchMetadata;
1015

1116
public class PlanResponse {
1217

13-
public TripPlan plan;
14-
public TripSearchMetadata metadata;
15-
public List<RoutingError> messages = new ArrayList<>();
16-
public DebugOutput debugOutput;
17-
public PageCursor previousPageCursor;
18-
public PageCursor nextPageCursor;
18+
private final TripPlan plan;
19+
20+
private final TripSearchMetadata metadata;
21+
private final List<RoutingError> messages;
22+
private final DebugOutput debugOutput;
23+
private final PageCursor previousPageCursor;
24+
private final PageCursor nextPageCursor;
25+
26+
PlanResponse(
27+
TripPlan plan,
28+
@Nullable TripSearchMetadata metadata,
29+
@Nullable List<RoutingError> messages,
30+
@Nullable DebugOutput debugOutput,
31+
@Nullable PageCursor previousPageCursor,
32+
@Nullable PageCursor nextPageCursor
33+
) {
34+
this.plan = Objects.requireNonNull(plan);
35+
this.metadata = metadata;
36+
this.messages = messages != null ? messages : new ArrayList<>();
37+
this.debugOutput = debugOutput;
38+
this.previousPageCursor = previousPageCursor;
39+
this.nextPageCursor = nextPageCursor;
40+
}
41+
42+
public static PlanResponseBuilder of() {
43+
return new PlanResponseBuilder();
44+
}
45+
46+
@Nullable
47+
public DebugOutput debugOutput() {
48+
return debugOutput;
49+
}
50+
51+
public List<RoutingError> messages() {
52+
return messages;
53+
}
54+
55+
@Nullable
56+
public TripSearchMetadata metadata() {
57+
return metadata;
58+
}
59+
60+
@Nullable
61+
public PageCursor nextPageCursor() {
62+
return nextPageCursor;
63+
}
64+
65+
@Nullable
66+
public PageCursor previousPageCursor() {
67+
return previousPageCursor;
68+
}
1969

2070
@Override
2171
public String toString() {
@@ -36,4 +86,73 @@ public String toString() {
3686
'}'
3787
);
3888
}
89+
90+
public Instant date() {
91+
return plan.date;
92+
}
93+
94+
public Place from() {
95+
return plan.from;
96+
}
97+
98+
public Place to() {
99+
return plan.to;
100+
}
101+
102+
public List<Itinerary> itineraries() {
103+
return plan.itineraries;
104+
}
105+
106+
public static class PlanResponseBuilder {
107+
108+
private TripPlan plan;
109+
private TripSearchMetadata metadata;
110+
private List<RoutingError> messages = new ArrayList<>();
111+
private DebugOutput debugOutput;
112+
private PageCursor previousPageCursor;
113+
private PageCursor nextPageCursor;
114+
115+
PlanResponseBuilder() {}
116+
117+
public PlanResponseBuilder withPlan(TripPlan plan) {
118+
this.plan = plan;
119+
return this;
120+
}
121+
122+
public PlanResponseBuilder withMetadata(@Nullable TripSearchMetadata metadata) {
123+
this.metadata = metadata;
124+
return this;
125+
}
126+
127+
public PlanResponseBuilder withMessages(List<RoutingError> messages) {
128+
this.messages = messages != null ? new ArrayList<>(messages) : new ArrayList<>();
129+
return this;
130+
}
131+
132+
public PlanResponseBuilder withDebugOutput(@Nullable DebugOutput debugOutput) {
133+
this.debugOutput = debugOutput;
134+
return this;
135+
}
136+
137+
public PlanResponseBuilder withPreviousPageCursor(@Nullable PageCursor previousPageCursor) {
138+
this.previousPageCursor = previousPageCursor;
139+
return this;
140+
}
141+
142+
public PlanResponseBuilder withNextPageCursor(@Nullable PageCursor nextPageCursor) {
143+
this.nextPageCursor = nextPageCursor;
144+
return this;
145+
}
146+
147+
public PlanResponse build() {
148+
return new PlanResponse(
149+
this.plan,
150+
this.metadata,
151+
this.messages,
152+
this.debugOutput,
153+
this.previousPageCursor,
154+
this.nextPageCursor
155+
);
156+
}
157+
}
39158
}

‎application/src/main/java/org/opentripplanner/apis/transmodel/model/plan/TripType.java‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,39 +31,39 @@ public static GraphQLObjectType create(
3131
.name("dateTime")
3232
.description("The time and date of travel")
3333
.type(dateTimeScalar)
34-
.dataFetcher(env -> ((PlanResponse) env.getSource()).plan.date.toEpochMilli())
34+
.dataFetcher(env -> ((PlanResponse) env.getSource()).date().toEpochMilli())
3535
.build()
3636
)
3737
.field(
3838
GraphQLFieldDefinition.newFieldDefinition()
3939
.name("metadata")
4040
.description("The trip request metadata.")
4141
.type(tripMetadataType)
42-
.dataFetcher(env -> ((PlanResponse) env.getSource()).metadata)
42+
.dataFetcher(env -> ((PlanResponse) env.getSource()).metadata())
4343
.build()
4444
)
4545
.field(
4646
GraphQLFieldDefinition.newFieldDefinition()
4747
.name("fromPlace")
4848
.description("The origin")
4949
.type(new GraphQLNonNull(placeType))
50-
.dataFetcher(env -> ((PlanResponse) env.getSource()).plan.from)
50+
.dataFetcher(env -> ((PlanResponse) env.getSource()).from())
5151
.build()
5252
)
5353
.field(
5454
GraphQLFieldDefinition.newFieldDefinition()
5555
.name("toPlace")
5656
.description("The destination")
5757
.type(new GraphQLNonNull(placeType))
58-
.dataFetcher(env -> ((PlanResponse) env.getSource()).plan.to)
58+
.dataFetcher(env -> ((PlanResponse) env.getSource()).to())
5959
.build()
6060
)
6161
.field(
6262
GraphQLFieldDefinition.newFieldDefinition()
6363
.name("tripPatterns")
6464
.description("A list of possible trip patterns")
6565
.type(new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(tripPatternType))))
66-
.dataFetcher(env -> ((PlanResponse) env.getSource()).plan.itineraries)
66+
.dataFetcher(env -> ((PlanResponse) env.getSource()).itineraries())
6767
.build()
6868
)
6969
.field(
@@ -73,7 +73,8 @@ public static GraphQLObjectType create(
7373
.deprecate("Use routingErrors instead")
7474
.type(new GraphQLNonNull(new GraphQLList(Scalars.GraphQLString)))
7575
.dataFetcher(env ->
76-
((PlanResponse) env.getSource()).messages.stream()
76+
((PlanResponse) env.getSource()).messages()
77+
.stream()
7778
.map(routingError -> PlannerErrorMapper.mapMessage(routingError).message)
7879
.map(Enum::name)
7980
.collect(Collectors.toList())
@@ -90,7 +91,8 @@ public static GraphQLObjectType create(
9091
GraphQLArgument.newArgument().name("language").type(Scalars.GraphQLString).build()
9192
)
9293
.dataFetcher(env ->
93-
((PlanResponse) env.getSource()).messages.stream()
94+
((PlanResponse) env.getSource()).messages()
95+
.stream()
9496
.map(routingError -> PlannerErrorMapper.mapMessage(routingError).message)
9597
.map(message -> message.get(GraphQLUtils.getLocale(env)))
9698
.collect(Collectors.toList())
@@ -102,7 +104,7 @@ public static GraphQLObjectType create(
102104
.name("routingErrors")
103105
.description("A list of routing errors, and fields which caused them")
104106
.type(new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(routingErrorType))))
105-
.dataFetcher(env -> ((PlanResponse) env.getSource()).messages)
107+
.dataFetcher(env -> ((PlanResponse) env.getSource()).messages())
106108
.build()
107109
)
108110
.field(
@@ -122,7 +124,7 @@ public static GraphQLObjectType create(
122124
.build()
123125
)
124126
)
125-
.dataFetcher(env -> ((PlanResponse) env.getSource()).debugOutput)
127+
.dataFetcher(env -> ((PlanResponse) env.getSource()).debugOutput())
126128
.build()
127129
)
128130
.field(
@@ -136,7 +138,7 @@ public static GraphQLObjectType create(
136138
)
137139
.type(Scalars.GraphQLString)
138140
.dataFetcher(env -> {
139-
final PageCursor pageCursor = ((PlanResponse) env.getSource()).previousPageCursor;
141+
final PageCursor pageCursor = ((PlanResponse) env.getSource()).previousPageCursor();
140142
return pageCursor != null ? pageCursor.encode() : null;
141143
})
142144
.build()
@@ -152,7 +154,7 @@ public static GraphQLObjectType create(
152154
)
153155
.type(Scalars.GraphQLString)
154156
.dataFetcher(env -> {
155-
final PageCursor pageCursor = ((PlanResponse) env.getSource()).nextPageCursor;
157+
final PageCursor pageCursor = ((PlanResponse) env.getSource()).nextPageCursor();
156158
return pageCursor != null ? pageCursor.encode() : null;
157159
})
158160
.build()

0 commit comments

Comments
 (0)