fix(csharp/src/Drivers/Apache): Improve handling of authentication and server type enumeration parsing (apache#2574)

birschick-bq · web-flow · commit 5a7ce9675eec · 2025-03-03T15:58:22.000-08:00
Improves idiomatic handling of authentication and server type enumerations Fixes apache#2569
diff --git a/csharp/src/Drivers/Apache/Hive2/HiveServer2AuthType.cs b/csharp/src/Drivers/Apache/Hive2/HiveServer2AuthType.cs
@@ -19,7 +19,6 @@ namespace Apache.Arrow.Adbc.Drivers.Apache.Hive2
 {
     internal enum HiveServer2AuthType
     {
-        Invalid = 0,
         None,
         UsernameOnly,
         Basic,
@@ -46,7 +45,7 @@ internal static bool TryParse(string? authType, out HiveServer2AuthType authType
                     authTypeValue = HiveServer2AuthType.Basic;
                     return true;
                 default:
-                    authTypeValue = HiveServer2AuthType.Invalid;
+                    authTypeValue = default;
                     return false;
             }
         }
diff --git a/csharp/src/Drivers/Apache/Hive2/HiveServer2HttpConnection.cs b/csharp/src/Drivers/Apache/Hive2/HiveServer2HttpConnection.cs
@@ -65,7 +65,10 @@ private void ValidateAuthentication()
             Properties.TryGetValue(AdbcOptions.Username, out string? username);
             Properties.TryGetValue(AdbcOptions.Password, out string? password);
             Properties.TryGetValue(HiveServer2Parameters.AuthType, out string? authType);
-            bool isValidAuthType = HiveServer2AuthTypeParser.TryParse(authType, out HiveServer2AuthType authTypeValue);
+            if (!HiveServer2AuthTypeParser.TryParse(authType, out HiveServer2AuthType authTypeValue))
+            {
+                throw new ArgumentOutOfRangeException(HiveServer2Parameters.AuthType, authType, $"Unsupported {HiveServer2Parameters.AuthType} value.");
+            }
             switch (authTypeValue)
             {
                 case HiveServer2AuthType.Basic:
@@ -143,10 +146,10 @@ public override AdbcStatement CreateStatement()
         }
 
         internal override IArrowArrayStream NewReader<T>(T statement, Schema schema) => new HiveServer2Reader(
-                statement,
-                schema,
-                dataTypeConversion: statement.Connection.DataTypeConversion,
-                enableBatchSizeStopCondition: false);
+            statement,
+            schema,
+            dataTypeConversion: statement.Connection.DataTypeConversion,
+            enableBatchSizeStopCondition: false);
 
         protected override TTransport CreateTransport()
         {
@@ -155,7 +158,10 @@ protected override TTransport CreateTransport()
             Properties.TryGetValue(HiveServer2Parameters.Path, out string? path);
             Properties.TryGetValue(HiveServer2Parameters.Port, out string? port);
             Properties.TryGetValue(HiveServer2Parameters.AuthType, out string? authType);
-            bool isValidAuthType = HiveServer2AuthTypeParser.TryParse(authType, out HiveServer2AuthType authTypeValue);
+            if (!HiveServer2AuthTypeParser.TryParse(authType, out HiveServer2AuthType authTypeValue))
+            {
+                throw new ArgumentOutOfRangeException(HiveServer2Parameters.AuthType, authType, $"Unsupported {HiveServer2Parameters.AuthType} value.");
+            }
             Properties.TryGetValue(AdbcOptions.Username, out string? username);
             Properties.TryGetValue(AdbcOptions.Password, out string? password);
             Properties.TryGetValue(AdbcOptions.Uri, out string? uri);
diff --git a/csharp/src/Drivers/Apache/Hive2/HiveServer2TransportType.cs b/csharp/src/Drivers/Apache/Hive2/HiveServer2TransportType.cs
@@ -19,7 +19,6 @@ namespace Apache.Arrow.Adbc.Drivers.Apache.Hive2
 {
     internal enum HiveServer2TransportType
     {
-        Invalid = 0,
         Http,
         Empty = int.MaxValue,
     }
@@ -40,7 +39,7 @@ internal static bool TryParse(string? serverType, out HiveServer2TransportType s
                     serverTypeValue = HiveServer2TransportType.Http;
                     return true;
                 default:
-                    serverTypeValue = HiveServer2TransportType.Invalid;
+                    serverTypeValue = default;
                     return false;
             }
         }
diff --git a/csharp/src/Drivers/Apache/Impala/ImpalaAuthType.cs b/csharp/src/Drivers/Apache/Impala/ImpalaAuthType.cs
@@ -19,7 +19,6 @@ namespace Apache.Arrow.Adbc.Drivers.Apache.Impala
 {
     internal enum ImpalaAuthType
     {
-        Invalid = 0,
         None,
         UsernameOnly,
         Basic,
@@ -46,7 +45,7 @@ internal static bool TryParse(string? authType, out ImpalaAuthType authTypeValue
                     authTypeValue = ImpalaAuthType.Basic;
                     return true;
                 default:
-                    authTypeValue = ImpalaAuthType.Invalid;
+                    authTypeValue = default;
                     return false;
             }
         }
diff --git a/csharp/src/Drivers/Apache/Impala/ImpalaHttpConnection.cs b/csharp/src/Drivers/Apache/Impala/ImpalaHttpConnection.cs
@@ -49,7 +49,10 @@ protected override void ValidateAuthentication()
             Properties.TryGetValue(AdbcOptions.Username, out string? username);
             Properties.TryGetValue(AdbcOptions.Password, out string? password);
             Properties.TryGetValue(ImpalaParameters.AuthType, out string? authType);
-            bool isValidAuthType = ImpalaAuthTypeParser.TryParse(authType, out ImpalaAuthType authTypeValue);
+            if (!ImpalaAuthTypeParser.TryParse(authType, out ImpalaAuthType authTypeValue))
+            {
+                throw new ArgumentOutOfRangeException(ImpalaParameters.AuthType, authType, $"Unsupported {ImpalaParameters.AuthType} value.");
+            }
             switch (authTypeValue)
             {
                 case ImpalaAuthType.Basic:
@@ -130,7 +133,10 @@ protected override TTransport CreateTransport()
             Properties.TryGetValue(ImpalaParameters.Path, out string? path);
             Properties.TryGetValue(ImpalaParameters.Port, out string? port);
             Properties.TryGetValue(ImpalaParameters.AuthType, out string? authType);
-            bool isValidAuthType = ImpalaAuthTypeParser.TryParse(authType, out ImpalaAuthType authTypeValue);
+            if (!ImpalaAuthTypeParser.TryParse(authType, out ImpalaAuthType authTypeValue))
+            {
+                throw new ArgumentOutOfRangeException(ImpalaParameters.AuthType, authType, $"Unsupported {ImpalaParameters.AuthType} value.");
+            }
             Properties.TryGetValue(AdbcOptions.Username, out string? username);
             Properties.TryGetValue(AdbcOptions.Password, out string? password);
             Properties.TryGetValue(AdbcOptions.Uri, out string? uri);
diff --git a/csharp/src/Drivers/Apache/Impala/ImpalaServerType.cs b/csharp/src/Drivers/Apache/Impala/ImpalaServerType.cs
@@ -19,7 +19,6 @@ namespace Apache.Arrow.Adbc.Drivers.Apache.Impala
 {
     internal enum ImpalaServerType
     {
-        Invalid = 0,
         Http,
         Standard,
         Empty = int.MaxValue,
@@ -44,7 +43,7 @@ internal static bool TryParse(string? serverType, out ImpalaServerType serverTyp
                     serverTypeValue = ImpalaServerType.Standard;
                     return true;
                 default:
-                    serverTypeValue = ImpalaServerType.Invalid;
+                    serverTypeValue = default;
                     return false;
             }
         }
diff --git a/csharp/src/Drivers/Apache/Impala/ImpalaStandardConnection.cs b/csharp/src/Drivers/Apache/Impala/ImpalaStandardConnection.cs
@@ -40,7 +40,10 @@ protected override void ValidateAuthentication()
             Properties.TryGetValue(AdbcOptions.Username, out string? username);
             Properties.TryGetValue(AdbcOptions.Password, out string? password);
             Properties.TryGetValue(ImpalaParameters.AuthType, out string? authType);
-            bool isValidAuthType = ImpalaAuthTypeParser.TryParse(authType, out ImpalaAuthType authTypeValue);
+            if (!ImpalaAuthTypeParser.TryParse(authType, out ImpalaAuthType authTypeValue))
+            {
+                throw new ArgumentOutOfRangeException(ImpalaParameters.AuthType, authType, $"Unsupported {ImpalaParameters.AuthType} value.");
+            }
             switch (authTypeValue)
             {
                 case ImpalaAuthType.None:
@@ -120,7 +123,10 @@ protected override TOpenSessionReq CreateSessionRequest()
             Properties.TryGetValue(AdbcOptions.Username, out string? username);
             Properties.TryGetValue(AdbcOptions.Password, out string? password);
             Properties.TryGetValue(ImpalaParameters.AuthType, out string? authType);
-            bool isValidAuthType = ImpalaAuthTypeParser.TryParse(authType, out ImpalaAuthType authTypeValue);
+            if (!ImpalaAuthTypeParser.TryParse(authType, out ImpalaAuthType authTypeValue))
+            {
+                throw new ArgumentOutOfRangeException(ImpalaParameters.AuthType, authType, $"Unsupported {ImpalaParameters.AuthType} value.");
+            }
             TOpenSessionReq request = new TOpenSessionReq(TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V7)
             {
                 CanUseMultipleCatalogs = true,
diff --git a/csharp/src/Drivers/Apache/Spark/SparkAuthType.cs b/csharp/src/Drivers/Apache/Spark/SparkAuthType.cs
@@ -19,7 +19,6 @@ namespace Apache.Arrow.Adbc.Drivers.Apache.Spark
 {
     internal enum SparkAuthType
     {
-        Invalid = 0,
         None,
         UsernameOnly,
         Basic,
@@ -50,7 +49,7 @@ internal static bool TryParse(string? authType, out SparkAuthType authTypeValue)
                     authTypeValue = SparkAuthType.Token;
                     return true;
                 default:
-                    authTypeValue = SparkAuthType.Invalid;
+                    authTypeValue = default;
                     return false;
             }
         }
diff --git a/csharp/src/Drivers/Apache/Spark/SparkConnectionFactory.cs b/csharp/src/Drivers/Apache/Spark/SparkConnectionFactory.cs
@@ -40,7 +40,6 @@ public static SparkConnection NewConnection(IReadOnlyDictionary<string, string>
                 // TODO: Re-enable when properly supported
                 //SparkServerType.Standard => new SparkStandardConnection(properties),
                 _ => throw new ArgumentOutOfRangeException(nameof(properties), $"Unsupported or unknown value '{type}' given for property '{SparkParameters.Type}'. Supported types: {ServerTypeParser.SupportedList}"),
-
             };
         }
 
diff --git a/csharp/src/Drivers/Apache/Spark/SparkHttpConnection.cs b/csharp/src/Drivers/Apache/Spark/SparkHttpConnection.cs
@@ -51,7 +51,10 @@ protected override void ValidateAuthentication()
             Properties.TryGetValue(AdbcOptions.Username, out string? username);
             Properties.TryGetValue(AdbcOptions.Password, out string? password);
             Properties.TryGetValue(SparkParameters.AuthType, out string? authType);
-            bool isValidAuthType = SparkAuthTypeParser.TryParse(authType, out SparkAuthType authTypeValue);
+            if (!SparkAuthTypeParser.TryParse(authType, out SparkAuthType authTypeValue))
+            {
+                throw new ArgumentOutOfRangeException(SparkParameters.AuthType, authType, $"Unsupported {SparkParameters.AuthType} value.");
+            }
             switch (authTypeValue)
             {
                 case SparkAuthType.Token:
@@ -138,7 +141,10 @@ protected override TTransport CreateTransport()
             Properties.TryGetValue(SparkParameters.Path, out string? path);
             Properties.TryGetValue(SparkParameters.Port, out string? port);
             Properties.TryGetValue(SparkParameters.AuthType, out string? authType);
-            bool isValidAuthType = SparkAuthTypeParser.TryParse(authType, out SparkAuthType authTypeValue);
+            if (!SparkAuthTypeParser.TryParse(authType, out SparkAuthType authTypeValue))
+            {
+                throw new ArgumentOutOfRangeException(SparkParameters.AuthType, authType, $"Unsupported {SparkParameters.AuthType} value.");
+            }
             Properties.TryGetValue(SparkParameters.Token, out string? token);
             Properties.TryGetValue(AdbcOptions.Username, out string? username);
             Properties.TryGetValue(AdbcOptions.Password, out string? password);
diff --git a/csharp/src/Drivers/Apache/Spark/SparkServerType.cs b/csharp/src/Drivers/Apache/Spark/SparkServerType.cs
@@ -19,7 +19,6 @@ namespace Apache.Arrow.Adbc.Drivers.Apache.Spark
 {
     internal enum SparkServerType
     {
-        Invalid = 0,
         Http,
         Databricks,
         Standard,
@@ -48,7 +47,7 @@ internal static bool TryParse(string? serverType, out SparkServerType serverType
                     serverTypeValue = SparkServerType.Standard;
                     return true;
                 default:
-                    serverTypeValue = SparkServerType.Invalid;
+                    serverTypeValue = default;
                     return false;
             }
         }
diff --git a/csharp/src/Drivers/Apache/Spark/SparkStandardConnection.cs b/csharp/src/Drivers/Apache/Spark/SparkStandardConnection.cs
@@ -37,7 +37,10 @@ protected override void ValidateAuthentication()
             Properties.TryGetValue(AdbcOptions.Username, out string? username);
             Properties.TryGetValue(AdbcOptions.Password, out string? password);
             Properties.TryGetValue(SparkParameters.AuthType, out string? authType);
-            bool isValidAuthType = SparkAuthTypeParser.TryParse(authType, out SparkAuthType authTypeValue);
+            if (!SparkAuthTypeParser.TryParse(authType, out SparkAuthType authTypeValue))
+            {
+                throw new ArgumentOutOfRangeException(SparkParameters.AuthType, authType, $"Unsupported {SparkParameters.AuthType} value.");
+            }
             switch (authTypeValue)
             {
                 case SparkAuthType.None:
@@ -112,7 +115,10 @@ protected override TOpenSessionReq CreateSessionRequest()
             Properties.TryGetValue(AdbcOptions.Username, out string? username);
             Properties.TryGetValue(AdbcOptions.Password, out string? password);
             Properties.TryGetValue(SparkParameters.AuthType, out string? authType);
-            bool isValidAuthType = SparkAuthTypeParser.TryParse(authType, out SparkAuthType authTypeValue);
+            if (!SparkAuthTypeParser.TryParse(authType, out SparkAuthType authTypeValue))
+            {
+                throw new ArgumentOutOfRangeException(SparkParameters.AuthType, authType, $"Unsupported {SparkParameters.AuthType} value.");
+            }
             TOpenSessionReq request = base.CreateSessionRequest();
             switch (authTypeValue)
             {

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,6 @@ namespace Apache.Arrow.Adbc.Drivers.Apache.Hive2`
`19`	`19`	`{`
`20`	`20`	`internal enum HiveServer2AuthType`
`21`	`21`	`{`
`22`		`- Invalid = 0,`
`23`	`22`	`None,`
`24`	`23`	`UsernameOnly,`
`25`	`24`	`Basic,`
`@@ -46,7 +45,7 @@ internal static bool TryParse(string? authType, out HiveServer2AuthType authType`
`46`	`45`	`authTypeValue = HiveServer2AuthType.Basic;`
`47`	`46`	`return true;`
`48`	`47`	`default:`
`49`		`- authTypeValue = HiveServer2AuthType.Invalid;`
	`48`	`+ authTypeValue = default;`
`50`	`49`	`return false;`
`51`	`50`	`}`
`52`	`51`	`}`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,6 @@ public static SparkConnection NewConnection(IReadOnlyDictionary<string, string>`
`40`	`40`	`// TODO: Re-enable when properly supported`
`41`	`41`	`//SparkServerType.Standard => new SparkStandardConnection(properties),`
`42`	`42`	`_ => throw new ArgumentOutOfRangeException(nameof(properties), $"Unsupported or unknown value '{type}' given for property '{SparkParameters.Type}'. Supported types: {ServerTypeParser.SupportedList}"),`
`43`		`-`
`44`	`43`	`};`
`45`	`44`	`}`
`46`	`45`