Merge branch '407395-fix-nuget-version-sorting' into 'master'

Add custom nuget version sorter

See merge request !122484



Merged-by: Sashi Kumar Kumaresan <skumar@gitlab.com>
Approved-by: Radamanthus Batnag <rbatnag@gitlab.com>
Approved-by: Sashi Kumar Kumaresan <skumar@gitlab.com>
Reviewed-by: Sashi Kumar Kumaresan <skumar@gitlab.com>
Reviewed-by: Radamanthus Batnag <rbatnag@gitlab.com>
Reviewed-by: Moaz Khalifa <mkhalifa@gitlab.com>
Co-authored-by: moaz-khalifa <mkhalifa@gitlab.com>

app/presenters/packages/nuget/packages_metadata_presenter.rb

@@ -59,8 +59,8 @@ def upper_version
       end
 
       def sorted_versions
-        versions = @packages.map(&:version).compact
-        VersionSorter.sort(versions)
+        versions = @packages.filter_map(&:version)
+        sort_versions(versions)
       end
       strong_memoize_attr :sorted_versions
     end

app/presenters/packages/nuget/presenter_helpers.rb

@@ -4,6 +4,7 @@ module Packages
   module Nuget
     module PresenterHelpers
       include ::API::Helpers::RelatedResourcesHelpers
+      include Packages::Nuget::VersionHelpers
 
       PACKAGE_DEPENDENCY_GROUP = 'PackageDependencyGroup'
       PACKAGE_DEPENDENCY = 'PackageDependency'

app/presenters/packages/nuget/search_results_presenter.rb

@@ -45,8 +45,8 @@ def build_package_versions(packages)
       end
 
       def latest_version(packages)
-        versions = packages.map(&:version).compact
-        VersionSorter.sort(versions).last
+        versions = packages.filter_map(&:version)
+        sort_versions(versions).last
       end
     end
   end

app/presenters/packages/nuget/version_helpers.rb

+# frozen_string_literal: true
+
+module Packages
+  module Nuget
+    module VersionHelpers
+      private
+
+      def sort_versions(versions)
+        versions.sort { |a, b| compare_versions(a, b) }
+      end
+
+      # NuGet version sorting algorithm as per https://semver.org/spec/v2.0.0.html#spec-item-11
+      def compare_versions(version_a, version_b)
+        return 0 if version_a == version_b
+        return 1 if version_b.nil?
+        return -1 if version_a.nil?
+
+        a_without_build_meta, a_build_meta = version_a.split('+', 2)
+        b_without_build_meta, b_build_meta = version_b.split('+', 2)
+
+        a_core, a_pre = a_without_build_meta.split(/-/, 2)
+        b_core, b_pre = b_without_build_meta.split(/-/, 2)
+
+        a_core_parts = a_core.split('.')
+        b_core_parts = b_core.split('.')
+
+        compare_core_parts(a_core_parts, b_core_parts) ||
+          compare_pre_release_parts(a_pre, b_pre) ||
+          pick_non_nil(a_pre, b_pre) ||
+          compare_build_meta_parts(a_build_meta, b_build_meta)
+      end
+
+      def compare_core_parts(a_core_parts, b_core_parts)
+        while a_core_parts.any? || b_core_parts.any?
+          a_part = a_core_parts.shift&.to_i || 0
+          b_part = b_core_parts.shift&.to_i || 0
+          return a_part <=> b_part if a_part != b_part
+        end
+      end
+
+      def compare_pre_release_parts(a_pre, b_pre)
+        return unless a_pre && b_pre
+
+        a_pre_parts = a_pre.split('.').map(&:downcase)
+        b_pre_parts = b_pre.split('.').map(&:downcase)
+
+        while a_pre_parts.any? || b_pre_parts.any?
+          a_pre_part = a_pre_parts.shift
+          b_pre_part = b_pre_parts.shift
+
+          # Empty parts are considered lower
+          return -1 if a_pre_part.nil?
+          return 1 if b_pre_part.nil?
+
+          a_num = a_pre_part.to_i
+          b_num = b_pre_part.to_i
+          next if a_num == b_num && a_pre_part.to_s == b_pre_part.to_s # Both are same numeric/alphanumeric parts
+
+          return select_numeric_before_alphanumeric(a_num, a_pre_part, b_num, b_pre_part) ||
+              compare_numeric_parts(a_pre_part, a_num, b_pre_part, b_num) ||
+              a_pre_part <=> b_pre_part
+        end
+      end
+
+      def compare_build_meta_parts(a_build_meta, b_build_meta)
+        (a_build_meta || '').casecmp(b_build_meta || '')
+      end
+
+      def select_numeric_before_alphanumeric(a_num, a_pre_part, b_num, b_pre_part)
+        return -1 if a_num != b_num && numeric?(a_pre_part) && !numeric?(b_pre_part)
+        return 1 if a_num != b_num && !numeric?(a_pre_part) && numeric?(b_pre_part)
+      end
+
+      def numeric?(pre_part)
+        !!Integer(pre_part, exception: false)
+      end
+
+      def compare_numeric_parts(a_pre_part, a_num, b_pre_part, b_num)
+        a_num <=> b_num if a_num != b_num && numeric?(a_pre_part) && numeric?(b_pre_part)
+      end
+
+      def pick_non_nil(var_a, var_b)
+        return -1 if var_a && !var_b
+        return 1 if !var_a && var_b
+      end
+    end
+  end
+end

spec/presenters/packages/nuget/packages_metadata_presenter_spec.rb

@@ -71,5 +71,13 @@
         end
       end
     end
+
+    it 'returns sorted versions' do
+      item = subject.first
+      sorted_versions = presenter.send(:sort_versions, packages.map(&:version))
+
+      expect(item[:lower_version]).to eq sorted_versions.first
+      expect(item[:upper_version]).to eq sorted_versions.last
+    end
   end
 end

spec/presenters/packages/nuget/search_results_presenter_spec.rb

@@ -30,12 +30,12 @@
       expect_package_result(pkg_c, packages_c.first.name, packages_c.map(&:version))
     end
 
-    def expect_package_result(package_json, name, versions, tags = [], with_metadatum: false)
+    def expect_package_result(package_json, name, versions, tags = [], with_metadatum: false) # rubocop:disable Metrics/AbcSize
       expect(package_json[:type]).to eq 'Package'
       expect(package_json[:name]).to eq(name)
       expect(package_json[:total_downloads]).to eq 0
       expect(package_json[:verified]).to be_truthy
-      expect(package_json[:version]).to eq VersionSorter.sort(versions).last
+      expect(package_json[:version]).to eq presenter.send(:sort_versions, versions).last
       versions.zip(package_json[:versions]).each do |version, version_json|
         expect(version_json[:json_url]).to end_with("#{version}.json")
         expect(version_json[:downloads]).to eq 0

spec/presenters/packages/nuget/version_helpers_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Packages::Nuget::VersionHelpers, feature_category: :package_registry do
+  include described_class
+
+  describe '#sort_versions' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:unsorted_versions, :expected_result) do
+      ['1.0.0-a1b', '1.0.0-abb', '1.0.0-a11'] | ['1.0.0-a11', '1.0.0-a1b', '1.0.0-abb']
+
+      ['1.8.6-10pre', '1.8.6-5pre', '1.8.6-05pre', '1.8.6-9'] | ['1.8.6-9', '1.8.6-05pre', '1.8.6-10pre', '1.8.6-5pre']
+
+      ['8.4.0-MOR-4077-TabControl.1', '8.4.0-max-migration.1', '8.4.0-develop-nuget20230418.1',
+        '8.4.0-MOR-4077-TabControl.2'] |
+        ['8.4.0-develop-nuget20230418.1', '8.4.0-max-migration.1', '8.4.0-MOR-4077-TabControl.1',
+          '8.4.0-MOR-4077-TabControl.2']
+
+      ['1.0.0-beta+build.1', '1.0.0-beta.11', '1.0.0-beta.2', '1.0.0-alpha', '1.0.0-alpha.1', '1.0.0-alpha.2',
+        '1.0.0-alpha.beta', '2.0.0', '1.0.0-rc.1', '1.0.0-beta', '2.0.0-alpha', '1.0.0', '1.0.0-rc.1+build.1',
+        '1.0.0+build', '1.0.0+build.1', '1.0.1-rc.1', '1.0.1', '1.0.1+build.2', '1.1.0-alpha', '1.1.0'] |
+        ['1.0.0-alpha', '1.0.0-alpha.1', '1.0.0-alpha.2', '1.0.0-alpha.beta', '1.0.0-beta', '1.0.0-beta+build.1',
+          '1.0.0-beta.2', '1.0.0-beta.11', '1.0.0-rc.1', '1.0.0-rc.1+build.1', '1.0.0', '1.0.0+build', '1.0.0+build.1',
+          '1.0.1-rc.1', '1.0.1', '1.0.1+build.2', '1.1.0-alpha', '1.1.0', '2.0.0-alpha', '2.0.0']
+    end
+
+    with_them do
+      it 'sorts versions in ascending order' do
+        expect(sort_versions(unsorted_versions)).to eq(expected_result)
+        expect(VersionSorter.sort(unsorted_versions)).not_to eq(expected_result)
+      end
+    end
+  end
+end