Improve Support for Versions with Preambles (#4558)

- [x] This pull request is related to an issue. - #2394
microsoft · Jun 20, 2024 · 38aac87 · 38aac87
1 parent 57907ac
commit 38aac87
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 3 deletions.
diff --git a/src/AppInstallerCLITests/Versions.cpp b/src/AppInstallerCLITests/Versions.cpp
@@ -56,6 +56,19 @@ TEST_CASE("VersionParseWithWhitespace", "[versions]")
     }
 }
 
+TEST_CASE("VersionParseWithPreamble", "[versions]")
+{
+    Version version("v1.2.3.4");
+    const auto& parts = version.GetParts();
+    REQUIRE(parts.size() == 4);
+    for (size_t i = 0; i < parts.size(); ++i)
+    {
+        INFO(i);
+        REQUIRE(parts[i].Integer == static_cast<uint64_t>(i + 1));
+        REQUIRE(parts[i].Other == "");
+    }
+}
+
 TEST_CASE("VersionParseCorner", "[versions]")
 {
     Version version1("");
@@ -91,6 +104,14 @@ TEST_CASE("VersionParseCorner", "[versions]")
     REQUIRE(parts[0].Other == "");
     REQUIRE(parts[1].Integer == 1);
     REQUIRE(parts[1].Other == "");
+
+    Version version7("v1.2a");
+    parts = version7.GetParts();
+    REQUIRE(parts.size() == 2);
+    REQUIRE(parts[0].Integer == 1);
+    REQUIRE(parts[0].Other == "");
+    REQUIRE(parts[1].Integer == 2);
+    REQUIRE(parts[1].Other == "a");
 }
 
 void RequireLessThan(std::string_view a, std::string_view b)
@@ -134,9 +155,18 @@ TEST_CASE("VersionCompare", "[versions]")
     RequireLessThan("13.9.8", "14.1");
 
     RequireEqual("1.0", "1.0.0");
+
     // Ensure whitespace doesn't affect equality
     RequireEqual("1.0", "1.0 ");
     RequireEqual("1.0", "1. 0");
+
+    // Ensure versions with preambles are sorted correctly
+    RequireEqual("1.0", "Version 1.0");
+    RequireEqual("foo1", "bar1");
+    RequireLessThan("v0.0.1", "0.0.2");
+    RequireLessThan("v0.0.1", "v0.0.2");
+    RequireLessThan("1.a2", "1.b1");
+    RequireLessThan("alpha", "beta");
 }
 
 TEST_CASE("VersionAndChannelSort", "[versions]")

diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp
@@ -80,7 +80,7 @@ namespace AppInstaller::Manifest
                 }
             }
 
-            for (const Version& ext : m_extensions)
+            for (const RawVersion& ext : m_extensions)
             {
                 if (ext.GetParts().empty() || ext.GetParts()[0].Integer != 0)
                 {
@@ -105,7 +105,7 @@ namespace AppInstaller::Manifest
 
     bool ManifestVer::HasExtension(std::string_view extension) const
     {
-        for (const Version& ext : m_extensions)
+        for (const RawVersion& ext : m_extensions)
         {
             const auto& parts = ext.GetParts();
             if (!parts.empty() && parts[0].Integer == 0 && parts[0].Other == extension)

diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h
@@ -15,6 +15,7 @@ namespace AppInstaller::Manifest
 
     using string_t = Utility::NormalizedString;
     using namespace std::string_view_literals;
+    using Utility::RawVersion;
 
     // The maximum supported major version known about by this code.
     constexpr uint64_t s_MaxSupportedMajorVersion = 1;
@@ -77,7 +78,7 @@ namespace AppInstaller::Manifest
         bool HasExtension(std::string_view extension) const;
 
     private:
-        std::vector<Version> m_extensions;
+        std::vector<RawVersion> m_extensions;
     };
 
     enum class InstallerTypeEnum

diff --git a/src/AppInstallerSharedLib/Public/AppInstallerVersions.h b/src/AppInstallerSharedLib/Public/AppInstallerVersions.h
@@ -119,12 +119,22 @@ namespace AppInstaller::Utility
 
         std::string m_version;
         std::vector<Part> m_parts;
+        bool m_trimPrefix = true;
         ApproximateComparator m_approximateComparator = ApproximateComparator::None;
 
       // Remove trailing empty parts (0 or empty)
         void Trim();
     };
 
+    // Version that does not have leading non-digit characters trimmed
+    struct RawVersion : protected Version
+    {
+        RawVersion() { m_trimPrefix = false; }
+        RawVersion(std::string version, std::string_view splitChars = DefaultSplitChars);
+
+        using Version::GetParts;
+    };
+
     // Four parts version number: 16-bits.16-bits.16-bits.16-bits
     struct UInt64Version : public Version
     {

diff --git a/src/AppInstallerSharedLib/Versions.cpp b/src/AppInstallerSharedLib/Versions.cpp
@@ -8,6 +8,7 @@ namespace AppInstaller::Utility
 {
     using namespace std::string_view_literals;
 
+    static constexpr std::string_view s_Digit_Characters = "0123456789"sv;
     static constexpr std::string_view s_Version_Part_Latest = "Latest"sv;
     static constexpr std::string_view s_Version_Part_Unknown = "Unknown"sv;
 
@@ -19,6 +20,12 @@ namespace AppInstaller::Utility
         Assign(std::move(version), splitChars);
     }
 
+    RawVersion::RawVersion(std::string version, std::string_view splitChars)
+    {
+        m_trimPrefix = false;
+        Assign(std::move(version), splitChars);
+    }
+
     Version::Version(Version baseVersion, ApproximateComparator approximateComparator) : Version(std::move(baseVersion))
     {
         if (approximateComparator == ApproximateComparator::None)
@@ -56,6 +63,14 @@ namespace AppInstaller::Utility
             baseVersion = m_version.substr(s_Approximate_Greater_Than.length(), m_version.length() - s_Approximate_Greater_Than.length());
         }
 
+        // If there is a digit before the split character, or no split characters exist, trim off all leading non-digit characters
+        size_t digitPos = baseVersion.find_first_of(s_Digit_Characters);
+        size_t splitPos = baseVersion.find_first_of(splitChars);
+        if (m_trimPrefix && digitPos != std::string::npos && (splitPos == std::string::npos || digitPos < splitPos))
+        {
+            baseVersion.erase(0, digitPos);
+        }
+
         // Then parse the base version
         size_t pos = 0;