diff options
author | Rémy Coutable <remy@rymai.me> | 2017-04-05 09:25:54 +0200 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2017-04-10 16:10:18 +0200 |
commit | 05aa038c42a64da76a419b8177ba6c7242f67687 (patch) | |
tree | 238dd03ec4ea03d85a03117d7a2d6859efa16366 | |
parent | 9c576cc7e9414e64b4aac0544615e7d5e92c3558 (diff) | |
download | gitlab-ce-05aa038c42a64da76a419b8177ba6c7242f67687.tar.gz |
Add a Gitlab::Git::Env to store Git-specific env thread-safely
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r-- | lib/gitlab/git/env.rb | 38 | ||||
-rw-r--r-- | spec/lib/gitlab/git/env_spec.rb | 102 |
2 files changed, 140 insertions, 0 deletions
diff --git a/lib/gitlab/git/env.rb b/lib/gitlab/git/env.rb new file mode 100644 index 00000000000..0fdc57ec954 --- /dev/null +++ b/lib/gitlab/git/env.rb @@ -0,0 +1,38 @@ +module Gitlab + module Git + # Ephemeral (per request) storage for environment variables that some Git + # commands may need. + # + # For example, in pre-receive hooks, new objects are put in a temporary + # $GIT_OBJECT_DIRECTORY. Without it set, the new objects cannot be retrieved + # (this would break push rules for instance). + # + # This class is thread-safe via RequestStore. + class Env + WHITELISTED_GIT_VARIABLES = %w[ + GIT_OBJECT_DIRECTORY + GIT_ALTERNATE_OBJECT_DIRECTORIES + ].freeze + + def self.set(env) + return unless RequestStore.active? + + RequestStore.store[:gitlab_git_env] = whitelist_git_env(env) + end + + def self.all + return {} unless RequestStore.active? + + RequestStore.fetch(:gitlab_git_env) { {} } + end + + def self.[](key) + all[key] + end + + def self.whitelist_git_env(env) + env.select { |key, _| WHITELISTED_GIT_VARIABLES.include?(key.to_s) }.with_indifferent_access + end + end + end +end diff --git a/spec/lib/gitlab/git/env_spec.rb b/spec/lib/gitlab/git/env_spec.rb new file mode 100644 index 00000000000..d9df99bfe05 --- /dev/null +++ b/spec/lib/gitlab/git/env_spec.rb @@ -0,0 +1,102 @@ +require 'spec_helper' + +describe Gitlab::Git::Env do + describe "#set" do + context 'with RequestStore.store disabled' do + before do + allow(RequestStore).to receive(:active?).and_return(false) + end + + it 'does not store anything' do + described_class.set(GIT_OBJECT_DIRECTORY: 'foo') + + expect(described_class.all).to be_empty + end + end + + context 'with RequestStore.store enabled' do + before do + allow(RequestStore).to receive(:active?).and_return(true) + end + + it 'whitelist some `GIT_*` variables and stores them using RequestStore' do + described_class.set( + GIT_OBJECT_DIRECTORY: 'foo', + GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar', + GIT_EXEC_PATH: 'baz', + PATH: '~/.bin:/bin') + + expect(described_class[:GIT_OBJECT_DIRECTORY]).to eq('foo') + expect(described_class[:GIT_ALTERNATE_OBJECT_DIRECTORIES]).to eq('bar') + expect(described_class[:GIT_EXEC_PATH]).to be_nil + expect(described_class[:bar]).to be_nil + end + end + end + + describe "#all" do + context 'with RequestStore.store enabled' do + before do + allow(RequestStore).to receive(:active?).and_return(true) + described_class.set( + GIT_OBJECT_DIRECTORY: 'foo', + GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar') + end + + it 'returns an env hash' do + expect(described_class.all).to eq({ + 'GIT_OBJECT_DIRECTORY' => 'foo', + 'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'bar' + }) + end + end + end + + describe "#[]" do + context 'with RequestStore.store enabled' do + before do + allow(RequestStore).to receive(:active?).and_return(true) + end + + before do + described_class.set( + GIT_OBJECT_DIRECTORY: 'foo', + GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar') + end + + it 'returns a stored value for an existing key' do + expect(described_class[:GIT_OBJECT_DIRECTORY]).to eq('foo') + end + + it 'returns nil for an non-existing key' do + expect(described_class[:foo]).to be_nil + end + end + end + + describe 'thread-safety' do + context 'with RequestStore.store enabled' do + before do + allow(RequestStore).to receive(:active?).and_return(true) + described_class.set(GIT_OBJECT_DIRECTORY: 'foo') + end + + it 'is thread-safe' do + another_thread = Thread.new do + described_class.set(GIT_OBJECT_DIRECTORY: 'bar') + + Thread.stop + described_class[:GIT_OBJECT_DIRECTORY] + end + + # Ensure another_thread runs first + sleep 0.1 until another_thread.stop? + + expect(described_class[:GIT_OBJECT_DIRECTORY]).to eq('foo') + + another_thread.run + expect(another_thread.value).to eq('bar') + end + end + end +end |