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

UInt fields now always require to write a getter resolver #2550

Open
mdestagnol opened this issue Feb 14, 2023 · 10 comments
Open

UInt fields now always require to write a getter resolver #2550

mdestagnol opened this issue Feb 14, 2023 · 10 comments

Comments

@mdestagnol
Copy link

mdestagnol commented Feb 14, 2023

The behavior is a regression between 0.17.22 and 0.17.24.

On 0.17.24, everytime an object contain an UInt, that field requires to write a resolver getter (even though I have a custom marshaller) or it triggers the following error:

UInt is incompatible with uint or ID is incompatible with uint

On 0.17.22, it was able to figure it out.

The scalar are pointing to the following custom marshaller I have for UInt:

// MarshalUInt returns a writer function for this object
func MarshalUInt(u uint) graphql.Marshaler {
	return graphql.WriterFunc(func(w io.Writer) {
		_, _ = io.WriteString(w, fmt.Sprint(u))
	})
}

// UnmarshalUInt parses a graphQL object
func UnmarshalUInt(v interface{}) (uint, error) {

	var tmp int64
	var err error

	switch v := v.(type) {
	case json.Number:
		tmp, err = v.Int64()
	case int64:
		tmp = v
	default:
	}
	if err != nil {
		return 0, serror.NewBadRequestError("UInt not a valid number")
	}
	if tmp < 0 {
		return 0, serror.NewBadRequestError("UInt not a positive integer")
	}
	return uint(tmp), nil

}
@mstephano
Copy link
Contributor

Hi @mdestagnol, can you provide code example of your issue?

@mstephano
Copy link
Contributor

mstephano commented Feb 14, 2023

Can you also verify if your scenario is not covered in the scalars example nor in the integration tests?

@mdestagnol
Copy link
Author

I confirm I don't think it is

@mstephano
Copy link
Contributor

mstephano commented Feb 15, 2023

Can you provide an example of your gqlgen.yml and *.graphql files? It will help me to debug and fix your issue.

One thing I am thinking of, does your model have a property as uint, and in your .graphql file, that property is an Int? If so, you should normally not need a scalar and custom marshaller (see example below). But I will still wait for your code example to debug properly.

Example:

// Go
type MyObject struct {
  AttributeOne uint `json:"attributeOne"`
}
# GraphQL
type MyObject {
  attributeOne Int!
}

@mstephano
Copy link
Contributor

@mdestagnol Any follow up on your side?

@ryan-lang
Copy link

I believe I'm encountering the same issue, after upgrading from 0.17.1 to 0.17.24. The following was working correctly in 0.17.1.

# gqlgen.yaml

models:
  Uint:
    model:
      - github.com/99designs/gqlgen/graphql.Uint32
      - github.com/99designs/gqlgen/graphql.Uint
      - github.com/99designs/gqlgen/graphql.Uint64
# schema.graphqls

scalar Uint

type PagedList {
  totalCount: Uint!
  pageNumber: Uint!
  pageSize: Uint!
  items: [Item!]!
}

type Query{
  tableItems: PagedList!
}

In the new version, a pagedListResolver type is being generated, with required getter resolvers for TotalCount, PageNumber, and PageSize.

func (r *pagedListResolver) TotalCount(ctx context.Context, obj *model.PagedList) (uint32, error) {
	panic(fmt.Errorf("not implemented: TotalCount - totalCount"))
}
// etc...

@ryan-lang
Copy link

ryan-lang commented Feb 27, 2023

Further investigation shows the version that introduces the new behavior is 0.17.23.

@mstephano
Copy link
Contributor

mstephano commented Mar 2, 2023

@ryan-lang The change was to not need a scalar for uint and uint32, and be able to use Int directly in graphql for those type. However, the uint64 will still need a scalar with custom marshalling because it is bigger than int64 (example of marshalling here).

For your code, are you able to detect what fields are uint and uint32, and put them as Int in your graphqls file? Also remove them from gqlgen.yaml under model of scalar Uint. Let me know if it works:

ex:

# gqlgen.yaml

models:
  Uint:
    model:
      - github.com/99designs/gqlgen/graphql.Uint64
# schema.graphqls

scalar Uint

type PagedList {
  totalCount: Uint!
  pageNumber: Int!
  pageSize: Int!
  items: [Item!]!
}

type Query{
  tableItems: PagedList!
}

@ryan-lang
Copy link

ryan-lang commented Mar 2, 2023

Thank you, I think I understand. So it sounds like this change breaks the ability to have generated models with uint32 fields?

It seems with your suggested change, modelgen will always build PageNumber and PageSize as plain int fields in the generated go code, precluding any use of uint32 values, unless I explicitly define & bind a PagedList struct.

@atmngw
Copy link

atmngw commented Apr 24, 2023

I confirmed that the issue has been resolved by migrating from version 0.17.24 to 0.17.30.
Thank you.

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

4 participants