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

Schema builder fixes; add schema validation of subscription fields #3867

Merged
merged 4 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,17 @@ public async Task Widget_Null_Nullable()
[Fact]
public async Task NotSubscriptionField()
{
var result = await ExecuteAsync("subscription { notSubscriptionField }");
result.ShouldNotBeSuccessful();
result.ShouldBeSimilarTo("""{"errors":[{"message":"Handled custom exception: Stream resolver not set for field \u0027notSubscriptionField\u0027.","locations":[{"line":1,"column":16}],"path":["notSubscriptionField"],"extensions":{"code":"INVALID_OPERATION","codes":["INVALID_OPERATION"]}}],"data":null}""");
var queryType = new ObjectGraphType() { Name = "Query" };
queryType.Field<StringGraphType>("dummy");
var subscriptionType = new ObjectGraphType() { Name = "Subscription" };
subscriptionType.Field<StringGraphType>("notSubscriptionField");
var schema = new Schema()
{
Query = queryType,
Subscription = subscriptionType
};
Should.Throw<InvalidOperationException>(() => schema.Initialize())
.Message.ShouldBe("The field 'notSubscriptionField' of the subscription root type 'Subscription' must have StreamResolver set.");
}

public int Counter = 0;
Expand Down Expand Up @@ -567,7 +575,7 @@ public static IObservable<MyWidget> TestNullHandling([FromServices] IObservable<

public static IObservable<string> TestObservableNull() => null!;

public static string NotSubscriptionField() => "testing";
//public static string NotSubscriptionField() => "testing";
}

private class MyWidget
Expand Down
15 changes: 12 additions & 3 deletions src/GraphQL.Tests/Utilities/SchemaBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void should_set_subscription_by_name()
}
""";

var schema = Schema.For(definitions);
var schema = Schema.For(definitions, b => b.Types.Include<DummySubscription>("Subscription"));
schema.Initialize();

var subscription = schema.Subscription;
Expand All @@ -109,6 +109,15 @@ public void should_set_subscription_by_name()
subscription.Fields.Single().Name.ShouldBe("subscribe");
}

private class DummySubscription
{
[GraphQLMetadata("subscribe")]
public string SubscribeResolver(IResolveFieldContext context) => (string)context.Source!;

[GraphQLMetadata(ResolverType = ResolverType.StreamResolver)]
public IObservable<string> Subscribe() => throw new NotImplementedException();
}

[Fact]
public void configures_schema_from_schema_type()
{
Expand All @@ -132,7 +141,7 @@ public void configures_schema_from_schema_type()
}
""";

var schema = Schema.For(definitions);
var schema = Schema.For(definitions, b => b.Types.Include<DummySubscription>("MySubscription"));
schema.Initialize();

var query = schema.Query;
Expand Down Expand Up @@ -177,7 +186,7 @@ public void configures_schema_from_schema_type_and_directives()
}
""";

var schema = Schema.For(definitions);
var schema = Schema.For(definitions, b => b.Types.Include<DummySubscription>("MySubscription"));
schema.Directives.Register(new Directive("public", DirectiveLocation.Schema));
schema.Directives.Register(new Directive("requireAuth", DirectiveLocation.Object) { Arguments = new QueryArguments(new QueryArgument<StringGraphType> { Name = "role" }) });
schema.Directives.Register(new Directive("traits", DirectiveLocation.FieldDefinition) { Arguments = new QueryArguments(new QueryArgument<NonNullGraphType<BooleanGraphType>> { Name = "volatile" }, new QueryArgument<BooleanGraphType> { Name = "documented" }, new QueryArgument<EnumerationGraphType<TestEnum>> { Name = "enumerated" }) });
Expand Down
7 changes: 6 additions & 1 deletion src/GraphQL/Utilities/SchemaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,19 @@ private Schema BuildSchemaFrom(GraphQLDocument document)

var directives = new List<Directive>();

// process the schema definition first
foreach (var def in document.Definitions)
{
if (def is GraphQLSchemaDefinition schemaDef)
{
_schemaDef = schemaDef;
Copy link
Member Author

@Shane32 Shane32 Jan 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_schemaDef must be set first or subscription resolvers don't get set properly. Who knows; maybe it affects more stuff.

schema.SetAstType(schemaDef);
}
else if (def is GraphQLObjectTypeDefinition objDef)
}

foreach (var def in document.Definitions)
{
if (def is GraphQLObjectTypeDefinition objDef)
{
var type = ToObjectGraphType(objDef);
_types[type.Name] = type;
Expand Down
4 changes: 2 additions & 2 deletions src/GraphQL/Utilities/TypeSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void Include(Type type)
/// </summary>
public void Include(string name, Type type)
{
_typeConfigurations[name].Type = type;
For(name).Type = type;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ForAll only triggers when using For, but seems to clearly indicate that it should operate on all types. So I've changed Include to call For and thereby execute any configured ForAll code.

}

/// <summary>
Expand Down Expand Up @@ -113,7 +113,7 @@ public void Include(Type type, Type typeOfType)
/// </summary>
public void Include(string name, Type type, Type typeOfType)
{
var config = _typeConfigurations[name];
var config = For(name);
config.Type = type;
config.IsTypeOfFunc = obj => obj?.GetType().IsAssignableFrom(typeOfType) ?? false;
}
Expand Down
8 changes: 8 additions & 0 deletions src/GraphQL/Utilities/Visitors/SchemaValidationVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,14 @@ public override void VisitSchema(ISchema schema)
var n3 = schema.Subscription?.Name;
if (n1 == n2 && n1 != null || n1 == n3 && n1 != null || n2 == n3 && n2 != null)
throw new InvalidOperationException("The query, mutation, and subscription root types must all be different types if provided.");
if (schema.Subscription != null)
{
foreach (var field in schema.Subscription.Fields.List)
{
if (field.StreamResolver == null)
throw new InvalidOperationException($"The field '{field.Name}' of the subscription root type '{schema.Subscription.Name}' must have StreamResolver set.");
}
}
}

// See 'Type Validation' section in https://spec.graphql.org/October2021/#sec-Unions
Expand Down
Loading