From 91b9cbff8de538c2966bca94c517ee6aa346f79b Mon Sep 17 00:00:00 2001 From: Josh Frye Date: Fri, 22 Jan 2016 11:13:37 -0800 Subject: [PATCH 1/2] First pass at deleting projects in the background. --- CHANGELOG | 3 +++ app/controllers/projects_controller.rb | 8 ++++++-- app/models/project.rb | 1 + app/services/delete_user_service.rb | 2 +- app/services/destroy_group_service.rb | 2 +- app/services/projects/destroy_service.rb | 6 ++++++ app/workers/project_destroy_worker.rb | 17 +++++++++++++++++ ...60122185421_add_pending_delete_to_project.rb | 5 +++++ db/schema.rb | 7 ++++--- lib/api/projects.rb | 4 ++-- spec/models/hooks/system_hook_spec.rb | 6 +++--- 11 files changed, 49 insertions(+), 12 deletions(-) create mode 100644 app/workers/project_destroy_worker.rb create mode 100644 db/migrate/20160122185421_add_pending_delete_to_project.rb diff --git a/CHANGELOG b/CHANGELOG index 9dec6f9809e..8a6a4042cb0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,8 @@ Please view this file on the master branch, on stable branches it's out of date. +v 8.6.0 (unreleased) + - Delete project and associations in a background worker + v 8.5.0 (unreleased) - Ensure rake tasks that don't need a DB connection can be run without one - Add "visibility" flag to GET /projects api endpoint diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 935f7d75c6a..4df5095bd94 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -93,6 +93,10 @@ class ProjectsController < ApplicationController return end + if @project.pending_delete? + flash[:alert] = "Project queued for delete." + end + respond_to do |format| format.html do if @project.repository_exists? @@ -120,8 +124,8 @@ class ProjectsController < ApplicationController def destroy return access_denied! unless can?(current_user, :remove_project, @project) - ::Projects::DestroyService.new(@project, current_user, {}).execute - flash[:alert] = "Project '#{@project.name}' was deleted." + ::Projects::DestroyService.new(@project, current_user, {}).pending_delete! + flash[:alert] = "Project '#{@project.name}' will be deleted." redirect_to dashboard_projects_path rescue Projects::DestroyService::DestroyError => ex diff --git a/app/models/project.rb b/app/models/project.rb index 238932f59a7..043f08b9a13 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -36,6 +36,7 @@ # build_coverage_regex :string # build_allow_git_fetch :boolean default(TRUE), not null # build_timeout :integer default(3600), not null +# pending_delete :boolean # require 'carrierwave/orm/activerecord' diff --git a/app/services/delete_user_service.rb b/app/services/delete_user_service.rb index e622fd5ea5d..173e50c9206 100644 --- a/app/services/delete_user_service.rb +++ b/app/services/delete_user_service.rb @@ -13,7 +13,7 @@ class DeleteUserService user.personal_projects.each do |project| # Skip repository removal because we remove directory with namespace # that contain all this repositories - ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute + ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete! end user.destroy diff --git a/app/services/destroy_group_service.rb b/app/services/destroy_group_service.rb index d929a676293..9189de390a2 100644 --- a/app/services/destroy_group_service.rb +++ b/app/services/destroy_group_service.rb @@ -9,7 +9,7 @@ class DestroyGroupService @group.projects.each do |project| # Skip repository removal because we remove directory with namespace # that contain all this repositories - ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute + ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete! end @group.destroy diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 28872c89259..294157b4f0e 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -6,6 +6,12 @@ module Projects DELETED_FLAG = '+deleted' + def pending_delete! + project.update_attribute(:pending_delete, true) + + ProjectDestroyWorker.perform_in(1.minute, project.id, current_user.id, params) + end + def execute return false unless can?(current_user, :remove_project, project) diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb new file mode 100644 index 00000000000..d06e4480292 --- /dev/null +++ b/app/workers/project_destroy_worker.rb @@ -0,0 +1,17 @@ +class ProjectDestroyWorker + include Sidekiq::Worker + + sidekiq_options queue: :default + + def perform(project_id, user_id, params) + begin + project = Project.find(project_id) + rescue ActiveRecord::RecordNotFound + return + end + + user = User.find(user_id) + + ::Projects::DestroyService.new(project, user, params).execute + end +end diff --git a/db/migrate/20160122185421_add_pending_delete_to_project.rb b/db/migrate/20160122185421_add_pending_delete_to_project.rb new file mode 100644 index 00000000000..457922942b0 --- /dev/null +++ b/db/migrate/20160122185421_add_pending_delete_to_project.rb @@ -0,0 +1,5 @@ +class AddPendingDeleteToProject < ActiveRecord::Migration + def change + add_column :projects, :pending_delete, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 2a2911bfbc7..8c9cbe73c87 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -677,6 +677,7 @@ ActiveRecord::Schema.define(version: 20160128212447) do t.string "build_coverage_regex" t.boolean "build_allow_git_fetch", default: true, null: false t.integer "build_timeout", default: 3600, null: false + t.boolean "pending_delete" end add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree @@ -728,9 +729,9 @@ ActiveRecord::Schema.define(version: 20160128212447) do t.string "type" t.string "title" t.integer "project_id" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.boolean "active", null: false + t.datetime "created_at" + t.datetime "updated_at" + t.boolean "active", default: false, null: false t.text "properties" t.boolean "template", default: false t.boolean "push_events", default: true diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 71bb342f844..1f991e600e3 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -187,7 +187,7 @@ module API else present @forked_project, with: Entities::Project, user_can_admin_project: can?(current_user, :admin_project, @forked_project) - end + end end # Update an existing project @@ -246,7 +246,7 @@ module API # DELETE /projects/:id delete ":id" do authorize! :remove_project, user_project - ::Projects::DestroyService.new(user_project, current_user, {}).execute + ::Projects::DestroyService.new(user_project, current_user, {}).pending_delete! end # Mark this project as forked from another diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 138b87a9a06..fd1513cab1b 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -36,7 +36,7 @@ describe SystemHook, models: true do it "project_destroy hook" do user = create(:user) project = create(:empty_project, namespace: user.namespace) - Projects::DestroyService.new(project, user, {}).execute + Projects::DestroyService.new(project, user, {}).pending_delete! expect(WebMock).to have_requested(:post, @system_hook.url).with( body: /project_destroy/, headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } @@ -65,7 +65,7 @@ describe SystemHook, models: true do project = create(:project) project.team << [user, :master] expect(WebMock).to have_requested(:post, @system_hook.url).with( - body: /user_add_to_team/, + body: /user_add_to_team/, headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } ).once end @@ -76,7 +76,7 @@ describe SystemHook, models: true do project.team << [user, :master] project.project_members.destroy_all expect(WebMock).to have_requested(:post, @system_hook.url).with( - body: /user_remove_from_team/, + body: /user_remove_from_team/, headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } ).once end From f255313ea47b433b428b3b90452aede8c9a6618c Mon Sep 17 00:00:00 2001 From: Josh Frye Date: Thu, 28 Jan 2016 08:48:46 -0500 Subject: [PATCH 2/2] Update CHANGELOG --- CHANGELOG | 4 +--- .../20160122185421_add_pending_delete_to_project.rb | 2 +- db/schema.rb | 8 ++++---- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 8a6a4042cb0..66ca75e70da 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,8 +1,5 @@ Please view this file on the master branch, on stable branches it's out of date. -v 8.6.0 (unreleased) - - Delete project and associations in a background worker - v 8.5.0 (unreleased) - Ensure rake tasks that don't need a DB connection can be run without one - Add "visibility" flag to GET /projects api endpoint @@ -41,6 +38,7 @@ v 8.4.1 and Nokogiri (1.6.7.2) - Fix redirect loop during import - Fix diff highlighting for all syntax themes + - Delete project and associations in a background worker v 8.4.0 - Allow LDAP users to change their email if it was not set by the LDAP server diff --git a/db/migrate/20160122185421_add_pending_delete_to_project.rb b/db/migrate/20160122185421_add_pending_delete_to_project.rb index 457922942b0..046a5d8fc32 100644 --- a/db/migrate/20160122185421_add_pending_delete_to_project.rb +++ b/db/migrate/20160122185421_add_pending_delete_to_project.rb @@ -1,5 +1,5 @@ class AddPendingDeleteToProject < ActiveRecord::Migration def change - add_column :projects, :pending_delete, :boolean + add_column :projects, :pending_delete, :boolean, default: false end end diff --git a/db/schema.rb b/db/schema.rb index 8c9cbe73c87..73589cb1f17 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -677,7 +677,7 @@ ActiveRecord::Schema.define(version: 20160128212447) do t.string "build_coverage_regex" t.boolean "build_allow_git_fetch", default: true, null: false t.integer "build_timeout", default: 3600, null: false - t.boolean "pending_delete" + t.boolean "pending_delete", default: false end add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree @@ -729,9 +729,9 @@ ActiveRecord::Schema.define(version: 20160128212447) do t.string "type" t.string "title" t.integer "project_id" - t.datetime "created_at" - t.datetime "updated_at" - t.boolean "active", default: false, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.boolean "active", null: false t.text "properties" t.boolean "template", default: false t.boolean "push_events", default: true