diff options
author | Shinya Maeda <shinya@gitlab.com> | 2018-12-25 12:48:26 +0300 |
---|---|---|
committer | Shinya Maeda <shinya@gitlab.com> | 2018-12-31 08:35:57 +0300 |
commit | 8f1e96c89bd3ffe6ae47c275df2d1e919d42c39a (patch) | |
tree | 783bdd82084985736a8a07ab13d0e9f335f2fcd0 /app | |
parent | dc8a8c7d998e2c1f78fcf60f8dc45b572f62abe8 (diff) |
Add spec for Release API
Add spec for all release API - GET, POST, PUT, DELETE.
Also, fixes some minior bugs.
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/projects/tags/releases_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/projects/tags_controller.rb | 11 | ||||
-rw-r--r-- | app/models/release.rb | 27 | ||||
-rw-r--r-- | app/policies/project_policy.rb | 2 | ||||
-rw-r--r-- | app/policies/release_policy.rb | 5 | ||||
-rw-r--r-- | app/services/create_release_service.rb | 40 | ||||
-rw-r--r-- | app/services/delete_release_service.rb | 41 | ||||
-rw-r--r-- | app/services/releases/concerns.rb | 48 | ||||
-rw-r--r-- | app/services/releases/create_service.rb | 48 | ||||
-rw-r--r-- | app/services/releases/destroy_service.rb | 25 | ||||
-rw-r--r-- | app/services/releases/update_service.rb | 32 | ||||
-rw-r--r-- | app/services/tags/destroy_service.rb | 10 | ||||
-rw-r--r-- | app/services/update_release_service.rb | 18 |
13 files changed, 189 insertions, 120 deletions
diff --git a/app/controllers/projects/tags/releases_controller.rb b/app/controllers/projects/tags/releases_controller.rb index 3725df010b0..334e1847cc8 100644 --- a/app/controllers/projects/tags/releases_controller.rb +++ b/app/controllers/projects/tags/releases_controller.rb @@ -4,7 +4,7 @@ class Projects::Tags::ReleasesController < Projects::ApplicationController # Authorize before_action :require_non_empty_project before_action :authorize_download_code! - before_action :authorize_update_release! + before_action :authorize_push_code! before_action :tag before_action :release diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index 555e066b810..a17c050b696 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -48,8 +48,15 @@ class Projects::TagsController < Projects::ApplicationController if result[:status] == :success # Release creation with Tags was deprecated in GitLab 11.7 if params[:release_description].present? - release_params = { tag: params[:tag_name], description: params[:release_description] } - CreateReleaseService.new(@project, current_user, release_params).execute + release_params = { + tag: params[:tag_name], + name: params[:tag_name], + description: params[:release_description] + } + + Releases::CreateService + .new(@project, current_user, release_params) + .execute end @tag = result[:tag] diff --git a/app/models/release.rb b/app/models/release.rb index 7377af84e0b..df3dfe1cf2f 100644 --- a/app/models/release.rb +++ b/app/models/release.rb @@ -2,6 +2,7 @@ class Release < ActiveRecord::Base include CacheMarkdownField + include Gitlab::Utils::StrongMemoize cache_markdown_field :description @@ -15,25 +16,25 @@ class Release < ActiveRecord::Base delegate :repository, to: :project - def self.by_tag(project, tag) - self.find_by(project: project, tag: tag) - end - def commit - git_tag = repository.find_tag(tag) - repository.commit(git_tag.dereferenced_target) + strong_memoize(:commit) do + repository.commit(actual_sha) + end end - def sources_formats - @sources_formats ||= %w(zip tar.gz tar.bz2 tar).freeze + def tag_missing? + actual_tag.nil? end - # TODO: placeholder for frontend API compatibility - def links - [] + private + + def actual_sha + sha || actual_tag&.dereferenced_target end - def assets_count - links.size + sources_formats.size + def actual_tag + strong_memoize(:actual_tag) do + repository.find_tag(tag) + end end end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 88cab816fe4..3146f26bed5 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -270,7 +270,7 @@ class ProjectPolicy < BasePolicy enable :update_cluster enable :admin_cluster enable :create_environment_terminal - enable :admin_release + enable :destroy_release end rule { (mirror_available & can?(:admin_project)) | admin }.enable :admin_remote_mirror diff --git a/app/policies/release_policy.rb b/app/policies/release_policy.rb new file mode 100644 index 00000000000..d7f9e5d7445 --- /dev/null +++ b/app/policies/release_policy.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class ReleasePolicy < BasePolicy + delegate { @subject.project } +end diff --git a/app/services/create_release_service.rb b/app/services/create_release_service.rb deleted file mode 100644 index facf2a729ad..00000000000 --- a/app/services/create_release_service.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -class CreateReleaseService < BaseService - def execute(ref = nil) - return error('Unauthorized', 401) unless Ability.allowed?(current_user, :create_release, project) - - tag_result = find_or_create_tag(ref) - return tag_result if tag_result[:status] != :success - - create_release(tag_result[:tag]) - end - - private - - def find_or_create_tag(ref) - tag = repository.find_tag(params[:tag]) - return success(tag: tag) if tag - return error('Tag does not exist', 404) if ref.blank? - - Tags::CreateService.new(project, current_user).execute(params[:tag], ref, nil) - end - - def create_release(tag) - release = Release.by_tag(project, tag.name) - - if release - error('Release already exists', 409) - else - create_params = { - author: current_user, - name: tag.name, - sha: tag.dereferenced_target.sha - }.merge(params) - - release = project.releases.create!(create_params) - - success(tag: tag, release: release) - end - end -end diff --git a/app/services/delete_release_service.rb b/app/services/delete_release_service.rb deleted file mode 100644 index 4e595971949..00000000000 --- a/app/services/delete_release_service.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -class DeleteReleaseService < BaseService - include Gitlab::Utils::StrongMemoize - - def execute - return error('Tag does not exist', 404) unless existing_tag - return error('Release does not exist', 404) unless release - return error('Access Denied', 403) unless allowed? - - if release.destory - success(release: release) - else - error(release.errors.messages || '400 Bad request', 400) - end - end - - private - - def allowed? - Ability.allowed?(current_user, :admin_release, release) - end - - def release - strong_memoize(:release) do - project.releases.find_by_tag(@tag_name) - end - end - - def existing_tag - strong_memoize(:existing_tag) do - repository.find_tag(@tag_name) - end - end - - def repository - strong_memoize(:repository) do - project.repository - end - end -end diff --git a/app/services/releases/concerns.rb b/app/services/releases/concerns.rb new file mode 100644 index 00000000000..a04bb8f9e14 --- /dev/null +++ b/app/services/releases/concerns.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Releases + module Concerns + extend ActiveSupport::Concern + include Gitlab::Utils::StrongMemoize + + included do + def tag_name + params[:tag] + end + + def ref + params[:ref] + end + + def name + params[:name] + end + + def description + params[:description] + end + + def release + strong_memoize(:release) do + project.releases.find_by_tag(tag_name) + end + end + + def existing_tag + strong_memoize(:existing_tag) do + repository.find_tag(tag_name) + end + end + + def tag_exist? + existing_tag.present? + end + + def repository + strong_memoize(:repository) do + project.repository + end + end + end + end +end diff --git a/app/services/releases/create_service.rb b/app/services/releases/create_service.rb new file mode 100644 index 00000000000..210bdfbc699 --- /dev/null +++ b/app/services/releases/create_service.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Releases + class CreateService < BaseService + include Releases::Concerns + + def execute + return error('Access Denied', 403) unless allowed? + return error('Release already exists', 409) if release + + new_tag = nil + + unless tag_exist? + return error('Ref is not specified', 422) unless ref + + result = Tags::CreateService + .new(project, current_user) + .execute(tag_name, ref, nil) + + return result unless result[:status] == :success + + new_tag = result[:tag] + end + + create_release(existing_tag || new_tag) + end + + private + + def allowed? + Ability.allowed?(current_user, :create_release, project) + end + + def create_release(tag) + release = project.releases.create!( + name: name, + description: description, + author: current_user, + tag: tag.name, + sha: tag.dereferenced_target.sha + ) + + success(tag: tag, release: release) + rescue ActiveRecord::RecordInvalid => e + error(e.message, 400) + end + end +end diff --git a/app/services/releases/destroy_service.rb b/app/services/releases/destroy_service.rb new file mode 100644 index 00000000000..8c2bc3b4e6e --- /dev/null +++ b/app/services/releases/destroy_service.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Releases + class DestroyService < BaseService + include Releases::Concerns + + def execute + return error('Tag does not exist', 404) unless existing_tag + return error('Release does not exist', 404) unless release + return error('Access Denied', 403) unless allowed? + + if release.destroy + success(tag: existing_tag, release: release) + else + error(release.errors.messages || '400 Bad request', 400) + end + end + + private + + def allowed? + Ability.allowed?(current_user, :destroy_release, release) + end + end +end diff --git a/app/services/releases/update_service.rb b/app/services/releases/update_service.rb new file mode 100644 index 00000000000..fabfa398c59 --- /dev/null +++ b/app/services/releases/update_service.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Releases + class UpdateService < BaseService + include Releases::Concerns + + def execute + return error('Tag does not exist', 404) unless existing_tag + return error('Release does not exist', 404) unless release + return error('Access Denied', 403) unless allowed? + return error('params is empty', 400) if empty_params? + + if release.update(params) + success(tag: existing_tag, release: release) + else + error(release.errors.messages || '400 Bad request', 400) + end + end + + private + + def allowed? + Ability.allowed?(current_user, :update_release, release) + end + + # rubocop: disable CodeReuse/ActiveRecord + def empty_params? + params.except(:tag).empty? + end + # rubocop: enable CodeReuse/ActiveRecord + end +end diff --git a/app/services/tags/destroy_service.rb b/app/services/tags/destroy_service.rb index 6bfef09ac54..cab507946b4 100644 --- a/app/services/tags/destroy_service.rb +++ b/app/services/tags/destroy_service.rb @@ -2,7 +2,6 @@ module Tags class DestroyService < BaseService - # rubocop: disable CodeReuse/ActiveRecord def execute(tag_name) repository = project.repository tag = repository.find_tag(tag_name) @@ -12,8 +11,12 @@ module Tags end if repository.rm_tag(current_user, tag_name) - release = project.releases.find_by(tag: tag_name) - release&.destroy + ## + # When a tag in a repository is destroyed, + # release assets will be destroyed too. + Releases::DestroyService + .new(project, current_user, tag: tag_name) + .execute push_data = build_push_data(tag) EventCreateService.new.push(project, current_user, push_data) @@ -27,7 +30,6 @@ module Tags rescue Gitlab::Git::PreReceiveError => ex error(ex.message) end - # rubocop: enable CodeReuse/ActiveRecord def error(message, return_code = 400) super(message).merge(return_code: return_code) diff --git a/app/services/update_release_service.rb b/app/services/update_release_service.rb deleted file mode 100644 index f1d5d023100..00000000000 --- a/app/services/update_release_service.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -class UpdateReleaseService < BaseService - def execute - return error('Unauthorized', 401) unless Ability.allowed?(current_user, :update_release, project) - - tag_name = params[:tag] - release = Release.by_tag(project, tag_name) - - return error('Release does not exist', 404) if release.blank? - - if release.update(params) - success(release: release) - else - error(release.errors.messages || '400 Bad request', 400) - end - end -end |