Fix instrumentation autoconfiguration conditionals fix #392

Author: oliemansm

example-graphql-tools/src/main/resources/application.yml

@@ -6,6 +6,7 @@ server:
 graphql:
   servlet:
     exception-handlers-enabled: true
+    tracing-enabled: "metrics-only"
 graphiql:
   headers:
     Authorization: "Bearer 05bd9a5f3fe0408f89520946b0fe1b06"

graphql-spring-boot-autoconfigure/src/main/java/graphql/kickstart/spring/web/boot/GraphQLInstrumentationAutoConfiguration.java

@@ -1,14 +1,14 @@
 package graphql.kickstart.spring.web.boot;
 
+import graphql.analysis.MaxQueryComplexityInstrumentation;
+import graphql.analysis.MaxQueryDepthInstrumentation;
+import graphql.execution.instrumentation.tracing.TracingInstrumentation;
 import graphql.kickstart.servlet.GraphQLWebsocketServlet;
 import graphql.kickstart.spring.web.boot.metrics.MetricsInstrumentation;
 import graphql.kickstart.spring.web.boot.metrics.TracingNoResolversInstrumentation;
 import graphql.kickstart.spring.web.boot.metrics.WebsocketMetrics;
-import graphql.analysis.MaxQueryComplexityInstrumentation;
-import graphql.analysis.MaxQueryDepthInstrumentation;
-import graphql.execution.instrumentation.tracing.TracingInstrumentation;
 import io.micrometer.core.instrument.MeterRegistry;
-import org.springframework.beans.factory.annotation.Value;
+import lombok.RequiredArgsConstructor;
 import org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration;
 import org.springframework.boot.actuate.autoconfigure.metrics.export.simple.SimpleMetricsExportAutoConfiguration;
 import org.springframework.boot.autoconfigure.AutoConfigureAfter;
@@ -25,34 +25,28 @@
  * @author Marcel Overdijk
  */
 @Configuration
+@RequiredArgsConstructor
 @ConditionalOnClass(MetricsAutoConfiguration.class)
 @AutoConfigureAfter({MetricsAutoConfiguration.class, SimpleMetricsExportAutoConfiguration.class,
     GraphQLWebsocketAutoConfiguration.class})
 @ConditionalOnProperty(value = "graphql.servlet.enabled", havingValue = "true", matchIfMissing = true)
 @EnableConfigurationProperties({GraphQLServletProperties.class})
 public class GraphQLInstrumentationAutoConfiguration {
 
-  @Value("${graphql.servlet.maxQueryComplexity:#{null}}")
-  private Integer maxQueryComplexity;
-
-  @Value("${graphql.servlet.maxQueryDepth:#{null}}")
-  private Integer maxQueryDepth;
-
-  @Value("${graphql.servlet.tracing-enabled:'false'}")
-  private String tracingEnabled;
+  private final GraphQLServletProperties graphqlServletProperties;
 
   @Bean
   @ConditionalOnMissingBean({TracingInstrumentation.class, MetricsInstrumentation.class})
-  @ConditionalOnExpression("${graphql.servlet.tracing-enabled:'false'}.equals('metrics-only')"
-      + "|| ${graphql.servlet.tracing-enabled:'false'}.equals(true)")
+  @ConditionalOnExpression("'${graphql.servlet.tracing-enabled}' == 'metrics-only' "
+      + "|| '${graphql.servlet.tracing-enabled}' == 'true'")
   public TracingInstrumentation tracingInstrumentation() {
     return new TracingInstrumentation();
   }
 
   @Bean
   @ConditionalOnMissingBean
   @ConditionalOnExpression("${graphql.servlet.actuator-metrics:false}"
-      + "&& ${graphql.servlet.tracing-enabled:'false'}.toString().equals('false')")
+      + "&& '${graphql.servlet.tracing-enabled}' == 'false'")
   public TracingNoResolversInstrumentation tracingNoResolversInstrumentation() {
     return new TracingNoResolversInstrumentation();
   }
@@ -61,29 +55,31 @@ public TracingNoResolversInstrumentation tracingNoResolversInstrumentation() {
   @ConditionalOnMissingBean
   @ConditionalOnProperty(value = "graphql.servlet.max-query-complexity")
   public MaxQueryComplexityInstrumentation maxQueryComplexityInstrumentation() {
-    return new MaxQueryComplexityInstrumentation(maxQueryComplexity);
+    return new MaxQueryComplexityInstrumentation(graphqlServletProperties.getMaxQueryComplexity());
   }
 
   @Bean
   @ConditionalOnMissingBean
   @ConditionalOnProperty(value = "graphql.servlet.max-query-depth")
   public MaxQueryDepthInstrumentation maxQueryDepthInstrumentation() {
-    return new MaxQueryDepthInstrumentation(maxQueryDepth);
+    return new MaxQueryDepthInstrumentation(graphqlServletProperties.getMaxQueryDepth());
   }
 
   @Bean
   @ConditionalOnProperty(value = "graphql.servlet.actuator-metrics", havingValue = "true")
   @ConditionalOnBean({MeterRegistry.class, TracingInstrumentation.class})
   @ConditionalOnMissingBean
   public MetricsInstrumentation metricsInstrumentation(MeterRegistry meterRegistry) {
-    return new MetricsInstrumentation(meterRegistry, Boolean.TRUE.toString().equals(tracingEnabled));
+    return new MetricsInstrumentation(meterRegistry,
+        Boolean.TRUE.toString().equals(graphqlServletProperties.getTracingEnabled()));
   }
 
   @Bean
   @ConditionalOnProperty(value = "graphql.servlet.actuator-metrics", havingValue = "true")
   @ConditionalOnBean({MeterRegistry.class, GraphQLWebsocketServlet.class})
   @ConditionalOnMissingBean
-  public WebsocketMetrics websocketMetrics(MeterRegistry meterRegistry, GraphQLWebsocketServlet websocketServlet) {
+  public WebsocketMetrics websocketMetrics(MeterRegistry meterRegistry,
+      GraphQLWebsocketServlet websocketServlet) {
     return new WebsocketMetrics(meterRegistry, websocketServlet);
   }
 

graphql-spring-boot-autoconfigure/src/main/java/graphql/kickstart/spring/web/boot/GraphQLServletProperties.java

@@ -29,31 +29,29 @@
 @ConfigurationProperties(prefix = "graphql.servlet")
 public class GraphQLServletProperties {
 
+  private boolean enabled = true;
+  private boolean corsEnabled = true;
   private String mapping = "/graphql";
   private boolean exceptionHandlersEnabled = false;
   private long subscriptionTimeout = 0;
   private ContextSetting contextSetting = ContextSetting.PER_QUERY_WITH_INSTRUMENTATION;
   private long asyncTimeout = 30000;
-
-  private boolean mappingIsServletWildcard() {
-    return getMapping().endsWith("/*");
-  }
-
-  private boolean mappingIsAntWildcard() {
-    return getMapping().endsWith("/**");
-  }
+  private String tracingEnabled = "false";
+  private boolean actuatorMetrics;
+  private Integer maxQueryComplexity;
+  private Integer maxQueryDepth;
 
   /**
    * @return the servlet mapping, coercing into an appropriate wildcard for servlets (ending in /*)
    */
   public String getServletMapping() {
-    final String mapping = getMapping();
+    final String originalMapping = getMapping();
     if (mappingIsAntWildcard()) {
-      return mapping.replaceAll("\\*$", "");
+      return originalMapping.replaceAll("\\*$", "");
     } else if (mappingIsServletWildcard()) {
-      return mapping;
+      return originalMapping;
     } else {
-      return mapping.endsWith("/") ? mapping + "*" : mapping + "/*";
+      return originalMapping.endsWith("/") ? originalMapping + "*" : originalMapping + "/*";
     }
   }
 
@@ -62,14 +60,22 @@ public String getServletMapping() {
    * matchers (ending in /**)
    */
   public String getCorsMapping() {
-    final String mapping = getMapping();
+    final String originalMapping = getMapping();
     if (mappingIsAntWildcard()) {
-      return mapping;
+      return originalMapping;
     } else if (mappingIsServletWildcard()) {
-      return mapping + "*";
+      return originalMapping + "*";
     } else {
-      return mapping.endsWith("/") ? mapping + "**" : mapping + "/**";
+      return originalMapping.endsWith("/") ? originalMapping + "**" : originalMapping + "/**";
     }
   }
 
+  private boolean mappingIsServletWildcard() {
+    return getMapping().endsWith("/*");
+  }
+
+  private boolean mappingIsAntWildcard() {
+    return getMapping().endsWith("/**");
+  }
+
 }

graphql-spring-boot-autoconfigure/src/main/java/graphql/kickstart/spring/web/boot/GraphQLSubscriptionWebsocketProperties.java

@@ -0,0 +1,12 @@
+package graphql.kickstart.spring.web.boot;
+
+import lombok.Data;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+
+@Data
+@ConfigurationProperties("graphql.servlet.subscriptions.websocket")
+class GraphQLSubscriptionWebsocketProperties {
+
+  private String path = "/subscriptions";
+
+}

graphql-spring-boot-autoconfigure/src/main/java/graphql/kickstart/spring/web/boot/GraphQLWebsocketAutoConfiguration.java

@@ -17,8 +17,8 @@
 import java.util.Set;
 import java.util.function.Supplier;
 import javax.websocket.server.ServerContainer;
+import lombok.RequiredArgsConstructor;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.AutoConfigureAfter;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
@@ -33,19 +33,18 @@
 import org.springframework.web.socket.server.standard.ServerEndpointRegistration;
 
 @Configuration
+@RequiredArgsConstructor
 @ConditionalOnWebApplication
 @ConditionalOnClass(DispatcherServlet.class)
 @Conditional(OnSchemaOrSchemaProviderBean.class)
 @ConditionalOnProperty(value = "graphql.servlet.websocket.enabled", havingValue = "true", matchIfMissing = true)
 @AutoConfigureAfter({GraphQLJavaToolsAutoConfiguration.class})
-@EnableConfigurationProperties(GraphQLSubscriptionApolloProperties.class)
+@EnableConfigurationProperties({GraphQLSubscriptionApolloProperties.class,
+    GraphQLSubscriptionWebsocketProperties.class})
 public class GraphQLWebsocketAutoConfiguration {
 
-  @Value("${graphql.servlet.subscriptions.websocket.path:/subscriptions}")
-  private String websocketPath;
-
-  @Autowired
-  private GraphQLSubscriptionApolloProperties apolloProperties;
+  private final GraphQLSubscriptionApolloProperties apolloProperties;
+  private final GraphQLSubscriptionWebsocketProperties websocketProperties;
 
   @Bean
   @ConditionalOnMissingBean
@@ -80,7 +79,8 @@ public GraphQLWebsocketServlet graphQLWebsocketServlet(
       listeners.addAll(connectionListeners);
     }
     keepAliveListener().ifPresent(listeners::add);
-    return new GraphQLWebsocketServlet(graphQLInvoker, invocationInputFactory, graphQLObjectMapper, listeners);
+    return new GraphQLWebsocketServlet(graphQLInvoker, invocationInputFactory, graphQLObjectMapper,
+        listeners);
   }
 
   private Optional<SubscriptionConnectionListener> keepAliveListener() {
@@ -95,7 +95,7 @@ private Optional<SubscriptionConnectionListener> keepAliveListener() {
   @Bean
   @ConditionalOnClass(ServerContainer.class)
   public ServerEndpointRegistration serverEndpointRegistration(GraphQLWebsocketServlet servlet) {
-    return new GraphQLWsServerEndpointRegistration(websocketPath, servlet);
+    return new GraphQLWsServerEndpointRegistration(websocketProperties.getPath(), servlet);
   }
 
   @Bean

graphql-spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json

@@ -5,50 +5,5 @@
     }
   ],
   "properties": [
-    {
-      "name": "graphql.servlet.enabled",
-      "defaultValue": true,
-      "type": "java.lang.Boolean"
-    },
-    {
-      "name": "graphql.servlet.mapping",
-      "defaultValue": "/graphql",
-      "type": "java.lang.String"
-    },
-    {
-      "name": "graphql.servlet.corsEnabled",
-      "defaultValue": true,
-      "type": "java.lang.Boolean"
-    },
-    {
-      "name": "graphql.servlet.asyncModeEnabled",
-      "defaultValue": false,
-      "type": "java.lang.Boolean"
-    },
-    {
-      "name": "graphql.servlet.subscriptions.websocket.path",
-      "defaultValue": "/subscriptions",
-      "type": "java.lang.String"
-    },
-    {
-      "name": "graphql.servlet.tracingEnabled",
-      "defaultValue": false,
-      "type": "java.lang.String"
-    },
-    {
-      "name": "graphql.servlet.maxQueryComplexity",
-      "defaultValue": null,
-      "type": "java.lang.Integer"
-    },
-    {
-      "name": "graphql.servlet.maxQueryDepth",
-      "defaultValue": null,
-      "type": "java.lang.Integer"
-    },
-    {
-      "name": "graphql.servlet.actuator-metrics",
-      "defaultValue": false,
-      "type": "java.lang.Boolean"
-    }
   ]
 }