[Updating] Add versioning to UpdateState.json (#12744)

- fix "Check for updates" hang if UpdateState.json was deleted while the page is open

src/common/UnitTests-CommonLib/UnitTestsVersionHelper.cpp

@@ -43,79 +43,82 @@ namespace UnitTestsVersionHelper
         }
         TEST_METHOD (stringConstructorShouldProperlyInitializationVersionNumbers)
         {
-            VersionHelper sut("v0.12.3");
+            auto sut = VersionHelper::fromString("v0.12.3");
-
+            Assert::IsTrue(sut.has_value());
-            Assert::AreEqual(0ull, sut.major);
+            Assert::AreEqual(0ull, sut->major);
-            Assert::AreEqual(12ull, sut.minor);
+            Assert::AreEqual(12ull, sut->minor);
-            Assert::AreEqual(3ull, sut.revision);
+            Assert::AreEqual(3ull, sut->revision);
         }
         TEST_METHOD (stringConstructorShouldProperlyInitializationWithDifferentVersionNumbers)
         {
-            VersionHelper sut("v2.25.1");
+            auto sut = VersionHelper::fromString(L"v2.25.1");
+            Assert::IsTrue(sut.has_value());
 
-            Assert::AreEqual(2ull, sut.major);
+            Assert::AreEqual(2ull, sut->major);
-            Assert::AreEqual(25ull, sut.minor);
+            Assert::AreEqual(25ull, sut->minor);
-            Assert::AreEqual(1ull, sut.revision);
+            Assert::AreEqual(1ull, sut->revision);
         }
         TEST_METHOD (emptyStringNotAccepted)
         {
-            Assert::ExpectException<std::logic_error>([] {
+            auto sut = VersionHelper::fromString("");
-                VersionHelper sut("");
+
-            });
+            Assert::IsFalse(sut.has_value());
         }
         TEST_METHOD (invalidStringNotAccepted1)
         {
-            Assert::ExpectException<std::logic_error>([] {
+            auto sut = VersionHelper::fromString(L"v2a.");
-                VersionHelper sut("v2a.");
+
-            });
+            Assert::IsFalse(sut.has_value());
         }
         TEST_METHOD (invalidStringNotAccepted2)
         {
-            Assert::ExpectException<std::logic_error>([] {
+            auto sut = VersionHelper::fromString(L"12abc2vv.0");
-                VersionHelper sut("12abc2vv.0");
+
-            });
+            Assert::IsFalse(sut.has_value());
         }
         TEST_METHOD (invalidStringNotAccepted3)
         {
-            Assert::ExpectException<std::logic_error>([] {
+            auto sut = VersionHelper::fromString("123");
-                VersionHelper sut("123");
+
-            });
+            Assert::IsFalse(sut.has_value());
         }
         TEST_METHOD (invalidStringNotAccepted4)
         {
-            Assert::ExpectException<std::logic_error>([] {
+            auto sut = VersionHelper::fromString(L"v1v2v3");
-                VersionHelper sut("v1v2v3");
+
-            });
+            Assert::IsFalse(sut.has_value());
         }
         TEST_METHOD (invalidStringNotAccepted5)
         {
-            Assert::ExpectException<std::logic_error>([] {
+            auto sut = VersionHelper::fromString("v.1.2.3v");
-                VersionHelper sut("v.1.2.3v");
+
-            });
+            Assert::IsFalse(sut.has_value());
         }
         TEST_METHOD (partialVersionStringNotAccepted1)
         {
-            Assert::ExpectException<std::logic_error>([] {
+            auto sut = VersionHelper::fromString(L"v1.");
-                VersionHelper sut("v1.");
+
-            });
+            Assert::IsFalse(sut.has_value());
         }
         TEST_METHOD (partialVersionStringNotAccepted2)
         {
-            Assert::ExpectException<std::logic_error>([] {
+            auto sut = VersionHelper::fromString("v1.2");
-                VersionHelper sut("v1.2");
+
-            });
+            Assert::IsFalse(sut.has_value());
         }
         TEST_METHOD (partialVersionStringNotAccepted3)
         {
-            Assert::ExpectException<std::logic_error>([] {
+            auto sut = VersionHelper::fromString(L"v1.2.");
-                VersionHelper sut("v1.2.");
+
-            });
+            Assert::IsFalse(sut.has_value());
         }
         TEST_METHOD (parsedWithoutLeadingV)
         {
             VersionHelper expected{ 12ull, 13ull, 111ull };
-            VersionHelper actual("12.13.111");
+            auto actual = VersionHelper::fromString(L"12.13.111");
-            Assert::AreEqual(actual, expected);
+
+            Assert::IsTrue(actual.has_value());
+            Assert::AreEqual(*actual, expected);
         }
         TEST_METHOD (whenMajorVersionIsGreaterComparisonOperatorShouldReturnProperValue)
         {

src/common/updating/pch.h

@@ -35,3 +35,4 @@
 
 #endif //PCH_H
 
+namespace fs = std::filesystem;

src/common/updating/updateState.cpp

@@ -3,12 +3,15 @@
 
 #include <common/utils/json.h>
 #include <common/utils/timeutil.h>
+#include <common/version/helper.h>
+#include <common/version/version.h>
 #include <common/SettingsAPI/settings_helpers.h>
 
 namespace
 {
     const wchar_t PERSISTENT_STATE_FILENAME[] = L"\\UpdateState.json";
     const wchar_t UPDATE_STATE_MUTEX[] = L"Local\\PowerToysRunnerUpdateStateMutex";
+    const VersionHelper CURRENT_VERSION(VERSION_MAJOR, VERSION_MINOR, VERSION_REVISION);
 }
 
 UpdateState deserialize(const json::JsonObject& json)
@@ -34,19 +37,40 @@ json::JsonObject serialize(const UpdateState& state)
     json.SetNamedValue(L"state", json::value(static_cast<double>(state.state)));
     json.SetNamedValue(L"downloadedInstallerFilename", json::value(state.downloadedInstallerFilename));
 
+    json.SetNamedValue(L"updateStateFileVersion", json::value(CURRENT_VERSION.toWstring()));
+
     return json;
 }
 
+bool IsOldFileVersion(const std::wstring_view fileVersion)
+{
+    if (fileVersion == L"")
+    {
+        return true;
+    }
+
+    const auto parsedVer = VersionHelper::fromString(fileVersion);
+    return !parsedVer.has_value() || *parsedVer != CURRENT_VERSION;
+}
+
 UpdateState UpdateState::read()
 {
     const auto filename = PTSettingsHelper::get_root_save_folder_location() + PERSISTENT_STATE_FILENAME;
     std::optional<json::JsonObject> json;
+    wil::unique_mutex_nothrow mutex{ CreateMutexW(nullptr, FALSE, UPDATE_STATE_MUTEX) };
+    auto lock = mutex.acquire();
+    json = json::from_file(filename);
+
+    if (json.has_value() && !IsOldFileVersion(json->GetNamedString(L"updateStateFileVersion", L"").c_str()))
     {
-        wil::unique_mutex_nothrow mutex{ CreateMutexW(nullptr, FALSE, UPDATE_STATE_MUTEX) };
+        return deserialize(*json);
-        auto lock = mutex.acquire();
+    }
-        json = json::from_file(filename);
+    else
+    {
+        std::error_code _;
+        fs::remove(filename, _);
+        return UpdateState{};
     }
-    return json ? deserialize(*json) : UpdateState{};
 }
 
 void UpdateState::store(std::function<void(UpdateState&)> stateModifier)

src/common/updating/updating.cpp

@@ -36,14 +36,7 @@ namespace updating
 
     std::optional<VersionHelper> extract_version_from_release_object(const json::JsonObject& release_object)
     {
-        try
+        return VersionHelper::fromString(release_object.GetNamedString(L"tag_name"));
-        {
-            return VersionHelper{ winrt::to_string(release_object.GetNamedString(L"tag_name")) };
-        }
-        catch (...)
-        {
-        }
-        return std::nullopt;
     }
 
     std::pair<Uri, std::wstring> extract_installer_asset_download_info(const json::JsonObject& release_object)

src/common/utils/string_utils.h

@@ -22,28 +22,34 @@ struct default_trim_arg<wchar_t>
 };
 
 template<typename CharT>
-inline std::basic_string_view<CharT> left_trim(std::basic_string_view<CharT> s, const std::basic_string_view<CharT> chars_to_trim = default_trim_arg<CharT>::value)
+inline std::basic_string_view<CharT> left_trim(std::basic_string_view<CharT> s,
+                                               const std::basic_string_view<CharT> chars_to_trim = default_trim_arg<CharT>::value)
 {
     s.remove_prefix(std::min<size_t>(s.find_first_not_of(chars_to_trim), size(s)));
     return s;
 }
 
 template<typename CharT>
-inline std::basic_string_view<CharT> right_trim(std::basic_string_view<CharT> s, const std::basic_string_view<CharT> chars_to_trim = default_trim_arg<CharT>::value)
+inline std::basic_string_view<CharT> right_trim(std::basic_string_view<CharT> s,
+                                                const std::basic_string_view<CharT> chars_to_trim = default_trim_arg<CharT>::value)
 {
     s.remove_suffix(std::min<size_t>(size(s) - s.find_last_not_of(chars_to_trim) - 1, size(s)));
     return s;
 }
 
 template<typename CharT>
-inline std::basic_string_view<CharT> trim(std::basic_string_view<CharT> s, const std::basic_string_view<CharT> chars_to_trim = default_trim_arg<CharT>::value)
+inline std::basic_string_view<CharT> trim(std::basic_string_view<CharT> s,
+                                          const std::basic_string_view<CharT> chars_to_trim = default_trim_arg<CharT>::value)
 {
     return left_trim(right_trim(s, chars_to_trim), chars_to_trim);
 }
 
-inline void replace_chars(std::string& s, const std::string_view chars_to_replace, const char replacement_char)
+template<typename CharT>
+inline void replace_chars(std::basic_string<CharT>& s,
+                          const std::basic_string_view<CharT> chars_to_replace,
+                          const CharT replacement_char)
 {
-    for (const char c : chars_to_replace)
+    for (const CharT c : chars_to_replace)
     {
         std::replace(begin(s), end(s), c, replacement_char);
     }

src/common/version/helper.cpp

@@ -5,23 +5,6 @@
 #include <algorithm>
 #include <sstream>
 
-VersionHelper::VersionHelper(std::string str)
-{
-    // Remove whitespaces chars and a leading 'v'
-    str = left_trim<char>(trim<char>(str), "v");
-    // Replace '.' with spaces
-    replace_chars(str, ".", ' ');
-
-    std::istringstream ss{ str };
-    ss >> major;
-    ss >> minor;
-    ss >> revision;
-    if (ss.fail() || !ss.eof())
-    {
-        throw std::logic_error("VersionHelper: couldn't parse the supplied version string");
-    }
-}
-
 VersionHelper::VersionHelper(const size_t major, const size_t minor, const size_t revision) :
     major{ major },
     minor{ minor },
@@ -29,6 +12,60 @@ VersionHelper::VersionHelper(const size_t major, const size_t minor, const size_
 {
 }
 
+template<typename CharT>
+struct Constants;
+
+template<>
+struct Constants<char>
+{
+    static inline const char* V = "v";
+    static inline const char* DOT = ".";
+    static inline const char SPACE = ' ';
+};
+
+template<>
+struct Constants<wchar_t>
+{
+    static inline const wchar_t* V = L"v";
+    static inline const wchar_t* DOT = L".";
+    static inline const wchar_t SPACE = L' ';
+};
+
+template<typename CharT>
+std::optional<VersionHelper> fromString(std::basic_string_view<CharT> str)
+{
+    try
+    {
+        str = left_trim<CharT>(trim<CharT>(str), Constants<CharT>::V);
+        std::basic_string<CharT> spacedStr{ str };
+        replace_chars<CharT>(spacedStr, Constants<CharT>::DOT, Constants<CharT>::SPACE);
+
+        std::basic_istringstream<CharT> ss{ spacedStr };
+        VersionHelper result{ 0, 0, 0 };
+        ss >> result.major;
+        ss >> result.minor;
+        ss >> result.revision;
+        if (!ss.fail() && ss.eof())
+        {
+            return result;
+        }
+    }
+    catch (...)
+    {
+    }
+    return std::nullopt;
+}
+
+std::optional<VersionHelper> VersionHelper::fromString(std::string_view s)
+{
+    return ::fromString(s);
+}
+
+std::optional<VersionHelper> VersionHelper::fromString(std::wstring_view s)
+{
+    return ::fromString(s);
+}
+
 std::wstring VersionHelper::toWstring() const
 {
     std::wstring result{ L"v" };

src/common/version/helper.h

@@ -1,14 +1,17 @@
 #pragma once
 
 #include <string>
+#include <optional>
 #include <compare>
 
 struct VersionHelper
 {
-    VersionHelper(std::string str);
     VersionHelper(const size_t major, const size_t minor, const size_t revision);
 
     auto operator<=>(const VersionHelper&) const = default;
+    
+    static std::optional<VersionHelper> fromString(std::string_view s);
+    static std::optional<VersionHelper> fromString(std::wstring_view s);
 
     size_t major;
     size_t minor;

src/settings-ui/Microsoft.PowerToys.Settings.UI.Library/ViewModels/GeneralViewModel.cs

@@ -466,6 +466,7 @@ namespace Microsoft.PowerToys.Settings.UI.Library.ViewModels
         // callback function to launch the URL to check for updates.
         private void CheckForUpdatesClick()
         {
+            RefreshUpdatingState();
             IsNewVersionDownloading = string.IsNullOrEmpty(UpdatingSettingsConfig.DownloadedInstallerFilename);
             NotifyPropertyChanged(nameof(IsDownloadAllowed));