Commit 30b36c92 authored by Jacob Vosmaer's avatar Jacob Vosmaer

Use an exception to pass messages

parent 0beae70e
...@@ -170,12 +170,17 @@ class ProjectsController < ApplicationController ...@@ -170,12 +170,17 @@ class ProjectsController < ApplicationController
end end
def housekeeping def housekeeping
message = ::Projects::HousekeepingService.new(@project).execute ::Projects::HousekeepingService.new(@project).execute
respond_to do |format| redirect_to(
flash[:notice] = message project_path(@project),
format.html { redirect_to project_path(@project) } notice: "Housekeeping successfully started"
end )
rescue ::Projects::HousekeepingService::LeaseTaken => ex
redirect_to(
edit_project_path(@project),
alert: ex.to_s
)
end end
def toggle_star def toggle_star
......
...@@ -79,6 +79,7 @@ class GitPushService < BaseService ...@@ -79,6 +79,7 @@ class GitPushService < BaseService
housekeeping = Projects::HousekeepingService.new(@project) housekeeping = Projects::HousekeepingService.new(@project)
housekeeping.increment! housekeeping.increment!
housekeeping.execute if housekeeping.needed? housekeeping.execute if housekeeping.needed?
rescue Projects::HousekeepingService::LeaseTaken
end end
def process_default_branch def process_default_branch
......
...@@ -11,20 +11,22 @@ module Projects ...@@ -11,20 +11,22 @@ module Projects
LEASE_TIMEOUT = 3600 LEASE_TIMEOUT = 3600
class LeaseTaken < StandardError
def to_s
"Somebody already triggered housekeeping for this project in the past #{LEASE_TIMEOUT / 60} minutes"
end
end
def initialize(project) def initialize(project)
@project = project @project = project
end end
def execute def execute
if !try_obtain_lease raise LeaseTaken if !try_obtain_lease
return "Housekeeping was already triggered in the past #{LEASE_TIMEOUT / 60} minutes"
end
GitlabShellWorker.perform_async(:gc, @project.path_with_namespace) GitlabShellWorker.perform_async(:gc, @project.path_with_namespace)
@project.pushes_since_gc = 0 ensure
@project.save! @project.update_column(:pushes_since_gc, 0)
"Housekeeping successfully started"
end end
def needed? def needed?
...@@ -32,8 +34,7 @@ module Projects ...@@ -32,8 +34,7 @@ module Projects
end end
def increment! def increment!
@project.pushes_since_gc += 1 @project.increment!(:pushes_since_gc)
@project.save!
end end
private private
......
...@@ -402,7 +402,7 @@ describe GitPushService, services: true do ...@@ -402,7 +402,7 @@ describe GitPushService, services: true do
end end
describe "housekeeping" do describe "housekeeping" do
let(:housekeeping) { instance_double('Projects::HousekeepingService', increment!: nil, needed?: false) } let(:housekeeping) { Projects::HousekeepingService.new(project) }
before do before do
allow(Projects::HousekeepingService).to receive(:new).and_return(housekeeping) allow(Projects::HousekeepingService).to receive(:new).and_return(housekeeping)
...@@ -414,13 +414,25 @@ describe GitPushService, services: true do ...@@ -414,13 +414,25 @@ describe GitPushService, services: true do
execute_service(project, user, @oldrev, @newrev, @ref) execute_service(project, user, @oldrev, @newrev, @ref)
end end
it 'performs housekeeping when needed' do context 'when housekeeping is needed' do
expect(housekeeping).to receive(:needed?).and_return(true) before do
allow(housekeeping).to receive(:needed?).and_return(true)
end
it 'performs housekeeping' do
expect(housekeeping).to receive(:execute) expect(housekeeping).to receive(:execute)
execute_service(project, user, @oldrev, @newrev, @ref) execute_service(project, user, @oldrev, @newrev, @ref)
end end
it 'does not raise an exception' do
allow(housekeeping).to receive(:try_obtain_lease).and_return(false)
execute_service(project, user, @oldrev, @newrev, @ref)
end
end
it 'increments the push counter' do it 'increments the push counter' do
expect(housekeeping).to receive(:increment!) expect(housekeeping).to receive(:increment!)
......
...@@ -14,7 +14,7 @@ describe Projects::HousekeepingService do ...@@ -14,7 +14,7 @@ describe Projects::HousekeepingService do
expect(subject).to receive(:try_obtain_lease).and_return(true) expect(subject).to receive(:try_obtain_lease).and_return(true)
expect(GitlabShellWorker).to receive(:perform_async).with(:gc, project.path_with_namespace) expect(GitlabShellWorker).to receive(:perform_async).with(:gc, project.path_with_namespace)
expect(subject.execute).to include('successfully started') subject.execute
expect(project.pushes_since_gc).to eq(0) expect(project.pushes_since_gc).to eq(0)
end end
...@@ -22,8 +22,8 @@ describe Projects::HousekeepingService do ...@@ -22,8 +22,8 @@ describe Projects::HousekeepingService do
expect(subject).to receive(:try_obtain_lease).and_return(false) expect(subject).to receive(:try_obtain_lease).and_return(false)
expect(GitlabShellWorker).not_to receive(:perform_async) expect(GitlabShellWorker).not_to receive(:perform_async)
expect(subject.execute).to include('already triggered') expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken)
expect(project.pushes_since_gc).to eq(3) expect(project.pushes_since_gc).to eq(0)
end 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