Merge pull request #312 from github/fixes/settings

shana · web-flow · commit 2cd9aef5700b · 2017-05-08T20:17:03.000+02:00
Fix to initialize settings and store EnableTraceLogging
diff --git a/src/GitHub.Api/AppConfiguration.cs b/src/GitHub.Api/AppConfiguration.cs
@@ -8,27 +8,11 @@ class AppConfiguration : IAppConfiguration
     {
         public AppConfiguration()
         {
-            applicationName = ApplicationInfo.ApplicationName;
-            applicationDescription = ApplicationInfo.ApplicationDescription;
-
             var executingAssembly = typeof(AppConfiguration).Assembly;
             AssemblyName = executingAssembly.GetName();
             ProductHeader = new ProductHeaderValue(ApplicationInfo.ApplicationSafeName, AssemblyName.Version.ToString());
         }
 
-        readonly string applicationName;
-        readonly string applicationDescription;
-
-        /// <summary>
-        /// Name of this application
-        /// </summary>
-        public string ApplicationName { get { return applicationName; } }
-
-        /// <summary>
-        /// Name of this application
-        /// </summary>
-        public string ApplicationDescription { get { return applicationDescription; } }
-
         /// <summary>
         /// The currently executing assembly.
         /// </summary>
diff --git a/src/GitHub.Api/ApplicationManagerBase.cs b/src/GitHub.Api/ApplicationManagerBase.cs
@@ -26,8 +26,16 @@ protected void Initialize(IUIDispatcher uiDispatcher)
         {
             // accessing Environment triggers environment initialization if it hasn't happened yet
             LocalSettings = new LocalSettings(Environment);
-            UserSettings = new UserSettings(Environment, ApplicationInfo.ApplicationName);
-            SystemSettings = new SystemSettings(Environment, ApplicationInfo.ApplicationName);
+            UserSettings = new UserSettings(Environment);
+            SystemSettings = new SystemSettings(Environment);
+
+            LocalSettings.Initialize();
+
+            UserSettings.Initialize();
+
+            Logging.TracingEnabled = UserSettings.Get("EnableTraceLogging", false);
+
+            SystemSettings.Initialize();
 
             Platform = new Platform(Environment, FileSystem, uiDispatcher);
             ProcessManager = new ProcessManager(Environment, Platform.GitEnvironment, CancellationToken);
diff --git a/src/GitHub.Api/Authentication/Keychain.cs b/src/GitHub.Api/Authentication/Keychain.cs
@@ -28,6 +28,8 @@ class ConnectionCacheItem
     /// </summary>
     class Keychain : IKeychain
     {
+        const string ConnectionFile = "connections.json";
+
         private readonly ILogging logger = Logging.GetLogger<Keychain>();
 
         private readonly ICredentialManager credentialManager;
@@ -42,7 +44,7 @@ class Keychain : IKeychain
 
         public Keychain(IEnvironment environment, ICredentialManager credentialManager)
         {
-            cachePath = environment.ConnectionCachePath;
+            cachePath = environment.UserCachePath.Combine(ConnectionFile);
             this.credentialManager = credentialManager;
         }
 
diff --git a/src/GitHub.Api/DefaultEnvironment.cs b/src/GitHub.Api/DefaultEnvironment.cs
@@ -26,7 +26,8 @@ public string GetEnvironmentVariable(string variable)
         public string UnityAssetsPath { get; set; }
         public string UnityProjectPath { get; set; }
         public string ExtensionInstallPath { get; set; }
-        public NPath ConnectionCachePath { get; set; }
+        public NPath UserCachePath { get; set; }
+        public NPath SystemCachePath { get; set; }
         public string UserProfilePath { get { return Environment.GetEnvironmentVariable("USERPROFILE"); } }
         public string Path { get { return Environment.GetEnvironmentVariable("PATH"); } }
         public string NewLine { get { return Environment.NewLine; } }
diff --git a/src/GitHub.Api/IAppConfiguration.cs b/src/GitHub.Api/IAppConfiguration.cs
@@ -6,8 +6,6 @@ namespace GitHub.Unity
     // Represents the currently executing program.
     interface IAppConfiguration
     {
-        string ApplicationName { get; }
-        string ApplicationDescription { get; }
         AssemblyName AssemblyName { get; }
         ProductHeaderValue ProductHeader { get; }
     }
diff --git a/src/GitHub.Api/IEnvironment.cs b/src/GitHub.Api/IEnvironment.cs
@@ -19,9 +19,10 @@ interface IEnvironment
         string UnityAssetsPath { get; set; }
         string UnityProjectPath { get; set; }
         string ExtensionInstallPath { get; set; }
-        NPath ConnectionCachePath { get; set; }
+        NPath UserCachePath { get; set; }
         string RepositoryPath { get; }
         string GitInstallPath { get; }
         IRepository Repository { get; set; }
+        NPath SystemCachePath { get; set; }
     }
 }
diff --git a/src/GitHub.Api/LocalSettings.cs b/src/GitHub.Api/LocalSettings.cs
@@ -16,15 +16,11 @@ abstract class BaseSettings : ISettings
         public abstract void Unset(string key);
 
         protected virtual string SettingsFileName { get; set; }
-        protected virtual string SettingsPath { get; set; }
+        protected virtual NPath SettingsPath { get; set; }
     }
 
     class JsonBackedSettings : BaseSettings
     {
-        private static readonly ILogging logger = Logging.GetLogger<JsonBackedSettings>();
-
-        private const string SettingsParseError = "Failed to parse settings file at '{0}'";
-
         private string cachePath;
         private CacheData cacheData = new CacheData();
         private Action<string> dirCreate;
@@ -33,9 +29,11 @@ class JsonBackedSettings : BaseSettings
         private Func<string, bool> fileExists;
         private Func<string, Encoding, string> readAllText;
         private Action<string, string> writeAllText;
+        private readonly ILogging logger;
 
         public JsonBackedSettings()
         {
+            logger = Logging.GetLogger(GetType());
             fileExists = (path) => File.Exists(path);
             readAllText = (path, encoding) => File.ReadAllText(path, encoding);
             writeAllText = (path, content) => File.WriteAllText(path, content);
@@ -46,7 +44,7 @@ public JsonBackedSettings()
 
         public override void Initialize()
         {
-            cachePath = Path.Combine(SettingsPath, SettingsFileName);
+            cachePath = SettingsPath.Combine(SettingsFileName);
             LoadFromCache(cachePath);
         }
 
@@ -100,6 +98,8 @@ public override void Rename(string oldKey, string newKey)
 
         private void LoadFromCache(string cachePath)
         {
+            logger.Trace("LoadFromCache: {0}", cachePath);
+
             EnsureCachePath(cachePath);
 
             if (!fileExists(cachePath))
@@ -111,8 +111,9 @@ private void LoadFromCache(string cachePath)
             {
                 cacheData = SimpleJson.DeserializeObject<CacheData>(data);
             }
-            catch
+            catch(Exception ex)
             {
+                logger.Error(ex, "LoadFromCache Error");
                 cacheData = null;
             }
 
@@ -126,6 +127,8 @@ private void LoadFromCache(string cachePath)
 
         private bool SaveToCache(string cachePath)
         {
+            logger.Trace("SaveToCache: {0}", cachePath);
+
             EnsureCachePath(cachePath);
 
             try
@@ -135,8 +138,7 @@ private bool SaveToCache(string cachePath)
             }
             catch (Exception ex)
             {
-                logger.Error(SettingsParseError, cachePath);
-                logger.Error(ex);
+                logger.Error(ex, "SaveToCache Error");
                 return false;
             }
 
@@ -155,7 +157,7 @@ private void EnsureCachePath(string cachePath)
 
         private class CacheData
         {
-            public Dictionary<string, object> GitHubUnity = new Dictionary<string, object>();
+            public Dictionary<string, object> GitHubUnity { get; set; } = new Dictionary<string, object>();
         }
 
     }
@@ -177,9 +179,9 @@ class UserSettings : JsonBackedSettings
     {
         private const string settingsFileName = "settings.json";
 
-        public UserSettings(IEnvironment environment, string path)
+        public UserSettings(IEnvironment environment)
         {
-            SettingsPath = environment.GetSpecialFolder(Environment.SpecialFolder.LocalApplicationData).ToNPath().Combine(path);
+            SettingsPath = environment.UserCachePath;
         }
 
         protected override string SettingsFileName { get { return settingsFileName; } }
@@ -189,9 +191,9 @@ class SystemSettings : JsonBackedSettings
     {
         private const string settingsFileName = "settings.json";
 
-        public SystemSettings(IEnvironment environment, string path)
+        public SystemSettings(IEnvironment environment)
         {
-            SettingsPath = environment.GetSpecialFolder(Environment.SpecialFolder.ApplicationData).ToNPath().Combine(path);
+            SettingsPath = environment.SystemCachePath;
         }
 
         protected override string SettingsFileName { get { return settingsFileName; } }
diff --git a/src/GitHub.Logging/Logging.cs b/src/GitHub.Logging/Logging.cs
@@ -4,13 +4,6 @@ namespace GitHub.Unity
 {
     public static class Logging
     {
-        static Logging()
-        {
-#if DEVELOPER_BUILD
-            tracingEnabled = true;
-#endif
-        }
-
         private static bool tracingEnabled;
 
         public static bool TracingEnabled
diff --git a/src/IntegrationTests/Git/IntegrationTestEnvironment.cs b/src/IntegrationTests/Git/IntegrationTestEnvironment.cs
@@ -108,11 +108,20 @@ public string ExtensionInstallPath
             set { throw new NotImplementedException(); }
         }
 
-        public NPath ConnectionCachePath
+        public NPath UserCachePath
         {
             get
             {
-                return GetSpecialFolder(Environment.SpecialFolder.LocalApplicationData).ToNPath().Combine("connections.json");
+                return GetSpecialFolder(Environment.SpecialFolder.LocalApplicationData).ToNPath().Combine(ApplicationInfo.ApplicationName);
+            }
+            set { throw new NotImplementedException(); }
+        }
+
+        public NPath SystemCachePath
+        {
+            get
+            {
+                return GetSpecialFolder(Environment.SpecialFolder.CommonApplicationData).ToNPath().Combine(ApplicationInfo.ApplicationName);
             }
             set { throw new NotImplementedException(); }
         }
diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/ApplicationManager.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/ApplicationManager.cs
@@ -84,9 +84,12 @@ protected override void InitializeEnvironment()
             Environment.UnityAssetsPath = assetsPath.ToString(SlashMode.Forward);
             Environment.UnityProjectPath = projectPath.ToString(SlashMode.Forward);
 
-            Environment.ConnectionCachePath = Environment.GetSpecialFolder(System.Environment.SpecialFolder.LocalApplicationData)
+            Environment.UserCachePath = Environment.GetSpecialFolder(System.Environment.SpecialFolder.LocalApplicationData)
                                              .ToNPath()
-                                             .Combine(AppConfiguration.ApplicationName, "connections.json");
+                                             .Combine(ApplicationInfo.ApplicationName);
+            Environment.SystemCachePath = Environment.GetSpecialFolder(System.Environment.SpecialFolder.CommonApplicationData)
+                                             .ToNPath()
+                                             .Combine(ApplicationInfo.ApplicationName);
 
             base.InitializeEnvironment();
         }
diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/SettingsView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/SettingsView.cs
@@ -676,6 +676,8 @@ private void OnLoggingSettingsGui()
             if (EditorGUI.EndChangeCheck())
             {
                 Logging.TracingEnabled = traceLogging;
+                EntryPoint.AppManager.UserSettings.Set("EnableTraceLogging", traceLogging);
+
                 GUI.FocusControl(null);
             }
 

Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,8 @@ class ConnectionCacheItem`
`28`	`28`	`/// </summary>`
`29`	`29`	`class Keychain : IKeychain`
`30`	`30`	`{`
	`31`	`+ const string ConnectionFile = "connections.json";`
	`32`	`+`
`31`	`33`	`private readonly ILogging logger = Logging.GetLogger<Keychain>();`
`32`	`34`
`33`	`35`	`private readonly ICredentialManager credentialManager;`
`@@ -42,7 +44,7 @@ class Keychain : IKeychain`
`42`	`44`
`43`	`45`	`public Keychain(IEnvironment environment, ICredentialManager credentialManager)`
`44`	`46`	`{`
`45`		`- cachePath = environment.ConnectionCachePath;`
	`47`	`+ cachePath = environment.UserCachePath.Combine(ConnectionFile);`
`46`	`48`	`this.credentialManager = credentialManager;`
`47`	`49`	`}`
`48`	`50`
Original file line number	Diff line number	Diff line change
`@@ -6,8 +6,6 @@ namespace GitHub.Unity`
`6`	`6`	`// Represents the currently executing program.`
`7`	`7`	`interface IAppConfiguration`
`8`	`8`	`{`
`9`		`- string ApplicationName { get; }`
`10`		`- string ApplicationDescription { get; }`
`11`	`9`	`AssemblyName AssemblyName { get; }`
`12`	`10`	`ProductHeaderValue ProductHeader { get; }`
`13`	`11`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,9 +19,10 @@ interface IEnvironment`
`19`	`19`	`string UnityAssetsPath { get; set; }`
`20`	`20`	`string UnityProjectPath { get; set; }`
`21`	`21`	`string ExtensionInstallPath { get; set; }`
`22`		`- NPath ConnectionCachePath { get; set; }`
	`22`	`+ NPath UserCachePath { get; set; }`
`23`	`23`	`string RepositoryPath { get; }`
`24`	`24`	`string GitInstallPath { get; }`
`25`	`25`	`IRepository Repository { get; set; }`
	`26`	`+ NPath SystemCachePath { get; set; }`
`26`	`27`	`}`
`27`	`28`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,15 +16,11 @@ abstract class BaseSettings : ISettings`
`16`	`16`	`public abstract void Unset(string key);`
`17`	`17`
`18`	`18`	`protected virtual string SettingsFileName { get; set; }`
`19`		`- protected virtual string SettingsPath { get; set; }`
	`19`	`+ protected virtual NPath SettingsPath { get; set; }`
`20`	`20`	`}`
`21`	`21`
`22`	`22`	`class JsonBackedSettings : BaseSettings`
`23`	`23`	`{`
`24`		`- private static readonly ILogging logger = Logging.GetLogger<JsonBackedSettings>();`
`25`		`-`
`26`		`- private const string SettingsParseError = "Failed to parse settings file at '{0}'";`
`27`		`-`
`28`	`24`	`private string cachePath;`
`29`	`25`	`private CacheData cacheData = new CacheData();`
`30`	`26`	`private Action<string> dirCreate;`
`@@ -33,9 +29,11 @@ class JsonBackedSettings : BaseSettings`
`33`	`29`	`private Func<string, bool> fileExists;`
`34`	`30`	`private Func<string, Encoding, string> readAllText;`
`35`	`31`	`private Action<string, string> writeAllText;`
	`32`	`+ private readonly ILogging logger;`
`36`	`33`
`37`	`34`	`public JsonBackedSettings()`
`38`	`35`	`{`
	`36`	`+ logger = Logging.GetLogger(GetType());`
`39`	`37`	`fileExists = (path) => File.Exists(path);`
`40`	`38`	`readAllText = (path, encoding) => File.ReadAllText(path, encoding);`
`41`	`39`	`writeAllText = (path, content) => File.WriteAllText(path, content);`
`@@ -46,7 +44,7 @@ public JsonBackedSettings()`
`46`	`44`
`47`	`45`	`public override void Initialize()`
`48`	`46`	`{`
`49`		`- cachePath = Path.Combine(SettingsPath, SettingsFileName);`
	`47`	`+ cachePath = SettingsPath.Combine(SettingsFileName);`
`50`	`48`	`LoadFromCache(cachePath);`
`51`	`49`	`}`
`52`	`50`
`@@ -100,6 +98,8 @@ public override void Rename(string oldKey, string newKey)`
`100`	`98`
`101`	`99`	`private void LoadFromCache(string cachePath)`
`102`	`100`	`{`
	`101`	`+ logger.Trace("LoadFromCache: {0}", cachePath);`
	`102`	`+`
`103`	`103`	`EnsureCachePath(cachePath);`
`104`	`104`
`105`	`105`	`if (!fileExists(cachePath))`
`@@ -111,8 +111,9 @@ private void LoadFromCache(string cachePath)`
`111`	`111`	`{`
`112`	`112`	`cacheData = SimpleJson.DeserializeObject<CacheData>(data);`
`113`	`113`	`}`
`114`		`- catch`
	`114`	`+ catch(Exception ex)`
`115`	`115`	`{`
	`116`	`+ logger.Error(ex, "LoadFromCache Error");`
`116`	`117`	`cacheData = null;`
`117`	`118`	`}`
`118`	`119`
`@@ -126,6 +127,8 @@ private void LoadFromCache(string cachePath)`
`126`	`127`
`127`	`128`	`private bool SaveToCache(string cachePath)`
`128`	`129`	`{`
	`130`	`+ logger.Trace("SaveToCache: {0}", cachePath);`
	`131`	`+`
`129`	`132`	`EnsureCachePath(cachePath);`
`130`	`133`
`131`	`134`	`try`
`@@ -135,8 +138,7 @@ private bool SaveToCache(string cachePath)`
`135`	`138`	`}`
`136`	`139`	`catch (Exception ex)`
`137`	`140`	`{`
`138`		`- logger.Error(SettingsParseError, cachePath);`
`139`		`- logger.Error(ex);`
	`141`	`+ logger.Error(ex, "SaveToCache Error");`
`140`	`142`	`return false;`
`141`	`143`	`}`
`142`	`144`
`@@ -155,7 +157,7 @@ private void EnsureCachePath(string cachePath)`
`155`	`157`
`156`	`158`	`private class CacheData`
`157`	`159`	`{`
`158`		`- public Dictionary<string, object> GitHubUnity = new Dictionary<string, object>();`
	`160`	`+ public Dictionary<string, object> GitHubUnity { get; set; } = new Dictionary<string, object>();`
`159`	`161`	`}`
`160`	`162`
`161`	`163`	`}`
`@@ -177,9 +179,9 @@ class UserSettings : JsonBackedSettings`
`177`	`179`	`{`
`178`	`180`	`private const string settingsFileName = "settings.json";`
`179`	`181`
`180`		`- public UserSettings(IEnvironment environment, string path)`
	`182`	`+ public UserSettings(IEnvironment environment)`
`181`	`183`	`{`
`182`		`- SettingsPath = environment.GetSpecialFolder(Environment.SpecialFolder.LocalApplicationData).ToNPath().Combine(path);`
	`184`	`+ SettingsPath = environment.UserCachePath;`
`183`	`185`	`}`
`184`	`186`
`185`	`187`	`protected override string SettingsFileName { get { return settingsFileName; } }`
`@@ -189,9 +191,9 @@ class SystemSettings : JsonBackedSettings`
`189`	`191`	`{`
`190`	`192`	`private const string settingsFileName = "settings.json";`
`191`	`193`
`192`		`- public SystemSettings(IEnvironment environment, string path)`
	`194`	`+ public SystemSettings(IEnvironment environment)`
`193`	`195`	`{`
`194`		`- SettingsPath = environment.GetSpecialFolder(Environment.SpecialFolder.ApplicationData).ToNPath().Combine(path);`
	`196`	`+ SettingsPath = environment.SystemCachePath;`
`195`	`197`	`}`
`196`	`198`
`197`	`199`	`protected override string SettingsFileName { get { return settingsFileName; } }`
Original file line number	Diff line number	Diff line change
`@@ -4,13 +4,6 @@ namespace GitHub.Unity`
`4`	`4`	`{`
`5`	`5`	`public static class Logging`
`6`	`6`	`{`
`7`		`- static Logging()`
`8`		`- {`
`9`		`-#if DEVELOPER_BUILD`
`10`		`- tracingEnabled = true;`
`11`		`-#endif`
`12`		`- }`
`13`		`-`
`14`	`7`	`private static bool tracingEnabled;`
`15`	`8`
`16`	`9`	`public static bool TracingEnabled`
Original file line number	Diff line number	Diff line change
`@@ -108,11 +108,20 @@ public string ExtensionInstallPath`
`108`	`108`	`set { throw new NotImplementedException(); }`
`109`	`109`	`}`
`110`	`110`
`111`		`- public NPath ConnectionCachePath`
	`111`	`+ public NPath UserCachePath`
`112`	`112`	`{`
`113`	`113`	`get`
`114`	`114`	`{`
`115`		`- return GetSpecialFolder(Environment.SpecialFolder.LocalApplicationData).ToNPath().Combine("connections.json");`
	`115`	`+ return GetSpecialFolder(Environment.SpecialFolder.LocalApplicationData).ToNPath().Combine(ApplicationInfo.ApplicationName);`
	`116`	`+ }`
	`117`	`+ set { throw new NotImplementedException(); }`
	`118`	`+ }`
	`119`	`+`
	`120`	`+ public NPath SystemCachePath`
	`121`	`+ {`
	`122`	`+ get`
	`123`	`+ {`
	`124`	`+ return GetSpecialFolder(Environment.SpecialFolder.CommonApplicationData).ToNPath().Combine(ApplicationInfo.ApplicationName);`
`116`	`125`	`}`
`117`	`126`	`set { throw new NotImplementedException(); }`
`118`	`127`	`}`