Skip to content

Commit

Permalink
[Fusion] Correctly handle null items in ResolveByKey (#7217)
Browse files Browse the repository at this point in the history
  • Loading branch information
tobias-tengler authored Jul 3, 2024
1 parent 3ce8a28 commit 27bcec2
Show file tree
Hide file tree
Showing 5 changed files with 270 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ private Dictionary<string, JsonElement> UnwrapResult(

foreach (var element in data.EnumerateArray())
{
if (element.TryGetProperty(key, out var keyValue))
if (element.ValueKind is not JsonValueKind.Null &&
element.TryGetProperty(key, out var keyValue))
{
result.TryAdd(FormatKeyValue(keyValue), element);
}
Expand Down
53 changes: 53 additions & 0 deletions src/HotChocolate/Fusion/test/Core.Tests/DemoIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1861,6 +1861,59 @@ query TopProducts {
Assert.Null(result.ExpectQueryResult().Errors);
}

[Fact]
public async Task ResolveByKey_Handles_Null_Item_Correctly()
{
// arrange
using var demoProject = await DemoProject.CreateAsync();

// act
var fusionGraph = await new FusionGraphComposer(logFactory: _logFactory).ComposeAsync(
new[]
{
demoProject.Products.ToConfiguration(),
demoProject.Resale.ToConfiguration(),
}, new FusionFeatureCollection(FusionFeatures.NodeField));

var executor = await new ServiceCollection()
.AddSingleton(demoProject.HttpClientFactory)
.AddSingleton(demoProject.WebSocketConnectionFactory)
.AddFusionGatewayServer()
.ConfigureFromDocument(SchemaFormatter.FormatAsDocument(fusionGraph))
.BuildRequestExecutorAsync();

var request = Parse(
"""
{
viewer {
# The second product does not exist in the products subgraph
recommendedResalableProducts {
edges {
node {
product? {
id
name
}
}
}
}
}
}
""");

// act
await using var result = await executor.ExecuteAsync(
OperationRequestBuilder
.New()
.SetDocument(request)
.Build());

// assert
var snapshot = new Snapshot();
CollectSnapshotData(snapshot, request, result);
await snapshot.MatchMarkdownAsync();
}

public sealed class HotReloadConfiguration : IObservable<GatewayConfiguration>
{
private GatewayConfiguration _configuration;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
# ResolveByKey_Handles_Null_Item_Correctly

## Result

```json
{
"errors": [
{
"message": "Cannot return null for non-nullable field.",
"locations": [
{
"line": 9,
"column": 13
}
],
"path": [
"viewer",
"recommendedResalableProducts",
"edges",
1,
"node",
"product",
"name"
],
"extensions": {
"code": "HC0018"
}
}
],
"data": {
"viewer": {
"recommendedResalableProducts": {
"edges": [
{
"node": {
"product": {
"id": "UHJvZHVjdDox",
"name": "Table"
}
}
},
{
"node": {
"product": null
}
},
{
"node": {
"product": {
"id": "UHJvZHVjdDoz",
"name": "Chair"
}
}
}
]
}
}
}
}
```

## Request

```graphql
{
viewer {
recommendedResalableProducts {
edges {
node {
product? {
id
name
}
}
}
}
}
}
```

## QueryPlan Hash

```text
143888B7D930092D26863B1CCBEFA6A7A06739E8
```

## QueryPlan

```json
{
"document": "{ viewer { recommendedResalableProducts { edges { node { product? { id name } } } } } }",
"rootNode": {
"type": "Sequence",
"nodes": [
{
"type": "Resolve",
"subgraph": "Resale",
"document": "query fetch_viewer_1 { viewer { recommendedResalableProducts { edges { node { product? { id __fusion_exports__1: id } } } } } }",
"selectionSetId": 0,
"provides": [
{
"variable": "__fusion_exports__1"
}
]
},
{
"type": "Compose",
"selectionSetIds": [
0
]
},
{
"type": "ResolveByKeyBatch",
"subgraph": "Products",
"document": "query fetch_viewer_2($__fusion_exports__1: [ID!]!) { nodes(ids: $__fusion_exports__1) { ... on Product { name __fusion_exports__1: id } } }",
"selectionSetId": 5,
"path": [
"nodes"
],
"requires": [
{
"variable": "__fusion_exports__1"
}
]
},
{
"type": "Compose",
"selectionSetIds": [
5
]
}
]
},
"state": {
"__fusion_exports__1": "Product_id"
}
}
```

44 changes: 44 additions & 0 deletions src/HotChocolate/Fusion/test/Shared/DemoProject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using HotChocolate.Fusion.Shared.Shipping;
using HotChocolate.Fusion.Shared.Books;
using HotChocolate.Fusion.Shared.Authors;
using HotChocolate.Fusion.Shared.Resale;
using HotChocolate.Transport.Http;
using HotChocolate.Types.Descriptors;
using HotChocolate.Utilities.Introspection;
Expand All @@ -33,6 +34,7 @@ private DemoProject(
DemoSubgraph patient1,
DemoSubgraph books,
DemoSubgraph authors,
DemoSubgraph resale,
IHttpClientFactory clientFactory,
IWebSocketConnectionFactory webSocketConnectionFactory)
{
Expand All @@ -46,6 +48,7 @@ private DemoProject(
Patient1 = patient1;
Books = books;
Authors = authors;
Resale = resale;
HttpClientFactory = clientFactory;
WebSocketConnectionFactory = webSocketConnectionFactory;
}
Expand All @@ -72,6 +75,8 @@ private DemoProject(

public DemoSubgraph Authors { get; }

public DemoSubgraph Resale { get; }

public static async Task<DemoProject> CreateAsync(CancellationToken ct = default)
{
var disposables = new List<IDisposable>();
Expand Down Expand Up @@ -257,6 +262,26 @@ public static async Task<DemoProject> CreateAsync(CancellationToken ct = default
.IntrospectServerAsync(authorsClient, ct)
.ConfigureAwait(false);

var resale = testServerFactory.Create(
s => s
.AddRouting()
.AddGraphQLServer(disableCostAnalyzer: true)
.AddQueryType<ResaleQuery>()
.AddGlobalObjectIdentification()
.AddMutationConventions()
.AddUploadType()
.AddConvention<INamingConventions>(_ => new DefaultNamingConventions()),
c => c
.UseRouting()
.UseEndpoints(endpoints => endpoints.MapGraphQL()));
disposables.Add(resale);

var resaleClient = resale.CreateClient();
resaleClient.BaseAddress = new Uri("http://localhost:5000/graphql");
var resaleSchema = await IntrospectionClient
.IntrospectServerAsync(resaleClient, ct)
.ConfigureAwait(false);

var httpClients = new Dictionary<string, Func<HttpClient>>
{
{
Expand Down Expand Up @@ -349,6 +374,16 @@ public static async Task<DemoProject> CreateAsync(CancellationToken ct = default
return httpClient;
}
},
{
"Resale", () =>
{
// ReSharper disable once AccessToDisposedClosure
var httpClient = resale.CreateClient();
httpClient.BaseAddress = new Uri("http://localhost:5000/graphql");
httpClient.DefaultRequestHeaders.AddGraphQLPreflight();
return httpClient;
}
},
};

var webSocketClients = new Dictionary<string, Func<IWebSocketConnection>>
Expand Down Expand Up @@ -380,6 +415,9 @@ public static async Task<DemoProject> CreateAsync(CancellationToken ct = default
{
"Authors", () => new MockWebSocketConnection(authors.CreateWebSocketClient())
},
{
"Resale", () => new MockWebSocketConnection(resale.CreateWebSocketClient())
},
};

return new DemoProject(
Expand Down Expand Up @@ -438,6 +476,12 @@ public static async Task<DemoProject> CreateAsync(CancellationToken ct = default
new Uri("ws://localhost:5000/graphql"),
authorsSchema,
authors),
new DemoSubgraph(
"Resale",
resaleClient.BaseAddress,
new Uri("ws://localhost:5000/graphql"),
resaleSchema,
resale),
new MockHttpClientFactory(httpClients),
new MockWebSocketConnectionFactory(webSocketClients));
}
Expand Down
32 changes: 32 additions & 0 deletions src/HotChocolate/Fusion/test/Shared/Resale/ResaleQuery.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using HotChocolate.Types;
using HotChocolate.Types.Relay;

namespace HotChocolate.Fusion.Shared.Resale;

[GraphQLName("Query")]
public class ResaleQuery
{
public Viewer GetViewer() => new();

[NodeResolver]
public Product? GetProductById([ID] int id) => new(id);
}

public class Viewer
{
[UsePaging]
public List<RecommendedResalableProduct> GetRecommendedResalableProducts()
{
return new()
{
new RecommendedResalableProduct(new Product(1)),
// This product doesn't exist on the products subgraph
new RecommendedResalableProduct(new Product(5)),
new RecommendedResalableProduct(new Product(3))
};
}
}

public record RecommendedResalableProduct(Product Product);

public record Product([property: ID] int Id);

0 comments on commit 27bcec2

Please sign in to comment.