Commit 1365e961 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'ac-releases-api-with-assets' into 'master'

Support asset links creation in `POST /projects/:id/release`

See merge request gitlab-org/gitlab-ce!24056
parent 549f2e41
...@@ -10,6 +10,10 @@ class Release < ActiveRecord::Base ...@@ -10,6 +10,10 @@ class Release < ActiveRecord::Base
# releases prior to 11.7 have no author # releases prior to 11.7 have no author
belongs_to :author, class_name: 'User' belongs_to :author, class_name: 'User'
has_many :links, class_name: 'Releases::Link'
accepts_nested_attributes_for :links, allow_destroy: true
validates :description, :project, :tag, presence: true validates :description, :project, :tag, presence: true
scope :sorted, -> { order(created_at: :desc) } scope :sorted, -> { order(created_at: :desc) }
...@@ -26,6 +30,16 @@ class Release < ActiveRecord::Base ...@@ -26,6 +30,16 @@ class Release < ActiveRecord::Base
actual_tag.nil? actual_tag.nil?
end end
def assets_count
links.count + sources.count
end
def sources
strong_memoize(:sources) do
Releases::Source.all(project, tag)
end
end
private private
def actual_sha def actual_sha
......
# frozen_string_literal: true
module Releases
class Link < ActiveRecord::Base
self.table_name = 'release_links'
belongs_to :release
validates :url, presence: true, url: true
validates :name, presence: true, uniqueness: { scope: :release }
scope :sorted, -> { order(created_at: :desc) }
def internal?
url.start_with?(release.project.web_url)
end
def external?
!internal?
end
end
end
# frozen_string_literal: true
module Releases
class Source
include ActiveModel::Model
attr_accessor :project, :tag_name, :format
FORMATS = %w(zip tar.gz tar.bz2 tar).freeze
class << self
def all(project, tag_name)
Releases::Source::FORMATS.map do |format|
Releases::Source.new(project: project,
tag_name: tag_name,
format: format)
end
end
end
def url
Gitlab::Routing
.url_helpers
.project_archive_url(project,
id: File.join(tag_name, archive_prefix),
format: format)
end
private
def archive_prefix
"#{project.path}-#{tag_name.tr('/', '-')}"
end
end
end
...@@ -43,11 +43,12 @@ module Releases ...@@ -43,11 +43,12 @@ module Releases
description: description, description: description,
author: current_user, author: current_user,
tag: tag.name, tag: tag.name,
sha: tag.dereferenced_target.sha sha: tag.dereferenced_target.sha,
links_attributes: params.dig(:assets, 'links') || []
) )
success(tag: tag, release: release) success(tag: tag, release: release)
rescue ActiveRecord::RecordInvalid => e rescue => e
error(e.message, 400) error(e.message, 400)
end end
end end
......
---
title: Support CURD operation for Links as one of the Release assets
merge_request: 24056
author:
type: changed
# frozen_string_literal: true
class CreateReleasesLinkTable < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
create_table :release_links, id: :bigserial do |t|
t.references :release, null: false, index: false, foreign_key: { on_delete: :cascade }
t.string :url, null: false
t.string :name, null: false
t.timestamps_with_timezone null: false
t.index [:release_id, :url], unique: true
t.index [:release_id, :name], unique: true
end
end
end
...@@ -2509,6 +2509,16 @@ ActiveRecord::Schema.define(version: 20190103140724) do ...@@ -2509,6 +2509,16 @@ ActiveRecord::Schema.define(version: 20190103140724) do
t.index ["source_type", "source_id"], name: "index_redirect_routes_on_source_type_and_source_id", using: :btree t.index ["source_type", "source_id"], name: "index_redirect_routes_on_source_type_and_source_id", using: :btree
end end
create_table "release_links", id: :bigserial, force: :cascade do |t|
t.integer "release_id", null: false
t.string "url", null: false
t.string "name", null: false
t.datetime_with_timezone "created_at", null: false
t.datetime_with_timezone "updated_at", null: false
t.index ["release_id", "name"], name: "index_release_links_on_release_id_and_name", unique: true, using: :btree
t.index ["release_id", "url"], name: "index_release_links_on_release_id_and_url", unique: true, using: :btree
end
create_table "releases", force: :cascade do |t| create_table "releases", force: :cascade do |t|
t.string "tag" t.string "tag"
t.text "description" t.text "description"
...@@ -3388,6 +3398,7 @@ ActiveRecord::Schema.define(version: 20190103140724) do ...@@ -3388,6 +3398,7 @@ ActiveRecord::Schema.define(version: 20190103140724) do
add_foreign_key "protected_tags", "projects", name: "fk_8e4af87648", on_delete: :cascade add_foreign_key "protected_tags", "projects", name: "fk_8e4af87648", on_delete: :cascade
add_foreign_key "push_event_payloads", "events", name: "fk_36c74129da", on_delete: :cascade add_foreign_key "push_event_payloads", "events", name: "fk_36c74129da", on_delete: :cascade
add_foreign_key "push_rules", "projects", name: "fk_83b29894de", on_delete: :cascade add_foreign_key "push_rules", "projects", name: "fk_83b29894de", on_delete: :cascade
add_foreign_key "release_links", "releases", on_delete: :cascade
add_foreign_key "releases", "projects", name: "fk_47fe2a0596", on_delete: :cascade add_foreign_key "releases", "projects", name: "fk_47fe2a0596", on_delete: :cascade
add_foreign_key "releases", "users", column: "author_id", name: "fk_8e4456f90f", on_delete: :nullify add_foreign_key "releases", "users", column: "author_id", name: "fk_8e4456f90f", on_delete: :nullify
add_foreign_key "remote_mirrors", "projects", name: "fk_43a9aa4ca8", on_delete: :cascade add_foreign_key "remote_mirrors", "projects", name: "fk_43a9aa4ca8", on_delete: :cascade
......
...@@ -1121,12 +1121,34 @@ module API ...@@ -1121,12 +1121,34 @@ module API
expose :description expose :description
end end
module Releases
class Link < Grape::Entity
expose :id
expose :name
expose :url
expose :external?, as: :external
end
class Source < Grape::Entity
expose :format
expose :url
end
end
class Release < TagRelease class Release < TagRelease
expose :name expose :name
expose :description_html expose :description_html
expose :created_at expose :created_at
expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? } expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? }
expose :commit, using: Entities::Commit expose :commit, using: Entities::Commit
expose :assets do
expose :assets_count, as: :count
expose :sources, using: Entities::Releases::Source
expose :links, using: Entities::Releases::Link do |release, options|
release.links.sorted
end
end
end end
class Tag < Grape::Entity class Tag < Grape::Entity
......
...@@ -49,6 +49,12 @@ module API ...@@ -49,6 +49,12 @@ module API
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: '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'
optional :assets, type: Hash do
optional :links, type: Array do
requires :name, type: String
requires :url, type: String
end
end
end end
post ':id/releases' do post ':id/releases' do
authorize_create_release! authorize_create_release!
......
...@@ -28,7 +28,8 @@ project_tree: ...@@ -28,7 +28,8 @@ project_tree:
- notes: - notes:
:author :author
- releases: - releases:
:author - :author
- :links
- project_members: - project_members:
- :user - :user
- merge_requests: - merge_requests:
......
...@@ -25,7 +25,8 @@ module Gitlab ...@@ -25,7 +25,8 @@ module Gitlab
custom_attributes: 'ProjectCustomAttribute', custom_attributes: 'ProjectCustomAttribute',
project_badges: 'Badge', project_badges: 'Badge',
metrics: 'MergeRequest::Metrics', metrics: 'MergeRequest::Metrics',
ci_cd_settings: 'ProjectCiCdSetting' }.freeze ci_cd_settings: 'ProjectCiCdSetting',
links: 'Releases::Link' }.freeze
USER_REFERENCES = %w[author_id assignee_id updated_by_id merged_by_id latest_closed_by_id user_id created_by_id last_edited_by_id merge_user_id resolved_by_id closed_by_id].freeze USER_REFERENCES = %w[author_id assignee_id updated_by_id merged_by_id latest_closed_by_id user_id created_by_id last_edited_by_id merge_user_id resolved_by_id closed_by_id].freeze
......
# frozen_string_literal: true
FactoryBot.define do
factory :release_link, class: ::Releases::Link do
release
sequence(:name) { |n| "release-18.#{n}.dmg" }
sequence(:url) { |n| "https://example.com/scrambled-url/app-#{n}.zip" }
end
end
...@@ -12,6 +12,25 @@ ...@@ -12,6 +12,25 @@
}, },
"author": { "author": {
"oneOf": [{ "type": "null" }, { "$ref": "public_api/v4/user/basic.json" }] "oneOf": [{ "type": "null" }, { "$ref": "public_api/v4/user/basic.json" }]
},
"assets": {
"count": { "type": "integer" },
"links": {
"type": "array",
"items": {
"id": "integer",
"name": "string",
"url": "string",
"external": "boolean"
}
},
"sources": {
"type": "array",
"items": {
"format": "zip",
"url": "string"
}
}
} }
}, },
"additionalProperties": false "additionalProperties": false
......
...@@ -66,6 +66,9 @@ snippets: ...@@ -66,6 +66,9 @@ snippets:
releases: releases:
- author - author
- project - project
- links
links:
- release
project_members: project_members:
- created_by - created_by
- user - user
......
...@@ -121,6 +121,13 @@ Release: ...@@ -121,6 +121,13 @@ Release:
- project_id - project_id
- created_at - created_at
- updated_at - updated_at
Releases::Link:
- id
- release_id
- url
- name
- created_at
- updated_at
ProjectMember: ProjectMember:
- id - id
- access_level - access_level
......
...@@ -10,10 +10,35 @@ RSpec.describe Release do ...@@ -10,10 +10,35 @@ RSpec.describe Release do
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:author).class_name('User') } it { is_expected.to belong_to(:author).class_name('User') }
it { is_expected.to have_many(:links).class_name('Releases::Link') }
end end
describe 'validation' do describe 'validation' 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 '#assets_count' do
subject { release.assets_count }
it 'returns the number of sources' do
is_expected.to eq(Releases::Source::FORMATS.count)
end
context 'when a links exists' do
let!(:link) { create(:release_link, release: release) }
it 'counts the link as an asset' do
is_expected.to eq(1 + Releases::Source::FORMATS.count)
end
end
end
describe '#sources' do
subject { release.sources }
it 'returns sources' do
is_expected.to all(be_a(Releases::Source))
end
end
end end
# frozen_string_literal: true
require 'spec_helper'
describe Releases::Link do
let(:release) { create(:release, project: project) }
let(:project) { create(:project) }
describe 'associations' do
it { is_expected.to belong_to(:release) }
end
describe 'validation' do
it { is_expected.to validate_presence_of(:url) }
it { is_expected.to validate_presence_of(:name) }
context 'when url is invalid' do
let(:link) { build(:release_link, url: 'hoge') }
it 'will be invalid' do
expect(link).to be_invalid
end
end
context 'when duplicate name is added to a release' do
let!(:link) { create(:release_link, name: 'alpha', release: release) }
it 'raises an error' do
expect do
create(:release_link, name: 'alpha', release: release)
end.to raise_error(ActiveRecord::RecordInvalid)
end
end
end
describe '.sorted' do
subject { described_class.sorted }
let!(:link_1) { create(:release_link, name: 'alpha', release: release, created_at: 1.day.ago) }
let!(:link_2) { create(:release_link, name: 'beta', release: release, created_at: 2.days.ago) }
it 'returns a list of links by created_at order' do
is_expected.to eq([link_1, link_2])
end
end
describe '#internal?' do
subject { link.internal? }
let(:link) { build(:release_link, release: release, url: url) }
let(:url) { "#{project.web_url}/-/jobs/140463678/artifacts/download" }
it { is_expected.to be_truthy }
context 'when link does not include project web url' do
let(:url) { 'https://google.com/-/jobs/140463678/artifacts/download' }
it { is_expected.to be_falsy }
end
end
describe '#external?' do
subject { link.external? }
let(:link) { build(:release_link, release: release, url: url) }
let(:url) { 'https://google.com/-/jobs/140463678/artifacts/download' }
it { is_expected.to be_truthy }
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Releases::Source do
set(:project) { create(:project, :repository, name: 'finance-cal') }
let(:tag_name) { 'v1.0' }
describe '.all' do
subject { described_class.all(project, tag_name) }
it 'returns all formats of sources' do
expect(subject.map(&:format))
.to match_array(described_class::FORMATS)
end
end
describe '#url' do
subject { source.url }
let(:source) do
described_class.new(project: project, tag_name: tag_name, format: format)
end
let(:format) { 'zip' }
it 'returns zip archived source url' do
is_expected
.to eq("#{project.web_url}/-/archive/v1.0/finance-cal-v1.0.zip")
end
context 'when ref is directory structure' do
let(:tag_name) { 'beta/v1.0' }
it 'converts slash to dash' do
is_expected
.to eq("#{project.web_url}/-/archive/beta/v1.0/finance-cal-beta-v1.0.zip")
end
end
end
end
...@@ -121,6 +121,7 @@ describe API::Releases do ...@@ -121,6 +121,7 @@ describe API::Releases do
expect(json_response['description']).to eq('This is v0.1') expect(json_response['description']).to eq('This is v0.1')
expect(json_response['author']['name']).to eq(maintainer.name) expect(json_response['author']['name']).to eq(maintainer.name)
expect(json_response['commit']['id']).to eq(commit.id) expect(json_response['commit']['id']).to eq(commit.id)
expect(json_response['assets']['count']).to eq(4)
end end
it 'matches response schema' do it 'matches response schema' do
...@@ -128,6 +129,53 @@ describe API::Releases do ...@@ -128,6 +129,53 @@ describe API::Releases do
expect(response).to match_response_schema('release') expect(response).to match_response_schema('release')
end end
it 'contains source information as assets' do
get api("/projects/#{project.id}/releases/v0.1", maintainer)
expect(json_response['assets']['sources'].map { |h| h['format'] })
.to match_array(release.sources.map(&:format))
expect(json_response['assets']['sources'].map { |h| h['url'] })
.to match_array(release.sources.map(&:url))
end
context 'when release has link asset' do
let!(:link) do
create(:release_link,
release: release,
name: 'release-18.04.dmg',
url: url)
end
let(:url) { 'https://my-external-hosting.example.com/scrambled-url/app.zip' }
it 'contains link information as assets' do
get api("/projects/#{project.id}/releases/v0.1", maintainer)
expect(json_response['assets']['links'].count).to eq(1)
expect(json_response['assets']['links'].first['id']).to eq(link.id)
expect(json_response['assets']['links'].first['name'])
.to eq('release-18.04.dmg')
expect(json_response['assets']['links'].first['url'])
.to eq('https://my-external-hosting.example.com/scrambled-url/app.zip')
expect(json_response['assets']['links'].first['external'])
.to be_truthy
end
context 'when link is internal' do
let(:url) do
"#{project.web_url}/-/jobs/artifacts/v11.6.0-rc4/download?" \
"job=rspec-mysql+41%2F50"
end
it 'has external false' do
get api("/projects/#{project.id}/releases/v0.1", maintainer)
expect(json_response['assets']['links'].first['external'])
.to be_falsy
end
end
end
end end
context 'when specified tag is not found in the project' do context 'when specified tag is not found in the project' do
...@@ -254,6 +302,90 @@ describe API::Releases do ...@@ -254,6 +302,90 @@ describe API::Releases do
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:forbidden)
end end
end end
context 'when create assets altogether' do
let(:base_params) do
{
name: 'New release',
tag_name: 'v0.1',
description: 'Super nice release'
}
end
context 'when create one asset' do
let(:params) do
base_params.merge({
assets: {
links: [{ name: 'beta', url: 'https://dosuken.example.com/inspection.exe' }]
}
})
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 an asset with specified parameters' do
post api("/projects/#{project.id}/releases", maintainer), params: params
expect(json_response['assets']['links'].count).to eq(1)
expect(json_response['assets']['links'].first['name']).to eq('beta')
expect(json_response['assets']['links'].first['url'])
.to eq('https://dosuken.example.com/inspection.exe')
end
it 'matches response schema' do
post api("/projects/#{project.id}/releases", maintainer), params: params
expect(response).to match_response_schema('release')
end
end
context 'when create two assets' do
let(:params) do
base_params.merge({
assets: {
links: [
{ name: 'alpha', url: 'https://dosuken.example.com/alpha.exe' },
{ name: 'beta', url: 'https://dosuken.example.com/beta.exe' }
]
}
})
end
it 'creates two assets with specified parameters' do
post api("/projects/#{project.id}/releases", maintainer), params: params
expect(json_response['assets']['links'].count).to eq(2)
expect(json_response['assets']['links'].map { |h| h['name'] })
.to match_array(%w[alpha beta])
expect(json_response['assets']['links'].map { |h| h['url'] })
.to match_array(%w[https://dosuken.example.com/alpha.exe
https://dosuken.example.com/beta.exe])
end
context 'when link names are duplicates' do
let(:params) do
base_params.merge({
assets: {
links: [
{ name: 'alpha', url: 'https://dosuken.example.com/alpha.exe' },
{ name: 'alpha', url: 'https://dosuken.example.com/beta.exe' }
]
}
})
end
it 'recognizes as a bad request' do
post api("/projects/#{project.id}/releases", maintainer), params: params
expect(response).to have_gitlab_http_status(:bad_request)
end
end
end
end
end end
context 'when tag does not exist in git repository' do context 'when tag does not exist in git repository' 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