Add latest changes from gitlab-org/gitlab@master

9 files changed, 316 insertions, 192 deletions

diff --git a/spec/fixtures/emails/no_content_with_quote.eml b/spec/fixtures/emails/no_content_with_quote.eml
new file mode 100644
index 00000000000..e2e86c2ea4c
--- /dev/null
+++ b/spec/fixtures/emails/no_content_with_quote.eml
@@ -0,0 +1,23 @@
+Return-Path: <jake@adventuretime.ooo>
+Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400
+Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400
+Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700
+Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700
+Date: Thu, 13 Jun 2013 17:03:48 -0400
+From: Jake the Dog <jake@adventuretime.ooo>
+To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo
+Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
+Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
+Mime-Version: 1.0
+Content-Type: text/plain;
+ charset=ISO-8859-1
+Content-Transfer-Encoding: 7bit
+X-Sieve: CMU Sieve 2.2
+X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu,
+ 13 Jun 2013 14:03:48 -0700 (PDT)
+X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1
+
+--
+> quote line 1
+> quote line 2
+> quote line 3
diff --git a/spec/lib/gitlab/database/load_balancing/host_list_spec.rb b/spec/lib/gitlab/database/load_balancing/host_list_spec.rb
index 6a358b5d430..ad4ca18d5e6 100644
--- a/spec/lib/gitlab/database/load_balancing/host_list_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing/host_list_spec.rb
@@ -56,44 +56,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::HostList do
     end
   end
 
-  describe '#manage_pool?' do
-    context 'when the testing pool belongs to one host of the host list' do
-      it 'returns true' do
-        pool = host_list.hosts.first.pool
-
-        expect(host_list.manage_pool?(pool)).to be(true)
-      end
-    end
-
-    context 'when the testing pool belongs to a former host of the host list' do
-      it 'returns false' do
-        pool = host_list.hosts.first.pool
-        host_list.hosts = [
-          Gitlab::Database::LoadBalancing::Host.new('foo', load_balancer)
-        ]
-
-        expect(host_list.manage_pool?(pool)).to be(false)
-      end
-    end
-
-    context 'when the testing pool belongs to a new host of the host list' do
-      it 'returns true' do
-        host = Gitlab::Database::LoadBalancing::Host.new('foo', load_balancer)
-        host_list.hosts = [host]
-
-        expect(host_list.manage_pool?(host.pool)).to be(true)
-      end
-    end
-
-    context 'when the testing pool does not have any relation with the host list' do
-      it 'returns false' do
-        host = Gitlab::Database::LoadBalancing::Host.new('foo', load_balancer)
-
-        expect(host_list.manage_pool?(host.pool)).to be(false)
-      end
-    end
-  end
-
   describe '#hosts' do
     it 'returns a copy of the host' do
       first = host_list.hosts
diff --git a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb
index 358f382bc39..c647f5a8f5d 100644
--- a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb
@@ -137,126 +137,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
     end
   end
 
-  describe '#db_role_for_connection' do
-    context 'when the load balancer creates the connection with #read' do
-      it 'returns :replica' do
-        role = nil
-        lb.read do |connection|
-          role = lb.db_role_for_connection(connection)
-        end
-
-        expect(role).to be(:replica)
-      end
-    end
-
-    context 'when the load balancer uses nested #read' do
-      it 'returns :replica' do
-        roles = []
-        lb.read do |connection_1|
-          lb.read do |connection_2|
-            roles << lb.db_role_for_connection(connection_2)
-          end
-          roles << lb.db_role_for_connection(connection_1)
-        end
-
-        expect(roles).to eq([:replica, :replica])
-      end
-    end
-
-    context 'when the load balancer creates the connection with #read_write' do
-      it 'returns :primary' do
-        role = nil
-        lb.read_write do |connection|
-          role = lb.db_role_for_connection(connection)
-        end
-
-        expect(role).to be(:primary)
-      end
-    end
-
-    context 'when the load balancer uses nested #read_write' do
-      it 'returns :primary' do
-        roles = []
-        lb.read_write do |connection_1|
-          lb.read_write do |connection_2|
-            roles << lb.db_role_for_connection(connection_2)
-          end
-          roles << lb.db_role_for_connection(connection_1)
-        end
-
-        expect(roles).to eq([:primary, :primary])
-      end
-    end
-
-    context 'when the load balancer falls back the connection creation to primary' do
-      it 'returns :primary' do
-        allow(lb).to receive(:serialization_failure?).and_return(true)
-
-        role = nil
-        raised = 7 # 2 hosts = 6 retries
-
-        lb.read do |connection|
-          if raised > 0
-            raised -= 1
-            raise
-          end
-
-          role = lb.db_role_for_connection(connection)
-        end
-
-        expect(role).to be(:primary)
-      end
-    end
-
-    context 'when the load balancer uses replica after recovery from a failure' do
-      it 'returns :replica' do
-        allow(lb).to receive(:connection_error?).and_return(true)
-
-        role = nil
-        raised = false
-
-        lb.read do |connection|
-          unless raised
-            raised = true
-            raise
-          end
-
-          role = lb.db_role_for_connection(connection)
-        end
-
-        expect(role).to be(:replica)
-      end
-    end
-
-    context 'when the connection comes from a pool managed by the host list' do
-      it 'returns :replica' do
-        connection = double(:connection)
-        allow(connection).to receive(:pool).and_return(lb.host_list.hosts.first.pool)
-
-        expect(lb.db_role_for_connection(connection)).to be(:replica)
-      end
-    end
-
-    context 'when the connection comes from the primary pool' do
-      it 'returns :primary' do
-        connection = double(:connection)
-        allow(connection).to receive(:pool).and_return(lb.send(:pool))
-
-        expect(lb.db_role_for_connection(connection)).to be(:primary)
-      end
-    end
-
-    context 'when the connection does not come from any known pool' do
-      it 'returns nil' do
-        connection = double(:connection)
-        pool = double(:connection_pool)
-        allow(connection).to receive(:pool).and_return(pool)
-
-        expect(lb.db_role_for_connection(connection)).to be(nil)
-      end
-    end
-  end
-
   describe '#host' do
     it 'returns the secondary host to use' do
       expect(lb.host).to be_an_instance_of(Gitlab::Database::LoadBalancing::Host)
diff --git a/spec/lib/gitlab/database/load_balancing_spec.rb b/spec/lib/gitlab/database/load_balancing_spec.rb
index fb482061d7c..08a97b7c1bf 100644
--- a/spec/lib/gitlab/database/load_balancing_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing_spec.rb
@@ -296,55 +296,37 @@ RSpec.describe Gitlab::Database::LoadBalancing do
   end
 
   describe '.db_role_for_connection' do
-    let(:connection) { double(:conneciton) }
-
     context 'when the load balancing is not configured' do
-      before do
-        allow(described_class).to receive(:enable?).and_return(false)
-      end
+      let(:connection) { ActiveRecord::Base.connection }
 
       it 'returns primary' do
-        expect(described_class.db_role_for_connection(connection)).to be(:primary)
+        expect(described_class.db_role_for_connection(connection)).to eq(:primary)
       end
     end
 
     context 'when the load balancing is configured' do
-      let(:proxy) { described_class::ConnectionProxy.new(%w(foo)) }
-      let(:load_balancer) { described_class::LoadBalancer.new(%w(foo)) }
+      let(:db_host) { ActiveRecord::Base.connection_pool.db_config.host }
+      let(:proxy) { described_class::ConnectionProxy.new([db_host]) }
 
-      before do
-        allow(described_class).to receive(:enable?).and_return(true)
-        allow(described_class).to receive(:proxy).and_return(proxy)
-        allow(proxy).to receive(:load_balancer).and_return(load_balancer)
+      context 'when a proxy connection is used' do
+        it 'returns :unknown' do
+          expect(described_class.db_role_for_connection(proxy)).to eq(:unknown)
+        end
       end
 
-      context 'when the load balancer returns :replica' do
+      context 'when a read connection is used' do
         it 'returns :replica' do
-          allow(load_balancer).to receive(:db_role_for_connection).and_return(:replica)
-
-          expect(described_class.db_role_for_connection(connection)).to be(:replica)
-
-          expect(load_balancer).to have_received(:db_role_for_connection).with(connection)
+          proxy.load_balancer.read do |connection|
+            expect(described_class.db_role_for_connection(connection)).to eq(:replica)
+          end
         end
       end
 
-      context 'when the load balancer returns :primary' do
+      context 'when a read_write connection is used' do
         it 'returns :primary' do
-          allow(load_balancer).to receive(:db_role_for_connection).and_return(:primary)
-
-          expect(described_class.db_role_for_connection(connection)).to be(:primary)
-
-          expect(load_balancer).to have_received(:db_role_for_connection).with(connection)
-        end
-      end
-
-      context 'when the load balancer returns nil' do
-        it 'returns nil' do
-          allow(load_balancer).to receive(:db_role_for_connection).and_return(nil)
-
-          expect(described_class.db_role_for_connection(connection)).to be(nil)
-
-          expect(load_balancer).to have_received(:db_role_for_connection).with(connection)
+          proxy.load_balancer.read_write do |connection|
+            expect(described_class.db_role_for_connection(connection)).to eq(:primary)
+          end
         end
       end
     end
diff --git a/spec/lib/gitlab/database/transaction/context_spec.rb b/spec/lib/gitlab/database/transaction/context_spec.rb
new file mode 100644
index 00000000000..65d52b4d099
--- /dev/null
+++ b/spec/lib/gitlab/database/transaction/context_spec.rb
@@ -0,0 +1,144 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Database::Transaction::Context do
+  subject { described_class.new }
+
+  let(:data) { subject.context }
+
+  before do
+    stub_const("#{described_class}::LOG_THROTTLE", 100)
+  end
+
+  describe '#set_start_time' do
+    before do
+      subject.set_start_time
+    end
+
+    it 'sets start_time' do
+      expect(data).to have_key(:start_time)
+    end
+  end
+
+  describe '#increment_savepoints' do
+    before do
+      2.times { subject.increment_savepoints }
+    end
+
+    it { expect(data[:savepoints]).to eq(2) }
+  end
+
+  describe '#increment_rollbacks' do
+    before do
+      3.times { subject.increment_rollbacks }
+    end
+
+    it { expect(data[:rollbacks]).to eq(3) }
+  end
+
+  describe '#increment_releases' do
+    before do
+      4.times { subject.increment_releases }
+    end
+
+    it { expect(data[:releases]).to eq(4) }
+  end
+
+  describe '#set_depth' do
+    before do
+      subject.set_depth(2)
+    end
+
+    it { expect(data[:depth]).to eq(2) }
+  end
+
+  describe '#track_sql' do
+    before do
+      subject.track_sql('SELECT 1')
+      subject.track_sql('SELECT * FROM users')
+    end
+
+    it { expect(data[:queries]).to eq(['SELECT 1', 'SELECT * FROM users']) }
+  end
+
+  describe '#duration' do
+    before do
+      subject.set_start_time
+    end
+
+    it { expect(subject.duration).to be >= 0 }
+  end
+
+  context 'when depth is low' do
+    it 'does not log data upon COMMIT' do
+      expect(subject).not_to receive(:application_info)
+
+      subject.commit
+    end
+
+    it 'does not log data upon ROLLBACK' do
+      expect(subject).not_to receive(:application_info)
+
+      subject.rollback
+    end
+
+    it '#should_log? returns false' do
+      expect(subject.should_log?).to be false
+    end
+  end
+
+  shared_examples 'logs transaction data' do
+    it 'logs once upon COMMIT' do
+      expect(subject).to receive(:application_info).and_call_original
+
+      2.times { subject.commit }
+    end
+
+    it 'logs once upon ROLLBACK' do
+      expect(subject).to receive(:application_info).once
+
+      2.times { subject.rollback }
+    end
+
+    it 'logs again when log throttle duration passes' do
+      expect(subject).to receive(:application_info).twice.and_call_original
+
+      2.times { subject.commit }
+
+      data[:last_log_timestamp] -= (described_class::LOG_THROTTLE_DURATION + 1)
+
+      subject.commit
+    end
+
+    it '#should_log? returns true' do
+      expect(subject.should_log?).to be true
+    end
+  end
+
+  context 'when depth exceeds threshold' do
+    before do
+      subject.set_depth(described_class::LOG_DEPTH_THRESHOLD + 1)
+    end
+
+    it_behaves_like 'logs transaction data'
+  end
+
+  context 'when savepoints count exceeds threshold' do
+    before do
+      data[:savepoints] = described_class::LOG_SAVEPOINTS_THRESHOLD + 1
+    end
+
+    it_behaves_like 'logs transaction data'
+  end
+
+  context 'when duration exceeds threshold' do
+    before do
+      subject.set_start_time
+
+      data[:start_time] -= (described_class::LOG_DURATION_S_THRESHOLD + 1)
+    end
+
+    it_behaves_like 'logs transaction data'
+  end
+end
diff --git a/spec/lib/gitlab/database/transaction/observer_spec.rb b/spec/lib/gitlab/database/transaction/observer_spec.rb
new file mode 100644
index 00000000000..7aa24217dc3
--- /dev/null
+++ b/spec/lib/gitlab/database/transaction/observer_spec.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Database::Transaction::Observer do
+  # Use the delete DB strategy so that the test won't be wrapped in a transaction
+  describe '.instrument_transactions', :delete do
+    let(:transaction_context) { ActiveRecord::Base.connection.transaction_manager.transaction_context }
+    let(:context) { transaction_context.context }
+
+    around do |example|
+      # Emulate production environment when SQL comments come first to avoid truncation
+      Marginalia::Comment.prepend_comment = true
+      subscriber = described_class.register!
+
+      example.run
+
+      ActiveSupport::Notifications.unsubscribe(subscriber)
+      Marginalia::Comment.prepend_comment = false
+    end
+
+    it 'tracks transaction data', :aggregate_failures do
+      ActiveRecord::Base.transaction do
+        ActiveRecord::Base.transaction(requires_new: true) do
+          User.first
+
+          expect(transaction_context).to be_a(::Gitlab::Database::Transaction::Context)
+          expect(context.keys).to match_array(%i(start_time depth savepoints queries))
+          expect(context[:depth]).to eq(2)
+          expect(context[:savepoints]).to eq(1)
+          expect(context[:queries].length).to eq(1)
+        end
+      end
+
+      expect(context[:depth]).to eq(2)
+      expect(context[:savepoints]).to eq(1)
+      expect(context[:releases]).to eq(1)
+    end
+
+    describe '.extract_sql_command' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:sql, :expected) do
+        'SELECT 1' | 'SELECT 1'
+        '/* test comment */ SELECT 1' | 'SELECT 1'
+        '/* test comment */ ROLLBACK TO SAVEPOINT point1' | 'ROLLBACK TO SAVEPOINT '
+        'SELECT 1 /* trailing comment */' | 'SELECT 1 /* trailing comment */'
+      end
+
+      with_them do
+        it do
+          expect(described_class.extract_sql_command(sql)).to eq(expected)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb
index e76a5d3fe32..c0ac40e3249 100644
--- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb
+++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb
@@ -110,6 +110,60 @@ RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do
     end
   end
 
+  context 'when email contains reply' do
+    shared_examples 'no content message' do
+      context 'when email contains quoted text only' do
+        let(:email_raw) { fixture_file('emails/no_content_with_quote.eml') }
+
+        it 'raises an EmptyEmailError' do
+          expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError)
+        end
+      end
+
+      context 'when email contains quoted text and quick commands only' do
+        let(:email_raw) { fixture_file('emails/commands_only_reply.eml') }
+
+        it 'does not create a discussion' do
+          expect { receiver.execute }.not_to change { noteable.notes.count }
+        end
+      end
+    end
+
+    context 'when noteable is not an issue' do
+      let_it_be(:note) { create(:note_on_merge_request, project: project) }
+
+      it_behaves_like 'no content message'
+
+      context 'when email contains text, quoted text and quick commands' do
+        let(:email_raw) { fixture_file('emails/commands_in_reply.eml') }
+
+        it 'creates a discussion without appended reply' do
+          expect { receiver.execute }.to change { noteable.notes.count }.by(1)
+          new_note = noteable.notes.last
+
+          expect(new_note.note).not_to include('<details><summary>...</summary>')
+        end
+      end
+    end
+
+    context 'when noteable is an issue' do
+      let_it_be(:note) { create(:note_on_issue, project: project) }
+
+      it_behaves_like 'no content message'
+
+      context 'when email contains text, quoted text and quick commands' do
+        let(:email_raw) { fixture_file('emails/commands_in_reply.eml') }
+
+        it 'creates a discussion with appended reply' do
+          expect { receiver.execute }.to change { noteable.notes.count }.by(1)
+          new_note = noteable.notes.last
+
+          expect(new_note.note).to include('<details><summary>...</summary>')
+        end
+      end
+    end
+  end
+
   context 'when note is not a discussion' do
     let(:note) { create(:note_on_merge_request, project: project) }
 
diff --git a/spec/lib/gitlab/email/reply_parser_spec.rb b/spec/lib/gitlab/email/reply_parser_spec.rb
index bc4c6cf007d..3b01b568fb4 100644
--- a/spec/lib/gitlab/email/reply_parser_spec.rb
+++ b/spec/lib/gitlab/email/reply_parser_spec.rb
@@ -228,5 +228,21 @@ RSpec.describe Gitlab::Email::ReplyParser do
           BODY
         )
     end
+
+    it "appends trimmed reply when when append_reply option is true" do
+      body = <<-BODY.strip_heredoc.chomp
+      The reply by email functionality should be extended to allow creating a new issue by email.
+      even when the email is forwarded to the project which may include lines that begin with ">"
+
+      there should be a quote below this line:
+      BODY
+
+      reply = <<-BODY.strip_heredoc.chomp
+      > this is a quote
+      BODY
+
+      expect(test_parse_body(fixture_file("emails/valid_new_issue_with_quote.eml"), { append_reply: true }))
+        .to contain_exactly(body, reply)
+    end
   end
 end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index f2bc814738d..ddf12c8e4c4 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -640,6 +640,12 @@ RSpec.describe Group do
       it { is_expected.to match_array([private_group, internal_group]) }
     end
 
+    describe 'private_only' do
+      subject { described_class.private_only.to_a }
+
+      it { is_expected.to match_array([private_group]) }
+    end
+
     describe 'with_onboarding_progress' do
       subject { described_class.with_onboarding_progress }