Merge branch 'gitaly-mandatory-20180704-jv' into 'master'

Make blob and other RPC's mandatory

Closes gitaly#331, gitaly#384, gitaly#446, and gitaly#383

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

5 files changed, 36 insertions, 171 deletions

diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb
index 604bb11e712..96fa94d5790 100644
--- a/lib/gitlab/git/blob.rb
+++ b/lib/gitlab/git/blob.rb
@@ -50,13 +50,7 @@ module Gitlab
         end
 
         def raw(repository, sha)
-          Gitlab::GitalyClient.migrate(:git_blob_raw) do |is_enabled|
-            if is_enabled
-              repository.gitaly_blob_client.get_blob(oid: sha, limit: MAX_DATA_DISPLAY_SIZE)
-            else
-              rugged_raw(repository, sha, limit: MAX_DATA_DISPLAY_SIZE)
-            end
-          end
+          repository.gitaly_blob_client.get_blob(oid: sha, limit: MAX_DATA_DISPLAY_SIZE)
         end
 
         # Returns an array of Blob instances, specified in blob_references as
@@ -207,16 +201,7 @@ module Gitlab
 
         return if @loaded_all_data
 
-        @data = Gitlab::GitalyClient.migrate(:git_blob_load_all_data) do |is_enabled|
-          begin
-            if is_enabled
-              repository.gitaly_blob_client.get_blob(oid: id, limit: -1).data
-            else
-              repository.lookup(id).content
-            end
-          end
-        end
-
+        @data = repository.gitaly_blob_client.get_blob(oid: id, limit: -1).data
         @loaded_all_data = true
         @loaded_size = @data.bytesize
       end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index b355e5e87ff..420790f45d0 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -439,31 +439,11 @@ module Gitlab
           raise ArgumentError.new("invalid Repository#log limit: #{limit.inspect}")
         end
 
-        gitaly_migrate(:find_commits) do |is_enabled|
-          if is_enabled
-            gitaly_commit_client.find_commits(options)
-          else
-            raw_log(options).map { |c| Commit.decorate(self, c) }
-          end
+        wrapped_gitaly_errors do
+          gitaly_commit_client.find_commits(options)
         end
       end
 
-      # Used in gitaly-ruby
-      def raw_log(options)
-        sha =
-          unless options[:all]
-            actual_ref = options[:ref] || root_ref
-            begin
-              sha_from_ref(actual_ref)
-            rescue Rugged::OdbError, Rugged::InvalidError, Rugged::ReferenceError
-              # Return an empty array if the ref wasn't found
-              return []
-            end
-          end
-
-        log_by_shell(sha, options)
-      end
-
       def count_commits(options)
         options = process_count_commits_options(options.dup)
 
@@ -1223,12 +1203,10 @@ module Gitlab
       end
 
       def find_commits_by_message(query, ref, path, limit, offset)
-        gitaly_migrate(:commits_by_message) do |is_enabled|
-          if is_enabled
-            find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
-          else
-            find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
-          end
+        wrapped_gitaly_errors do
+          gitaly_commit_client
+            .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
+            .map { |c| commit(c) }
         end
       end
 
@@ -1238,12 +1216,8 @@ module Gitlab
       end
 
       def last_commit_for_path(sha, path)
-        gitaly_migrate(:last_commit_for_path) do |is_enabled|
-          if is_enabled
-            last_commit_for_path_by_gitaly(sha, path)
-          else
-            last_commit_for_path_by_rugged(sha, path)
-          end
+        wrapped_gitaly_errors do
+          gitaly_commit_client.last_commit_for_path(sha, path)
         end
       end
 
@@ -1454,46 +1428,6 @@ module Gitlab
         end
       end
 
-      # Gitaly note: JV: although #log_by_shell shells out to Git I think the
-      # complexity is such that we should migrate it as Ruby before trying to
-      # do it in Go.
-      def log_by_shell(sha, options)
-        limit = options[:limit].to_i
-        offset = options[:offset].to_i
-        use_follow_flag = options[:follow] && options[:path].present?
-
-        # We will perform the offset in Ruby because --follow doesn't play well with --skip.
-        # See: https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520
-        offset_in_ruby = use_follow_flag && options[:offset].present?
-        limit += offset if offset_in_ruby
-
-        cmd = %w[log]
-        cmd << "--max-count=#{limit}"
-        cmd << '--format=%H'
-        cmd << "--skip=#{offset}" unless offset_in_ruby
-        cmd << '--follow' if use_follow_flag
-        cmd << '--no-merges' if options[:skip_merges]
-        cmd << "--after=#{options[:after].iso8601}" if options[:after]
-        cmd << "--before=#{options[:before].iso8601}" if options[:before]
-
-        if options[:all]
-          cmd += %w[--all --reverse]
-        else
-          cmd << sha
-        end
-
-        # :path can be a string or an array of strings
-        if options[:path].present?
-          cmd << '--'
-          cmd += Array(options[:path])
-        end
-
-        raw_output, _status = run_git(cmd)
-        lines = offset_in_ruby ? raw_output.lines.drop(offset) : raw_output.lines
-
-        lines.map! { |c| Rugged::Commit.new(rugged, c.strip) }
-      end
-
       # We are trying to deprecate this method because it does a lot of work
       # but it seems to be used only to look up submodule URL's.
       # https://gitlab.com/gitlab-org/gitaly/issues/329
@@ -1623,11 +1557,6 @@ module Gitlab
         end
       end
 
-      def last_commit_for_path_by_rugged(sha, path)
-        sha = last_commit_id_for_path_by_shelling_out(sha, path)
-        commit(sha)
-      end
-
       # Returns true if the given ref name exists
       #
       # Ref names must start with `refs/`.
@@ -1820,35 +1749,6 @@ module Gitlab
         raise CommandError, @gitlab_projects.output
       end
 
-      def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
-        ref ||= root_ref
-
-        args = %W(
-          log #{ref} --pretty=%H --skip #{offset}
-          --max-count #{limit} --grep=#{query} --regexp-ignore-case
-        )
-        args = args.concat(%W(-- #{path})) if path.present?
-
-        git_log_results = run_git(args).first.lines
-
-        git_log_results.map { |c| commit(c.chomp) }.compact
-      end
-
-      def find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
-        gitaly_commit_client
-          .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
-          .map { |c| commit(c) }
-      end
-
-      def last_commit_for_path_by_gitaly(sha, path)
-        gitaly_commit_client.last_commit_for_path(sha, path)
-      end
-
-      def last_commit_id_for_path_by_shelling_out(sha, path)
-        args = %W(rev-list --max-count=1 #{sha} -- #{path})
-        run_git_with_timeout(args, Gitlab::Git::Popen::FAST_GIT_PROCESS_TIMEOUT).first.strip
-      end
-
       def rugged_merge_base(from, to)
         rugged.merge_base(from, to)
       rescue Rugged::ReferenceError
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index 79483ee05f8..55c899912f9 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -37,21 +37,14 @@ module Gitlab
       end
 
       def send_git_blob(repository, blob)
-        params = if Gitlab::GitalyClient.feature_enabled?(:workhorse_raw_show, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT)
-                   {
-                     'GitalyServer' => gitaly_server_hash(repository),
-                     'GetBlobRequest' => {
-                       repository: repository.gitaly_repository.to_h,
-                       oid: blob.id,
-                       limit: -1
-                     }
-                   }
-                 else
-                   {
-                     'RepoPath' => repository.path_to_repo,
-                     'BlobId' => blob.id
-                   }
-                 end
+        params = {
+          'GitalyServer' => gitaly_server_hash(repository),
+          'GetBlobRequest' => {
+            repository: repository.gitaly_repository.to_h,
+            oid: blob.id,
+            limit: -1
+          }
+        }
 
         [
           SEND_DATA_HEADER,
diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb
index b6061df349d..6900c4189b7 100644
--- a/spec/lib/gitlab/git/blob_spec.rb
+++ b/spec/lib/gitlab/git/blob_spec.rb
@@ -532,8 +532,8 @@ describe Gitlab::Git::Blob, seed_helper: true do
     subject { blob.load_all_data!(repository) }
 
     it 'loads missing data' do
-      expect(Gitlab::GitalyClient).to receive(:migrate)
-        .with(:git_blob_load_all_data).and_return(full_data)
+      expect(repository.gitaly_blob_client).to receive(:get_blob)
+        .and_return(double(:response, data: full_data))
 
       subject
 
@@ -544,8 +544,7 @@ describe Gitlab::Git::Blob, seed_helper: true do
       let(:blob) { Gitlab::Git::Blob.new(name: 'test', size: 4, data: full_data) }
 
       it "doesn't perform any loading" do
-        expect(Gitlab::GitalyClient).not_to receive(:migrate)
-          .with(:git_blob_load_all_data)
+        expect(repository.gitaly_blob_client).not_to receive(:get_blob)
 
         subject
 
diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
index 570580e0cfc..8fdcfe79fb5 100644
--- a/spec/lib/gitlab/workhorse_spec.rb
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -413,34 +413,22 @@ describe Gitlab::Workhorse do
 
     subject { described_class.send_git_blob(repository, blob) }
 
-    context 'when Gitaly workhorse_raw_show feature is enabled' do
-      it 'sets the header correctly' do
-        key, command, params = decode_workhorse_header(subject)
-
-        expect(key).to eq('Gitlab-Workhorse-Send-Data')
-        expect(command).to eq('git-blob')
-        expect(params).to eq({
-          'GitalyServer' => {
-            address: Gitlab::GitalyClient.address(project.repository_storage),
-            token: Gitlab::GitalyClient.token(project.repository_storage)
-          },
-          'GetBlobRequest' => {
-            repository: repository.gitaly_repository.to_h,
-            oid: blob.id,
-            limit: -1
-          }
-        }.deep_stringify_keys)
-      end
-    end
-
-    context 'when Gitaly workhorse_raw_show feature is disabled', :disable_gitaly do
-      it 'sets the header correctly' do
-        key, command, params = decode_workhorse_header(subject)
+    it 'sets the header correctly' do
+      key, command, params = decode_workhorse_header(subject)
 
-        expect(key).to eq('Gitlab-Workhorse-Send-Data')
-        expect(command).to eq('git-blob')
-        expect(params).to eq('RepoPath' => repository.path_to_repo, 'BlobId' => blob.id)
-      end
+      expect(key).to eq('Gitlab-Workhorse-Send-Data')
+      expect(command).to eq('git-blob')
+      expect(params).to eq({
+        'GitalyServer' => {
+          address: Gitlab::GitalyClient.address(project.repository_storage),
+          token: Gitlab::GitalyClient.token(project.repository_storage)
+        },
+        'GetBlobRequest' => {
+          repository: repository.gitaly_repository.to_h,
+          oid: blob.id,
+          limit: -1
+        }
+      }.deep_stringify_keys)
     end
   end