Properly respect the enableAvatica and enableJsonOverHttp options. (a…

…pache#3834)
Gamehu · Jan 11, 2017 · 7662061 · 7662061
1 parent d80bec8
commit 7662061
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 64 deletions.
diff --git a/docs/content/configuration/broker.md b/docs/content/configuration/broker.md
@@ -92,7 +92,7 @@ The broker's [built-in SQL server](../querying/sql.html) can be configured throu
 |Property|Description|Default|
 |--------|-----------|-------|
 |`druid.sql.enable`|Whether to enable SQL at all, including background metadata fetching. If false, this overrides all other SQL-related properties and disables SQL metadata, serving, and planning completely.|false|
-|`druid.sql.server.enableAvatica`|Whether to enable an Avatica server at `/druid/v2/sql/avatica/`.|false|
+|`druid.sql.server.enableAvatica`|Whether to enable an Avatica server at `/druid/v2/sql/avatica/`.|true|
 |`druid.sql.server.enableJsonOverHttp`|Whether to enable a simple JSON over HTTP route at `/druid/v2/sql/`.|true|
 
 #### SQL Planner Configuration

diff --git a/sql/src/main/java/io/druid/sql/avatica/DruidAvaticaHandler.java b/sql/src/main/java/io/druid/sql/avatica/DruidAvaticaHandler.java
@@ -40,22 +40,18 @@ public class DruidAvaticaHandler extends AvaticaJsonHandler
 {
   static final String AVATICA_PATH = "/druid/v2/sql/avatica/";
 
-  private final ServerConfig config;
-
   @Inject
   public DruidAvaticaHandler(
       final CalciteConnection connection,
       @Self final DruidNode druidNode,
-      final AvaticaMonitor avaticaMonitor,
-      final ServerConfig config
+      final AvaticaMonitor avaticaMonitor
   ) throws InstantiationException, IllegalAccessException, InvocationTargetException
   {
     super(
         new LocalService((Meta) CalciteMetaImpl.class.getConstructors()[0].newInstance(connection), avaticaMonitor),
         avaticaMonitor
     );
 
-    this.config = config;
     setServerRpcMetadata(new Service.RpcMetadataResponse(druidNode.getHostAndPort()));
   }
 
@@ -70,7 +66,7 @@ public void handle(
     // This is not integrated with the experimental authorization framework.
     // (Non-trivial since we don't know the dataSources up-front)
 
-    if (config.isEnableAvatica() && request.getRequestURI().equals(AVATICA_PATH)) {
+    if (request.getRequestURI().equals(AVATICA_PATH)) {
       super.handle(target, baseRequest, request, response);
     }
   }

diff --git a/sql/src/main/java/io/druid/sql/avatica/ServerConfig.java b/sql/src/main/java/io/druid/sql/avatica/ServerConfig.java
diff --git a/sql/src/main/java/io/druid/sql/guice/SqlModule.java b/sql/src/main/java/io/druid/sql/guice/SqlModule.java
@@ -32,7 +32,6 @@
 import io.druid.server.metrics.MetricsModule;
 import io.druid.sql.avatica.AvaticaMonitor;
 import io.druid.sql.avatica.DruidAvaticaHandler;
-import io.druid.sql.avatica.ServerConfig;
 import io.druid.sql.calcite.DruidSchema;
 import io.druid.sql.calcite.planner.Calcites;
 import io.druid.sql.calcite.planner.PlannerConfig;
@@ -45,6 +44,8 @@
 public class SqlModule implements Module
 {
   private static final String PROPERTY_SQL_ENABLE = "druid.sql.enable";
+  private static final String PROPERTY_SQL_ENABLE_JSON_OVER_HTTP = "druid.sql.server.enableJsonOverHttp";
+  private static final String PROPERTY_SQL_ENABLE_AVATICA = "druid.sql.server.enableAvatica";
 
   @Inject
   private Properties props;
@@ -57,13 +58,18 @@ public SqlModule()
   public void configure(Binder binder)
   {
     if (isEnabled()) {
-      JsonConfigProvider.bind(binder, "druid.sql.server", ServerConfig.class);
       JsonConfigProvider.bind(binder, "druid.sql.planner", PlannerConfig.class);
-      Jerseys.addResource(binder, SqlResource.class);
-      binder.bind(AvaticaMonitor.class).in(LazySingleton.class);
-      JettyBindings.addHandler(binder, DruidAvaticaHandler.class);
-      MetricsModule.register(binder, AvaticaMonitor.class);
       LifecycleModule.register(binder, DruidSchema.class);
+
+      if (isJsonOverHttpEnabled()) {
+        Jerseys.addResource(binder, SqlResource.class);
+      }
+
+      if (isAvaticaEnabled()) {
+        binder.bind(AvaticaMonitor.class).in(LazySingleton.class);
+        JettyBindings.addHandler(binder, DruidAvaticaHandler.class);
+        MetricsModule.register(binder, AvaticaMonitor.class);
+      }
     }
   }
 
@@ -85,4 +91,16 @@ private boolean isEnabled()
     Preconditions.checkNotNull(props, "props");
     return Boolean.valueOf(props.getProperty(PROPERTY_SQL_ENABLE, "false"));
   }
+
+  private boolean isJsonOverHttpEnabled()
+  {
+    Preconditions.checkNotNull(props, "props");
+    return Boolean.valueOf(props.getProperty(PROPERTY_SQL_ENABLE_JSON_OVER_HTTP, "true"));
+  }
+
+  private boolean isAvaticaEnabled()
+  {
+    Preconditions.checkNotNull(props, "props");
+    return Boolean.valueOf(props.getProperty(PROPERTY_SQL_ENABLE_AVATICA, "true"));
+  }
 }
diff --git a/sql/src/test/java/io/druid/sql/avatica/DruidAvaticaHandlerTest.java b/sql/src/test/java/io/druid/sql/avatica/DruidAvaticaHandlerTest.java
@@ -74,19 +74,10 @@ public void setUp() throws Exception
         ),
         plannerConfig
     );
-    final ServerConfig serverConfig = new ServerConfig()
-    {
-      @Override
-      public boolean isEnableAvatica()
-      {
-        return true;
-      }
-    };
     final DruidAvaticaHandler handler = new DruidAvaticaHandler(
         serverConnection,
         new DruidNode("dummy", "dummy", 1),
-        new AvaticaMonitor(),
-        serverConfig
+        new AvaticaMonitor()
     );
     final int port = new Random().nextInt(9999) + 10000;
     server = new Server(new InetSocketAddress("127.0.0.1", port));