Assume that when server customizers are present, we use them.

apache · Apr 10, 2020 · acd429a · acd429a
1 parent bf5431d
commit acd429a
Show file tree

Hide file tree

Showing 5 changed files with 6 additions and 21 deletions.
diff --git a/queryserver-it/src/it/java/org/apache/phoenix/end2end/ServerCustomizersIT.java b/queryserver-it/src/it/java/org/apache/phoenix/end2end/ServerCustomizersIT.java
@@ -49,7 +49,6 @@ public class ServerCustomizersIT extends BaseHBaseManagedTimeIT {
     @BeforeClass
     public static synchronized void setup() throws Exception {
         Configuration conf = getTestClusterConfig();
-        conf.set(QueryServerProperties.QUERY_SERVER_CUSTOMIZERS_ENABLED, "true");
         PQS_UTIL = new QueryServerTestUtil(conf);
         PQS_UTIL.startLocalHBaseCluster(ServerCustomizersIT.class);
         // Register a test jetty server customizer

diff --git a/queryserver/src/main/java/org/apache/phoenix/queryserver/QueryServerOptions.java b/queryserver/src/main/java/org/apache/phoenix/queryserver/QueryServerOptions.java
@@ -37,7 +37,6 @@ public class QueryServerOptions {
     public static final boolean DEFAULT_QUERY_SERVER_CUSTOM_AUTH_ENABLED = false;
     public static final String DEFAULT_QUERY_SERVER_REMOTEUSEREXTRACTOR_PARAM = "doAs";
     public static final boolean DEFAULT_QUERY_SERVER_DISABLE_KERBEROS_LOGIN = false;
-    public static final boolean DEFAULT_QUERY_SERVER_CUSTOMIZERS_ENABLED = false;
 
     public static final boolean DEFAULT_QUERY_SERVER_TLS_ENABLED = false;
     //We default to empty *store password

diff --git a/queryserver/src/main/java/org/apache/phoenix/queryserver/QueryServerProperties.java b/queryserver/src/main/java/org/apache/phoenix/queryserver/QueryServerProperties.java
@@ -50,8 +50,6 @@ public class QueryServerProperties {
             "phoenix.queryserver.spnego.auth.disabled";
     public static final String QUERY_SERVER_WITH_REMOTEUSEREXTRACTOR_ATTRIB =
             "phoenix.queryserver.withRemoteUserExtractor";
-    public static final String QUERY_SERVER_CUSTOMIZERS_ENABLED =
-            "phoenix.queryserver.customizers.enabled";
     public static final String QUERY_SERVER_CUSTOM_AUTH_ENABLED =
             "phoenix.queryserver.custom.auth.enabled";
     public static final String QUERY_SERVER_REMOTEUSEREXTRACTOR_PARAM =

diff --git a/queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java b/queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java
@@ -475,9 +475,11 @@ public void setRemoteUserExtractorIfNecessary(HttpServer.Builder builder, Config
   @VisibleForTesting
   public void enableServerCustomizersIfNecessary(HttpServer.Builder<Server> builder,
                                                  Configuration conf, AvaticaServerConfiguration avaticaServerConfiguration) {
-    if (conf.getBoolean(QueryServerProperties.QUERY_SERVER_CUSTOMIZERS_ENABLED,
-            QueryServerOptions.DEFAULT_QUERY_SERVER_CUSTOMIZERS_ENABLED)) {
-      builder.withServerCustomizers(createServerCustomizers(conf, avaticaServerConfiguration), Server.class);
+    // Always try to enable the "provided" ServerCustomizers. The expectation is that the Factory implementation
+    // will have toggles for each provided customizer, rather than a global toggle to enable customizers.
+    List<ServerCustomizer<Server>> customizers = createServerCustomizers(conf, avaticaServerConfiguration);
+    if (customizers != null && !customizers.isEmpty()) {
+      builder.withServerCustomizers(customizers, Server.class);
     }
   }
 

diff --git a/queryserver/src/test/java/org/apache/phoenix/queryserver/server/ServerCustomizersTest.java b/queryserver/src/test/java/org/apache/phoenix/queryserver/server/ServerCustomizersTest.java
@@ -73,21 +73,8 @@ public List<ServerCustomizer<Server>> createServerCustomizers(Configuration conf
             }
         });
         Configuration conf = new Configuration(false);
-        conf.set(QueryServerProperties.QUERY_SERVER_CUSTOMIZERS_ENABLED, "true");
         QueryServer queryServer = new QueryServer();
         List<ServerCustomizer<Server>> actual = queryServer.createServerCustomizers(conf, avaticaServerConfiguration);
         Assert.assertEquals("Customizers are different", expected, actual);
     }
-
-    @Test
-    @SuppressWarnings("unchecked")
-    public void testEnableCustomizers() {
-        AvaticaServerConfiguration avaticaServerConfiguration = null;
-        HttpServer.Builder builder = mock(HttpServer.Builder.class);
-        Configuration conf = new Configuration(false);
-        conf.set(QueryServerProperties.QUERY_SERVER_CUSTOMIZERS_ENABLED, "true");
-        QueryServer queryServer = new QueryServer();
-        queryServer.enableServerCustomizersIfNecessary(builder, conf, avaticaServerConfiguration);
-        verify(builder).withServerCustomizers(anyList(), any(Class.class));
-    }
-}
+}