Optimized Resolver Composition (#6542)

ChilliCream · Sep 17, 2023 · 19ed59d · 19ed59d
1 parent e1c6c79
commit 19ed59d
Show file tree

Hide file tree

Showing 73 changed files with 372 additions and 512 deletions.
diff --git a/src/HotChocolate/Fusion/src/Composition/Entities/EntityResolver.cs b/src/HotChocolate/Fusion/src/Composition/Entities/EntityResolver.cs
@@ -34,6 +34,9 @@ public EntityResolver(
         SubgraphName = subgraphName ?? throw new ArgumentNullException(nameof(subgraphName));
     }
 
+    /// <summary>
+    /// Gets the type of this resolver.
+    /// </summary>
     public EntityResolverKind Kind { get; }
 
     /// <summary>

diff --git a/src/HotChocolate/Fusion/src/Composition/Entities/EntityResolversCollection.cs b/src/HotChocolate/Fusion/src/Composition/Entities/EntityResolversCollection.cs
@@ -8,6 +8,7 @@ internal class EntityResolversCollection : ICollection<EntityResolver>
 {
     private readonly List<EntityResolver> _resolvers = new();
     private readonly Dictionary<ResolverKey, EntityResolver> _map = new();
+    private readonly HashSet<ResolverSignatureKey> _signatureKeys = new();
 
     public int Count => _resolvers.Count;
 
@@ -18,13 +19,14 @@ public bool Contains(EntityResolver item)
 
     public void Add(EntityResolver item)
     {
+        _signatureKeys.Add(item);
         _map.Add(item, item);
         _resolvers.Add(item);
     }
 
     public bool TryAdd(EntityResolver item)
     {
-        if (_map.TryAdd(item, item))
+        if (_signatureKeys.Add(item) && _map.TryAdd(item, item))
         {
             _resolvers.Add(item);
             return true;
@@ -39,6 +41,7 @@ public bool Remove(EntityResolver item)
         {
             _map.Remove(item);
             _resolvers.Remove(resolver);
+            _signatureKeys.Remove(resolver);
             return true;
         }
 
@@ -69,7 +72,7 @@ public bool Equals(ResolverKey? other)
                 return false;
             }
 
-            return string.Equals(SubgraphName, other.Value.SubgraphName) &&
+            return string.Equals(SubgraphName, other.Value.SubgraphName, StringComparison.Ordinal) &&
                 SyntaxComparer.BySyntax.Equals(SelectionSet, other.Value.SelectionSet);
         }
 
@@ -93,4 +96,55 @@ public override string ToString()
         public static implicit operator ResolverKey(EntityResolver value)
             => new(value.SubgraphName, value.SelectionSet); 
     }
+
+    private readonly record struct ResolverSignatureKey(string SubgraphName, string StateKeys, EntityResolverKind Kind)
+    {
+        public bool Equals(ResolverSignatureKey? other)
+        {
+            if (other is null)
+            {
+                return false;
+            }
+
+            return string.Equals(SubgraphName, other.Value.SubgraphName, StringComparison.Ordinal) &&
+                string.Equals(StateKeys, other.Value.StateKeys, StringComparison.Ordinal) &&
+                Kind.Equals(other.Value.Kind);
+        }
+
+        public override int GetHashCode()
+            => HashCode.Combine(SubgraphName, StateKeys, Kind);
+
+        private bool PrintMembers(StringBuilder builder)
+        {
+            builder.AppendLine(SubgraphName);
+            builder.AppendLine(StateKeys);
+            builder.AppendLine(Kind.ToString());
+            return true;
+        }
+
+        public override string ToString()
+        {
+            var builder = new StringBuilder();
+            PrintMembers(builder);
+            return builder.ToString();
+        }
+
+        public static implicit operator ResolverSignatureKey(EntityResolver value)
+        {
+            if (value.Variables.Count == 1)
+            {
+                return new ResolverSignatureKey(value.SubgraphName, value.Variables.Keys.First(), value.Kind);
+            }
+
+            var builder = new StringBuilder();
+
+            foreach (var stateKey in value.Variables.OrderBy(t => t.Key))
+            {
+                builder.Append('>');
+                builder.Append(stateKey.Key);
+            }
+
+            return new ResolverSignatureKey(value.SubgraphName, builder.ToString(), value.Kind);
+        }
+    }
 }
diff --git a/src/HotChocolate/Fusion/src/Composition/Pipeline/Enrichers/PatternEntityEnricher.cs b/src/HotChocolate/Fusion/src/Composition/Pipeline/Enrichers/PatternEntityEnricher.cs
@@ -133,7 +133,8 @@ private static bool TryResolveKeyArgument(
     {
         if (entityResolverField.Arguments.TryGetField(keyField.Name, out keyArgument))
         {
-            return keyArgument.Type.Equals(keyField.Type, TypeComparison.Structural);
+            return !keyArgument.ContainsIsDirective() && 
+                keyArgument.Type.Equals(keyField.Type, TypeComparison.Structural);
         }
 
         if (entityResolverField.Arguments.Count == 1)
@@ -163,7 +164,8 @@ private static bool TryResolveKeyArgument(
             }
         }
 
-        return keyArgument?.Type.Equals(keyField.Type, TypeComparison.Structural) ?? false;
+        return (keyArgument?.Type.Equals(keyField.Type, TypeComparison.Structural) ?? false) && 
+            !keyArgument.ContainsIsDirective();
     }
 
     private static void TryRegisterBatchEntityResolver(
@@ -244,7 +246,7 @@ private static bool TryResolveBatchKeyArgument(
     {
         if (entityResolverField.Arguments.TryGetField(keyField.Name, out keyArgument))
         {
-            if (keyArgument.Type.IsListType())
+            if (keyArgument.Type.IsListType() && !keyArgument.ContainsIsDirective())
             {
                 return keyArgument.Type.Equals(keyField.Type.InnerType(), TypeComparison.Structural);
             }
@@ -257,7 +259,7 @@ private static bool TryResolveBatchKeyArgument(
         {
             keyArgument = entityResolverField.Arguments.First();
 
-            if (keyArgument.Type.IsListType())
+            if (keyArgument.Type.IsListType() && !keyArgument.ContainsIsDirective())
             {
                 return keyArgument.Type.Equals(keyField.Type.InnerType(), TypeComparison.Structural);
             }
@@ -287,7 +289,8 @@ private static bool TryResolveBatchKeyArgument(
         }
 
         if (keyArgument?.Type.IsListType() is true && 
-            keyArgument.Type.InnerType().Equals(keyField.Type, TypeComparison.Structural))
+            keyArgument.Type.InnerType().Equals(keyField.Type, TypeComparison.Structural) &&
+            !keyArgument.ContainsIsDirective())
         {
             return true;
         }

diff --git a/...Fusion/test/CommandLine.Tests/__snapshots__/ComposeCommandTests.Compose_Fusion_Graph.snap b/...Fusion/test/CommandLine.Tests/__snapshots__/ComposeCommandTests.Compose_Fusion_Graph.snap
@@ -77,7 +77,7 @@ type SomeData {
   accountValue: String! @source(subgraph: "Accounts")
 }
 
-type User implements Node @variable(subgraph: "Accounts", name: "User_id", select: "id") @resolver(subgraph: "Accounts", select: "{ userById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Accounts", select: "{ usersById(ids: $User_id) }", arguments: [ { name: "User_id", type: "[ID!]!" } ], kind: "BATCH") @resolver(subgraph: "Accounts", select: "{ userById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Accounts", select: "{ node(id: $User_id) { ... on User { ... User } } }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Accounts", select: "{ nodes(ids: $User_id) { ... on User { ... User } } }", arguments: [ { name: "User_id", type: "[ID!]!" } ], kind: "BATCH") {
+type User implements Node @variable(subgraph: "Accounts", name: "User_id", select: "id") @resolver(subgraph: "Accounts", select: "{ userById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Accounts", select: "{ usersById(ids: $User_id) }", arguments: [ { name: "User_id", type: "[ID!]!" } ], kind: "BATCH") {
   birthdate: Date! @source(subgraph: "Accounts")
   id: ID! @source(subgraph: "Accounts")
   name: String! @source(subgraph: "Accounts")

diff --git a/...andLine.Tests/__snapshots__/ComposeCommandTests.Compose_Fusion_Graph_Append_Subgraph.snap b/...andLine.Tests/__snapshots__/ComposeCommandTests.Compose_Fusion_Graph_Append_Subgraph.snap
@@ -125,12 +125,12 @@ type AddUserPayload {
   user: User @source(subgraph: "Accounts")
 }
 
-type Product @variable(subgraph: "Reviews2", name: "Product_id", select: "id") @resolver(subgraph: "Reviews2", select: "{ productById(id: $Product_id) }", arguments: [ { name: "Product_id", type: "ID!" } ]) @resolver(subgraph: "Reviews2", select: "{ productById(id: $Product_id) }", arguments: [ { name: "Product_id", type: "ID!" } ]) {
+type Product @variable(subgraph: "Reviews2", name: "Product_id", select: "id") @resolver(subgraph: "Reviews2", select: "{ productById(id: $Product_id) }", arguments: [ { name: "Product_id", type: "ID!" } ]) {
   id: ID! @source(subgraph: "Reviews2")
   reviews: [Review!]! @source(subgraph: "Reviews2")
 }
 
-type Review implements Node @variable(subgraph: "Reviews2", name: "Review_id", select: "id") @resolver(subgraph: "Reviews2", select: "{ reviewById(id: $Review_id) }", arguments: [ { name: "Review_id", type: "ID!" } ]) @resolver(subgraph: "Reviews2", select: "{ node(id: $Review_id) { ... on Review { ... Review } } }", arguments: [ { name: "Review_id", type: "ID!" } ]) @resolver(subgraph: "Reviews2", select: "{ nodes(ids: $Review_id) { ... on Review { ... Review } } }", arguments: [ { name: "Review_id", type: "[ID!]!" } ], kind: "BATCH") {
+type Review implements Node @variable(subgraph: "Reviews2", name: "Review_id", select: "id") @resolver(subgraph: "Reviews2", select: "{ reviewById(id: $Review_id) }", arguments: [ { name: "Review_id", type: "ID!" } ]) @resolver(subgraph: "Reviews2", select: "{ nodes(ids: $Review_id) { ... on Review { ... Review } } }", arguments: [ { name: "Review_id", type: "[ID!]!" } ], kind: "BATCH") {
   author: User! @source(subgraph: "Reviews2")
   body: String! @source(subgraph: "Reviews2")
   id: ID! @source(subgraph: "Reviews2")
@@ -143,7 +143,7 @@ type SomeData {
 }
 
 "The user who wrote the review."
-type User implements Node @variable(subgraph: "Accounts", name: "User_id", select: "id") @variable(subgraph: "Reviews2", name: "User_id", select: "id") @resolver(subgraph: "Accounts", select: "{ userById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Accounts", select: "{ usersById(ids: $User_id) }", arguments: [ { name: "User_id", type: "[ID!]!" } ], kind: "BATCH") @resolver(subgraph: "Reviews2", select: "{ authorById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Accounts", select: "{ userById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Reviews2", select: "{ authorById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Accounts", select: "{ node(id: $User_id) { ... on User { ... User } } }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Accounts", select: "{ nodes(ids: $User_id) { ... on User { ... User } } }", arguments: [ { name: "User_id", type: "[ID!]!" } ], kind: "BATCH") @resolver(subgraph: "Reviews2", select: "{ node(id: $User_id) { ... on User { ... User } } }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Reviews2", select: "{ nodes(ids: $User_id) { ... on User { ... User } } }", arguments: [ { name: "User_id", type: "[ID!]!" } ], kind: "BATCH") {
+type User implements Node @variable(subgraph: "Accounts", name: "User_id", select: "id") @variable(subgraph: "Reviews2", name: "User_id", select: "id") @resolver(subgraph: "Accounts", select: "{ userById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Accounts", select: "{ usersById(ids: $User_id) }", arguments: [ { name: "User_id", type: "[ID!]!" } ], kind: "BATCH") @resolver(subgraph: "Reviews2", select: "{ authorById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ]) @resolver(subgraph: "Reviews2", select: "{ nodes(ids: $User_id) { ... on User { ... User } } }", arguments: [ { name: "User_id", type: "[ID!]!" } ], kind: "BATCH") {
   birthdate: Date! @source(subgraph: "Accounts")
   id: ID! @source(subgraph: "Accounts") @source(subgraph: "Reviews2")
   name: String! @source(subgraph: "Accounts") @source(subgraph: "Reviews2")

diff --git a/src/HotChocolate/Fusion/test/Composition.Tests/CompositionTestBase.cs b/src/HotChocolate/Fusion/test/Composition.Tests/CompositionTestBase.cs
@@ -9,12 +9,30 @@ public abstract class CompositionTestBase(ITestOutputHelper output)
 {
     private readonly Func<ICompositionLog> _logFactory = () => new TestCompositionLog(output);
 
-    protected async Task Succeed(string schemaA, string schemaB)
+     protected async Task Succeed(string schema, string[]? extensions = null)
     {
         // arrange
         var configA = new SubgraphConfiguration(
             "A",
-            schemaA,
+            schema,
+            extensions ?? Array.Empty<string>(),
+            new[] { new HttpClientConfiguration(new Uri("https://localhost:5001/graphql")) });
+
+        // act
+        var composer = new FusionGraphComposer(logFactory: _logFactory);
+        var fusionConfig = await composer.ComposeAsync(new[] { configA });
+
+        SchemaFormatter
+            .FormatAsString(fusionConfig)
+            .MatchSnapshot(extension: ".graphql");
+    }
+
+    protected async Task Succeed(string schema, string schemaB)
+    {
+        // arrange
+        var configA = new SubgraphConfiguration(
+            "A",
+            schema,
             Array.Empty<string>(),
             new[] { new HttpClientConfiguration(new Uri("https://localhost:5001/graphql")) });
 
@@ -32,7 +50,7 @@ protected async Task Succeed(string schemaA, string schemaB)
             .FormatAsString(fusionConfig)
             .MatchSnapshot(extension: ".graphql");
     }
-    
+
     protected async Task Fail(string schemaA, string schemaB)
     {
         // arrange
@@ -54,12 +72,12 @@ protected async Task Fail(string schemaA, string schemaB)
         await composer.TryComposeAsync(new[] { configA, configB });
 
         var snapshot = new Snapshot();
-        
+
         foreach (var error in log.Errors)
         {
             snapshot.Add(error.Message);
         }
 
         await snapshot.MatchAsync();
     }
-}
+}
diff --git a/src/HotChocolate/Fusion/test/Composition.Tests/DataTypesTests.cs b/src/HotChocolate/Fusion/test/Composition.Tests/DataTypesTests.cs
@@ -1,4 +1,5 @@
 using CookieCrumble;
+using HotChocolate.Fusion.Composition;
 using HotChocolate.Fusion.Shared;
 using HotChocolate.Skimmed.Serialization;
 using Xunit.Abstractions;
@@ -19,7 +20,7 @@ public async Task Compose_Schema_With_Data_Only()
             schema {
               query: Query
             }
-            
+
             type Query {
               someData: SomeData
             }
@@ -41,7 +42,7 @@ type OtherData {
             schema {
               query: Query
             }
-            
+
             type Query {
               someData: SomeData
             }
@@ -65,4 +66,4 @@ type OtherData {
             .FormatAsString(fusionConfig)
             .MatchSnapshot(extension: ".graphql");
     }
-}
+}
diff --git a/src/HotChocolate/Fusion/test/Composition.Tests/ResolverCompositionTests.cs b/src/HotChocolate/Fusion/test/Composition.Tests/ResolverCompositionTests.cs
@@ -0,0 +1,17 @@
+using ChilliCream.Testing;
+using Xunit.Abstractions;
+
+namespace HotChocolate.Fusion.Composition;
+
+public class ResolverCompositionTests(ITestOutputHelper output) : CompositionTestBase(output)
+{
+    [Fact]
+    public async Task Ensure_Node_Resolver_Only_Picked_If_Needed()
+        => await Succeed(FileResource.Open("test1.graphql"));
+
+    [Fact]
+    public async Task Merge_Meta_Data_Correctly()
+        => await Succeed(
+            FileResource.Open("test1.graphql"),
+            new[] { FileResource.Open("test1.extensions.graphql") });
+}
diff --git a/src/HotChocolate/Fusion/test/Composition.Tests/__resources__/test1.extensions.graphql b/src/HotChocolate/Fusion/test/Composition.Tests/__resources__/test1.extensions.graphql
@@ -0,0 +1,3 @@
+extend type Query {
+  userByUsername(username: String! @is(field: "name")): User
+}
diff --git a/src/HotChocolate/Fusion/test/Composition.Tests/__resources__/test1.graphql b/src/HotChocolate/Fusion/test/Composition.Tests/__resources__/test1.graphql
@@ -0,0 +1,38 @@
+interface Node {
+  id: ID!
+}
+
+type PageInfo {
+  hasNextPage: Boolean!
+  hasPreviousPage: Boolean!
+  startCursor: String
+  endCursor: String
+}
+
+type Query {
+  node(id: ID!): Node
+  nodes(ids: [ID!]!): [Node]!
+  userById(id: ID!): User
+  userByUsername(username: String!): User
+  users(first: Int, after: String, last: Int, before: String): UsersConnection
+}
+
+type User implements Node {
+  id: ID!
+  name: String!
+  birthdate: String!
+  username: String!
+}
+
+type UsersConnection {
+  pageInfo: PageInfo!
+  edges: [UsersEdge!]
+  nodes: [User!]
+}
+
+type UsersEdge {
+  cursor: String!
+  node: User!
+}
+
+scalar DateTime
diff --git a/...on/test/Composition.Tests/__snapshots__/DemoIntegrationTests.Accounts_And_Reviews.graphql b/...on/test/Composition.Tests/__snapshots__/DemoIntegrationTests.Accounts_And_Reviews.graphql
@@ -60,7 +60,6 @@ type AddUserPayload {
 
 type Product
   @variable(subgraph: "Reviews", name: "Product_id", select: "id")
-  @resolver(subgraph: "Reviews", select: "{ productById(id: $Product_id) }", arguments: [ { name: "Product_id", type: "ID!" } ])
   @resolver(subgraph: "Reviews", select: "{ productById(id: $Product_id) }", arguments: [ { name: "Product_id", type: "ID!" } ]) {
   id: ID!
     @source(subgraph: "Reviews")
@@ -71,7 +70,6 @@ type Product
 type Review implements Node
   @variable(subgraph: "Reviews", name: "Review_id", select: "id")
   @resolver(subgraph: "Reviews", select: "{ reviewById(id: $Review_id) }", arguments: [ { name: "Review_id", type: "ID!" } ])
-  @resolver(subgraph: "Reviews", select: "{ node(id: $Review_id) { ... on Review { ... Review } } }", arguments: [ { name: "Review_id", type: "ID!" } ])
   @resolver(subgraph: "Reviews", select: "{ nodes(ids: $Review_id) { ... on Review { ... Review } } }", arguments: [ { name: "Review_id", type: "[ID!]!" } ], kind: "BATCH") {
   author: User!
     @source(subgraph: "Reviews")
@@ -95,10 +93,6 @@ type User implements Node
   @resolver(subgraph: "Accounts", select: "{ userById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ])
   @resolver(subgraph: "Accounts", select: "{ usersById(ids: $User_id) }", arguments: [ { name: "User_id", type: "[ID!]!" } ], kind: "BATCH")
   @resolver(subgraph: "Reviews", select: "{ authorById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ])
-  @resolver(subgraph: "Accounts", select: "{ userById(id: $User_id) }", arguments: [ { name: "User_id", type: "ID!" } ])
-  @resolver(subgraph: "Accounts", select: "{ node(id: $User_id) { ... on User { ... User } } }", arguments: [ { name: "User_id", type: "ID!" } ])
-  @resolver(subgraph: "Accounts", select: "{ nodes(ids: $User_id) { ... on User { ... User } } }", arguments: [ { name: "User_id", type: "[ID!]!" } ], kind: "BATCH")
-  @resolver(subgraph: "Reviews", select: "{ node(id: $User_id) { ... on User { ... User } } }", arguments: [ { name: "User_id", type: "ID!" } ])
   @resolver(subgraph: "Reviews", select: "{ nodes(ids: $User_id) { ... on User { ... User } } }", arguments: [ { name: "User_id", type: "[ID!]!" } ], kind: "BATCH") {
   birthdate: Date!
     @source(subgraph: "Accounts")