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

Generated Federation code for fields with @external directive applied on a list which is used in a @requires field is wrong. #2559

Closed
vamshiaruru-virgodesigns opened this issue Feb 20, 2023 · 4 comments
Labels
federation Related to Apollo federation help wanted Extra attention is needed

Comments

@vamshiaruru-virgodesigns
Copy link

vamshiaruru-virgodesigns commented Feb 20, 2023

What happened?

Hi everyone, thanks for the library!

I am trying to port one of my services from python to go. This service is a subgraph in a larger supergraph, and makes use of @external and @requires directives. As a simplified example, I have a type like this:

type MetadataItem {
  key: String! @external
  value: String! @external
}

type Checkout @key(fields: "id") {
  id: ID!
  metadata: [MetadataItem]! @external
  token: UUID! @external
  transformed: Transformation
    @requires(
      fields: "metadata { key value }  token"
    )
}

This seems to require implementation of a resolver called findCheckoutByID (I imagine this is the resolve_reference equivalent in gqlgen?).

In federation.go I see this output:

			case "findCheckoutByID":
				id0, err := ec.unmarshalNID2string(ctx, rep["id"])
				if err != nil {
					return fmt.Errorf(`unmarshalling param 0 for findCheckoutByID(): %w`, err)
				}
				entity, err := ec.resolvers.Entity().FindCheckoutByID(ctx, id0)
				if err != nil {
					return fmt.Errorf(`resolving Entity "Checkout": %w`, err)
				}

				entity.Token, err = ec.unmarshalNUUID2string(ctx, rep["token"])
				if err != nil {
					return err
				}
				entity.Metadata.Key, err = ec.unmarshalNString2string(ctx, rep["metadata"].(map[string]interface{})["key"])
				if err != nil {
					return err
				}
				entity.Metadata.Value, err = ec.unmarshalNString2string(ctx, rep["metadata"].(map[string]interface{})["value"])
				if err != nil {
					return err
				}
				// More code here

and the error

entity.Metadata.Value undefined (type []*model.MetadataItem has no field or method Value)

Which makes sense because, entity.Metadata is a list, not a struct. Here's the generated Checkout entity

type Checkout struct {
	ID                  string              `json:"id"`
	Token               string              `json:"token"`
	Metadata            []*MetadataItem     `json:"metadata"`
	// rest of the fields
}

As you can see, there's a mismatch. I am not sure how to proceed here.

What did you expect?

I expect that the metadata must be constructed as a list, not as a single struct.

Minimal Reproduction Example

type Review {
  body: String
  author: User @provides(fields: "username")
  product: Product
}

type MetadataItem {
  key: String @external
  value: String @external
}

extend type User @key(fields: "id") {
  id: ID! @external # External directive not required for key fields in federation v2
  reviews: [Review]
}

extend type Product @key(fields: "upc") {
  upc: String! @external # External directive not required for key fields in federation v2
  metadata: [MetadataItem] @external
  reviews: [Review]
  result: String @requires(fields: "metadata { key value }")
}

versions

  • go run github.com/99designs/gqlgen version? v0.17.24
  • go version? go version go1.19 darwin/amd64

In general, we are planning to move some of our services to golang and our existing services use extensive use of federation directives, specifically external, requires and inaccessible. Does gqlgen handle all these requirements well? I see from here: https://www.apollographql.com/docs/federation/building-supergraphs/supported-subgraphs/#go, that @requires is not supported. Is there any plan to support it anytime soon? (as it is a big blocker for us).

Any help is appreciated, thank you.

@vamshiaruru-virgodesigns vamshiaruru-virgodesigns changed the title Generated Federation code for fields with @external directive applied on a list field is wrong. Generated Federation code for fields with @external directive applied on a list which is used in a @requires field is wrong. Feb 20, 2023
@StevenACoffman
Copy link
Collaborator

StevenACoffman commented Feb 22, 2023

gqlgen have fully passed Apollo Federation (1 and 2) Validation tests in the past, and even has had contributions from Apollo engineers. It looks like something in their test suite changed. If there is some nuance they noticed we are missing, then PRs are very welcome!

@vamshiaruru-virgodesigns
Copy link
Author

Thanks for the reply! I understand that apollo test suite has changed, but the actual bug I mentioned in the post hasn't got anything to do with the apollo test suite as I understand it. It is a bug with implementation of requires in gqlgen itself, as it is ignoring array fields.
I'd love to contribute and maybe fix it, but unfortunately I'm new to go as well as gqlgen. I'll try to go through the code base to see maybe if I can do anything, but unfortunately I am not very confident.

@mihirpmehta
Copy link

Facing similar issue where getting "interface conversion: interface {} is nil, not map[string]interface {}" error in subgraph's federation go when that subgraph has dependency on other subgraph entity's field.

@dariuszkuc
Copy link
Contributor

fixed in #2884

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
federation Related to Apollo federation help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants