getoutreach / goql

A GraphQL client package written in Go.
Apache License 2.0
19 stars 2 forks source link

fix: tokenize fields based on node level #97

Closed grevych closed 2 years ago

grevych commented 2 years ago

What this PR does / why we need it

Re-bootstrapping with a recent version flags a race condition when running tests:

 :: Running go test (or_test)

  60ms . ·······················↷·································✖✖✖✖✖✖
       graphql_test

 63 tests, 1 skipped, 6 failures in 8.945s

=== Skipped
=== SKIP: . TestDoCustom (0.00s)
    do_test.go:51:

=== Failed
=== FAIL: . TestCustomOperationWithHeaders (0.01s)
    testing.go:1152: race detected during execution of test

=== FAIL: . TestQuery (0.01s)
    testing.go:1152: race detected during execution of test

=== FAIL: . TestCustomOperation (0.01s)
    testing.go:1152: race detected during execution of test

=== FAIL: . TestQueryWithHeaders (0.01s)
    testing.go:1152: race detected during execution of test

=== FAIL: . TestMutate (0.01s)
    testing.go:1152: race detected during execution of test

=== FAIL: . TestMutateWithHeaders (0.01s)
    testing.go:1152: race detected during execution of test

DONE 63 tests, 1 skipped, 6 failures in 8.963s
make: *** [test] Error 1

The issue occurs in the marshal method when multiple requests try to update the declaration name of the same cached operation https://github.com/getoutreach/goql/blob/6f72c1efa60eb971ac68dd8dbd602d133083d6e0/query.go#L636

The proposed fix splits field tokenisation into different methods based on the level of the tree where the node is at. If the node is the root element, its declaration is not tokenised. Instead, the wrapper + args from tokens are passed to the writer. For the rest of the nodes, the tokenisation happens as usual.

Jira ID

N/A

Notes for your reviewers

grevych commented 2 years ago

We cannot remove it. It's used for the rest of the fields to tokenise them https://github.com/getoutreach/goql/pull/97/commits/34beaa5e8a0fbde6b807b812b50c780cefa33630#diff-66f9c6b00ef4ad1dbef1133865440121c8d03aec3333e8b6dd97ac3560b91ac7R260

getoutreach-ci-1[bot] commented 2 years ago

:tada: This PR is included in version 1.9.4 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: