Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: GraphiQL may render broken and cannot be fully disabled #1183

Merged
merged 3 commits into from Oct 21, 2022
Merged

fix: GraphiQL may render broken and cannot be fully disabled #1183

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9fdc12a
Fix issues with graphiql on reactive DGS
foo4u Aug 5, 2022
6741dc3
Fix graphiql on webmvc
foo4u Aug 5, 2022
b6bf8b6
Merge branch 'master' into srossillo.fix-graphiql-issues
srinivasankavitha Oct 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions ...in/com/netflix/graphql/dgs/webflux/autoconfiguration/DgsWebfluxConfigurationProperties.kt
View file
Open in desktop
Expand Up @@ -48,6 +48,7 @@ class DgsWebfluxConfigurationProperties(
* Configuration properties for the GraphiQL endpoint.
*/
data class DgsGraphiQLConfigurationProperties(
@DefaultValue("true") var enabled: Boolean = true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this used?

Copy link
Contributor Author

@foo4u foo4u Sep 6, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used by Spring's metadata generator. The property isn't used directly by DGS b/c it's referred to in full dot notation as a @Conditional. This is mainly useful for IDE autocompletion.

dgs-framework/graphql-dgs-spring-webflux-autoconfigure/src/main/kotlin/com/netflix/graphql/dgs/webflux/autoconfiguration/DgsWebFluxAutoConfiguration.kt

Lines 144 to 147 in 8fa2368

@ConditionalOnProperty(name = ["dgs.graphql.graphiql.enabled"], havingValue = "true", matchIfMissing = true)
open fun graphiQlConfigurer(configProps: DgsWebfluxConfigurationProperties): GraphiQlConfigurer {
return GraphiQlConfigurer(configProps)
}

/** Path to the GraphiQL endpoint without trailing slash. */
@DefaultValue("/graphiql") var path: String = "/graphiql",
/** GraphiQL title */
Expand Down
2 changes: 1 addition & 1 deletion ...e/src/main/kotlin/com/netflix/graphql/dgs/webflux/autoconfiguration/GraphiQlConfigurer.kt
View file
Open in desktop
Expand Up @@ -35,7 +35,7 @@ class GraphiQlConfigurer(private val configProps: DgsWebfluxConfigurationPropert
val graphiQLTitle = configProps.graphiql.title
registry
.addResourceHandler(configProps.graphiql.path + "/**")
.addResourceLocations("classpath:/static/graphiql/")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my understanding, could you explain why moving the location out of the static directory addresses the race condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DGS_GRAPHQL_PATH is sometimes unset because there's a race condition between Spring's built in static resource handler and DGS' GraphiQL handlers in both Spring MVC and WebFlux. The alternative option to changing the path for graphiql would be to force applications to configure spring.web.resources.add-mappings to false; it defaults to true.

So, Spring attempts to serve all assets under resources/static as static files. This means the resources are served as-is from disk / JAR, and not processed by the DGS handler. Depending on bean registration order, the DGS handler may be registered after Spring's static resource handler.

From my understanding about Spring, it's best NOT to store templates that require server side rendering in /resources/[static|public].

.addResourceLocations("classpath:/graphiql/")
.resourceChain(true)
.addResolver(PathResourceResolver())
.addTransformer(TokenReplacingTransformer(mapOf("<DGS_GRAPHQL_PATH>" to graphqlPath, "<DGS_GRAPHIQL_TITLE>" to graphiQLTitle), configProps))
Expand Down
0 ...main/resources/static/graphiql/index.html → ...re/src/main/resources/graphiql/index.html
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion ...figure/src/main/kotlin/com/netflix/graphql/dgs/webmvc/autoconfigure/GraphiQLConfigurer.kt
View file
Open in desktop
Expand Up @@ -53,7 +53,7 @@ open class GraphiQLConfigurer(
logger.info("Configuring GraphiQL to use GraphQL endpoint at '{}'", graphqlPath)
registry
.addResourceHandler("/graphiql/**")
.addResourceLocations("classpath:/static/graphiql/")
.addResourceLocations("classpath:/graphiql/")
.setCachePeriod(3600)
.resourceChain(true)
.addResolver(PathResourceResolver())
Expand Down
0 ...main/resources/static/graphiql/index.html → ...re/src/main/resources/graphiql/index.html
View file
Open in desktop
File renamed without changes.