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,6 +14,8 @@ 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,
@@ -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: [
+          typeDefs: this.uploadsConfig
+            ? [
                 gql`
                   scalar Upload
                 `,
-          ].concat(typeDefs),
+              ].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,7 +419,6 @@ 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,
@@ -431,11 +430,6 @@ describe('apollo-server-express', () => {
           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;
-        }
       });
     });
 

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,6 +38,10 @@ export class ApolloServer extends ApolloServerBase {
     return true;
   }
 
+  protected supportsUploads(): boolean {
+    return true;
+  }
+
   public async applyMiddleware({
     app,
     cors,
@@ -48,21 +49,9 @@ export class ApolloServer extends ApolloServerBase {
     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) {
@@ -70,10 +59,8 @@ export class ApolloServer extends ApolloServerBase {
           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