Eliminate mutable field anti-patterns

Author: Akanksha-kedia

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

@@ -13,6 +13,7 @@
  */
 package io.trino.gateway.ha.clustermonitor;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.util.concurrent.SimpleTimeLimiter;
 import io.airlift.log.Logger;
 import io.airlift.units.Duration;
@@ -39,7 +40,7 @@ public class ClusterStatsJdbcMonitor
 {
     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 +50,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 +70,32 @@ 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());
-            // automatically set ssl config based on url protocol
-            properties.setProperty("SSL", String.valueOf(parsedUrl.getProtocol().equals("https")));
+            // Create connection properties from immutable map
+            connectionProperties = new Properties();
+            // Remove any existing SSL property to avoid confusion
+            for (String key : properties.keySet()) {
+                if (!key.equalsIgnoreCase("SSL")) {
+                    connectionProperties.setProperty(key, properties.get(key));
+                }
+            }
+            // Set SSL config based on url protocol, always taking precedence
+            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

@@ -18,10 +18,10 @@
 import io.trino.gateway.ha.clustermonitor.ClusterStats;
 import io.trino.gateway.ha.clustermonitor.TrinoStatus;
 import io.trino.gateway.ha.config.HaGatewayConfiguration;
+import io.trino.gateway.ha.config.UIConfiguration;
 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.domain.Result;
 import io.trino.gateway.ha.domain.RoutingRule;
 import io.trino.gateway.ha.domain.TableData;
@@ -78,7 +78,6 @@ 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 RoutingRulesManager routingRulesManager;
 
@@ -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();
+        // UI configuration is immutable during application lifetime, no need to copy
+        this.uiConfiguration = requireNonNull(configuration.getUiConfiguration(), "uiConfiguration is null");
         this.routingRulesManager = requireNonNull(routingRulesManager, "routingRulesManager is null");
         RoutingRulesConfiguration routingRules = configuration.getRoutingRules();
         isRulesEngineEnabled = routingRules.isRulesEngineEnabled();