Skip to content

Commit

Permalink
Adding a processor for handling deletion of merge requests.
Browse files Browse the repository at this point in the history
Replacing MergeRequests#destroy with a call to #soft_delete, which posts a message to the MQ.
This processor will both delete the ref in the target repository and delete the merge request itself.

The ref in the tracking repository is not deleted yet, nor are updates in the backend handled for merge requests
that are closed or reopened.
  • Loading branch information
Marius Mathiesen committed Jul 17, 2009
1 parent 13a5339 commit 75f054d
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 10 deletions.
2 changes: 1 addition & 1 deletion app/controllers/merge_requests_controller.rb
Expand Up @@ -207,7 +207,7 @@ def update
end end


def destroy def destroy
@merge_request.destroy @merge_request.soft_delete
flash[:success] = I18n.t "merge_requests_controller.destroy_success" flash[:success] = I18n.t "merge_requests_controller.destroy_success"
redirect_to [@owner, @repository] redirect_to [@owner, @repository]
end end
Expand Down
6 changes: 6 additions & 0 deletions app/models/merge_request.rb
Expand Up @@ -500,6 +500,12 @@ def push_new_branch_to_tracking_repo
def delete_target_repository_ref def delete_target_repository_ref
source_repository.git.git.push({},target_repository.full_repository_path, ":#{merge_branch_name}") source_repository.git.git.push({},target_repository.full_repository_path, ":#{merge_branch_name}")
end end

# Since we'll be deleting the ref in the backend, this will be handled in the message queue
def soft_delete
msg = {:merge_request_id => to_param, :action => "delete"}
publish :merge_request_backend_updates, msg.to_json
end


def tracking_repository def tracking_repository
target_repository.create_tracking_repository unless target_repository.has_tracking_repository? target_repository.create_tracking_repository unless target_repository.has_tracking_repository?
Expand Down
45 changes: 45 additions & 0 deletions app/processors/merge_request_git_backend_processor.rb
@@ -0,0 +1,45 @@
# encoding: utf-8
#--
# Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies)
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Affero General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#++
class MergeRequestGitBackendProcessor < ApplicationProcessor

subscribes_to :merge_request_backend_updates

def on_message(message)
@body = ActiveSupport::JSON.decode(message)
send("do_#{action}")
end

def action
@body["action"].to_sym
end

def merge_request
@merge_request ||= MergeRequest.find(@body["merge_request_id"])
end

private
def do_delete
logger.info("Deleting tracking branch #{merge_request.merge_branch_name} for merge request in target repository #{merge_request.target_repository.id}")
begin
merge_request.delete_target_repository_ref
rescue Grit::NoSuchPathError => e
logger.error "Could not find Git path. Message is #{e.message}"
end
merge_request.destroy
end
end
4 changes: 2 additions & 2 deletions config/messaging.rb
Expand Up @@ -14,5 +14,5 @@
s.destination :archive_repo, '/queue/GitoriousRepositoryArchiving' s.destination :archive_repo, '/queue/GitoriousRepositoryArchiving'
s.destination :cc_message, '/queue/GitoriousEmailNotifications' s.destination :cc_message, '/queue/GitoriousEmailNotifications'
s.destination :mirror_merge_request, '/queue/GitoriousMergeRequestCreation' s.destination :mirror_merge_request, '/queue/GitoriousMergeRequestCreation'

s.destination :merge_request_backend_updates, '/queue/GitoriousMergeRequestBackend'
end end
5 changes: 3 additions & 2 deletions test/functional/merge_requests_controller_test.rb
Expand Up @@ -641,12 +641,13 @@ def do_delete
assert_equal @source_repository, assigns(:merge_request).source_repository assert_equal @source_repository, assigns(:merge_request).source_repository
end end


should "deletes the record" do should "soft-delete the record" do
login_as :johan login_as :johan
MergeRequest.stubs(:find).returns(@merge_request)
@merge_request.expects(:soft_delete)
do_delete do_delete
assert_redirected_to(project_repository_path(@project, @target_repository)) assert_redirected_to(project_repository_path(@project, @target_repository))
assert_match(/merge request was retracted/i, flash[:success]) assert_match(/merge request was retracted/i, flash[:success])
assert_nil MergeRequest.find_by_id(@merge_request.id)
end end


should "only allows the owner to delete" do should "only allows the owner to delete" do
Expand Down
15 changes: 10 additions & 5 deletions test/unit/merge_request_test.rb
Expand Up @@ -658,11 +658,16 @@ def setup
@source_git.expects(:push).with({}, @merge_request.target_repository.full_repository_path, ":#{@merge_request.merge_branch_name}") @source_git.expects(:push).with({}, @merge_request.target_repository.full_repository_path, ":#{@merge_request.merge_branch_name}")
@merge_request.delete_target_repository_ref @merge_request.delete_target_repository_ref
end end
end


should_eventually "delete the target repository when being closed" context "Soft deletion of merge requests" do

setup do
should_eventually "recreate the ref in the target repository when reopened" @merge_request = merge_requests(:moes_to_johans_open)

end
should_eventually "delete both from the target and tracking repository when being deleted" should "send a message when being soft deleted" do
p = proc {@merge_request.soft_delete}
message = find_message_with_queue_and_regexp('/queue/GitoriousMergeRequestBackend', /.*/) {p.call}
assert_equal({'merge_request_id' => @merge_request.id.to_s, "action" => "delete"}, message)
end
end end
end end
75 changes: 75 additions & 0 deletions test/unit/processors/merge_request_git_backend_processor_test.rb
@@ -0,0 +1,75 @@
# encoding: utf-8
#--
# Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies)
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Affero General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#++


require File.dirname(__FILE__) + '/../../test_helper'

class MergeRequestGitBackendProcessorTest < ActiveSupport::TestCase

def setup
@processor = MergeRequestGitBackendProcessor.new
@merge_request = merge_requests(:moes_to_johans)
@repository = @merge_request.target_repository
end

def teardown
@processor = nil
end

context "Deleting the merge request and its tracking branch" do
setup do
@msg = {:merge_request_id => @merge_request.to_param, :action => "delete"}
@processor.instance_variable_set("@merge_request", @merge_request)
end

should "delete the tracking repo and the merge request itself" do
@merge_request.expects(:delete_target_repository_ref).once
@merge_request.expects(:destroy).once
@processor.on_message(@msg.to_json)
end

should "handle non-existing target gits" do
@merge_request.expects(:destroy).once
assert_nothing_raised do
@processor.on_message(@msg.to_json)
end
end
end

context "Parsing the action" do
should "understand the delete command" do
msg = {:merge_request_id => @merge_request.to_param, :action => "delete"}
@processor.expects(:do_delete).once
@processor.on_message(msg.to_json)
assert_equal :delete, @processor.action
assert_equal @merge_request, @processor.merge_request
end

should "understand the close command" do
msg = {:merge_request_id => @merge_request.to_param, :action => "close"}
@processor.expects(:do_close)
@processor.on_message(msg.to_json)
end

should "understand the reopen command" do
msg = {:merge_request_id => @merge_request.to_param, :action => "reopen"}
@processor.expects(:do_reopen)
@processor.on_message(msg.to_json)
end
end
end

0 comments on commit 75f054d

Please sign in to comment.