diff options
| -rw-r--r-- | lib/api/access_requests.rb | 11 | ||||
| -rw-r--r-- | lib/api/award_emoji.rb | 4 | ||||
| -rw-r--r-- | lib/api/boards.rb | 11 | ||||
| -rw-r--r-- | lib/api/broadcast_messages.rb | 4 | ||||
| -rw-r--r-- | lib/api/deploy_keys.rb | 5 | ||||
| -rw-r--r-- | lib/api/environments.rb | 4 | ||||
| -rw-r--r-- | lib/api/groups.rb | 7 | ||||
| -rw-r--r-- | lib/api/helpers.rb | 17 | ||||
| -rw-r--r-- | lib/api/issues.rb | 4 | ||||
| -rw-r--r-- | lib/api/labels.rb | 5 | ||||
| -rw-r--r-- | lib/api/members.rb | 7 | ||||
| -rw-r--r-- | lib/api/merge_requests.rb | 4 | ||||
| -rw-r--r-- | lib/api/notes.rb | 6 | ||||
| -rw-r--r-- | lib/api/project_hooks.rb | 5 | ||||
| -rw-r--r-- | lib/api/project_snippets.rb | 4 | ||||
| -rw-r--r-- | lib/api/projects.rb | 5 | ||||
| -rw-r--r-- | lib/api/runner.rb | 6 | ||||
| -rw-r--r-- | lib/api/runners.rb | 7 | ||||
| -rw-r--r-- | lib/api/services.rb | 4 | ||||
| -rw-r--r-- | lib/api/snippets.rb | 4 | ||||
| -rw-r--r-- | lib/api/system_hooks.rb | 5 | ||||
| -rw-r--r-- | lib/api/triggers.rb | 5 | ||||
| -rw-r--r-- | lib/api/users.rb | 47 | ||||
| -rw-r--r-- | spec/requests/api/tags_spec.rb | 4 | 
24 files changed, 71 insertions, 114 deletions
| diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb index 0c5b8862d79..4fa9b2b2494 100644 --- a/lib/api/access_requests.rb +++ b/lib/api/access_requests.rb @@ -67,13 +67,12 @@ module API          end          delete ":id/access_requests/:user_id" do            source = find_source(source_type, params[:id]) -          member = source.public_send(:requesters).find_by!(user_id: params[:user_id]) +          member = source.requesters.find_by!(user_id: params[:user_id]) -          check_unmodified_since(member.updated_at) - -          status 204 -          ::Members::DestroyService.new(source, current_user, params) -            .execute(:requesters) +          destroy_conditionally!(member) do +            ::Members::DestroyService.new(source, current_user, params) +              .execute(:requesters) +          end          end        end      end diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb index 51a8587d26e..8e3851640da 100644 --- a/lib/api/award_emoji.rb +++ b/lib/api/award_emoji.rb @@ -85,12 +85,10 @@ module API            end            delete "#{endpoint}/:award_id" do              award = awardable.award_emoji.find(params[:award_id]) -            check_unmodified_since(award.updated_at)              unauthorized! unless award.user == current_user || current_user.admin? -            status 204 -            award.destroy +            destroy_conditionally!(award)            end          end        end diff --git a/lib/api/boards.rb b/lib/api/boards.rb index d36df77dc6c..0d11c5fc971 100644 --- a/lib/api/boards.rb +++ b/lib/api/boards.rb @@ -122,14 +122,13 @@ module API          end          delete "/lists/:list_id" do            authorize!(:admin_list, user_project) -            list = board_lists.find(params[:list_id]) -          check_unmodified_since(list.updated_at) - -          service = ::Boards::Lists::DestroyService.new(user_project, current_user) -          unless service.execute(list) -            render_api_error!({ error: 'List could not be deleted!' }, 400) +          destroy_conditionally!(list) do |list| +            service = ::Boards::Lists::DestroyService.new(user_project, current_user) +            unless service.execute(list) +              render_api_error!({ error: 'List could not be deleted!' }, 400) +            end            end          end        end diff --git a/lib/api/broadcast_messages.rb b/lib/api/broadcast_messages.rb index 352972b584a..0b45621ce7b 100644 --- a/lib/api/broadcast_messages.rb +++ b/lib/api/broadcast_messages.rb @@ -90,10 +90,8 @@ module API        end        delete ':id' do          message = find_message -        check_unmodified_since(message.updated_at) -        status 204 -        message.destroy +        destroy_conditionally!(message)        end      end    end diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 971cc816454..f405c341398 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -125,10 +125,7 @@ module API          key = user_project.deploy_keys_projects.find_by(deploy_key_id: params[:key_id])          not_found!('Deploy Key') unless key -        check_unmodified_since(key.updated_at) - -        status 204 -        key.destroy +        destroy_conditionally!(key)        end      end    end diff --git a/lib/api/environments.rb b/lib/api/environments.rb index 3fc423ae79a..e33269f9483 100644 --- a/lib/api/environments.rb +++ b/lib/api/environments.rb @@ -78,10 +78,8 @@ module API          authorize! :update_environment, user_project          environment = user_project.environments.find(params[:environment_id]) -        check_unmodified_since(environment.updated_at) -        status 204 -        environment.destroy +        destroy_conditionally!(environment)        end        desc 'Stops an existing environment' do diff --git a/lib/api/groups.rb b/lib/api/groups.rb index c9b32a85487..ee2ad27837b 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -117,11 +117,10 @@ module API        delete ":id" do          group = find_group!(params[:id])          authorize! :admin_group, group -         -        check_unmodified_since(group.updated_at) -        status 204 -        ::Groups::DestroyService.new(group, current_user).execute +        destroy_conditionally!(group) do |group| +          ::Groups::DestroyService.new(group, current_user).execute +        end        end        desc 'Get a list of projects in this group.' do diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 1c74a14d91c..8d4f8c01903 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -11,14 +11,25 @@ module API        declared(params, options).to_h.symbolize_keys      end -    def check_unmodified_since(last_modified) -      if_unmodified_since = Time.parse(headers['If-Unmodified-Since']) if headers.key?('If-Unmodified-Since') rescue nil +    def check_unmodified_since!(last_modified) +      if_unmodified_since = Time.parse(headers['If-Unmodified-Since']) rescue nil -      if if_unmodified_since && if_unmodified_since < last_modified +      if if_unmodified_since && last_modified > if_unmodified_since          render_api_error!('412 Precondition Failed', 412)        end      end +    def destroy_conditionally!(resource, last_update_field: :updated_at) +      check_unmodified_since!(resource.public_send(last_update_field)) + +      status 204 +      if block_given? +        yield resource +      else +        resource.destroy +      end +    end +      def current_user        return @current_user if defined?(@current_user) diff --git a/lib/api/issues.rb b/lib/api/issues.rb index cee9898d3a6..6503629e2a2 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -230,10 +230,8 @@ module API          not_found!('Issue') unless issue          authorize!(:destroy_issue, issue) -        check_unmodified_since(issue.updated_at) -        status 204 -        issue.destroy +        destroy_conditionally!(issue)        end        desc 'List merge requests closing issue'  do diff --git a/lib/api/labels.rb b/lib/api/labels.rb index 45fa57fdf55..c0cf618ee8d 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -56,10 +56,7 @@ module API          label = user_project.labels.find_by(title: params[:name])          not_found!('Label') unless label -        check_unmodified_since(label.updated_at) - -        status 204 -        label.destroy +        destroy_conditionally!(label)        end        desc 'Update an existing label. At least one optional parameter is required.' do diff --git a/lib/api/members.rb b/lib/api/members.rb index 5634f123eca..a5d3d7f25a0 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -93,12 +93,11 @@ module API          end          delete ":id/members/:user_id" do            source = find_source(source_type, params[:id]) -          # Ensure that memeber exists            member = source.members.find_by!(user_id: params[:user_id]) -          check_unmodified_since(member.updated_at) -          status 204 -          ::Members::DestroyService.new(source, current_user, declared_params).execute +          destroy_conditionally!(member) do +            ::Members::DestroyService.new(source, current_user, declared_params).execute +          end          end        end      end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index c6fecc1aa6c..969c6064662 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -164,10 +164,8 @@ module API          merge_request = find_project_merge_request(params[:merge_request_iid])          authorize!(:destroy_merge_request, merge_request) -        check_unmodified_since(merge_request.updated_at) -        status 204 -        merge_request.destroy +        destroy_conditionally!(merge_request)        end        params do diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 58d71787aca..e116448c15b 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -131,10 +131,10 @@ module API            note = user_project.notes.find(params[:note_id])            authorize! :admin_note, note -          check_unmodified_since(note.updated_at) -          status 204 -          ::Notes::DestroyService.new(user_project, current_user).execute(note) +          destroy_conditionally!(note) do |note| +            ::Notes::DestroyService.new(user_project, current_user).execute(note) +          end          end        end      end diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 74d736fda59..5b457bbe639 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -96,10 +96,7 @@ module API        delete ":id/hooks/:hook_id" do          hook = user_project.hooks.find(params.delete(:hook_id)) -        check_unmodified_since(hook.updated_at) - -        status 204 -        hook.destroy +        destroy_conditionally!(hook)        end      end    end diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb index 645162d564d..704e8c6718d 100644 --- a/lib/api/project_snippets.rb +++ b/lib/api/project_snippets.rb @@ -116,10 +116,8 @@ module API          not_found!('Snippet') unless snippet          authorize! :admin_project_snippet, snippet -        check_unmodified_since(snippet.updated_at) -        status 204 -        snippet.destroy +        destroy_conditionally!(snippet)        end        desc 'Get a raw project snippet' diff --git a/lib/api/projects.rb b/lib/api/projects.rb index eab0ca0b3c9..36fe3f243b9 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -334,9 +334,10 @@ module API        desc 'Remove a project'        delete ":id" do          authorize! :remove_project, user_project -        check_unmodified_since(user_project.updated_at) -        ::Projects::DestroyService.new(user_project, current_user, {}).async_execute +        destroy_conditionally!(user_project) do +          ::Projects::DestroyService.new(user_project, current_user, {}).async_execute +        end          accepted!        end diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 88fc62d33df..daa8ddbe251 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -45,8 +45,10 @@ module API        end        delete '/' do          authenticate_runner! -        status 204 -        Ci::Runner.find_by_token(params[:token]).destroy + +        runner = Ci::Runner.find_by_token(params[:token]) + +        destroy_conditionally!(runner)        end        desc 'Validates authentication credentials' do diff --git a/lib/api/runners.rb b/lib/api/runners.rb index e3b2eb904b7..68c2120cc15 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -79,10 +79,8 @@ module API          runner = get_runner(params[:id])          authenticate_delete_runner!(runner) -        check_unmodified_since(runner.updated_at) -        status 204 -        runner.destroy! +        destroy_conditionally!(runner)        end      end @@ -137,8 +135,7 @@ module API          runner = runner_project.runner          forbidden!("Only one project associated with the runner. Please remove the runner instead") if runner.projects.count == 1 -        status 204 -        runner_project.destroy +        destroy_conditionally!(runner_project)        end      end diff --git a/lib/api/services.rb b/lib/api/services.rb index 4fef3383e5e..2b5ef75b6bf 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -655,8 +655,8 @@ module API        end        delete ":id/services/:service_slug" do          service = user_project.find_or_initialize_service(params[:service_slug].underscore) -        # Todo, not sure -        check_unmodified_since(service.updated_at) +        # Todo: Check if this done the right way +        check_unmodified_since!(service.updated_at)          attrs = service_attributes(service).inject({}) do |hash, key|            hash.merge!(key => nil) diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index 7107b3d669c..00eb7c60f16 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -122,10 +122,8 @@ module API          return not_found!('Snippet') unless snippet          authorize! :destroy_personal_snippet, snippet -        check_unmodified_since(snippet.updated_at) -        status 204 -        snippet.destroy +        destroy_conditionally!(snippet)        end        desc 'Get a raw snippet' do diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb index 64066a75b15..6b6a03e3300 100644 --- a/lib/api/system_hooks.rb +++ b/lib/api/system_hooks.rb @@ -66,10 +66,7 @@ module API          hook = SystemHook.find_by(id: params[:id])          not_found!('System hook') unless hook -        check_unmodified_since(hook.updated_at) - -        status 204 -        hook.destroy +        destroy_conditionally!(hook)        end      end    end diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb index 4ae70c65759..c9fee7e5193 100644 --- a/lib/api/triggers.rb +++ b/lib/api/triggers.rb @@ -140,10 +140,7 @@ module API          trigger = user_project.triggers.find(params.delete(:trigger_id))          return not_found!('Trigger') unless trigger -        check_unmodified_since(trigger.updated_at) - -        status 204 -        trigger.destroy +        destroy_conditionally!(trigger)        end      end    end diff --git a/lib/api/users.rb b/lib/api/users.rb index 942bb72cf97..d7c7b9ae9c1 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -230,13 +230,7 @@ module API          key = user.keys.find_by(id: params[:key_id])          not_found!('Key') unless key -<<<<<<< HEAD -        status 204 -======= -        check_unmodified_since(key.updated_at) - ->>>>>>> API: Respect the 'If-Unmodified-Since' for delete endpoints -        key.destroy +        destroy_conditionally!(key)        end        desc 'Add an email address to a specified user. Available only for admins.' do @@ -292,14 +286,11 @@ module API          email = user.emails.find_by(id: params[:email_id])          not_found!('Email') unless email -<<<<<<< HEAD -        Emails::DestroyService.new(user, email: email.email).execute -======= -        check_unmodified_since(email.updated_at) +        destroy_conditionally!(email) do |email| +          Emails::DestroyService.new(current_user, email: email.email).execute +        end -        email.destroy          user.update_secondary_emails! ->>>>>>> API: Respect the 'If-Unmodified-Since' for delete endpoints        end        desc 'Delete a user. Available only for admins.' do @@ -315,14 +306,9 @@ module API          user = User.find_by(id: params[:id])          not_found!('User') unless user -<<<<<<< HEAD -        status 204 -        user.delete_async(deleted_by: current_user, params: params) -======= -        check_unmodified_since(user.updated_at) - -        ::Users::DestroyService.new(current_user).execute(user) ->>>>>>> API: Respect the 'If-Unmodified-Since' for delete endpoints +        destroy_conditionally!(user) do +          user.delete_async(deleted_by: current_user, params: params) +        end        end        desc 'Block a user. Available only for admins.' @@ -500,10 +486,7 @@ module API          key = current_user.keys.find_by(id: params[:key_id])          not_found!('Key') unless key -        check_unmodified_since(key.updated_at) - -        status 204 -        key.destroy +        destroy_conditionally!(key)        end        desc "Get the currently authenticated user's email addresses" do @@ -554,9 +537,11 @@ module API          email = current_user.emails.find_by(id: params[:email_id])          not_found!('Email') unless email -<<<<<<< HEAD -        status 204 -        Emails::DestroyService.new(current_user, email: email.email).execute +        destroy_conditionally!(email) do |email| +          Emails::DestroyService.new(current_user, email: email.email).execute +        end + +        current_user.update_secondary_emails!        end        desc 'Get a list of user activities' @@ -572,12 +557,6 @@ module API            .reorder(last_activity_on: :asc)          present paginate(activities), with: Entities::UserActivity -======= -        check_unmodified_since(email.updated_at) - -        email.destroy -        current_user.update_secondary_emails! ->>>>>>> API: Respect the 'If-Unmodified-Since' for delete endpoints        end      end    end diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index 9884c1ec206..54900c75eb8 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -283,7 +283,7 @@ describe API::Tags do          it_behaves_like '404 response' do            let(:request) { delete api(route, current_user) } -          let(:message) { 'No such tag' } +          let(:message) { '404 Tag Not Found' }          end        end @@ -394,7 +394,7 @@ describe API::Tags do          it_behaves_like '404 response' do            let(:request) { put api(route, current_user), description: new_description } -          let(:message) { 'Tag does not exist' } +          let(:message) { '404 Tag Not Found' }          end        end | 
