Skip to content

Commit

Permalink
[Fusion] Added pre-merge validation rule "LookupMustNotReturnListRule" (
Browse files Browse the repository at this point in the history
  • Loading branch information
danielreynolds1 authored Jan 2, 2025
1 parent 0f469a5 commit 7df44ba
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 0 deletions.
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
}
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
}
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."
]
}
};
}
}

0 comments on commit 7df44ba

Please sign in to comment.