[apache#2167] refactor: refactor TestGravitinoConnector to enable Sta…

…ticGuardedByInstance error-prone

build.gradle.kts

@@ -334,6 +334,7 @@ subprojects {
         "RandomCast",
         "RandomModInteger",
         "ReferenceEquality",
+        "StaticGuardedByInstance",
         "SelfAssignment",
         "SelfComparison",
         "SelfEquals",

trino-connector/src/main/java/com/datastrato/gravitino/trino/connector/GravitinoConnectorFactory.java

@@ -10,6 +10,7 @@
 import com.datastrato.gravitino.trino.connector.catalog.CatalogConnectorFactory;
 import com.datastrato.gravitino.trino.connector.catalog.CatalogConnectorManager;
 import com.datastrato.gravitino.trino.connector.catalog.CatalogInjector;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import io.trino.spi.TrinoException;
@@ -32,6 +33,11 @@ public String getName() {
     return DEFAULT_CONNECTOR_NAME;
   }
 
+  @VisibleForTesting
+  CatalogConnectorManager getCatalogConnectorManager() {
+    return catalogConnectorManager;
+  }
+
   /**
    * This function call by trino creates a connector. It creates DummyGravitinoConnector at first.
    * Another time's it get GravitinoConnector by CatalogConnectorManager
@@ -58,12 +64,6 @@ public Connector create(
           catalogConnectorManager =
               new CatalogConnectorManager(catalogInjector, catalogConnectorFactory);
           catalogConnectorManager.config(config);
-
-          // For testing
-          if (GravitinoPlugin.gravitinoClient != null) {
-            catalogConnectorManager.setGravitinoClient(GravitinoPlugin.gravitinoClient);
-            GravitinoPlugin.catalogConnectorManager = catalogConnectorManager;
-          }
           catalogConnectorManager.start();
 
         } catch (Exception e) {

trino-connector/src/main/java/com/datastrato/gravitino/trino/connector/GravitinoPlugin.java

@@ -4,19 +4,13 @@
  */
 package com.datastrato.gravitino.trino.connector;
 
-import com.datastrato.gravitino.client.GravitinoClient;
-import com.datastrato.gravitino.trino.connector.catalog.CatalogConnectorManager;
 import com.google.common.collect.ImmutableList;
 import io.trino.spi.Plugin;
 import io.trino.spi.connector.ConnectorFactory;
 
 /** Trino plugin endpoint, using java spi mechanism */
 public class GravitinoPlugin implements Plugin {
 
-  // For testing.
-  public static CatalogConnectorManager catalogConnectorManager;
-  public static GravitinoClient gravitinoClient;
-
   @Override
   public Iterable<ConnectorFactory> getConnectorFactories() {
     return ImmutableList.of(new GravitinoConnectorFactory());

trino-connector/src/test/java/com/datastrato/gravitino/trino/connector/TestGravitinoConnector.java

@@ -10,6 +10,8 @@
 import static org.testng.Assert.assertEquals;
 
 import com.datastrato.gravitino.NameIdentifier;
+import com.datastrato.gravitino.client.GravitinoClient;
+import com.datastrato.gravitino.trino.connector.catalog.CatalogConnectorManager;
 import io.trino.Session;
 import io.trino.plugin.memory.MemoryPlugin;
 import io.trino.testing.AbstractTestQueryFramework;
@@ -36,6 +38,8 @@ public class TestGravitinoConnector extends AbstractTestQueryFramework {
 
   GravitinoMockServer server;
 
+  GravitinoClient gravitinoClient;
+
   @BeforeMethod
   public void reloadCatalog() {
     server.reloadCatalogs();
@@ -44,15 +48,16 @@ public void reloadCatalog() {
   @Override
   protected QueryRunner createQueryRunner() throws Exception {
     server = closeAfterClass(new GravitinoMockServer());
-    GravitinoPlugin.gravitinoClient = server.createGravitinoClient();
+    gravitinoClient = server.createGravitinoClient();
 
     Session session = testSessionBuilder().setCatalog("gravitino").build();
     QueryRunner queryRunner = null;
     try {
       // queryRunner = LocalQueryRunner.builder(session).build();
       queryRunner = DistributedQueryRunner.builder(session).setNodeCount(1).build();
 
-      queryRunner.installPlugin(new GravitinoPlugin());
+      TestGravitinoPlugin gravitinoPlugin = new TestGravitinoPlugin();
+      queryRunner.installPlugin(gravitinoPlugin);
       queryRunner.installPlugin(new MemoryPlugin());
 
       {
@@ -70,10 +75,14 @@ protected QueryRunner createQueryRunner() throws Exception {
         properties.put("gravitino.uri", "http://127.0.0.1:8090");
         queryRunner.createCatalog("test1", "gravitino", properties);
       }
-      server.setCatalogConnectorManager(GravitinoPlugin.catalogConnectorManager);
+
+      CatalogConnectorManager catalogConnectorManager =
+          gravitinoPlugin.getCatalogConnectorManager();
+      catalogConnectorManager.setGravitinoClient(gravitinoClient);
+      server.setCatalogConnectorManager(catalogConnectorManager);
       // Wait for the catalog to be created. Wait for at least 30 seconds.
       int max_tries = 35;
-      while (GravitinoPlugin.catalogConnectorManager.getCatalogs().isEmpty() && max_tries > 0) {
+      while (catalogConnectorManager.getCatalogs().isEmpty() && max_tries > 0) {
         Thread.sleep(1000);
         max_tries--;
       }
@@ -253,8 +262,7 @@ public void testCreateCatalog() throws Exception {
     assertThat(computeActual("show catalogs").getOnlyColumnAsSet()).doesNotContain("test.memory1");
 
     // test metalake named test1. the connnector name is test1
-    GravitinoPlugin.gravitinoClient.createMetalake(
-        NameIdentifier.ofMetalake("test1"), "", Collections.emptyMap());
+    gravitinoClient.createMetalake(NameIdentifier.ofMetalake("test1"), "", Collections.emptyMap());
 
     assertUpdate("call test1.system.create_catalog('memory1', 'memory', Map())");
     assertThat(computeActual("show catalogs").getOnlyColumnAsSet()).contains("test1.memory1");

trino-connector/src/test/java/com/datastrato/gravitino/trino/connector/TestGravitinoConnectorFactory.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+
+package com.datastrato.gravitino.trino.connector;
+
+import io.trino.spi.connector.Connector;
+import io.trino.spi.connector.ConnectorContext;
+import java.util.Map;
+
+class TestGravitinoConnectorFactory extends GravitinoConnectorFactory {
+  private Runnable hook;
+
+  void setHook(Runnable hook) {
+    this.hook = hook;
+  }
+
+  @Override
+  public Connector create(
+      String catalogName, Map<String, String> requiredConfig, ConnectorContext context) {
+    Connector connector = super.create(catalogName, requiredConfig, context);
+    hook.run();
+    return connector;
+  }
+}

trino-connector/src/test/java/com/datastrato/gravitino/trino/connector/TestGravitinoPlugin.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+
+package com.datastrato.gravitino.trino.connector;
+
+import com.datastrato.gravitino.trino.connector.catalog.CatalogConnectorManager;
+import com.google.common.collect.ImmutableList;
+import io.trino.spi.connector.ConnectorFactory;
+
+public class TestGravitinoPlugin extends GravitinoPlugin {
+  private TestGravitinoConnectorFactory factory;
+  private CatalogConnectorManager catalogConnectorManager;
+
+  @Override
+  public Iterable<ConnectorFactory> getConnectorFactories() {
+    factory = new TestGravitinoConnectorFactory();
+    factory.setHook(this::init);
+    return ImmutableList.of(factory);
+  }
+
+  private void init() {
+    catalogConnectorManager = factory.getCatalogConnectorManager();
+  }
+
+  public CatalogConnectorManager getCatalogConnectorManager() {
+    return catalogConnectorManager;
+  }
+}