diff options
31 files changed, 1018 insertions, 362 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 diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 98ae4f83ad2..7116ab2882b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1099,20 +1099,6 @@ module API expose :created_at expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? } expose :commit, using: Entities::Commit - - expose :assets do - expose :assets_count, as: :count - expose :links - expose :sources do |release, _opts| - archive_path = "#{release.project.path}-#{release.tag.tr('/', '-')}" - release.sources_formats.map do |format| - { - format: format, - url: Gitlab::Routing.url_helpers.project_archive_url(release.project, id: File.join(release.tag, archive_path), format: format) - } - end - end - end end class Tag < Grape::Entity diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 742d3760533..c3eca713712 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -255,18 +255,6 @@ module API authorize! :update_build, user_project end - def authorize_create_release! - authorize! :create_release, user_project - end - - def authorize_read_release! - authorize! :read_release, user_project - end - - def authorize_update_release! - authorize! :update_release, user_project - end - def require_gitlab_workhorse! unless env['HTTP_GITLAB_WORKHORSE'].present? forbidden!('Request should be executed via GitLab Workhorse') diff --git a/lib/api/releases.rb b/lib/api/releases.rb index fe226a5ec4c..37d06988e64 100644 --- a/lib/api/releases.rb +++ b/lib/api/releases.rb @@ -4,10 +4,11 @@ module API class Releases < Grape::API include PaginationParams - RELEASE_ENDPOINT_REQUIREMETS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(tag_name: API::NO_SLASH_URL_PART_REGEX) + RELEASE_ENDPOINT_REQUIREMETS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS + .merge(tag_name: API::NO_SLASH_URL_PART_REGEX) before { error!('404 Not Found', 404) unless Feature.enabled?(:releases_page, user_project) } - before { authorize_read_release! } + before { authorize_read_releases! } params do requires :id, type: String, desc: 'The ID of a project' @@ -31,11 +32,10 @@ module API success Entities::Release end params do - requires :tag_name, type: String, desc: 'The name of the tag' + requires :tag_name, type: String, desc: 'The name of the tag', as: :tag end get ':id/releases/:tag_name', requirements: RELEASE_ENDPOINT_REQUIREMETS do - release = user_project.releases.find_by_tag(params[:tag_name]) - not_found!('Release') unless release + authorize_read_release! present release, with: Entities::Release end @@ -45,25 +45,22 @@ module API success Entities::Release end params do - requires :name, type: String, desc: 'The name of the release' - requires :tag_name, type: String, desc: 'The name of the tag', as: :tag - requires :description, type: String, desc: 'The release notes' - optional :ref, type: String, desc: 'The commit sha or branch name' + requires :tag_name, type: String, desc: 'The name of the tag', as: :tag + requires :name, type: String, desc: 'The name of the release' + requires :description, type: String, desc: 'The release notes' + optional :ref, type: String, desc: 'The commit sha or branch name' end post ':id/releases' do authorize_create_release! - attributes = declared(params) - ref = attributes.delete(:ref) - attributes.delete(:id) - - result = ::CreateReleaseService.new(user_project, current_user, attributes) - .execute(ref) + result = ::Releases::CreateService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute if result[:status] == :success present result[:release], with: Entities::Release else - render_api_error!(result[:message], 400) + render_api_error!(result[:message], result[:http_status]) end end @@ -73,15 +70,15 @@ module API end params do requires :tag_name, type: String, desc: 'The name of the tag', as: :tag - requires :name, type: String, desc: 'The name of the release' - requires :description, type: String, desc: 'Release notes with markdown support' + optional :name, type: String, desc: 'The name of the release' + optional :description, type: String, desc: 'Release notes with markdown support' end put ':id/releases/:tag_name', requirements: RELEASE_ENDPOINT_REQUIREMETS do authorize_update_release! - attributes = declared(params) - attributes.delete(:id) - result = UpdateReleaseService.new(user_project, current_user, attributes).execute + result = ::Releases::UpdateService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute if result[:status] == :success present result[:release], with: Entities::Release @@ -98,11 +95,11 @@ module API requires :tag_name, type: String, desc: 'The name of the tag', as: :tag end delete ':id/releases/:tag_name', requirements: RELEASE_ENDPOINT_REQUIREMETS do - authorize_update_release! + authorize_destroy_release! - attributes = declared(params) - attributes.delete(:id) - result = DeleteReleaseService.new(user_project, current_user, attributes).execute + result = ::Releases::DestroyService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute if result[:status] == :success present result[:release], with: Entities::Release @@ -111,5 +108,31 @@ module API end end end + + helpers do + def authorize_create_release! + authorize! :create_release, user_project + end + + def authorize_read_releases! + authorize! :read_release, user_project + end + + def authorize_read_release! + authorize! :read_release, release + end + + def authorize_update_release! + authorize! :update_release, release + end + + def authorize_destroy_release! + authorize! :destroy_release, release + end + + def release + @release ||= user_project.releases.find_by_tag(params[:tag]) + end + end end end diff --git a/lib/api/tags.rb b/lib/api/tags.rb index e1e9a7fbae5..aacdca3871a 100644 --- a/lib/api/tags.rb +++ b/lib/api/tags.rb @@ -60,10 +60,15 @@ module API if result[:status] == :success # Release creation with Tags API was deprecated in GitLab 11.7 if params[:release_description].present? - CreateReleaseService.new( - user_project, current_user, - tag: params[:tag_name], description: params[:release_description] - ).execute + release_create_params = { + tag: params[:tag_name], + name: params[:tag_name], # Name can be specified in new API + description: params[:release_description] + } + + ::Releases::CreateService + .new(user_project, current_user, release_create_params) + .execute end present result[:tag], @@ -107,9 +112,18 @@ module API post ':id/repository/tags/:tag_name/release', requirements: TAG_ENDPOINT_REQUIREMENTS do authorize_create_release! - attributes = declared(params) - attributes.delete(:id) - result = CreateReleaseService.new(user_project, current_user, attributes) + ## + # Legacy API does not support tag auto creation. + not_found!('Tag') unless user_project.repository.find_tag(params[:tag]) + + release_create_params = { + tag: params[:tag], + name: params[:tag], # Name can be specified in new API + description: params[:description] + } + + result = ::Releases::CreateService + .new(user_project, current_user, release_create_params) .execute if result[:status] == :success @@ -124,18 +138,15 @@ module API success Entities::TagRelease end params do - requires :tag_name, type: String, desc: 'The name of the tag' + requires :tag_name, type: String, desc: 'The name of the tag', as: :tag requires :description, type: String, desc: 'Release notes with markdown support' end put ':id/repository/tags/:tag_name/release', requirements: TAG_ENDPOINT_REQUIREMENTS do authorize_update_release! - result = UpdateReleaseService.new( - user_project, - current_user, - tag: params[:tag_name], - description: params[:description] - ).execute + result = ::Releases::UpdateService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute if result[:status] == :success present result[:release], with: Entities::TagRelease @@ -144,5 +155,19 @@ module API end end end + + helpers do + def authorize_create_release! + authorize! :create_release, user_project + end + + def authorize_update_release! + authorize! :update_release, release + end + + def release + @release ||= user_project.releases.find_by_tag(params[:tag]) + end + end end end diff --git a/spec/factories/releases.rb b/spec/factories/releases.rb index eaf3999e9ff..cab6b4a811f 100644 --- a/spec/factories/releases.rb +++ b/spec/factories/releases.rb @@ -1,9 +1,15 @@ FactoryBot.define do factory :release do tag "v1.1.0" + sha 'b83d6e391c22777fca1ed3012fce84f633d7fed0' name { tag } description "Awesome release" project author + + trait :legacy do + sha nil + author nil + end end end diff --git a/spec/fixtures/api/schemas/release.json b/spec/fixtures/api/schemas/release.json new file mode 100644 index 00000000000..844405c3acd --- /dev/null +++ b/spec/fixtures/api/schemas/release.json @@ -0,0 +1,18 @@ +{ + "type": "object", + "required": ["name", "tag_name"], + "properties": { + "name": { "type": "string" }, + "tag_name": { "type": "string" }, + "description": { "type": "string" }, + "description_html": { "type": "string" }, + "created_at": { "type": "date" }, + "commit": { + "oneOf": [{ "type": "null" }, { "$ref": "public_api/v4/commit/basic.json" }] + }, + "author": { + "oneOf": [{ "type": "null" }, { "$ref": "public_api/v4/user/basic.json" }] + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/releases.json b/spec/fixtures/api/schemas/releases.json new file mode 100644 index 00000000000..e26215707fe --- /dev/null +++ b/spec/fixtures/api/schemas/releases.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "release.json" } +} diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index d2df21d7bb5..6bc3792eb22 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -138,7 +138,7 @@ describe Gitlab::LegacyGithubImport::Importer do let(:release2) do double( - tag_name: 'v2.0.0', + tag_name: 'v1.1.0', name: 'Second release', body: nil, draft: false, diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 914fa4e2553..92ba2d82f58 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -16,18 +16,4 @@ RSpec.describe Release do it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:description) } end - - describe '.by_tag' do - let(:tag) { release.tag } - - subject { described_class.by_tag(project, tag) } - - it { is_expected.to eq(release) } - - context 'when no releases exists' do - let(:tag) { 'not-existing' } - - it { is_expected.to be_nil } - end - end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index ca339dd8ef5..9cb20854f6e 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -48,7 +48,7 @@ describe ProjectPolicy do update_deployment admin_project_snippet admin_project_member admin_note admin_wiki admin_project admin_commit_status admin_build admin_container_image - admin_pipeline admin_environment admin_deployment admin_release add_cluster + admin_pipeline admin_environment admin_deployment destroy_release add_cluster ] end @@ -184,7 +184,7 @@ describe ProjectPolicy do :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment, :create_cluster, :read_cluster, :update_cluster, :admin_cluster, :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment, - :admin_release + :destroy_release ] expect_disallowed(*repository_permissions) diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 7bdbe2ac06f..a44e37f9cd5 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -1,35 +1,532 @@ require 'spec_helper' describe API::Releases do - let(:user) { create(:user) } - let(:guest) { create(:user).tap { |u| project.add_guest(u) } } - let(:project) { create(:project, :repository, creator: user, path: 'my.project') } - let(:tag_name) { project.repository.find_tag('v1.1.0').name } - - let(:project_id) { project.id } - let(:current_user) { nil } + let(:project) { create(:project, :repository, :private) } + let(:maintainer) { create(:user) } + let(:reporter) { create(:user) } + let(:non_project_member) { create(:user) } + let(:commit) { create(:commit, project: project) } before do - project.add_maintainer(user) + project.add_maintainer(maintainer) + project.add_reporter(reporter) + + project.repository.add_tag(maintainer, 'v0.1', commit.id) + project.repository.add_tag(maintainer, 'v0.2', commit.id) end describe 'GET /projects/:id/releases' do - # TODO: + context 'when there are two releases' do + let!(:release_1) do + create(:release, + project: project, + tag: 'v0.1', + author: maintainer, + created_at: 2.days.ago) + end + + let!(:release_2) do + create(:release, + project: project, + tag: 'v0.2', + author: maintainer, + created_at: 1.day.ago) + end + + it 'returns 200 HTTP status' do + get api("/projects/#{project.id}/releases", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns releases ordered by created_at' do + get api("/projects/#{project.id}/releases", maintainer) + + expect(json_response.count).to eq(2) + expect(json_response.first['tag_name']).to eq(release_2.tag) + expect(json_response.second['tag_name']).to eq(release_1.tag) + end + + it 'matches response schema' do + get api("/projects/#{project.id}/releases", maintainer) + + expect(response).to match_response_schema('releases') + end + end + + context 'when tag does not exist in git repository' do + let!(:release) { create(:release, project: project, tag: 'v1.1.5') } + + it 'returns the tag' do + get api("/projects/#{project.id}/releases", maintainer) + + expect(json_response.count).to eq(1) + expect(json_response.first['tag_name']).to eq('v1.1.5') + expect(release).to be_tag_missing + end + end + + context 'when user is not a project member' do + it 'cannot find the project' do + get api("/projects/#{project.id}/releases", non_project_member) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'allows the request' do + get api("/projects/#{project.id}/releases", non_project_member) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(releases_page: false) + end + + it 'cannot find the API' do + get api("/projects/#{project.id}/releases", maintainer) + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe 'GET /projects/:id/releases/:tag_name' do - # TODO: + context 'when there is a release' do + let!(:release) do + create(:release, + project: project, + tag: 'v0.1', + sha: commit.id, + author: maintainer, + description: 'This is v0.1') + end + + it 'returns 200 HTTP status' do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns a release entry' do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(json_response['tag_name']).to eq(release.tag) + expect(json_response['description']).to eq('This is v0.1') + expect(json_response['author']['name']).to eq(maintainer.name) + expect(json_response['commit']['id']).to eq(commit.id) + end + + it 'matches response schema' do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(response).to match_response_schema('release') + end + end + + context 'when specified tag is not found in the project' do + it 'cannot find the release entry' do + get api("/projects/#{project.id}/releases/non_exist_tag", maintainer) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + let!(:release) { create(:release, tag: 'v0.1', project: project) } + + it 'cannot find the project' do + get api("/projects/#{project.id}/releases/v0.1", non_project_member) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'allows the request' do + get api("/projects/#{project.id}/releases/v0.1", non_project_member) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(releases_page: false) + end + + it 'cannot find the API' do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe 'POST /projects/:id/releases' do - # TODO: + let(:params) do + { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release' + } + end + + it 'accepts the request' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to have_gitlab_http_status(:created) + end + + it 'creates a new release' do + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.to change { Release.count }.by(1) + + expect(project.releases.last.name).to eq('New release') + expect(project.releases.last.tag).to eq('v0.1') + expect(project.releases.last.description).to eq('Super nice release') + end + + context 'when description is empty' do + let(:params) do + { + name: 'New release', + tag_name: 'v0.1', + description: '' + } + end + + it 'returns an error as validation failure' do + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.not_to change { Release.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']) + .to eq("Validation failed: Description can't be blank") + end + end + + it 'matches response schema' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to match_response_schema('release') + end + + it 'does not create a new tag' do + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.not_to change { Project.find_by_id(project.id).repository.tag_count } + end + + context 'when user is a reporter' do + it 'forbids the request' do + post api("/projects/#{project.id}/releases", reporter), params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + it 'forbids the request' do + post api("/projects/#{project.id}/releases", non_project_member), + params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'forbids the request' do + post api("/projects/#{project.id}/releases", non_project_member), + params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + context 'when tag does not exist in git repository' do + let(:params) do + { + name: 'Android ~ Ice Cream Sandwich ~', + tag_name: tag_name, + description: 'Android 4.0–4.0.4 "Ice Cream Sandwich" is the ninth' \ + 'version of the Android mobile operating system developed' \ + 'by Google.', + ref: 'master' + } + end + + let(:tag_name) { 'v4.0' } + + it 'creates a new tag' do + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.to change { Project.find_by_id(project.id).repository.tag_count }.by(1) + + expect(project.repository.find_tag('v4.0').dereferenced_target.id) + .to eq(project.repository.commit('master').id) + end + + it 'creates a new release' do + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.to change { Release.count }.by(1) + + expect(project.releases.last.name).to eq('Android ~ Ice Cream Sandwich ~') + expect(project.releases.last.tag).to eq('v4.0') + expect(project.releases.last.description).to eq( + 'Android 4.0–4.0.4 "Ice Cream Sandwich" is the ninth' \ + 'version of the Android mobile operating system developed' \ + 'by Google.') + end + + context 'when tag name is HEAD' do + let(:tag_name) { 'HEAD' } + + it 'returns an error as failure on tag creation' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(json_response['message']).to eq('Tag name invalid') + end + end + + context 'when tag name is empty' do + let(:tag_name) { '' } + + it 'returns an error as failure on tag creation' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(json_response['message']).to eq('Tag name invalid') + end + end + end + + context 'when release already exists' do + before do + create(:release, project: project, tag: 'v0.1', name: 'New release') + end + + it 'returns an error as conflicted request' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to have_gitlab_http_status(:conflict) + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(releases_page: false) + end + + it 'cannot find the API' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe 'PUT /projects/:id/releases/:tag_name' do - # TODO: + let(:params) { { description: 'Best release ever!' } } + + let!(:release) do + create(:release, + project: project, + tag: 'v0.1', + name: 'New release', + description: 'Super nice release') + end + + it 'accepts the request' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'updates the description' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(project.releases.last.description).to eq('Best release ever!') + end + + it 'does not change other attributes' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(project.releases.last.tag).to eq('v0.1') + expect(project.releases.last.name).to eq('New release') + end + + it 'matches response schema' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(response).to match_response_schema('release') + end + + context 'when user tries to update sha' do + let(:params) { { sha: 'xxx' } } + + it 'does not allow the request' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when params is empty' do + let(:params) { {} } + + it 'does not allow the request' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when there are no corresponding releases' do + let!(:release) { } + + it 'forbids the request' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is a reporter' do + it 'forbids the request' do + put api("/projects/#{project.id}/releases/v0.1", reporter), params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + it 'forbids the request' do + put api("/projects/#{project.id}/releases/v0.1", non_project_member), + params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'forbids the request' do + put api("/projects/#{project.id}/releases/v0.1", non_project_member), + params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(releases_page: false) + end + + it 'cannot find the API' do + put api("/projects/#{project.id}/releases/v0.1", non_project_member), + params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe 'DELETE /projects/:id/releases/:tag_name' do - # TODO: + let!(:release) do + create(:release, + project: project, + tag: 'v0.1', + name: 'New release', + description: 'Super nice release') + end + + it 'accepts the request' do + delete api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'destroys the release' do + expect do + delete api("/projects/#{project.id}/releases/v0.1", maintainer) + end.to change { Release.count }.by(-1) + end + + it 'does not remove a tag in repository' do + expect do + delete api("/projects/#{project.id}/releases/v0.1", maintainer) + end.not_to change { Project.find_by_id(project.id).repository.tag_count } + end + + it 'matches response schema' do + delete api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(response).to match_response_schema('release') + end + + context 'when there are no corresponding releases' do + let!(:release) { } + + it 'forbids the request' do + delete api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is a reporter' do + it 'forbids the request' do + delete api("/projects/#{project.id}/releases/v0.1", reporter) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + it 'forbids the request' do + delete api("/projects/#{project.id}/releases/v0.1", non_project_member) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'forbids the request' do + delete api("/projects/#{project.id}/releases/v0.1", non_project_member) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(releases_page: false) + end + + it 'cannot find the API' do + delete api("/projects/#{project.id}/releases/v0.1", non_project_member) + + expect(response).to have_gitlab_http_status(:not_found) + end + end end end diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index 12cfac96d31..d09b6fe72b1 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -107,9 +107,12 @@ describe API::Tags do context 'with releases' do let(:description) { 'Awesome release!' } - before do - release = project.releases.find_or_initialize_by(tag: tag_name) - release.update(description: description) + let!(:release) do + create(:release, + :legacy, + project: project, + tag: tag_name, + description: description) end it 'returns an array of project tags with release info' do @@ -373,7 +376,7 @@ describe API::Tags do it_behaves_like '404 response' do let(:request) { post api(route, current_user), params: { description: description } } - let(:message) { 'Tag does not exist' } + let(:message) { '404 Tag Not Found' } end end @@ -398,10 +401,7 @@ describe API::Tags do end context 'on tag with existing release' do - before do - release = project.releases.find_or_initialize_by(tag: tag_name) - release.update(description: description) - end + let!(:release) { create(:release, :legacy, project: project, tag: tag_name, description: description) } it 'returns 409 if there is already a release' do post api(route, user), params: { description: description } @@ -420,9 +420,12 @@ describe API::Tags do shared_examples_for 'repository update release' do context 'on tag with existing release' do - before do - release = project.releases.find_or_initialize_by(tag: tag_name) - release.update(description: description) + let!(:release) do + create(:release, + :legacy, + project: project, + tag: tag_name, + description: description) end it 'updates the release description' do @@ -437,9 +440,9 @@ describe API::Tags do context 'when tag does not exist' do let(:tag_name) { 'unknown' } - it_behaves_like '404 response' do + it_behaves_like '403 response' do let(:request) { put api(route, current_user), params: { description: new_description } } - let(:message) { 'Tag does not exist' } + let(:message) { '403 Forbidden' } end end @@ -464,9 +467,9 @@ describe API::Tags do end context 'when release does not exist' do - it_behaves_like '404 response' do + it_behaves_like '403 response' do let(:request) { put api(route, current_user), params: { description: new_description } } - let(:message) { 'Release does not exist' } + let(:message) { '403 Forbidden' } end end end diff --git a/spec/services/create_release_service_spec.rb b/spec/services/create_release_service_spec.rb deleted file mode 100644 index 1dd74a1bcdb..00000000000 --- a/spec/services/create_release_service_spec.rb +++ /dev/null @@ -1,78 +0,0 @@ -require 'spec_helper' - -describe CreateReleaseService do - let(:project) { create(:project, :repository) } - let(:user) { create(:user) } - let(:tag_name) { project.repository.tag_names.first } - let(:name) { 'Bionic Beaver'} - let(:description) { 'Awesome release!' } - let(:params) { { tag: tag_name, name: name, description: description } } - let(:service) { described_class.new(project, user, params) } - let(:ref) { nil } - - before do - project.add_maintainer(user) - end - - shared_examples 'a successful release creation' do - it 'creates a new release' do - result = service.execute(ref) - expect(result[:status]).to eq(:success) - release = project.releases.find_by(tag: tag_name) - expect(release).not_to be_nil - expect(release.description).to eq(description) - expect(release.name).to eq(name) - expect(release.author).to eq(user) - end - end - - it_behaves_like 'a successful release creation' - - it 'raises an error if the tag does not exist' do - service.params[:tag] = 'foobar' - - result = service.execute - expect(result[:status]).to eq(:error) - end - - it 'keeps track of the commit sha' do - tag = project.repository.find_tag(tag_name) - sha = tag.dereferenced_target.sha - result = service.execute - - expect(result[:status]).to eq(:success) - expect(project.releases.find_by(tag: tag_name).sha).to eq(sha) - end - - context 'when ref is provided' do - let(:ref) { 'master' } - let(:tag_name) { 'foobar' } - - it_behaves_like 'a successful release creation' - - it 'creates a tag if the tag does not exist' do - expect(project.repository.ref_exists?("refs/tags/#{tag_name}")).to be_falsey - - result = service.execute(ref) - expect(result[:status]).to eq(:success) - expect(project.repository.ref_exists?("refs/tags/#{tag_name}")).to be_truthy - - release = project.releases.find_by(tag: tag_name) - expect(release).not_to be_nil - end - end - - context 'there already exists a release on a tag' do - before do - service.execute - end - - it 'raises an error and does not update the release' do - service.params[:description] = 'The best release!' - - result = service.execute - expect(result[:status]).to eq(:error) - expect(project.releases.find_by(tag: tag_name).description).to eq(description) - end - end -end diff --git a/spec/services/delete_release_service_spec.rb b/spec/services/delete_release_service_spec.rb deleted file mode 100644 index faefdb342bb..00000000000 --- a/spec/services/delete_release_service_spec.rb +++ /dev/null @@ -1 +0,0 @@ -#TODO:
\ No newline at end of file diff --git a/spec/services/releases/create_service_spec.rb b/spec/services/releases/create_service_spec.rb new file mode 100644 index 00000000000..612e9f152e7 --- /dev/null +++ b/spec/services/releases/create_service_spec.rb @@ -0,0 +1,72 @@ +require 'spec_helper' + +describe Releases::CreateService do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + let(:tag_name) { project.repository.tag_names.first } + let(:tag_sha) { project.repository.find_tag(tag_name).dereferenced_target.sha } + let(:name) { 'Bionic Beaver' } + let(:description) { 'Awesome release!' } + let(:params) { { tag: tag_name, name: name, description: description, ref: ref } } + let(:ref) { nil } + let(:service) { described_class.new(project, user, params) } + + before do + project.add_maintainer(user) + end + + describe '#execute' do + shared_examples 'a successful release creation' do + it 'creates a new release' do + result = service.execute + expect(result[:status]).to eq(:success) + expect(result[:tag]).not_to be_nil + expect(result[:release]).not_to be_nil + expect(result[:release].description).to eq(description) + expect(result[:release].name).to eq(name) + expect(result[:release].author).to eq(user) + expect(result[:release].sha).to eq(tag_sha) + end + end + + it_behaves_like 'a successful release creation' + + context 'when the tag does not exist' do + let(:tag_name) { 'non-exist-tag' } + + it 'raises an error' do + result = service.execute + + expect(result[:status]).to eq(:error) + end + end + + context 'when ref is provided' do + let(:ref) { 'master' } + let(:tag_name) { 'foobar' } + + it_behaves_like 'a successful release creation' + + it 'creates a tag if the tag does not exist' do + expect(project.repository.ref_exists?("refs/tags/#{tag_name}")).to be_falsey + + result = service.execute + expect(result[:status]).to eq(:success) + expect(result[:tag]).not_to be_nil + expect(result[:release]).not_to be_nil + end + end + + context 'there already exists a release on a tag' do + let!(:release) do + create(:release, project: project, tag: tag_name, description: description) + end + + it 'raises an error and does not update the release' do + result = service.execute + expect(result[:status]).to eq(:error) + expect(project.releases.find_by(tag: tag_name).description).to eq(description) + end + end + end +end diff --git a/spec/services/releases/destroy_service_spec.rb b/spec/services/releases/destroy_service_spec.rb new file mode 100644 index 00000000000..dd5b8708f36 --- /dev/null +++ b/spec/services/releases/destroy_service_spec.rb @@ -0,0 +1,61 @@ +require 'spec_helper' + +describe Releases::DestroyService do + let(:project) { create(:project, :repository) } + let(:mainatiner) { create(:user) } + let(:repoter) { create(:user) } + let(:tag) { 'v1.1.0' } + let!(:release) { create(:release, project: project, tag: tag) } + let(:service) { described_class.new(project, user, params) } + let(:params) { { tag: tag } } + let(:user) { mainatiner } + + before do + project.add_maintainer(mainatiner) + project.add_reporter(repoter) + end + + describe '#execute' do + subject { service.execute } + + context 'when there is a release' do + it 'removes the release' do + expect { subject }.to change { project.releases.count }.by(-1) + end + + it 'returns the destroyed object' do + is_expected.to include(status: :success, release: release) + end + end + + context 'when tag is not found' do + let(:tag) { 'v1.1.1' } + + it 'returns an error' do + is_expected.to include(status: :error, + message: 'Tag does not exist', + http_status: 404) + end + end + + context 'when release is not found' do + let!(:release) { } + + it 'returns an error' do + is_expected.to include(status: :error, + message: 'Release does not exist', + http_status: 404) + end + end + + context 'when user does not have permission' do + let(:user) { repoter } + + it 'returns an error' do + is_expected.to include(status: :error, + message: 'Access Denied', + http_status: 403) + end + end + end +end diff --git a/spec/services/releases/update_service_spec.rb b/spec/services/releases/update_service_spec.rb new file mode 100644 index 00000000000..6c68f364739 --- /dev/null +++ b/spec/services/releases/update_service_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +describe Releases::UpdateService do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + let(:new_name) { 'A new name' } + let(:new_description) { 'The best release!' } + let(:params) { { name: new_name, description: new_description, tag: tag_name } } + let(:service) { described_class.new(project, user, params) } + let!(:release) { create(:release, project: project, author: user, tag: tag_name) } + let(:tag_name) { 'v1.1.0' } + + before do + project.add_developer(user) + end + + describe '#execute' do + shared_examples 'a failed update' do + it 'raises an error' do + result = service.execute + expect(result[:status]).to eq(:error) + end + end + + it 'successfully updates an existing release' do + result = service.execute + expect(result[:status]).to eq(:success) + expect(result[:release].name).to eq(new_name) + expect(result[:release].description).to eq(new_description) + end + + context 'when the tag does not exists' do + let(:tag_name) { 'foobar' } + + it_behaves_like 'a failed update' + end + + context 'when the release does not exist' do + let!(:release) { } + + it_behaves_like 'a failed update' + end + + context 'with an invalid update' do + let(:new_description) { '' } + + it_behaves_like 'a failed update' + end + end +end diff --git a/spec/services/update_release_service_spec.rb b/spec/services/update_release_service_spec.rb deleted file mode 100644 index 4378d5d072a..00000000000 --- a/spec/services/update_release_service_spec.rb +++ /dev/null @@ -1,53 +0,0 @@ -require 'spec_helper' - -describe UpdateReleaseService do - let(:project) { create(:project, :repository) } - let(:user) { create(:user) } - let(:tag_name) { project.repository.tag_names.first } - let(:description) { 'Awesome release!' } - let(:new_name) { 'A new name' } - let(:new_description) { 'The best release!' } - let(:params) { { name: new_name, description: new_description, tag: tag_name } } - let(:service) { described_class.new(project, user, params) } - let(:create_service) { CreateReleaseService.new(project, user, tag: tag_name, description: description) } - - before do - create_service.execute - end - - shared_examples 'a failed update' do - it 'raises an error' do - result = service.execute - expect(result[:status]).to eq(:error) - end - end - - it 'successfully updates an existing release' do - result = service.execute - expect(result[:status]).to eq(:success) - - release = project.releases.find_by(tag: tag_name) - expect(release.name).to eq(new_name) - expect(release.description).to eq(new_description) - end - - context 'when the tag does not exists' do - let(:tag_name) { 'foobar' } - - it_behaves_like 'a failed update' - end - - context 'when the release does not exist' do - before do - project.releases.delete_all - end - - it_behaves_like 'a failed update' - end - - context 'with an invalid update' do - let(:new_description) { '' } - - it_behaves_like 'a failed update' - end -end |