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

[Fusion] Added pre-merge validation rule "ProvidesFieldsMissingExternalRule" #7885

Merged
merged 1 commit into from
Jan 1, 2025
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 @@ -15,6 +15,7 @@ public static class LogEntryCodes
public const string OutputFieldTypesNotMergeable = "OUTPUT_FIELD_TYPES_NOT_MERGEABLE";
public const string ProvidesDirectiveInFieldsArg = "PROVIDES_DIRECTIVE_IN_FIELDS_ARG";
public const string ProvidesFieldsHasArgs = "PROVIDES_FIELDS_HAS_ARGS";
public const string ProvidesFieldsMissingExternal = "PROVIDES_FIELDS_MISSING_EXTERNAL";
public const string RootMutationUsed = "ROOT_MUTATION_USED";
public const string RootQueryUsed = "ROOT_QUERY_USED";
public const string RootSubscriptionUsed = "ROOT_SUBSCRIPTION_USED";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,29 @@ public static LogEntry ProvidesFieldsHasArguments(
schema);
}

public static LogEntry ProvidesFieldsMissingExternal(
string providedFieldName,
string providedTypeName,
Directive providesDirective,
string fieldName,
string typeName,
SchemaDefinition schema)
{
var coordinate = new SchemaCoordinate(typeName, fieldName);

return new LogEntry(
string.Format(
LogEntryHelper_ProvidesFieldsMissingExternal,
coordinate,
schema.Name,
new SchemaCoordinate(providedTypeName, providedFieldName)),
LogEntryCodes.ProvidesFieldsMissingExternal,
LogSeverity.Error,
coordinate,
providesDirective,
schema);
}

public static LogEntry RootMutationUsed(SchemaDefinition schema)
{
return new LogEntry(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
using HotChocolate.Fusion.Events;
using static HotChocolate.Fusion.Logging.LogEntryHelper;

namespace HotChocolate.Fusion.PreMergeValidation.Rules;

/// <summary>
/// <para>
/// The <c>@provides</c> directive indicates that an object type field will supply additional fields
/// belonging to the return type in this execution-specific path. Any field listed in the
/// <c>@provides(fields: ...)</c> argument must therefore be <i>external</i> in the local schema,
/// meaning that the local schema itself does <b>not</b> provide it.
/// </para>
/// <para>
/// This rule disallows selecting non-external fields in a <c>@provides</c> selection set. If a
/// field is already provided by the same schema in all execution paths, there is no need to
/// <c>@provide</c>.
/// </para>
/// </summary>
/// <seealso href="https://graphql.github.io/composite-schemas-spec/draft/#sec-Provides-Fields-Missing-External">
/// Specification
/// </seealso>
internal sealed class ProvidesFieldsMissingExternalRule : IEventHandler<ProvidesFieldEvent>
{
public void Handle(ProvidesFieldEvent @event, CompositionContext context)
{
var (providedField, providedType, providesDirective, field, type, schema) = @event;

if (!ValidationHelper.IsExternal(providedField))
{
context.Log.Write(
ProvidesFieldsMissingExternal(
providedField.Name,
providedType.Name,
providesDirective,
field.Name,
type.Name,
schema));
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@
<data name="LogEntryHelper_ProvidesFieldsHasArguments" xml:space="preserve">
<value>The @provides directive on field '{0}' in schema '{1}' references field '{2}', which must not have arguments.</value>
</data>
<data name="LogEntryHelper_ProvidesFieldsMissingExternal" xml:space="preserve">
<value>The @provides directive on field '{0}' in schema '{1}' references field '{2}', which must be marked as external.</value>
</data>
<data name="LogEntryHelper_RootMutationUsed" xml:space="preserve">
<value>The root mutation type in schema '{0}' must be named 'Mutation'.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ private CompositionResult<SchemaDefinition> MergeSchemaDefinitions(CompositionCo
new OutputFieldTypesMergeableRule(),
new ProvidesDirectiveInFieldsArgumentRule(),
new ProvidesFieldsHasArgumentsRule(),
new ProvidesFieldsMissingExternalRule(),
new RootMutationUsedRule(),
new RootQueryUsedRule(),
new RootSubscriptionUsedRule()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
using HotChocolate.Fusion.Logging;
using HotChocolate.Fusion.PreMergeValidation;
using HotChocolate.Fusion.PreMergeValidation.Rules;

namespace HotChocolate.Composition.PreMergeValidation.Rules;

public sealed class ProvidesFieldsMissingExternalRuleTests : CompositionTestBase
{
private readonly PreMergeValidator _preMergeValidator =
new([new ProvidesFieldsMissingExternalRule()]);

[Theory]
[MemberData(nameof(ValidExamplesData))]
public void Examples_Valid(string[] sdl)
{
// arrange
var context = CreateCompositionContext(sdl);

// act
var result = _preMergeValidator.Validate(context);

// assert
Assert.True(result.IsSuccess);
Assert.True(context.Log.IsEmpty);
}

[Theory]
[MemberData(nameof(InvalidExamplesData))]
public void Examples_Invalid(string[] sdl, string[] errorMessages)
{
// arrange
var context = CreateCompositionContext(sdl);

// act
var result = _preMergeValidator.Validate(context);

// assert
Assert.True(result.IsFailure);
Assert.Equal(errorMessages, context.Log.Select(e => e.Message).ToArray());
Assert.True(context.Log.All(e => e.Code == "PROVIDES_FIELDS_MISSING_EXTERNAL"));
Assert.True(context.Log.All(e => e.Severity == LogSeverity.Error));
}

public static TheoryData<string[]> ValidExamplesData()
{
return new TheoryData<string[]>
{
// Here, the "Order" type from this schema is providing fields on "User" through
// @provides. The "name" field of "User" is not defined in this schema; it is declared
// with @external indicating that the "name" field comes from elsewhere. Thus,
// referencing "name" under @provides(fields: "name") is valid.
{
[
"""
type Order {
id: ID!
customer: User @provides(fields: "name")
}

type User @key(fields: "id") {
id: ID!
name: String @external
}
"""
]
}
};
}

public static TheoryData<string[], string[]> InvalidExamplesData()
{
return new TheoryData<string[], string[]>
{
// In this example, "User.address" is not marked as @external in the same schema that
// applies @provides. This means the schema already provides the "address" field in all
// possible paths, so using @provides(fields: "address") is invalid.
{
[
"""
type User {
id: ID!
address: String
}

type Order {
id: ID!
buyer: User @provides(fields: "address")
}
"""
],
[
"The @provides directive on field 'Order.buyer' in schema 'A' references " +
"field 'User.address', which must be marked as external."
]
},
// Nested field.
{
[
"""
type User {
id: ID!
info: UserInfo
}

type UserInfo {
address: String
}

type Order {
id: ID!
buyer: User @provides(fields: "info { address }")
}
"""
],
[
"The @provides directive on field 'Order.buyer' in schema 'A' references " +
"field 'User.info', which must be marked as external.",

"The @provides directive on field 'Order.buyer' in schema 'A' references " +
"field 'UserInfo.address', which must be marked as external."
]
}
};
}
}
Loading