Improve GCS config validation

Author: wendigo

lib/trino-filesystem-gcs/src/main/java/io/trino/filesystem/gcs/GcsDefaultAuth.java

@@ -37,7 +37,6 @@ public GcsDefaultAuth(GcsFileSystemConfig config)
     {
         String jsonKey = config.getJsonKey();
         String jsonKeyFilePath = config.getJsonKeyFilePath();
-        config.validate();
 
         if (jsonKey != null) {
             try (InputStream inputStream = new ByteArrayInputStream(jsonKey.getBytes(UTF_8))) {

lib/trino-filesystem-gcs/src/main/java/io/trino/filesystem/gcs/GcsFileSystemConfig.java

@@ -29,7 +29,6 @@
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 
-import static com.google.common.base.Preconditions.checkState;
 import static io.airlift.units.DataSize.Unit.MEGABYTE;
 
 public class GcsFileSystemConfig
@@ -269,14 +268,13 @@ public boolean isRetryDelayValid()
         return minBackoffDelay.compareTo(maxBackoffDelay) <= 0;
     }
 
-    public void validate()
+    @AssertTrue(message = "Either gcs.use-access-token or gcs.json-key or gcs.json-key-file-path must be set")
+    public boolean isAuthMethodValid()
     {
-        // This cannot be normal validation, as it would make it impossible to write TestGcsFileSystemConfig.testExplicitPropertyMappings
-
         if (useGcsAccessToken) {
-            checkState(jsonKey == null, "Cannot specify 'gcs.json-key' when 'gcs.use-access-token' is set");
-            checkState(jsonKeyFilePath == null, "Cannot specify 'gcs.json-key-file-path' when 'gcs.use-access-token' is set");
+            return jsonKey == null && jsonKeyFilePath == null;
         }
-        checkState(jsonKey == null || jsonKeyFilePath == null, "'gcs.json-key' and 'gcs.json-key-file-path' cannot be both set");
+
+        return (jsonKey == null) ^ (jsonKeyFilePath == null);
     }
 }

lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/TestGcsFileSystemConfig.java

@@ -19,11 +19,9 @@
 import jakarta.validation.constraints.AssertTrue;
 import org.junit.jupiter.api.Test;
 
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 
 import static io.airlift.configuration.testing.ConfigAssertions.assertFullMapping;
 import static io.airlift.configuration.testing.ConfigAssertions.assertRecordedDefaults;
@@ -32,7 +30,6 @@
 import static io.airlift.units.DataSize.Unit.MEGABYTE;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 public class TestGcsFileSystemConfig
 {
@@ -59,10 +56,7 @@ void testDefaults()
 
     @Test
     void testExplicitPropertyMappings()
-            throws IOException
     {
-        Path jsonKeyFile = Files.createTempFile(null, null);
-
         Map<String, String> properties = ImmutableMap.<String, String>builder()
                 .put("gcs.read-block-size", "51MB")
                 .put("gcs.write-block-size", "52MB")
@@ -71,8 +65,6 @@ void testExplicitPropertyMappings()
                 .put("gcs.project-id", "project")
                 .put("gcs.endpoint", "http://custom.dns.org:8000")
                 .put("gcs.use-access-token", "true")
-                .put("gcs.json-key", "{}")
-                .put("gcs.json-key-file-path", jsonKeyFile.toString())
                 .put("gcs.client.max-retries", "10")
                 .put("gcs.client.backoff-scale-factor", "4.0")
                 .put("gcs.client.max-retry-time", "10s")
@@ -89,40 +81,41 @@ void testExplicitPropertyMappings()
                 .setProjectId("project")
                 .setEndpoint(Optional.of("http://custom.dns.org:8000"))
                 .setUseGcsAccessToken(true)
-                .setJsonKey("{}")
-                .setJsonKeyFilePath(jsonKeyFile.toString())
                 .setMaxRetries(10)
                 .setBackoffScaleFactor(4.0)
                 .setMaxRetryTime(new Duration(10, SECONDS))
                 .setMinBackoffDelay(new Duration(20, MILLISECONDS))
                 .setMaxBackoffDelay(new Duration(20, MILLISECONDS))
                 .setApplicationId("application id");
-        assertFullMapping(properties, expected);
+        assertFullMapping(properties, expected, Set.of("gcs.json-key", "gcs.json-key-file-path"));
     }
 
     @Test
     public void testValidation()
     {
-        assertThatThrownBy(
+        assertFailsValidation(
                 new GcsFileSystemConfig()
                         .setUseGcsAccessToken(true)
-                        .setJsonKey("{}}")::validate)
-                .isInstanceOf(IllegalStateException.class)
-                .hasMessage("Cannot specify 'gcs.json-key' when 'gcs.use-access-token' is set");
+                        .setJsonKey("{}}"),
+                "authMethodValid",
+                "Either gcs.use-access-token or gcs.json-key or gcs.json-key-file-path must be set",
+                AssertTrue.class);
 
-        assertThatThrownBy(
+        assertFailsValidation(
                 new GcsFileSystemConfig()
                         .setUseGcsAccessToken(true)
-                        .setJsonKeyFilePath("/dev/null")::validate)
-                .isInstanceOf(IllegalStateException.class)
-                .hasMessage("Cannot specify 'gcs.json-key-file-path' when 'gcs.use-access-token' is set");
+                        .setJsonKeyFilePath("/dev/null"),
+                "authMethodValid",
+                "Either gcs.use-access-token or gcs.json-key or gcs.json-key-file-path must be set",
+                AssertTrue.class);
 
-        assertThatThrownBy(
+        assertFailsValidation(
                 new GcsFileSystemConfig()
                         .setJsonKey("{}")
-                        .setJsonKeyFilePath("/dev/null")::validate)
-                .isInstanceOf(IllegalStateException.class)
-                .hasMessage("'gcs.json-key' and 'gcs.json-key-file-path' cannot be both set");
+                        .setJsonKeyFilePath("/dev/null"),
+                "authMethodValid",
+                "Either gcs.use-access-token or gcs.json-key or gcs.json-key-file-path must be set",
+                AssertTrue.class);
 
         assertFailsValidation(
                 new GcsFileSystemConfig()