Skip to content

Commit 7fe26c5

Browse files
author
agairol
committed
Merge branch 'master' into support-graphql-contenttype
# Conflicts: # src/main/java/graphql/servlet/GraphQLServlet.java
2 parents 52ff6c1 + 387ac86 commit 7fe26c5

File tree

6 files changed

+102
-98
lines changed

6 files changed

+102
-98
lines changed

‎build.gradle‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,6 @@ dependencies {
5151

5252
// Servlet
5353
compile 'javax.servlet:javax.servlet-api:3.0.1'
54-
// Multipart support
55-
compile 'commons-fileupload:commons-fileupload:1.3.1'
5654

5755
// GraphQL
5856
compile 'com.graphql-java:graphql-java:8.0'

‎src/main/java/graphql/servlet/GraphQLContext.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
package graphql.servlet;
22

3-
import org.apache.commons.fileupload.FileItem;
4-
53
import javax.security.auth.Subject;
64
import javax.servlet.http.HttpServletRequest;
75
import javax.servlet.http.HttpServletResponse;
6+
import javax.servlet.http.Part;
87
import java.util.List;
98
import java.util.Map;
109
import java.util.Optional;
@@ -14,7 +13,7 @@ public class GraphQLContext {
1413
private Optional<HttpServletResponse> response;
1514

1615
private Optional<Subject> subject = Optional.empty();
17-
private Optional<Map<String, List<FileItem>>> files = Optional.empty();
16+
private Optional<Map<String, List<Part>>> files = Optional.empty();
1817

1918
public GraphQLContext(Optional<HttpServletRequest> request, Optional<HttpServletResponse> response) {
2019
this.request = request;
@@ -45,11 +44,11 @@ public void setSubject(Optional<Subject> subject) {
4544
this.subject = subject;
4645
}
4746

48-
public Optional<Map<String, List<FileItem>>> getFiles() {
47+
public Optional<Map<String, List<Part>>> getFiles() {
4948
return files;
5049
}
5150

52-
public void setFiles(Optional<Map<String, List<FileItem>>> files) {
51+
public void setFiles(Optional<Map<String, List<Part>>> files) {
5352
this.files = files;
5453
}
5554
}

‎src/main/java/graphql/servlet/GraphQLServlet.java‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import com.fasterxml.jackson.databind.ObjectMapper;
1010
import com.fasterxml.jackson.databind.ObjectReader;
1111
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
12+
import com.google.common.io.ByteStreams;
1213
import com.google.common.io.CharStreams;
1314
import graphql.ExecutionInput;
1415
import graphql.ExecutionResult;
@@ -19,10 +20,6 @@
1920
import graphql.introspection.IntrospectionQuery;
2021
import graphql.schema.GraphQLFieldDefinition;
2122
import graphql.schema.GraphQLSchema;
22-
import org.apache.commons.fileupload.FileItem;
23-
import org.apache.commons.fileupload.FileItemFactory;
24-
import org.apache.commons.fileupload.disk.DiskFileItemFactory;
25-
import org.apache.commons.fileupload.servlet.ServletFileUpload;
2623
import org.slf4j.Logger;
2724
import org.slf4j.LoggerFactory;
2825

@@ -32,6 +29,7 @@
3229
import javax.servlet.http.HttpServlet;
3330
import javax.servlet.http.HttpServletRequest;
3431
import javax.servlet.http.HttpServletResponse;
32+
import javax.servlet.http.Part;
3533
import java.io.BufferedInputStream;
3634
import java.io.ByteArrayOutputStream;
3735
import java.io.IOException;
@@ -40,6 +38,7 @@
4038
import java.security.AccessController;
4139
import java.security.PrivilegedAction;
4240
import java.util.ArrayList;
41+
import java.util.Collection;
4342
import java.util.Collections;
4443
import java.util.HashMap;
4544
import java.util.Iterator;
@@ -52,6 +51,7 @@
5251
import java.util.function.Consumer;
5352
import java.util.function.Function;
5453
import java.util.stream.Collectors;
54+
import java.util.stream.Stream;
5555

5656
/**
5757
* @author Andrew Potter
@@ -76,19 +76,17 @@ public abstract class GraphQLServlet extends HttpServlet implements Servlet, Gra
7676

7777
private final LazyObjectMapperBuilder lazyObjectMapperBuilder;
7878
private final List<GraphQLServletListener> listeners;
79-
private final ServletFileUpload fileUpload;
8079

8180
private final HttpRequestHandler getHandler;
8281
private final HttpRequestHandler postHandler;
8382

8483
public GraphQLServlet() {
85-
this(null, null, null);
84+
this(null, null);
8685
}
8786

88-
public GraphQLServlet(ObjectMapperConfigurer objectMapperConfigurer, List<GraphQLServletListener> listeners, FileItemFactory fileItemFactory) {
87+
public GraphQLServlet(ObjectMapperConfigurer objectMapperConfigurer, List<GraphQLServletListener> listeners) {
8988
this.lazyObjectMapperBuilder = new LazyObjectMapperBuilder(objectMapperConfigurer != null ? objectMapperConfigurer : new DefaultObjectMapperConfigurer());
9089
this.listeners = listeners != null ? new ArrayList<>(listeners) : new ArrayList<>();
91-
this.fileUpload = new ServletFileUpload(fileItemFactory != null ? fileItemFactory : new DiskFileItemFactory());
9290

9391
this.getHandler = (request, response) -> {
9492
final GraphQLContext context = createContext(Optional.of(request), Optional.of(response));
@@ -130,15 +128,21 @@ public GraphQLServlet(ObjectMapperConfigurer objectMapperConfigurer, List<GraphQ
130128
final Object rootObject = createRootObject(Optional.of(request), Optional.of(response));
131129

132130
try {
131+
Collection<Part> parts = request.getParts();
133132
if (APPLICATION_GRAPHQL.equals(request.getContentType())) {
134133
String query = CharStreams.toString(request.getReader());
135134
doQuery(query, null, null, getSchemaProvider().getSchema(request), context, rootObject, request, response);
136-
} else if (ServletFileUpload.isMultipartContent(request)) {
137-
final Map<String, List<FileItem>> fileItems = fileUpload.parseParameterMap(request);
135+
} else if (!parts.isEmpty()) {
136+
final Map<String, List<Part>> fileItems = parts.stream()
137+
.collect(Collectors.toMap(
138+
Part::getName,
139+
Collections::singletonList,
140+
(l1, l2) -> Stream.concat(l1.stream(), l2.stream()).collect(Collectors.toList())));
141+
138142
context.setFiles(Optional.of(fileItems));
139143

140144
if (fileItems.containsKey("graphql")) {
141-
final Optional<FileItem> graphqlItem = getFileItem(fileItems, "graphql");
145+
final Optional<Part> graphqlItem = getFileItem(fileItems, "graphql");
142146
if (graphqlItem.isPresent()) {
143147
InputStream inputStream = graphqlItem.get().getInputStream();
144148

@@ -155,7 +159,7 @@ public GraphQLServlet(ObjectMapperConfigurer objectMapperConfigurer, List<GraphQ
155159
}
156160
}
157161
} else if (fileItems.containsKey("query")) {
158-
final Optional<FileItem> queryItem = getFileItem(fileItems, "query");
162+
final Optional<Part> queryItem = getFileItem(fileItems, "query");
159163
if (queryItem.isPresent()) {
160164
InputStream inputStream = queryItem.get().getInputStream();
161165

@@ -167,18 +171,19 @@ public GraphQLServlet(ObjectMapperConfigurer objectMapperConfigurer, List<GraphQ
167171
doBatchedQuery(getGraphQLRequestMapper().readValues(inputStream), getSchemaProvider().getSchema(request), context, rootObject, request, response);
168172
return;
169173
} else {
170-
String query = new String(queryItem.get().get());
174+
175+
String query = new String(ByteStreams.toByteArray(inputStream));
171176

172177
Map<String, Object> variables = null;
173-
final Optional<FileItem> variablesItem = getFileItem(fileItems, "variables");
178+
final Optional<Part> variablesItem = getFileItem(fileItems, "variables");
174179
if (variablesItem.isPresent()) {
175-
variables = deserializeVariables(new String(variablesItem.get().get()));
180+
variables = deserializeVariables(new String(ByteStreams.toByteArray(variablesItem.get().getInputStream())));
176181
}
177182

178183
String operationName = null;
179-
final Optional<FileItem> operationNameItem = getFileItem(fileItems, "operationName");
184+
final Optional<Part> operationNameItem = getFileItem(fileItems, "operationName");
180185
if (operationNameItem.isPresent()) {
181-
operationName = new String(operationNameItem.get().get()).trim();
186+
operationName = new String(ByteStreams.toByteArray(operationNameItem.get().getInputStream())).trim();
182187
}
183188

184189
doQuery(query, operationName, variables, getSchemaProvider().getSchema(request), context, rootObject, request, response);
@@ -279,13 +284,8 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S
279284
doRequest(req, resp, postHandler);
280285
}
281286

282-
private Optional<FileItem> getFileItem(Map<String, List<FileItem>> fileItems, String name) {
283-
List<FileItem> items = fileItems.get(name);
284-
if(items == null || items.isEmpty()) {
285-
return Optional.empty();
286-
}
287-
288-
return items.stream().findFirst();
287+
private Optional<Part> getFileItem(Map<String, List<Part>> fileItems, String name) {
288+
return Optional.ofNullable(fileItems.get(name)).filter(list -> !list.isEmpty()).map(list -> list.get(0));
289289
}
290290

291291
private GraphQL newGraphQL(GraphQLSchema schema) {

‎src/main/java/graphql/servlet/SimpleGraphQLServlet.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public SimpleGraphQLServlet(final GraphQLSchema schema, ExecutionStrategyProvide
5454
*/
5555
@Deprecated
5656
public SimpleGraphQLServlet(GraphQLSchemaProvider schemaProvider, ExecutionStrategyProvider executionStrategyProvider, ObjectMapperConfigurer objectMapperConfigurer, List<GraphQLServletListener> listeners, Instrumentation instrumentation, GraphQLErrorHandler errorHandler, GraphQLContextBuilder contextBuilder, GraphQLRootObjectBuilder rootObjectBuilder, PreparsedDocumentProvider preparsedDocumentProvider) {
57-
super(objectMapperConfigurer, listeners, null);
57+
super(objectMapperConfigurer, listeners);
5858

5959
this.schemaProvider = schemaProvider;
6060
this.executionStrategyProvider = executionStrategyProvider;
@@ -91,7 +91,7 @@ public SimpleGraphQLServlet(GraphQLSchemaProvider schemaProvider, ExecutionStrat
9191
}
9292

9393
protected SimpleGraphQLServlet(Builder builder) {
94-
super(builder.objectMapperConfigurer, builder.listeners, null);
94+
super(builder.objectMapperConfigurer, builder.listeners);
9595

9696
this.schemaProvider = builder.schemaProvider;
9797
this.executionStrategyProvider = builder.executionStrategyProvider;

‎src/test/groovy/graphql/servlet/GraphQLServletSpec.groovy‎

Lines changed: 18 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -401,9 +401,7 @@ class GraphQLServletSpec extends Specification {
401401
request.setContentType("multipart/form-data, boundary=test")
402402
request.setMethod("POST")
403403

404-
request.setContent(new TestMultipartContentBuilder()
405-
.addPart('graphql', mapper.writeValueAsString([query: 'query { echo(arg:"test") }']))
406-
.build())
404+
request.addPart(TestMultipartContentBuilder.createPart('graphql', mapper.writeValueAsString([query: 'query { echo(arg:"test") }'])))
407405

408406
when:
409407
servlet.doPost(request, response)
@@ -418,9 +416,7 @@ class GraphQLServletSpec extends Specification {
418416
setup:
419417
request.setContentType("multipart/form-data, boundary=test")
420418
request.setMethod("POST")
421-
request.setContent(new TestMultipartContentBuilder()
422-
.addPart('query', 'query { echo(arg:"test") }')
423-
.build())
419+
request.addPart(TestMultipartContentBuilder.createPart('query', 'query { echo(arg:"test") }'))
424420

425421
when:
426422
servlet.doPost(request, response)
@@ -435,10 +431,8 @@ class GraphQLServletSpec extends Specification {
435431
setup:
436432
request.setContentType("multipart/form-data, boundary=test")
437433
request.setMethod("POST")
438-
request.setContent(new TestMultipartContentBuilder()
439-
.addPart('query', 'query one{ echoOne: echo(arg:"test-one") } query two{ echoTwo: echo(arg:"test-two") }')
440-
.addPart('operationName', 'two')
441-
.build())
434+
request.addPart(TestMultipartContentBuilder.createPart('query', 'query one{ echoOne: echo(arg:"test-one") } query two{ echoTwo: echo(arg:"test-two") }'))
435+
request.addPart(TestMultipartContentBuilder.createPart('operationName', 'two'))
442436

443437
when:
444438
servlet.doPost(request, response)
@@ -454,10 +448,8 @@ class GraphQLServletSpec extends Specification {
454448
setup:
455449
request.setContentType("multipart/form-data, boundary=test")
456450
request.setMethod("POST")
457-
request.setContent(new TestMultipartContentBuilder()
458-
.addPart('query', 'query echo{ echo: echo(arg:"test") }')
459-
.addPart('operationName', '')
460-
.build())
451+
request.addPart(TestMultipartContentBuilder.createPart('query', 'query echo{ echo: echo(arg:"test") }'))
452+
request.addPart(TestMultipartContentBuilder.createPart('operationName', ''))
461453

462454
when:
463455
servlet.doPost(request, response)
@@ -472,10 +464,8 @@ class GraphQLServletSpec extends Specification {
472464
setup:
473465
request.setContentType("multipart/form-data, boundary=test")
474466
request.setMethod("POST")
475-
request.setContent(new TestMultipartContentBuilder()
476-
.addPart('query', 'query Echo($arg: String) { echo(arg:$arg) }')
477-
.addPart('variables', '{"arg": "test"}')
478-
.build())
467+
request.addPart(TestMultipartContentBuilder.createPart('query', 'query Echo($arg: String) { echo(arg:$arg) }'))
468+
request.addPart(TestMultipartContentBuilder.createPart('variables', '{"arg": "test"}'))
479469

480470
when:
481471
servlet.doPost(request, response)
@@ -490,10 +480,8 @@ class GraphQLServletSpec extends Specification {
490480
setup:
491481
request.setContentType("multipart/form-data, boundary=test")
492482
request.setMethod("POST")
493-
request.setContent(new TestMultipartContentBuilder()
494-
.addPart('query', 'query { echo(arg:"test") }')
495-
.addPart('test', 'test')
496-
.build())
483+
request.addPart(TestMultipartContentBuilder.createPart('query', 'query { echo(arg:"test") }'))
484+
request.addPart(TestMultipartContentBuilder.createPart('test', 'test'))
497485

498486
when:
499487
servlet.doPost(request, response)
@@ -581,9 +569,7 @@ class GraphQLServletSpec extends Specification {
581569
request.setContentType("multipart/form-data, boundary=test")
582570
request.setMethod("POST")
583571

584-
request.setContent(new TestMultipartContentBuilder()
585-
.addPart('graphql', '[{ "query": "query { echo(arg:\\"test\\") }" }, { "query": "query { echo(arg:\\"test\\") }" }]')
586-
.build())
572+
request.addPart(TestMultipartContentBuilder.createPart('graphql', '[{ "query": "query { echo(arg:\\"test\\") }" }, { "query": "query { echo(arg:\\"test\\") }" }]'))
587573

588574
when:
589575
servlet.doPost(request, response)
@@ -600,9 +586,7 @@ class GraphQLServletSpec extends Specification {
600586
request.setContentType("multipart/form-data, boundary=test")
601587
request.setMethod("POST")
602588

603-
request.setContent(new TestMultipartContentBuilder()
604-
.addPart('graphql', '[{ "query": "query { echo(arg:\\"test\\") }", "test": "test" }, { "query": "query { echo(arg:\\"test\\") }", "test": "test" }]')
605-
.build())
589+
request.addPart(TestMultipartContentBuilder.createPart('graphql', '[{ "query": "query { echo(arg:\\"test\\") }", "test": "test" }, { "query": "query { echo(arg:\\"test\\") }", "test": "test" }]'))
606590

607591
when:
608592
servlet.doPost(request, response)
@@ -618,9 +602,7 @@ class GraphQLServletSpec extends Specification {
618602
setup:
619603
request.setContentType("multipart/form-data, boundary=test")
620604
request.setMethod("POST")
621-
request.setContent(new TestMultipartContentBuilder()
622-
.addPart('query', '[{ "query": "query { echo(arg:\\"test\\") }" }, { "query": "query { echo(arg:\\"test\\") }" }]')
623-
.build())
605+
request.addPart(TestMultipartContentBuilder.createPart('query', '[{ "query": "query { echo(arg:\\"test\\") }" }, { "query": "query { echo(arg:\\"test\\") }" }]'))
624606

625607
when:
626608
servlet.doPost(request, response)
@@ -636,9 +618,7 @@ class GraphQLServletSpec extends Specification {
636618
setup:
637619
request.setContentType("multipart/form-data, boundary=test")
638620
request.setMethod("POST")
639-
request.setContent(new TestMultipartContentBuilder()
640-
.addPart('query', '[{ "query": "query one{ echoOne: echo(arg:\\"test-one\\") } query two{ echoTwo: echo(arg:\\"test-two\\") }", "operationName": "one" }, { "query": "query one{ echoOne: echo(arg:\\"test-one\\") } query two{ echoTwo: echo(arg:\\"test-two\\") }", "operationName": "two" }]')
641-
.build())
621+
request.addPart(TestMultipartContentBuilder.createPart('query', '[{ "query": "query one{ echoOne: echo(arg:\\"test-one\\") } query two{ echoTwo: echo(arg:\\"test-two\\") }", "operationName": "one" }, { "query": "query one{ echoOne: echo(arg:\\"test-one\\") } query two{ echoTwo: echo(arg:\\"test-two\\") }", "operationName": "two" }]'))
642622

643623
when:
644624
servlet.doPost(request, response)
@@ -656,9 +636,7 @@ class GraphQLServletSpec extends Specification {
656636
setup:
657637
request.setContentType("multipart/form-data, boundary=test")
658638
request.setMethod("POST")
659-
request.setContent(new TestMultipartContentBuilder()
660-
.addPart('query', '[{ "query": "query echo{ echo: echo(arg:\\"test\\") }", "operationName": "" }, { "query": "query echo{ echo: echo(arg:\\"test\\") }", "operationName": "" }]')
661-
.build())
639+
request.addPart(TestMultipartContentBuilder.createPart('query', '[{ "query": "query echo{ echo: echo(arg:\\"test\\") }", "operationName": "" }, { "query": "query echo{ echo: echo(arg:\\"test\\") }", "operationName": "" }]'))
662640

663641
when:
664642
servlet.doPost(request, response)
@@ -674,9 +652,7 @@ class GraphQLServletSpec extends Specification {
674652
setup:
675653
request.setContentType("multipart/form-data, boundary=test")
676654
request.setMethod("POST")
677-
request.setContent(new TestMultipartContentBuilder()
678-
.addPart('query', '[{ "query": "query echo($arg: String) { echo(arg:$arg) }", "variables": { "arg": "test" } }, { "query": "query echo($arg: String) { echo(arg:$arg) }", "variables": { "arg": "test" } }]')
679-
.build())
655+
request.addPart(TestMultipartContentBuilder.createPart('query', '[{ "query": "query echo($arg: String) { echo(arg:$arg) }", "variables": { "arg": "test" } }, { "query": "query echo($arg: String) { echo(arg:$arg) }", "variables": { "arg": "test" } }]'))
680656

681657
when:
682658
servlet.doPost(request, response)
@@ -692,9 +668,7 @@ class GraphQLServletSpec extends Specification {
692668
setup:
693669
request.setContentType("multipart/form-data, boundary=test")
694670
request.setMethod("POST")
695-
request.setContent(new TestMultipartContentBuilder()
696-
.addPart('query', '[{ "query": "query { echo(arg:\\"test\\") }", "test": "test" }, { "query": "query { echo(arg:\\"test\\") }", "test": "test" }]')
697-
.build())
671+
request.addPart(TestMultipartContentBuilder.createPart('query', '[{ "query": "query { echo(arg:\\"test\\") }", "test": "test" }, { "query": "query { echo(arg:\\"test\\") }", "test": "test" }]'))
698672

699673
when:
700674
servlet.doPost(request, response)
@@ -867,6 +841,7 @@ class GraphQLServletSpec extends Specification {
867841
mockInputStream.markSupported() >> true
868842
mockRequest.getInputStream() >> mockInputStream
869843
mockRequest.getMethod() >> "POST"
844+
mockRequest.getParts() >> Collections.emptyList()
870845

871846
when:
872847
servlet.doPost(mockRequest, response)

0 commit comments

Comments
 (0)