Add spec for Release API

Add spec for all release API - GET, POST, PUT, DELETE.
Also, fixes some minior bugs.

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