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

uint32 causes type mismatch #79

Closed
rhymes opened this issue Apr 5, 2018 · 7 comments
Closed

uint32 causes type mismatch #79

rhymes opened this issue Apr 5, 2018 · 7 comments
Labels
bug Something isn't working

Comments

@rhymes
Copy link

rhymes commented Apr 5, 2018

Expected Behaviour

I would expect that having a model defined as:

type Model struct {
 Interval uint32
}

and a schema

type Model {
    interval: Int!
}

and running:

gqlgen -typemap types.json -schema schema.graphql                                                                                                                                                         
type mismatch on Model.interval, expected int got uint32

would not result in that error.

Actual Behavior

It seems that the generator doesn't support unsigned integers but maybe I'm missing something.

Minimal graphql.schema and models to reproduce

See above

@vektah vektah added the bug Something isn't working label Apr 5, 2018
@vektah
Copy link
Collaborator

vektah commented Apr 5, 2018

Not currently supported, but I'll take a look when I get a chance. In the meantime, you may need use the generated model and explicitly cast between them

@rhymes
Copy link
Author

rhymes commented Apr 5, 2018

@vektah no problem, I'll see how I can do it. do you have any pointers in the code base on how to marshal and unmarshal integers?

@vektah
Copy link
Collaborator

vektah commented Apr 5, 2018

There are some general notes on how the marshaling works in the docs

It might be enough to set the field.CastType in here somewhere for all the various int types, but I'm not 100%.

Otherwise you would need to define marshalers over here for all the different types

And then in the binding code check for the various int types on the field and override the field.Marshaler to use the correct type.

I guess the bigger question is how should overflows be handled etc, on the wire anything above an int32 probably isn't going to be read by many clients.

@rhymes
Copy link
Author

rhymes commented Apr 5, 2018

Yeah, according to the spec Int represents a signed 32bit integers which is kind of a bummer because our app is using both unsigned 32 bit integers and unsigned 64 bit integers.

The spec though is conservative to support languages that don't have unsigned integers or 64 bit integers (like Javascript).

I would probably add the possibility to use them being very clear to the developer that if they don't control the client they might incur in overflow issues because the target language doesn't handle integers that big.

@vektah
Copy link
Collaborator

vektah commented Apr 5, 2018

Thinking about this some more, you probably should just define a custom scalar marshshaler for Uint that maps to uint32 and use that in place of Int in the schema. This way you control the representation. Maybe its a raw number, maybe its a string but its probably up to each person using the lib to decide how they want it.

eg:
schema

scalar Uint

type Model {
    interval: Uint!
}
package mytypes

import (
	"fmt"
	"io"
	"strconv"
)

func MarshalUInt(i uint64) Marshaler {
	return WriterFunc(func(w io.Writer) {
		io.WriteString(w, fmt.Sprintf("%d",i))
	})
}

func UnmarshalUint(v interface{}) (uint64, error) {
	switch v := v.(type) {
	case int:
		return v.(uint64), nil
	default:
		return 0, fmt.Errorf("%T is not an int", v)
	}
}

I think that cast from int to uint64 is safe on 64 bit systems (it will wrap, but the negative will unwrap?...), but its worth testing some large uints make it through serialisation correctly.

types.json

{
    "Uint": "mytypes.Uint"
}

@rhymes
Copy link
Author

rhymes commented Apr 5, 2018

It's working! Thank you!

This is how I've done it for now:

scalar UInt32

type Model {
    interval: UInt32!
}
import (
	"fmt"
	"io"

	graphql "github.com/vektah/gqlgen/graphql"
)

func MarshalUInt32(i uint32) graphql.Marshaler {
	return graphql.WriterFunc(func(w io.Writer) {
		io.WriteString(w, fmt.Sprintf("%d", i))
	})
}

func UnmarshalUInt32(v interface{}) (uint32, error) {
	switch v := v.(type) {
	case int:
		return uint32(v), nil
	default:
		return 0, fmt.Errorf("%T is not an uint32", v)
	}
}

It was super easy to hookup resolvers to the domain logic and the query tool builtin is very handy! Thank you!

@vektah vektah closed this as completed Apr 18, 2018
@yuya-kanai
Copy link

I had issue with json.numbers so I added more cases.

import (
	"encoding/json"
	"fmt"
	"io"
	"strconv"
	"github.com/99designs/gqlgen/graphql"
)

func MarshalUint(i uint) graphql.Marshaler {
	return graphql.WriterFunc(func(w io.Writer) {
		io.WriteString(w, fmt.Sprintf("%d", i))
	})
}

func UnmarshalUint(v interface{}) (uint, error) {
	switch v := v.(type) {
	case string:
		i, err := strconv.Atoi(v)
		if err != nil {
			return 0, fmt.Errorf("string failed to be parsed: %v", err)
		} else {
			return uint(i), nil
		}

	case int:
		return uint(v), nil
	case int64:
		return uint(v), nil
	case json.Number:
		i, err := strconv.Atoi(string(v))
		if err != nil {
			return 0, fmt.Errorf("json.Number failed to be parsed: %v", err)
		} else {
			return uint(i), nil
		}
	default:
		return 0, fmt.Errorf("%T is not an int", v)
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants