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 "LookupMustNotReturnListRule" #7890

Merged
merged 3 commits into from
Jan 2, 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 @@ -12,6 +12,7 @@ public static class LogEntryCodes
public const string KeyFieldsSelectInvalidType = "KEY_FIELDS_SELECT_INVALID_TYPE";
public const string KeyInvalidFields = "KEY_INVALID_FIELDS";
public const string KeyInvalidSyntax = "KEY_INVALID_SYNTAX";
public const string LookupMustNotReturnList = "LOOKUP_MUST_NOT_RETURN_LIST";
public const string LookupShouldHaveNullableReturnType = "LOOKUP_SHOULD_HAVE_NULLABLE_RETURN_TYPE";
public const string OutputFieldTypesNotMergeable = "OUTPUT_FIELD_TYPES_NOT_MERGEABLE";
public const string ProvidesDirectiveInFieldsArg = "PROVIDES_DIRECTIVE_IN_FIELDS_ARG";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,25 @@ public static LogEntry KeyInvalidSyntax(
schema);
}

public static LogEntry LookupMustNotReturnList(
OutputFieldDefinition field,
INamedTypeDefinition type,
SchemaDefinition schema)
{
var coordinate = new SchemaCoordinate(type.Name, field.Name);

return new LogEntry(
string.Format(
LogEntryHelper_LookupMustNotReturnList,
coordinate,
schema.Name),
LogEntryCodes.LookupMustNotReturnList,
LogSeverity.Error,
coordinate,
field,
schema);
}

public static LogEntry LookupShouldHaveNullableReturnType(
OutputFieldDefinition field,
INamedTypeDefinition type,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using HotChocolate.Fusion.Events;
using HotChocolate.Skimmed;
using static HotChocolate.Fusion.Logging.LogEntryHelper;

namespace HotChocolate.Fusion.PreMergeValidation.Rules;

/// <summary>
/// Fields annotated with the <c>@lookup</c> directive are intended to retrieve a single entity
/// based on provided arguments. To avoid ambiguity in entity resolution, such fields must return a
/// single object and not a list. This validation rule enforces that any field annotated with
/// <c>@lookup</c> must have a return type that is <b>NOT</b> a list.
/// </summary>
/// <seealso href="https://graphql.github.io/composite-schemas-spec/draft/#sec--lookup-must-not-return-a-list">
/// Specification
/// </seealso>
internal sealed class LookupMustNotReturnListRule : IEventHandler<OutputFieldEvent>
{
public void Handle(OutputFieldEvent @event, CompositionContext context)
{
var (field, type, schema) = @event;

if (ValidationHelper.IsLookup(field) && field.Type.NullableType() is ListTypeDefinition)
{
context.Log.Write(LookupMustNotReturnList(field, type, 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 @@ -63,6 +63,9 @@
<data name="LogEntryHelper_KeyInvalidSyntax" xml:space="preserve">
<value>A @key directive on type '{0}' in schema '{1}' contains invalid syntax in the 'fields' argument.</value>
</data>
<data name="LogEntryHelper_LookupMustNotReturnList" xml:space="preserve">
<value>The lookup field '{0}' in schema '{1}' must not return a list.</value>
</data>
<data name="LogEntryHelper_LookupShouldHaveNullableReturnType" xml:space="preserve">
<value>The lookup field '{0}' in schema '{1}' should return a nullable type.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ private CompositionResult<SchemaDefinition> MergeSchemaDefinitions(CompositionCo
new KeyFieldsSelectInvalidTypeRule(),
new KeyInvalidFieldsRule(),
new KeyInvalidSyntaxRule(),
new LookupMustNotReturnListRule(),
new LookupShouldHaveNullableReturnTypeRule(),
new OutputFieldTypesMergeableRule(),
new ProvidesDirectiveInFieldsArgumentRule(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
using HotChocolate.Fusion.Logging;
using HotChocolate.Fusion.PreMergeValidation;
using HotChocolate.Fusion.PreMergeValidation.Rules;

namespace HotChocolate.Composition.PreMergeValidation.Rules;

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

[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 == "LOOKUP_MUST_NOT_RETURN_LIST"));
Assert.True(context.Log.All(e => e.Severity == LogSeverity.Error));
}

public static TheoryData<string[]> ValidExamplesData()
{
return new TheoryData<string[]>
{
// In this example, "userById" returns a "User" object, satisfying the requirement.
{
[
"""
type Query {
userById(id: ID!): User @lookup
}
glen-84 marked this conversation as resolved.
Show resolved Hide resolved

type User {
id: ID!
name: String
}
"""
]
}
};
}

public static TheoryData<string[], string[]> InvalidExamplesData()
{
return new TheoryData<string[], string[]>
{
// Here, "usersByIds" returns a list of "User" objects, which violates the requirement
// that a @lookup field must return a single object.
{
[
"""
type Query {
usersByIds(ids: [ID!]!): [User!] @lookup
}
glen-84 marked this conversation as resolved.
Show resolved Hide resolved

type User {
id: ID!
name: String
}
"""
],
[
"The lookup field 'Query.usersByIds' in schema 'A' must not return a list."
]
},
// Non-null list.
{
[
"""
type Query {
usersByIds(ids: [ID!]!): [User!]! @lookup
}

type User {
id: ID!
name: String
}
"""
],
[
"The lookup field 'Query.usersByIds' in schema 'A' must not return a list."
]
}
glen-84 marked this conversation as resolved.
Show resolved Hide resolved
};
}
}
Loading