Revert "Allow dynamic schema resolution (apollographql#447)"

This reverts commit 40c3c3c.
bikov · Nov 3, 2019 · 0fb62d2 · 0fb62d2
1 parent 2033ee5
commit 0fb62d2
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 102 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,7 +1,6 @@
 # Changelog
 
 ### vNEXT
-- Allow dynamically specifying/overriding the schema in the object returned from `onOperation` [PR #447](https://github.com/apollographql/subscriptions-transport-ws/pull/447)
 
 ### v0.9.13
 - Allow connectionParams to be a Promise [PR #443](https://github.com/apollographql/subscriptions-transport-ws/pull/443)

diff --git a/README.md b/README.md
@@ -303,10 +303,10 @@ ReactDOM.render(
 ### `Constructor(options, socketOptions | socketServer)`
 - `options: {ServerOptions}`
   * `rootValue?: any` : Root value to use when executing GraphQL root operations
-  * `schema?: GraphQLSchema` : GraphQL schema object. If not provided, you have to return the schema as a property on the object returned from `onOperation`.
+  * `schema?: GraphQLSchema` : GraphQL schema object
   * `execute?: (schema, document, rootValue, contextValue, variableValues, operationName) => Promise<ExecutionResult> | AsyncIterator<ExecutionResult>` : GraphQL `execute` function, provide the default one from `graphql` package. Return value of `AsyncItrator` is also valid since this package also support reactive `execute` methods.
   * `subscribe?: (schema, document, rootValue, contextValue, variableValues, operationName) => Promise<ExecutionResult | AsyncIterator<ExecutionResult>>` : GraphQL `subscribe` function, provide the default one from `graphql` package.
-  * `onOperation?: (message: SubscribeMessage, params: SubscriptionOptions, webSocket: WebSocket)` : optional method to create custom params that will be used when resolving this operation. It can also be used to dynamically resolve the schema that will be used for the particular operation.
+  * `onOperation?: (message: SubscribeMessage, params: SubscriptionOptions, webSocket: WebSocket)` : optional method to create custom params that will be used when resolving this operation
   * `onOperationComplete?: (webSocket: WebSocket, opId: string)` : optional method that called when a GraphQL operation is done (for query and mutation it's immediately, and for subscriptions when unsubscribing)
   * `onConnect?: (connectionParams: Object, webSocket: WebSocket, context: ConnectionContext)` : optional method that called when a client connects to the socket, called with the `connectionParams` from the client, if the return value is an object, its elements will be added to the context. return `false` or throw an exception to reject the connection. May return a Promise.
   * `onDisconnect?: (webSocket: WebSocket, context: ConnectionContext)` : optional method that called when a client disconnects

diff --git a/docs/source/lifecycle-events.md b/docs/source/lifecycle-events.md
@@ -6,7 +6,7 @@ title: Lifecycle Events
 
 * `onConnect` - called upon client connection, with the `connectionParams` passed to `SubscriptionsClient` - you can return a Promise and reject the connection by throwing an exception. The resolved return value will be appended to the GraphQL `context` of your subscriptions.
 * `onDisconnect` - called when the client disconnects.
-* `onOperation` - called when the client executes a GraphQL operation - use this method to create custom params that will be used when resolving the operation. You can use this method to override the GraphQL schema that will be used in the operation.
+* `onOperation` - called when the client executes a GraphQL operation - use this method to create custom params that will be used when resolving the operation.
 * `onOperationComplete` - called when client's operation has been done it's execution (for subscriptions called when unsubscribe, and for query/mutation called immediately).
 
 ```js
@@ -16,15 +16,7 @@ const subscriptionsServer = new SubscriptionServer(
       // ...
     },
     onOperation: (message, params, webSocket) => {
-      // Manipulate and return the params, e.g.
-      params.context.randomId = uuid.v4();
-
-      // Or specify a schema override
-      if (shouldOverrideSchema()) {
-        params.schema = newSchema;
-      }
-
-      return params;
+      // ...
     },
     onOperationComplete: webSocket => {
       // ...

diff --git a/src/server.ts b/src/server.ts
@@ -29,7 +29,6 @@ export interface ExecutionParams<TContext = any> {
   formatResponse?: Function;
   formatError?: Function;
   callback?: Function;
-  schema?: GraphQLSchema;
 }
 
 export type ConnectionContext = {
@@ -203,6 +202,10 @@ export class SubscriptionServer {
       throw new Error('Must provide `execute` for websocket server constructor.');
     }
 
+    if (!schema) {
+      throw new Error('`schema` is missing');
+    }
+
     this.schema = schema;
     this.rootValue = rootValue;
     this.execute = execute;
@@ -315,7 +318,6 @@ export class SubscriptionServer {
               formatResponse: <any>undefined,
               formatError: <any>undefined,
               callback: <any>undefined,
-              schema: this.schema,
             };
             let promisedParams = Promise.resolve(baseParams);
 
@@ -327,25 +329,17 @@ export class SubscriptionServer {
               promisedParams = Promise.resolve(this.onOperation(messageForCallback, baseParams, connectionContext.socket));
             }
 
-            promisedParams.then((params) => {
+            promisedParams.then((params: any) => {
               if (typeof params !== 'object') {
                 const error = `Invalid params returned from onOperation! return values must be an object!`;
                 this.sendError(connectionContext, opId, { message: error });
 
                 throw new Error(error);
               }
 
-              if (!params.schema) {
-                const error = 'Missing schema information. The GraphQL schema should be provided either statically in' +
-                  ' the `SubscriptionServer` constructor or as a property on the object returned from onOperation!';
-                this.sendError(connectionContext, opId, { message: error });
-
-                throw new Error(error);
-              }
-
               const document = typeof baseParams.query !== 'string' ? baseParams.query : parse(baseParams.query);
               let executionPromise: Promise<AsyncIterator<ExecutionResult> | ExecutionResult>;
-              const validationErrors = validate(params.schema, document, this.specifiedRules);
+              const validationErrors = validate(this.schema, document, this.specifiedRules);
 
               if ( validationErrors.length > 0 ) {
                 executionPromise = Promise.resolve({ errors: validationErrors });
@@ -354,7 +348,7 @@ export class SubscriptionServer {
                 if (this.subscribe && isASubscriptionOperation(document, params.operationName)) {
                   executor = this.subscribe;
                 }
-                executionPromise = Promise.resolve(executor(params.schema,
+                executionPromise = Promise.resolve(executor(this.schema,
                   document,
                   this.rootValue,
                   params.context,

diff --git a/src/test/tests.ts b/src/test/tests.ts
@@ -1389,81 +1389,10 @@ describe('Server', function () {
     }).to.throw();
   });
 
-  it('should throw an exception when schema is not provided', (done) => {
-    server = createServer(notFoundRequestListener);
-    server.listen(SERVER_EXECUTOR_TESTS_PORT);
-
-    SubscriptionServer.create({
-      execute,
-    }, {
-      server,
-      path: '/',
-    });
-
-    let errorMessage: string;
-
-    const client = new SubscriptionClient(`ws://localhost:${SERVER_EXECUTOR_TESTS_PORT}/`);
-    client.onConnected(() => {
-      client.request({
-        query: `query { testString }`,
-        variables: {},
-      }).subscribe({
-        next: (res) => {
-          assert(false, 'expected error to be thrown');
-        },
-        error: (err) => {
-          errorMessage = err.message;
-          expect(errorMessage).to.contain('Missing schema information');
-          done();
-        },
-        complete: () => {
-          assert(false, 'expected error to be thrown');
-        },
-      });
-    });
-  });
-
-  it('should use schema provided in onOperation', (done) => {
-    server = createServer(notFoundRequestListener);
-    server.listen(SERVER_EXECUTOR_TESTS_PORT);
-
-    SubscriptionServer.create({
-      execute,
-      onOperation: () => {
-        return {
-          schema,
-        };
-      },
-    }, {
-      server,
-      path: '/',
-    });
-
-    let msgCnt = 0;
-
-    const client = new SubscriptionClient(`ws://localhost:${SERVER_EXECUTOR_TESTS_PORT}/`);
-    client.onConnected(() => {
-      client.request({
-        query: `query { testString }`,
-        variables: {},
-      }).subscribe({
-        next: (res) => {
-          if ( res.errors ) {
-            assert(false, 'unexpected error from request');
-          }
-
-          expect(res.data).to.deep.equal({ testString: 'value' });
-          msgCnt ++;
-        },
-        error: (err) => {
-          assert(false, 'unexpected error from request');
-        },
-        complete: () => {
-          expect(msgCnt).to.equals(1);
-          done();
-        },
-      });
-    });
+  it('should throw an exception when using execute but schema is missing', () => {
+    expect(() => {
+      new SubscriptionServer({ execute: {} as any }, { server: httpServer });
+    }).to.throw();
   });
 
   it('should accept execute method than returns a Promise (original execute)', (done) => {
@@ -1927,7 +1856,7 @@ describe('Server', function () {
       client1.unsubscribeAll();
       expect(numResults1).to.equals(1);
       done();
-    }, 400);
+    }, 300);
 
   });