Merge branch 'fix/handle-large-commit-tag-message' into 'master'

Add handling for commit/tags with big messages

See merge request gitlab-org/gitlab-ce!17892

Author: Robert Speicher

.flayignore

@@ -10,3 +10,7 @@ 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

lib/gitlab/git/commit.rb

@@ -6,6 +6,7 @@ class Commit
 
       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 @@ def find(repo, commit_id = "HEAD")
             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 @@ def find(repo, commit_id = "HEAD")
           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 @@ def rugged_extract_signature(repository, commit_id)
             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 @@ def initialize(repository, raw_commit, head = nil)
         else
           raise "Invalid raw commit type: #{raw_commit.class}"
         end
-
-        @repository = repository
-        @head = head
       end
 
       def sha
@@ -518,7 +549,7 @@ def init_from_gitaly(commit)
         # 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 @@ def gitaly_commit_author_from_rugged(author_or_committer)
       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

@@ -1962,7 +1962,12 @@ def tags_from_rugged
           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 @@ def remote_tags(remote)
           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

@@ -1,17 +1,99 @@
 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 @@ def get_commit_signatures(commit_ids)
         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 @@ def add_tag(tag_name, user, target, message)
           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 @@ def branch_names_contains_sha(sha, limit: 0)
         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 @@ def consume_find_all_branches_response(response)
 
       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/util.rb

@@ -21,20 +21,6 @@ def git_repository(gitaly_repository)
                                       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

@@ -0,0 +1,9 @@
+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