Commit 6a2decf5 authored by Alessio Caiazza's avatar Alessio Caiazza Committed by Shinya Maeda

Refactor Release services

CreateReleaseService and UpdateReleaseService now takes all the release
attributes as constructor parameters. This will simplify attribute
expansion
parent a7aaad96
...@@ -48,8 +48,8 @@ class Projects::TagsController < Projects::ApplicationController ...@@ -48,8 +48,8 @@ class Projects::TagsController < Projects::ApplicationController
if result[:status] == :success if result[:status] == :success
# Release creation with Tags was deprecated in GitLab 11.7 # Release creation with Tags was deprecated in GitLab 11.7
if params[:release_description].present? if params[:release_description].present?
CreateReleaseService.new(@project, current_user) release_params = { tag: params[:tag_name], description: params[:release_description] }
.execute(params[:tag_name], params[:release_description]) CreateReleaseService.new(@project, current_user, release_params).execute
end end
@tag = result[:tag] @tag = result[:tag]
......
...@@ -15,6 +15,10 @@ class Release < ActiveRecord::Base ...@@ -15,6 +15,10 @@ class Release < ActiveRecord::Base
delegate :repository, to: :project delegate :repository, to: :project
def self.by_tag(project, tag)
self.find_by(project: project, tag: tag)
end
def commit def commit
git_tag = repository.find_tag(tag) git_tag = repository.find_tag(tag)
repository.commit(git_tag.dereferenced_target) repository.commit(git_tag.dereferenced_target)
......
# frozen_string_literal: true # frozen_string_literal: true
class CreateReleaseService < BaseService class CreateReleaseService < BaseService
def execute(tag_name, release_description, name: nil, ref: nil) def execute(ref = nil)
repository = project.repository return error('Unauthorized', 401) unless Ability.allowed?(current_user, :create_release, project)
tag = repository.find_tag(tag_name)
if tag.blank? && ref.present? tag_result = find_or_create_tag(ref)
result = create_tag(tag_name, ref) return tag_result if tag_result[:status] != :success
return result unless result[:status] == :success
tag = result[:tag] create_release(tag_result[:tag])
end
if tag.present?
create_release(tag, name, release_description)
else
error('Tag does not exist', 404)
end
end
def success(release)
super().merge(release: release)
end end
private private
def create_release(tag, name, description) def find_or_create_tag(ref)
release = project.releases.find_by(tag: tag.name) # rubocop: disable CodeReuse/ActiveRecord 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 if release
error('Release already exists', 409) error('Release already exists', 409)
else else
release = project.releases.create!( create_params = {
tag: tag.name,
name: name || tag.name,
sha: tag.dereferenced_target.sha,
author: current_user, author: current_user,
description: description name: tag.name,
) sha: tag.dereferenced_target.sha
}.merge(params)
success(release) release = project.releases.create!(create_params)
end
end
def create_tag(tag_name, ref) success(tag: tag, release: release)
Tags::CreateService.new(project, current_user) end
.execute(tag_name, ref, nil)
end end
end end
# frozen_string_literal: true # frozen_string_literal: true
class UpdateReleaseService < BaseService class UpdateReleaseService < BaseService
attr_accessor :tag_name
def initialize(project, user, tag_name, params)
super(project, user, params)
@tag_name = tag_name
end
# rubocop: disable CodeReuse/ActiveRecord
def execute def execute
repository = project.repository return error('Unauthorized', 401) unless Ability.allowed?(current_user, :update_release, project)
existing_tag = repository.find_tag(@tag_name)
if existing_tag tag_name = params[:tag]
release = project.releases.find_by(tag: @tag_name) release = Release.by_tag(project, tag_name)
if release return error('Release does not exist', 404) if release.blank?
if release.update(params)
success(release) if release.update(params)
else success(release: release)
error(release.errors.messages || '400 Bad request', 400)
end
else
error('Release does not exist', 404)
end
else else
error('Tag does not exist', 404) error(release.errors.messages || '400 Bad request', 400)
end end
end end
# rubocop: enable CodeReuse/ActiveRecord
def success(release)
super().merge(release: release)
end
end end
...@@ -46,15 +46,19 @@ module API ...@@ -46,15 +46,19 @@ module API
end end
params do params do
requires :name, type: String, desc: 'The name of the release' requires :name, type: String, desc: 'The name of the release'
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: 'The release notes' requires :description, type: String, desc: 'The release notes'
optional :ref, type: String, desc: 'The commit sha or branch name' optional :ref, type: String, desc: 'The commit sha or branch name'
end end
post ':id/releases' do post ':id/releases' do
authorize_create_release! authorize_create_release!
result = ::CreateReleaseService.new(user_project, current_user) attributes = declared(params)
.execute(params[:tag_name], params[:description], params[:name], params[:ref]) ref = attributes.delete(:ref)
attributes.delete(:id)
result = ::CreateReleaseService.new(user_project, current_user, attributes)
.execute(ref)
if result[:status] == :success if result[:status] == :success
present result[:release], with: Entities::Release present result[:release], with: Entities::Release
...@@ -68,7 +72,7 @@ module API ...@@ -68,7 +72,7 @@ module API
success Entities::Release success Entities::Release
end end
params do 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 :name, type: String, desc: 'The name of the release' requires :name, type: String, desc: 'The name of the release'
requires :description, type: String, desc: 'Release notes with markdown support' requires :description, type: String, desc: 'Release notes with markdown support'
end end
...@@ -76,8 +80,8 @@ module API ...@@ -76,8 +80,8 @@ module API
authorize_update_release! authorize_update_release!
attributes = declared(params) attributes = declared(params)
tag = attributes.delete(:tag_name) attributes.delete(:id)
result = UpdateReleaseService.new(user_project, current_user, tag, attributes).execute result = UpdateReleaseService.new(user_project, current_user, attributes).execute
if result[:status] == :success if result[:status] == :success
present result[:release], with: Entities::Release present result[:release], with: Entities::Release
......
...@@ -60,8 +60,10 @@ module API ...@@ -60,8 +60,10 @@ module API
if result[:status] == :success if result[:status] == :success
# Release creation with Tags API was deprecated in GitLab 11.7 # Release creation with Tags API was deprecated in GitLab 11.7
if params[:release_description].present? if params[:release_description].present?
CreateReleaseService.new(user_project, current_user) CreateReleaseService.new(
.execute(params[:tag_name], params[:release_description]) user_project, current_user,
tag: params[:tag_name], description: params[:release_description]
).execute
end end
present result[:tag], present result[:tag],
...@@ -99,14 +101,16 @@ module API ...@@ -99,14 +101,16 @@ module API
success Entities::TagRelease success Entities::TagRelease
end end
params do 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' requires :description, type: String, desc: 'Release notes with markdown support'
end end
post ':id/repository/tags/:tag_name/release', requirements: TAG_ENDPOINT_REQUIREMENTS do post ':id/repository/tags/:tag_name/release', requirements: TAG_ENDPOINT_REQUIREMENTS do
authorize_create_release! authorize_create_release!
result = CreateReleaseService.new(user_project, current_user) attributes = declared(params)
.execute(params[:tag_name], params[:description]) attributes.delete(:id)
result = CreateReleaseService.new(user_project, current_user, attributes)
.execute
if result[:status] == :success if result[:status] == :success
present result[:release], with: Entities::TagRelease present result[:release], with: Entities::TagRelease
...@@ -129,7 +133,7 @@ module API ...@@ -129,7 +133,7 @@ module API
result = UpdateReleaseService.new( result = UpdateReleaseService.new(
user_project, user_project,
current_user, current_user,
params[:tag_name], tag: params[:tag_name],
description: params[:description] description: params[:description]
).execute ).execute
......
...@@ -16,4 +16,18 @@ RSpec.describe Release do ...@@ -16,4 +16,18 @@ RSpec.describe Release do
it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_presence_of(:description) } it { is_expected.to validate_presence_of(:description) }
end 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 end
...@@ -6,12 +6,17 @@ describe CreateReleaseService do ...@@ -6,12 +6,17 @@ describe CreateReleaseService do
let(:tag_name) { project.repository.tag_names.first } let(:tag_name) { project.repository.tag_names.first }
let(:name) { 'Bionic Beaver'} let(:name) { 'Bionic Beaver'}
let(:description) { 'Awesome release!' } let(:description) { 'Awesome release!' }
let(:service) { described_class.new(project, user) } let(:params) { { tag: tag_name, name: name, description: description } }
let(:service) { described_class.new(project, user, params) }
let(:ref) { nil } let(:ref) { nil }
before do
project.add_maintainer(user)
end
shared_examples 'a successful release creation' do shared_examples 'a successful release creation' do
it 'creates a new release' do it 'creates a new release' do
result = service.execute(tag_name, description, name: name, ref: ref) result = service.execute(ref)
expect(result[:status]).to eq(:success) expect(result[:status]).to eq(:success)
release = project.releases.find_by(tag: tag_name) release = project.releases.find_by(tag: tag_name)
expect(release).not_to be_nil expect(release).not_to be_nil
...@@ -24,14 +29,16 @@ describe CreateReleaseService do ...@@ -24,14 +29,16 @@ describe CreateReleaseService do
it_behaves_like 'a successful release creation' it_behaves_like 'a successful release creation'
it 'raises an error if the tag does not exist' do it 'raises an error if the tag does not exist' do
result = service.execute("foobar", description) service.params[:tag] = 'foobar'
result = service.execute
expect(result[:status]).to eq(:error) expect(result[:status]).to eq(:error)
end end
it 'keeps track of the commit sha' do it 'keeps track of the commit sha' do
tag = project.repository.find_tag(tag_name) tag = project.repository.find_tag(tag_name)
sha = tag.dereferenced_target.sha sha = tag.dereferenced_target.sha
result = service.execute(tag_name, description, name: name) result = service.execute
expect(result[:status]).to eq(:success) expect(result[:status]).to eq(:success)
expect(project.releases.find_by(tag: tag_name).sha).to eq(sha) expect(project.releases.find_by(tag: tag_name).sha).to eq(sha)
...@@ -46,7 +53,7 @@ describe CreateReleaseService do ...@@ -46,7 +53,7 @@ describe CreateReleaseService do
it 'creates a tag if the tag does not exist' do it 'creates a tag if the tag does not exist' do
expect(project.repository.ref_exists?("refs/tags/#{tag_name}")).to be_falsey expect(project.repository.ref_exists?("refs/tags/#{tag_name}")).to be_falsey
result = service.execute(tag_name, description, name: name, ref: ref) result = service.execute(ref)
expect(result[:status]).to eq(:success) expect(result[:status]).to eq(:success)
expect(project.repository.ref_exists?("refs/tags/#{tag_name}")).to be_truthy expect(project.repository.ref_exists?("refs/tags/#{tag_name}")).to be_truthy
...@@ -57,11 +64,13 @@ describe CreateReleaseService do ...@@ -57,11 +64,13 @@ describe CreateReleaseService do
context 'there already exists a release on a tag' do context 'there already exists a release on a tag' do
before do before do
service.execute(tag_name, description) service.execute
end end
it 'raises an error and does not update the release' do it 'raises an error and does not update the release' do
result = service.execute(tag_name, 'The best release!') service.params[:description] = 'The best release!'
result = service.execute
expect(result[:status]).to eq(:error) expect(result[:status]).to eq(:error)
expect(project.releases.find_by(tag: tag_name).description).to eq(description) expect(project.releases.find_by(tag: tag_name).description).to eq(description)
end end
......
...@@ -7,12 +7,12 @@ describe UpdateReleaseService do ...@@ -7,12 +7,12 @@ describe UpdateReleaseService do
let(:description) { 'Awesome release!' } let(:description) { 'Awesome release!' }
let(:new_name) { 'A new name' } let(:new_name) { 'A new name' }
let(:new_description) { 'The best release!' } let(:new_description) { 'The best release!' }
let(:params) { { name: new_name, description: new_description } } let(:params) { { name: new_name, description: new_description, tag: tag_name } }
let(:service) { described_class.new(project, user, tag_name, params) } let(:service) { described_class.new(project, user, params) }
let(:create_service) { CreateReleaseService.new(project, user) } let(:create_service) { CreateReleaseService.new(project, user, tag: tag_name, description: description) }
before do before do
create_service.execute(tag_name, description) create_service.execute
end end
shared_examples 'a failed update' do shared_examples 'a failed update' do
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment