Skip to content

Commit 1ab2fa3

Browse files
tobias-tenglertobias-tengler
authored
[Fusion] Fixed composition of spec directives (#7643)
1 parent f122129 commit 1ab2fa3

16 files changed

+280
-87
lines changed

‎src/HotChocolate/Fusion/src/Composition/Extensions/MergeExtensions.cs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -173,18 +173,15 @@ internal static void MergeDirectivesWith(
173173
foreach (var directive in source.Directives)
174174
{
175175
if (context.FusionTypes.IsFusionDirective(directive.Name)
176-
|| BuiltIns.IsBuiltInDirective(directive.Name)
176+
// @deprecated is handled separately
177+
|| directive.Name == BuiltIns.Deprecated.Name
177178
// @tag is handled separately
178179
|| directive.Name == "tag")
179180
{
180181
continue;
181182
}
182183

183-
if (context.FusionGraph.DirectiveDefinitions.TryGetDirective(directive.Name, out var directiveDefinition)
184-
&& directiveDefinition.IsSpecDirective)
185-
{
186-
continue;
187-
}
184+
context.FusionGraph.DirectiveDefinitions.TryGetDirective(directive.Name, out var directiveDefinition);
188185

189186
if (!target.Directives.ContainsName(directive.Name))
190187
{

‎src/HotChocolate/Fusion/src/Composition/FusionTypes.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ private DirectiveDefinition RegisterVariableDirectiveType(
183183
ScalarTypeDefinition selection)
184184
{
185185
var directiveType = new DirectiveDefinition(name);
186+
directiveType.IsRepeatable = true;
186187
directiveType.Arguments.Add(new InputFieldDefinition(NameArg, new NonNullTypeDefinition(typeName)));
187188
directiveType.Arguments.Add(new InputFieldDefinition(SelectArg, selection));
188189
directiveType.Arguments.Add(new InputFieldDefinition(ArgumentArg, typeName));
@@ -259,6 +260,7 @@ private DirectiveDefinition RegisterResolverDirectiveType(
259260
EnumTypeDefinition resolverKind)
260261
{
261262
var directiveType = new DirectiveDefinition(name);
263+
directiveType.IsRepeatable = true;
262264
directiveType.Locations |= Types.DirectiveLocation.Object;
263265
directiveType.Arguments.Add(new InputFieldDefinition(SelectArg, new NonNullTypeDefinition(selectionSet)));
264266
directiveType.Arguments.Add(new InputFieldDefinition(SubgraphArg, new NonNullTypeDefinition(typeName)));
@@ -296,6 +298,7 @@ private DirectiveDefinition RegisterSourceDirectiveType(string name, ScalarTypeD
296298
new InputFieldDefinition(NameArg, typeName),
297299
},
298300
};
301+
directiveType.IsRepeatable = true;
299302
directiveType.Features.Set(new FusionTypeMetadata { IsFusionType = true });
300303
_fusionGraph.DirectiveDefinitions.Add(directiveType);
301304
return directiveType;
@@ -344,6 +347,7 @@ private DirectiveDefinition RegisterTransportDirectiveType(
344347
ScalarTypeDefinition uri)
345348
{
346349
var directiveType = new DirectiveDefinition(name);
350+
directiveType.IsRepeatable = true;
347351
directiveType.Locations = Types.DirectiveLocation.FieldDefinition;
348352
directiveType.Arguments.Add(new InputFieldDefinition(SubgraphArg, new NonNullTypeDefinition(typeName)));
349353
directiveType.Arguments.Add(new InputFieldDefinition(ClientGroupArg, typeName));

‎src/HotChocolate/Fusion/src/Composition/Pipeline/MergeTypeMiddleware.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,12 @@ private static void MergeDirectiveDefinition(
118118
if (!target.Arguments.TryGetField(sourceArgument.Name, out var targetArgument))
119119
{
120120
context.Log.Write(LogEntryHelper.DirectiveDefinitionArgumentMismatch(new SchemaCoordinate(source.Name), source));
121-
return;
121+
continue;
122122
}
123123

124124
if (!sourceArgument.Type.Equals(targetArgument.Type, TypeComparison.Structural))
125125
{
126126
context.Log.Write(LogEntryHelper.DirectiveDefinitionArgumentMismatch(new SchemaCoordinate(source.Name), source));
127-
return;
128127
}
129128
}
130129

‎src/HotChocolate/Fusion/test/Composition.Tests/DirectiveTests.cs

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using CookieCrumble;
2+
using HotChocolate.Fusion.Composition.Features;
23
using HotChocolate.Fusion.Metadata;
34
using HotChocolate.Fusion.Shared;
45
using HotChocolate.Language;
@@ -358,6 +359,179 @@ directive @test(arg: String) repeatable on FIELD_DEFINITION
358359
""");
359360
}
360361

362+
[Fact]
363+
public async Task Repeat_Repeatable_Directive_Applied_Multiple_Times_On_Same_Field()
364+
{
365+
// arrange
366+
var subgraphA = await TestSubgraph.CreateAsync(
367+
"""
368+
type Query {
369+
foo: SubType @test(arg: "A1") @test(arg: "A2")
370+
}
371+
372+
type SubType {
373+
bar: String @test(arg: "A1") @test(arg: "A2")
374+
}
375+
376+
directive @test(arg: String) repeatable on FIELD_DEFINITION
377+
""");
378+
379+
var subgraphB = await TestSubgraph.CreateAsync(
380+
"""
381+
type Query {
382+
foo: SubType @test(arg: "B1") @test(arg: "B2")
383+
}
384+
385+
type SubType {
386+
bar: String @test(arg: "B1") @test(arg: "B2")
387+
}
388+
389+
directive @test(arg: String) repeatable on FIELD_DEFINITION
390+
""");
391+
392+
using var subgraphs = new TestSubgraphCollection(output, [subgraphA, subgraphB]);
393+
394+
// act
395+
var fusionGraph = await subgraphs.GetFusionGraphAsync();
396+
397+
// assert
398+
GetSchemaWithoutFusion(fusionGraph).MatchInlineSnapshot(
399+
"""
400+
schema {
401+
query: Query
402+
}
403+
404+
type Query {
405+
foo: SubType @test(arg: "A1") @test(arg: "A2") @test(arg: "B1") @test(arg: "B2")
406+
}
407+
408+
type SubType {
409+
bar: String @test(arg: "A1") @test(arg: "A2") @test(arg: "B1") @test(arg: "B2")
410+
}
411+
412+
directive @test(arg: String) repeatable on FIELD_DEFINITION
413+
""");
414+
}
415+
416+
[Fact]
417+
public async Task Properly_Compose_TypeSystem_Spec_Directives()
418+
{
419+
// arrange
420+
var subgraphSchemaA = """
421+
type Query {
422+
field: CustomScalar @deprecated(reason: "Deprecated")
423+
field2: String
424+
}
425+
426+
scalar CustomScalar @specifiedBy(url: "https://foo.bar")
427+
""";
428+
429+
var subgraphSchemaB = """
430+
type Query {
431+
field: CustomScalar @deprecated(reason: "Deprecated")
432+
field2: String
433+
}
434+
435+
scalar CustomScalar @specifiedBy(url: "https://foo.bar")
436+
""";
437+
438+
var features = new FusionFeatureCollection(FusionFeatures.NodeField);
439+
440+
var configurations = new [] { subgraphSchemaA, subgraphSchemaB }
441+
.Select((schema, index) =>
442+
{
443+
return new SubgraphConfiguration(
444+
index.ToString(),
445+
schema,
446+
string.Empty,
447+
new IClientConfiguration[]
448+
{
449+
new HttpClientConfiguration(new Uri("http://localhost:5000/graphql")),
450+
},
451+
null);
452+
});
453+
454+
// act
455+
var fusionGraph = await new FusionGraphComposer(logFactory:_logFactory)
456+
.ComposeAsync(configurations, features);
457+
458+
// assert
459+
GetSchemaWithoutFusion(fusionGraph).MatchInlineSnapshot(
460+
"""
461+
schema {
462+
query: Query
463+
}
464+
465+
type Query {
466+
field: CustomScalar @deprecated(reason: "Deprecated")
467+
field2: String
468+
}
469+
470+
scalar CustomScalar @specifiedBy(url: "https:\/\/foo.bar")
471+
""");
472+
}
473+
474+
[Fact]
475+
public async Task Properly_Compose_TypeSystem_Spec_Directives_When_They_Are_Part_Of_Subgraph_Schema()
476+
{
477+
// arrange
478+
var subgraphSchemaA = """
479+
type Query {
480+
field: CustomScalar @deprecated(reason: "Deprecated")
481+
field2: String
482+
}
483+
484+
scalar CustomScalar @specifiedBy(url: "https://foo.bar")
485+
""";
486+
487+
var subgraphSchemaB = """
488+
type Query {
489+
field: CustomScalar @deprecated(reason: "Deprecated")
490+
field2: String
491+
}
492+
493+
"The `@specifiedBy` directive is used within the type system definition language to provide a URL for specifying the behavior of custom scalar definitions."
494+
directive @specifiedBy("The specifiedBy URL points to a human-readable specification. This field will only read a result for scalar types." url: String!) on SCALAR
495+
496+
scalar CustomScalar @specifiedBy(url: "https://foo.bar")
497+
""";
498+
499+
var features = new FusionFeatureCollection(FusionFeatures.NodeField);
500+
501+
var configurations = new [] { subgraphSchemaA, subgraphSchemaB }
502+
.Select((schema, index) =>
503+
{
504+
return new SubgraphConfiguration(
505+
index.ToString(),
506+
schema,
507+
string.Empty,
508+
new IClientConfiguration[]
509+
{
510+
new HttpClientConfiguration(new Uri("http://localhost:5000/graphql")),
511+
},
512+
null);
513+
});
514+
515+
// act
516+
var fusionGraph = await new FusionGraphComposer(logFactory:_logFactory)
517+
.ComposeAsync(configurations, features);
518+
519+
// assert
520+
GetSchemaWithoutFusion(fusionGraph).MatchInlineSnapshot(
521+
"""
522+
schema {
523+
query: Query
524+
}
525+
526+
type Query {
527+
field: CustomScalar @deprecated(reason: "Deprecated")
528+
field2: String
529+
}
530+
531+
scalar CustomScalar @specifiedBy(url: "https:\/\/foo.bar")
532+
""");
533+
}
534+
361535
private static DocumentNode GetSchemaWithoutFusion(SchemaDefinition fusionGraph)
362536
{
363537
var fusionGraphDoc = Utf8GraphQLParser.Parse(SchemaFormatter.FormatAsString(fusionGraph));

‎src/HotChocolate/Skimmed/src/Skimmed/BuiltIns/BooleanTypeDefinition.cs

Lines changed: 0 additions & 10 deletions
This file was deleted.

‎src/HotChocolate/Skimmed/src/Skimmed/BuiltIns/BuiltIns.cs

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,40 +6,35 @@ public static class String
66
{
77
public const string Name = nameof(String);
88

9-
public static StringTypeDefinition Create()
10-
=> new();
9+
public static ScalarTypeDefinition Create() => new(Name) { IsSpecScalar = true };
1110
}
1211

1312
public static class Boolean
1413
{
1514
public const string Name = nameof(Boolean);
1615

17-
public static BooleanTypeDefinition Create()
18-
=> new();
16+
public static ScalarTypeDefinition Create() => new(Name) { IsSpecScalar = true };
1917
}
2018

2119
public static class Float
2220
{
2321
public const string Name = nameof(Float);
2422

25-
public static FloatTypeDefinition Create()
26-
=> new();
23+
public static ScalarTypeDefinition Create() => new(Name) { IsSpecScalar = true };
2724
}
2825

2926
public static class ID
3027
{
3128
public const string Name = nameof(ID);
3229

33-
public static IDTypeDefinition Create()
34-
=> new();
30+
public static ScalarTypeDefinition Create() => new(Name) { IsSpecScalar = true };
3531
}
3632

3733
public static class Int
3834
{
3935
public const string Name = nameof(Int);
4036

41-
public static IntTypeDefinition Create()
42-
=> new();
37+
public static ScalarTypeDefinition Create() => new(Name) { IsSpecScalar = true };
4338
}
4439

4540
public static class Include
@@ -49,13 +44,13 @@ public static class Include
4944

5045
public static IncludeDirectiveDefinition Create(SchemaDefinition schema)
5146
{
52-
if (!schema.Types.TryGetType<BooleanTypeDefinition>(Boolean.Name, out var typeDef))
47+
if (!schema.Types.TryGetType<ScalarTypeDefinition>(Boolean.Name, out var booleanTypeDef))
5348
{
54-
typeDef = new BooleanTypeDefinition();
55-
schema.Types.Add(typeDef);
49+
booleanTypeDef = Boolean.Create();
50+
schema.Types.Add(booleanTypeDef);
5651
}
5752

58-
return new IncludeDirectiveDefinition(typeDef);
53+
return new IncludeDirectiveDefinition(booleanTypeDef);
5954
}
6055
}
6156

@@ -66,13 +61,13 @@ public static class Skip
6661

6762
public static SkipDirectiveDefinition Create(SchemaDefinition schema)
6863
{
69-
if (!schema.Types.TryGetType<BooleanTypeDefinition>(Boolean.Name, out var typeDef))
64+
if (!schema.Types.TryGetType<ScalarTypeDefinition>(Boolean.Name, out var booleanTypeDef))
7065
{
71-
typeDef = new BooleanTypeDefinition();
72-
schema.Types.Add(typeDef);
66+
booleanTypeDef = Boolean.Create();
67+
schema.Types.Add(booleanTypeDef);
7368
}
7469

75-
return new SkipDirectiveDefinition(typeDef);
70+
return new SkipDirectiveDefinition(booleanTypeDef);
7671
}
7772
}
7873

@@ -83,13 +78,30 @@ public static class Deprecated
8378

8479
public static DeprecatedDirectiveDefinition Create(SchemaDefinition schema)
8580
{
86-
if (!schema.Types.TryGetType<StringTypeDefinition>(String.Name, out var typeDef))
81+
if (!schema.Types.TryGetType<ScalarTypeDefinition>(String.Name, out var stringTypeDef))
8782
{
88-
typeDef = new StringTypeDefinition();
89-
schema.Types.Add(typeDef);
83+
stringTypeDef = String.Create();
84+
schema.Types.Add(stringTypeDef);
9085
}
9186

92-
return new DeprecatedDirectiveDefinition(typeDef);
87+
return new DeprecatedDirectiveDefinition(stringTypeDef);
88+
}
89+
}
90+
91+
public static class SpecifiedBy
92+
{
93+
public const string Name = "specifiedBy";
94+
public const string Url = "url";
95+
96+
public static SpecifiedByDirectiveDefinition Create(SchemaDefinition schema)
97+
{
98+
if (!schema.Types.TryGetType<ScalarTypeDefinition>(String.Name, out var stringTypeDef))
99+
{
100+
stringTypeDef = String.Create();
101+
schema.Types.Add(stringTypeDef);
102+
}
103+
104+
return new SpecifiedByDirectiveDefinition(stringTypeDef);
93105
}
94106
}
95107

@@ -110,6 +122,7 @@ public static bool IsBuiltInDirective(string name)
110122
Include.Name => true,
111123
Skip.Name => true,
112124
Deprecated.Name => true,
125+
SpecifiedBy.Name => true,
113126
_ => false
114127
};
115128
}

0 commit comments

Comments
 (0)