From 049a1a8b4cb77844c72c871421fb35a664d91dc5 Mon Sep 17 00:00:00 2001
From: Carlo <34414634+csuriano23@users.noreply.github.com>
Date: Wed, 21 Sep 2022 15:19:11 +0200
Subject: [PATCH] Fix payload parsing for subscriptions (#1148)

* Fix payload parsing for subscriptions

* Refactor for instance properties access

* Refactor for instance properties access - renaming variable node

* Add tests for parametrical subscriptions

* Improve naming and destructuring
---
 src/subscriptions.ts        | 15 +++-------
 tests/subscriptions.spec.ts | 57 ++++++++++++++++++++++++++++++++++---
 2 files changed, 57 insertions(+), 15 deletions(-)

diff --git a/src/subscriptions.ts b/src/subscriptions.ts
index e40e79a5..45ebaf61 100644
--- a/src/subscriptions.ts
+++ b/src/subscriptions.ts
@@ -84,17 +84,13 @@ export class SubscriptionManager {
       throw new Error(`Subscription '${name}' is not available`);
     }
 
-    const { document, operationName, variables } = this.operations.get(name)!;
-
     logger.info(`[Subscription] Start ${id}`, event);
 
     const result = await this.execute({
       id,
       name,
       url: event.url,
-      document,
-      operationName,
-      variables,
+      variables: event.variables,
       contextValue,
     });
 
@@ -151,22 +147,19 @@ export class SubscriptionManager {
 
   private async execute({
     id,
-    document,
     name,
     url,
-    operationName,
     variables,
     contextValue,
   }: {
     id: ID;
     name: SubscriptionFieldName;
     url: string;
-    document: DocumentNode;
-    operationName: string;
     variables: Record<string, any>;
     contextValue: ContextValue;
   }) {
-    const variableNodes = this.operations.get(name)!.variables;
+    const { document, operationName, variables: variableNodes } = this.operations.get(name)!;
+
     const variableValues = variableNodes.reduce((values, variable) => {
       const value = parseVariable({
         value: variables[variable.variable.name.value],
@@ -180,7 +173,7 @@ export class SubscriptionManager {
 
       return {
         ...values,
-        [name]: value,
+        [variable.variable.name.value]: value,
       };
     }, {});
 
diff --git a/tests/subscriptions.spec.ts b/tests/subscriptions.spec.ts
index 020b6627..db46791f 100644
--- a/tests/subscriptions.spec.ts
+++ b/tests/subscriptions.spec.ts
@@ -6,7 +6,7 @@ jest.mock('cross-undici-fetch', () => ({
 
 import { fetch } from 'cross-undici-fetch';
 import { makeExecutableSchema } from '@graphql-tools/schema';
-import { PubSub } from 'graphql-subscriptions';
+import { PubSub, withFilter } from 'graphql-subscriptions';
 import supertest from 'supertest';
 import express from 'express';
 import bodyParser from 'body-parser';
@@ -21,19 +21,23 @@ const delay = (ms: number) => {
 const testBook1 = {
   id: 'book-id-1',
   title: 'Test Book 1',
+  author: 'Test Author 1',
 };
 const testBook2 = {
   id: 'book-id-2',
   title: 'Test Book 2',
+  author: 'Test Author 2',
 };
 const BOOK_ADDED = 'BOOK_ADDED';
 const typeDefs = /* GraphQL */ `
   type Book {
     id: ID!
     title: String!
+    author: String!
   }
   type Subscription {
-    onBook: Book!
+    onBook: Book!,
+    onBookBy(author: String!): Book!
   }
   type Query {
     books: [Book!]
@@ -75,7 +79,7 @@ test('should start subscriptions', async () => {
       'Content-Type': 'application/json',
     },
     body: JSON.stringify({
-      data: { onBook: { id: 'book-id-1', title: 'Test Book 1' } },
+      data: { onBook: { id: 'book-id-1', title: 'Test Book 1', author: 'Test Author 1' } },
     }),
   });
   pubsub.publish(BOOK_ADDED, { onBook: testBook2 });
@@ -88,7 +92,7 @@ test('should start subscriptions', async () => {
     },
     body: JSON.stringify({ 
       data: {
-        onBook: { id: 'book-id-2', title: 'Test Book 2' } 
+        onBook: { id: 'book-id-2', title: 'Test Book 2', author: 'Test Author 2' } 
       }
     }),
   });
@@ -127,3 +131,48 @@ test('should stop subscriptions', async () => {
   await delay(1000);
   expect(fetch).toBeCalledTimes(1);
 });
+
+test('should start subscriptions with parameters', async () => {
+  (fetch as jest.Mock).mockClear();
+  const pubsub = new PubSub();
+  const sofa = useSofa({
+    basePath: '/api',
+    schema: makeExecutableSchema({
+      typeDefs,
+      resolvers: {
+        Subscription: {
+          onBookBy: {
+            subscribe: withFilter(() => pubsub.asyncIterator(BOOK_ADDED), ( { onBookBy: { author: publishedBy } }, { author: targetAuthor }) => {
+              return publishedBy === targetAuthor;
+            }),
+          },
+        },
+      },
+    }),
+  });
+
+  const app = express();
+  app.use(bodyParser.json());
+  app.use('/api', sofa);
+
+  const res = await supertest(app)
+    .post('/api/webhook')
+    .send({ subscription: 'onBookBy', url: '/bookBy', variables: { author: 'Test Author 1' } })
+    .expect(200);
+  expect(res.body).toEqual({ id: expect.any(String) });
+  pubsub.publish(BOOK_ADDED, { onBookBy: testBook1 });
+  await delay(1000);
+  expect(fetch).toBeCalledTimes(1);
+  expect(fetch).toBeCalledWith('/bookBy', {
+    method: 'POST',
+    headers: {
+      'Content-Type': 'application/json',
+    },
+    body: JSON.stringify({
+      data: { onBookBy: { id: 'book-id-1', title: 'Test Book 1', author: 'Test Author 1' } },
+    }),
+  });
+  pubsub.publish(BOOK_ADDED, { onBookBy: testBook2 });
+  await delay(1000);
+  expect(fetch).toBeCalledTimes(1);
+});