Commit 7bab4817 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'repo-remove' into fix-group-remove

Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

Conflicts:
	spec/features/projects_spec.rb
parents 1edff534 58ab8a4a
...@@ -41,6 +41,7 @@ v 7.12.0 (unreleased) ...@@ -41,6 +41,7 @@ v 7.12.0 (unreleased)
- Add an option to automatically sign-in with an Omniauth provider - Add an option to automatically sign-in with an Omniauth provider
- Better performance for web editor (switched from satellites to rugged) - Better performance for web editor (switched from satellites to rugged)
- GitLab CI service sends .gitlab-ci.yaml in each push call - GitLab CI service sends .gitlab-ci.yaml in each push call
- When remove project - move repository and schedule it removal
v 7.11.4 v 7.11.4
- Fix missing bullets when creating lists - Fix missing bullets when creating lists
......
...@@ -97,18 +97,15 @@ class ProjectsController < ApplicationController ...@@ -97,18 +97,15 @@ class ProjectsController < ApplicationController
return access_denied! unless can?(current_user, :remove_project, @project) return access_denied! unless can?(current_user, :remove_project, @project)
::Projects::DestroyService.new(@project, current_user, {}).execute ::Projects::DestroyService.new(@project, current_user, {}).execute
flash[:alert] = 'Project deleted.'
respond_to do |format| if request.referer.include?('/admin')
format.html do redirect_to admin_namespaces_projects_path
flash[:alert] = 'Project deleted.' else
redirect_to dashboard_path
if request.referer.include?('/admin')
redirect_to admin_namespaces_projects_path
else
redirect_to dashboard_path
end
end
end end
rescue Projects::DestroyService::DestroyError => ex
redirect_to edit_project_path(@project), alert: ex.message
end end
def autocomplete_sources def autocomplete_sources
......
module Projects module Projects
class DestroyService < BaseService class DestroyService < BaseService
include Gitlab::ShellAdapter
class DestroyError < StandardError; end
DELETED_FLAG = '+deleted'
def execute def execute
return false unless can?(current_user, :remove_project, project) return false unless can?(current_user, :remove_project, project)
project.team.truncate project.team.truncate
project.repository.expire_cache unless project.empty_repo? project.repository.expire_cache unless project.empty_repo?
if project.destroy repo_path = project.path_with_namespace
GitlabShellWorker.perform_async( wiki_path = repo_path + '.wiki'
:remove_repository,
project.path_with_namespace Project.transaction do
) project.destroy!
GitlabShellWorker.perform_async( unless remove_repository(repo_path)
:remove_repository, raise_error('Failed to remove project repository. Please try again or contact administrator')
project.path_with_namespace + ".wiki" end
)
unless remove_repository(wiki_path)
raise_error('Failed to remove wiki repository. Please try again or contact administrator')
end
end
project.satellite.destroy
log_info("Project \"#{project.name}\" was removed")
system_hook_service.execute_hooks_for(project, :destroy)
true
end
project.satellite.destroy private
log_info("Project \"#{project.name}\" was removed") def remove_repository(path)
system_hook_service.execute_hooks_for(project, :destroy) unless gitlab_shell.exists?(path + '.git')
true return true
end end
new_path = removal_path(path)
if gitlab_shell.mv_repository(path, new_path)
log_info("Repository \"#{path}\" moved to \"#{new_path}\"")
GitlabShellWorker.perform_in(5.minutes, :remove_repository, new_path)
else
false
end
end
def raise_error(message)
raise DestroyError.new(message)
end
# Build a path for removing repositories
# We use `+` because its not allowed by GitLab so user can not create
# project with name cookies+119+deleted and capture someone stalled repository
#
# gitlab/cookies.git -> gitlab/cookies+119+deleted.git
#
def removal_path(path)
"#{path}+#{project.id}#{DELETED_FLAG}"
end end
end end
end end
...@@ -244,6 +244,16 @@ module Gitlab ...@@ -244,6 +244,16 @@ module Gitlab
end end
end end
# Check if such directory exists in repositories.
#
# Usage:
# exists?('gitlab')
# exists?('gitlab/cookies.git')
#
def exists?(dir_name)
File.exists?(full_path(dir_name))
end
protected protected
def gitlab_shell_path def gitlab_shell_path
...@@ -264,10 +274,6 @@ module Gitlab ...@@ -264,10 +274,6 @@ module Gitlab
File.join(repos_path, dir_name) File.join(repos_path, dir_name)
end end
def exists?(dir_name)
File.exists?(full_path(dir_name))
end
def gitlab_shell_projects_path def gitlab_shell_projects_path
File.join(gitlab_shell_path, 'bin', 'gitlab-projects') File.join(gitlab_shell_path, 'bin', 'gitlab-projects')
end end
......
...@@ -47,13 +47,6 @@ feature 'Project' do ...@@ -47,13 +47,6 @@ feature 'Project' do
it 'should remove project' do it 'should remove project' do
expect { remove_project }.to change {Project.count}.by(-1) expect { remove_project }.to change {Project.count}.by(-1)
end end
it 'should delete the project from disk' do
expect(GitlabShellWorker).to receive(:perform_async).
with(:remove_repository, /#{project.path_with_namespace}/).twice
remove_project
end
end end
def remove_project def remove_project
......
...@@ -57,14 +57,14 @@ describe API::API, api: true do ...@@ -57,14 +57,14 @@ describe API::API, api: true do
expect(json_response.first['name']).to eq(project.name) expect(json_response.first['name']).to eq(project.name)
expect(json_response.first['owner']['username']).to eq(user.username) expect(json_response.first['owner']['username']).to eq(user.username)
end end
it 'should include the project labels as the tag_list' do it 'should include the project labels as the tag_list' do
get api('/projects', user) get api('/projects', user)
response.status.should == 200 response.status.should == 200
json_response.should be_an Array json_response.should be_an Array
json_response.first.keys.should include('tag_list') json_response.first.keys.should include('tag_list')
end end
context 'and using search' do context 'and using search' do
it 'should return searched project' do it 'should return searched project' do
get api('/projects', user), { search: project.name } get api('/projects', user), { search: project.name }
...@@ -792,11 +792,6 @@ describe API::API, api: true do ...@@ -792,11 +792,6 @@ describe API::API, api: true do
describe 'DELETE /projects/:id' do describe 'DELETE /projects/:id' do
context 'when authenticated as user' do context 'when authenticated as user' do
it 'should remove project' do it 'should remove project' do
expect(GitlabShellWorker).to(
receive(:perform_async).with(:remove_repository,
/#{project.path_with_namespace}/)
).twice
delete api("/projects/#{project.id}", user) delete api("/projects/#{project.id}", user)
expect(response.status).to eq(200) expect(response.status).to eq(200)
end end
......
require 'spec_helper'
describe Projects::DestroyService do
let!(:user) { create(:user) }
let!(:project) { create(:project, namespace: user.namespace) }
let!(:path) { project.repository.path_to_repo }
let!(:remove_path) { path.sub(/\.git\Z/, "+#{project.id}+deleted.git") }
context 'Sidekiq inline' do
before do
# Run sidekiq immediatly to check that renamed repository will be removed
Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
end
it { Project.all.should_not include(project) }
it { Dir.exists?(path).should be_falsey }
it { Dir.exists?(remove_path).should be_falsey }
end
context 'Sidekiq fake' do
before do
# Dont run sidekiq to check if renamed repository exists
Sidekiq::Testing.fake! { destroy_project(project, user, {}) }
end
it { Project.all.should_not include(project) }
it { Dir.exists?(path).should be_falsey }
it { Dir.exists?(remove_path).should be_truthy }
end
def destroy_project(project, user, params)
Projects::DestroyService.new(project, user, params).execute
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