From eaf60bb5441190e2ffcf219b3169bda2237d57cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 29 Aug 2017 23:10:41 -0300 Subject: [PATCH] Implement /internal/post_receive unified endpoint for PostReceive tasks --- lib/api/helpers/internal_helpers.rb | 4 + lib/api/internal.rb | 17 ++++- lib/gitlab/reference_counter.rb | 44 +++++++++++ spec/lib/gitlab/reference_counter_spec.rb | 37 ++++++++++ spec/requests/api/internal_spec.rb | 89 +++++++++++++++++++++++ 5 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 lib/gitlab/reference_counter.rb create mode 100644 spec/lib/gitlab/reference_counter_spec.rb diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index ecb79317093..f57ff0f2632 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -42,6 +42,10 @@ module API ::Users::ActivityService.new(actor, 'Git SSH').execute if commands.include?(params[:action]) end + def merge_request_urls + ::MergeRequests::GetUrlsService.new(project).execute(params[:changes]) + end + private def set_project diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 8b007869dc3..622bd9650e4 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -68,7 +68,7 @@ module API end get "/merge_request_urls" do - ::MergeRequests::GetUrlsService.new(project).execute(params[:changes]) + merge_request_urls end # @@ -155,6 +155,21 @@ module API # render_api_error!(e, 500) # end end + + post '/post_receive' do + status 200 + + PostReceive.perform_async(params[:gl_repository], params[:identifier], + params[:changes]) + broadcast_message = BroadcastMessage.current&.last&.message + reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease + + { + merge_request_urls: merge_request_urls, + broadcast_message: broadcast_message, + reference_counter_decreased: reference_counter_decreased + } + end end end end diff --git a/lib/gitlab/reference_counter.rb b/lib/gitlab/reference_counter.rb new file mode 100644 index 00000000000..bb26f1b610a --- /dev/null +++ b/lib/gitlab/reference_counter.rb @@ -0,0 +1,44 @@ +module Gitlab + class ReferenceCounter + REFERENCE_EXPIRE_TIME = 600 + + attr_reader :gl_repository, :key + + def initialize(gl_repository) + @gl_repository = gl_repository + @key = "git-receive-pack-reference-counter:#{gl_repository}" + end + + def value + Gitlab::Redis::SharedState.with { |redis| (redis.get(key) || 0).to_i } + end + + def increase + redis_cmd do |redis| + redis.incr(key) + redis.expire(key, REFERENCE_EXPIRE_TIME) + end + end + + def decrease + redis_cmd do |redis| + current_value = redis.decr(key) + if current_value < 0 + Rails.logger.warn("Reference counter for #{gl_repository} decreased" \ + " when its value was less than 1. Reseting the counter.") + redis.del(key) + end + end + end + + private + + def redis_cmd + Gitlab::Redis::SharedState.with { |redis| yield(redis) } + true + rescue => e + Rails.logger.warn("GitLab: An unexpected error occurred in writing to Redis: #{e}") + false + end + end +end diff --git a/spec/lib/gitlab/reference_counter_spec.rb b/spec/lib/gitlab/reference_counter_spec.rb new file mode 100644 index 00000000000..b2344d1870a --- /dev/null +++ b/spec/lib/gitlab/reference_counter_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Gitlab::ReferenceCounter do + let(:redis) { double('redis') } + let(:reference_counter_key) { "git-receive-pack-reference-counter:project-1" } + let(:reference_counter) { described_class.new('project-1') } + + before do + allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis) + end + + it 'increases and set the expire time of a reference count for a path' do + expect(redis).to receive(:incr).with(reference_counter_key) + expect(redis).to receive(:expire).with(reference_counter_key, + described_class::REFERENCE_EXPIRE_TIME) + expect(reference_counter.increase).to be(true) + end + + it 'decreases the reference count for a path' do + allow(redis).to receive(:decr).and_return(0) + expect(redis).to receive(:decr).with(reference_counter_key) + expect(reference_counter.decrease).to be(true) + end + + it 'warns if attempting to decrease a counter with a value of one or less, and resets the counter' do + expect(redis).to receive(:decr).and_return(-1) + expect(redis).to receive(:del) + expect(Rails.logger).to receive(:warn).with("Reference counter for project-1" \ + " decreased when its value was less than 1. Reseting the counter.") + expect(reference_counter.decrease).to be(true) + end + + it 'get the reference count for a path' do + allow(redis).to receive(:get).and_return(1) + expect(reference_counter.value).to be(1) + end +end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index e9c30dba8d4..a6c804fb2b3 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -660,6 +660,95 @@ describe API::Internal do # end # end + describe 'POST /internal/post_receive' do + let(:gl_repository) { "project-#{project.id}" } + let(:identifier) { 'key-123' } + let(:reference_counter) { double('ReferenceCounter') } + + let(:valid_params) do + { + gl_repository: gl_repository, + secret_token: secret_token, + identifier: identifier, + changes: changes + } + end + + let(:changes) do + "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch" + end + + before do + project.team << [user, :developer] + end + + it 'enqueues a PostReceive worker job' do + expect(PostReceive).to receive(:perform_async) + .with(gl_repository, identifier, changes) + + post api("/internal/post_receive"), valid_params + end + + it 'decreases the reference counter and returns the result' do + expect(Gitlab::ReferenceCounter).to receive(:new).with(gl_repository) + .and_return(reference_counter) + expect(reference_counter).to receive(:decrease).and_return(true) + + post api("/internal/post_receive"), valid_params + + expect(json_response['reference_counter_decreased']).to be(true) + end + + it 'returns link to create new merge request' do + post api("/internal/post_receive"), valid_params + + expect(json_response['merge_request_urls']).to match [{ + "branch_name" => "new_branch", + "url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", + "new_merge_request" => true + }] + end + + it 'returns empty array if printing_merge_request_link_enabled is false' do + project.update!(printing_merge_request_link_enabled: false) + + post api("/internal/post_receive"), valid_params + + expect(json_response['merge_request_urls']).to eq([]) + end + + context 'broadcast message exists' do + let!(:broadcast_message) { create(:broadcast_message, starts_at: 1.day.ago, ends_at: 1.day.from_now ) } + + it 'returns one broadcast message' do + post api("/internal/post_receive"), valid_params + + expect(response).to have_http_status(200) + expect(json_response['broadcast_message']).to eq(broadcast_message.message) + end + end + + context 'broadcast message does not exist' do + it 'returns empty string' do + post api("/internal/post_receive"), valid_params + + expect(response).to have_http_status(200) + expect(json_response['broadcast_message']).to eq(nil) + end + end + + context 'nil broadcast message' do + it 'returns empty string' do + allow(BroadcastMessage).to receive(:current).and_return(nil) + + post api("/internal/post_receive"), valid_params + + expect(response).to have_http_status(200) + expect(json_response['broadcast_message']).to eq(nil) + end + end + end + def project_with_repo_path(path) double().tap do |fake_project| allow(fake_project).to receive_message_chain('repository.path_to_repo' => path)