diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2019-01-04 14:17:28 +0000 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2019-01-04 14:17:28 +0000 |
commit | f857d66b8d6d6b96ca80d28c58e901a82bfa5c85 (patch) | |
tree | 9f4491bfeefe0feeb57a4cd037bda090b81b5747 | |
parent | f77175f35368685b9e777fbd4ee42fcad799a786 (diff) | |
parent | 124d0cea577060c0eba1079c38e20a7ab8578fee (diff) | |
download | gitlab-ce-f857d66b8d6d6b96ca80d28c58e901a82bfa5c85.tar.gz |
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
-rw-r--r-- | app/models/release.rb | 14 | ||||
-rw-r--r-- | app/models/releases/link.rb | 22 | ||||
-rw-r--r-- | app/models/releases/source.rb | 35 | ||||
-rw-r--r-- | app/services/releases/create_service.rb | 5 | ||||
-rw-r--r-- | changelogs/unreleased/ac-releases-api-with-assets.yml | 5 | ||||
-rw-r--r-- | db/migrate/20181228175414_create_releases_link_table.rb | 19 | ||||
-rw-r--r-- | db/schema.rb | 11 | ||||
-rw-r--r-- | lib/api/entities.rb | 22 | ||||
-rw-r--r-- | lib/api/releases.rb | 6 | ||||
-rw-r--r-- | lib/gitlab/import_export/import_export.yml | 3 | ||||
-rw-r--r-- | lib/gitlab/import_export/relation_factory.rb | 3 | ||||
-rw-r--r-- | spec/factories/releases/link.rb | 9 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/release.json | 19 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/all_models.yml | 3 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/safe_model_attributes.yml | 7 | ||||
-rw-r--r-- | spec/models/release_spec.rb | 25 | ||||
-rw-r--r-- | spec/models/releases/link_spec.rb | 70 | ||||
-rw-r--r-- | spec/models/releases/source_spec.rb | 41 | ||||
-rw-r--r-- | spec/requests/api/releases_spec.rb | 132 |
19 files changed, 447 insertions, 4 deletions
diff --git a/app/models/release.rb b/app/models/release.rb index df3dfe1cf2f..0dae5c90394 100644 --- a/app/models/release.rb +++ b/app/models/release.rb @@ -10,6 +10,10 @@ class Release < ActiveRecord::Base # releases prior to 11.7 have no author 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 scope :sorted, -> { order(created_at: :desc) } @@ -26,6 +30,16 @@ class Release < ActiveRecord::Base actual_tag.nil? end + def assets_count + links.count + sources.count + end + + def sources + strong_memoize(:sources) do + Releases::Source.all(project, tag) + end + end + private def actual_sha diff --git a/app/models/releases/link.rb b/app/models/releases/link.rb new file mode 100644 index 00000000000..766cb2efff2 --- /dev/null +++ b/app/models/releases/link.rb @@ -0,0 +1,22 @@ +# 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 diff --git a/app/models/releases/source.rb b/app/models/releases/source.rb new file mode 100644 index 00000000000..4d3d54457af --- /dev/null +++ b/app/models/releases/source.rb @@ -0,0 +1,35 @@ +# 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 diff --git a/app/services/releases/create_service.rb b/app/services/releases/create_service.rb index 73fcebf79af..c6e143d440d 100644 --- a/app/services/releases/create_service.rb +++ b/app/services/releases/create_service.rb @@ -43,11 +43,12 @@ module Releases description: description, author: current_user, tag: tag.name, - sha: tag.dereferenced_target.sha + sha: tag.dereferenced_target.sha, + links_attributes: params.dig(:assets, 'links') || [] ) success(tag: tag, release: release) - rescue ActiveRecord::RecordInvalid => e + rescue => e error(e.message, 400) end end diff --git a/changelogs/unreleased/ac-releases-api-with-assets.yml b/changelogs/unreleased/ac-releases-api-with-assets.yml new file mode 100644 index 00000000000..b29319ae683 --- /dev/null +++ b/changelogs/unreleased/ac-releases-api-with-assets.yml @@ -0,0 +1,5 @@ +--- +title: Support CURD operation for Links as one of the Release assets +merge_request: 24056 +author: +type: changed diff --git a/db/migrate/20181228175414_create_releases_link_table.rb b/db/migrate/20181228175414_create_releases_link_table.rb new file mode 100644 index 00000000000..03558202137 --- /dev/null +++ b/db/migrate/20181228175414_create_releases_link_table.rb @@ -0,0 +1,19 @@ +# 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 diff --git a/db/schema.rb b/db/schema.rb index 63267120c58..97daf8ee617 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1798,6 +1798,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 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| t.string "tag" t.text "description" @@ -2439,6 +2449,7 @@ ActiveRecord::Schema.define(version: 20190103140724) do add_foreign_key "protected_tag_create_access_levels", "users" 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 "release_links", "releases", 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 "remote_mirrors", "projects", on_delete: :cascade diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 7116ab2882b..97ccd97e883 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1093,12 +1093,34 @@ module API expose :description 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 expose :name expose :description_html 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 :sources, using: Entities::Releases::Source + expose :links, using: Entities::Releases::Link do |release, options| + release.links.sorted + end + end end class Tag < Grape::Entity diff --git a/lib/api/releases.rb b/lib/api/releases.rb index 37d06988e64..c3d4101528c 100644 --- a/lib/api/releases.rb +++ b/lib/api/releases.rb @@ -49,6 +49,12 @@ module API 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' + optional :assets, type: Hash do + optional :links, type: Array do + requires :name, type: String + requires :url, type: String + end + end end post ':id/releases' do authorize_create_release! diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 25cdd5ab121..9fb1ae9f64b 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -28,7 +28,8 @@ project_tree: - notes: :author - releases: - :author + - :author + - :links - project_members: - :user - merge_requests: diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index a4902e2104f..a0f4dcfb772 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -23,7 +23,8 @@ module Gitlab custom_attributes: 'ProjectCustomAttribute', project_badges: 'Badge', 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 diff --git a/spec/factories/releases/link.rb b/spec/factories/releases/link.rb new file mode 100644 index 00000000000..d23db6d4bad --- /dev/null +++ b/spec/factories/releases/link.rb @@ -0,0 +1,9 @@ +# 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 diff --git a/spec/fixtures/api/schemas/release.json b/spec/fixtures/api/schemas/release.json index 844405c3acd..45fa8b074d4 100644 --- a/spec/fixtures/api/schemas/release.json +++ b/spec/fixtures/api/schemas/release.json @@ -12,6 +12,25 @@ }, "author": { "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 diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index c8c74883640..d3cae137c3c 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -66,6 +66,9 @@ snippets: releases: - author - project +- links +links: +- release project_members: - created_by - user diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 24b1f2d995b..2422868474e 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -120,6 +120,13 @@ Release: - project_id - created_at - updated_at +Releases::Link: +- id +- release_id +- url +- name +- created_at +- updated_at ProjectMember: - id - access_level diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 92ba2d82f58..157c96c1f65 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -10,10 +10,35 @@ RSpec.describe Release do describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:author).class_name('User') } + it { is_expected.to have_many(:links).class_name('Releases::Link') } end describe 'validation' do it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:description) } 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 diff --git a/spec/models/releases/link_spec.rb b/spec/models/releases/link_spec.rb new file mode 100644 index 00000000000..e88c186cbb8 --- /dev/null +++ b/spec/models/releases/link_spec.rb @@ -0,0 +1,70 @@ +# 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 diff --git a/spec/models/releases/source_spec.rb b/spec/models/releases/source_spec.rb new file mode 100644 index 00000000000..c5213196962 --- /dev/null +++ b/spec/models/releases/source_spec.rb @@ -0,0 +1,41 @@ +# 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 diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index a44e37f9cd5..978fa0142c2 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -121,6 +121,7 @@ describe API::Releases do 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) + expect(json_response['assets']['count']).to eq(4) end it 'matches response schema' do @@ -128,6 +129,53 @@ describe API::Releases do expect(response).to match_response_schema('release') 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 context 'when specified tag is not found in the project' do @@ -254,6 +302,90 @@ describe API::Releases do expect(response).to have_gitlab_http_status(:forbidden) 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 context 'when tag does not exist in git repository' do |