Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disable input validation when use_struct_references is enabled #344

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ When releasing a new version:
### Breaking changes:

- omitempty validation:
- forbid `omitempty: false` (including implicit behaviour) when using pointer on non-null, no-default input field
- forbid `omitempty: false` (including implicit behaviour) when using pointer on non-null input field

### New features:

Expand All @@ -34,6 +34,7 @@ When releasing a new version:
- omitempty validation:
- allow `omitempty` on non-nullable input field, if the field has a default
- allow `omitempty: false` on an input field, even when it is non-nullable
- don't do `omitempty` and `pointer` input types validation when `use_struct_reference` is used, as the generated type is often not compatible with validation logic.

## v0.7.0

Expand Down
33 changes: 21 additions & 12 deletions generate/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,9 @@ func (g *generator) convertType(
def := g.schema.Types[typ.Name()]
goTyp, err := g.convertDefinition(
namePrefix, def, typ.Position, selectionSet, options, queryOptions)
if err != nil {
return nil, err
}

if g.getStructReference(def) {
if options.Pointer == nil || *options.Pointer {
Expand All @@ -274,7 +277,8 @@ func (g *generator) convertType(
Elem: goTyp,
}
}
return goTyp, err

return goTyp, nil
}

// getStructReference decides if a field should be of pointer type and have the omitempty flag set.
Expand Down Expand Up @@ -450,17 +454,22 @@ func (g *generator) convertDefinition(
return nil, err
}

// Try to protect against generating field type that has possibility to send `null` to non-nullable graphQL
// type. This does not protect against lists/slices, as Go zero-slices are already serialized as `null`
// (which can therefore currently send invalid graphQL value - e.g. `null` for [String!]!)
// And does not protect against custom MarshalJSON.
_, isPointer := fieldGoType.(*goPointerType)
if field.Type.NonNull && isPointer && !fieldOptions.GetOmitempty() {
return nil, errorf(pos, "pointer on non-null input field can only be used together with omitempty: %s.%s", name, field.Name)
}

if fieldOptions.GetOmitempty() && field.Type.NonNull && field.DefaultValue == nil {
return nil, errorf(pos, "omitempty may only be used on optional arguments: %s.%s", name, field.Name)
if !g.Config.StructReferences {
// Only do these validation when StructReferences are not used, as that can generate types that would not
// pass these validations. See https://github.com/Khan/genqlient/issues/342

// Try to protect against generating field type that has possibility to send `null` to non-nullable graphQL
// type. This does not protect against lists/slices, as Go zero-slices are already serialized as `null`
// (which can therefore currently send invalid graphQL value - e.g. `null` for [String!]!).
// And does not protect against custom MarshalJSON.
_, isPointer := fieldGoType.(*goPointerType)
if field.Type.NonNull && isPointer && !fieldOptions.GetOmitempty() {
return nil, errorf(pos, "pointer on non-null input field can only be used together with omitempty: %s.%s", name, field.Name)
}

if fieldOptions.GetOmitempty() && field.Type.NonNull && field.DefaultValue == nil {
return nil, errorf(pos, "omitempty may only be used on optional arguments: %s.%s", name, field.Name)
}
}

goType.Fields[i] = &goStructField{
Expand Down
5 changes: 5 additions & 0 deletions generate/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ func TestGenerateWithConfig(t *testing.T) {
Enums: map[string]CasingAlgorithm{"Role": CasingRaw},
},
}},
{
"UseStructReference", "", []string{"UseStructReference.graphql"}, &Config{
StructReferences: true,
},
},
}

sourceFilename := "SimpleQuery.graphql"
Expand Down
6 changes: 6 additions & 0 deletions generate/testdata/queries/UseStructReference.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#https://github.com/Khan/genqlient/issues/342
query UseStructReference(
$input: UseStructReferencesInput!
) {
useStructReferencesInput(input: $input)
}
13 changes: 13 additions & 0 deletions generate/testdata/queries/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ type Query {
# But it is here for completeness - maybe it will be used in future or cause some other unexpected issues.
default(input: InputWithDefaults! = {field: "input omitted"}): Boolean
omitempty(input: OmitemptyInput): Boolean
useStructReferencesInput(input: UseStructReferencesInput!): Boolean
}

type Mutation {
Expand Down Expand Up @@ -226,3 +227,15 @@ input OmitemptyInput {
field: String!
nullableField: String
}

input StructInput {
field: String
}

input UseStructReferencesInput {
struct: StructInput!
nullableStruct: StructInput
list: [StructInput!]!
listOfNullable: [StructInput]!
nullableList: [StructInput!]
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"operations": [
{
"operationName": "UseStructReference",
"query": "\nquery UseStructReference ($input: UseStructReferencesInput!) {\n\tuseStructReferencesInput(input: $input)\n}\n",
"sourceLocation": "testdata/queries/UseStructReference.graphql"
}
]
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading