Commit e3a6fedb authored by Sean McGivern's avatar Sean McGivern

Merge branch 'postreceive-gl-repository' into 'master'

Use gl_repository exclusively on PostReceive

Closes gitaly#201

See merge request !12290
parents 36e17d0f e7470534
...@@ -2,11 +2,11 @@ class PostReceive ...@@ -2,11 +2,11 @@ class PostReceive
include Sidekiq::Worker include Sidekiq::Worker
include DedicatedSidekiqQueue include DedicatedSidekiqQueue
def perform(project_identifier, identifier, changes) def perform(gl_repository, identifier, changes)
project, is_wiki = parse_project_identifier(project_identifier) project, is_wiki = Gitlab::GlRepository.parse(gl_repository)
if project.nil? if project.nil?
log("Triggered hook for non-existing project with identifier \"#{project_identifier}\"") log("Triggered hook for non-existing project with gl_repository \"#{gl_repository}\"")
return false return false
end end
...@@ -59,21 +59,6 @@ class PostReceive ...@@ -59,21 +59,6 @@ class PostReceive
# Nothing defined here yet. # Nothing defined here yet.
end end
# To maintain backwards compatibility, we accept both gl_repository or
# repository paths as project identifiers. Our plan is to migrate to
# gl_repository only with the following plan:
# 9.2: Handle both possible values. Keep Gitlab-Shell sending only repo paths
# 9.3 (or patch release): Make GitLab Shell pass gl_repository if present
# 9.4 (or patch release): Make GitLab Shell always pass gl_repository
# 9.5 (or patch release): Handle only gl_repository as project identifier on this method
def parse_project_identifier(project_identifier)
if project_identifier.start_with?('/')
Gitlab::RepoPath.parse(project_identifier)
else
Gitlab::GlRepository.parse(project_identifier)
end
end
def log(message) def log(message)
Gitlab::GitLogger.error("POST-RECEIVE: #{message}") Gitlab::GitLogger.error("POST-RECEIVE: #{message}")
end end
......
...@@ -4,7 +4,7 @@ describe PostReceive do ...@@ -4,7 +4,7 @@ describe PostReceive do
let(:changes) { "123456 789012 refs/heads/tést\n654321 210987 refs/tags/tag" } let(:changes) { "123456 789012 refs/heads/tést\n654321 210987 refs/tags/tag" }
let(:wrongly_encoded_changes) { changes.encode("ISO-8859-1").force_encoding("UTF-8") } let(:wrongly_encoded_changes) { changes.encode("ISO-8859-1").force_encoding("UTF-8") }
let(:base64_changes) { Base64.encode64(wrongly_encoded_changes) } let(:base64_changes) { Base64.encode64(wrongly_encoded_changes) }
let(:project_identifier) { "project-#{project.id}" } let(:gl_repository) { "project-#{project.id}" }
let(:key) { create(:key, user: project.owner) } let(:key) { create(:key, user: project.owner) }
let(:key_id) { key.shell_id } let(:key_id) { key.shell_id }
...@@ -19,22 +19,14 @@ describe PostReceive do ...@@ -19,22 +19,14 @@ describe PostReceive do
end end
context 'with a non-existing project' do context 'with a non-existing project' do
let(:project_identifier) { "project-123456789" } let(:gl_repository) { "project-123456789" }
let(:error_message) do let(:error_message) do
"Triggered hook for non-existing project with identifier \"#{project_identifier}\"" "Triggered hook for non-existing project with gl_repository \"#{gl_repository}\""
end end
it "returns false and logs an error" do it "returns false and logs an error" do
expect(Gitlab::GitLogger).to receive(:error).with("POST-RECEIVE: #{error_message}") expect(Gitlab::GitLogger).to receive(:error).with("POST-RECEIVE: #{error_message}")
expect(described_class.new.perform(project_identifier, key_id, base64_changes)).to be(false) expect(described_class.new.perform(gl_repository, key_id, base64_changes)).to be(false)
end
end
context "with an absolute path as the project identifier" do
it "searches the project by full path" do
expect(Project).to receive(:find_by_full_path).with(project.full_path, follow_redirects: true).and_call_original
described_class.new.perform(pwd(project), key_id, base64_changes)
end end
end end
...@@ -49,7 +41,7 @@ describe PostReceive do ...@@ -49,7 +41,7 @@ describe PostReceive do
it "calls GitTagPushService" do it "calls GitTagPushService" do
expect_any_instance_of(GitPushService).to receive(:execute).and_return(true) expect_any_instance_of(GitPushService).to receive(:execute).and_return(true)
expect_any_instance_of(GitTagPushService).not_to receive(:execute) expect_any_instance_of(GitTagPushService).not_to receive(:execute)
described_class.new.perform(project_identifier, key_id, base64_changes) described_class.new.perform(gl_repository, key_id, base64_changes)
end end
end end
...@@ -59,7 +51,7 @@ describe PostReceive do ...@@ -59,7 +51,7 @@ describe PostReceive do
it "calls GitTagPushService" do it "calls GitTagPushService" do
expect_any_instance_of(GitPushService).not_to receive(:execute) expect_any_instance_of(GitPushService).not_to receive(:execute)
expect_any_instance_of(GitTagPushService).to receive(:execute).and_return(true) expect_any_instance_of(GitTagPushService).to receive(:execute).and_return(true)
described_class.new.perform(project_identifier, key_id, base64_changes) described_class.new.perform(gl_repository, key_id, base64_changes)
end end
end end
...@@ -69,12 +61,12 @@ describe PostReceive do ...@@ -69,12 +61,12 @@ describe PostReceive do
it "does not call any of the services" do it "does not call any of the services" do
expect_any_instance_of(GitPushService).not_to receive(:execute) expect_any_instance_of(GitPushService).not_to receive(:execute)
expect_any_instance_of(GitTagPushService).not_to receive(:execute) expect_any_instance_of(GitTagPushService).not_to receive(:execute)
described_class.new.perform(project_identifier, key_id, base64_changes) described_class.new.perform(gl_repository, key_id, base64_changes)
end end
end end
context "gitlab-ci.yml" do context "gitlab-ci.yml" do
subject { described_class.new.perform(project_identifier, key_id, base64_changes) } subject { described_class.new.perform(gl_repository, key_id, base64_changes) }
context "creates a Ci::Pipeline for every change" do context "creates a Ci::Pipeline for every change" do
before do before do
...@@ -111,7 +103,7 @@ describe PostReceive do ...@@ -111,7 +103,7 @@ describe PostReceive do
it 'calls SystemHooksService' do it 'calls SystemHooksService' do
expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(fake_hook_data, :repository_update_hooks).and_return(true) expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(fake_hook_data, :repository_update_hooks).and_return(true)
described_class.new.perform(project_identifier, key_id, base64_changes) described_class.new.perform(gl_repository, key_id, base64_changes)
end end
end end
end end
...@@ -119,7 +111,7 @@ describe PostReceive do ...@@ -119,7 +111,7 @@ describe PostReceive do
context "webhook" do context "webhook" do
it "fetches the correct project" do it "fetches the correct project" do
expect(Project).to receive(:find_by).with(id: project.id.to_s) expect(Project).to receive(:find_by).with(id: project.id.to_s)
described_class.new.perform(project_identifier, key_id, base64_changes) described_class.new.perform(gl_repository, key_id, base64_changes)
end end
it "does not run if the author is not in the project" do it "does not run if the author is not in the project" do
...@@ -129,7 +121,7 @@ describe PostReceive do ...@@ -129,7 +121,7 @@ describe PostReceive do
expect(project).not_to receive(:execute_hooks) expect(project).not_to receive(:execute_hooks)
expect(described_class.new.perform(project_identifier, key_id, base64_changes)).to be_falsey expect(described_class.new.perform(gl_repository, key_id, base64_changes)).to be_falsey
end end
it "asks the project to trigger all hooks" do it "asks the project to trigger all hooks" do
...@@ -137,18 +129,14 @@ describe PostReceive do ...@@ -137,18 +129,14 @@ describe PostReceive do
expect(project).to receive(:execute_hooks).twice expect(project).to receive(:execute_hooks).twice
expect(project).to receive(:execute_services).twice expect(project).to receive(:execute_services).twice
described_class.new.perform(project_identifier, key_id, base64_changes) described_class.new.perform(gl_repository, key_id, base64_changes)
end end
it "enqueues a UpdateMergeRequestsWorker job" do it "enqueues a UpdateMergeRequestsWorker job" do
allow(Project).to receive(:find_by).and_return(project) allow(Project).to receive(:find_by).and_return(project)
expect(UpdateMergeRequestsWorker).to receive(:perform_async).with(project.id, project.owner.id, any_args) expect(UpdateMergeRequestsWorker).to receive(:perform_async).with(project.id, project.owner.id, any_args)
described_class.new.perform(project_identifier, key_id, base64_changes) described_class.new.perform(gl_repository, key_id, base64_changes)
end end
end end
def pwd(project)
File.join(Gitlab.config.repositories.storages.default['path'], project.path_with_namespace)
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment