SNOW-990111: Refactor easy logging improvements

snowflakedb · Jan 17, 2024 · 8d31672 · 8d31672
1 parent 9d2596c
commit 8d31672
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 44 deletions.
diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs
@@ -5,8 +5,6 @@
 using System;
 using System.IO;
 using System.Runtime.InteropServices;
-using Snowflake.Data.Client;
-using Snowflake.Data.Core;
 using Snowflake.Data.Core.Tools;
 using Snowflake.Data.Log;
 
@@ -36,10 +34,15 @@ internal EasyLoggingConfigFinder()
 
         public virtual string FindConfigFilePath(string configFilePathFromConnectionString)
         {
-            return GetFilePathFromInputParameter(configFilePathFromConnectionString, "connection string")
-                    ?? GetFilePathEnvironmentVariable()
-                    ?? GetFilePathFromDriverLocation()
-                    ?? GetFilePathFromHomeDirectory();
+            var configFilePath = GetFilePathFromInputParameter(configFilePathFromConnectionString, "connection string")
+                              ?? GetFilePathEnvironmentVariable()
+                              ?? GetFilePathFromDriverLocation()
+                              ?? GetFilePathFromHomeDirectory();
+            if (configFilePath != null)
+            {
+                CheckIfValidPermissions(configFilePath);
+            }
+            return configFilePath;
         }
 
         private string GetFilePathEnvironmentVariable()
@@ -52,19 +55,12 @@ private string GetFilePathEnvironmentVariable()
 
         private string GetFilePathFromInputParameter(string filePath, string inputDescription)
         {
-            if (!string.IsNullOrEmpty(filePath))
-            {
-                if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
-                {
-                    CheckIfValidPermissions(filePath);
-                }
-                s_logger.Info($"Using config file specified from {inputDescription}");
-                return filePath;
-            }
-            else
+            if (string.IsNullOrEmpty(filePath))
             {
                 return null;
             }
+            s_logger.Info($"Using config file specified from {inputDescription}");
+            return filePath;
         }
 
         private string GetHomeDirectory() =>_environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile);
@@ -95,21 +91,17 @@ private string OnlyIfFileExists(string filePath, string directoryDescription)
         {
             if (_fileOperations.Exists(filePath))
             {
-                if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
-                {
-                    CheckIfValidPermissions(filePath);
-                }
                 s_logger.Info($"Using config file specified from {directoryDescription} directory");
                 return filePath;
             }
-            else
-            {
-                return null;
-            }
+            return null;
         }
 
         private void CheckIfValidPermissions(string filePath)
         {
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+                return;
+
             // Check if others have permissions to modify the file and fail if so
             int filePermissions;
             string commandParameters = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "-c '%a'" : "-f %A";

diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs
@@ -3,7 +3,9 @@
  */
 
 using System;
+using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 using System.Reflection;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
@@ -46,7 +48,7 @@ private ClientConfig TryToParseFile(string fileContent)
             try {
                 var config = JsonConvert.DeserializeObject<ClientConfig>(fileContent);
                 Validate(config);
-                CheckForUnknownFields(fileContent, config);
+                CheckForUnknownFields(fileContent);
                 return config;
             }
             catch (Exception e)
@@ -65,29 +67,21 @@ private void Validate(ClientConfig config)
             }
         }
 
-        private void CheckForUnknownFields(string fileContent, ClientConfig config)
+        private void CheckForUnknownFields(string fileContent)
         {
             // Parse the specified config file and get the key-value pairs from the "common" section
-            JObject obj = (JObject)(JObject.Parse(fileContent).First.First);
-            bool isUnknownField = true;
-            foreach (var keyValuePair in obj)
+            List<string> knownProperties = new List<string>();
+            foreach (var property in typeof(ClientConfigCommonProps).GetProperties())
             {
-                foreach(var property in config.CommonProps.GetType().GetProperties())
-                {
-                    var jsonPropertyAttribute = property.GetCustomAttribute<JsonPropertyAttribute>();
-                    if (keyValuePair.Key.Equals(jsonPropertyAttribute.PropertyName))
-                    {
-                        isUnknownField = false;
-                        break;
-                    }
-                }
-                if (isUnknownField)
-                {
-                    s_logger.Warn($"Unknown field from config: {keyValuePair.Key}");
-                }
-
-                isUnknownField = true;
+                var jsonPropertyAttribute = property.GetCustomAttribute<JsonPropertyAttribute>();
+                knownProperties.Add(jsonPropertyAttribute.PropertyName);
             }
+
+            JObject.Parse(fileContent).GetValue("common")
+                .Cast<JProperty>()
+                .Where(property => knownProperties.Contains(property.Name))
+                .ToList()
+                .ForEach(unknownKey => s_logger.Warn($"Unknown field from config: {unknownKey}"));
         }
     }
 }