Use AppsAndFeaturesEntries DisplayVersion info for installed package version mapping

src/AppInstallerCLICore/Workflows/UpdateFlow.cpp

@@ -16,7 +16,7 @@ namespace AppInstaller::CLI::Workflow
     {
         bool IsUpdateVersionApplicable(const Utility::Version& installedVersion, const Utility::Version& updateVersion)
         {
-            return (installedVersion < updateVersion || (installedVersion.IsLatest() && updateVersion.IsLatest()));
+            return installedVersion < updateVersion;
         }
 
         void AddToPackagesToInstallIfNotPresent(std::vector<std::unique_ptr<Execution::Context>>& packagesToInstall, std::unique_ptr<Execution::Context> packageContext)

src/AppInstallerCLITests/Versions.cpp

@@ -195,8 +195,8 @@ TEST_CASE("VersionUnknownLessThanLatest", "[versions]")
 TEST_CASE("ApproximateVersionParse", "[versions]")
 {
     Version v1_0{ "1.0" };
-    Version v1_0_LessThan = Version::CreateLessThanApproximateVersion(v1_0);
-    Version v1_0_GreaterThan = Version::CreateGreaterThanApproximateVersion(v1_0);
+    Version v1_0_LessThan{ v1_0, Version::ApproximateComparator::LessThan };
+    Version v1_0_GreaterThan{ v1_0, Version::ApproximateComparator::GreaterThan };
 
     Version v1_0_LessThanFromString = Version{ "< 1.0" };
     Version v1_0_GreaterThanFromString = Version{ "> 1.0" };
@@ -207,6 +207,9 @@ TEST_CASE("ApproximateVersionParse", "[versions]")
 
     REQUIRE(v1_0_LessThan == v1_0_LessThanFromString);
     REQUIRE(v1_0_GreaterThan == v1_0_GreaterThanFromString);
+
+    REQUIRE_THROWS(Version{ v1_0_LessThan, Version::ApproximateComparator::LessThan });
+    REQUIRE_THROWS(Version{ Version::CreateUnknown(), Version::ApproximateComparator::LessThan });
 }
 
 TEST_CASE("ApproximateVersionCompare", "[versions]")
@@ -226,11 +229,6 @@ TEST_CASE("ApproximateVersionCompare", "[versions]")
     RequireLessThan("< latest", "latest");
     RequireLessThan("latest", "> latest");
     RequireLessThan("9999", "< latest");
-
-    // With unknown
-    RequireLessThan("< unknown", "unknown");
-    RequireLessThan("unknown", "> unknown");
-    RequireLessThan("> unknown", "0.0.1");
 }
 
 TEST_CASE("VersionRange", "[versions]")

src/AppInstallerCommonCore/Manifest/Manifest.cpp

@@ -94,7 +94,7 @@ namespace AppInstaller::Manifest
 
         for (auto const& installer : Installers)
         {
-            if (DoesInstallerTypeWriteAppsAndFeaturesEntry(installer.InstallerType) && installer.InstallerType != InstallerTypeEnum::Portable)
+            if (DoesInstallerTypeSupportArpVersionRange(installer.InstallerType))
             {
                 for (auto const& entry : installer.AppsAndFeaturesEntries)
                 {

src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp

@@ -448,6 +448,18 @@ namespace AppInstaller::Manifest
             );
     }
 
+    bool DoesInstallerTypeSupportArpVersionRange(InstallerTypeEnum installerType)
+    {
+        return (
+            installerType == InstallerTypeEnum::Exe ||
+            installerType == InstallerTypeEnum::Inno ||
+            installerType == InstallerTypeEnum::Msi ||
+            installerType == InstallerTypeEnum::Nullsoft ||
+            installerType == InstallerTypeEnum::Wix ||
+            installerType == InstallerTypeEnum::Burn
+            );
+    }
+
     bool IsInstallerTypeCompatible(InstallerTypeEnum type1, InstallerTypeEnum type2)
     {
         // Unknown type cannot be compatible with any other

src/AppInstallerCommonCore/Public/AppInstallerVersions.h

@@ -11,9 +11,10 @@ namespace AppInstaller::Utility
 
     // Creates a comparable version object from a string.
     // Versions are parsed by:
-    //  1. Splitting the string based on the given splitChars (or DefaultSplitChars)
-    //  2. Parsing a leading, positive integer from each split part
-    //  3. Saving any remaining, non-digits as a supplemental value
+    //  1. Parse approximate comparator sign if applicable
+    //  2. Splitting the string based on the given splitChars (or DefaultSplitChars)
+    //  3. Parsing a leading, positive integer from each split part
+    //  4. Saving any remaining, non-digits as a supplemental value
     //
     // Versions are compared by:
     //  for each part in each version
@@ -22,8 +23,20 @@ namespace AppInstaller::Utility
     //      else if integers not equal, return comparison of integers
     //      else if only one side has a non-empty string part, it is less
     //      else if string parts not equal, return comparison of strings
+    //  if all parts are same, use approximate comparator if applicable
+    //
+    //  Note: approximate to another approximate version is invalid.
+    //        approximate to Unknown is invalid.
     struct Version
     {
+        // Used in approximate version to indicate the relation to the base version.
+        enum class ApproximateComparator
+        {
+            None,
+            LessThan,
+            GreaterThan,
+        };
+
         // The default characters to split a version string on.
         constexpr static std::string_view DefaultSplitChars = "."sv;
 
@@ -33,6 +46,9 @@ namespace AppInstaller::Utility
             Version(std::string(version), splitChars) {}
         Version(std::string&& version, std::string_view splitChars = DefaultSplitChars);
 
+        // Constructing an approximate version from a base version.
+        Version(const Version& baseVersion, ApproximateComparator approximateComparator);
+
         // Resets the version's value to the input.
         void Assign(std::string&& version, std::string_view splitChars = DefaultSplitChars);
 
@@ -72,24 +88,12 @@ namespace AppInstaller::Utility
             std::string Other;
         };
 
-        // Used in approximate version to indicate the relation to the base version.
-        enum class ApproximateComparator
-        {
-            None,
-            LessThan,
-            GreaterThan,
-        };
-
         // Gets the part breakdown for a given version; used for tests.
         const std::vector<Part>& GetParts() const { return m_parts; }
 
         // Returns if the version is an approximate version.
         bool IsApproximateVersion() const { return m_approximateComparator != ApproximateComparator::None; }
 
-        // Static methods to create an approximate version from a base version.
-        static Version CreateLessThanApproximateVersion(const Version& base);
-        static Version CreateGreaterThanApproximateVersion(const Version& base);
-
     protected:
 
         bool IsBaseVersionLatest() const;

src/AppInstallerCommonCore/Public/winget/ManifestCommon.h

@@ -274,6 +274,9 @@ bool HasExtension(std::string_view extension) const;
     // Gets a value indicating whether the given installer type writes ARP entry.
     bool DoesInstallerTypeWriteAppsAndFeaturesEntry(InstallerTypeEnum installerType);
 
+    // Gets a value indicating whether the given installer type supports ARP version range.
+    bool DoesInstallerTypeSupportArpVersionRange(InstallerTypeEnum installerType);
+
     // Checks whether 2 installer types are compatible. E.g. inno and exe are update compatible
     bool IsInstallerTypeCompatible(InstallerTypeEnum type1, InstallerTypeEnum type2);
 

src/AppInstallerCommonCore/Public/winget/ManifestValidation.h

@@ -53,7 +53,7 @@ namespace AppInstaller::Manifest
         const char* const ScopeNotSupported = "Scope is not supported for InstallerType portable.";
         const char* const ApproximateVersionNotAllowed = "Approximate version not allowed.";
         const char* const ArpVersionOverlapWithIndex = "DisplayVersion declared in the manifest has overlap with existing DisplayVersion range in the index. Existing DisplayVersion range in index: ";
-        const char* const ArpVersionValidationInternalError = "Encountered internal error during DisplayVersion validation against index.";
+        const char* const ArpVersionValidationInternalError = "Internal error while validating DisplayVersion against index.";
     }
 
     struct ValidationError

src/AppInstallerCommonCore/Versions.cpp

@@ -19,6 +19,26 @@ namespace AppInstaller::Utility
         Assign(std::move(version), splitChars);
     }
 
+    Version::Version(const Version& baseVersion, ApproximateComparator approximateComparator) : Version(baseVersion)
+    {
+        if (approximateComparator == ApproximateComparator::None)
+        {
+            return;
+        }
+
+        THROW_HR_IF(E_INVALIDARG, baseVersion.IsApproximateVersion() || baseVersion.IsUnknown());
+
+        m_approximateComparator = approximateComparator;
+        if (approximateComparator == ApproximateComparator::LessThan)
+        {
+            m_version = std::string{ s_Approximate_Less_Than } + m_version;
+        }
+        else if (approximateComparator == ApproximateComparator::GreaterThan)
+        {
+            m_version = std::string{ s_Approximate_Greater_Than } + m_version;
+        }
+    }
+
     void Version::Assign(std::string&& version, std::string_view splitChars)
     {
         m_version = std::move(version);
@@ -28,12 +48,12 @@ namespace AppInstaller::Utility
         if (CaseInsensitiveStartsWith(m_version, s_Approximate_Less_Than))
         {
             m_approximateComparator = ApproximateComparator::LessThan;
-            baseVersion = m_version.substr(2, m_version.length() - 2);
+            baseVersion = m_version.substr(s_Approximate_Less_Than.length(), m_version.length() - s_Approximate_Less_Than.length());
         }
         else if (CaseInsensitiveStartsWith(m_version, s_Approximate_Greater_Than))
         {
             m_approximateComparator = ApproximateComparator::GreaterThan;
-            baseVersion = m_version.substr(2, m_version.length() - 2);
+            baseVersion = m_version.substr(s_Approximate_Greater_Than.length(), m_version.length() - s_Approximate_Greater_Than.length());
         }
 
         // Then parse the base version
@@ -62,24 +82,12 @@ namespace AppInstaller::Utility
                 break;
             }
         }
+
+        THROW_HR_IF(E_INVALIDARG, m_approximateComparator != ApproximateComparator::None && IsBaseVersionUnknown());
     }
 
     bool Version::operator<(const Version& other) const
     {
-        // If base versions are same, result is based on approximate comparator
-        Version thisBase = *this;
-        thisBase.m_approximateComparator = ApproximateComparator::None;
-        Version otherBase = other;
-        otherBase.m_approximateComparator = ApproximateComparator::None;
-        if (thisBase == otherBase)
-        {
-            // Only true if this is less than, other is not, OR this is none, other is greater than
-            return (m_approximateComparator == ApproximateComparator::LessThan && other.m_approximateComparator != ApproximateComparator::LessThan) ||
-                (m_approximateComparator == ApproximateComparator::None && other.m_approximateComparator == ApproximateComparator::GreaterThan);
-        }
-
-        // The approximate comparator can be ignored now, just compare the base version.
-
         // Sort Latest higher than any other values
         bool thisIsLatest = IsBaseVersionLatest();
         bool otherIsLatest = other.IsBaseVersionLatest();
@@ -122,8 +130,19 @@ namespace AppInstaller::Utility
             // else parts are equal, so continue to next part
         }
 
-        // All parts tested were equal, so this is only less if there are more parts in other.
-        return m_parts.size() < other.m_parts.size();
+        // All parts tested were equal
+        if (m_parts.size() == other.m_parts.size())
+        {
+            // Only true if this is less than, other is not, OR this is none, other is greater than
+            return (m_approximateComparator == ApproximateComparator::LessThan && other.m_approximateComparator != ApproximateComparator::LessThan) ||
+                (m_approximateComparator == ApproximateComparator::None && other.m_approximateComparator == ApproximateComparator::GreaterThan);
+        }
+        else
+        {
+            // Else this is only less if there are more parts in other.
+            return m_parts.size() < other.m_parts.size();
+        }
+
     }
 
     bool Version::operator>(const Version& other) const
@@ -177,7 +196,7 @@ namespace AppInstaller::Utility
 
     bool Version::IsLatest() const
     {
-        return (m_approximateComparator == ApproximateComparator::None && IsBaseVersionLatest());
+        return (m_approximateComparator != ApproximateComparator::LessThan && IsBaseVersionLatest());
     }
 
     Version Version::CreateLatest()
@@ -190,7 +209,7 @@ namespace AppInstaller::Utility
 
     bool Version::IsUnknown() const
     {
-        return (m_approximateComparator == ApproximateComparator::None && IsBaseVersionUnknown());
+        return IsBaseVersionUnknown();
     }
 
     Version Version::CreateUnknown()
@@ -201,18 +220,6 @@ namespace AppInstaller::Utility
         return result;
     }
 
-    Version Version::CreateLessThanApproximateVersion(const Version& base)
-    {
-        THROW_HR_IF(E_INVALIDARG, base.IsApproximateVersion());
-        return Version{ std::string{ s_Approximate_Less_Than } + base.ToString() };
-    }
-
-    Version Version::CreateGreaterThanApproximateVersion(const Version& base)
-    {
-        THROW_HR_IF(E_INVALIDARG, base.IsApproximateVersion());
-        return Version{ std::string{ s_Approximate_Greater_Than } + base.ToString() };
-    }
-
     bool Version::IsBaseVersionLatest() const
     {
         return (m_parts.size() == 1 && m_parts[0].Integer == 0 && Utility::CaseInsensitiveEquals(m_parts[0].Other, s_Version_Part_Latest));
@@ -350,8 +357,7 @@ namespace AppInstaller::Utility
             return false;
         }
 
-        return (m_minVersion >= other.m_minVersion && m_minVersion <= other.m_maxVersion) ||
-            (m_maxVersion >= other.m_minVersion && m_maxVersion <= other.m_maxVersion);
+        return m_minVersion <= other.m_maxVersion && m_maxVersion >= other.m_minVersion;
     }
 
     bool VersionRange::IsSameAsSingleVersion(const Version& version) const

src/AppInstallerRepositoryCore/ArpVersionValidation.cpp

@@ -9,7 +9,7 @@ namespace AppInstaller::Repository
 {
     namespace
     {
-        std::vector<Utility::VersionRange> GetArpVersionRangesByPackageRowId(Microsoft::SQLiteIndex* index, Microsoft::SQLiteIndex::IdType packageRowId, const Utility::VersionAndChannel& excludeVersionAndChannel = {})
+        std::vector<Utility::VersionRange> GetArpVersionRangesByPackageRowId(const Microsoft::SQLiteIndex* index, Microsoft::SQLiteIndex::IdType packageRowId, const Utility::VersionAndChannel& excludeVersionAndChannel = {})
         {
             std::vector<Utility::VersionRange> result;
 
@@ -44,7 +44,7 @@ namespace AppInstaller::Repository
         }
     }
 
-    void ValidateManifestArpVersion(Microsoft::SQLiteIndex* index, const Manifest::Manifest& manifest)
+    void ValidateManifestArpVersion(const Microsoft::SQLiteIndex* index, const Manifest::Manifest& manifest)
     {
         try
         {
@@ -90,7 +90,7 @@ namespace AppInstaller::Repository
         }
     }
 
-    bool ValidateArpVersionConsistency(Microsoft::SQLiteIndex* index)
+    bool ValidateArpVersionConsistency(const Microsoft::SQLiteIndex* index)
     {
         try
         {

src/AppInstallerRepositoryCore/ArpVersionValidation.h

@@ -7,8 +7,8 @@
 namespace AppInstaller::Repository
 {
     // Validate the manifest arp version range against index. Any validation failures will be thrown as ManifestException for better message back to caller.
-    void ValidateManifestArpVersion(Microsoft::SQLiteIndex* index, const Manifest::Manifest& manifest);
+    void ValidateManifestArpVersion(const Microsoft::SQLiteIndex* index, const Manifest::Manifest& manifest);
 
     // Validate the arp version consistency across the index.
-    bool ValidateArpVersionConsistency(Microsoft::SQLiteIndex* index);
+    bool ValidateArpVersionConsistency(const Microsoft::SQLiteIndex* index);
 }