Skip to content

Commit bab60b7

Browse files
committed
Demonstrate poor handling of cyclic datastructures
This PR is not meant to ever be merged. This is a demonstration of some broken behavior around handling datastructures with cyclic dependencies. The problem is if we provide a struct to `MarshalQueryWithOptions()` and that struct has cyclic fields, even if not populated, and even if we provide a Field that limits the output to a set depth, `MarshalQueryWithOptions()` will still recurse infinitely and cause a panic due to exhausting the stack. This is a big problem because goql is a GraphQL library, and GraphQL is for querying graph datastructures, and datastructures in graphs will very commonly have cyclic relationships because graphs have cycles :) The correct behavior would be to: 1. If no Fields are provided, do not recursively walk into fields of structs that've already been walked. This is safe to do because GraphQL already doesn't allow infinite expansion; if you want relationships, even recursive ones, to a particular depth then GQL will force you to specify that in your query. Thus we can assume that you'll have to specify that to `goql` via a `Fields` param. 2. If `Fields` *are* provided, we can recursively walk already walked fields, but only to the depth specified in those `Fields`. Future work will have to be done to get this behavior to work.
1 parent 45e8fe0 commit bab60b7

File tree

1 file changed

+30
-0
lines changed

1 file changed

+30
-0
lines changed

query_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,16 @@ import (
77
"github.com/pmezard/go-difflib/difflib"
88
)
99

10+
type CyclicParent struct {
11+
Name string
12+
Child *CyclicChild
13+
}
14+
15+
type CyclicChild struct {
16+
Name string
17+
Parent *CyclicParent
18+
}
19+
1020
// TestMarshalQuery tests the MarshalQuery function.
1121
func TestMarshalQuery(t *testing.T) {
1222
tt := []struct {
@@ -321,6 +331,26 @@ differentField
321331
testQuery {
322332
overrideName
323333
}
334+
}`,
335+
},
336+
{
337+
Name: "CyclicStructures",
338+
Input: struct {
339+
TestQuery struct {
340+
Parent *CyclicParent
341+
}
342+
}{},
343+
Fields: nil,
344+
Option: OptFallbackJSONTag,
345+
ExpectedOutput: `query {
346+
testQuery {
347+
parent {
348+
name
349+
child {
350+
name
351+
}
352+
}
353+
}
324354
}`,
325355
},
326356
}

0 commit comments

Comments
 (0)