Merge branch '31869-duplicate-rows-with-the-same-tag-name-in-release-3' into 'master'

Resolve "Duplicate rows with the same tag name in Release"

See merge request gitlab-org/gitlab!82335

app/models/release.rb

@@ -25,6 +25,8 @@ class Release < ApplicationRecord
   before_create :set_released_at
 
   validates :project, :tag, presence: true
+  validates :tag, uniqueness: { scope: :project_id }
+
   validates :description, length: { maximum: Gitlab::Database::MAX_TEXT_SIZE_LIMIT }, if: :description_changed?
   validates_associated :milestone_releases, message: -> (_, obj) { obj[:value].map(&:errors).map(&:full_messages).join(",") }
   validates :links, nested_attributes_duplicates: { scope: :release, child_attributes: %i[name url filepath] }

db/post_migrate/20220307192534_create_index_for_remove_duplicate_project_tag_releases.rb

+# frozen_string_literal: true
+
+class CreateIndexForRemoveDuplicateProjectTagReleases < Gitlab::Database::Migration[1.0]
+  INDEX_NAME = 'index_releases_on_id_project_id_tag'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :releases,
+                         %i[project_id tag id],
+                         name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :releases, name: INDEX_NAME
+  end
+end

db/post_migrate/20220307192610_remove_duplicate_project_tag_releases.rb

+# frozen_string_literal: true
+
+class RemoveDuplicateProjectTagReleases < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  class Release < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'releases'
+  end
+
+  def up
+    Release.each_batch(of: 5000) do |relation|
+      relation
+        .where('exists (select 1 from releases r2 where r2.project_id = releases.project_id and r2.tag = releases.tag and r2.id > releases.id)')
+        .delete_all
+    end
+  end
+
+  def down
+    # no-op
+    #
+    # releases with the same tag within a project have been removed
+    # and therefore the duplicate release data is no longer available
+  end
+end

db/post_migrate/20220307192645_remove_index_for_remove_duplicate_project_tag_releases.rb

+# frozen_string_literal: true
+
+class RemoveIndexForRemoveDuplicateProjectTagReleases < Gitlab::Database::Migration[1.0]
+  INDEX_NAME = 'index_releases_on_id_project_id_tag'
+
+  disable_ddl_transaction!
+
+  def up
+    remove_concurrent_index_by_name :releases, name: INDEX_NAME
+  end
+
+  def down
+    add_concurrent_index :releases,
+                         %i[project_id tag id],
+                         name: INDEX_NAME
+  end
+end

db/post_migrate/20220307192725_create_unique_index_release_tag_project.rb

+# frozen_string_literal: true
+
+class CreateUniqueIndexReleaseTagProject < Gitlab::Database::Migration[1.0]
+  INDEX_NAME = 'index_releases_on_project_tag_unique'
+  OLD_INDEX_NAME = 'index_releases_on_project_id_and_tag'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :releases,
+                         %i[project_id tag],
+                         unique: true,
+                         name: INDEX_NAME
+    remove_concurrent_index_by_name :releases, name: OLD_INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :releases, name: INDEX_NAME
+    add_concurrent_index :releases,
+                         %i[project_id tag],
+                         name: OLD_INDEX_NAME
+  end
+end

db/schema_migrations/20220307192534

+b8adcc6d7dc76fd18037de9b2b204e7db8803564df19cbd59f928901c8d97b9c
\ No newline at end of file

db/schema_migrations/20220307192610

+3dd34a92230e36fe1e9761ce39e4edb2a3289c972ce56347e87d8e36818e46d1
\ No newline at end of file

db/schema_migrations/20220307192645

+c31db54f15cff7b21272cc2e9e962419ba4422582c227c5af4131fe56c1fc9f8
\ No newline at end of file

db/schema_migrations/20220307192725

+d1761614c3ac0e8bd33eff58134091ec6c1834ecde3e47290a80da45ab207923
\ No newline at end of file

db/structure.sql

@@ -28797,7 +28797,7 @@ CREATE INDEX index_releases_on_author_id_id_created_at ON releases USING btree (
 
 CREATE INDEX index_releases_on_project_id_and_released_at_and_id ON releases USING btree (project_id, released_at, id);
 
-CREATE INDEX index_releases_on_project_id_and_tag ON releases USING btree (project_id, tag);
+CREATE UNIQUE INDEX index_releases_on_project_tag_unique ON releases USING btree (project_id, tag);
 
 CREATE INDEX index_releases_on_released_at ON releases USING btree (released_at);
 

ee/spec/services/releases/update_service_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe Releases::UpdateService do
   let(:project) { create(:project, :repository, group: group) }
   let(:user) { create(:user) }
   let(:params) { { tag: tag_name } }
-  let!(:release) { create(:release, project: project) }
+  let!(:release) { create(:release, project: project, tag: tag_name) }
   let(:tag_name) { 'v1.1.0' }
   let(:service) { described_class.new(project, user, params_with_milestones) }
 

spec/controllers/projects/releases_controller_spec.rb

@@ -339,7 +339,7 @@ RSpec.describe Projects::ReleasesController do
     end
 
     context 'order_by parameter' do
-      let!(:latest_release) { create(:release, project: project, released_at: Time.current) }
+      let!(:latest_release) { create(:release, project: project, released_at: Time.current, tag: 'latest') }
 
       shared_examples_for 'redirects to latest release ordered by using released_at' do
         it do
@@ -352,8 +352,8 @@ RSpec.describe Projects::ReleasesController do
       end
 
       before do
-        create(:release, project: project, released_at: 1.day.ago)
-        create(:release, project: project, released_at: 2.days.ago)
+        create(:release, project: project, released_at: 1.day.ago, tag: 'alpha')
+        create(:release, project: project, released_at: 2.days.ago, tag: 'beta')
       end
 
       context 'invalid parameter' do

spec/controllers/projects/tags/releases_controller_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe Projects::Tags::ReleasesController do
   let!(:project) { create(:project, :repository) }
   let!(:user)    { create(:user) }
-  let!(:release) { create(:release, project: project) }
+  let!(:release) { create(:release, project: project, tag: "v1.1.0") }
   let!(:tag)     { release.tag }
 
   before do
@@ -27,7 +27,7 @@ RSpec.describe Projects::Tags::ReleasesController do
     end
 
     it 'retrieves an existing release' do
-      response = get :edit, params: { namespace_id: project.namespace, project_id: project, tag_id: release.tag }
+      response = get :edit, params: { namespace_id: project.namespace, project_id: project, tag_id: tag }
 
       release = assigns(:release)
       expect(release).not_to be_nil

spec/controllers/projects/tags_controller_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe Projects::TagsController do
   let(:project) { create(:project, :public, :repository) }
-  let!(:release) { create(:release, project: project) }
+  let!(:release) { create(:release, project: project, tag: "v1.1.0") }
   let!(:invalid_release) { create(:release, project: project, tag: 'does-not-exist') }
   let(:user) { create(:user) }
 

spec/factories/releases.rb

@@ -2,7 +2,9 @@
 
 FactoryBot.define do
   factory :release do
-    tag { "v1.1.0" }
+    sequence :tag do |n|
+      "v1.#{n}.0"
+    end
     sha { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' }
     name { tag }
     description { "Awesome release" }

spec/features/projects/releases/user_views_edit_release_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe 'User edits Release', :js do
   let_it_be(:project) { create(:project, :repository) }
   let_it_be(:user) { create(:user) }
 
-  let(:release) { create(:release, :with_milestones, milestones_count: 1, project: project, name: 'The first release' ) }
+  let(:release) { create(:release, :with_milestones, milestones_count: 1, project: project, name: 'The first release', tag: "v1.1.0" ) }
   let(:release_link) { create(:release_link, release: release) }
 
   before do

spec/migrations/20220307192610_remove_duplicate_project_tag_releases_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe RemoveDuplicateProjectTagReleases do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:users) { table(:users) }
+  let(:releases) { table(:releases) }
+
+  let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') }
+  let(:project) { projects.create!(namespace_id: namespace.id, name: 'foo') }
+
+  let(:dup_releases) do
+    Array.new(4).fill do |i|
+      rel = releases.new(project_id: project.id,
+                         tag: "duplicate tag",
+                         released_at: (DateTime.now + i.days))
+      rel.save!(validate: false)
+      rel
+    end
+  end
+
+  let(:valid_release) do
+    releases.create!(
+      project_id: project.id,
+      tag: "valid tag",
+      released_at: DateTime.now
+    )
+  end
+
+  describe '#up' do
+    it "correctly removes duplicate tags from the same project" do
+      expect(dup_releases.length).to eq 4
+      expect(valid_release).not_to be nil
+      expect(releases.where(tag: 'duplicate tag').count).to eq 4
+      expect(releases.where(tag: 'valid tag').count).to eq 1
+
+      migrate!
+
+      expect(releases.where(tag: 'duplicate tag').count).to eq 1
+      expect(releases.where(tag: 'valid tag').count).to eq 1
+      expect(releases.all.map(&:tag)).to match_array ['valid tag', 'duplicate tag']
+    end
+  end
+end

spec/requests/api/releases_spec.rb

@@ -160,7 +160,7 @@ RSpec.describe API::Releases do
         get api("/projects/#{project.id}/releases", maintainer)
       end.count
 
-      create_list(:release, 2, :with_evidence, project: project, tag: 'v0.1', author: maintainer)
+      create_list(:release, 2, :with_evidence, project: project, author: maintainer)
       create_list(:release, 2, project: project)
       create_list(:release_link, 2, release: project.releases.first)
       create_list(:release_link, 2, release: project.releases.last)
@@ -467,10 +467,10 @@ RSpec.describe API::Releases do
           it "exposes tag and commit" do
             create(:release,
                    project: project,
-                   tag: 'v0.1',
+                   tag: 'v0.0.1',
                    author: maintainer,
                    created_at: 2.days.ago)
-            get api("/projects/#{project.id}/releases/v0.1", guest)
+            get api("/projects/#{project.id}/releases/v0.0.1", guest)
 
             expect(response).to match_response_schema('public_api/v4/release')
           end

spec/views/projects/tags/index.html.haml_spec.rb

@@ -6,7 +6,11 @@ RSpec.describe 'projects/tags/index.html.haml' do
   let_it_be(:project)  { create(:project, :repository) }
   let_it_be(:tags)     { project.repository.tags }
   let_it_be(:git_tag)  { project.repository.tags.last }
-  let_it_be(:release)  { create(:release, project: project, sha: git_tag.target_commit.sha) }
+  let_it_be(:release)  do
+    create(:release, project: project,
+           sha: git_tag.target_commit.sha,
+           tag: 'v1.1.0')
+  end
 
   let(:pipeline) { create(:ci_pipeline, :success, project: project, ref: git_tag.name, sha: release.sha) }