From fa3d5024513f1709b86cbaf5fdb6a5e81138652c Mon Sep 17 00:00:00 2001
From: David Bennett <dbennett-msft@outlook.com>
Date: Wed, 24 Jan 2024 16:13:55 -0800
Subject: [PATCH] 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
---
 .../Widgets/GitHubRepositoryWidget.cs         | 39 ++++++++++++--
 .../Widgets/GitHubUserWidget.cs               | 51 +++++++++++++++++--
 2 files changed, 81 insertions(+), 9 deletions(-)

diff --git a/src/GitHubExtension/Widgets/GitHubRepositoryWidget.cs b/src/GitHubExtension/Widgets/GitHubRepositoryWidget.cs
index b577bd3..09b9501 100644
--- 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
index fd190a1..d348761 100644
--- 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)
         {