Skip to content

bug: Duplicate arguments in graphql query builder in rare events

What does this MR do and why?

This is not a bug you would run into on production (most likely), only when debugging.

When I was debugging, I was using puts graphql_query.query in graphql_network.rb - this method modified state by appending to @resource_declarations - it was a bit tricky to discover why in some cases there was an extra or even third parameter for $iids in my graphql query - but this fix should work :)

Given you call Gitlab::Triage::GraphqlQueries::QueryBuilder.query more than once,
The method resource_query will modify state (@resource_declarations) for each call,
Leading to wrongly generated graphql queries.

Using the reproduction steps from !304 (merged), you will get the following GraphQL generated:

before after
before after

image

query($source: ID!, $after: String, $iids: [String!], $iids: [String!]) {
  project(fullPath: $source) {
    id
    issues(after: $after, state: opened, iids: $iids) {
      pageInfo {
        hasNextPage
        endCursor
      }
      nodes {
        id iid title updatedAt createdAt webUrl projectId userDiscussionsCount
      }
    }
  }
}
query($source: ID!, $after: String, $iids: [String!]) {
  project(fullPath: $source) {
    id
    issues(after: $after, iids: $iids) {
      pageInfo {
        hasNextPage
        endCursor
      }
      nodes {
        id iid title updatedAt createdAt webUrl projectId userDiscussionsCount
      }
    }
  }
}

Reproduction

Call Gitlab::Triage::GraphqlQueries::QueryBuilder.query more than once, the easiest place to do this is in Gitlab::Triage::GraphqlNetwork.query#L26

Testsuite

bundle exec rspec spec/gitlab/triage/graphql_queries/query_builder_spec.rb    

Without fix

....F..................................................................

Failures:

  1) Gitlab::Triage::GraphqlQueries::QueryBuilder#query with iids condition + discussions does not modify state when called multiple times
     Failure/Error: expect(resource_declarations).to eq(resource_declarations.uniq)
     
       expected: ["$source: ID!", "$after: String", "$iids: [String!]"]
            got: ["$source: ID!", "$after: String", "$iids: [String!]", "$iids: [String!]"]
     
       (compared using ==)
     # ./spec/gitlab/triage/graphql_queries/query_builder_spec.rb:106:in `block (4 levels) in <top (required)>'

Finished in 0.04506 seconds (files took 0.50376 seconds to load)
71 examples, 1 failure

Failed examples:

rspec ./spec/gitlab/triage/graphql_queries/query_builder_spec.rb:101 # Gitlab::Triage::GraphqlQueries::QueryBuilder#query with iids condition + discussions does not modify state when called multiple times

With fix

.......................................................................

Finished in 0.03628 seconds (files took 0.5922 seconds to load)
71 examples, 0 failures
Edited by Jos Ahrens

Merge request reports