GraphQLSwift / Graphiti

The Swift GraphQL Schema framework for macOS and Linux
MIT License
526 stars 66 forks source link

Add Federation Support #98

Closed samisuteria closed 1 year ago

samisuteria commented 1 year ago

A basic implementation of federation. Some issues I have and could use guidance:

d-exclaimation commented 1 year ago

How to apply directives to fields/types (ie: @key(fields: "id"), @inaccessible, @shareable )

@NeedleInAJayStack is directive support already in Graphiti?

NeedleInAJayStack commented 1 year ago

How to apply directives to fields/types (ie: @key(fields: "id"), @inaccessible, @shareable )

@NeedleInAJayStack is directive support already in Graphiti?

Not at the moment, but it's deinitely a highly sought-after feature

I should say, just for clarity, that the GraphQL standard directives 'skip' and 'include' are supported, but custom directives like Apollo Federation relies on are not supported today.

samisuteria commented 1 year ago

Not at the moment, but it's deinitely a highly sought-after feature

I can work on that feature - would you prefer directive support before merging in federation?

NeedleInAJayStack commented 1 year ago

Awesome, thank you so much @samisuteria! Yeah, from my understanding federation support typically uses custom directives, so solving custom directives in a general way first makes sense to me. Thanks again, and great work so far!

samisuteria commented 1 year ago

Federation can work without directives in the application code. If you are writing schema first (I'm not sure the split between schema-first and code-first in the GraphQL ecosystem) you can enable federation without directives. As long as the service sdl query returns the schema federation will work correctly. Even Apollo's reference server avoids using directives in the code and just loads the schema from the file system here.

query {
   _service { sdl }
}

If you check out the samisuteria/graphiti branch on https://github.com/samisuteria/apollo-federation-subgraph-compatibility/ and run make test subgraph=swift-graphiti you'll see it passes all tests except federated tracing (which seems like a much larger project based on: https://www.apollographql.com/docs/graphos/metrics/sending-operation-metrics#tracing-format) and it appears about half the subgraph libraries have warning levels of support for it https://www.apollographql.com/docs/federation/building-supergraphs/supported-subgraphs .

*************
Federation v1 compatibility
*************
_service PASS
@key (single) PASS
@key (multi) PASS
@key (composite) PASS
repeatable @key PASS
@requires PASS
@provides PASS
federated tracing WARNING

*************
Federation v2 compatibility
*************
@link PASS
@shareable PASS
@tag PASS
@override PASS
@inaccessible PASS

I still plan on working on adding directive/sdl support for Graphiti but I think this PR is ready for review and can be used by people who are schema-first.

d-exclaimation commented 1 year ago

I still plan on working on adding directive/sdl support for Graphiti but I think this PR is ready for review and can be used by people who are schema-first.

Sounds good for me. Nice work 👍 . While It's probably a bit awkward atm given you can't really fully do schema-first or use directive with Graphiti, those feature can be separated and worked on later with future PR / releases.

NeedleInAJayStack commented 1 year ago

Great info @samisuteria! Thanks for walking me through that, and in that case I'm totally ok with this going in before directive support.

Sorry about the delay - I'll get a review in this weekend.

NeedleInAJayStack commented 1 year ago

@samisuteria See what you think of this twist on your implementation: https://github.com/NeedleInAJayStack/Graphiti/tree/federation_jay

If you're okay with it, we can pull those commits into this MR.

samisuteria commented 1 year ago

@NeedleInAJayStack I really like the changes on your branch and the Key implementation / mapping to the Resolver argument types is a much better way to implement federation.

The syntax for defining keys in the schema seems a bit complicated (anytime you have multiple builder closures in a function).

What do you think of something like this? It would just require keys: [KeyComponent<ObjectType, Resolver, Context>] to be mutable.

Type(Product.self) {
    Field("id", at: \.id)
    Field("sku", at: \.sku)
    Field("package", at: \.package)
    Field("variation", at: \.variation)
    Field("dimensions", at: \.dimensions)
    Field("createdBy", at: \.createdBy)
    Field("notes", at: \.notes)
    Field("research", at: \.research)
}
.key(at: ProductResolver.getProduct1) {
    Argument("id", at: \.id)
}
.key(at: ProductResolver.getProduct2) {
    Argument("sku", at: \.sku)
    Argument("package", at: \.package)
}
.key(at: ProductResolver.getProduct3) {
    Argument("sku", at: \.sku)
    Argument("variation", at: \.variation)
}

Eventually when the library can generate the SDL itself, I imagine other directives would work the same way and the syntax would be similar to SwiftUI's ViewModifier

NeedleInAJayStack commented 1 year ago

@samisuteria Ooo, I love that idea. You're right, it looks cleaner and better preserves the public API

NeedleInAJayStack commented 1 year ago

@samisuteria I just implemented your suggestion on my branch. If you like it, feel free to pull those commits into this MR. Thanks again!

samisuteria commented 1 year ago

Just pulled in the changes - thank you so much for the help on this :)

samisuteria commented 1 year ago

This is a useful dockerfile if you're on Apple Silicon and want to test older versions of Swift:

FROM swift:5.6-focal as swift5.6

WORKDIR /build

# Cache dependencies
COPY ./Package.* ./
RUN swift package resolve

COPY . .
RUN swift test --parallel

FROM swiftarm/swift:5.5.3-debian-buster as swift5.5

WORKDIR /build

# Cache dependencies
COPY ./Package.* ./
RUN swift package resolve

COPY . .
RUN swift test --parallel

FROM swiftarm/swift:5.4.3-focal-multi-arch as swift5.4

WORKDIR /build

# Cache dependencies
COPY ./Package.* ./
RUN swift package resolve

COPY . .
RUN swift test --parallel

FROM swift:latest

WORKDIR /build

# Make sure all stages actually run
COPY --from=swift5.6 ./build ./build-5.6
COPY --from=swift5.5 ./build ./build-5.5
COPY --from=swift5.4 ./build ./build-5.4