Prevent using graphtype as model #3316
Conversation
github-actions
bot
added
the
test
|
This PR doesn't work because
|
|
Solution 2 implemented here |
Codecov Report
@@ Coverage Diff @@
## master #3316 +/- ##
=======================================
Coverage 84.04% 84.04%
=======================================
Files 376 376
Lines 16179 16181 +2
Branches 2600 2601 +1
=======================================
+ Hits 13597 13599 +2
Misses 1964 1964
Partials 618 618
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
|
||
| Query = query; | ||
| } | ||
| } | ||
|
|
||
| public class DataGraphType : ObjectGraphType<DataGraphType> |
There was a problem hiding this comment.
That means that in theory in very specific cases someone may want to inherit from ObjectGraphType<OtherGraphType>.
| @@ -17,6 +17,9 @@ public abstract class ComplexGraphType<TSourceType> : GraphType, IComplexGraphTy | |||
| /// <inheritdoc/> | |||
| protected ComplexGraphType() | |||
| { | |||
| if (typeof(IGraphType).IsAssignableFrom(typeof(TSourceType)) && GetType() != typeof(Introspection.__Type)) | |||
There was a problem hiding this comment.
🤔 Maybe in future we introduce some flag (GlobalSwitch?) to allow this behavior.
There was a problem hiding this comment.
I understand that for 99,99% cases this check helps to solve developer problems.
|
|
||
| Query = query; | ||
| } | ||
| } | ||
|
|
||
| public class NullableSchemaType : ObjectGraphType<NullableSchemaType> |
There was a problem hiding this comment.
These are tests, after all - would they really be coded like this for production use? I highly doubt it — running all the graph constructor code for a data model? Seems highly unlikely.
Could be though - I’m not sure.
There was a problem hiding this comment.
Tests, yes. We ourselves write a code that should not be written.
|
Maybe a global switch. I figured that (a) if there’s a problem then someone will raise an issue (b) people can use the non-generic version of ObjectGraphType if necessary in these rare circumstances. I suggest we wait until someone complains before adding a global switch. |
|
Of course. |
…TRAC-6694-upgrade-graphql-dotnet-server-to-7.1 * commit '13da37d7c0649cc6186714a6671272af49f06d85': (1255 commits) Add type integrity check when existing types are found (graphql-dotnet#3332) Switch to Nullability.Source (graphql-dotnet#3314) Add missing authorization extensions from field and connection builders (graphql-dotnet#3324) Prevent using graphtype as model (graphql-dotnet#3316) Remove init-only properties from pre-.NET 5 targets (graphql-dotnet#3323) Update UnionGraphType to support CLR references (graphql-dotnet#3320) Bump BenchmarkDotNet from 0.13.1 to 0.13.2 (graphql-dotnet#3313) Bump Microsoft.NET.Test.Sdk from 17.3.0 to 17.3.1 (graphql-dotnet#3311) Fix InputFieldsAndArgumentsOfCorrectLength validation rule (graphql-dotnet#3307) Bump Shouldly from 4.0.3 to 4.1.0 (graphql-dotnet#3304) Add test (graphql-dotnet#3302) !(a is T) -> a is not T (graphql-dotnet#3300) User not duplicated in context (graphql-dotnet#3298) Bump deps (graphql-dotnet#3295) Restore `Field<TGraphType>()` method (graphql-dotnet#3294) Include v7 migration document link in readme (graphql-dotnet#3290) Bump GraphQL-Parser to 8.1.0 (graphql-dotnet#3289) Migration notes updates (graphql-dotnet#3287) Simplify configuration (graphql-dotnet#3286) Introduce ErrorInfoProviderOptions.ExposeExceptionDetailsMode (graphql-dotnet#3276) ... # Conflicts: # docs/package.json # src/GraphQL/GraphQL.csproj # src/GraphQL/Types/GraphTypesLookup.cs
Implemented within
ComplexGraphType<TSourceType>Applies to:
ObjectGraphType<TSourceType>InputObjectGraphType<TSourceType>AutoRegisteringObjectGraphType<TSourceType>AutoRegisteringInputObjectGraphType<TSourceType>