Merge branch 'gitaly-set-delete-config' into 'master'

Add Repository#set_config and #delete_config

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

GITALY_SERVER_VERSION

@@ -1 +1 @@
-0.110.0
+0.111.0

Gemfile

@@ -418,7 +418,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC client
-gem 'gitaly-proto', '~> 0.103.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 0.105.0', require: 'gitaly'
 gem 'grpc', '~> 1.11.0'
 
 # Locked until https://github.com/google/protobuf/issues/4210 is closed

Gemfile.lock

@@ -282,7 +282,7 @@ GEM
       gettext_i18n_rails (>= 0.7.1)
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
-    gitaly-proto (0.103.0)
+    gitaly-proto (0.105.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.10)
     github-linguist (5.3.3)
@@ -1037,7 +1037,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly-proto (~> 0.103.0)
+  gitaly-proto (~> 0.105.0)
   github-linguist (~> 5.3.3)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-gollum-lib (~> 4.2)

Gemfile.rails5.lock

@@ -285,7 +285,7 @@ GEM
       gettext_i18n_rails (>= 0.7.1)
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
-    gitaly-proto (0.103.0)
+    gitaly-proto (0.105.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.10)
     github-linguist (5.3.3)
@@ -1047,7 +1047,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly-proto (~> 0.103.0)
+  gitaly-proto (~> 0.105.0)
   github-linguist (~> 5.3.3)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-gollum-lib (~> 4.2)

lib/gitlab/git/repository.rb

@@ -1115,8 +1115,18 @@ module Gitlab
         # This guard avoids Gitaly log/error spam
         raise NoRepository, 'repository does not exist' unless exists?
 
+        set_config('gitlab.fullpath' => full_path)
+      end
+
+      def set_config(entries)
         wrapped_gitaly_errors do
-          gitaly_repository_client.write_config(full_path: full_path)
+          gitaly_repository_client.set_config(entries)
+        end
+      end
+
+      def delete_config(*keys)
+        wrapped_gitaly_errors do
+          gitaly_repository_client.delete_config(keys)
         end
       end
 

lib/gitlab/gitaly_client/repository_service.rb

@@ -265,17 +265,39 @@ module Gitlab
         true
       end
 
-      def write_config(full_path:)
+      def set_config(entries)
-        request = Gitaly::WriteConfigRequest.new(repository: @gitaly_repo, full_path: full_path)
+        return if entries.empty?
-        response = GitalyClient.call(
+
+        request = Gitaly::SetConfigRequest.new(repository: @gitaly_repo)
+        entries.each do |key, value|
+          request.entries << build_set_config_entry(key, value)
+        end
+
+        GitalyClient.call(
           @storage,
           :repository_service,
-          :write_config,
+          :set_config,
           request,
           timeout: GitalyClient.fast_timeout
         )
 
-        raise Gitlab::Git::OSError.new(response.error) unless response.error.empty?
+        nil
+      end
+
+      def delete_config(keys)
+        return if keys.empty?
+
+        request = Gitaly::DeleteConfigRequest.new(repository: @gitaly_repo, keys: keys)
+
+        GitalyClient.call(
+          @storage,
+          :repository_service,
+          :delete_config,
+          request,
+          timeout: GitalyClient.fast_timeout
+        )
+
+        nil
       end
 
       def license_short_name
@@ -352,6 +374,23 @@ module Gitlab
           timeout: timeout
         )
       end
+
+      def build_set_config_entry(key, value)
+        entry = Gitaly::SetConfigRequest::Entry.new(key: key)
+
+        case value
+        when String
+          entry.value_str = value
+        when Integer
+          entry.value_int32 = value
+        when TrueClass, FalseClass
+          entry.value_bool = value
+        else
+          raise InvalidArgument, "invalid git config value: #{value.inspect}"
+        end
+
+        entry
+      end
     end
   end
 end

spec/lib/gitlab/git/repository_spec.rb

@@ -1856,6 +1856,54 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
+  describe '#set_config' do
+    let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
+    let(:rugged) { repository_rugged }
+    let(:entries) do
+      {
+        'test.foo1' => 'bla bla',
+        'test.foo2' => 1234,
+        'test.foo3' => true
+      }
+    end
+
+    it 'can set config settings' do
+      expect(repository.set_config(entries)).to be_nil
+
+      expect(rugged.config['test.foo1']).to eq('bla bla')
+      expect(rugged.config['test.foo2']).to eq('1234')
+      expect(rugged.config['test.foo3']).to eq('true')
+    end
+
+    after do
+      entries.keys.each { |k| rugged.config.delete(k) }
+    end
+  end
+
+  describe '#delete_config' do
+    let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
+    let(:rugged) { repository_rugged }
+    let(:entries) do
+      {
+        'test.foo1' => 'bla bla',
+        'test.foo2' => 1234,
+        'test.foo3' => true
+      }
+    end
+
+    it 'can delete config settings' do
+      entries.each do |key, value|
+        rugged.config[key] = value
+      end
+
+      expect(repository.delete_config(*%w[does.not.exist test.foo1 test.foo2])).to be_nil
+
+      config_keys = rugged.config.each_key.to_a
+      expect(config_keys).not_to include('test.foo1')
+      expect(config_keys).not_to include('test.foo2')
+    end
+  end
+
   describe '#merge' do
     let(:repository) do
       Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')