Move Uploads to constructor and remove enhanceSchema (#1204)

* move uploads into server constructor

* remove enhanceSchema

* address feedback

packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts

@@ -3,7 +3,7 @@ import {
   ApolloError,
   AuthenticationError,
   ForbiddenError,
-} from 'apollo-server-core';
+} from 'apollo-server-errors';
 import { RESTDataSource } from '../RESTDataSource';
 
 import fetch, { mockFetch, unmockFetch } from '../../../../__mocks__/fetch';

packages/apollo-server-core/src/ApolloServer.ts

@@ -1,8 +1,4 @@
-import {
-  makeExecutableSchema,
-  addMockFunctionsToSchema,
-  mergeSchemas,
-} from 'graphql-tools';
+import { makeExecutableSchema, addMockFunctionsToSchema } from 'graphql-tools';
 import { Server as HttpServer } from 'http';
 import {
   execute,
@@ -18,12 +14,14 @@ import { GraphQLExtension } from 'graphql-extensions';
 import { EngineReportingAgent } from 'apollo-engine-reporting';
 import { InMemoryLRUCache } from 'apollo-datasource-rest';
 
+import { GraphQLUpload } from 'apollo-upload-server';
+
 import {
   SubscriptionServer,
   ExecutionParams,
 } from 'subscriptions-transport-ws';
 
-//use as default persisted query store
+// use as default persisted query store
 import Keyv = require('keyv');
 import QuickLru = require('quick-lru');
 
@@ -38,6 +36,7 @@ import {
   Context,
   ContextFunction,
   SubscriptionServerOptions,
+  FileUploadOptions,
 } from './types';
 
 import { gql } from './index';
@@ -65,6 +64,7 @@ export class ApolloServerBase {
   private engineReportingAgent?: EngineReportingAgent;
   private extensions: Array<() => GraphQLExtension>;
   protected subscriptionServerOptions?: SubscriptionServerOptions;
+  protected uploadsConfig?: FileUploadOptions;
 
   // set by installSubscriptionHandlers.
   private subscriptionServer?: SubscriptionServer;
@@ -83,6 +83,7 @@ export class ApolloServerBase {
       extensions,
       engine,
       subscriptions,
+      uploads,
       ...requestOptions
     } = config;
 
@@ -128,16 +129,41 @@ export class ApolloServerBase {
     this.requestOptions = requestOptions as GraphQLOptions;
     this.context = context;
 
+    if (uploads !== false) {
+      if (this.supportsUploads()) {
+        if (uploads === true || typeof uploads === 'undefined') {
+          this.uploadsConfig = {};
+        } else {
+          this.uploadsConfig = uploads;
+        }
+        //This is here to check if uploads is requested without support. By
+        //default we enable them if supported by the integration
+      } else if (uploads) {
+        throw new Error(
+          'This implementation of ApolloServer does not support file uploads because the environmnet cannot accept multi-part forms',
+        );
+      }
+    }
+
+    //Add upload resolver
+    if (this.uploadsConfig) {
+      if (resolvers && !resolvers.Upload) {
+        resolvers.Upload = GraphQLUpload;
+      }
+    }
+
     this.schema = schema
       ? schema
       : makeExecutableSchema({
           //we add in the upload scalar, so that schemas that don't include it
           //won't error when we makeExecutableSchema
-          typeDefs: [
-            gql`
-              scalar Upload
-            `,
-          ].concat(typeDefs),
+          typeDefs: this.uploadsConfig
+            ? [
+                gql`
+                  scalar Upload
+                `,
+              ].concat(typeDefs)
+            : typeDefs,
           schemaDirectives,
           resolvers,
         });
@@ -182,6 +208,9 @@ export class ApolloServerBase {
         }
         // This is part of the public API.
         this.subscriptionsPath = this.subscriptionServerOptions.path;
+
+        //This is here to check if subscriptions are requested without support. By
+        //default we enable them if supported by the integration
       } else if (subscriptions) {
         throw new Error(
           'This implementation of ApolloServer does not support GraphQL subscriptions.',
@@ -196,14 +225,6 @@ export class ApolloServerBase {
     this.graphqlPath = path;
   }
 
-  // If this is more generally useful to things other than Upload, we can make
-  // it public.
-  protected enhanceSchema(schema: GraphQLSchema) {
-    this.schema = mergeSchemas({
-      schemas: [this.schema, schema],
-    });
-  }
-
   public async stop() {
     if (this.subscriptionServer) await this.subscriptionServer.close();
     if (this.engineReportingAgent) {
@@ -280,6 +301,10 @@ export class ApolloServerBase {
     return false;
   }
 
+  protected supportsUploads(): boolean {
+    return false;
+  }
+
   //This function is used by the integrations to generate the graphQLOptions
   //from an object containing the request and other integration specific
   //options

packages/apollo-server-core/src/types.ts

@@ -29,6 +29,8 @@ export interface SubscriptionServerOptions {
   onDisconnect?: (websocket: WebSocket, context: ConnectionContext) => any;
 }
 
+// This configuration is shared between all integrations and should include
+// fields that are not specific to a single integration
 export interface Config
   extends Pick<
       GraphQLOptions<Context<any>>,
@@ -55,6 +57,17 @@ export interface Config
   extensions?: Array<() => GraphQLExtension>;
   persistedQueries?: PersistedQueryOptions | false;
   subscriptions?: Partial<SubscriptionServerOptions> | string | false;
+  //https://github.com/jaydenseric/apollo-upload-server#options
+  uploads?: boolean | FileUploadOptions;
+}
+
+export interface FileUploadOptions {
+  //Max allowed non-file multipart form field size in bytes; enough for your queries (default: 1 MB).
+  maxFieldSize?: number;
+  //Max allowed file size in bytes (default: Infinity).
+  maxFileSize?: number;
+  //Max allowed number of files (default: Infinity).
+  maxFiles?: number;
 }
 
 export interface MiddlewareOptions {

packages/apollo-server-express/src/ApolloServer.test.ts

@@ -419,23 +419,17 @@ describe('apollo-server-express', () => {
         body.append('map', JSON.stringify({ 1: ['variables.file'] }));
         body.append('1', fs.createReadStream('package.json'));
 
-        try {
-          const resolved = await fetch(`http://localhost:${port}/graphql`, {
-            method: 'POST',
-            body,
-          });
-          const response = await resolved.json();
+        const resolved = await fetch(`http://localhost:${port}/graphql`, {
+          method: 'POST',
+          body,
+        });
+        const response = await resolved.json();
 
-          expect(response.data.singleUpload).to.deep.equal({
-            filename: 'package.json',
-            encoding: '7bit',
-            mimetype: 'application/json',
-          });
-        } catch (error) {
-          // This error began appearing randomly and seems to be a dev dependency bug.
-          // https://github.com/jaydenseric/apollo-upload-server/blob/18ecdbc7a1f8b69ad51b4affbd986400033303d4/test.js#L39-L42
-          if (error.code !== 'EPIPE') throw error;
-        }
+        expect(response.data.singleUpload).to.deep.equal({
+          filename: 'package.json',
+          encoding: '7bit',
+          mimetype: 'application/json',
+        });
       });
     });
 

packages/apollo-server-express/src/ApolloServer.ts

@@ -9,13 +9,10 @@ import * as typeis from 'type-is';
 
 import { graphqlExpress } from './expressApollo';
 
-import {
-  processRequest as processFileUploads,
-  GraphQLUpload,
-} from 'apollo-upload-server';
+import { processRequest as processFileUploads } from 'apollo-upload-server';
 
 export { GraphQLOptions, GraphQLExtension } from 'apollo-server-core';
-import { GraphQLOptions, gql, makeExecutableSchema } from 'apollo-server-core';
+import { GraphQLOptions, FileUploadOptions } from 'apollo-server-core';
 
 export interface ServerRegistration {
   // Note: You can also pass a connect.Server here. If we changed this field to
@@ -31,12 +28,10 @@ export interface ServerRegistration {
   onHealthCheck?: (req: express.Request) => Promise<any>;
   disableHealthCheck?: boolean;
   gui?: boolean | PlaygroundMiddlewareOptions;
-  //https://github.com/jaydenseric/apollo-upload-server#options
-  uploads?: boolean | Record<string, any>;
 }
 
 const fileUploadMiddleware = (
-  uploadsConfig: Record<string, any>,
+  uploadsConfig: FileUploadOptions,
   server: ApolloServerBase,
 ) => (
   req: express.Request,
@@ -80,6 +75,10 @@ export class ApolloServer extends ApolloServerBase {
     return true;
   }
 
+  protected supportsUploads(): boolean {
+    return true;
+  }
+
   public applyMiddleware({
     app,
     path,
@@ -88,7 +87,6 @@ export class ApolloServer extends ApolloServerBase {
     disableHealthCheck,
     gui,
     onHealthCheck,
-    uploads,
   }: ServerRegistration) {
     if (!path) path = '/graphql';
 
@@ -113,20 +111,8 @@ export class ApolloServer extends ApolloServerBase {
     }
 
     let uploadsMiddleware;
-    if (uploads !== false) {
-      this.enhanceSchema(
-        makeExecutableSchema({
-          typeDefs: gql`
-            scalar Upload
-          `,
-          resolvers: { Upload: GraphQLUpload },
-        }),
-      );
-
-      uploadsMiddleware = fileUploadMiddleware(
-        typeof uploads !== 'boolean' ? uploads : {},
-        this,
-      );
+    if (this.uploadsConfig) {
+      uploadsMiddleware = fileUploadMiddleware(this.uploadsConfig, this);
     }
 
     // XXX multiple paths?

packages/apollo-server-hapi/src/ApolloServer.ts

@@ -5,17 +5,14 @@ import {
   renderPlaygroundPage,
   MiddlewareOptions as PlaygroundMiddlewareOptions,
 } from 'graphql-playground-html';
-import {
-  processRequest as processFileUploads,
-  GraphQLUpload,
-} from 'apollo-upload-server';
+import { processRequest as processFileUploads } from 'apollo-upload-server';
 
 import { graphqlHapi } from './hapiApollo';
 
 export { GraphQLOptions, GraphQLExtension } from 'apollo-server-core';
-import { GraphQLOptions, gql, makeExecutableSchema } from 'apollo-server-core';
+import { GraphQLOptions, FileUploadOptions } from 'apollo-server-core';
 
-function handleFileUploads(uploadsConfig: Record<string, any>) {
+function handleFileUploads(uploadsConfig: FileUploadOptions) {
   return async (request: hapi.Request) => {
     if (request.mime === 'multipart/form-data') {
       Object.defineProperty(request, 'payload', {
@@ -41,39 +38,29 @@ export class ApolloServer extends ApolloServerBase {
     return true;
   }
 
+  protected supportsUploads(): boolean {
+    return true;
+  }
+
   public async applyMiddleware({
     app,
     cors,
     path,
     disableHealthCheck,
     gui,
     onHealthCheck,
-    uploads,
   }: ServerRegistration) {
     if (!path) path = '/graphql';
 
-    if (uploads !== false) {
-      this.enhanceSchema(
-        makeExecutableSchema({
-          typeDefs: gql`
-            scalar Upload
-          `,
-          resolvers: { Upload: GraphQLUpload },
-        }),
-      );
-    }
-
     await app.ext({
       type: 'onRequest',
       method: async function(request, h) {
         if (request.path !== path) {
           return h.continue;
         }
 
-        if (uploads !== false) {
-          await handleFileUploads(typeof uploads !== 'boolean' ? uploads : {})(
-            request,
-          );
+        if (this.uploadsConfig) {
+          await handleFileUploads(this.uploadsConfig)(request);
         }
 
         // Note: if you enable a gui in production and expect to be able to see your