Skip to content

Commit d3c70b3

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 d3c70b3

File tree

1 file changed

+57
-0
lines changed

1 file changed

+57
-0
lines changed

query_test.go

Lines changed: 57 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,53 @@ differentField
321331
testQuery {
322332
overrideName
323333
}
334+
}`,
335+
},
336+
{
337+
Name: "CyclicStructuresWithFields",
338+
Input: struct {
339+
TestQuery struct {
340+
Parent *CyclicParent
341+
}
342+
}{},
343+
Fields: Fields{
344+
"parent": Fields{
345+
"name": true,
346+
"child": Fields{
347+
"name": true,
348+
},
349+
},
350+
},
351+
Option: OptFallbackJSONTag,
352+
ExpectedOutput: `query {
353+
testQuery {
354+
parent {
355+
name
356+
child {
357+
name
358+
}
359+
}
360+
}
361+
}`,
362+
},
363+
{
364+
Name: "CyclicStructuresWithoutFields",
365+
Input: struct {
366+
TestQuery struct {
367+
Parent *CyclicParent
368+
}
369+
}{},
370+
Fields: nil,
371+
Option: OptFallbackJSONTag,
372+
ExpectedOutput: `query {
373+
testQuery {
374+
parent {
375+
name
376+
child {
377+
name
378+
}
379+
}
380+
}
324381
}`,
325382
},
326383
}

0 commit comments

Comments
 (0)