summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinya Maeda <shinya@gitlab.com>2019-01-02 10:40:33 +0900
committerShinya Maeda <shinya@gitlab.com>2019-01-04 22:17:02 +0900
commit66755c9ed506af9f51022a678ed26e5d31ee87ac (patch)
treeae453c58166702161f7544e74995c43822017deb
parentb4f4edd4eed2e469c2b19a12166232f48eaf2181 (diff)
downloadgitlab-ce-66755c9ed506af9f51022a678ed26e5d31ee87ac.tar.gz
Support CURD operation for release asset links
- Add Releases::Links model - Expose it in release API - Add integration tests
-rw-r--r--app/models/release.rb14
-rw-r--r--app/models/releases/link.rb22
-rw-r--r--app/models/releases/source.rb34
-rw-r--r--app/services/releases/create_service.rb3
-rw-r--r--changelogs/unreleased/ac-releases-api-with-assets.yml5
-rw-r--r--db/migrate/20181228175414_create_releases_link_table.rb18
-rw-r--r--db/schema.rb10
-rw-r--r--lib/api/entities.rb22
-rw-r--r--lib/api/releases.rb11
-rw-r--r--lib/gitlab/import_export/import_export.yml3
-rw-r--r--lib/gitlab/import_export/relation_factory.rb3
-rw-r--r--spec/factories/releases/link.rb7
-rw-r--r--spec/fixtures/api/schemas/release.json18
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml3
-rw-r--r--spec/requests/api/releases_spec.rb201
15 files changed, 371 insertions, 3 deletions
diff --git a/app/models/release.rb b/app/models/release.rb
index df3dfe1cf2f..2e014fcf882 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.size + sources.size
+ 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..dc0482002ac
--- /dev/null
+++ b/app/models/releases/source.rb
@@ -0,0 +1,34 @@
+# 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_path),
+ format: format)
+ end
+
+ private
+
+ def archive_path
+ "#{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..1590d9137cc 100644
--- a/app/services/releases/create_service.rb
+++ b/app/services/releases/create_service.rb
@@ -43,7 +43,8 @@ module Releases
description: description,
author: current_user,
tag: tag.name,
- sha: tag.dereferenced_target.sha
+ sha: tag.dereferenced_target.sha,
+ links_attributes: params[:links_attributes] || []
)
success(tag: tag, release: release)
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..bf1e50dd36e
--- /dev/null
+++ b/db/migrate/20181228175414_create_releases_link_table.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class CreateReleasesLinkTable < ActiveRecord::Migration[5.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ def change
+ create_table :release_links 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, :name], unique: true
+ end
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 63267120c58..55b9a6aaf99 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1798,6 +1798,15 @@ 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", 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
+ end
+
create_table "releases", force: :cascade do |t|
t.string "tag"
t.text "description"
@@ -2439,6 +2448,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..63c755f3620 100644
--- a/lib/api/releases.rb
+++ b/lib/api/releases.rb
@@ -49,6 +49,10 @@ 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 :links_attributes, type: Array do
+ requires :name, type: String
+ requires :url, type: String
+ end
end
post ':id/releases' do
authorize_create_release!
@@ -72,6 +76,13 @@ module API
requires :tag_name, type: String, desc: 'The name of the tag', as: :tag
optional :name, type: String, desc: 'The name of the release'
optional :description, type: String, desc: 'Release notes with markdown support'
+ optional :links_attributes, type: Array do
+ optional :id, type: Integer
+ optional :name, type: String
+ optional :url, type: String
+ optional :_destroy, type: Integer
+ at_least_one_of :name, :url, :_destroy
+ end
end
put ':id/releases/:tag_name', requirements: RELEASE_ENDPOINT_REQUIREMETS do
authorize_update_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..4825e54d2f0
--- /dev/null
+++ b/spec/factories/releases/link.rb
@@ -0,0 +1,7 @@
+FactoryBot.define do
+ factory :release_link, class: ::Releases::Link do
+ release
+ name "release-18.04.dmg"
+ url 'https://my-external-hosting.example.com/scrambled-url/app.zip'
+ end
+end
diff --git a/spec/fixtures/api/schemas/release.json b/spec/fixtures/api/schemas/release.json
index 844405c3acd..234b82c5306 100644
--- a/spec/fixtures/api/schemas/release.json
+++ b/spec/fixtures/api/schemas/release.json
@@ -12,6 +12,24 @@
},
"author": {
"oneOf": [{ "type": "null" }, { "$ref": "public_api/v4/user/basic.json" }]
+ },
+ "assets": {
+ "count": { "type": "integer" },
+ "links": {
+ "type": "array",
+ "items": {
+ "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/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb
index a44e37f9cd5..43615475a50 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,52 @@ 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['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 +301,76 @@ describe API::Releases do
expect(response).to have_gitlab_http_status(:forbidden)
end
end
+
+ context 'when create assets altogether' do
+ context 'when create one asset' do
+ let(:params) do
+ {
+ name: 'New release',
+ tag_name: 'v0.1',
+ description: 'Super nice release',
+ links_attributes: [
+ {
+ 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
+ {
+ name: 'New release',
+ tag_name: 'v0.1',
+ description: 'Super nice release',
+ links_attributes: [
+ {
+ 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
+ end
+ end
end
context 'when tag does not exist in git repository' do
@@ -434,6 +551,90 @@ describe API::Releases do
end
end
+ context 'when links_attributes param is specified' do
+ context 'when the release does not have any link assets' do
+ let(:params) do
+ { links_attributes: [{ name: 'Beta release',
+ url: 'http://dosuken.com/win.exe' }] }
+ end
+
+ it 'creates an asset' do
+ put api("/projects/#{project.id}/releases/v0.1", maintainer),
+ params: params
+
+ expect(json_response['assets']['links'].count).to eq(1)
+ expect(json_response['assets']['links'].first['name'])
+ .to eq('Beta release')
+ expect(json_response['assets']['links'].first['url'])
+ .to eq('http://dosuken.com/win.exe')
+ end
+
+ context 'when url is invalid' do
+ let(:params) do
+ { links_attributes: [{ name: 'Beta release',
+ url: 'SELECT 1 from ci_builds;' }] }
+ end
+
+ it 'returns an error' do
+ put api("/projects/#{project.id}/releases/v0.1", maintainer),
+ params: params
+
+ expect(json_response['message']['links.url'].first)
+ .to include('Only allowed protocols are http, https')
+ end
+ end
+ end
+
+ context 'when the release has asset links' do
+ let!(:release_link_1) do
+ create(:release_link,
+ name: 'gcc',
+ url: 'http://dosuken.com/executable-gcc',
+ release: release,
+ created_at: 1.day.ago)
+ end
+
+ let!(:release_link_2) do
+ create(:release_link,
+ name: 'llvm',
+ url: 'http://dosuken.com/executable-llvm',
+ release: release,
+ created_at: 2.days.ago)
+ end
+
+ context 'when updates link names' do
+ let(:params) do
+ { links_attributes: [{ id: release_link_1.id, name: 'bin-gcc' },
+ { id: release_link_2.id, name: 'bin-llvm' }] }
+ end
+
+ it 'updates the asset' do
+ put api("/projects/#{project.id}/releases/v0.1", maintainer),
+ params: params
+
+ expect(json_response['assets']['links'].first['name'])
+ .to eq('bin-gcc')
+ expect(json_response['assets']['links'].second['name'])
+ .to eq('bin-llvm')
+ end
+ end
+
+ context 'when destroys an asset' do
+ let(:params) do
+ { links_attributes: [{ id: release_link_1.id, _destroy: '1' }] }
+ end
+
+ it 'updates the asset' do
+ put api("/projects/#{project.id}/releases/v0.1", maintainer),
+ params: params
+
+ expect(json_response['assets']['links'].count).to eq(1)
+ expect(json_response['assets']['links'].first['name']).to eq('llvm')
+ end
+ end
+ end
+ end
+
context 'when feature flag is disabled' do
before do
stub_feature_flags(releases_page: false)