Merge branch 'ce-to-ee-2018-05-18' into 'master'

CE upstream - 2018-05-18 21:23 UTC

See merge request gitlab-org/gitlab-ee!5778

.flayignore

@@ -17,6 +17,10 @@ lib/gitlab/background_migration/*
 app/models/project_services/kubernetes_service.rb
 lib/gitlab/workhorse.rb
 lib/gitlab/ci/trace/chunked_io.rb
+lib/gitlab/gitaly_client/ref_service.rb
+lib/gitlab/gitaly_client/commit_service.rb
+lib/gitlab/git/commit.rb
+lib/gitlab/git/tag.rb
 
 ee/db/**/*
 ee/app/serializers/ee/merge_request_widget_entity.rb

GITALY_SERVER_VERSION

-0.100.0
+0.102.0

changelogs/unreleased/18524-fix-double-brackets-in-wiki-markdown.yml

+---
+title: Fix double-brackets being linkified in wiki markdown
+merge_request: 18524
+author: brewingcode
+type: fixed

lib/banzai/filter/gollum_tags_filter.rb

@@ -58,6 +58,9 @@ module Banzai
 
       def call
         doc.search(".//text()").each do |node|
+          # Do not perform linking inside <code> blocks
+          next unless node.ancestors('code').empty?
+
           # A Gollum ToC tag is `[[_TOC_]]`, but due to MarkdownFilter running
           # before this one, it will be converted into `[[<em>TOC</em>]]`, so it
           # needs special-case handling

lib/gitlab/git/commit.rb

@@ -6,6 +6,7 @@ module Gitlab
 
       attr_accessor :raw_commit, :head
 
+      MAX_COMMIT_MESSAGE_DISPLAY_SIZE = 10.megabytes
       MIN_SHA_LENGTH = 7
       SERIALIZE_KEYS = [
         :id, :message, :parent_ids,
@@ -63,9 +64,7 @@ module Gitlab
             if is_enabled
               repo.gitaly_commit_client.find_commit(commit_id)
             else
-              obj = repo.rev_parse_target(commit_id)
-
-              obj.is_a?(Rugged::Commit) ? obj : nil
+              rugged_find(repo, commit_id)
             end
           end
 
@@ -76,6 +75,12 @@ module Gitlab
           nil
         end
 
+        def rugged_find(repo, commit_id)
+          obj = repo.rev_parse_target(commit_id)
+
+          obj.is_a?(Rugged::Commit) ? obj : nil
+        end
+
         # Get last commit for HEAD
         #
         # Ex.
@@ -297,11 +302,40 @@ module Gitlab
             nil
           end
         end
+
+        def get_message(repository, commit_id)
+          BatchLoader.for({ repository: repository, commit_id: commit_id }).batch do |items, loader|
+            items_by_repo = items.group_by { |i| i[:repository] }
+
+            items_by_repo.each do |repo, items|
+              commit_ids = items.map { |i| i[:commit_id] }
+
+              messages = get_messages(repository, commit_ids)
+
+              messages.each do |commit_sha, message|
+                loader.call({ repository: repository, commit_id: commit_sha }, message)
+              end
+            end
+          end
+        end
+
+        def get_messages(repository, commit_ids)
+          repository.gitaly_migrate(:commit_messages) do |is_enabled|
+            if is_enabled
+              repository.gitaly_commit_client.get_commit_messages(commit_ids)
+            else
+              commit_ids.map { |id| [id, rugged_find(repository, id).message] }.to_h
+            end
+          end
+        end
       end
 
       def initialize(repository, raw_commit, head = nil)
         raise "Nil as raw commit passed" unless raw_commit
 
+        @repository = repository
+        @head = head
+
         case raw_commit
         when Hash
           init_from_hash(raw_commit)
@@ -312,9 +346,6 @@ module Gitlab
         else
           raise "Invalid raw commit type: #{raw_commit.class}"
         end
-
-        @repository = repository
-        @head = head
       end
 
       def sha
@@ -518,7 +549,7 @@ module Gitlab
         # TODO: Once gitaly "takes over" Rugged consider separating the
         # subject from the message to make it clearer when there's one
         # available but not the other.
-        @message = (commit.body.presence || commit.subject).dup
+        @message = message_from_gitaly_body
         @authored_date = Time.at(commit.author.date.seconds).utc
         @author_name = commit.author.name.dup
         @author_email = commit.author.email.dup
@@ -570,6 +601,25 @@ module Gitlab
       def refs(repo)
         repo.refs_hash[id]
       end
+
+      def message_from_gitaly_body
+        return @raw_commit.subject.dup if @raw_commit.body_size.zero?
+        return @raw_commit.body.dup if full_body_fetched_from_gitaly?
+
+        if @raw_commit.body_size > MAX_COMMIT_MESSAGE_DISPLAY_SIZE
+          "#{@raw_commit.subject}\n\n--commit message is too big".strip
+        else
+          fetch_body_from_gitaly
+        end
+      end
+
+      def full_body_fetched_from_gitaly?
+        @raw_commit.body.bytesize == @raw_commit.body_size
+      end
+
+      def fetch_body_from_gitaly
+        self.class.get_message(@repository, id)
+      end
     end
   end
 end

lib/gitlab/git/repository.rb

@@ -1477,10 +1477,19 @@ module Gitlab
       def search_files_by_content(query, ref)
         return [] if empty? || query.blank?
 
-        offset = 2
-        args = %W(grep -i -I -n -z --before-context #{offset} --after-context #{offset} -E -e #{Regexp.escape(query)} #{ref || root_ref})
+        safe_query = Regexp.escape(query)
+        ref ||= root_ref
+
+        gitaly_migrate(:search_files_by_content) do |is_enabled|
+          if is_enabled
+            gitaly_repository_client.search_files_by_content(ref, safe_query)
+          else
+            offset = 2
+            args = %W(grep -i -I -n -z --before-context #{offset} --after-context #{offset} -E -e #{safe_query} #{ref})
 
-        run_git(args).first.scrub.split(/^--\n/)
+            run_git(args).first.scrub.split(/^--\n/)
+          end
+        end
       end
 
       def can_be_merged?(source_sha, target_branch)
@@ -1495,12 +1504,19 @@ module Gitlab
 
       def search_files_by_name(query, ref)
         safe_query = Regexp.escape(query.sub(%r{^/*}, ""))
+        ref ||= root_ref
 
         return [] if empty? || safe_query.blank?
 
-        args = %W(ls-tree -r --name-status --full-tree #{ref || root_ref} -- #{safe_query})
+        gitaly_migrate(:search_files_by_name) do |is_enabled|
+          if is_enabled
+            gitaly_repository_client.search_files_by_name(ref, safe_query)
+          else
+            args = %W(ls-tree -r --name-status --full-tree #{ref} -- #{safe_query})
 
-        run_git(args).first.lines.map(&:strip)
+            run_git(args).first.lines.map(&:strip)
+          end
+        end
       end
 
       def find_commits_by_message(query, ref, path, limit, offset)
@@ -1960,7 +1976,12 @@ module Gitlab
           end
 
           target_commit = Gitlab::Git::Commit.find(self, ref.target)
-          Gitlab::Git::Tag.new(self, ref.name, ref.target, target_commit, message)
+          Gitlab::Git::Tag.new(self, {
+            name: ref.name,
+            target: ref.target,
+            target_commit: target_commit,
+            message: message
+          })
         end.sort_by(&:name)
       end
 

lib/gitlab/git/repository_mirroring.rb

@@ -35,7 +35,11 @@ module Gitlab
           next if name =~ /\^\{\}\Z/
 
           target_commit = Gitlab::Git::Commit.find(self, target)
-          Gitlab::Git::Tag.new(self, name, target, target_commit)
+          Gitlab::Git::Tag.new(self, {
+            name: name,
+            target: target,
+            target_commit: target_commit
+          })
         end.compact
       end
 

lib/gitlab/git/tag.rb

 module Gitlab
   module Git
     class Tag < Ref
-      attr_reader :object_sha
+      extend Gitlab::EncodingHelper
+
+      attr_reader :object_sha, :repository
+
+      MAX_TAG_MESSAGE_DISPLAY_SIZE = 10.megabytes
+      SERIALIZE_KEYS = %i[name target target_commit message].freeze
+
+      attr_accessor *SERIALIZE_KEYS # rubocop:disable Lint/AmbiguousOperator
+
+      class << self
+        def get_message(repository, tag_id)
+          BatchLoader.for({ repository: repository, tag_id: tag_id }).batch do |items, loader|
+            items_by_repo = items.group_by { |i| i[:repository] }
+
+            items_by_repo.each do |repo, items|
+              tag_ids = items.map { |i| i[:tag_id] }
+
+              messages = get_messages(repository, tag_ids)
+
+              messages.each do |id, message|
+                loader.call({ repository: repository, tag_id: id }, message)
+              end
+            end
+          end
+        end
+
+        def get_messages(repository, tag_ids)
+          repository.gitaly_migrate(:tag_messages) do |is_enabled|
+            if is_enabled
+              repository.gitaly_ref_client.get_tag_messages(tag_ids)
+            else
+              tag_ids.map do |id|
+                tag = repository.rugged.lookup(id)
+                message = tag.is_a?(Rugged::Commit) ? "" : tag.message
+
+                [id, message]
+              end.to_h
+            end
+          end
+        end
+      end
+
+      def initialize(repository, raw_tag)
+        @repository = repository
+        @raw_tag = raw_tag
+
+        case raw_tag
+        when Hash
+          init_from_hash
+        when Gitaly::Tag
+          init_from_gitaly
+        end
 
-      def initialize(repository, name, target, target_commit, message = nil)
         super(repository, name, target, target_commit)
+      end
+
+      def init_from_hash
+        raw_tag = @raw_tag.symbolize_keys
+
+        SERIALIZE_KEYS.each do |key|
+          send("#{key}=", raw_tag[key]) # rubocop:disable GitlabSecurity/PublicSend
+        end
+      end
+
+      def init_from_gitaly
+        @name = encode!(@raw_tag.name.dup)
+        @target = @raw_tag.id
+        @message = message_from_gitaly_tag
 
-        @message = message
+        if @raw_tag.target_commit.present?
+          @target_commit = Gitlab::Git::Commit.decorate(repository, @raw_tag.target_commit)
+        end
       end
 
       def message
         encode! @message
       end
+
+      private
+
+      def message_from_gitaly_tag
+        return @raw_tag.message.dup if full_message_fetched_from_gitaly?
+
+        if @raw_tag.message_size > MAX_TAG_MESSAGE_DISPLAY_SIZE
+          '--tag message is too big'
+        else
+          self.class.get_message(@repository, target)
+        end
+      end
+
+      def full_message_fetched_from_gitaly?
+        @raw_tag.message.bytesize == @raw_tag.message_size
+      end
     end
   end
 end

lib/gitlab/gitaly_client/commit_service.rb

@@ -334,6 +334,22 @@ module Gitlab
         signatures
       end
 
+      def get_commit_messages(commit_ids)
+        request = Gitaly::GetCommitMessagesRequest.new(repository: @gitaly_repo, commit_ids: commit_ids)
+        response = GitalyClient.call(@repository.storage, :commit_service, :get_commit_messages, request)
+
+        messages = Hash.new { |h, k| h[k] = ''.b }
+        current_commit_id = nil
+
+        response.each do |rpc_message|
+          current_commit_id = rpc_message.commit_id if rpc_message.commit_id.present?
+
+          messages[current_commit_id] << rpc_message.message
+        end
+
+        messages
+      end
+
       private
 
       def call_commit_diff(request_params, options = {})

lib/gitlab/gitaly_client/operation_service.rb

@@ -40,7 +40,7 @@ module Gitlab
           raise Gitlab::Git::Repository::TagExistsError
         end
 
-        Util.gitlab_tag_from_gitaly_tag(@repository, response.tag)
+        Gitlab::Git::Tag.new(@repository, response.tag)
       rescue GRPC::FailedPrecondition => e
         raise Gitlab::Git::Repository::InvalidRef, e
       end

lib/gitlab/gitaly_client/ref_service.rb

@@ -171,6 +171,22 @@ module Gitlab
         consume_ref_contains_sha_response(stream, :branch_names)
       end
 
+      def get_tag_messages(tag_ids)
+        request = Gitaly::GetTagMessagesRequest.new(repository: @gitaly_repo, tag_ids: tag_ids)
+        response = GitalyClient.call(@repository.storage, :ref_service, :get_tag_messages, request)
+
+        messages = Hash.new { |h, k| h[k] = ''.b }
+        current_tag_id = nil
+
+        response.each do |rpc_message|
+          current_tag_id = rpc_message.tag_id if rpc_message.tag_id.present?
+
+          messages[current_tag_id] << rpc_message.message
+        end
+
+        messages
+      end
+
       private
 
       def consume_refs_response(response)
@@ -210,7 +226,7 @@ module Gitlab
 
       def consume_tags_response(response)
         response.flat_map do |message|
-          message.tags.map { |gitaly_tag| Util.gitlab_tag_from_gitaly_tag(@repository, gitaly_tag) }
+          message.tags.map { |gitaly_tag| Gitlab::Git::Tag.new(@repository, gitaly_tag) }
         end
       end
 

lib/gitlab/gitaly_client/repository_service.rb

@@ -301,6 +301,16 @@ module Gitlab
 
         GitalyClient.call(@storage, :repository_service, :get_raw_changes, request)
       end
+
+      def search_files_by_name(ref, query)
+        request = Gitaly::SearchFilesByNameRequest.new(repository: @gitaly_repo, ref: ref, query: query)
+        GitalyClient.call(@storage, :repository_service, :search_files_by_name, request).flat_map(&:files)
+      end
+
+      def search_files_by_content(ref, query)
+        request = Gitaly::SearchFilesByContentRequest.new(repository: @gitaly_repo, ref: ref, query: query)
+        GitalyClient.call(@storage, :repository_service, :search_files_by_content, request).flat_map(&:matches)
+      end
     end
   end
 end

lib/gitlab/gitaly_client/util.rb

@@ -21,20 +21,6 @@ module Gitlab
                                       gitaly_repository.relative_path,
                                       gitaly_repository.gl_repository)
         end
-
-        def gitlab_tag_from_gitaly_tag(repository, gitaly_tag)
-          if gitaly_tag.target_commit.present?
-            commit = Gitlab::Git::Commit.decorate(repository, gitaly_tag.target_commit)
-          end
-
-          Gitlab::Git::Tag.new(
-            repository,
-            Gitlab::EncodingHelper.encode!(gitaly_tag.name.dup),
-            gitaly_tag.id,
-            commit,
-            Gitlab::EncodingHelper.encode!(gitaly_tag.message.chomp)
-          )
-        end
       end
     end
   end

spec/factories/gitaly/tag.rb

+FactoryBot.define do
+  factory :gitaly_tag, class: Gitaly::Tag do
+    skip_create
+
+    name { 'v3.1.4' }
+    message { 'Pie release' }
+    target_commit factory: :gitaly_commit
+  end
+end

spec/features/projects/wiki/markdown_preview_spec.rb

@@ -155,4 +155,27 @@ feature 'Projects > Wiki > User previews markdown changes', :js do
       end
     end
   end
+
+  it "does not linkify double brackets inside code blocks as expected" do
+    click_link 'New page'
+    page.within '#modal-new-wiki' do
+      fill_in :new_wiki_path, with: 'linkify_test'
+      click_button 'Create page'
+    end
+
+    page.within '.wiki-form' do
+      fill_in :wiki_content, with: <<-HEREDOC
+        `[[do_not_linkify]]`
+        ```
+        [[also_do_not_linkify]]
+        ```
+      HEREDOC
+      click_on "Preview"
+    end
+
+    expect(page).to have_content("do_not_linkify")
+
+    expect(page.html).to include('[[do_not_linkify]]')
+    expect(page.html).to include('[[also_do_not_linkify]]')
+  end
 end

spec/lib/banzai/filter/gollum_tags_filter_spec.rb

@@ -91,6 +91,12 @@ describe Banzai::Filter::GollumTagsFilter do
       expect(doc.at_css('a').text).to eq 'link-text'
       expect(doc.at_css('a')['href']).to eq expected_path
     end
+
+    it "inside back ticks will be exempt from linkification" do
+      doc = filter('<code>[[link-in-backticks]]</code>', project_wiki: project_wiki)
+
+      expect(doc.at_css('code').text).to eq '[[link-in-backticks]]'
+    end
   end
 
   context 'table of contents' do

spec/lib/gitlab/git/commit_spec.rb

@@ -66,7 +66,8 @@ describe Gitlab::Git::Commit, seed_helper: true do
   describe "Commit info from gitaly commit" do
     let(:subject) { "My commit".force_encoding('ASCII-8BIT') }
     let(:body) { subject + "My body".force_encoding('ASCII-8BIT') }
-    let(:gitaly_commit) { build(:gitaly_commit, subject: subject, body: body) }
+    let(:body_size) { body.length }
+    let(:gitaly_commit) { build(:gitaly_commit, subject: subject, body: body, body_size: body_size) }
     let(:id) { gitaly_commit.id }
     let(:committer) { gitaly_commit.committer }
     let(:author) { gitaly_commit.author }
@@ -83,10 +84,30 @@ describe Gitlab::Git::Commit, seed_helper: true do
     it { expect(commit.committer_email).to eq(committer.email) }
     it { expect(commit.parent_ids).to eq(gitaly_commit.parent_ids) }
 
-    context 'no body' do
+    context 'body_size != body.size' do
       let(:body) { "".force_encoding('ASCII-8BIT') }
 
-      it { expect(commit.safe_message).to eq(subject) }
+      context 'zero body_size' do
+        it { expect(commit.safe_message).to eq(subject) }
+      end
+
+      context 'body_size less than threshold' do
+        let(:body_size) { 123 }
+
+        it 'fetches commit message seperately' do
+          expect(described_class).to receive(:get_message).with(repository, id)
+
+          commit.safe_message
+        end
+      end
+
+      context 'body_size greater than threshold' do
+        let(:body_size) { described_class::MAX_COMMIT_MESSAGE_DISPLAY_SIZE + 1 }
+
+        it 'returns the suject plus a notice about message size' do
+          expect(commit.safe_message).to eq("My commit\n\n--commit message is too big")
+        end
+      end
     end
   end
 
@@ -589,6 +610,35 @@ describe Gitlab::Git::Commit, seed_helper: true do
     it { is_expected.not_to include("feature") }
   end
 
+  describe '.get_message' do
+    let(:commit_ids) { %w[6d394385cf567f80a8fd85055db1ab4c5295806f cfe32cf61b73a0d5e9f13e774abde7ff789b1660] }
+
+    subject do
+      commit_ids.map { |id| described_class.get_message(repository, id) }
+    end
+
+    shared_examples 'getting commit messages' do
+      it 'gets commit messages' do
+        expect(subject).to contain_exactly(
+          "Added contributing guide\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n",
+          "Add submodule\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n"
+        )
+      end
+    end
+
+    context 'when Gitaly commit_messages feature is enabled' do
+      it_behaves_like 'getting commit messages'
+
+      it 'gets messages in one batch', :request_store do
+        expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1)
+      end
+    end
+
+    context 'when Gitaly commit_messages feature is disabled', :disable_gitaly do
+      it_behaves_like 'getting commit messages'
+    end
+  end
+
   def sample_commit_hash
     {
       author_email: "dmitriy.zaporozhets@gmail.com",

spec/lib/gitlab/git/tag_spec.rb

@@ -32,4 +32,56 @@ describe Gitlab::Git::Tag, seed_helper: true do
   context 'when Gitaly tags feature is disabled', :skip_gitaly_mock do
     it_behaves_like 'Gitlab::Git::Repository#tags'
   end
+
+  describe '.get_message' do
+    let(:tag_ids) { %w[f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b] }
+
+    subject do
+      tag_ids.map { |id| described_class.get_message(repository, id) }
+    end
+
+    shared_examples 'getting tag messages' do
+      it 'gets tag messages' do
+        expect(subject[0]).to eq("Release\n")
+        expect(subject[1]).to eq("Version 1.1.0\n")
+      end
+    end
+
+    context 'when Gitaly tag_messages feature is enabled' do
+      it_behaves_like 'getting tag messages'
+
+      it 'gets messages in one batch', :request_store do
+        expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1)
+      end
+    end
+
+    context 'when Gitaly tag_messages feature is disabled', :disable_gitaly do
+      it_behaves_like 'getting tag messages'
+    end
+  end
+
+  describe 'tag into from Gitaly tag' do
+    context 'message_size != message.size' do
+      let(:gitaly_tag) { build(:gitaly_tag, message: ''.b, message_size: message_size) }
+      let(:tag) { described_class.new(repository, gitaly_tag) }
+
+      context 'message_size less than threshold' do
+        let(:message_size) { 123 }
+
+        it 'fetches tag message seperately' do
+          expect(described_class).to receive(:get_message).with(repository, gitaly_tag.id)
+
+          tag.message
+        end
+      end
+
+      context 'message_size greater than threshold' do
+        let(:message_size) { described_class::MAX_TAG_MESSAGE_DISPLAY_SIZE + 1 }
+
+        it 'returns a notice about message size' do
+          expect(tag.message).to eq("--tag message is too big")
+        end
+      end
+    end
+  end
 end

spec/models/repository_spec.rb

@@ -671,7 +671,7 @@ describe Repository do
     end
   end
 
-  describe "search_files_by_content" do
+  shared_examples "search_files_by_content" do
     let(:results) { repository.search_files_by_content('feature', 'master') }
     subject { results }
 
@@ -718,7 +718,7 @@ describe Repository do
     end
   end
 
-  describe "search_files_by_name" do
+  shared_examples "search_files_by_name" do
     let(:results) { repository.search_files_by_name('files', 'master') }
 
     it 'returns result' do
@@ -758,6 +758,16 @@ describe Repository do
     end
   end
 
+  describe 'with gitaly enabled' do
+    it_behaves_like 'search_files_by_content'
+    it_behaves_like 'search_files_by_name'
+  end
+
+  describe 'with gitaly disabled', :disable_gitaly do
+    it_behaves_like 'search_files_by_content'
+    it_behaves_like 'search_files_by_name'
+  end
+
   describe '#async_remove_remote' do
     before do
       masterrev = repository.find_branch('master').dereferenced_target

spec/services/projects/update_remote_mirror_service_spec.rb

@@ -343,7 +343,11 @@ describe Projects::UpdateRemoteMirrorService do
       tag = repository.find_tag(name)
       target = tag.try(:target)
       target_commit = tag.try(:dereferenced_target)
-      tags << Gitlab::Git::Tag.new(repository.raw_repository, name, target, target_commit)
+      tags << Gitlab::Git::Tag.new(repository.raw_repository, {
+        name: name,
+        target: target,
+        target_commit: target_commit
+      })
     end
   end