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

Unable to add type "Service" in federation #1256

Closed
josemarluedke opened this issue Jul 20, 2020 · 12 comments
Closed

Unable to add type "Service" in federation #1256

josemarluedke opened this issue Jul 20, 2020 · 12 comments

Comments

@josemarluedke
Copy link

I'm declaring a GraphQL type called "Service" in a federated gqlgen. gqlgen fails to generate because of "redeclared service type".

Reproduction: https://github.com/josemarluedke/gqlgen-fed-service-type
GraphQL: https://github.com/josemarluedke/gqlgen-fed-service-type/blob/master/graph/schema.graphqls

What happened?

➜ go generate ./...
validation failed: packages.Load: /Users/josemarluedke/code/tmp/gqlgen-fed-service-type/graph/generated/generated.go:62:2: Service redeclared
/Users/josemarluedke/code/tmp/gqlgen-fed-service-type/graph/generated/generated.go:58:2: 	other declaration of Service
/Users/josemarluedke/code/tmp/gqlgen-fed-service-type/graph/generated/generated.go:135:27: e.complexity.Service.SDL undefined (type struct{ID func(childComplexity int) int} has no field or method SDL)
/Users/josemarluedke/code/tmp/gqlgen-fed-service-type/graph/generated/generated.go:139:31: e.complexity.Service.SDL undefined (type struct{ID func(childComplexity int) int} has no field or method SDL)
exit status 1

Here is the generated code: https://github.com/josemarluedke/gqlgen-fed-service-type/blob/master/graph/generated/generated.go#L58-L64

What did you expect?

Only one type Service declared. No conflict with the Federation.

Minimal graphql.schema and models to reproduce

Full repro: https://github.com/josemarluedke/gqlgen-fed-service-type
GraphQL: https://github.com/josemarluedke/gqlgen-fed-service-type/blob/master/graph/schema.graphqls

versions

  • gqlgen version v0.11.3
  • go version 1.13.6
  • go modules
@stale
Copy link

stale bot commented Oct 18, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 18, 2020
@frederikhors
Copy link
Collaborator

nope

@stale stale bot removed the stale label Oct 18, 2020
@moensch
Copy link

moensch commented Oct 23, 2020

I just hit the same thing - glad it got reported. The ComplexityRoot ends up with two child structs named Service.

One of them represents my actual model, the other looks like this:

	Service struct {
		SDL func(childComplexity int) int
	}

gqlgen version v0.13.0
go version 1.14.3
using go modules

@zhevron
Copy link

zhevron commented Dec 16, 2020

I just found this one as well when trying to migrate an existing schema to use federation, so renaming my type is hardly an option without causing breaking changes.

It seems to be caused by an internal model representing the service (as required by Apollo Federation) defined here: https://github.com/99designs/gqlgen/blob/master/plugin/federation/fedruntime/runtime.go#L6

This is currently a blocker for us, so I'll take a stab at solving it in a way that works for us. If anyone has any bright ideas as to how this can be fixed without renaming the internal struct, I'm open to suggestions.

@moensch
Copy link

moensch commented Feb 22, 2021

It seems to be caused by an internal model representing the service (as required by Apollo Federation) defined here: https://github.com/99designs/gqlgen/blob/master/plugin/federation/fedruntime/runtime.go#L6

@zhevron I took a look at this too and believe it's a limitation in this function here:

func ToGo(name string) string {
if name == "_" {
return "_"
}
runes := make([]rune, 0, len(name))
wordWalker(name, func(info *wordInfo) {
word := info.Word
if info.MatchCommonInitial {
word = strings.ToUpper(word)
} else if !info.HasCommonInitial {
if strings.ToUpper(word) == word || strings.ToLower(word) == word {
// FOO or foo → Foo
// FOo → FOo
word = UcFirst(strings.ToLower(word))
}
}
runes = append(runes, []rune(word)...)
})
return string(runes)
}

The actual schema type is _service, but this function, when converting schema types to Go types, essentially ignores underscores.

I started an attempt to fix it by doing something as simple as this - but soon got stuck five layers deep in templates and code generation with no way out :-/

if strings.HasPrefix(name, "_") {
  return fmt.Sprintf("Underscore%s", string(runes))
}

The real "blow up" seems to occur here:

// Service is the service object that the
// generated.go file will return for the _service
// query
type Service struct {
SDL string `json:"sdl"`
}

So a different approach I took was through renaming fedruntime.Service to fedruntime.ApolloFederationService.

@zhevron
Copy link

zhevron commented Feb 22, 2021

@moensch You are absolutely correct that this is what causes it. We had our own entities named Entity and Service which in turn caused the problem. Just like you, I looked into the issue and found no real way but to mess with the templating.

This is how I fixed it for us, but YMMV: zhevron@cecd727

In short, I renamed the gqlgen Entity to FederatedEntity and Service to FederatedService. This also requires your user code to reference the FederatedEntity interface instead of the Entity interface, so it is a breaking change to the API.
The breaking change is also why I'm hesitant to push this back as a change. I'd like to find a way to solve this without causing any real API changes.

@moensch
Copy link

moensch commented Feb 22, 2021

I hear you. We are at a point where we could very easily still absorb upstream API changes. I would suggest you throw the PR out there and start gathering feedback - it looks eerily similar to what I've come up with so far.

I also tried looking at the code surrounding "complexity" to see why this is smashing types from different packages into the same "namespace". This seemed wrong to me, and is one of probably two places a fix could help avoid the need to rename this Go type.

@zhevron
Copy link

zhevron commented Feb 22, 2021

I'll take one last pass at the solution to see if I can at least move the "logic" of the change out of the core codegen package and into the federation plugin package. Seems weird to have a plugin halfway implemented in core. Hopefully I'll have something in the next couple of days or sooner, depending on my workload.

@AndrewRayCode
Copy link

Our team just hit this as well. We have a type named Service in the graph. Changing the name isn't an option because it's a value type used in other subgraphs and we have to match it identically.

I'm confused why something like this doesn't work:

in gqlgen.yml

  Service:
    model: app/model.ServiceTest

This seems to be handled fine by gqlgen, but the error still happens, and the duplicate Service struct gets put in the ComplexityRoot struct.

@AndrewRayCode
Copy link

@zhevron sorry for double ping, I mostly copy-pasta'd your work into a PR. It's what we're attempting in our own fork for now, so either way thanks for the code.

@zhevron
Copy link

zhevron commented Jun 25, 2021

@AndrewRayCode No worries at all. I'm glad you could take the time to make this. I've been absolutely swamped since the last update and will be for the foreseeable future. Turns out it's hard to get the time to address this when your workaround fork is working just fine.

AndrewRayCode added a commit to ConsultingMD/gqlgen that referenced this issue Jun 30, 2021
@StevenACoffman
Copy link
Collaborator

Hi! I merged #1531 which I think resolves this in a more general way. If you encounter problems, please open a new issue and reference both this and that. Thanks!

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

6 participants