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

GraphQL schema building fails and is inefficient #16182

Closed
gvkries opened this issue May 28, 2024 · 6 comments · Fixed by #16184
Closed

GraphQL schema building fails and is inefficient #16182

gvkries opened this issue May 28, 2024 · 6 comments · Fixed by #16184
Labels
Milestone

Comments

@gvkries
Copy link
Contributor

gvkries commented May 28, 2024

Describe the bug

After merging #16143, building the GraphQL schema may fail if a field is resolved through DI multiple times.
E.g. if we have a content type with a media field and also a CustomUserSetting with another media field as well, it fails with:

System.InvalidOperationException: A different instance of the GraphType 'MediaFieldQueryObjectType' with the name 
'MediaField' has already been registered within the schema. Please use the same instance for all references 
within the schema, or use GraphQLTypeReference to reference a type instantiated elsewhere.
To view additional trace enable GlobalSwitches.TrackGraphTypeInitialization switch.
   at GraphQL.Types.SchemaTypes.EnsureTypeEquality(IGraphType existingType, IGraphType newType, TypeCollectionContext context) in /_/src/GraphQL/Types/Collections/SchemaTypes.cs:line 803
   at GraphQL.Types.SchemaTypes.AddTypeIfNotRegistered(IGraphType type, TypeCollectionContext context) in /_/src/GraphQL/Types/Collections/SchemaTypes.cs:line 780
   at GraphQL.Types.SchemaTypes.Initialize(ISchema schema, IServiceProvider serviceProvider, IEnumerable`1 graphTypeMappings) in /_/src/GraphQL/Types/Collections/SchemaTypes.cs:line 277
   at GraphQL.Types.SchemaTypes..ctor(ISchema schema, IServiceProvider serviceProvider, IEnumerable`1 graphTypeMappings) in /_/src/GraphQL/Types/Collections/SchemaTypes.cs:line 190
   at GraphQL.Types.SchemaTypes..ctor(ISchema schema, IServiceProvider serviceProvider) in /_/src/GraphQL/Types/Collections/SchemaTypes.cs:line 175
   at GraphQL.Types.Schema.CreateSchemaTypes() in /_/src/GraphQL/Types/Schema.cs:line 477
   at GraphQL.Types.Schema.CreateAndInitializeSchemaTypes() in /_/src/GraphQL/Types/Schema.cs:line 448
   at GraphQL.Types.Schema.Initialize() in /_/src/GraphQL/Types/Schema.cs:line 196
   at OrchardCore.Apis.GraphQL.Services.SchemaService.GetSchemaAsync() 

[...]

This comes from the fact that ObjectGraphType<T>s are now resolved multiple times as @tropcicstefan pointed out. The types are added manually to the schema and are also resolved by initializing the schema. This causes conflicting instances being used.

As we cannot register the GraphQL types as neither Scoped nor Singleton, we must ensure that building the schema will always use the correct instances. This may require some rework how we build the schema.

@MikeAlhayek
Copy link
Member

@gvkries Isn't this addresses by #16151?

Do you know if this issue exists in 1.8 or just main?

@gvkries
Copy link
Contributor Author

gvkries commented May 28, 2024

@gvkries Isn't this addresses by #16151?

Do you know if this issue exists in 1.8 or just main?

Nope, I found this today. It's in main only. I'm providing an fix soon, I hope...

@MikeAlhayek MikeAlhayek added this to the 2.0 milestone May 28, 2024
Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

@MikeAlhayek
Copy link
Member

okay. This is a 2.0 issue. If your PR is ready before the triage meeting in 2 hours, we can probably review the changes today. Trying to finish all the issues that are tagged with 2.0

@gvkries
Copy link
Contributor Author

gvkries commented May 28, 2024

I added a PR as a proposal. Not sure about it, maybe too many changes... 😇 I changed how the GraphQL types are accessed quite a lot.

@gvkries
Copy link
Contributor Author

gvkries commented May 30, 2024

Note that reverting #16143 will not fix the error, because schema building breaks with a different error...

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

Successfully merging a pull request may close this issue.

2 participants