-
Notifications
You must be signed in to change notification settings - Fork 299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Integrate Spring Graphql with DGS Framework #1766
Conversation
Uncomment pagination data fetcher. Clean up code, define @connection directive in schema. Complete the future for GraphQL context. format Fix dataloader context Implemented remaining query executor methods First step implementing a Spring GraphQL based QueryExecutor Use runtimewiringConfigurer to pick up both spring-graphql and dgs runtimewiring. Fix lint errors. Configure schema path properly and uncomment some fetchers for use with @RequestHeader etc. Fix data loader registry calls and add argument handlers for request/cookie etc. Remove unsued sample app. Update starter dependencies. Update deps. Update dep lock.
…to be processed as DgsTypeDefinitionRegistry and conditionally enable it based on DgsPagination module.
…1779) * Inject schema resources from spring-graphql into DgsSchemaProvider. * Minor fixes.
Thanks for all the work so far! In addition to the excellent background, I would like to add what I am most excited about is unifying the community. Not only for contributions in the way of code, but even more importantly in terms of reporting issues, providing feedback and ideas. It's what drives an open source project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial feedback below, by no means an exhaustive review, but there should be plenty for discussion and an iteration.
import java.util.function.BiFunction | ||
import java.util.function.Consumer | ||
|
||
class DgsGraphQLSourceBuilder(private val dgsSchemaProvider: DgsSchemaProvider, private val reloadSchemaIndicator: DefaultDgsQueryExecutor.ReloadSchemaIndicator) : SchemaResourceBuilder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GraphQlSource.Builder
hierarchy has 2 alternatives. GraphQlSource.Builder
itself exposes options independent of how the GraphQLSchema
is created. SchemaResourceBuilder
on the other hand is for builders that create the GraphQLSchema
by parsing schema files, and building a RuntimeWiring
. On the implementation side, AbstractGraphQlSourceBuilder
has the common parts, while DefaultSchemaResourceGraphQlSourceBuilder
implements SchemaResouceBuilder
and prepares GraphQLSchema
from those inputs, while alternative builders can skip that part and there is an ExternalSchemaGraphQlSourceBuilder
.
This was originally created for SDL vs programmatic API in GraphQL Java, so if anyone wants to build GraphQLSchema
programmatically they can extend AbstractGraphQlSourceBuilder
. The same applies to DGS as well I think, and even though it also parses schema files and does things the SDL way, it has its own way of building the GraphQLSchema
so it makes sense to extend AbstractGraphQlSourceBuilder
and override the initGraphQlSchema
method to call dgsSchemaProvider.schema()
. So that will become a very trivial builder and it won't have to repeat anything from AbstractGraphQlSourceBuilder
as it currently does.
Likewise, I don't think DgsGraphQLSourceBuilder
should have to be an implementation of SchemaResourceBuilder
, but rather it only needs to be an AbstractGraphQlSourceBuilder
which implements the base GraphQlSource.Builder.
As it is currently, the builder doesn't have anything meaningful it can do with the extra options it gets from implementing the SchemaResourceBuilder
subtype.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comments. What you suggest makes sense to us. Looking at the latest sources, AbstractGraphQlSourceBuilder is not public. Could you update that so we can use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I think we started out with just one package private builder before the hierarchy evolved. It does make sense for the base class to be public as it's more broadly useful, and I've made that change spring-projects/spring-graphql@5fcf803.
} | ||
|
||
override fun build(): GraphQlSource { | ||
return ReloadableGraphQLSource(reload(), this, reloadSchemaIndicator) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One specific thing, if you are to extend AbstractGraphQlSourceBuilder
instead, obviously that creates the very simple FixedGraphQlSource
while what you want is one that resolves at runtime, possibly by reloading. This can be achieved by changing ReloadableGraphQLSource
a little to be created with a GraphQlSource.Builder
rather than with a GraphQlSource
. That way it can call build again whenever it wants.
This should be a fairly small refactoring that will keep the builder focused on building a single instance, and ReloadableGraphQlSource
responsible for all that has to do with reloading. We could also consider adding a protected method to create the GraphQlSource
instance if needed.
} | ||
|
||
override fun inspectSchemaMappings(reportConsumer: Consumer<SchemaReport>): SchemaResourceBuilder { | ||
this.schemaReportConsumer = reportConsumer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SchemaReport is also worth a mention. What's required to run it is a GraphQLSchema
and a RuntimeWiring
and it's currently run from DefaultSchemaResourceGraphQlSourceBuilder
, and since you are replacing that, you'll need to invoke it from the DgsGraphQlSourceBuilder
.
As part of spring-projects/spring-graphql#878, I've added a protected method on AbstractGraphQlSourceBuilder
that lets you hook in at the point where GraphQL.Builder
is customized and that is where we invoke the schema report if the application has explicitly provided a SchemaReport
consumer.
If you look at the commit attached to that issue, you can see how it's done. DGS can do something similar but would need to obtain the resulting RuntimeWiring
from DgsSchemaProvider
somehow. Mainly what we need from the RuntimeWiring
is the map of DataFetchers's if it helps to know that.
…amework into spring-graphql-refactored
…ctored # Conflicts: # graphql-dgs-example-java/dependencies.lock # graphql-dgs-reactive/dependencies.lock # graphql-dgs-spring-boot-micrometer/dependencies.lock # graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DgsSchemaProviderTest.kt
…amework into spring-graphql-refactored
…spring-graphql-refactored
…-SNAPSHOT of spring-graphql.
…ctored # Conflicts: # dependencies.lock # graphql-dgs-client/dependencies.lock # graphql-dgs-example-java-webflux/dependencies.lock # graphql-dgs-example-java/dependencies.lock # graphql-dgs-example-shared/dependencies.lock # graphql-dgs-extended-scalars/dependencies.lock # graphql-dgs-extended-validation/dependencies.lock # graphql-dgs-mocking/dependencies.lock # graphql-dgs-pagination/dependencies.lock # graphql-dgs-platform-dependencies/dependencies.lock # graphql-dgs-platform/dependencies.lock # graphql-dgs-reactive/dependencies.lock # graphql-dgs-spring-boot-micrometer/dependencies.lock # graphql-dgs-spring-boot-oss-autoconfigure/dependencies.lock # graphql-dgs-spring-boot-starter/dependencies.lock # graphql-dgs-spring-webflux-autoconfigure/dependencies.lock # graphql-dgs-spring-webmvc-autoconfigure/dependencies.lock # graphql-dgs-spring-webmvc/dependencies.lock # graphql-dgs-subscription-types/dependencies.lock # graphql-dgs-subscriptions-graphql-sse-autoconfigure/dependencies.lock # graphql-dgs-subscriptions-graphql-sse/dependencies.lock # graphql-dgs-subscriptions-sse-autoconfigure/dependencies.lock # graphql-dgs-subscriptions-sse/dependencies.lock # graphql-dgs-subscriptions-websockets-autoconfigure/dependencies.lock # graphql-dgs-subscriptions-websockets/dependencies.lock # graphql-dgs-webflux-starter/dependencies.lock # graphql-dgs/dependencies.lock # graphql-error-types/dependencies.lock
…spring-graphql-refactored
The DGS and Spring-GraphQL teams are super excited to introduce deep integration between the DGS framework and Spring-GraphQL. This will bring the community together, and we can continue building the best possible GraphQL framework for Spring Boot in the future.
Getting Started with DGS/Spring-GraphQL
Continue reading to learn how we got here, and why it's so exciting, but if you want to get started, here is how.
You can opt-in to use DGS/Spring-GraphQL by replacing the starter dependency.
Replace
implementation "com.netflix.graphql.dgs:graphql-dgs-spring-boot-starter"
withimplementation "com.netflix.graphql.dgs:graphql-dgs-spring-graphql-starter"
.You also need to add either Spring WebMVC or Spring WebFlux explicitly.
With this integration, it is technically possible to mix and match the DGS/Spring-GraphQL programming models. However, to maintain consistency in your codebase and to take full advantage of DGS features, we recommend sticking with the DGS programming model. Additional features from spring-graphql will be available via existing spring-graphql extensions, such as multipart-spring-graphql and the Subscription Callback mechanism in the JVM Federation library.
Background - Two competing frameworks
The DGS Framework provides Java developers with a programming model on top of Spring Boot to create GraphQL services. Netflix open-sourced the DGS framework in 2021, and has been the widely adopted GraphQL Java framework by many companies.
While we continued adoption and development on the DGS Framework, the team at Spring were also exploring GraphQL support for Spring Boot. This effort was already underway prior to open-sourcing the DGS framework, while the Spring GraphQL efforts were also yet to be announced. After open sourcing the framework, both teams convened to discuss the current state and future direction. At the time Spring GraphQL was not as feature rich in comparison to the DGS Framework and we both decided to continue what we were doing. While both the frameworks started with different goals and approaches, over the past year, Spring-GraphQL has matured and reached feature parity with many aspects of the DGS framework. This resulted in two "competing" frameworks in the community that largely solved the same problem.
Today, new users must choose between one or the other, raising questions about which framework to adopt. It might also mean missing out on features available in one framework but not the other. This is not an ideal situation for the Java/GraphQL community.
For the maintainers of both frameworks, it would be much more efficient to collaborate on features and improvements instead of having to solve the same problem twice. Notably, bringing the communities together is a highly desirable goal!
Why not just EOL one of the frameworks?
The DGS framework is widely used and plays a vital role in the architecture of many companies, including Netflix. Moving away from the framework in favor of Spring-GraphQL would be a costly migration without any real benefits.
Although Spring-GraphQL doesn't have the user base of DGS, it makes sense from a Spring Framework perspective to have an out-of-the-box GraphQL offering, just like Spring supports REST.
The way forward
@srinivasankavitha and @paulbakker from the DGS team have worked together with Rossen Stoyanchev and Brian Clozel from the Spring-GraphQL team on integration between the frameworks. Special thanks to @kilink for being our first early adopter and contributing many bug fixes and performance improvements as part of this effort.
With this integration, you can pull in additional features from Spring GraphQL. We also eliminate the need for part of the DGS code that integrates the framework with Spring MVC/WebFlux, since there isn't much benefit to duplicating low level functionality.
For the near term, the DGS/Spring-Graphql integration will be available as an opt-in feature via a different spring-graphql flavor of the DGS starter. We plan to make this the default mode in the long term after we see some level of successful adoption.
Technical implementation
Both DGS and Spring-GraphQL are designed with modularity and extensibility in mind. This makes it feasible to integrate the two frameworks. The following diagrams show how the frameworks are integrated at a high level.
Today, the DGS framework looks as follows.
A user uses the DGS programming model, such as
@DgsComponent
and@DgsQuery
to define data fetchers etc.A GraphQL query comes in through either WebMVC or WebFlux, and is executed by the
DgsQueryExecutor
, which builds on the graphql-java library to execute the query.The
DgsQueryExecutor
is also used directly when writing tests.With the Spring-GraphQL integration, a user can write code with both the DGS programming model and/or the Spring-GraphQL programming model. A GraphQL query comes in through WebMVC/WebFlux/RSocket, which Spring-GraphQL now handles. The representation of the schema (a
GraphQLSchema
from graphql-java) is created by the DGS framework, and used by both the DGS and Spring-GraphQL components. Spring-GraphQL'sExecutionGraphQLService
now handles the actual query execution, while the DGSQueryExecutor
becomes a proxy on top ofExecutionGraphQLService
so that existing test code continues to work.Required Changes
The good news is that the new integration has been mostly a drop-in replacement, not requiring any major code changes for the user. If you use file uploads, you will need to explicitly add a new dependency multipart-spring-graphql.
Known Gaps
At this time, we are lacking support for SSE based subscriptions. This is on the roadmap and will be made available depending on support in spring-graphql.
Performance
At Netflix, we tested the DGS/Spring-GraphQL integration on some of our largest services. Surprisingly, we uncovered a few performance issues in Spring WebMVC/Spring GraphQL with this integration. The Spring team has quickly addressed these issues, and the performance is now even better compared to baseline performance of Netflix applications with just the regular DGS Framework.
Configuration
There is some overlap between configuration properties for DGS and Spring-GraphQL. Where properties overlap, we use the DGS property for the best backward compatibility. The following list is the overlapping properties.
dgs.graphql.schema-locations
spring.graphql.schema.locations
dgs.graphql.schema-locations
spring.graphql.schema.fileExtensions
dgs.graphql.schema-locations
includes the pathdgs.graphql.graphiql.enabled
spring.graphql.graphiql.enabled
dgs.graphql.graphiql.enabled
dgs.graphql.graphiql.path
spring.graphql.graphiql.path
dgs.graphql.graphiql.path
dgs.graphql.websocket.connection-init-timeout
spring.graphql.websocket.connection-init-timeout