Eliminate mutable field anti-patterns

Author: Akanksha-kedia

gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsJdbcMonitor.java

@@ -32,14 +32,16 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeoutException;
 
+import com.google.common.collect.ImmutableMap;
+
 import static java.util.concurrent.TimeUnit.SECONDS;
 
 public class ClusterStatsJdbcMonitor
         implements ClusterStatsMonitor
 {
     private static final Logger log = Logger.get(ClusterStatsJdbcMonitor.class);
 
-    private final Properties properties; // TODO Avoid using a mutable field
+    private final ImmutableMap<String, String> properties;
     private final Duration queryTimeout;
 
     private static final String STATE_QUERY = "SELECT state, COUNT(*) as count "
@@ -49,15 +51,16 @@ public class ClusterStatsJdbcMonitor
 
     public ClusterStatsJdbcMonitor(BackendStateConfiguration backendStateConfiguration, MonitorConfiguration monitorConfiguration)
     {
-        properties = new Properties();
-        properties.setProperty("user", backendStateConfiguration.getUsername());
-        properties.setProperty("password", backendStateConfiguration.getPassword());
-        properties.setProperty("SSL", String.valueOf(backendStateConfiguration.getSsl()));
+        ImmutableMap.Builder<String, String> propertiesBuilder = ImmutableMap.builder();
+        propertiesBuilder.put("user", backendStateConfiguration.getUsername());
+        propertiesBuilder.put("password", backendStateConfiguration.getPassword());
+        propertiesBuilder.put("SSL", String.valueOf(backendStateConfiguration.getSsl()));
         // explicitPrepare is a valid property for Trino versions >= 431. To avoid compatibility
         // issues with versions < 431, this property is left unset when explicitPrepare=true, which is the default
         if (!monitorConfiguration.isExplicitPrepare()) {
-            properties.setProperty("explicitPrepare", "false");
+            propertiesBuilder.put("explicitPrepare", "false");
         }
+        properties = propertiesBuilder.build();
         queryTimeout = monitorConfiguration.getQueryTimeout();
         log.info("state check configured");
     }
@@ -68,23 +71,27 @@ public ClusterStats monitor(ProxyBackendConfiguration backend)
         String url = backend.getProxyTo();
         ClusterStats.Builder clusterStats = ClusterStatsMonitor.getClusterStatsBuilder(backend);
         String jdbcUrl;
+        Properties connectionProperties;
         try {
             URL parsedUrl = new URL(url);
             jdbcUrl = String
                     .format("jdbc:trino://%s:%s/system",
                             parsedUrl.getHost(),
                             parsedUrl.getPort() == -1 ? parsedUrl.getDefaultPort() : parsedUrl.getPort());
+            // Create connection properties from immutable map
+            connectionProperties = new Properties();
+            connectionProperties.putAll(properties);
             // automatically set ssl config based on url protocol
-            properties.setProperty("SSL", String.valueOf(parsedUrl.getProtocol().equals("https")));
+            connectionProperties.setProperty("SSL", String.valueOf(parsedUrl.getProtocol().equals("https")));
         }
         catch (MalformedURLException e) {
             throw new IllegalArgumentException("Invalid backend URL: " + url, e);
         }
 
-        try (Connection conn = DriverManager.getConnection(jdbcUrl, properties);
+        try (Connection conn = DriverManager.getConnection(jdbcUrl, connectionProperties);
                 PreparedStatement statement = SimpleTimeLimiter.create(Executors.newSingleThreadExecutor()).callWithTimeout(
                         () -> conn.prepareStatement(STATE_QUERY), 10, SECONDS)) {
-            statement.setString(1, (String) properties.get("user"));
+            statement.setString(1, properties.get("user"));
             statement.setQueryTimeout((int) queryTimeout.roundTo(SECONDS));
             Map<String, Integer> partialState = new HashMap<>();
             ResultSet rs = statement.executeQuery();

gateway-ha/src/main/java/io/trino/gateway/ha/resource/GatewayWebAppResource.java

@@ -21,7 +21,7 @@
 import io.trino.gateway.ha.config.ProxyBackendConfiguration;
 import io.trino.gateway.ha.config.RoutingRulesConfiguration;
 import io.trino.gateway.ha.config.RulesType;
-import io.trino.gateway.ha.config.UIConfiguration;
+import io.trino.gateway.ha.config.ImmutableUIConfiguration;
 import io.trino.gateway.ha.domain.Result;
 import io.trino.gateway.ha.domain.RoutingRule;
 import io.trino.gateway.ha.domain.TableData;
@@ -78,8 +78,7 @@ public class GatewayWebAppResource
     private final ResourceGroupsManager resourceGroupsManager;
     private final boolean isRulesEngineEnabled;
     private final RulesType ruleType;
-    // TODO Avoid putting mutable objects in fields
-    private final UIConfiguration uiConfiguration;
+    private final ImmutableUIConfiguration immutableUiConfiguration;
     private final RoutingRulesManager routingRulesManager;
 
     @Inject
@@ -95,7 +94,8 @@ public GatewayWebAppResource(
         this.queryHistoryManager = requireNonNull(queryHistoryManager, "queryHistoryManager is null");
         this.backendStateManager = requireNonNull(backendStateManager, "backendStateManager is null");
         this.resourceGroupsManager = requireNonNull(resourceGroupsManager, "resourceGroupsManager is null");
-        this.uiConfiguration = configuration.getUiConfiguration();
+        // Create immutable copy of UI configuration to ensure thread safety
+        this.immutableUiConfiguration = ImmutableUIConfiguration.copyOf(configuration.getUiConfiguration());
         this.routingRulesManager = requireNonNull(routingRulesManager, "routingRulesManager is null");
         RoutingRulesConfiguration routingRules = configuration.getRoutingRules();
         isRulesEngineEnabled = routingRules.isRulesEngineEnabled();
@@ -487,6 +487,6 @@ public Response updateRoutingRules(RoutingRule routingRule)
     @Path("/getUIConfiguration")
     public Response getUIConfiguration()
     {
-        return Response.ok(Result.ok(uiConfiguration)).build();
+        return Response.ok(Result.ok(immutableUiConfiguration)).build();
     }
 }