Add latest changes from gitlab-org/gitlab@master

3 files changed, 386 insertions, 4 deletions

diff --git a/spec/lib/gitlab/database/batch_count_spec.rb b/spec/lib/gitlab/database/batch_count_spec.rb
index 7842323d009..7be84b8f980 100644
--- a/spec/lib/gitlab/database/batch_count_spec.rb
+++ b/spec/lib/gitlab/database/batch_count_spec.rb
@@ -86,10 +86,6 @@ describe Gitlab::Database::BatchCount do
   end
 
   describe '#batch_distinct_count' do
-    it 'counts with :id field' do
-      expect(described_class.batch_distinct_count(model, :id)).to eq(5)
-    end
-
     it 'counts with column field' do
       expect(described_class.batch_distinct_count(model, column)).to eq(2)
     end
@@ -137,6 +133,12 @@ describe Gitlab::Database::BatchCount do
       it 'returns fallback if batch size is less than min required' do
         expect(described_class.batch_distinct_count(model, column, batch_size: small_batch_size)).to eq(fallback)
       end
+
+      it 'will raise an error if distinct count with the :id column is requested' do
+        expect do
+          described_class.batch_count(described_class.batch_distinct_count(model, :id))
+        end.to raise_error 'Use distinct count only with non id fields'
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb
index 8b765ce122d..3db9320c021 100644
--- a/spec/lib/gitlab/database/migration_helpers_spec.rb
+++ b/spec/lib/gitlab/database/migration_helpers_spec.rb
@@ -2046,4 +2046,333 @@ describe Gitlab::Database::MigrationHelpers do
       model.bulk_migrate_in(10.minutes, [%w(Class hello world)])
     end
   end
+
+  describe '#check_constraint_name' do
+    it 'returns a valid constraint name' do
+      name = model.check_constraint_name(:this_is_a_very_long_table_name,
+                                         :with_a_very_long_column_name,
+                                         :with_a_very_long_type)
+
+      expect(name).to be_an_instance_of(String)
+      expect(name).to start_with('check_')
+      expect(name.length).to eq(16)
+    end
+  end
+
+  describe '#check_constraint_exists?' do
+    before do
+      ActiveRecord::Base.connection.execute(
+        'ALTER TABLE projects ADD CONSTRAINT check_1 CHECK (char_length(path) <= 5) NOT VALID'
+      )
+    end
+
+    after do
+      ActiveRecord::Base.connection.execute(
+        'ALTER TABLE projects DROP CONSTRAINT IF EXISTS check_1'
+      )
+    end
+
+    it 'returns true if a constraint exists' do
+      expect(model.check_constraint_exists?(:projects, 'check_1'))
+        .to be_truthy
+    end
+
+    it 'returns false if a constraint does not exist' do
+      expect(model.check_constraint_exists?(:projects, 'this_does_not_exist'))
+        .to be_falsy
+    end
+
+    it 'returns false if a constraint with the same name exists in another table' do
+      expect(model.check_constraint_exists?(:users, 'check_1'))
+        .to be_falsy
+    end
+  end
+
+  describe '#add_check_constraint' do
+    before do
+      allow(model).to receive(:check_constraint_exists?).and_return(false)
+    end
+
+    context 'inside a transaction' do
+      it 'raises an error' do
+        expect(model).to receive(:transaction_open?).and_return(true)
+
+        expect do
+          model.add_check_constraint(
+            :test_table,
+            'name IS NOT NULL',
+            'check_name_not_null'
+          )
+        end.to raise_error(RuntimeError)
+      end
+    end
+
+    context 'outside a transaction' do
+      before do
+        allow(model).to receive(:transaction_open?).and_return(false)
+      end
+
+      context 'when the constraint is already defined in the database' do
+        it 'does not create a constraint' do
+          expect(model).to receive(:check_constraint_exists?)
+                       .with(:test_table, 'check_name_not_null')
+                       .and_return(true)
+
+          expect(model).not_to receive(:execute).with(/ADD CONSTRAINT/)
+
+          # setting validate: false to only focus on the ADD CONSTRAINT command
+          model.add_check_constraint(
+            :test_table,
+            'name IS NOT NULL',
+            'check_name_not_null',
+            validate: false
+          )
+        end
+      end
+
+      context 'when the constraint is not defined in the database' do
+        it 'creates the constraint' do
+          expect(model).to receive(:with_lock_retries).and_call_original
+          expect(model).to receive(:execute).with(/ADD CONSTRAINT check_name_not_null/)
+
+          # setting validate: false to only focus on the ADD CONSTRAINT command
+          model.add_check_constraint(
+            :test_table,
+            'char_length(name) <= 255',
+            'check_name_not_null',
+            validate: false
+          )
+        end
+      end
+
+      context 'when validate is not provided' do
+        it 'performs validation' do
+          expect(model).to receive(:check_constraint_exists?)
+                       .with(:test_table, 'check_name_not_null')
+                       .and_return(false).exactly(1)
+
+          expect(model).to receive(:disable_statement_timeout).and_call_original
+          expect(model).to receive(:execute).with(/statement_timeout/)
+          expect(model).to receive(:with_lock_retries).and_call_original
+          expect(model).to receive(:execute).with(/ADD CONSTRAINT check_name_not_null/)
+
+          # we need the check constraint to exist so that the validation proceeds
+          expect(model).to receive(:check_constraint_exists?)
+                       .with(:test_table, 'check_name_not_null')
+                       .and_return(true).exactly(1)
+
+          expect(model).to receive(:execute).ordered.with(/VALIDATE CONSTRAINT/)
+          expect(model).to receive(:execute).ordered.with(/RESET ALL/)
+
+          model.add_check_constraint(
+            :test_table,
+            'char_length(name) <= 255',
+            'check_name_not_null'
+          )
+        end
+      end
+
+      context 'when validate is provided with a falsey value' do
+        it 'skips validation' do
+          expect(model).not_to receive(:disable_statement_timeout)
+          expect(model).to receive(:with_lock_retries).and_call_original
+          expect(model).to receive(:execute).with(/ADD CONSTRAINT/)
+          expect(model).not_to receive(:execute).with(/VALIDATE CONSTRAINT/)
+
+          model.add_check_constraint(
+            :test_table,
+            'char_length(name) <= 255',
+            'check_name_not_null',
+            validate: false
+          )
+        end
+      end
+
+      context 'when validate is provided with a truthy value' do
+        it 'performs validation' do
+          expect(model).to receive(:check_constraint_exists?)
+                       .with(:test_table, 'check_name_not_null')
+                       .and_return(false).exactly(1)
+
+          expect(model).to receive(:disable_statement_timeout).and_call_original
+          expect(model).to receive(:execute).with(/statement_timeout/)
+          expect(model).to receive(:with_lock_retries).and_call_original
+          expect(model).to receive(:execute).with(/ADD CONSTRAINT check_name_not_null/)
+
+          expect(model).to receive(:check_constraint_exists?)
+                       .with(:test_table, 'check_name_not_null')
+                       .and_return(true).exactly(1)
+
+          expect(model).to receive(:execute).ordered.with(/VALIDATE CONSTRAINT/)
+          expect(model).to receive(:execute).ordered.with(/RESET ALL/)
+
+          model.add_check_constraint(
+            :test_table,
+            'char_length(name) <= 255',
+            'check_name_not_null',
+            validate: true
+          )
+        end
+      end
+    end
+  end
+
+  describe '#validate_check_constraint' do
+    context 'when the constraint does not exist' do
+      it 'raises an error' do
+        error_message = /Could not find check constraint "check_1" on table "test_table"/
+
+        expect(model).to receive(:check_constraint_exists?).and_return(false)
+
+        expect do
+          model.validate_check_constraint(:test_table, 'check_1')
+        end.to raise_error(RuntimeError, error_message)
+      end
+    end
+
+    context 'when the constraint exists' do
+      it 'performs validation' do
+        validate_sql = /ALTER TABLE test_table VALIDATE CONSTRAINT check_name/
+
+        expect(model).to receive(:check_constraint_exists?).and_return(true)
+        expect(model).to receive(:disable_statement_timeout).and_call_original
+        expect(model).to receive(:execute).with(/statement_timeout/)
+        expect(model).to receive(:execute).ordered.with(validate_sql)
+        expect(model).to receive(:execute).ordered.with(/RESET ALL/)
+
+        model.validate_check_constraint(:test_table, 'check_name')
+      end
+    end
+  end
+
+  describe '#remove_check_constraint' do
+    it 'removes the constraint' do
+      drop_sql = /ALTER TABLE test_table\s+DROP CONSTRAINT IF EXISTS check_name/
+
+      expect(model).to receive(:with_lock_retries).and_call_original
+      expect(model).to receive(:execute).with(drop_sql)
+
+      model.remove_check_constraint(:test_table, 'check_name')
+    end
+  end
+
+  describe '#add_text_limit' do
+    context 'when it is called with the default options' do
+      it 'calls add_check_constraint with an infered constraint name and validate: true' do
+        constraint_name = model.check_constraint_name(:test_table,
+                                                      :name,
+                                                      'max_length')
+        check = "char_length(name) <= 255"
+
+        expect(model).to receive(:check_constraint_name).and_call_original
+        expect(model).to receive(:add_check_constraint)
+                     .with(:test_table, check, constraint_name, validate: true)
+
+        model.add_text_limit(:test_table, :name, 255)
+      end
+    end
+
+    context 'when all parameters are provided' do
+      it 'calls add_check_constraint with the correct parameters' do
+        constraint_name = 'check_name_limit'
+        check = "char_length(name) <= 255"
+
+        expect(model).not_to receive(:check_constraint_name)
+        expect(model).to receive(:add_check_constraint)
+                     .with(:test_table, check, constraint_name, validate: false)
+
+        model.add_text_limit(
+          :test_table,
+          :name,
+          255,
+          constraint_name: constraint_name,
+          validate: false
+        )
+      end
+    end
+  end
+
+  describe '#validate_text_limit' do
+    context 'when constraint_name is not provided' do
+      it 'calls validate_check_constraint with an infered constraint name' do
+        constraint_name = model.check_constraint_name(:test_table,
+                                                      :name,
+                                                      'max_length')
+
+        expect(model).to receive(:check_constraint_name).and_call_original
+        expect(model).to receive(:validate_check_constraint)
+                     .with(:test_table, constraint_name)
+
+        model.validate_text_limit(:test_table, :name)
+      end
+    end
+
+    context 'when constraint_name is provided' do
+      it 'calls validate_check_constraint with the correct parameters' do
+        constraint_name = 'check_name_limit'
+
+        expect(model).not_to receive(:check_constraint_name)
+        expect(model).to receive(:validate_check_constraint)
+                     .with(:test_table, constraint_name)
+
+        model.validate_text_limit(:test_table, :name, constraint_name: constraint_name)
+      end
+    end
+  end
+
+  describe '#remove_text_limit' do
+    context 'when constraint_name is not provided' do
+      it 'calls remove_check_constraint with an infered constraint name' do
+        constraint_name = model.check_constraint_name(:test_table,
+                                                      :name,
+                                                      'max_length')
+
+        expect(model).to receive(:check_constraint_name).and_call_original
+        expect(model).to receive(:remove_check_constraint)
+                     .with(:test_table, constraint_name)
+
+        model.remove_text_limit(:test_table, :name)
+      end
+    end
+
+    context 'when constraint_name is provided' do
+      it 'calls remove_check_constraint with the correct parameters' do
+        constraint_name = 'check_name_limit'
+
+        expect(model).not_to receive(:check_constraint_name)
+        expect(model).to receive(:remove_check_constraint)
+                     .with(:test_table, constraint_name)
+
+        model.remove_text_limit(:test_table, :name, constraint_name: constraint_name)
+      end
+    end
+  end
+
+  describe '#check_text_limit_exists?' do
+    context 'when constraint_name is not provided' do
+      it 'calls check_constraint_exists? with an infered constraint name' do
+        constraint_name = model.check_constraint_name(:test_table,
+                                                      :name,
+                                                      'max_length')
+
+        expect(model).to receive(:check_constraint_name).and_call_original
+        expect(model).to receive(:check_constraint_exists?)
+                     .with(:test_table, constraint_name)
+
+        model.check_text_limit_exists?(:test_table, :name)
+      end
+    end
+
+    context 'when constraint_name is provided' do
+      it 'calls check_constraint_exists? with the correct parameters' do
+        constraint_name = 'check_name_limit'
+
+        expect(model).not_to receive(:check_constraint_name)
+        expect(model).to receive(:check_constraint_exists?)
+                     .with(:test_table, constraint_name)
+
+        model.check_text_limit_exists?(:test_table, :name, constraint_name: constraint_name)
+      end
+    end
+  end
 end
diff --git a/spec/models/project_services/chat_message/pipeline_message_spec.rb b/spec/models/project_services/chat_message/pipeline_message_spec.rb
index 4210b52a8b9..e99148d1d1f 100644
--- a/spec/models/project_services/chat_message/pipeline_message_spec.rb
+++ b/spec/models/project_services/chat_message/pipeline_message_spec.rb
@@ -431,6 +431,57 @@ describe ChatMessage::PipelineMessage do
       end
     end
 
+    context "when jobs succeed on retries" do
+      before do
+        args[:builds] = [
+          { id: 1, name: "job-1", status: "failed", stage: "stage-1" },
+          { id: 2, name: "job-2", status: "failed", stage: "stage-2" },
+          { id: 3, name: "job-3", status: "failed", stage: "stage-3" },
+          { id: 7, name: "job-1", status: "failed", stage: "stage-1" },
+          { id: 8, name: "job-1", status: "success", stage: "stage-1" }
+        ]
+      end
+
+      it "do not return a job which succeeded on retry" do
+        expected_jobs = [
+          "<http://example.gitlab.com/-/jobs/3|job-3>",
+          "<http://example.gitlab.com/-/jobs/2|job-2>"
+        ]
+
+        expect(subject.attachments.first[:fields][3]).to eq(
+          title: "Failed jobs",
+          value: expected_jobs.join(", "),
+          short: true
+        )
+      end
+    end
+
+    context "when jobs failed even on retries" do
+      before do
+        args[:builds] = [
+          { id: 1, name: "job-1", status: "failed", stage: "stage-1" },
+          { id: 2, name: "job-2", status: "failed", stage: "stage-2" },
+          { id: 3, name: "job-3", status: "failed", stage: "stage-3" },
+          { id: 7, name: "job-1", status: "failed", stage: "stage-1" },
+          { id: 8, name: "job-1", status: "failed", stage: "stage-1" }
+        ]
+      end
+
+      it "returns only first instance of the failed job" do
+        expected_jobs = [
+          "<http://example.gitlab.com/-/jobs/3|job-3>",
+          "<http://example.gitlab.com/-/jobs/2|job-2>",
+          "<http://example.gitlab.com/-/jobs/1|job-1>"
+        ]
+
+        expect(subject.attachments.first[:fields][3]).to eq(
+          title: "Failed jobs",
+          value: expected_jobs.join(", "),
+          short: true
+        )
+      end
+    end
+
     context "when the CI config file contains a YAML error" do
       let(:has_yaml_errors) { true }