Merge branch '437616_fix_custom_regex_case' into 'master'

Fix changelog tag detection logic (second attempt)

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144738



Merged-by: Dylan Griffith <dyl.griffith@gmail.com>
Approved-by: Reuben Pereira <2967854-rpereira2@users.noreply.gitlab.com>
Approved-by: Marcin Sedlak-Jakubowski <msedlakjakubowski@gitlab.com>
Approved-by: Dylan Griffith <dyl.griffith@gmail.com>
Reviewed-by: Vasilii Iakliushin <viakliushin@gitlab.com>
Reviewed-by: Reuben Pereira <2967854-rpereira2@users.noreply.gitlab.com>
Co-authored-by: Vasilii Iakliushin <viakliushin@gitlab.com>

app/finders/repositories/changelog_tag_finder.rb

@@ -33,40 +33,30 @@ def initialize(project, regex: Gitlab::Changelog::Config::DEFAULT_TAG_REGEX)
 
     def execute(new_version)
       tags = {}
-      versions = [new_version]
-
-      begin
-        regex = Gitlab::UntrustedRegexp.new(@regex)
-      rescue RegexpError => e
-        # The error messages produced by default are not very helpful, so we
-        # raise a better one here. We raise the specific error here so its
-        # message is displayed in the API (where we catch this specific
-        # error).
-        raise(
-          Gitlab::Changelog::Error,
-          "The regular expression to use for finding the previous tag for a version is invalid: #{e.message}"
-        )
-      end
 
-      @project.repository.tags.each do |tag|
-        matches = regex.match(tag.name)
+      # Custom regex matcher extracts versions from repository tags
+      custom_regex_matcher = matcher(@regex)
 
-        next unless matches
+      if Feature.enabled?(:update_changelog_logic, @project)
+        # Default regex macher extracts the user provided version
+        default_regex_matcher = matcher(Gitlab::Changelog::Config::DEFAULT_TAG_REGEX)
 
-        # When using this class for generating changelog data for a range of
-        # commits, we want to compare against the tag of the last _stable_
-        # release; not some random RC that came after that.
-        next if matches[:pre]
+        requested_version = extract_version(new_version, default_regex_matcher)
+
+        unless requested_version
+          raise Gitlab::Changelog::Error,
+            _("The requested `version` attribute format is not correct. Use formats such as `1.0.0` or `v1.0.0`.")
+        end
+      else
+        requested_version = new_version
+      end
 
-        major = matches[:major]
-        minor = matches[:minor]
-        patch = matches[:patch]
-        build = matches[:meta]
+      versions = [requested_version]
 
-        next unless major && minor && patch
+      @project.repository.tags.each do |tag|
+        version = extract_version(tag.name, custom_regex_matcher)
 
-        version = "#{major}.#{minor}.#{patch}"
-        version += "+#{build}" if build
+        next unless version
 
         tags[version] = tag
         versions << version
@@ -74,9 +64,47 @@ def execute(new_version)
 
       VersionSorter.sort!(versions)
 
-      index = versions.index(new_version)
+      index = versions.index(requested_version)
 
       tags[versions[index - 1]] if index&.positive?
     end
+
+    private
+
+    def matcher(regex)
+      Gitlab::UntrustedRegexp.new(regex)
+    rescue RegexpError => e
+      # The error messages produced by default are not very helpful, so we
+      # raise a better one here. We raise the specific error here so its
+      # message is displayed in the API (where we catch this specific
+      # error).
+      raise(
+        Gitlab::Changelog::Error,
+        "The regular expression to use for finding the previous tag for a version is invalid: #{e.message}"
+      )
+    end
+
+    def extract_version(string, version_matcher)
+      matches = version_matcher.match(string)
+
+      return unless matches
+
+      # When using this class for generating changelog data for a range of
+      # commits, we want to compare against the tag of the last _stable_
+      # release; not some random RC that came after that.
+      return if matches[:pre]
+
+      major = matches[:major]
+      minor = matches[:minor]
+      patch = matches[:patch]
+      build = matches[:meta]
+
+      return unless major && minor && patch
+
+      version = "#{major}.#{minor}.#{patch}"
+      version += "+#{build}" if build
+
+      version
+    end
   end
 end

config/feature_flags/gitlab_com_derisk/update_changelog_logic.yml

+---
+name: update_changelog_logic
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/441804
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144738
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/442016
+milestone: '16.10'
+group: group::source code
+type: gitlab_com_derisk
+default_enabled: false

doc/api/repositories.md

@@ -366,6 +366,9 @@ If the `version` attribute is `2.1.0`, GitLab uses tag `v2.0.0`. And when the
 version is `1.1.1`, or `1.2.0`, GitLab uses tag v1.1.0. The tag `v1.0.0-pre1` is
 never used, because pre-release tags are ignored.
 
+The `version` attribute can start with `v`. For example: `v1.0.0`.
+The response is the same as for `version` value `1.0.0`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/437616) in GitLab 16.10.
+
 If `from` is unspecified and no tag to use is found, the API produces an error.
 To solve such an error, you must explicitly specify a value for the `from`
 attribute.

locale/gitlab.pot

@@ -50202,6 +50202,9 @@ msgstr ""
 msgid "The repository must be accessible over %{code_open}http://%{code_close}, %{code_open}https://%{code_close}, %{code_open}ssh://%{code_close} or %{code_open}git://%{code_close}."
 msgstr ""
+msgid "The requested `version` attribute format is not correct. Use formats such as `1.0.0` or `v1.0.0`."
+msgstr ""
+
 msgid "The resource that you are attempting to access does not exist or you don't have permission to perform this action."
 msgstr ""

spec/finders/repositories/changelog_tag_finder_spec.rb

@@ -2,13 +2,16 @@
 
 require 'spec_helper'
 
-RSpec.describe Repositories::ChangelogTagFinder do
+RSpec.describe Repositories::ChangelogTagFinder, feature_category: :source_code_management do
   let(:project) { build_stubbed(:project) }
-  let(:finder) { described_class.new(project) }
+  let(:finder) { described_class.new(project, regex: regex) }
+  let(:regex) { Gitlab::Changelog::Config::DEFAULT_TAG_REGEX }
 
   describe '#execute' do
     context 'when the regular expression is invalid' do
       it 'raises Gitlab::Changelog::Error' do
+        allow(project.repository).to receive(:tags).and_return([double(:tag, name: 'v1.1.0')])
+
         expect { described_class.new(project, regex: 'foo+*').execute('1.2.3') }
           .to raise_error(Gitlab::Changelog::Error)
       end
@@ -35,6 +38,130 @@
         expect(finder.execute('1.0.0')).to eq(tag4)
         expect(finder.execute('0.9.0')).to eq(tag6)
         expect(finder.execute('0.6.0')).to eq(tag7)
+
+        # with v at the beginning
+        expect(finder.execute('v2.1.0')).to eq(tag3)
+        expect { finder.execute('wrong_version') }.to raise_error(Gitlab::Changelog::Error)
+      end
+
+      context 'when feature flag "update_changelog_logic" is disabled' do
+        before do
+          stub_feature_flags(update_changelog_logic: false)
+        end
+
+        it 'returns the previous tag' do
+          tag1 = double(:tag1, name: 'v1.0.0')
+          tag2 = double(:tag2, name: 'v1.1.0')
+          tag3 = double(:tag3, name: 'v2.0.0')
+          tag4 = double(:tag4, name: '0.9.0')
+          tag5 = double(:tag5, name: 'v0.8.0-pre1')
+          tag6 = double(:tag6, name: 'v0.7.0')
+          tag7 = double(:tag7, name: '0.5.0+42.ee.0')
+
+          allow(project.repository)
+            .to receive(:tags)
+            .and_return([tag1, tag3, tag2, tag4, tag5, tag6, tag7])
+
+          expect(finder.execute('2.1.0')).to eq(tag3)
+          expect(finder.execute('2.0.0')).to eq(tag2)
+          expect(finder.execute('1.5.0')).to eq(tag2)
+          expect(finder.execute('1.0.1')).to eq(tag1)
+          expect(finder.execute('1.0.0')).to eq(tag4)
+          expect(finder.execute('0.9.0')).to eq(tag6)
+          expect(finder.execute('0.6.0')).to eq(tag7)
+
+          # with v at the beginning
+          expect(finder.execute('v2.1.0')).to eq(nil)
+          expect(finder.execute('wrong_version')).to eq(nil)
+        end
+      end
+    end
+
+    context 'when GitLab release process' do
+      let(:regex) { '^v(?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)-ee$' }
+
+      let!(:previous_tag) { double(:tag1, name: 'v16.8.0-ee') }
+      let!(:rc_tag) { double(:tag2, name: 'v16.9.0-rc42-ee') }
+
+      before do
+        allow(project.repository)
+          .to receive(:tags)
+          .and_return([rc_tag, previous_tag])
+      end
+
+      it 'supports GitLab release process' do
+        expect(finder.execute('16.9.0')).to eq(previous_tag)
+        expect(finder.execute('16.8.0')).to eq(nil)
+      end
+
+      context 'when feature flag "update_changelog_logic" is disabled' do
+        before do
+          stub_feature_flags(update_changelog_logic: false)
+        end
+
+        it 'supports GitLab release process' do
+          expect(finder.execute('16.9.0')).to eq(previous_tag)
+          expect(finder.execute('16.8.0')).to eq(nil)
+        end
+      end
+    end
+
+    context 'when Omnibus release process' do
+      let(:regex) { '^(?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)(\+(?P<pre>rc\d+))?((\.|\+)(?P<meta>ee\.\d+))?$' }
+
+      let!(:previous_tag) { double(:tag1, name: '16.8.0+ee.0') }
+      let!(:rc_tag) { double(:tag2, name: '16.9.0+rc42.ee.0') }
+      let!(:previous_ce_tag) { double(:tag3, name: '16.7.0+ce.0') }
+
+      before do
+        allow(project.repository)
+          .to receive(:tags)
+          .and_return([rc_tag, previous_tag, previous_ce_tag])
+      end
+
+      it 'supports Omnibus release process' do
+        expect(finder.execute('16.9.0')).to eq(previous_tag)
+        expect(finder.execute('16.8.0')).to eq(previous_tag)
+        expect(finder.execute('16.7.0')).to eq(nil)
+      end
+
+      context 'when feature flag "update_changelog_logic" is disabled' do
+        before do
+          stub_feature_flags(update_changelog_logic: false)
+        end
+
+        it 'supports Omnibus release process' do
+          expect(finder.execute('16.9.0')).to eq(previous_tag)
+          expect(finder.execute('16.8.0')).to eq(previous_tag)
+          expect(finder.execute('16.7.0')).to eq(nil)
+        end
+      end
+    end
+
+    context 'when Gitaly release process' do
+      let!(:previous_tag) { double(:tag1, name: 'v16.8.0') }
+      let!(:rc_tag) { double(:tag2, name: 'v16.9.0-rc42') }
+
+      before do
+        allow(project.repository)
+          .to receive(:tags)
+          .and_return([rc_tag, previous_tag])
+      end
+
+      it 'supports Gitaly release process' do
+        expect(finder.execute('16.9.0')).to eq(previous_tag)
+        expect(finder.execute('16.8.0')).to eq(nil)
+      end
+
+      context 'when feature flag "update_changelog_logic" is disabled' do
+        before do
+          stub_feature_flags(update_changelog_logic: false)
+        end
+
+        it 'supports Gitaly release process' do
+          expect(finder.execute('16.9.0')).to eq(previous_tag)
+          expect(finder.execute('16.8.0')).to eq(nil)
+        end
       end
     end