Skip to content

Commit

Permalink
[Fusion] Added pre-merge validation rule "ProvidesFieldsMissingExtern…
Browse files Browse the repository at this point in the history
…alRule" (#7885)
  • Loading branch information
glen-84 authored Jan 1, 2025
1 parent 6dcaa60 commit 027dc0b
Show file tree
Hide file tree
Showing 7 changed files with 202 additions and 0 deletions.
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."
]
}
};
}
}

0 comments on commit 027dc0b

Please sign in to comment.