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

Arg unpacking issue #52

Closed
Teddy-Schmitz opened this issue Mar 17, 2018 · 4 comments
Closed

Arg unpacking issue #52

Teddy-Schmitz opened this issue Mar 17, 2018 · 4 comments

Comments

@Teddy-Schmitz
Copy link

Teddy-Schmitz commented Mar 17, 2018

Hi @vektah as requested here is a sample schema for the issue I saw. It is an input with a recursive type of itself.

input CatalogFilter {
    OR: [CatalogFilter!]
    AND: [CatalogFilter!]

    id: IntFilter
}

I pasted the code generated in the other issue, I think the issue is somewhere in the unmarshal func on line 84 in type.go

@vektah
Copy link
Collaborator

vektah commented Mar 17, 2018

That generates

func UnmarshalCatalogFilter(v interface{}) (CatalogFilter, error) {
	var it CatalogFilter

	for k, v := range v.(map[string]interface{}) {
		switch k {
		case "OR":
			var err error
			rawIf1 := v.([]interface{})
			it.OR = make([]CatalogFilter, len(rawIf1))
			for idx1 := range rawIf1 {

				it.OR[idx1], err = UnmarshalCatalogFilter(rawIf1[idx1])
			}
			if err != nil {
				return it, err
			}
		case "AND":
			var err error
			rawIf1 := v.([]interface{})
			it.AND = make([]CatalogFilter, len(rawIf1))
			for idx1 := range rawIf1 {

				it.AND[idx1], err = UnmarshalCatalogFilter(rawIf1[idx1])
			}
			if err != nil {
				return it, err
			}
		case "id":
			var err error

			it.ID, err = graphql.UnmarshalInt(v)
			if err != nil {
				return it, err
			}
		}
	}

	return it, nil
}

Which looks fine to me, are you up to date? #30 fixed a few issues around input generation.

@Teddy-Schmitz
Copy link
Author

Teddy-Schmitz commented Mar 17, 2018

hmm your right that one does generate ok. I gave you a bad example let me try again. This one definitely generates the wrong code. I just updated my dependencies and tried again.
Generated code.

func UnmarshalGPSFilter(v interface{}) (filters.GPS, error) {
	var it filters.GPS

	for k, v := range v.(map[string]interface{}) {
		switch k {
		case "OR":
			var err error
			var ptr1 filters.GPS
			rawIf2 := v.([]interface{})
			ptr1 = make([]*filters.GPS, len(rawIf2))
			for idx2 := range rawIf2 {
				var ptr3 filters.GPS

				ptr3, err = UnmarshalGPSFilter(rawIf2[idx2])
				ptr1[idx2] = &ptr3
			}
			it.OR = &ptr1
			if err != nil {
				return it, err
			}
		case "AND":
			var err error
			var ptr1 filters.GPS
			rawIf2 := v.([]interface{})
			ptr1 = make([]*filters.GPS, len(rawIf2))
			for idx2 := range rawIf2 {
				var ptr3 filters.GPS

				ptr3, err = UnmarshalGPSFilter(rawIf2[idx2])
				ptr1[idx2] = &ptr3
			}
			it.AND = &ptr1
			if err != nil {
				return it, err
			}
		case "id":
			var err error
			var ptr1 filters.IntFilter

			ptr1, err = UnmarshalIntFilter(v)
			it.ID = &ptr1
			if err != nil {
				return it, err
			}
		case "name":
			var err error
			var ptr1 filters.StringFilter

			ptr1, err = UnmarshalStringFilter(v)
			it.Name = &ptr1
			if err != nil {
				return it, err
			}
		}
	}

	return it, nil
}

Schema

input GPSFilter {
    OR: [GPSFilter!]
    AND: [GPSFilter!]

    id: IntFilter
    name: StringFilter
}

Looking at the implementations the difference between the two one is a pointer to a slice while the other is the slice itself

OR  *[]*GPS `json:"OR,omitempty"`
AND *[]*GPS `json:"AND,omitempty"`
OR  []*Catalog `json:"OR,omitempty"`
AND []*Catalog `json:"AND,omitempty"`

So now im not sure if this is a bug on your side or we can just change it to be the slice instead of the pointer. Or you want the code generation to cover both ways.

@vektah
Copy link
Collaborator

vektah commented Mar 18, 2018

I would suggest using generated input objects to avoid hand crafting them, but this should definitely "just work".

@Teddy-Schmitz
Copy link
Author

ya we will do that going forward, this is a "legacy" graphql using neelance so we have all the structs already created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants