From 2cf38e5b7bef233be40a4a0b478a2a83d28a79a4 Mon Sep 17 00:00:00 2001
From: Lukas Eipert <leipert@gitlab.com>
Date: Thu, 15 Oct 2020 23:44:28 +0200
Subject: [PATCH 01/10] Refactor StartupJS for GraphQL

This refactors our StartupJS for GraphQL to be a Apollo Link [0] based
application. Instead of manually filling the cache, we build an
interceptor which "short-circuits" the Apollo Link pipeline in case the
query can be found in StartupJS. In case the query fails, is not cached,
has different variables, is done more than once, we skip it down the
pipeline. Also if all Startup Queries have been done, it self-disables.

We also now batch all StartupJS requests into one GraphQL call.

[0]: https://www.apollographql.com/docs/link/overview/
---
 app/assets/javascripts/lib/graphql.js         |   3 +-
 .../lib/utils/apollo_startup_js_link.js       | 105 ++++++++++++
 app/assets/javascripts/repository/index.js    |  28 +---
 app/views/layouts/_startup_js.html.haml       |   8 +-
 app/views/projects/tree/show.html.haml        |   2 +-
 .../lib/utils/apollo_startup_js_link_spec.js  | 151 ++++++++++++++++++
 6 files changed, 264 insertions(+), 33 deletions(-)
 create mode 100644 app/assets/javascripts/lib/utils/apollo_startup_js_link.js
 create mode 100644 spec/frontend/lib/utils/apollo_startup_js_link_spec.js

diff --git a/app/assets/javascripts/lib/graphql.js b/app/assets/javascripts/lib/graphql.js
index 0e07f7d8e44a40..e0d9a903e0accc 100644
--- a/app/assets/javascripts/lib/graphql.js
+++ b/app/assets/javascripts/lib/graphql.js
@@ -5,6 +5,7 @@ import { ApolloLink } from 'apollo-link';
 import { BatchHttpLink } from 'apollo-link-batch-http';
 import csrf from '~/lib/utils/csrf';
 import PerformanceBarService from '~/performance_bar/services/performance_bar_service';
+import { StartupJSLink } from '~/lib/utils/apollo_startup_js_link';
 
 export const fetchPolicies = {
   CACHE_FIRST: 'cache-first',
@@ -62,7 +63,7 @@ export default (resolvers = {}, config = {}) => {
 
   return new ApolloClient({
     typeDefs: config.typeDefs,
-    link: ApolloLink.from([performanceBarLink, uploadsLink]),
+    link: ApolloLink.from([performanceBarLink, new StartupJSLink(), uploadsLink]),
     cache: new InMemoryCache({
       ...config.cacheConfig,
       freezeResults: config.assumeImmutableResults,
diff --git a/app/assets/javascripts/lib/utils/apollo_startup_js_link.js b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
new file mode 100644
index 00000000000000..011f5e5bd2e79d
--- /dev/null
+++ b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
@@ -0,0 +1,105 @@
+import { ApolloLink, Observable } from 'apollo-link';
+import { parse } from 'graphql';
+
+export class StartupJSLink extends ApolloLink {
+  constructor() {
+    super();
+    // FetchResult
+    this.fetchResult = null;
+    this.queryResults = null;
+    this.startupCalls = new Map();
+    this.fetchCall = window.gl?.startup_graphql_calls?.fetchCall;
+    this.parseStartupCalls(window.gl?.startup_graphql_calls?.calls || []);
+  }
+
+  // Extract operationNames from the queries and ensure that we can
+  // match operationName => element from result array
+  parseStartupCalls(calls) {
+    calls.forEach((call, index) => {
+      const { query, variables } = call;
+      const operationName = parse(query)?.definitions?.find(x => x.kind === 'OperationDefinition')
+        ?.name?.value;
+
+      if (operationName) {
+        this.startupCalls.set(operationName, {
+          index,
+          variables,
+        });
+      }
+    });
+  }
+
+  async getResult(index) {
+    if (!this.queryResults) {
+      this.queryResults = this.fetchCall.then(res => {
+        // Handle HTTP errors
+        if (!res.ok) {
+          throw new Error('fetchCall failed');
+        }
+        this.fetchResult = res;
+        return res.json();
+      });
+    }
+
+    return (await this.queryResults)[index];
+  }
+
+  static noopRequest = (operation, forward) => forward(operation);
+
+  disable() {
+    this.request = StartupJSLink.noopRequest;
+    this.fetchResult = null;
+    this.startupCalls = null;
+    this.queryResults = null;
+  }
+
+  request(operation, forward) {
+    // Disable StartupJSLink in case all calls are done or none are set up
+    if ((this.startupCalls && this.startupCalls.size === 0) || !this.fetchCall) {
+      this.disable();
+      return forward(operation);
+    }
+
+    const { operationName } = operation;
+
+    // Skip startup call if the operationName doesn't match
+    if (!this.startupCalls.has(operationName)) {
+      return forward(operation);
+    }
+
+    const { variables: startupVariables, index } = this.startupCalls.get(operationName);
+    this.startupCalls.delete(operationName);
+
+    // Skip startup call if the variables values do not match
+    const variables = Object.entries(operation.variables || {});
+    for (let i = 0; i < variables.length; i += 1) {
+      const [key, value] = variables[i];
+      if (startupVariables[key] !== value) {
+        return forward(operation);
+      }
+    }
+
+    return new Observable(observer => {
+      this.getResult(index)
+        .then(result => {
+          operation.setContext({ response: this.fetchResult });
+          // we have data and can send it to back up the link chain
+          observer.next(result);
+          observer.complete();
+        })
+        .catch(() => {
+          // StartupJS somehow failed, so let's forward it down the pipe
+          this.disable();
+          forward(operation).subscribe({
+            next: result => {
+              observer.next(result);
+            },
+            error: error => {
+              observer.error(error);
+            },
+            complete: observer.complete.bind(observer),
+          });
+        });
+    });
+  }
+}
diff --git a/app/assets/javascripts/repository/index.js b/app/assets/javascripts/repository/index.js
index a62b2d96c547e2..f56b141fe5cec2 100644
--- a/app/assets/javascripts/repository/index.js
+++ b/app/assets/javascripts/repository/index.js
@@ -1,5 +1,4 @@
 import Vue from 'vue';
-import PathLastCommitQuery from 'shared_queries/repository/path_last_commit.query.graphql';
 import { escapeFileUrl } from '../lib/utils/url_utility';
 import createRouter from './router';
 import App from './components/app.vue';
@@ -19,10 +18,6 @@ export default function setupVueRepositoryList() {
   const { dataset } = el;
   const { projectPath, projectShortPath, ref, escapedRef, fullName } = dataset;
   const router = createRouter(projectPath, escapedRef);
-  const pathRegex = /-\/tree\/[^/]+\/(.+$)/;
-  const matches = window.location.href.match(pathRegex);
-
-  const currentRoutePath = matches ? matches[1] : '';
 
   apolloProvider.clients.defaultClient.cache.writeData({
     data: {
@@ -48,28 +43,7 @@ export default function setupVueRepositoryList() {
       },
     });
 
-  if (window.gl.startup_graphql_calls) {
-    const query = window.gl.startup_graphql_calls.find(
-      call => call.operationName === 'pathLastCommit',
-    );
-    query.fetchCall
-      .then(res => res.json())
-      .then(res => {
-        apolloProvider.clients.defaultClient.writeQuery({
-          query: PathLastCommitQuery,
-          data: res.data,
-          variables: {
-            projectPath,
-            ref,
-            path: currentRoutePath,
-          },
-        });
-      })
-      .catch(() => {})
-      .finally(() => initLastCommitApp());
-  } else {
-    initLastCommitApp();
-  }
+  initLastCommitApp();
 
   router.afterEach(({ params: { path } }) => {
     setTitle(path, ref, fullName);
diff --git a/app/views/layouts/_startup_js.html.haml b/app/views/layouts/_startup_js.html.haml
index f312e00c394dc7..ed9025697dadd5 100644
--- a/app/views/layouts/_startup_js.html.haml
+++ b/app/views/layouts/_startup_js.html.haml
@@ -24,13 +24,13 @@
         headers: { "Content-Type": "application/json", 'X-CSRF-Token': "#{form_authenticity_token}" },
       };
 
-      gl.startup_graphql_calls = gl.startup_graphql_calls.map(call => ({
-        operationName: call.query.match(/^query (.+)\(/)[1],
+      gl.startup_graphql_calls = {
+        calls: gl.startup_graphql_calls,
         fetchCall: fetch(url, {
           ...opts,
           credentials: 'same-origin',
-          body: JSON.stringify(call)
+          body: JSON.stringify(gl.startup_graphql_calls)
         })
-      }))
+      }
     }
 
diff --git a/app/views/projects/tree/show.html.haml b/app/views/projects/tree/show.html.haml
index 4d8c357cee1f44..173a0a61220316 100644
--- a/app/views/projects/tree/show.html.haml
+++ b/app/views/projects/tree/show.html.haml
@@ -1,5 +1,5 @@
 - current_route_path = request.fullpath.match(/-\/tree\/[^\/]+\/(.+$)/).to_a[1]
-- add_page_startup_graphql_call('repository/path_last_commit', { projectPath: @project.full_path, ref: current_ref, path: current_route_path })
+- add_page_startup_graphql_call('repository/path_last_commit', { projectPath: @project.full_path, ref: current_ref, path: current_route_path || "" })
 - breadcrumb_title _("Repository")
 - @content_class = "limit-container-width" unless fluid_layout
 
diff --git a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
new file mode 100644
index 00000000000000..fd1364fdd348d5
--- /dev/null
+++ b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
@@ -0,0 +1,151 @@
+import { ApolloLink, Observable } from 'apollo-link';
+import { StartupJSLink } from '~/lib/utils/apollo_startup_js_link';
+
+describe('StartupJSLink', () => {
+  const FORWARDED_RESPONSE = 'FORWARDED_RESPONSE';
+
+  const STARTUP_JS_RESPONSE = 'STARTUP_JS_RESPONSE';
+  const OPERATION_NAME = 'startupJSQuery';
+  const STARTUP_JS_QUERY = `query ${OPERATION_NAME}($id: Int = 3){
+    name
+    id
+  }`;
+
+  const STARTUP_JS_RESPONSE_TWO = 'STARTUP_JS_RESPONSE_TWO';
+  const OPERATION_NAME_TWO = 'startupJSQueryTwo';
+  const STARTUP_JS_QUERY_TWO = `query ${OPERATION_NAME_TWO}($id: Int = 3){
+    id
+    name
+  }`;
+
+  let startupLink;
+  let link;
+
+  function mockFetchCall(status = 200, response = [STARTUP_JS_RESPONSE]) {
+    const p = {
+      ok: status >= 200 && status < 300,
+      status,
+      headers: new Headers({ 'Content-Type': 'application/json' }),
+      statusText: `MOCK-FETCH ${status}`,
+      clone: () => p,
+      json: () => Promise.resolve(response),
+    };
+    return Promise.resolve(p);
+  }
+
+  function mockOperation({ operationName = OPERATION_NAME, variables = { id: 3 } } = {}) {
+    return { operationName, variables, setContext: () => {} };
+  }
+
+  const setupLink = () => {
+    startupLink = new StartupJSLink();
+    link = ApolloLink.from([startupLink, new ApolloLink(() => Observable.of(FORWARDED_RESPONSE))]);
+  };
+
+  it('forwards requests if no calls are set up', done => {
+    setupLink();
+    link.request(mockOperation()).subscribe(result => {
+      expect(result).toEqual(FORWARDED_RESPONSE);
+      expect(startupLink.startupCalls).toBe(null);
+      expect(startupLink.request).toEqual(StartupJSLink.noopRequest);
+      done();
+    });
+  });
+
+  it('forwards requests if the operation is not pre-loaded', done => {
+    window.gl = {
+      startup_graphql_calls: {
+        fetchCall: mockFetchCall(),
+        calls: [{ query: STARTUP_JS_QUERY, variables: { id: 3 } }],
+      },
+    };
+    setupLink();
+    link.request(mockOperation({ operationName: 'notLoaded' })).subscribe(result => {
+      expect(result).toEqual(FORWARDED_RESPONSE);
+      expect(startupLink.startupCalls.size).toBe(1);
+      done();
+    });
+  });
+
+  it('forwards requests if the variables are not matching', done => {
+    window.gl = {
+      startup_graphql_calls: {
+        fetchCall: mockFetchCall(),
+        calls: [{ query: STARTUP_JS_QUERY, variables: { id: 4 } }],
+      },
+    };
+    setupLink();
+    link.request(mockOperation()).subscribe(result => {
+      expect(result).toEqual(FORWARDED_RESPONSE);
+      expect(startupLink.startupCalls.size).toBe(0);
+      done();
+    });
+  });
+
+  it('forwards the call if the fetchCall is failing with a HTTP Response', done => {
+    window.gl = {
+      startup_graphql_calls: {
+        fetchCall: mockFetchCall(404),
+        calls: [{ query: STARTUP_JS_QUERY, variables: { id: 3 } }],
+      },
+    };
+    setupLink();
+    link.request(mockOperation()).subscribe(result => {
+      expect(result).toEqual(FORWARDED_RESPONSE);
+      expect(startupLink.startupCalls).toBe(null);
+      done();
+    });
+  });
+
+  it('forwards the call if the errors (e.g. failing JSON)', done => {
+    window.gl = {
+      startup_graphql_calls: {
+        fetchCall: Promise.reject(new Error('Parsing failed')),
+        calls: [{ query: STARTUP_JS_QUERY, variables: { id: 3 } }],
+      },
+    };
+    setupLink();
+    link.request(mockOperation()).subscribe(result => {
+      expect(result).toEqual(FORWARDED_RESPONSE);
+      expect(startupLink.startupCalls).toBe(null);
+      done();
+    });
+  });
+
+  it('resolves the request if the operation is matching', done => {
+    window.gl = {
+      startup_graphql_calls: {
+        fetchCall: mockFetchCall(),
+        calls: [{ query: STARTUP_JS_QUERY, variables: { id: 3 } }],
+      },
+    };
+    setupLink();
+    link.request(mockOperation()).subscribe(result => {
+      expect(result).toEqual(STARTUP_JS_RESPONSE);
+      expect(startupLink.startupCalls.size).toBe(0);
+      done();
+    });
+  });
+
+  it('resolves multiple requests correctly', done => {
+    window.gl = {
+      startup_graphql_calls: {
+        fetchCall: mockFetchCall(200, [STARTUP_JS_RESPONSE, STARTUP_JS_RESPONSE_TWO]),
+        calls: [
+          { query: STARTUP_JS_QUERY, variables: { id: 3 } },
+          { query: STARTUP_JS_QUERY_TWO, variables: { id: 3 } },
+        ],
+      },
+    };
+    setupLink();
+    link.request(mockOperation({ operationName: OPERATION_NAME_TWO })).subscribe(result => {
+      expect(result).toEqual(STARTUP_JS_RESPONSE_TWO);
+      expect(startupLink.startupCalls.size).toBe(1);
+      link.request(mockOperation({ operationName: OPERATION_NAME })).subscribe(result2 => {
+        expect(result2).toEqual(STARTUP_JS_RESPONSE);
+        expect(startupLink.startupCalls.size).toBe(0);
+        done();
+      });
+    });
+  });
+});
-- 
GitLab


From ac8a927b47c3f16dcc15a59049befc8ff18ce971 Mon Sep 17 00:00:00 2001
From: Lukas Eipert <leipert@gitlab.com>
Date: Thu, 15 Oct 2020 23:49:17 +0200
Subject: [PATCH 02/10] Tree view: add all queries to Startup GraphQL

---
 .../repository/components/breadcrumbs.vue      |  2 +-
 .../repository/components/tree_content.vue     |  2 +-
 .../javascripts/repository/mixins/preload.js   |  2 +-
 .../queries/repository}/files.query.graphql    | 18 +++++++++++++++++-
 .../repository}/permissions.query.graphql      |  2 ++
 app/views/projects/tree/show.html.haml         |  2 ++
 6 files changed, 24 insertions(+), 4 deletions(-)
 rename app/{assets/javascripts/repository/queries => graphql/queries/repository}/files.query.graphql (78%)
 rename app/{assets/javascripts/repository/queries => graphql/queries/repository}/permissions.query.graphql (84%)

diff --git a/app/assets/javascripts/repository/components/breadcrumbs.vue b/app/assets/javascripts/repository/components/breadcrumbs.vue
index 677cb265942199..a1f1c77df2f70f 100644
--- a/app/assets/javascripts/repository/components/breadcrumbs.vue
+++ b/app/assets/javascripts/repository/components/breadcrumbs.vue
@@ -6,12 +6,12 @@ import {
   GlDropdownItem,
   GlIcon,
 } from '@gitlab/ui';
+import permissionsQuery from 'shared_queries/repository/permissions.query.graphql';
 import { joinPaths, escapeFileUrl } from '~/lib/utils/url_utility';
 import { __ } from '../../locale';
 import getRefMixin from '../mixins/get_ref';
 import projectShortPathQuery from '../queries/project_short_path.query.graphql';
 import projectPathQuery from '../queries/project_path.query.graphql';
-import permissionsQuery from '../queries/permissions.query.graphql';
 
 const ROW_TYPES = {
   header: 'header',
diff --git a/app/assets/javascripts/repository/components/tree_content.vue b/app/assets/javascripts/repository/components/tree_content.vue
index 78b8baaa75e842..b42f88631b52a3 100644
--- a/app/assets/javascripts/repository/components/tree_content.vue
+++ b/app/assets/javascripts/repository/components/tree_content.vue
@@ -1,9 +1,9 @@
 <script>
+import filesQuery from 'shared_queries/repository/files.query.graphql';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
 import { __ } from '../../locale';
 import FileTable from './table/index.vue';
 import getRefMixin from '../mixins/get_ref';
-import filesQuery from '../queries/files.query.graphql';
 import projectPathQuery from '../queries/project_path.query.graphql';
 import FilePreview from './preview/index.vue';
 import { readmeFile } from '../utils/readme';
diff --git a/app/assets/javascripts/repository/mixins/preload.js b/app/assets/javascripts/repository/mixins/preload.js
index cb1d7f3aac9e8d..c1607866941c1a 100644
--- a/app/assets/javascripts/repository/mixins/preload.js
+++ b/app/assets/javascripts/repository/mixins/preload.js
@@ -1,4 +1,4 @@
-import filesQuery from '../queries/files.query.graphql';
+import filesQuery from 'shared_queries/repository/files.query.graphql';
 import getRefMixin from './get_ref';
 import projectPathQuery from '../queries/project_path.query.graphql';
 
diff --git a/app/assets/javascripts/repository/queries/files.query.graphql b/app/graphql/queries/repository/files.query.graphql
similarity index 78%
rename from app/assets/javascripts/repository/queries/files.query.graphql
rename to app/graphql/queries/repository/files.query.graphql
index 9e9f5303dd46f5..232d98a932c2b1 100644
--- a/app/assets/javascripts/repository/queries/files.query.graphql
+++ b/app/graphql/queries/repository/files.query.graphql
@@ -1,6 +1,13 @@
-#import "~/graphql_shared/fragments/pageInfo.fragment.graphql"
+fragment PageInfo on PageInfo {
+  __typename
+  hasNextPage
+  hasPreviousPage
+  startCursor
+  endCursor
+}
 
 fragment TreeEntry on Entry {
+  __typename
   id
   sha
   name
@@ -16,10 +23,15 @@ query getFiles(
   $nextPageCursor: String
 ) {
   project(fullPath: $projectPath) {
+    __typename
     repository {
+      __typename
       tree(path: $path, ref: $ref) {
+        __typename
         trees(first: $pageSize, after: $nextPageCursor) {
+          __typename
           edges {
+            __typename
             node {
               ...TreeEntry
               webPath
@@ -30,7 +42,9 @@ query getFiles(
           }
         }
         submodules(first: $pageSize, after: $nextPageCursor) {
+          __typename
           edges {
+            __typename
             node {
               ...TreeEntry
               webUrl
@@ -42,7 +56,9 @@ query getFiles(
           }
         }
         blobs(first: $pageSize, after: $nextPageCursor) {
+          __typename
           edges {
+            __typename
             node {
               ...TreeEntry
               mode
diff --git a/app/assets/javascripts/repository/queries/permissions.query.graphql b/app/graphql/queries/repository/permissions.query.graphql
similarity index 84%
rename from app/assets/javascripts/repository/queries/permissions.query.graphql
rename to app/graphql/queries/repository/permissions.query.graphql
index 092fa44e2d0c65..c0262a882cd725 100644
--- a/app/assets/javascripts/repository/queries/permissions.query.graphql
+++ b/app/graphql/queries/repository/permissions.query.graphql
@@ -1,6 +1,8 @@
 query getPermissions($projectPath: ID!) {
   project(fullPath: $projectPath) {
+    __typename
     userPermissions {
+      __typename
       pushCode
       forkProject
       createMergeRequestIn
diff --git a/app/views/projects/tree/show.html.haml b/app/views/projects/tree/show.html.haml
index 173a0a61220316..355277b7d41017 100644
--- a/app/views/projects/tree/show.html.haml
+++ b/app/views/projects/tree/show.html.haml
@@ -1,5 +1,7 @@
 - current_route_path = request.fullpath.match(/-\/tree\/[^\/]+\/(.+$)/).to_a[1]
 - add_page_startup_graphql_call('repository/path_last_commit', { projectPath: @project.full_path, ref: current_ref, path: current_route_path || "" })
+- add_page_startup_graphql_call('repository/permissions', { projectPath: @project.full_path })
+- add_page_startup_graphql_call('repository/files', { nextPageCursor: "", pageSize: 100, projectPath: @project.full_path, ref: current_ref, path: current_route_path || "/"})
 - breadcrumb_title _("Repository")
 - @content_class = "limit-container-width" unless fluid_layout
 
-- 
GitLab


From 19d07ac7a34c4100516c271f5ca634d90b70d9a1 Mon Sep 17 00:00:00 2001
From: Lukas Eipert <leipert@gitlab.com>
Date: Fri, 16 Oct 2020 14:17:19 +0200
Subject: [PATCH 03/10] Stop batching startupJS GraphQL calls

It seems like batching might not be as efficient, it also makes the code
easier to read.
---
 .../lib/utils/apollo_startup_js_link.js       |  46 ++----
 app/views/layouts/_startup_js.html.haml       |   8 +-
 .../lib/utils/apollo_startup_js_link_spec.js  | 140 ++++++++++++------
 3 files changed, 110 insertions(+), 84 deletions(-)

diff --git a/app/assets/javascripts/lib/utils/apollo_startup_js_link.js b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
index 011f5e5bd2e79d..e68148a52427b0 100644
--- a/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
+++ b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
@@ -4,58 +4,37 @@ import { parse } from 'graphql';
 export class StartupJSLink extends ApolloLink {
   constructor() {
     super();
-    // FetchResult
-    this.fetchResult = null;
-    this.queryResults = null;
     this.startupCalls = new Map();
-    this.fetchCall = window.gl?.startup_graphql_calls?.fetchCall;
-    this.parseStartupCalls(window.gl?.startup_graphql_calls?.calls || []);
+    this.parseStartupCalls(window.gl?.startup_graphql_calls || []);
   }
 
   // Extract operationNames from the queries and ensure that we can
   // match operationName => element from result array
   parseStartupCalls(calls) {
-    calls.forEach((call, index) => {
-      const { query, variables } = call;
+    calls.forEach(call => {
+      const { query, variables, fetchCall } = call;
       const operationName = parse(query)?.definitions?.find(x => x.kind === 'OperationDefinition')
         ?.name?.value;
 
       if (operationName) {
         this.startupCalls.set(operationName, {
-          index,
           variables,
+          fetchCall,
         });
       }
     });
   }
 
-  async getResult(index) {
-    if (!this.queryResults) {
-      this.queryResults = this.fetchCall.then(res => {
-        // Handle HTTP errors
-        if (!res.ok) {
-          throw new Error('fetchCall failed');
-        }
-        this.fetchResult = res;
-        return res.json();
-      });
-    }
-
-    return (await this.queryResults)[index];
-  }
-
   static noopRequest = (operation, forward) => forward(operation);
 
   disable() {
     this.request = StartupJSLink.noopRequest;
-    this.fetchResult = null;
     this.startupCalls = null;
-    this.queryResults = null;
   }
 
   request(operation, forward) {
     // Disable StartupJSLink in case all calls are done or none are set up
-    if ((this.startupCalls && this.startupCalls.size === 0) || !this.fetchCall) {
+    if (this.startupCalls && this.startupCalls.size === 0) {
       this.disable();
       return forward(operation);
     }
@@ -67,7 +46,7 @@ export class StartupJSLink extends ApolloLink {
       return forward(operation);
     }
 
-    const { variables: startupVariables, index } = this.startupCalls.get(operationName);
+    const { variables: startupVariables, fetchCall } = this.startupCalls.get(operationName);
     this.startupCalls.delete(operationName);
 
     // Skip startup call if the variables values do not match
@@ -80,16 +59,21 @@ export class StartupJSLink extends ApolloLink {
     }
 
     return new Observable(observer => {
-      this.getResult(index)
+      fetchCall
+        .then(response => {
+          // Handle HTTP errors
+          if (!response.ok) {
+            throw new Error('fetchCall failed');
+          }
+          operation.setContext({ response });
+          return response.json();
+        })
         .then(result => {
-          operation.setContext({ response: this.fetchResult });
           // we have data and can send it to back up the link chain
           observer.next(result);
           observer.complete();
         })
         .catch(() => {
-          // StartupJS somehow failed, so let's forward it down the pipe
-          this.disable();
           forward(operation).subscribe({
             next: result => {
               observer.next(result);
diff --git a/app/views/layouts/_startup_js.html.haml b/app/views/layouts/_startup_js.html.haml
index ed9025697dadd5..9c488e4f40d0a1 100644
--- a/app/views/layouts/_startup_js.html.haml
+++ b/app/views/layouts/_startup_js.html.haml
@@ -24,13 +24,13 @@
         headers: { "Content-Type": "application/json", 'X-CSRF-Token': "#{form_authenticity_token}" },
       };
 
-      gl.startup_graphql_calls = {
-        calls: gl.startup_graphql_calls,
+      gl.startup_graphql_calls = gl.startup_graphql_calls.map(call => ({
+        ...call,
         fetchCall: fetch(url, {
           ...opts,
           credentials: 'same-origin',
-          body: JSON.stringify(gl.startup_graphql_calls)
+          body: JSON.stringify(call)
         })
-      }
+      }))
     }
 
diff --git a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
index fd1364fdd348d5..371e3757a45af8 100644
--- a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
+++ b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
@@ -21,7 +21,7 @@ describe('StartupJSLink', () => {
   let startupLink;
   let link;
 
-  function mockFetchCall(status = 200, response = [STARTUP_JS_RESPONSE]) {
+  function mockFetchCall(status = 200, response = STARTUP_JS_RESPONSE) {
     const p = {
       ok: status >= 200 && status < 300,
       status,
@@ -54,10 +54,13 @@ describe('StartupJSLink', () => {
 
   it('forwards requests if the operation is not pre-loaded', done => {
     window.gl = {
-      startup_graphql_calls: {
-        fetchCall: mockFetchCall(),
-        calls: [{ query: STARTUP_JS_QUERY, variables: { id: 3 } }],
-      },
+      startup_graphql_calls: [
+        {
+          fetchCall: mockFetchCall(),
+          query: STARTUP_JS_QUERY,
+          variables: { id: 3 },
+        },
+      ],
     };
     setupLink();
     link.request(mockOperation({ operationName: 'notLoaded' })).subscribe(result => {
@@ -67,60 +70,94 @@ describe('StartupJSLink', () => {
     });
   });
 
-  it('forwards requests if the variables are not matching', done => {
-    window.gl = {
-      startup_graphql_calls: {
-        fetchCall: mockFetchCall(),
-        calls: [{ query: STARTUP_JS_QUERY, variables: { id: 4 } }],
-      },
-    };
-    setupLink();
-    link.request(mockOperation()).subscribe(result => {
-      expect(result).toEqual(FORWARDED_RESPONSE);
-      expect(startupLink.startupCalls.size).toBe(0);
-      done();
+  describe('variable match errors: ', () => {
+    it('forwards requests if the variables are not matching', done => {
+      window.gl = {
+        startup_graphql_calls: [
+          {
+            fetchCall: mockFetchCall(),
+            query: STARTUP_JS_QUERY,
+            variables: { id: 'NOT_MATCHING' },
+          },
+        ],
+      };
+      setupLink();
+      link.request(mockOperation()).subscribe(result => {
+        expect(result).toEqual(FORWARDED_RESPONSE);
+        expect(startupLink.startupCalls.size).toBe(0);
+        done();
+      });
     });
   });
 
-  it('forwards the call if the fetchCall is failing with a HTTP Response', done => {
-    window.gl = {
-      startup_graphql_calls: {
-        fetchCall: mockFetchCall(404),
-        calls: [{ query: STARTUP_JS_QUERY, variables: { id: 3 } }],
-      },
-    };
-    setupLink();
-    link.request(mockOperation()).subscribe(result => {
-      expect(result).toEqual(FORWARDED_RESPONSE);
-      expect(startupLink.startupCalls).toBe(null);
-      done();
+  describe('error handling', () => {
+    it('forwards the call if the fetchCall is failing with a HTTP Error', done => {
+      window.gl = {
+        startup_graphql_calls: [
+          {
+            fetchCall: mockFetchCall(404),
+            query: STARTUP_JS_QUERY,
+            variables: { id: 3 },
+          },
+        ],
+      };
+      setupLink();
+      link.request(mockOperation()).subscribe(result => {
+        expect(result).toEqual(FORWARDED_RESPONSE);
+        expect(startupLink.startupCalls.size).toBe(0);
+        done();
+      });
+    });
+
+    it('forwards the call if it errors (e.g. failing JSON)', done => {
+      window.gl = {
+        startup_graphql_calls: [
+          {
+            fetchCall: Promise.reject(new Error('Parsing failed')),
+            query: STARTUP_JS_QUERY,
+            variables: { id: 3 },
+          },
+        ],
+      };
+      setupLink();
+      link.request(mockOperation()).subscribe(result => {
+        expect(result).toEqual(FORWARDED_RESPONSE);
+        expect(startupLink.startupCalls.size).toBe(0);
+        done();
+      });
     });
   });
 
-  it('forwards the call if the errors (e.g. failing JSON)', done => {
+  it('resolves the request if the operation is matching', done => {
     window.gl = {
-      startup_graphql_calls: {
-        fetchCall: Promise.reject(new Error('Parsing failed')),
-        calls: [{ query: STARTUP_JS_QUERY, variables: { id: 3 } }],
-      },
+      startup_graphql_calls: [
+        {
+          fetchCall: mockFetchCall(),
+          query: STARTUP_JS_QUERY,
+          variables: { id: 3 },
+        },
+      ],
     };
     setupLink();
     link.request(mockOperation()).subscribe(result => {
-      expect(result).toEqual(FORWARDED_RESPONSE);
-      expect(startupLink.startupCalls).toBe(null);
+      expect(result).toEqual(STARTUP_JS_RESPONSE);
+      expect(startupLink.startupCalls.size).toBe(0);
       done();
     });
   });
 
-  it('resolves the request if the operation is matching', done => {
+  it('resolves the request if the variables have a different order', done => {
     window.gl = {
-      startup_graphql_calls: {
-        fetchCall: mockFetchCall(),
-        calls: [{ query: STARTUP_JS_QUERY, variables: { id: 3 } }],
-      },
+      startup_graphql_calls: [
+        {
+          fetchCall: mockFetchCall(),
+          query: STARTUP_JS_QUERY,
+          variables: { id: 3, name: 'foo' },
+        },
+      ],
     };
     setupLink();
-    link.request(mockOperation()).subscribe(result => {
+    link.request(mockOperation({ variables: { name: 'foo', id: 3 } })).subscribe(result => {
       expect(result).toEqual(STARTUP_JS_RESPONSE);
       expect(startupLink.startupCalls.size).toBe(0);
       done();
@@ -129,13 +166,18 @@ describe('StartupJSLink', () => {
 
   it('resolves multiple requests correctly', done => {
     window.gl = {
-      startup_graphql_calls: {
-        fetchCall: mockFetchCall(200, [STARTUP_JS_RESPONSE, STARTUP_JS_RESPONSE_TWO]),
-        calls: [
-          { query: STARTUP_JS_QUERY, variables: { id: 3 } },
-          { query: STARTUP_JS_QUERY_TWO, variables: { id: 3 } },
-        ],
-      },
+      startup_graphql_calls: [
+        {
+          fetchCall: mockFetchCall(),
+          query: STARTUP_JS_QUERY,
+          variables: { id: 3 },
+        },
+        {
+          fetchCall: mockFetchCall(200, STARTUP_JS_RESPONSE_TWO),
+          query: STARTUP_JS_QUERY_TWO,
+          variables: { id: 3 },
+        },
+      ],
     };
     setupLink();
     link.request(mockOperation({ operationName: OPERATION_NAME_TWO })).subscribe(result => {
-- 
GitLab


From 6518a5349014d461e9df3358f0a78b128e51456b Mon Sep 17 00:00:00 2001
From: Lukas Eipert <leipert@gitlab.com>
Date: Fri, 16 Oct 2020 14:19:06 +0200
Subject: [PATCH 04/10] More robust variables comparison

We now check that there must be the same exact amount of variables.
---
 .../lib/utils/apollo_startup_js_link.js       | 29 +++++++---
 .../lib/utils/apollo_startup_js_link_spec.js  | 53 +++++++++++++++++++
 2 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/app/assets/javascripts/lib/utils/apollo_startup_js_link.js b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
index e68148a52427b0..4b00b37fbd8e77 100644
--- a/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
+++ b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
@@ -1,6 +1,27 @@
 import { ApolloLink, Observable } from 'apollo-link';
 import { parse } from 'graphql';
 
+/**
+ * Compares two set of variables, order independent
+ */
+const doVariablesMatch = (var1 = {}, var2 = {}) => {
+  const entries1 = Object.entries(var1);
+  const entries2 = Object.entries(var2);
+  if (entries1.length !== entries2.length) {
+    return false;
+  }
+
+  for (let i = 0; i < entries1.length; i += 1) {
+    const [key, value] = entries1[i];
+
+    if (var2[key] !== value) {
+      return false;
+    }
+  }
+
+  return true;
+};
+
 export class StartupJSLink extends ApolloLink {
   constructor() {
     super();
@@ -50,12 +71,8 @@ export class StartupJSLink extends ApolloLink {
     this.startupCalls.delete(operationName);
 
     // Skip startup call if the variables values do not match
-    const variables = Object.entries(operation.variables || {});
-    for (let i = 0; i < variables.length; i += 1) {
-      const [key, value] = variables[i];
-      if (startupVariables[key] !== value) {
-        return forward(operation);
-      }
+    if (!doVariablesMatch(startupVariables, operation.variables)) {
+      return forward(operation);
     }
 
     return new Observable(observer => {
diff --git a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
index 371e3757a45af8..a17065debdc4e8 100644
--- a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
+++ b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
@@ -88,6 +88,59 @@ describe('StartupJSLink', () => {
         done();
       });
     });
+
+    it('forwards requests if more variables are set in the operation', done => {
+      window.gl = {
+        startup_graphql_calls: [
+          {
+            fetchCall: mockFetchCall(),
+            query: STARTUP_JS_QUERY,
+          },
+        ],
+      };
+      setupLink();
+      link.request(mockOperation()).subscribe(result => {
+        expect(result).toEqual(FORWARDED_RESPONSE);
+        expect(startupLink.startupCalls.size).toBe(0);
+        done();
+      });
+    });
+
+    it('forwards requests if less variables are set in the operation', done => {
+      window.gl = {
+        startup_graphql_calls: [
+          {
+            fetchCall: mockFetchCall(),
+            query: STARTUP_JS_QUERY,
+            variables: { id: 3, name: 'tanuki' },
+          },
+        ],
+      };
+      setupLink();
+      link.request(mockOperation({ variables: { id: 3 } })).subscribe(result => {
+        expect(result).toEqual(FORWARDED_RESPONSE);
+        expect(startupLink.startupCalls.size).toBe(0);
+        done();
+      });
+    });
+
+    it('forwards requests if different variables are set', done => {
+      window.gl = {
+        startup_graphql_calls: [
+          {
+            fetchCall: mockFetchCall(),
+            query: STARTUP_JS_QUERY,
+            variables: { name: 'tanuki' },
+          },
+        ],
+      };
+      setupLink();
+      link.request(mockOperation({ variables: { id: 3 } })).subscribe(result => {
+        expect(result).toEqual(FORWARDED_RESPONSE);
+        expect(startupLink.startupCalls.size).toBe(0);
+        done();
+      });
+    });
   });
 
   describe('error handling', () => {
-- 
GitLab


From dd5126e4de171b6034060ab05bab919b1956a9b9 Mon Sep 17 00:00:00 2001
From: Lukas Eipert <leipert@gitlab.com>
Date: Fri, 16 Oct 2020 14:20:02 +0200
Subject: [PATCH 05/10] Check StartupJS GraphQL response

We are checking that the response is of the format `{data}` and also
doesn't contain any `{errors}`. If that is not true, we forward the
request down the stack.
---
 .../lib/utils/apollo_startup_js_link.js       |  4 ++
 .../lib/utils/apollo_startup_js_link_spec.js  | 58 ++++++++++++++++++-
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/app/assets/javascripts/lib/utils/apollo_startup_js_link.js b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
index 4b00b37fbd8e77..c1985e5d082825 100644
--- a/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
+++ b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
@@ -86,6 +86,10 @@ export class StartupJSLink extends ApolloLink {
           return response.json();
         })
         .then(result => {
+          if (result && (result.errors || !result.data)) {
+            throw new Error('Received GraphQL error');
+          }
+
           // we have data and can send it to back up the link chain
           observer.next(result);
           observer.complete();
diff --git a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
index a17065debdc4e8..3363edbcc1c9d7 100644
--- a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
+++ b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
@@ -2,22 +2,38 @@ import { ApolloLink, Observable } from 'apollo-link';
 import { StartupJSLink } from '~/lib/utils/apollo_startup_js_link';
 
 describe('StartupJSLink', () => {
-  const FORWARDED_RESPONSE = 'FORWARDED_RESPONSE';
+  const FORWARDED_RESPONSE = { data: 'FORWARDED_RESPONSE' };
 
-  const STARTUP_JS_RESPONSE = 'STARTUP_JS_RESPONSE';
+  const STARTUP_JS_RESPONSE = { data: 'STARTUP_JS_RESPONSE' };
   const OPERATION_NAME = 'startupJSQuery';
   const STARTUP_JS_QUERY = `query ${OPERATION_NAME}($id: Int = 3){
     name
     id
   }`;
 
-  const STARTUP_JS_RESPONSE_TWO = 'STARTUP_JS_RESPONSE_TWO';
+  const STARTUP_JS_RESPONSE_TWO = { data: 'STARTUP_JS_RESPONSE_TWO' };
   const OPERATION_NAME_TWO = 'startupJSQueryTwo';
   const STARTUP_JS_QUERY_TWO = `query ${OPERATION_NAME_TWO}($id: Int = 3){
     id
     name
   }`;
 
+  const ERROR_RESPONSE = {
+    data: {
+      user: null,
+    },
+    errors: [
+      {
+        path: ['user'],
+        locations: [{ line: 2, column: 3 }],
+        extensions: {
+          message: 'Object not found',
+          type: 2,
+        },
+      },
+    ],
+  };
+
   let startupLink;
   let link;
 
@@ -179,6 +195,42 @@ describe('StartupJSLink', () => {
         done();
       });
     });
+
+    it('forwards the call if the response contains an error', done => {
+      window.gl = {
+        startup_graphql_calls: [
+          {
+            fetchCall: mockFetchCall(200, ERROR_RESPONSE),
+            query: STARTUP_JS_QUERY,
+            variables: { id: 3 },
+          },
+        ],
+      };
+      setupLink();
+      link.request(mockOperation()).subscribe(result => {
+        expect(result).toEqual(FORWARDED_RESPONSE);
+        expect(startupLink.startupCalls.size).toBe(0);
+        done();
+      });
+    });
+
+    it("forwards the call if the response doesn't contain a data object", done => {
+      window.gl = {
+        startup_graphql_calls: [
+          {
+            fetchCall: mockFetchCall(200, { 'no-data': 'yay' }),
+            query: STARTUP_JS_QUERY,
+            variables: { id: 3 },
+          },
+        ],
+      };
+      setupLink();
+      link.request(mockOperation()).subscribe(result => {
+        expect(result).toEqual(FORWARDED_RESPONSE);
+        expect(startupLink.startupCalls.size).toBe(0);
+        done();
+      });
+    });
   });
 
   it('resolves the request if the operation is matching', done => {
-- 
GitLab


From 9cf6662c6d21fa737fc7da7669e504487b51c3aa Mon Sep 17 00:00:00 2001
From: Lukas Eipert <leipert@gitlab.com>
Date: Fri, 16 Oct 2020 14:22:13 +0200
Subject: [PATCH 06/10] Ensure that GraphQL StartupJS is resolved once

---
 .../lib/utils/apollo_startup_js_link_spec.js  | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
index 3363edbcc1c9d7..d996ee19440fc7 100644
--- a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
+++ b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
@@ -251,6 +251,28 @@ describe('StartupJSLink', () => {
     });
   });
 
+  it('resolves the request exactly once', done => {
+    window.gl = {
+      startup_graphql_calls: [
+        {
+          fetchCall: mockFetchCall(),
+          query: STARTUP_JS_QUERY,
+          variables: { id: 3 },
+        },
+      ],
+    };
+    setupLink();
+    link.request(mockOperation()).subscribe(result => {
+      expect(result).toEqual(STARTUP_JS_RESPONSE);
+      expect(startupLink.startupCalls.size).toBe(0);
+      done();
+      link.request(mockOperation()).subscribe(result2 => {
+        expect(result2).toEqual(FORWARDED_RESPONSE);
+        done();
+      });
+    });
+  });
+
   it('resolves the request if the variables have a different order', done => {
     window.gl = {
       startup_graphql_calls: [
-- 
GitLab


From f235ff11dcb5ff06fbc3be8f7fcfdf1a6c5c51ab Mon Sep 17 00:00:00 2001
From: Lukas 'Eipi' Eipert <leipert@gitlab.com>
Date: Fri, 16 Oct 2020 12:30:30 +0000
Subject: [PATCH 07/10] Apply 1 suggestion(s) to 1 file(s)

---
 spec/frontend/lib/utils/apollo_startup_js_link_spec.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
index d996ee19440fc7..31c2edee078a97 100644
--- a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
+++ b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
@@ -265,7 +265,6 @@ describe('StartupJSLink', () => {
     link.request(mockOperation()).subscribe(result => {
       expect(result).toEqual(STARTUP_JS_RESPONSE);
       expect(startupLink.startupCalls.size).toBe(0);
-      done();
       link.request(mockOperation()).subscribe(result2 => {
         expect(result2).toEqual(FORWARDED_RESPONSE);
         done();
-- 
GitLab


From 162ef16994dde404d58962ad2160bad8ac87d6ed Mon Sep 17 00:00:00 2001
From: Lukas Eipert <leipert@gitlab.com>
Date: Sat, 24 Oct 2020 16:28:06 +0200
Subject: [PATCH 08/10] Allow variables to be arrays

Sometimes our variables for GraphQL requests can be arrays (e.g. ids),
in that case a simple comparison doesn't cut it. If we rely on lodash's
isEqual, it seems like this solves this case (and others)
---
 .../lib/utils/apollo_startup_js_link.js       |  3 +-
 .../lib/utils/apollo_startup_js_link_spec.js  | 36 +++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/lib/utils/apollo_startup_js_link.js b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
index c1985e5d082825..fe6552b8a6b475 100644
--- a/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
+++ b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
@@ -1,5 +1,6 @@
 import { ApolloLink, Observable } from 'apollo-link';
 import { parse } from 'graphql';
+import { isEqual } from 'lodash';
 
 /**
  * Compares two set of variables, order independent
@@ -71,7 +72,7 @@ export class StartupJSLink extends ApolloLink {
     this.startupCalls.delete(operationName);
 
     // Skip startup call if the variables values do not match
-    if (!doVariablesMatch(startupVariables, operation.variables)) {
+    if (!isEqual(startupVariables, operation.variables)) {
       return forward(operation);
     }
 
diff --git a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
index 31c2edee078a97..0a59cd085ba903 100644
--- a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
+++ b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
@@ -157,6 +157,24 @@ describe('StartupJSLink', () => {
         done();
       });
     });
+
+    it('forwards requests if array variables have a different order', done => {
+      window.gl = {
+        startup_graphql_calls: [
+          {
+            fetchCall: mockFetchCall(),
+            query: STARTUP_JS_QUERY,
+            variables: { id: [3, 4] },
+          },
+        ],
+      };
+      setupLink();
+      link.request(mockOperation({ variables: { id: [4, 3] } })).subscribe(result => {
+        expect(result).toEqual(FORWARDED_RESPONSE);
+        expect(startupLink.startupCalls.size).toBe(0);
+        done();
+      });
+    });
   });
 
   describe('error handling', () => {
@@ -290,6 +308,24 @@ describe('StartupJSLink', () => {
     });
   });
 
+  it('resolves the request if the variables are of an array format', done => {
+    window.gl = {
+      startup_graphql_calls: [
+        {
+          fetchCall: mockFetchCall(),
+          query: STARTUP_JS_QUERY,
+          variables: { id: [3, 4] },
+        },
+      ],
+    };
+    setupLink();
+    link.request(mockOperation({ variables: { id: [3, 4] } })).subscribe(result => {
+      expect(result).toEqual(STARTUP_JS_RESPONSE);
+      expect(startupLink.startupCalls.size).toBe(0);
+      done();
+    });
+  });
+
   it('resolves multiple requests correctly', done => {
     window.gl = {
       startup_graphql_calls: [
-- 
GitLab


From 6d374664ce4b1c42e41f614234b459d38f83c425 Mon Sep 17 00:00:00 2001
From: Lukas Eipert <leipert@gitlab.com>
Date: Sat, 24 Oct 2020 17:22:17 +0200
Subject: [PATCH 09/10] Remove undefined variables

In one case our application code passed down `{ filePath: undefined }`
while the startup call had just an empty object `{}` as variables. As
`undefined` has no meaning in GraphQL we can filter it out before we
compare the objects.
---
 .../lib/utils/apollo_startup_js_link.js       | 31 ++++++++-----------
 .../lib/utils/apollo_startup_js_link_spec.js  | 18 +++++++++++
 2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/app/assets/javascripts/lib/utils/apollo_startup_js_link.js b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
index fe6552b8a6b475..5c120dd532f5a1 100644
--- a/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
+++ b/app/assets/javascripts/lib/utils/apollo_startup_js_link.js
@@ -1,26 +1,21 @@
 import { ApolloLink, Observable } from 'apollo-link';
 import { parse } from 'graphql';
-import { isEqual } from 'lodash';
+import { isEqual, pickBy } from 'lodash';
 
 /**
- * Compares two set of variables, order independent
+ * Remove undefined values from object
+ * @param obj
+ * @returns {Dictionary<unknown>}
  */
-const doVariablesMatch = (var1 = {}, var2 = {}) => {
-  const entries1 = Object.entries(var1);
-  const entries2 = Object.entries(var2);
-  if (entries1.length !== entries2.length) {
-    return false;
-  }
-
-  for (let i = 0; i < entries1.length; i += 1) {
-    const [key, value] = entries1[i];
+const pickDefinedValues = obj => pickBy(obj, x => x !== undefined);
 
-    if (var2[key] !== value) {
-      return false;
-    }
-  }
-
-  return true;
+/**
+ * Compares two set of variables, order independent
+ *
+ * Ignores undefined values (in the top level) and supports arrays etc.
+ */
+const variablesMatch = (var1 = {}, var2 = {}) => {
+  return isEqual(pickDefinedValues(var1), pickDefinedValues(var2));
 };
 
 export class StartupJSLink extends ApolloLink {
@@ -72,7 +67,7 @@ export class StartupJSLink extends ApolloLink {
     this.startupCalls.delete(operationName);
 
     // Skip startup call if the variables values do not match
-    if (!isEqual(startupVariables, operation.variables)) {
+    if (!variablesMatch(startupVariables, operation.variables)) {
       return forward(operation);
     }
 
diff --git a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
index 0a59cd085ba903..a87b8f3c9daa7b 100644
--- a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
+++ b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
@@ -308,6 +308,24 @@ describe('StartupJSLink', () => {
     });
   });
 
+  it('resolves the request if the variables have undefined values', done => {
+    window.gl = {
+      startup_graphql_calls: [
+        {
+          fetchCall: mockFetchCall(),
+          query: STARTUP_JS_QUERY,
+          variables: { name: 'foo' },
+        },
+      ],
+    };
+    setupLink();
+    link.request(mockOperation({ variables: { name: 'foo', undef: undefined } })).subscribe(result => {
+      expect(result).toEqual(STARTUP_JS_RESPONSE);
+      expect(startupLink.startupCalls.size).toBe(0);
+      done();
+    });
+  });
+
   it('resolves the request if the variables are of an array format', done => {
     window.gl = {
       startup_graphql_calls: [
-- 
GitLab


From 5465159679b768ad7c305d5856b032d1db0307e7 Mon Sep 17 00:00:00 2001
From: NataliaTepluhina <tarya.se@gmail.com>
Date: Mon, 26 Oct 2020 12:26:54 +0200
Subject: [PATCH 10/10] Fixed prettier issues

---
 .../lib/utils/apollo_startup_js_link_spec.js         | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
index a87b8f3c9daa7b..faead3ff8fe427 100644
--- a/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
+++ b/spec/frontend/lib/utils/apollo_startup_js_link_spec.js
@@ -319,11 +319,13 @@ describe('StartupJSLink', () => {
       ],
     };
     setupLink();
-    link.request(mockOperation({ variables: { name: 'foo', undef: undefined } })).subscribe(result => {
-      expect(result).toEqual(STARTUP_JS_RESPONSE);
-      expect(startupLink.startupCalls.size).toBe(0);
-      done();
-    });
+    link
+      .request(mockOperation({ variables: { name: 'foo', undef: undefined } }))
+      .subscribe(result => {
+        expect(result).toEqual(STARTUP_JS_RESPONSE);
+        expect(startupLink.startupCalls.size).toBe(0);
+        done();
+      });
   });
 
   it('resolves the request if the variables are of an array format', done => {
-- 
GitLab