Make catalog name be available in guice

trinodb · Oct 8, 2021 · a007051 · a007051
1 parent 6c7d4d8
commit a007051
Show file tree

Hide file tree

Showing 10 changed files with 56 additions and 42 deletions.
diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/CatalogNameModule.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/CatalogNameModule.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.trino.plugin.base;
+
+import com.google.inject.Binder;
+import com.google.inject.Module;
+
+import static java.util.Objects.requireNonNull;
+
+public class CatalogNameModule
+        implements Module
+{
+    private final String catalogName;
+
+    public CatalogNameModule(String catalogName)
+    {
+        this.catalogName = requireNonNull(catalogName, "catalogName is null");
+    }
+
+    @Override
+    public void configure(Binder binder)
+    {
+        binder.bind(CatalogName.class).toInstance(new CatalogName(catalogName));
+    }
+}
diff --git a/...no-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java b/...no-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java
@@ -14,6 +14,7 @@
 package io.trino.plugin.base.security;
 
 import com.google.common.collect.ImmutableSet;
+import io.trino.plugin.base.CatalogName;
 import io.trino.plugin.base.security.TableAccessControlRule.TablePrivilege;
 import io.trino.spi.connector.ConnectorAccessControl;
 import io.trino.spi.connector.ConnectorSecurityContext;
@@ -94,9 +95,9 @@ public class FileBasedAccessControl
     private final List<SessionPropertyAccessControlRule> sessionPropertyRules;
     private final Set<AnySchemaPermissionsRule> anySchemaPermissionsRules;
 
-    public FileBasedAccessControl(String catalogName, FileBasedAccessControlConfig config)
+    public FileBasedAccessControl(CatalogName catalogName, FileBasedAccessControlConfig config)
     {
-        this.catalogName = requireNonNull(catalogName, "catalogName is null");
+        this.catalogName = requireNonNull(catalogName, "catalogName is null").toString();
 
         AccessControlRules rules = parseJson(Paths.get(config.getConfigFile()), AccessControlRules.class);
         checkArgument(!rules.hasRoleRules(), "File connector access control does not support role rules: %s", config.getConfigFile());

diff --git a/...gin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControlModule.java b/...gin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControlModule.java
@@ -18,25 +18,18 @@
 import com.google.inject.Module;
 import com.google.inject.Provides;
 import io.airlift.log.Logger;
+import io.trino.plugin.base.CatalogName;
 import io.trino.spi.connector.ConnectorAccessControl;
 
 import static com.google.common.base.Suppliers.memoizeWithExpiration;
 import static io.airlift.configuration.ConfigBinder.configBinder;
-import static java.util.Objects.requireNonNull;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 public class FileBasedAccessControlModule
         implements Module
 {
     private static final Logger log = Logger.get(FileBasedAccessControlModule.class);
 
-    private final String catalogName;
-
-    public FileBasedAccessControlModule(String catalogName)
-    {
-        this.catalogName = requireNonNull(catalogName, "catalogName is null");
-    }
-
     @Override
     public void configure(Binder binder)
     {
@@ -45,7 +38,7 @@ public void configure(Binder binder)
 
     @Inject
     @Provides
-    public ConnectorAccessControl getConnectorAccessControl(FileBasedAccessControlConfig config)
+    public ConnectorAccessControl getConnectorAccessControl(CatalogName catalogName, FileBasedAccessControlConfig config)
     {
         if (config.getRefreshPeriod() != null) {
             return ForwardingConnectorAccessControl.of(memoizeWithExpiration(

diff --git a/...lugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java b/...lugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java
@@ -14,6 +14,7 @@
 package io.trino.plugin.base.security;
 
 import com.google.common.collect.ImmutableSet;
+import io.trino.plugin.base.CatalogName;
 import io.trino.spi.QueryId;
 import io.trino.spi.connector.ConnectorAccessControl;
 import io.trino.spi.connector.ConnectorSecurityContext;
@@ -440,7 +441,7 @@ private ConnectorAccessControl createAccessControl(String fileName)
         String path = this.getClass().getClassLoader().getResource(fileName).getPath();
         FileBasedAccessControlConfig config = new FileBasedAccessControlConfig();
         config.setConfigFile(path);
-        return new FileBasedAccessControl("test_catalog", config);
+        return new FileBasedAccessControl(new CatalogName("test_catalog"), config);
     }
 
     private static void assertDenied(ThrowingRunnable runnable)

diff --git a/plugin/trino-atop/src/main/java/io/trino/plugin/atop/AtopConnectorFactory.java b/plugin/trino-atop/src/main/java/io/trino/plugin/atop/AtopConnectorFactory.java
@@ -17,6 +17,7 @@
 import io.airlift.bootstrap.Bootstrap;
 import io.airlift.json.JsonModule;
 import io.trino.plugin.atop.AtopConnectorConfig.AtopSecurity;
+import io.trino.plugin.base.CatalogNameModule;
 import io.trino.plugin.base.security.AllowAllAccessControlModule;
 import io.trino.plugin.base.security.FileBasedAccessControlModule;
 import io.trino.spi.classloader.ThreadContextClassLoader;
@@ -65,8 +66,8 @@ public Connector create(String catalogName, Map<String, String> requiredConfig,
                             atopFactoryClass,
                             context.getTypeManager(),
                             context.getNodeManager(),
-                            context.getNodeManager().getEnvironment(),
-                            catalogName),
+                            context.getNodeManager().getEnvironment()),
+                    new CatalogNameModule(catalogName),
                     conditionalModule(
                             AtopConnectorConfig.class,
                             config -> config.getSecurity() == AtopSecurity.NONE,
@@ -75,7 +76,7 @@ public Connector create(String catalogName, Map<String, String> requiredConfig,
                             AtopConnectorConfig.class,
                             config -> config.getSecurity() == AtopSecurity.FILE,
                             binder -> {
-                                binder.install(new FileBasedAccessControlModule(catalogName));
+                                binder.install(new FileBasedAccessControlModule());
                                 binder.install(new JsonModule());
                             }));
 

diff --git a/plugin/trino-atop/src/main/java/io/trino/plugin/atop/AtopModule.java b/plugin/trino-atop/src/main/java/io/trino/plugin/atop/AtopModule.java
@@ -16,7 +16,6 @@
 import com.google.inject.Binder;
 import com.google.inject.Module;
 import com.google.inject.Scopes;
-import io.trino.plugin.base.CatalogName;
 import io.trino.spi.NodeManager;
 import io.trino.spi.type.TypeManager;
 
@@ -30,15 +29,13 @@ public class AtopModule
     private final TypeManager typeManager;
     private final NodeManager nodeManager;
     private final String environment;
-    private final String catalogName;
 
-    public AtopModule(Class<? extends AtopFactory> atopFactoryClass, TypeManager typeManager, NodeManager nodeManager, String environment, String catalogName)
+    public AtopModule(Class<? extends AtopFactory> atopFactoryClass, TypeManager typeManager, NodeManager nodeManager, String environment)
     {
         this.atopFactoryClass = requireNonNull(atopFactoryClass, "atopFactoryClass is null");
         this.typeManager = requireNonNull(typeManager, "typeManager is null");
         this.nodeManager = requireNonNull(nodeManager, "nodeManager is null");
         this.environment = requireNonNull(environment, "environment is null");
-        this.catalogName = requireNonNull(catalogName, "catalogName is null");
     }
 
     @Override
@@ -47,7 +44,6 @@ public void configure(Binder binder)
         binder.bind(TypeManager.class).toInstance(typeManager);
         binder.bind(NodeManager.class).toInstance(nodeManager);
         binder.bind(Environment.class).toInstance(new Environment(environment));
-        binder.bind(CatalogName.class).toInstance(new CatalogName(catalogName));
         binder.bind(AtopConnector.class).in(Scopes.SINGLETON);
         binder.bind(AtopMetadata.class).in(Scopes.SINGLETON);
         binder.bind(AtopSplitManager.class).in(Scopes.SINGLETON);

diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/InternalHiveConnectorFactory.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/InternalHiveConnectorFactory.java
@@ -23,7 +23,7 @@
 import io.airlift.bootstrap.LifeCycleManager;
 import io.airlift.event.client.EventModule;
 import io.airlift.json.JsonModule;
-import io.trino.plugin.base.CatalogName;
+import io.trino.plugin.base.CatalogNameModule;
 import io.trino.plugin.base.TypeDeserializerModule;
 import io.trino.plugin.base.classloader.ClassLoaderSafeConnectorAccessControl;
 import io.trino.plugin.base.classloader.ClassLoaderSafeConnectorPageSinkProvider;
@@ -87,6 +87,7 @@ public static Connector createConnector(String catalogName, Map<String, String>
         ClassLoader classLoader = InternalHiveConnectorFactory.class.getClassLoader();
         try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) {
             Bootstrap app = new Bootstrap(
+                    new CatalogNameModule(catalogName),
                     new EventModule(),
                     new MBeanModule(),
                     new ConnectorObjectNameGeneratorModule(catalogName, "io.trino.plugin.hive", "trino.plugin.hive"),
@@ -99,7 +100,7 @@ public static Connector createConnector(String catalogName, Map<String, String>
                     new HiveAzureModule(),
                     conditionalModule(RubixEnabledConfig.class, RubixEnabledConfig::isCacheEnabled, new RubixModule()),
                     new HiveMetastoreModule(metastore),
-                    new HiveSecurityModule(catalogName),
+                    new HiveSecurityModule(),
                     new HdfsAuthenticationModule(),
                     new HiveProcedureModule(),
                     new MBeanServerModule(),
@@ -109,7 +110,6 @@ public static Connector createConnector(String catalogName, Map<String, String>
                         binder.bind(VersionEmbedder.class).toInstance(context.getVersionEmbedder());
                         binder.bind(PageIndexerFactory.class).toInstance(context.getPageIndexerFactory());
                         binder.bind(PageSorter.class).toInstance(context.getPageSorter());
-                        binder.bind(CatalogName.class).toInstance(new CatalogName(catalogName));
                     },
                     binder -> newSetBinder(binder, EventListener.class),
                     binder -> bindSessionPropertiesProvider(binder, HiveSessionProperties.class),

diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/HiveSecurityModule.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/HiveSecurityModule.java
@@ -21,18 +21,10 @@
 
 import static io.airlift.configuration.ConditionalModule.conditionalModule;
 import static io.airlift.configuration.ConfigurationAwareModule.combine;
-import static java.util.Objects.requireNonNull;
 
 public class HiveSecurityModule
         extends AbstractConfigurationAwareModule
 {
-    private final String catalogName;
-
-    public HiveSecurityModule(String catalogName)
-    {
-        this.catalogName = requireNonNull(catalogName, "catalogName is null");
-    }
-
     @Override
     protected void setup(Binder binder)
     {
@@ -44,7 +36,7 @@ protected void setup(Binder binder)
         bindSecurityModule(
                 "file",
                 combine(
-                        new FileBasedAccessControlModule(catalogName),
+                        new FileBasedAccessControlModule(),
                         new StaticAccessControlMetadataModule()));
         bindSecurityModule(
                 "read-only",

diff --git a/...ino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/RaptorConnectorFactory.java b/...ino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/RaptorConnectorFactory.java
@@ -18,6 +18,7 @@
 import com.google.inject.Module;
 import io.airlift.bootstrap.Bootstrap;
 import io.airlift.json.JsonModule;
+import io.trino.plugin.base.CatalogNameModule;
 import io.trino.plugin.base.jmx.ConnectorObjectNameGeneratorModule;
 import io.trino.plugin.base.jmx.MBeanServerModule;
 import io.trino.plugin.raptor.legacy.backup.BackupModule;
@@ -69,6 +70,7 @@ public ConnectorHandleResolver getHandleResolver()
     public Connector create(String catalogName, Map<String, String> config, ConnectorContext context)
     {
         Bootstrap app = new Bootstrap(
+                new CatalogNameModule(catalogName),
                 new JsonModule(),
                 new MBeanModule(),
                 new ConnectorObjectNameGeneratorModule(catalogName, "io.trino.plugin.raptor.legacy", "trino.plugin.raptor.legacy"),
@@ -82,7 +84,7 @@ public Connector create(String catalogName, Map<String, String> config, Connecto
                 new BackupModule(backupProviders),
                 new StorageModule(),
                 new RaptorModule(catalogName),
-                new RaptorSecurityModule(catalogName));
+                new RaptorSecurityModule());
 
         Injector injector = app
                 .doNotInitializeLogging()

diff --git a/...tor-legacy/src/main/java/io/trino/plugin/raptor/legacy/security/RaptorSecurityModule.java b/...tor-legacy/src/main/java/io/trino/plugin/raptor/legacy/security/RaptorSecurityModule.java
@@ -21,24 +21,16 @@
 import io.trino.plugin.base.security.ReadOnlySecurityModule;
 
 import static io.airlift.configuration.ConditionalModule.conditionalModule;
-import static java.util.Objects.requireNonNull;
 
 public class RaptorSecurityModule
         extends AbstractConfigurationAwareModule
 {
-    private final String catalogName;
-
-    public RaptorSecurityModule(String catalogName)
-    {
-        this.catalogName = requireNonNull(catalogName, "catalogName is null");
-    }
-
     @Override
     protected void setup(Binder binder)
     {
         bindSecurityModule("none", new AllowAllAccessControlModule());
         bindSecurityModule("read-only", new ReadOnlySecurityModule());
-        bindSecurityModule("file", new FileBasedAccessControlModule(catalogName));
+        bindSecurityModule("file", new FileBasedAccessControlModule());
     }
 
     private void bindSecurityModule(String name, Module module)