Add data migration for widgets from string to JSON format (#331)

* Add data migration from string to JSON format * Fix node name and remove test condition * Fix migration for non-URL data * Add logging to the json parse failure * Add comment on developer id selection * Add extra paranoia try-catch
microsoft · Jan 25, 2024 · fa3d502 · fa3d502
1 parent 4411a60
commit fa3d502
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 9 deletions.
diff --git a/src/GitHubExtension/Widgets/GitHubRepositoryWidget.cs b/src/GitHubExtension/Widgets/GitHubRepositoryWidget.cs
@@ -2,6 +2,7 @@
 // Licensed under the MIT license.
 
 using System.Text;
+using System.Text.Json;
 using System.Text.Json.Nodes;
 using GitHubExtension.Client;
 using GitHubExtension.DataManager;
@@ -69,16 +70,46 @@ public override void OnActionInvoked(WidgetActionInvokedArgs actionInvokedArgs)
 
     protected override void ResetWidgetInfoFromState()
     {
+        JsonNode? dataObject = null;
+
         try
         {
-            var dataObject = JsonNode.Parse(ConfigurationData);
+            dataObject = JsonNode.Parse(ConfigurationData);
+        }
+        catch (JsonException e)
+        {
+            Log.Logger()?.ReportWarn(Name, ShortId, $"Failed to parse ConfigurationData; attempting migration. {e.Message}");
+            Log.Logger()?.ReportDebug(Name, ShortId, $"Json parse failure.", e);
 
-            if (dataObject == null)
+            try
             {
-                return;
+                // Old data versioning was not a Json string. If we attempt to parse
+                // and we get a failure, check if it is the old version.
+                if (!string.IsNullOrEmpty(ConfigurationData))
+                {
+                    Log.Logger()?.ReportInfo(Name, ShortId, $"Found string data format, migrating to JSON format. Data: {ConfigurationData}");
+                    var migratedState = new JsonObject
+                    {
+                        { "url", ConfigurationData },
+                    };
+                    ConfigurationData = migratedState.ToJsonString();
+                }
+                else
+                {
+                    ConfigurationData = EmptyJson;
+                }
             }
+            catch (Exception ex)
+            {
+                // Adding for abundance of caution because we have seen crashes in this space.
+                Log.Logger()?.ReportError(Name, ShortId, $"Unexpected failure during migration.", ex);
+            }
+        }
 
-            RepositoryUrl = dataObject["url"]?.GetValue<string>() ?? string.Empty;
+        try
+        {
+            dataObject ??= JsonNode.Parse(ConfigurationData);
+            RepositoryUrl = dataObject!["url"]?.GetValue<string>() ?? string.Empty;
         }
         catch (Exception e)
         {

diff --git a/src/GitHubExtension/Widgets/GitHubUserWidget.cs b/src/GitHubExtension/Widgets/GitHubUserWidget.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation and Contributors
 // Licensed under the MIT license.
 
+using System.Text.Json;
 using System.Text.Json.Nodes;
 using GitHubExtension.DataManager;
 using GitHubExtension.DeveloperId;
@@ -57,17 +58,57 @@ public override void OnWidgetContextChanged(WidgetContextChangedArgs contextChan
 
     protected override void ResetWidgetInfoFromState()
     {
+        JsonNode? dataObject = null;
+
         try
         {
-            var dataObject = JsonNode.Parse(ConfigurationData);
+            dataObject = JsonNode.Parse(ConfigurationData);
+        }
+        catch (JsonException e)
+        {
+            Log.Logger()?.ReportWarn(Name, ShortId, $"Failed to parse ConfigurationData; attempting migration. {e.Message}");
+            Log.Logger()?.ReportDebug(Name, ShortId, $"Json parse failure.", e);
 
-            if (dataObject == null)
+            try
             {
-                return;
+                // Old data versioning was not a Json string. If we attempt to parse
+                // and we get a failure, assume it is the old version.
+                if (!string.IsNullOrEmpty(ConfigurationData))
+                {
+                    Log.Logger()?.ReportInfo(Name, ShortId, $"Found string data format, migrating to JSON format. Data: {ConfigurationData}");
+                    var migratedState = new JsonObject
+                    {
+                        { "showCategory", ConfigurationData },
+                    };
+
+                    // Prior to this configuration change, multi-account was not supported. Assume that
+                    // if there is exactly one Developer Id that is the one the user had configured.
+                    // There is no else case here because if we leave this value absent, the widget will
+                    // change state to configuring, where the user can select the developer ID they want.
+                    if (DeveloperIdProvider.GetInstance().GetLoggedInDeveloperIds().DeveloperIds.Count() == 1)
+                    {
+                        migratedState.Add("account", DeveloperIdProvider.GetInstance().GetLoggedInDeveloperIds().DeveloperIds.First().LoginId);
+                    }
+
+                    ConfigurationData = migratedState.ToJsonString();
+                }
+                else
+                {
+                    ConfigurationData = EmptyJson;
+                }
+            }
+            catch (Exception ex)
+            {
+                // Adding for abundance of caution because we have seen crashes in this space.
+                Log.Logger()?.ReportError(Name, ShortId, $"Unexpected failure during migration.", ex);
             }
+        }
 
-            ShowCategory = EnumHelper.StringToSearchCategory(dataObject["showCategory"]?.GetValue<string>() ?? string.Empty);
-            DeveloperLoginId = dataObject["account"]?.GetValue<string>() ?? string.Empty;
+        try
+        {
+            dataObject ??= JsonNode.Parse(ConfigurationData);
+            ShowCategory = EnumHelper.StringToSearchCategory(dataObject!["showCategory"]?.GetValue<string>() ?? string.Empty);
+            DeveloperLoginId = dataObject!["account"]?.GetValue<string>() ?? string.Empty;
         }
         catch (Exception e)
         {