Skip to content

Commit e2c98dd

Browse files
oliemansmoliemansm
committed
Fix critical sonar bugs
1 parent ebf6798 commit e2c98dd

File tree

9 files changed

+88
-58
lines changed

9 files changed

+88
-58
lines changed

‎graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLInvoker.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,8 @@
1111
import java.util.ArrayList;
1212
import java.util.List;
1313
import java.util.concurrent.CompletableFuture;
14-
import lombok.AllArgsConstructor;
1514
import lombok.RequiredArgsConstructor;
1615

17-
@AllArgsConstructor
1816
@RequiredArgsConstructor
1917
public class GraphQLInvoker {
2018

@@ -34,14 +32,17 @@ public GraphQLQueryResult query(GraphQLInvocationInput invocationInput) {
3432

3533
public CompletableFuture<GraphQLQueryResult> queryAsync(GraphQLInvocationInput invocationInput) {
3634
if (invocationInput instanceof GraphQLSingleInvocationInput) {
37-
return executeAsync((GraphQLSingleInvocationInput)invocationInput).thenApply(GraphQLQueryResult::create);
35+
return executeAsync((GraphQLSingleInvocationInput) invocationInput)
36+
.thenApply(GraphQLQueryResult::create);
3837
}
3938
GraphQLBatchedInvocationInput batchedInvocationInput = (GraphQLBatchedInvocationInput) invocationInput;
4039
return executeAsync(batchedInvocationInput).thenApply(GraphQLQueryResult::create);
4140
}
4241

43-
private CompletableFuture<List<ExecutionResult>> executeAsync(GraphQLBatchedInvocationInput batchedInvocationInput) {
44-
GraphQL graphQL = batchedDataLoaderGraphQLBuilder.newGraphQL(batchedInvocationInput, graphQLBuilder);
42+
private CompletableFuture<List<ExecutionResult>> executeAsync(
43+
GraphQLBatchedInvocationInput batchedInvocationInput) {
44+
GraphQL graphQL = batchedDataLoaderGraphQLBuilder
45+
.newGraphQL(batchedInvocationInput, graphQLBuilder);
4546
return sequence(
4647
batchedInvocationInput.getExecutionInputs().stream()
4748
.map(executionInput -> proxy.executeAsync(graphQL, executionInput))
@@ -52,7 +53,7 @@ private CompletableFuture<List<ExecutionResult>> executeAsync(GraphQLBatchedInvo
5253
private <T> CompletableFuture<List<T>> sequence(List<CompletableFuture<T>> futures) {
5354
CompletableFuture[] futuresArray = futures.toArray(new CompletableFuture[0]);
5455
return CompletableFuture.allOf(futuresArray).thenApply(aVoid -> {
55-
List<T> result = new ArrayList<>(futures.size());
56+
List<T> result = new ArrayList<>(futures.size());
5657
for (CompletableFuture future : futuresArray) {
5758
assert future.isDone(); // per the API contract of allOf()
5859
result.add((T) future.join());

‎graphql-java-kickstart/src/main/java/graphql/kickstart/execution/instrumentation/AbstractTrackingApproach.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public abstract class AbstractTrackingApproach implements TrackingApproach {
2727

2828
private final RequestStack stack = new RequestStack();
2929

30-
public AbstractTrackingApproach(DataLoaderRegistry dataLoaderRegistry) {
30+
protected AbstractTrackingApproach(DataLoaderRegistry dataLoaderRegistry) {
3131
this.dataLoaderRegistry = dataLoaderRegistry;
3232
}
3333

@@ -54,12 +54,12 @@ public ExecutionStrategyInstrumentationContext beginExecutionStrategy(
5454
return new ExecutionStrategyInstrumentationContext() {
5555
@Override
5656
public void onDispatched(CompletableFuture<ExecutionResult> result) {
57-
57+
// default empty implementation
5858
}
5959

6060
@Override
6161
public void onCompleted(ExecutionResult result, Throwable t) {
62-
62+
// default empty implementation
6363
}
6464

6565
@Override
@@ -130,11 +130,12 @@ public DeferredFieldInstrumentationContext beginDeferredField(
130130
return new DeferredFieldInstrumentationContext() {
131131
@Override
132132
public void onDispatched(CompletableFuture<ExecutionResult> result) {
133-
133+
// default empty implementation
134134
}
135135

136136
@Override
137137
public void onCompleted(ExecutionResult result, Throwable t) {
138+
// default empty implementation
138139
}
139140

140141
@Override
@@ -173,6 +174,7 @@ public void onDispatched(CompletableFuture result) {
173174

174175
@Override
175176
public void onCompleted(Object result, Throwable t) {
177+
// default empty implementation
176178
}
177179
};
178180
}

‎graphql-java-kickstart/src/main/java/graphql/kickstart/execution/instrumentation/ConfigurableDispatchInstrumentation.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,14 @@
2525
import java.util.Map;
2626
import java.util.concurrent.CompletableFuture;
2727
import java.util.function.Function;
28+
import lombok.extern.slf4j.Slf4j;
2829
import org.dataloader.DataLoader;
2930
import org.dataloader.DataLoaderRegistry;
3031
import org.dataloader.stats.Statistics;
31-
import org.slf4j.Logger;
32-
import org.slf4j.LoggerFactory;
3332

33+
@Slf4j
3434
public class ConfigurableDispatchInstrumentation extends DataLoaderDispatcherInstrumentation {
3535

36-
private static final Logger log = LoggerFactory
37-
.getLogger(ConfigurableDispatchInstrumentation.class);
38-
3936
private final DataLoaderDispatcherInstrumentationOptions options;
4037

4138
private final Function<DataLoaderRegistry, TrackingApproach> approachFunction;
@@ -80,26 +77,26 @@ public DataFetcher<?> instrumentDataFetcher(DataFetcher<?> dataFetcher,
8077
// which allows them to work if used.
8178
return (DataFetcher<Object>) environment -> {
8279
Object obj = dataFetcher.get(environment);
83-
immediatelyDispatch(state);
80+
doImmediatelyDispatch(state);
8481
return obj;
8582
};
8683
}
8784

88-
private void immediatelyDispatch(DataLoaderDispatcherInstrumentationState state) {
85+
private void doImmediatelyDispatch(DataLoaderDispatcherInstrumentationState state) {
8986
state.getApproach().dispatch();
9087
}
9188

9289
@Override
9390
public InstrumentationContext<ExecutionResult> beginExecuteOperation(
9491
InstrumentationExecuteOperationParameters parameters) {
95-
if (!isDataLoaderCompatibleExecution(parameters.getExecutionContext())) {
92+
if (!isDataLoaderCompatible(parameters.getExecutionContext())) {
9693
DataLoaderDispatcherInstrumentationState state = parameters.getInstrumentationState();
9794
state.setAggressivelyBatching(false);
9895
}
9996
return new SimpleInstrumentationContext<>();
10097
}
10198

102-
private boolean isDataLoaderCompatibleExecution(ExecutionContext executionContext) {
99+
private boolean isDataLoaderCompatible(ExecutionContext executionContext) {
103100
//
104101
// currently we only support Query operations and ONLY with AsyncExecutionStrategy as the query ES
105102
// This may change in the future but this is the fix for now
@@ -123,10 +120,12 @@ public ExecutionStrategyInstrumentationContext beginExecutionStrategy(
123120
return new ExecutionStrategyInstrumentationContext() {
124121
@Override
125122
public void onDispatched(CompletableFuture<ExecutionResult> result) {
123+
// default empty implementation
126124
}
127125

128126
@Override
129127
public void onCompleted(ExecutionResult result, Throwable t) {
128+
// default empty implementation
130129
}
131130
};
132131

@@ -145,10 +144,12 @@ public DeferredFieldInstrumentationContext beginDeferredField(
145144
return new DeferredFieldInstrumentationContext() {
146145
@Override
147146
public void onDispatched(CompletableFuture<ExecutionResult> result) {
147+
// default empty implementation
148148
}
149149

150150
@Override
151151
public void onCompleted(ExecutionResult result, Throwable t) {
152+
// default empty implementation
152153
}
153154
};
154155

@@ -181,7 +182,7 @@ public CompletableFuture<ExecutionResult> instrumentExecutionResult(
181182
Map<Object, Object> currentExt = executionResult.getExtensions();
182183
Map<Object, Object> statsMap = new LinkedHashMap<>(
183184
currentExt == null ? Collections.emptyMap() : currentExt);
184-
Map<Object, Object> dataLoaderStats = buildStatsMap(state);
185+
Map<Object, Object> dataLoaderStats = buildStatisticsMap(state);
185186
statsMap.put("dataloader", dataLoaderStats);
186187

187188
log.debug("Data loader stats : {}", dataLoaderStats);
@@ -193,7 +194,7 @@ public CompletableFuture<ExecutionResult> instrumentExecutionResult(
193194
}
194195
}
195196

196-
private Map<Object, Object> buildStatsMap(DataLoaderDispatcherInstrumentationState state) {
197+
private Map<Object, Object> buildStatisticsMap(DataLoaderDispatcherInstrumentationState state) {
197198
DataLoaderRegistry dataLoaderRegistry = state.getDataLoaderRegistry();
198199
Statistics allStats = dataLoaderRegistry.getStatistics();
199200
Map<Object, Object> statsMap = new LinkedHashMap<>();

‎graphql-java-kickstart/src/main/java/graphql/kickstart/execution/subscriptions/DefaultSubscriptionSession.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public void sendErrorMessage(String id, Object payload) {
5959

6060
@Override
6161
public void sendCompleteMessage(String id) {
62-
62+
// default empty implementation
6363
}
6464

6565
@Override

‎graphql-java-kickstart/src/main/java/graphql/kickstart/execution/subscriptions/SubscriptionException.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22

33
public class SubscriptionException extends Exception {
44

5-
private Object payload;
5+
private final transient Object payload;
66

77
public SubscriptionException() {
8+
this(null);
89
}
910

1011
public SubscriptionException(Object payload) {

‎graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLMultipartInvocationInputParser.java‎

Lines changed: 48 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -49,43 +49,14 @@ public GraphQLInvocationInput getGraphQLInvocationInput(HttpServletRequest reque
4949
continue;
5050
}
5151

52-
final Optional<Part> queryItem = getPart(parts, key);
52+
final Optional<Part> queryItem = findPart(parts);
5353
if (!queryItem.isPresent()) {
54-
// If there is a part, but we don't see an item, then break and return BAD_REQUEST
55-
break;
54+
log.info("Bad POST multipart request: no part named {}", Arrays.toString(MULTIPART_KEYS));
55+
throw new GraphQLException(
56+
"Bad POST multipart request: no part named " + Arrays.toString(MULTIPART_KEYS));
5657
}
5758

58-
InputStream inputStream = queryItem.get().getInputStream();
59-
60-
final Optional<Map<String, List<String>>> variablesMap =
61-
getPart(parts, "map")
62-
.map(part -> {
63-
try (InputStream is = part.getInputStream()) {
64-
return graphQLObjectMapper.deserializeMultipartMap(is);
65-
} catch (IOException e) {
66-
throw new RuntimeException("Unable to read input stream from part", e);
67-
}
68-
});
69-
70-
String query = read(inputStream);
71-
if ("query".equals(key) && isSingleQuery(query)) {
72-
GraphQLRequest graphqlRequest = buildRequestFromQuery(query, graphQLObjectMapper, parts);
73-
variablesMap.ifPresent(m -> mapMultipartVariables(graphqlRequest, m, parts));
74-
return invocationInputFactory.create(graphqlRequest, request, response);
75-
} else {
76-
if (isSingleQuery(query)) {
77-
GraphQLRequest graphqlRequest = graphQLObjectMapper.readGraphQLRequest(query);
78-
variablesMap.ifPresent(m -> mapMultipartVariables(graphqlRequest, m, parts));
79-
return invocationInputFactory.create(graphqlRequest, request, response);
80-
} else {
81-
List<GraphQLRequest> graphqlRequests = graphQLObjectMapper
82-
.readBatchedGraphQLRequest(query);
83-
variablesMap.ifPresent(
84-
map -> graphqlRequests.forEach(r -> mapMultipartVariables(r, map, parts)));
85-
return invocationInputFactory
86-
.create(contextSetting, graphqlRequests, request, response);
87-
}
88-
}
59+
return getGraphQLInvocationInput(request, response, parts, key, queryItem.get());
8960
}
9061

9162
log.info("Bad POST multipart request: no part named {}", Arrays.toString(MULTIPART_KEYS));
@@ -96,8 +67,50 @@ public GraphQLInvocationInput getGraphQLInvocationInput(HttpServletRequest reque
9667
}
9768
}
9869

70+
private GraphQLInvocationInput getGraphQLInvocationInput(HttpServletRequest request,
71+
HttpServletResponse response, Map<String, List<Part>> parts, String key,
72+
Part queryItem) throws IOException {
73+
InputStream inputStream = queryItem.getInputStream();
74+
75+
final Optional<Map<String, List<String>>> variablesMap =
76+
getPart(parts, "map")
77+
.map(part -> {
78+
try (InputStream is = part.getInputStream()) {
79+
return graphQLObjectMapper.deserializeMultipartMap(is);
80+
} catch (IOException e) {
81+
throw new PartIOException("Unable to read input stream from part", e);
82+
}
83+
});
84+
85+
String query = read(inputStream);
86+
if ("query".equals(key) && isSingleQuery(query)) {
87+
GraphQLRequest graphqlRequest = buildRequestFromQuery(query, graphQLObjectMapper, parts);
88+
variablesMap.ifPresent(m -> mapMultipartVariables(graphqlRequest, m, parts));
89+
return invocationInputFactory.create(graphqlRequest, request, response);
90+
} else if (isSingleQuery(query)) {
91+
GraphQLRequest graphqlRequest = graphQLObjectMapper.readGraphQLRequest(query);
92+
variablesMap.ifPresent(m -> mapMultipartVariables(graphqlRequest, m, parts));
93+
return invocationInputFactory.create(graphqlRequest, request, response);
94+
} else {
95+
List<GraphQLRequest> graphqlRequests = graphQLObjectMapper.readBatchedGraphQLRequest(query);
96+
variablesMap.ifPresent(
97+
map -> graphqlRequests.forEach(r -> mapMultipartVariables(r, map, parts)));
98+
return invocationInputFactory
99+
.create(contextSetting, graphqlRequests, request, response);
100+
}
101+
}
102+
103+
private Optional<Part> findPart(Map<String, List<Part>> parts) {
104+
return Arrays.stream(MULTIPART_KEYS)
105+
.filter(parts::containsKey)
106+
.map(key -> getPart(parts, key))
107+
.findFirst()
108+
.map(Optional::get);
109+
}
110+
99111
private Optional<Part> getPart(Map<String, List<Part>> parts, String name) {
100-
return Optional.ofNullable(parts.get(name)).filter(list -> !list.isEmpty())
112+
return Optional.ofNullable(parts.get(name))
113+
.filter(list -> !list.isEmpty())
101114
.map(list -> list.get(0));
102115
}
103116

‎graphql-java-servlet/src/main/java/graphql/kickstart/servlet/PartIOException.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package graphql.kickstart.servlet;
2+
3+
public class PartIOException extends RuntimeException {
4+
5+
public PartIOException(String message, Throwable cause) {
6+
super(message, cause);
7+
}
8+
9+
}

‎graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SubscriptionAsyncListener.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ public void onError(AsyncEvent event) {
2828

2929
@Override
3030
public void onStartAsync(AsyncEvent event) {
31+
// default empty implementation
3132
}
3233

3334
}

‎graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/BufferedHttpServletResponse.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public class BufferedHttpServletResponse extends HttpServletResponseWrapper {
1818
private ServletOutputStream outputStream;
1919
private PrintWriter writer;
2020
private String errorMessage;
21+
2122
public BufferedHttpServletResponse(HttpServletResponse response) {
2223
super(response);
2324
}
@@ -130,6 +131,7 @@ public boolean isReady() {
130131

131132
@Override
132133
public void setWriteListener(WriteListener writeListener) {
134+
// write listener not supported
133135
}
134136

135137
public byte[] toByteArray() {

0 commit comments

Comments
 (0)