Code-First Federation 2 support #3144
Conversation
|
At first glance I like the look of it! |
|
Just popping in for a moment. This does look pretty interesting. A reminder that there are already some federation types that look almost exactly the same as these used with schema-first. Probably want to figure out some reuse there. https://github.com/graphql-dotnet/graphql-dotnet/tree/master/src/GraphQL/Utilities/Federation |
I think I re-used what I can (only
Also, I'm not sure what's the story with schema-first approach in general. Is it something graphql-dotnet actively supports? |
|
My point was - I think it would be fairly awkward to have two different implementations of federation in the same project. If we're upgrading federation support to add GraphType First support, it should be upgraded everywhere and not work differently when using Schema First vs. GraphType First.
I certainly hope so since I actively use it. 😉 Schema First at it's core uses the same GraphQL Framework. It is just a light schema builder on top of the core engine. From that regard, it is basically the same as the |
|
@joemcbride Let me first look into supporting schema-first in my implementation and then we can decide on either merging or deprecating the old one completely. |
|
@joemcbride do you know why I see this changes when running tests locally but in CI they are not present? |
It's because the latest version of Visual Studio was released a couple days ago. We need to merge this PR (and you need to be running the latest version of VS also, which presumably you are which is why you are having the issue): I'll merge it now; then merge |
|
Oh, ok. I rolled back these lines for now. I'll re-run tests after rebasing. |
|
I also see these changes which also seems to be VS Code version related: |
I can't help there; I never use VS Code. As far as I know, @sungam3r does not either; but perhaps @joemcbride does. |
Keep in mind that not all of |
Codecov Report
@@ Coverage Diff @@
## master #3144 +/- ##
==========================================
+ Coverage 84.43% 84.49% +0.06%
==========================================
Files 371 391 +20
Lines 16087 16541 +454
Branches 2599 2638 +39
==========================================
+ Hits 13583 13977 +394
- Misses 1882 1915 +33
- Partials 622 649 +27
Continue to review full report at Codecov.
|
Mithras
force-pushed
the
master
branch
3 times, most recently
from
8ed7f5b
to
e762183
Compare
|
I've added schema-first Federation 2 support. Can you please review and let me know if it's ok to remove existing Federation classes? |
|
bump |
|
Hi, |
|
If @sungam3r does not have time soon (it sounds like he's got a lot on his plate), then maybe I can review it. We should try to get this in v8. |
|
Definetly in v8. |
|
BTW just an FYI, I recently externalized the Federation Compatibility Tests -> you can now run them locally using NPX (https://www.npmjs.com/package/@apollo/federation-subgraph-compatibility) or through Github Actions (https://github.com/marketplace/actions/apollo-federation-subgraph-compatibility). Example GH action integration (workflow is invoked on the PRs) -> https://github.com/apollographql/federation-jvm/blob/master/.github/workflows/federation-compatibility.yaml#L35 |
Feels pretty good actually. Omnisharp works out of the box which has never happened with previous .NET versions.
As I have said, I've added new implementation for Feraration 2 with DataLoader, CodeFirst and SchemaFirst support. I intentionally haven't touched old implementation as there is pretty much zero overlap. Old implementation can be removed in like 10 minutes.
I let
Well I'm not sure how it's possible to use existing implementation which doesn't work with DataLoaders for anything more than Hello World. Anyway, I moved everything to |
done. |
|
@Mithras thanks, I remember about your work and this one in my todo list just after OpenTelemetry PR. |
|
If @sungam3r doesn't have time to merge it for v8, I’ll try to do so. However, v8 isn’t planned to be released anytime soon. |
|
Is there anything I can do to help move this PR along? I'm happy to spend some cycles completing out any last items ❤️ |
|
@michael-watson @Mithras Yes, I'm ready to help get this merged for v8. Recently in #3660 we have added CI tests for schema-first federation. We need similar projects added to this PR so that there is CI tests for code-first federation. We also need all new work to have XML comments. And then we need to review the level of support for Federation v2. (ok if not 100%, but we need it documented where we are at) Finally, we need approval from @joemcbride to add GraphQL.Federation nuget package. Then ready to review and merge. |
|
There is an integration test that tests most of everything. Anyway, I'm on vacation and probably won't even have internet for 1.5 weeks |
|
No problem @Mithras The purpose is to have end to end tests plus have the code double as a sample project. So the new schema-first tests demonstrate two fully functional GraphQL subgraphs created in GraphQL.NET, and the tests start hosting them at two endpoints and have Apollo Federation create a super graph by querying those endpoints, and then start Apollo Router, followed by actually running a query that pulls data from both subgraphs and verifying the result. I’d like to do the same for code-first federation. Sorry I’ve not looked this PR recently to see what tests exist. |
The federation tests doesn't use the NUGET_AUTH_TOKEN -- it only uses the GITHUB_TOKEN and only to pull the repository. See https://github.com/graphql-dotnet/graphql-dotnet/blob/master/.github/workflows/test-code.yml and scroll down to the
All the rest of the tests you can run from Visual Studio. |
|
I'd suggest just writing a powershell script to perform the same actions, or something like that. Here's ChatGPT's rough draft: # Change to src directory
Set-Location -Path .\src
# Install dependencies for Sample1
dotnet restore .\GraphQL.Federation.SchemaFirst.Sample1
# Build Sample1 and Sample2
dotnet build --no-restore .\GraphQL.Federation.SchemaFirst.Sample1
dotnet build .\GraphQL.Federation.SchemaFirst.Sample2
# Start Sample1 and Sample2
Start-Process dotnet -ArgumentList "run --no-build --project GraphQL.Federation.SchemaFirst.Sample1"
Start-Process dotnet -ArgumentList "run --no-build --project GraphQL.Federation.SchemaFirst.Sample2"
# Install Apollo Rover CLI
Invoke-RestMethod -Uri 'https://rover.apollo.dev/nix/latest' | Out-File -FilePath '.\rover_install.sh'
Start-Process "sh" -ArgumentList ".\rover_install.sh"
# Install jq
Start-Process "sudo" -ArgumentList "apt-get install -y jq"
# Wait for Sample1 and Sample2 to spin up
Function WaitForService ($url) {
For ($i=1; $i -le 60; $i++) {
Write-Host "Request $i to the URL..."
$response = Invoke-RestMethod -Uri $url -Method Get -StatusCodeVariable statusCode
if ($statusCode -eq 200) {
Write-Host "Received 200 response, step completed."
return
}
Write-Host "Did not receive a 200 response, sleeping for 0.5 second..."
Start-Sleep -Milliseconds 500
}
Write-Host "Timed out after 30 seconds, step failed."
Exit 1
}
WaitForService 'http://localhost:5601'
WaitForService 'http://localhost:5602'
# Change to src/Federation directory
Set-Location -Path .\Federation
# Build supergraph
& "$HOME/.rover/bin/rover" supergraph compose --config .\federation-$env:FEDERATION_VERSION-supergraph-config.yaml --elv2-license=accept | Out-File -FilePath '.\supergraph.graphql'
# Print supergraph
Get-Content -Path .\supergraph.graphql
# Download and Start router
Invoke-RestMethod -Uri 'https://router.apollo.dev/download/nix/latest' | Out-File -FilePath '.\router_install.sh'
Start-Process "sh" -ArgumentList ".\router_install.sh"
Start-Process .\router -ArgumentList "--dev --supergraph .\supergraph.graphql"
# Wait for router to spin up
WaitForService 'http://localhost:8088/health'
# Run GraphQL queries
Invoke-RestMethod -Uri 'http://127.0.0.1:4000' -Method Post -Headers @{"Content-Type" = "application/json"} -InFile '.\federation-request-1.json' | Out-File -FilePath '.\response-1.json'
Invoke-RestMethod -Uri 'http://127.0.0.1:4000' -Method Post -Headers @{"Content-Type" = "application/json"} -InFile '.\federation-request-2.json' | Out-File -FilePath '.\response-2.json'
Invoke-RestMethod -Uri 'http://127.0.0.1:4000' -Method Post -Headers @{"Content-Type" = "application/json"} -InFile '.\federation-request-3.json' | Out-File -FilePath '.\response-3.json'
# Function to compare JSON files
Function CompareJsonFiles ($actualFile, $expectedFile) {
$actualContent = Get-Content -Path $actualFile | ConvertFrom-Json | ConvertTo-Json
$expectedContent = Get-Content -Path $expectedFile | ConvertFrom-Json | ConvertTo-Json
if ($actualContent -eq $expectedContent) {
Write-Host "$actualFile and $expectedFile are identical."
} else {
Write-Host "$actualFile and $expectedFile differ."
Exit 1
}
}
# Compare query result 1 to expected response
$actualResponse1 = "actual-response-1.json"
$expectedResponse1 = "expected-response-1.json"
Get-Content -Path "response-1.json" | Out-File -FilePath $actualResponse1
Get-Content -Path "federation-response-1.json" | Out-File -FilePath $expectedResponse1
CompareJsonFiles $actualResponse1 $expectedResponse1
# Compare query result 2 to expected response
$actualResponse2 = "actual-response-2.json"
$expectedResponse2 = "expected-response-2.json"
Get-Content -Path "response-2.json" | Out-File -FilePath $actualResponse2
Get-Content -Path "federation-response-2.json" | Out-File -FilePath $expectedResponse2
CompareJsonFiles $actualResponse2 $expectedResponse2
# Compare query result 3 to expected response
$actualResponse3 = "actual-response-3.json"
$expectedResponse3 = "expected-response-3.json"
Get-Content -Path "response-3.json" | Out-File -FilePath $actualResponse3
Get-Content -Path "federation-response-3.json" | Out-File -FilePath $expectedResponse3
CompareJsonFiles $actualResponse3 $expectedResponse3Obviously you need changes for running on Windows, etc, even if it didn't make any mistakes. |
|
If you devise a good powershell script, add some comments that explain how to use it, post a PR with the file, and I'll merge it in. |
|
@michael-watson By the way, I'm okay with having a separate NuGet package for federation, or having it in the main GraphQL NuGet package. |
Subj.
Some details on what's in the PR:
3.1.
Attributes.*- All Federation 2 directives as attributes.3.2 Extensions.FederationFieldExtensions - All Federation 2 directives as
FieldType/FieldBuilderextensions.3.3 Extensions.FederationGraphTypeExtensions - All Federation 2 directives as
GraphTypeextensions. Also,.AddServices()and.AddEntities()to add Federation fields explicitly/manually (e.g. when you want to specify.AuthorizeWith()on them).3.4 Extensions.FederationGraphQLBuilderExtensions -
AddFederation()extension. See src/GraphQL.Federation.Tests/ServiceProviderFixture.cs.3.5
Schema.*- A few output types to support Federation.3.6. A bunch of
internalclasses.There are a few questionable hacks:
EntityTypewhich I do in FederationEntitiesSchemaNodeVisitor._serviceand_entitiesin a bit of a weird way (injects GraphType instances in ctor) because it doesn't want to work otherwise. Probably something about order of when schema initializes and visitors are called.graphql-dotnetby default but I'm not aware of any other ways of mapping Graph <-> CLR types. It's used by FederationQuerySchemaNodeVisitor.ResolveEntities().Hope this won't get stuck for years like #1669 did.