Merge branch 'master' into geo-base-scheduler-worker

Gemfile

@@ -16,6 +16,7 @@ gem 'mysql2', '~> 0.4.5', group: :mysql
 gem 'pg', '~> 0.18.2', group: :postgres
 
 gem 'rugged', '~> 0.25.1.1'
+gem 'grape-route-helpers', '~> 2.0.0'
 
 gem 'faraday', '~> 0.12'
 
@@ -63,7 +64,7 @@ gem 'browser', '~> 2.2'
 # LDAP Auth
 # GitLab fork with several improvements to original library. For full list of changes
 # see https://github.com/intridea/omniauth-ldap/compare/master...gitlabhq:master
-gem 'gitlab_omniauth-ldap', '~> 1.2.1', require: 'omniauth-ldap'
+gem 'gitlab_omniauth-ldap', '~> 2.0.3', require: 'omniauth-ldap'
 gem 'net-ldap'
 
 # Git Wiki

Gemfile.lock

@@ -313,11 +313,11 @@ GEM
       posix-spawn (~> 0.3)
     gitlab-license (1.0.0)
     gitlab-markup (1.5.1)
-    gitlab_omniauth-ldap (1.2.1)
-      net-ldap (~> 0.9)
-      omniauth (~> 1.0)
-      pyu-ruby-sasl (~> 0.0.3.1)
-      rubyntlm (~> 0.3)
+    gitlab_omniauth-ldap (2.0.3)
+      net-ldap (~> 0.16)
+      omniauth (~> 1.3)
+      pyu-ruby-sasl (>= 0.0.3.3, < 0.1)
+      rubyntlm (~> 0.5)
     globalid (0.3.7)
       activesupport (>= 4.1.0)
     gollum-grit_adapter (1.0.1)
@@ -370,6 +370,10 @@ GEM
     grape-entity (0.6.0)
       activesupport
       multi_json (>= 1.3.2)
+    grape-route-helpers (2.0.0)
+      activesupport
+      grape (~> 0.16, >= 0.16.0)
+      rake
     grpc (1.4.0)
       google-protobuf (~> 3.1)
       googleauth (~> 0.5.1)
@@ -495,7 +499,7 @@ GEM
     mustermann-grape (1.0.0)
       mustermann (~> 1.0.0)
     mysql2 (0.4.5)
-    net-ldap (0.12.1)
+    net-ldap (0.16.0)
     net-ntp (2.1.3)
     netrc (0.11.0)
     nokogiri (1.6.8.1)
@@ -769,7 +773,7 @@ GEM
       nokogiri (>= 1.5.10)
     ruby_parser (3.9.0)
       sexp_processor (~> 4.1)
-    rubyntlm (0.5.2)
+    rubyntlm (0.6.2)
     rubypants (0.2.0)
     rubyzip (1.2.1)
     rufus-scheduler (3.4.0)
@@ -1009,7 +1013,7 @@ DEPENDENCIES
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-license (~> 1.0)
   gitlab-markup (~> 1.5.1)
-  gitlab_omniauth-ldap (~> 1.2.1)
+  gitlab_omniauth-ldap (~> 2.0.3)
   gollum-lib (~> 4.2)
   gollum-rugged_adapter (~> 0.4.4)
   gon (~> 6.1.0)
@@ -1017,6 +1021,7 @@ DEPENDENCIES
   grape (~> 0.19.2)
   grape-entity (~> 0.6.0)
   gssapi
+  grape-route-helpers (~> 2.0.0)
   haml_lint (~> 0.21.0)
   hamlit (~> 2.6.1)
   hashie-forbidden_attributes

app/controllers/admin/dashboard_controller.rb

 class Admin::DashboardController < Admin::ApplicationController
   def index
-    @projects = Project.with_route.limit(10)
+    @projects = Project.without_deleted.with_route.limit(10)
     @users = User.limit(10)
     @groups = Group.with_route.limit(10)
     @license = License.current

app/services/ee/projects/destroy_service.rb

@@ -4,10 +4,14 @@ module EE
       def execute
         raise NotImplementedError unless defined?(super)
 
-        super
+        succeeded = super
 
-        mirror_cleanup(project)
-        log_geo_event(project)
+        if succeeded
+          mirror_cleanup(project)
+          log_geo_event(project)
+        end
+
+        succeeded
       end
 
       def mirror_cleanup(project)
@@ -29,7 +33,7 @@ module EE
         # Flush the cache for both repositories. This has to be done _before_
         # removing the physical repositories as some expiration code depends on
         # Git data (e.g. a list of branch names).
-        flush_caches(project, wiki_path)
+        flush_caches(project)
 
         trash_repositories!
         remove_tracking_entries!

app/services/projects/destroy_service.rb

@@ -16,38 +16,26 @@ module Projects
     def execute
       return false unless can?(current_user, :remove_project, project)
 
-      repo_path = project.path_with_namespace
-      wiki_path = repo_path + '.wiki'
-
       # Flush the cache for both repositories. This has to be done _before_
       # removing the physical repositories as some expiration code depends on
       # Git data (e.g. a list of branch names).
-      flush_caches(project, wiki_path)
+      flush_caches(project)
 
       Projects::UnlinkForkService.new(project, current_user).execute
 
-      Project.transaction do
-        project.team.truncate
-        project.destroy!
-        trash_repositories!
+      attempt_destroy_transaction(project)
 
-        unless remove_legacy_registry_tags
-          raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.')
-        end
-
-        unless remove_repository(repo_path)
-          raise_error('Failed to remove project repository. Please try again or contact administrator.')
-        end
-
-        unless remove_repository(wiki_path)
-          raise_error('Failed to remove wiki repository. Please try again or contact administrator.')
-        end
-      end
-
-      log_info("Project \"#{project.path_with_namespace}\" was removed")
       system_hook_service.execute_hooks_for(project, :destroy)
+      log_info("Project \"#{project.full_path}\" was removed")
 
       true
+    rescue => error
+      attempt_rollback(project, error.message)
+      false
+    rescue Exception => error # rubocop:disable Lint/RescueException
+      # Project.transaction can raise Exception
+      attempt_rollback(project, error.message)
+      raise
     end
 
     private
@@ -91,6 +79,26 @@ module Projects
       end
     end
 
+    def attempt_rollback(project, message)
+      return unless project
+
+      project.update_attributes(delete_error: message, pending_delete: false)
+      log_error("Deletion failed on #{project.full_path} with the following message: #{message}")
+    end
+
+    def attempt_destroy_transaction(project)
+      Project.transaction do
+        unless remove_legacy_registry_tags
+          raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.')
+        end
+
+        trash_repositories!
+
+        project.team.truncate
+        project.destroy!
+      end
+    end
+
     ##
     # This method makes sure that we correctly remove registry tags
     # for legacy image repository (when repository path equals project path).
@@ -117,7 +125,7 @@ module Projects
       "#{path}+#{project.id}#{DELETED_FLAG}"
     end
 
-    def flush_caches(project, wiki_path)
+    def flush_caches(project)
       project.repository.before_delete
 
       Repository.new(wiki_path, project).before_delete

app/views/layouts/nav/_new_project_sidebar.html.haml

@@ -90,9 +90,9 @@
                   List
 
             = nav_link(controller: :boards) do
-              = link_to project_boards_path(@project), title: 'Board' do
+              = link_to project_boards_path(@project), title: 'Boards' do
                 %span
-                  Board
+                  Boards
 
           - if project_nav_tab?(:merge_requests) && current_controller?(:merge_requests)
             = nav_link(controller: :merge_requests) do

app/views/projects/_deletion_failed.html.haml

+- project = local_assigns.fetch(:project)
+- return unless project.delete_error.present?
+
+.project-deletion-failed-message.alert.alert-warning
+  This project was scheduled for deletion, but failed with the following message:
+  = project.delete_error

app/views/projects/_flash_messages.html.haml

+- project = local_assigns.fetch(:project)
+- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
+
+= content_for flash_message_container do
+  = render partial: 'deletion_failed', locals: { project: project }
+  - if current_user && can?(current_user, :download_code, project)
+    = render 'shared/no_ssh'
+    = render 'shared/no_password'
+    = render 'shared/shared_runners_minutes_limit', project: project
+    - if project.above_size_limit?
+      = render 'above_size_limit_warning'

app/views/projects/empty.html.haml

 - @no_container = true
-- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
 
-= content_for flash_message_container do
-  - if current_user && can?(current_user, :download_code, @project)
-    = render 'shared/no_ssh'
-    = render 'shared/no_password'
-    = render 'shared/shared_runners_minutes_limit', project: @project
+= render partial: 'flash_messages', locals: { project: @project }
 
 = render "projects/head"
 = render "home_panel"

app/views/projects/show.html.haml

 - @no_container = true
 - breadcrumb_title "Project"
 - @content_class = "limit-container-width" unless fluid_layout
-- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
 
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, project_path(@project, rss_url_options), title: "#{@project.name} activity")
 
-= content_for flash_message_container do
-  - if current_user && can?(current_user, :download_code, @project)
-    = render 'shared/no_ssh'
-    = render 'shared/no_password'
-    = render 'shared/shared_runners_minutes_limit', project: @project
-    - if @project.above_size_limit?
-      = render 'above_size_limit_warning'
+= render partial: 'flash_messages', locals: { project: @project }
 
 = render "projects/head"
 = render "projects/last_push"

app/workers/project_destroy_worker.rb

@@ -3,14 +3,11 @@ class ProjectDestroyWorker
   include DedicatedSidekiqQueue
 
   def perform(project_id, user_id, params)
-    begin
-      project = Project.unscoped.find(project_id)
-    rescue ActiveRecord::RecordNotFound
-      return
-    end
-
+    project = Project.find(project_id)
     user = User.find(user_id)
 
     ::Projects::DestroyService.new(project, user, params.symbolize_keys).execute
+  rescue ActiveRecord::RecordNotFound => error
+    logger.error("Failed to delete project (#{project_id}): #{error.message}")
   end
 end

changelogs/unreleased-ee/breadcrumbs-issue-bards-plural.yml

+---
+title: Renamed board to boards in new project sidebar
+merge_request:
+author:

changelogs/unreleased/22600-related-resources-uris-using-grape-source-helpers.yml

+---
+title: Declare related resources into V4 API entities
+merge_request:
+author:

changelogs/unreleased/29289-project-destroy-clean-up-after-failure.yml

+---
+title: Handle errors while a project is being deleted asynchronously.
+merge_request: 11088
+author:

changelogs/unreleased/35453-pending-delete-projects-error-in-admin-dashboard-fix.yml

+---
+title: Fixes 500 error caused by pending delete projects in admin dashboard
+merge_request: 13067
+author:

changelogs/unreleased/bump-omniauth-ldap-gem-version.yml

+---
+title: Prevent LDAP login callback from being called with a GET request
+merge_request: 13059
+author:

config/initializers/grape_route_helpers_fix.rb

+if defined?(GrapeRouteHelpers)
+  module GrapeRouteHelpers
+    class DecoratedRoute
+      # GrapeRouteHelpers gem tries to parse the versions
+      # from a string, not supporting Grape `version` array definition.
+      #
+      # Without the following fix, we get this on route helpers generation:
+      #
+      # => undefined method `scan' for ["v3", "v4"]
+      #
+      # 2.0.0 implementation of this method:
+      #
+      # ```
+      # def route_versions
+      #   version_pattern = /[^\[",\]\s]+/
+      #   if route_version
+      #     route_version.scan(version_pattern)
+      #   else
+      #     [nil]
+      #   end
+      # end
+      # ```
+      def route_versions
+        return [nil] if route_version.nil? || route_version.empty?
+
+        if route_version.is_a?(String)
+          version_pattern = /[^\[",\]\s]+/
+          route_version.scan(version_pattern)
+        else
+          route_version
+        end
+      end
+    end
+  end
+end

config/routes/api.rb

 API::API.logger Rails.logger
-mount API::API => '/api'
+mount API::API => '/'

db/migrate/20170428064307_add_column_delete_error_to_projects.rb

+class AddColumnDeleteErrorToProjects < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def change
+    add_column :projects, :delete_error, :text
+  end
+end

db/schema.rb

@@ -1388,6 +1388,7 @@ ActiveRecord::Schema.define(version: 20170719182937) do
     t.datetime "last_repository_updated_at"
     t.string "ci_config_path"
     t.boolean "disable_overriding_approvers_per_merge_request"
+    t.text "delete_error"
   end
 
   add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree

doc/administration/high_availability/alpha_database.md

 # Configuring a Database for GitLab HA
 
 **Warning**
-This functionality should be considered alpha. Use with caution.
-The steps listed in this document may not leave you with a configuration that matches
-what the released version of the software will do.
+This functionality should be considered beta, use with caution.
 **Warning**
 
 You can choose to install and manage a database server (PostgreSQL/MySQL)

doc/integration/jenkins.md

@@ -10,12 +10,25 @@ project service settings. We may remove this in a future release and recommend
 using the new 'Jenkins CI' project service instead which is described in this
 document.
 
-The Jenkins integration includes:
+## Overview
 
-* Trigger a Jenkins build after push to a repository and/or when a merge request
-  is created
-* Show build status on Merge Request page, on each commit and on the project
-  home page
+[Jenkins](https://jenkins.io/) is a great Continuous Integration tool, similar to our built-in
+[GitLab CI](../ci/README.md).
+
+GitLab's Jenkins integration allows you to trigger a Jenkins build when you
+push code to a repository, or when a merge request is created. Additionally,
+it shows the pipeline status on merge requests widgets and on the project's home page.
+
+## Use cases
+
+- Suppose you are new to GitLab, and want to keep using Jenkins until you prepare
+your projects to build with [GitLab CI/CD](../ci/README.md). You set up the
+integration between GitLab and Jenkins, then you migrate to GitLab CI later. While
+you organize yourself and your team to onboard GitLab, you keep your pipelines
+running with Jenkins, but view the results in your project's repository in GitLab.
+- Your team uses [Jenkins Plugins](https://plugins.jenkins.io/) for other proceedings,
+therefore, you opt for keep using Jenkins to build your apps. Show the results of your
+pipelines directly in GitLab.
 
 ## Requirements
 

lib/api/api.rb

@@ -3,6 +3,7 @@ module API
     include APIGuard
 
     allow_access_with_scope :api
+    prefix :api
 
     version %w(v3 v4), using: :path
 

lib/api/entities.rb

@@ -92,6 +92,38 @@ module API
     end
 
     class Project < Grape::Entity
+      include ::API::Helpers::RelatedResourcesHelpers
+
+      expose :_links do
+        expose :self do |project|
+          expose_url(api_v4_projects_path(id: project.id))
+        end
+
+        expose :issues, if: -> (*args) { issues_available?(*args) } do |project|
+          expose_url(api_v4_projects_issues_path(id: project.id))
+        end
+
+        expose :merge_requests, if: -> (*args) { mrs_available?(*args) } do |project|
+          expose_url(api_v4_projects_merge_requests_path(id: project.id))
+        end
+
+        expose :repo_branches do |project|
+          expose_url(api_v4_projects_repository_branches_path(id: project.id))
+        end
+
+        expose :labels do |project|
+          expose_url(api_v4_projects_labels_path(id: project.id))
+        end
+
+        expose :events do |project|
+          expose_url(api_v4_projects_events_path(id: project.id))
+        end
+
+        expose :members do |project|
+          expose_url(api_v4_projects_members_path(id: project.id))
+        end
+      end
+
       expose :id, :description, :default_branch, :tag_list
       expose :archived?, as: :archived
       expose :visibility, :ssh_url_to_repo, :http_url_to_repo, :web_url
@@ -327,6 +359,26 @@ module API
     end
 
     class Issue < IssueBasic
+      include ::API::Helpers::RelatedResourcesHelpers
+
+      expose :_links do
+        expose :self do |issue|
+          expose_url(api_v4_project_issue_path(id: issue.project_id, issue_iid: issue.iid))
+        end
+
+        expose :notes do |issue|
+          expose_url(api_v4_projects_issues_notes_path(id: issue.project_id, noteable_id: issue.iid))
+        end
+
+        expose :award_emoji do |issue|
+          expose_url(api_v4_projects_issues_award_emoji_path(id: issue.project_id, issue_iid: issue.iid))
+        end
+
+        expose :project do |issue|
+          expose_url(api_v4_projects_path(id: issue.project_id))
+        end
+      end
+
       expose :subscribed do |issue, options|
         issue.subscribed?(options[:current_user], options[:project] || issue.project)
       end

lib/api/helpers/related_resources_helpers.rb

+module API
+  module Helpers
+    module RelatedResourcesHelpers
+      include GrapeRouteHelpers::NamedRouteMatcher
+
+      def issues_available?(project, options)
+        available?(:issues, project, options[:current_user])
+      end
+
+      def mrs_available?(project, options)
+        available?(:merge_requests, project, options[:current_user])
+      end
+
+      def expose_url(path)
+        url_options = Rails.application.routes.default_url_options
+        host, protocol, port = url_options.slice(:host, :protocol, :port).values
+
+        URI::HTTP.build(scheme: protocol, host: host, port: port, path: path).to_s
+      end
+
+      private
+
+      def available?(feature, project, current_user)
+        project.feature_available?(feature, current_user)
+      end
+    end
+  end
+end

lib/api/issues.rb

@@ -117,7 +117,7 @@ module API
       params do
         requires :issue_iid, type: Integer, desc: 'The internal ID of a project issue'
       end
-      get ":id/issues/:issue_iid" do
+      get ":id/issues/:issue_iid", as: :api_v4_project_issue do
         issue = find_project_issue(params[:issue_iid])
         present issue, with: Entities::Issue, current_user: current_user, project: user_project
       end

lib/api/v3/entities.rb

@@ -274,11 +274,41 @@ module API
         expose :job_events, as: :build_events
       end
 
-      class Issue < ::API::Entities::Issue
+      class ProjectEntity < Grape::Entity
+        expose :id, :iid
+        expose(:project_id) { |entity| entity&.project.try(:id) }
+        expose :title, :description
+        expose :state, :created_at, :updated_at
+      end
+
+      class IssueBasic < ProjectEntity
+        expose :label_names, as: :labels
+        expose :milestone, using: ::API::Entities::Milestone
+        expose :assignees, :author, using: ::API::Entities::UserBasic
+
+        expose :assignee, using: ::API::Entities::UserBasic do |issue, options|
+          issue.assignees.first
+        end
+
+        expose :user_notes_count
+        expose :upvotes, :downvotes
+        expose :due_date
+        expose :confidential
+        expose :weight, if: ->(issue, _) { issue.supports_weight? }
+
+        expose :web_url do |issue, options|
+          Gitlab::UrlBuilder.build(issue)
+        end
+      end
+
+      class Issue < IssueBasic
         unexpose :assignees
         expose :assignee do |issue, options|
           ::API::Entities::UserBasic.represent(issue.assignees.first, options)
         end
+        expose :subscribed do |issue, options|
+          issue.subscribed?(options[:current_user], options[:project] || issue.project)
+        end
       end
     end
   end

spec/controllers/admin/dashboard_controller_spec.rb

@@ -22,5 +22,21 @@ describe Admin::DashboardController do
 
       expect(response).to have_http_status(404)
     end
+
+    context 'with pending_delete projects' do
+      render_views
+
+      it 'does not retrieve projects that are pending deletion' do
+        sign_in(create(:admin))
+
+        project = create(:project)
+        pending_delete_project = create(:project, pending_delete: true)
+
+        get :index
+
+        expect(response.body).to match(project.name)
+        expect(response.body).not_to match(pending_delete_project.name)
+      end
+    end
   end
 end

spec/controllers/ee/admin/application_settings_controller_spec.rb

@@ -65,8 +65,10 @@ describe Admin::ApplicationSettingsController do # rubocop:disable RSpec/FilePat
     it 'updates mirror settings when repository mirrors is licensed' do
       stub_licensed_features(repository_mirrors: true)
 
+      mirror_delay = (Gitlab::Mirror.min_delay_upper_bound / 60) + 1
+
       settings = {
-        mirror_max_delay: 12,
+        mirror_max_delay: mirror_delay,
         mirror_max_capacity: 2,
         mirror_capacity_threshold: 2
       }

spec/features/projects/show_project_spec.rb

+require 'spec_helper'
+
+describe 'Project show page', feature: true do
+  context 'when project pending delete' do
+    let(:project) { create(:project, :empty_repo, pending_delete: true) }
+
+    before do
+      sign_in(project.owner)
+    end
+
+    it 'shows flash error if deletion for project fails' do
+      project.update_attributes(delete_error: "Something went wrong", pending_delete: false)
+
+      visit project_path(project)
+
+      expect(page).to have_selector('.project-deletion-failed-message')
+      expect(page).to have_content("This project was scheduled for deletion, but failed with the following message: #{project.delete_error}")
+    end
+  end
+end

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -412,6 +412,7 @@ Project:
 - service_desk_enabled
 - last_repository_updated_at
 - ci_config_path
+- delete_error
 Author:
 - name
 ProjectFeature:

spec/requests/api/issues_spec.rb

@@ -697,6 +697,19 @@ describe API::Issues do
       expect(json_response['confidential']).to be_falsy
     end
 
+    context 'links exposure' do
+      it 'exposes related resources full URIs' do
+        get api("/projects/#{project.id}/issues/#{issue.iid}", user)
+
+        links = json_response['_links']
+
+        expect(links['self']).to end_with("/api/v4/projects/#{project.id}/issues/#{issue.iid}")
+        expect(links['notes']).to end_with("/api/v4/projects/#{project.id}/issues/#{issue.iid}/notes")
+        expect(links['award_emoji']).to end_with("/api/v4/projects/#{project.id}/issues/#{issue.iid}/award_emoji")
+        expect(links['project']).to end_with("/api/v4/projects/#{project.id}")
+      end
+    end
+
     it "returns a project issue by internal id" do
       get api("/projects/#{project.id}/issues/#{issue.iid}", user)
 

spec/requests/api/projects_spec.rb

@@ -866,6 +866,38 @@ describe API::Projects do
         expect(json_response).not_to include("import_error")
       end
 
+      context 'links exposure' do
+        it 'exposes related resources full URIs' do
+          get api("/projects/#{project.id}", user)
+
+          links = json_response['_links']
+
+          expect(links['self']).to end_with("/api/v4/projects/#{project.id}")
+          expect(links['issues']).to end_with("/api/v4/projects/#{project.id}/issues")
+          expect(links['merge_requests']).to end_with("/api/v4/projects/#{project.id}/merge_requests")
+          expect(links['repo_branches']).to end_with("/api/v4/projects/#{project.id}/repository/branches")
+          expect(links['labels']).to end_with("/api/v4/projects/#{project.id}/labels")
+          expect(links['events']).to end_with("/api/v4/projects/#{project.id}/events")
+          expect(links['members']).to end_with("/api/v4/projects/#{project.id}/members")
+        end
+
+        it 'filters related URIs when their feature is not enabled' do
+          project = create(:empty_project, :public,
+                           :merge_requests_disabled,
+                           :issues_disabled,
+                           creator_id: user.id,
+                           namespace: user.namespace)
+
+          get api("/projects/#{project.id}", user)
+
+          links = json_response['_links']
+
+          expect(links.has_key?('merge_requests')).to be_falsy
+          expect(links.has_key?('issues')).to be_falsy
+          expect(links['self']).to end_with("/api/v4/projects/#{project.id}")
+        end
+      end
+
       describe 'permissions' do
         context 'all projects' do
           before do

spec/services/ee/projects/destroy_service_spec.rb

@@ -40,5 +40,14 @@ describe Projects::DestroyService, services: true do
         expect { subject.execute }.to change(Geo::RepositoryDeletedEvent, :count).by(1)
       end
     end
+
+    it 'does not log event to the Geo log if project deletion fails' do
+      expect_any_instance_of(Project)
+        .to receive(:destroy!).and_raise(StandardError.new('Other error message'))
+
+      Sidekiq::Testing.inline! do
+        expect { subject.execute }.not_to change(Geo::RepositoryDeletedEvent, :count)
+      end
+    end
   end
 end

spec/services/projects/destroy_service_spec.rb

@@ -36,6 +36,27 @@ describe Projects::DestroyService, services: true do
     end
   end
 
+  shared_examples 'handles errors thrown during async destroy' do |error_message|
+    it 'does not allow the error to bubble up' do
+      expect do
+        Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+      end.not_to raise_error
+    end
+
+    it 'unmarks the project as "pending deletion"' do
+      Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+
+      expect(project.reload.pending_delete).to be(false)
+    end
+
+    it 'stores an error message in `projects.delete_error`' do
+      Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+
+      expect(project.reload.delete_error).to be_present
+      expect(project.delete_error).to include(error_message)
+    end
+  end
+
   context 'Sidekiq inline' do
     before do
       # Run sidekiq immediatly to check that renamed repository will be removed
@@ -102,10 +123,51 @@ describe Projects::DestroyService, services: true do
     end
 
     it_behaves_like 'deleting the project with pipeline and build'
-  end
 
-  context 'with execute' do
-    it_behaves_like 'deleting the project with pipeline and build'
+    context 'errors' do
+      context 'when `remove_legacy_registry_tags` fails' do
+        before do
+          expect_any_instance_of(Projects::DestroyService)
+            .to receive(:remove_legacy_registry_tags).and_return(false)
+        end
+
+        it_behaves_like 'handles errors thrown during async destroy', "Failed to remove some tags"
+      end
+
+      context 'when `remove_repository` fails' do
+        before do
+          expect_any_instance_of(Projects::DestroyService)
+            .to receive(:remove_repository).and_return(false)
+        end
+
+        it_behaves_like 'handles errors thrown during async destroy', "Failed to remove project repository"
+      end
+
+      context 'when `execute` raises any other error' do
+        before do
+          expect_any_instance_of(Project)
+            .to receive(:destroy!).and_raise(StandardError.new("Other error message"))
+        end
+
+        it_behaves_like 'handles errors thrown during async destroy', "Other error message"
+      end
+
+      context 'when `execute` raises unexpected error' do
+        before do
+          expect_any_instance_of(Project)
+            .to receive(:destroy!).and_raise(Exception.new("Other error message"))
+        end
+
+        it 'allows error to bubble up and rolls back project deletion' do
+          expect do
+            Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+          end.to raise_error
+
+          expect(project.reload.pending_delete).to be(false)
+          expect(project.delete_error).to include("Other error message")
+        end
+      end
+    end
   end
 
   describe 'container registry' do
@@ -132,8 +194,7 @@ describe Projects::DestroyService, services: true do
           expect_any_instance_of(ContainerRepository)
             .to receive(:delete_tags!).and_return(false)
 
-          expect{ destroy_project(project, user) }
-            .to raise_error(ActiveRecord::RecordNotDestroyed)
+          expect(destroy_project(project, user)).to be false
         end
       end
     end
@@ -158,8 +219,7 @@ describe Projects::DestroyService, services: true do
           expect_any_instance_of(ContainerRepository)
             .to receive(:delete_tags!).and_return(false)
 
-          expect { destroy_project(project, user) }
-            .to raise_error(Projects::DestroyService::DestroyError)
+          expect(destroy_project(project, user)).to be false
         end
       end
     end

spec/workers/project_destroy_worker_spec.rb

 require 'spec_helper'
 
 describe ProjectDestroyWorker do
-  let(:project) { create(:project, :repository) }
+  let(:project) { create(:project, :repository, pending_delete: true) }
   let(:path) { project.repository.path_to_repo }
 
   subject { described_class.new }
 
-  describe "#perform" do
-    it "deletes the project" do
+  describe '#perform' do
+    it 'deletes the project' do
       subject.perform(project.id, project.owner.id, {})
 
       expect(Project.all).not_to include(project)
       expect(Dir.exist?(path)).to be_falsey
     end
 
-    it "deletes the project but skips repo deletion" do
+    it 'deletes the project but skips repo deletion' do
       subject.perform(project.id, project.owner.id, { "skip_repo" => true })
 
       expect(Project.all).not_to include(project)
       expect(Dir.exist?(path)).to be_truthy
     end
+
+    it 'does not raise error when project could not be found' do
+      expect do
+        subject.perform(-1, project.owner.id, {})
+      end.not_to raise_error
+    end
+
+    it 'does not raise error when user could not be found' do
+      expect do
+        subject.perform(project.id, -1, {})
+      end.not_to raise_error
+    end
   end
 end