Skip to content

Commit

Permalink
Demonstrate poor handling of cyclic datastructures
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lelandbatey committed Jan 24, 2025
1 parent 45e8fe0 commit d3c70b3
Showing 1 changed file with 57 additions and 0 deletions.
57 changes: 57 additions & 0 deletions query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ import (
"github.com/pmezard/go-difflib/difflib"
)

type CyclicParent struct {
Name string
Child *CyclicChild
}

type CyclicChild struct {
Name string
Parent *CyclicParent
}

// TestMarshalQuery tests the MarshalQuery function.
func TestMarshalQuery(t *testing.T) {
tt := []struct {
Expand Down Expand Up @@ -321,6 +331,53 @@ differentField
testQuery {
overrideName
}
}`,
},
{
Name: "CyclicStructuresWithFields",
Input: struct {
TestQuery struct {
Parent *CyclicParent
}
}{},
Fields: Fields{
"parent": Fields{
"name": true,
"child": Fields{
"name": true,
},
},
},
Option: OptFallbackJSONTag,
ExpectedOutput: `query {
testQuery {
parent {
name
child {
name
}
}
}
}`,
},
{
Name: "CyclicStructuresWithoutFields",
Input: struct {
TestQuery struct {
Parent *CyclicParent
}
}{},
Fields: nil,
Option: OptFallbackJSONTag,
ExpectedOutput: `query {
testQuery {
parent {
name
child {
name
}
}
}
}`,
},
}
Expand Down

0 comments on commit d3c70b3

Please sign in to comment.