Merge branch 'mc_rocha-remove-duplicated-dast-site-tokens-337932' into 'master'

Add a data migration to remove existing duplicates of DastSiteTokens

See merge request gitlab-org/gitlab!68578

db/post_migrate/20210823132600_remove_duplicate_dast_site_tokens.rb

+# frozen_string_literal: true
+
+class RemoveDuplicateDastSiteTokens < ActiveRecord::Migration[6.1]
+  disable_ddl_transaction!
+
+  class DastSiteToken < ApplicationRecord
+    self.table_name = 'dast_site_tokens'
+    self.inheritance_column = :_type_disabled
+
+    scope :duplicates, -> do
+      all_duplicates = select(:project_id, :url)
+                         .distinct
+                         .group(:project_id, :url)
+                         .having('count(*) > 1')
+                         .pluck('array_agg(id) as ids')
+
+      duplicate_ids = extract_duplicate_ids(all_duplicates)
+
+      where(id: duplicate_ids)
+    end
+
+    def self.extract_duplicate_ids(duplicates)
+      duplicates.flat_map { |ids| ids.first(ids.size - 1) }
+    end
+  end
+
+  def up
+    DastSiteToken.duplicates.delete_all
+  end
+
+  def down
+  end
+end

db/post_migrate/20210826193907_add_unique_index_dast_site_token_project_id_and_url.rb

+# frozen_string_literal: true
+
+class AddUniqueIndexDastSiteTokenProjectIdAndUrl < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_dast_site_token_on_project_id_and_url'
+
+  def up
+    add_concurrent_index :dast_site_tokens, [:project_id, :url], name: INDEX_NAME, unique: true
+  end
+
+  def down
+    remove_concurrent_index_by_name :dast_site_tokens, name: INDEX_NAME
+  end
+end

db/schema_migrations/20210823132600

+7324c3803c910338261556c65cae5d0827e78b77890386e402e056d480c3486b
\ No newline at end of file

db/schema_migrations/20210826193907

+b7916e025131f11da97ab89a01b32d1dbacf94bb96dc84877ba18404c8b0b2ba
\ No newline at end of file

db/structure.sql

@@ -23846,6 +23846,8 @@ CREATE UNIQUE INDEX index_dast_site_profiles_on_project_id_and_name ON dast_site
 
 CREATE UNIQUE INDEX index_dast_site_profiles_pipelines_on_ci_pipeline_id ON dast_site_profiles_pipelines USING btree (ci_pipeline_id);
 
+CREATE UNIQUE INDEX index_dast_site_token_on_project_id_and_url ON dast_site_tokens USING btree (project_id, url);
+
 CREATE INDEX index_dast_site_tokens_on_project_id ON dast_site_tokens USING btree (project_id);
 
 CREATE INDEX index_dast_site_validations_on_dast_site_token_id ON dast_site_validations USING btree (dast_site_token_id);

spec/migrations/remove_duplicate_dast_site_tokens_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require_migration!
+
+RSpec.describe RemoveDuplicateDastSiteTokens do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:dast_site_tokens) { table(:dast_site_tokens) }
+  let!(:namespace) { namespaces.create!(id: 1, name: 'group', path: 'group') }
+  let!(:project1) { projects.create!(id: 1, namespace_id: namespace.id, path: 'project1') }
+  # create non duplicate dast site token
+  let!(:dast_site_token1) { dast_site_tokens.create!(project_id: project1.id, url: 'https://gitlab.com', token: SecureRandom.uuid) }
+
+  context 'when duplicate dast site tokens exists' do
+    # create duplicate dast site token
+    let_it_be(:duplicate_url) { 'https://about.gitlab.com' }
+
+    let!(:project2) { projects.create!(id: 2, namespace_id: namespace.id, path: 'project2') }
+    let!(:dast_site_token2) { dast_site_tokens.create!(project_id: project2.id, url: duplicate_url, token: SecureRandom.uuid) }
+    let!(:dast_site_token3) { dast_site_tokens.create!(project_id: project2.id, url: 'https://temp_url.com', token: SecureRandom.uuid) }
+    let!(:dast_site_token4) { dast_site_tokens.create!(project_id: project2.id, url: 'https://other_temp_url.com', token: SecureRandom.uuid) }
+
+    before 'update URL to bypass uniqueness validation' do
+      dast_site_tokens.where(project_id: 2).update_all(url: duplicate_url)
+    end
+
+    describe 'migration up' do
+      it 'does remove duplicated dast site tokens' do
+        expect(dast_site_tokens.count).to eq(4)
+        expect(dast_site_tokens.where(project_id: 2, url: duplicate_url).size).to eq(3)
+
+        migrate!
+
+        expect(dast_site_tokens.count).to eq(2)
+        expect(dast_site_tokens.where(project_id: 2, url: duplicate_url).size).to eq(1)
+      end
+    end
+  end
+
+  context 'when duplicate dast site tokens does not exists' do
+    before do
+      dast_site_tokens.create!(project_id: 1, url: 'https://about.gitlab.com/handbook', token: SecureRandom.uuid)
+    end
+
+    describe 'migration up' do
+      it 'does remove duplicated dast site tokens' do
+        expect { migrate! }.not_to change(dast_site_tokens, :count)
+      end
+    end
+  end
+end