Renamed some static methods, removed file parts from context builder

jmccaull · jmccaull · commit a23067a66d6f · 2019-06-24T16:12:10.000-04:00
diff --git a/src/main/java/graphql/servlet/AbstractGraphQLHttpServlet.java b/src/main/java/graphql/servlet/AbstractGraphQLHttpServlet.java
@@ -203,7 +203,7 @@ public void init() {
                                     graphQLObjectMapper.readBatchedGraphQLRequest(inputStream);
                             variablesMap.ifPresent(map -> graphQLRequests.forEach(r -> mapMultipartVariables(r, map, fileItems)));
                             GraphQLBatchedInvocationInput batchedInvocationInput = invocationInputFactory.create(configuration.getContextSetting(),
-                                graphQLRequests, request, response, fileItems);
+                                graphQLRequests, request, response);
                             queryBatched(queryInvoker, batchedInvocationInput, request, response, configuration);
                             return;
                         } else {
@@ -216,7 +216,7 @@ public void init() {
 
                             variablesMap.ifPresent(m -> mapMultipartVariables(graphQLRequest, m, fileItems));
                             GraphQLSingleInvocationInput invocationInput =
-                                    invocationInputFactory.create(graphQLRequest, request, response, fileItems);
+                                    invocationInputFactory.create(graphQLRequest, request, response);
                             query(queryInvoker, graphQLObjectMapper, invocationInput, request, response);
                             return;
                         }
diff --git a/src/main/java/graphql/servlet/context/DefaultGraphQLContextBuilder.java b/src/main/java/graphql/servlet/context/DefaultGraphQLContextBuilder.java
@@ -14,13 +14,13 @@
 public class DefaultGraphQLContextBuilder implements GraphQLContextBuilder {
 
     @Override
-    public GraphQLContext build(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Map<String, List<Part>> fileParts) {
-        return DefaultGraphQLServletContext.createWebContext().with(httpServletRequest).with(httpServletResponse).with(fileParts).build();
+    public GraphQLContext build(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) {
+        return DefaultGraphQLServletContext.createServletContext().with(httpServletRequest).with(httpServletResponse).build();
     }
 
     @Override
     public GraphQLContext build(Session session, HandshakeRequest handshakeRequest) {
-        return DefaultGraphQLWebSocketContext.createWebContext().with(session).with(handshakeRequest).build();
+        return DefaultGraphQLWebSocketContext.createWebSocketContext().with(session).with(handshakeRequest).build();
     }
 
     @Override
diff --git a/src/main/java/graphql/servlet/context/DefaultGraphQLServletContext.java b/src/main/java/graphql/servlet/context/DefaultGraphQLServletContext.java
@@ -6,8 +6,6 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.Part;
-import java.util.Collection;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
@@ -16,15 +14,12 @@ class DefaultGraphQLServletContext extends DefaultGraphQLContext implements Grap
 
     private final HttpServletRequest httpServletRequest;
     private final HttpServletResponse httpServletResponse;
-    private final Map<String, List<Part>> parts;
 
     private DefaultGraphQLServletContext(DataLoaderRegistry dataLoaderRegistry, Subject subject, HttpServletRequest httpServletRequest,
-                                         HttpServletResponse httpServletResponse,
-                                         Map<String, List<Part>> parts) {
+                                         HttpServletResponse httpServletResponse) {
         super(dataLoaderRegistry, subject);
         this.httpServletRequest = httpServletRequest;
         this.httpServletResponse = httpServletResponse;
-        this.parts = parts;
     }
 
     @Override
@@ -39,30 +34,37 @@ public HttpServletResponse getHttpServletResponse() {
 
     @Override
     public List<Part> getFileParts() {
-        return parts.values()
-                .stream()
-                .flatMap(Collection::stream)
-                .filter(part -> part.getContentType() != null)
-                .collect(Collectors.toList());
+        try {
+            return httpServletRequest.getParts().stream()
+                    .filter(part -> part.getContentType() != null)
+                    .collect(Collectors.toList());
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
     }
 
     @Override
     public Map<String, List<Part>> getParts() {
-        return parts;
+        try {
+            return httpServletRequest.getParts()
+                .stream()
+                .collect(Collectors.groupingBy(Part::getName));
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
     }
 
-    public static Builder createWebContext(DataLoaderRegistry registry, Subject subject) {
+    public static Builder createServletContext(DataLoaderRegistry registry, Subject subject) {
         return new Builder(registry, subject);
     }
 
-    public static Builder createWebContext() {
+    public static Builder createServletContext() {
         return new Builder(null, null);
     }
 
     public static class Builder {
         private HttpServletRequest httpServletRequest;
         private HttpServletResponse httpServletResponse;
-        private Map<String, List<Part>> parts = new HashMap<>();
         private DataLoaderRegistry dataLoaderRegistry;
         private Subject subject;
 
@@ -72,23 +74,26 @@ private Builder(DataLoaderRegistry dataLoaderRegistry, Subject subject) {
         }
 
         public DefaultGraphQLServletContext build() {
-            return new DefaultGraphQLServletContext(dataLoaderRegistry, subject, httpServletRequest, httpServletResponse, parts);
+            return new DefaultGraphQLServletContext(dataLoaderRegistry, subject, httpServletRequest, httpServletResponse);
         }
 
         public Builder with(HttpServletRequest httpServletRequest) {
             this.httpServletRequest = httpServletRequest;
             return this;
         }
 
-        public Builder with(HttpServletResponse httpServletResponse) {
-            this.httpServletResponse = httpServletResponse;
+        public Builder with(DataLoaderRegistry dataLoaderRegistry) {
+            this.dataLoaderRegistry = dataLoaderRegistry;
             return this;
         }
 
-        public Builder with(Map<String, List<Part>> parts) {
-            if (parts != null) {
-                this.parts.putAll(parts);
-            }
+        public Builder with(Subject subject) {
+            this.subject = subject;
+            return this;
+        }
+
+        public Builder with(HttpServletResponse httpServletResponse) {
+            this.httpServletResponse = httpServletResponse;
             return this;
         }
     }
diff --git a/src/main/java/graphql/servlet/context/DefaultGraphQLWebSocketContext.java b/src/main/java/graphql/servlet/context/DefaultGraphQLWebSocketContext.java
@@ -35,11 +35,11 @@ public HandshakeRequest getHandshakeRequest() {
         return handshakeRequest;
     }
 
-    public static Builder createWebContext(DataLoaderRegistry registry, Subject subject) {
+    public static Builder createWebSocketContext(DataLoaderRegistry registry, Subject subject) {
         return new Builder(registry, subject);
     }
 
-    public static Builder createWebContext() {
+    public static Builder createWebSocketContext() {
         return new Builder(null, null);
     }
 
@@ -68,5 +68,15 @@ public Builder with(HandshakeRequest handshakeRequest) {
             return this;
         }
 
+        public Builder with(DataLoaderRegistry dataLoaderRegistry) {
+            this.dataLoaderRegistry = dataLoaderRegistry;
+            return this;
+        }
+
+        public Builder with(Subject subject) {
+            this.subject = subject;
+            return this;
+        }
+
     }
 }
diff --git a/src/main/java/graphql/servlet/context/GraphQLContextBuilder.java b/src/main/java/graphql/servlet/context/GraphQLContextBuilder.java
@@ -2,15 +2,12 @@
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-import javax.servlet.http.Part;
 import javax.websocket.Session;
 import javax.websocket.server.HandshakeRequest;
-import java.util.List;
-import java.util.Map;
 
 public interface GraphQLContextBuilder {
 
-    GraphQLContext build(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Map<String, List<Part>> fileParts);
+    GraphQLContext build(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse);
 
     GraphQLContext build(Session session, HandshakeRequest handshakeRequest);
 
diff --git a/src/main/java/graphql/servlet/input/GraphQLInvocationInputFactory.java b/src/main/java/graphql/servlet/input/GraphQLInvocationInputFactory.java
@@ -39,30 +39,21 @@ public GraphQLSchemaProvider getSchemaProvider() {
     }
 
     public GraphQLSingleInvocationInput create(GraphQLRequest graphQLRequest, HttpServletRequest request, HttpServletResponse response) {
-        return create(graphQLRequest, request, response, null);
-    }
-
-    public GraphQLSingleInvocationInput create(GraphQLRequest graphQLRequest, HttpServletRequest request, HttpServletResponse response, Map<String,
-            List<Part>> fileParts) {
-        return create(graphQLRequest, request, response, false, fileParts);
+        return create(graphQLRequest, request, response, false);
     }
 
     public GraphQLBatchedInvocationInput create(ContextSetting contextSetting, List<GraphQLRequest> graphQLRequests, HttpServletRequest request,
                                                 HttpServletResponse response) {
-        return create(contextSetting, graphQLRequests, request, response, null);
+        return create(contextSetting, graphQLRequests, request, response, false);
     }
 
-    public GraphQLBatchedInvocationInput create(ContextSetting contextSetting, List<GraphQLRequest> graphQLRequests, HttpServletRequest request,
-                                                HttpServletResponse response, Map<String, List<Part>> fileParts) {
-        return create(contextSetting, graphQLRequests, request, response, false, fileParts);
-    }
 
     public GraphQLSingleInvocationInput createReadOnly(GraphQLRequest graphQLRequest, HttpServletRequest request, HttpServletResponse response) {
-        return create(graphQLRequest, request, response, true, null);
+        return create(graphQLRequest, request, response, true);
     }
 
     public GraphQLBatchedInvocationInput createReadOnly(ContextSetting contextSetting, List<GraphQLRequest> graphQLRequests, HttpServletRequest request, HttpServletResponse response) {
-        return create(contextSetting, graphQLRequests, request, response, true, null);
+        return create(contextSetting, graphQLRequests, request, response, true);
     }
 
     public GraphQLSingleInvocationInput create(GraphQLRequest graphQLRequest) {
@@ -75,21 +66,21 @@ public GraphQLSingleInvocationInput create(GraphQLRequest graphQLRequest) {
     }
 
     private GraphQLSingleInvocationInput create(GraphQLRequest graphQLRequest, HttpServletRequest request, HttpServletResponse response,
-                                                boolean readOnly, Map<String, List<Part>> fileParts) {
+                                                boolean readOnly) {
         return new GraphQLSingleInvocationInput(
                 graphQLRequest,
                 readOnly ? schemaProviderSupplier.get().getReadOnlySchema(request) : schemaProviderSupplier.get().getSchema(request),
-                contextBuilderSupplier.get().build(request, response, fileParts),
+                contextBuilderSupplier.get().build(request, response),
                 rootObjectBuilderSupplier.get().build(request)
         );
     }
 
     private GraphQLBatchedInvocationInput create(ContextSetting contextSetting, List<GraphQLRequest> graphQLRequests, HttpServletRequest request,
-                                                 HttpServletResponse response, boolean readOnly, Map<String, List<Part>> fileParts) {
+                                                 HttpServletResponse response, boolean readOnly) {
         return contextSetting.getBatch(
                 graphQLRequests,
                 readOnly ? schemaProviderSupplier.get().getReadOnlySchema(request) : schemaProviderSupplier.get().getSchema(request),
-                () -> contextBuilderSupplier.get().build(request, response, fileParts),
+                () -> contextBuilderSupplier.get().build(request, response),
                 rootObjectBuilderSupplier.get().build(request)
         );
     }
diff --git a/src/main/java/graphql/servlet/instrumentation/RequestStack.java b/src/main/java/graphql/servlet/instrumentation/RequestStack.java
@@ -1,22 +1,14 @@
 package graphql.servlet.instrumentation;
 
-import com.sun.org.apache.bcel.internal.generic.Select;
 import graphql.Assert;
 import graphql.execution.ExecutionId;
-import graphql.language.Selection;
-import graphql.language.SelectionSet;
-import graphql.schema.GraphQLOutputType;
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Iterator;
 import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Set;
-import java.util.stream.Collectors;
 
 /**
  *  Manages sets of call stack state for ongoing executions.
diff --git a/src/test/groovy/graphql/servlet/DataLoaderDispatchingSpec.groovy b/src/test/groovy/graphql/servlet/DataLoaderDispatchingSpec.groovy
@@ -81,7 +81,7 @@ class DataLoaderDispatchingSpec extends Specification {
     def contextBuilder () {
         return new GraphQLContextBuilder() {
             @Override
-            GraphQLContext build(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Map<String, List<Part>> fileParts) {
+            GraphQLContext build(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) {
                 new DefaultGraphQLContext(registry(), null)
             }
 

Original file line number	Diff line number	Diff line change
`@@ -14,13 +14,13 @@`
`14`	`14`	`public class DefaultGraphQLContextBuilder implements GraphQLContextBuilder {`
`15`	`15`
`16`	`16`	`@Override`
`17`		`- public GraphQLContext build(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Map<String, List<Part>> fileParts) {`
`18`		`- return DefaultGraphQLServletContext.createWebContext().with(httpServletRequest).with(httpServletResponse).with(fileParts).build();`
	`17`	`+ public GraphQLContext build(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) {`
	`18`	`+ return DefaultGraphQLServletContext.createServletContext().with(httpServletRequest).with(httpServletResponse).build();`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`@Override`
`22`	`22`	`public GraphQLContext build(Session session, HandshakeRequest handshakeRequest) {`
`23`		`- return DefaultGraphQLWebSocketContext.createWebContext().with(session).with(handshakeRequest).build();`
	`23`	`+ return DefaultGraphQLWebSocketContext.createWebSocketContext().with(session).with(handshakeRequest).build();`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -35,11 +35,11 @@ public HandshakeRequest getHandshakeRequest() {`
`35`	`35`	`return handshakeRequest;`
`36`	`36`	`}`
`37`	`37`
`38`		`- public static Builder createWebContext(DataLoaderRegistry registry, Subject subject) {`
	`38`	`+ public static Builder createWebSocketContext(DataLoaderRegistry registry, Subject subject) {`
`39`	`39`	`return new Builder(registry, subject);`
`40`	`40`	`}`
`41`	`41`
`42`		`- public static Builder createWebContext() {`
	`42`	`+ public static Builder createWebSocketContext() {`
`43`	`43`	`return new Builder(null, null);`
`44`	`44`	`}`
`45`	`45`
`@@ -68,5 +68,15 @@ public Builder with(HandshakeRequest handshakeRequest) {`
`68`	`68`	`return this;`
`69`	`69`	`}`
`70`	`70`
	`71`	`+ public Builder with(DataLoaderRegistry dataLoaderRegistry) {`
	`72`	`+ this.dataLoaderRegistry = dataLoaderRegistry;`
	`73`	`+ return this;`
	`74`	`+ }`
	`75`	`+`
	`76`	`+ public Builder with(Subject subject) {`
	`77`	`+ this.subject = subject;`
	`78`	`+ return this;`
	`79`	`+ }`
	`80`	`+`
`71`	`81`	`}`
`72`	`82`	`}`
Original file line number	Diff line number	Diff line change
`@@ -81,7 +81,7 @@ class DataLoaderDispatchingSpec extends Specification {`
`81`	`81`	`def contextBuilder () {`
`82`	`82`	`return new GraphQLContextBuilder() {`
`83`	`83`	`@Override`
`84`		`- GraphQLContext build(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Map<String, List<Part>> fileParts) {`
	`84`	`+ GraphQLContext build(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) {`
`85`	`85`	`new DefaultGraphQLContext(registry(), null)`
`86`	`86`	`}`
`87`	`87`