diff --git a/doc/api/projects.md b/doc/api/projects.md index 32b26f3272649ad9dde6f37ec67c31972d12a8b2..56fa36a44f81d053a6e6e9f6e043b3ef66294dd1 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -2248,6 +2248,19 @@ Returned object: } ``` +## Remove a project avatar + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92604) in GitLab 15.4. + +To remove a project avatar, use a blank value for the `avatar` attribute. + +Example request: + +```shell +curl --request PUT --header "PRIVATE-TOKEN: " \ + --data "avatar=" "https://gitlab.example.com/api/v4/projects/5" +``` + ## Share project with group Allow to share project with group. diff --git a/lib/api/helpers/projects_helpers.rb b/lib/api/helpers/projects_helpers.rb index 628182ad1ab0bb69f7ebcf215becd5556e66fcfe..2d0e8ae4bfd60fee967b9785729edbcfdc3c8add 100644 --- a/lib/api/helpers/projects_helpers.rb +++ b/lib/api/helpers/projects_helpers.rb @@ -58,7 +58,7 @@ module ProjectsHelpers optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'Deprecated: Use :topics instead' optional :topics, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The list of topics for a project' # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 - optional :avatar, type: File, desc: 'Avatar image for project' # rubocop:disable Scalability/FileUploads + optional :avatar, type: ::API::Validations::Types::WorkhorseFile, desc: 'Avatar image for project' optional :printing_merge_request_link_enabled, type: Boolean, desc: 'Show link to create/view merge request when pushing from the command line' optional :merge_method, type: String, values: %w(ff rebase_merge merge), desc: 'The merge method used when merging merge requests' optional :suggestion_commit_message, type: String, desc: 'The commit message used to apply merge request suggestions' diff --git a/lib/api/projects.rb b/lib/api/projects.rb index f8dc3d45a5c8fa6425d23c3d8d7b088cd65da5b7..8c58cc585d8a1193a2dd737a74e06efa431b7057 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -453,6 +453,8 @@ def add_import_params(params) filter_attributes_using_license!(attrs) verify_update_project_attrs!(user_project, attrs) + user_project.remove_avatar! if attrs.key?(:avatar) && attrs[:avatar].nil? + result = ::Projects::UpdateService.new(user_project, current_user, attrs).execute if result[:status] == :success diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index fcbd0cc784b43cd9b372ced03c96eb594f9b74c5..ac534b4e71174862c056f7d74b787400b32eb49d 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -48,6 +48,7 @@ RSpec.describe API::Projects do include ProjectForksHelper + include WorkhorseHelpers include StubRequests let_it_be(:user) { create(:user) } @@ -1349,7 +1350,12 @@ it 'uploads avatar for project a project' do project = attributes_for(:project, avatar: fixture_file_upload('spec/fixtures/banana_sample.gif', 'image/gif')) - post api('/projects', user), params: project + workhorse_form_with_file( + api('/projects', user), + method: :post, + file_key: :avatar, + params: project + ) project_id = json_response['id'] expect(json_response['avatar_url']).to eq("http://localhost/uploads/-/system/project/avatar/#{project_id}/banana_sample.gif") @@ -1925,8 +1931,6 @@ end describe "POST /projects/:id/uploads/authorize" do - include WorkhorseHelpers - let(:headers) { workhorse_internal_api_request_header.merge({ 'HTTP_GITLAB_WORKHORSE' => 1 }) } context 'with authorized user' do @@ -3584,18 +3588,77 @@ def failure_message(diff) end end - it 'updates avatar' do - project_param = { - avatar: fixture_file_upload('spec/fixtures/banana_sample.gif', - 'image/gif') - } + context 'with changes to the avatar' do + let_it_be(:avatar_file) { fixture_file_upload('spec/fixtures/banana_sample.gif', 'image/gif') } + let_it_be(:alternate_avatar_file) { fixture_file_upload('spec/fixtures/rails_sample.png', 'image/png') } + let_it_be(:project_with_avatar, reload: true) do + create(:project, + :private, + :repository, + name: 'project-with-avatar', + creator_id: user.id, + namespace: user.namespace, + avatar: avatar_file) + end - put api("/projects/#{project3.id}", user), params: project_param + it 'uploads avatar to project without an avatar' do + workhorse_form_with_file( + api("/projects/#{project3.id}", user), + method: :put, + file_key: :avatar, + params: { avatar: avatar_file } + ) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['avatar_url']).to eq('http://localhost/uploads/'\ - '-/system/project/avatar/'\ - "#{project3.id}/banana_sample.gif") + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['avatar_url']).to eq('http://localhost/uploads/'\ + '-/system/project/avatar/'\ + "#{project3.id}/banana_sample.gif") + end + end + + it 'uploads and changes avatar to project with an avatar' do + workhorse_form_with_file( + api("/projects/#{project_with_avatar.id}", user), + method: :put, + file_key: :avatar, + params: { avatar: alternate_avatar_file } + ) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['avatar_url']).to eq('http://localhost/uploads/'\ + '-/system/project/avatar/'\ + "#{project_with_avatar.id}/rails_sample.png") + end + end + + it 'uploads and changes avatar to project among other changes' do + workhorse_form_with_file( + api("/projects/#{project_with_avatar.id}", user), + method: :put, + file_key: :avatar, + params: { description: 'changed description', avatar: avatar_file } + ) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['description']).to eq('changed description') + expect(json_response['avatar_url']).to eq('http://localhost/uploads/'\ + '-/system/project/avatar/'\ + "#{project_with_avatar.id}/banana_sample.gif") + end + end + + it 'removes avatar from project with an avatar' do + put api("/projects/#{project_with_avatar.id}", user), params: { avatar: '' } + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['avatar_url']).to be_nil + expect(project_with_avatar.reload.avatar_url).to be_nil + end + end end it 'updates auto_devops_deploy_strategy' do diff --git a/workhorse/internal/upstream/routes.go b/workhorse/internal/upstream/routes.go index c889f87ed96c319dcee92e43b713e3245ceed5b5..c56089904eb3305a51d82fee7d5a52dc2bf03ba3 100644 --- a/workhorse/internal/upstream/routes.go +++ b/workhorse/internal/upstream/routes.go @@ -51,7 +51,7 @@ const ( gitProjectPattern = `^/.+\.git/` geoGitProjectPattern = `^/[^-].+\.git/` // Prevent matching routes like /-/push_from_secondary projectPattern = `^/([^/]+/){1,}[^/]+/` - apiProjectPattern = apiPattern + `v4/projects/[^/]+/` // API: Projects can be encoded via group%2Fsubgroup%2Fproject + apiProjectPattern = apiPattern + `v4/projects/[^/]+` // API: Projects can be encoded via group%2Fsubgroup%2Fproject apiTopicPattern = apiPattern + `v4/topics` snippetUploadPattern = `^/uploads/personal_snippet` userUploadPattern = `^/uploads/user` @@ -269,37 +269,37 @@ func configureRoutes(u *upstream) { // https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56731. // Maven Artifact Repository - u.route("PUT", apiProjectPattern+`packages/maven/`, requestBodyUploader), + u.route("PUT", apiProjectPattern+`/packages/maven/`, requestBodyUploader), // Conan Artifact Repository u.route("PUT", apiPattern+`v4/packages/conan/`, requestBodyUploader), - u.route("PUT", apiProjectPattern+`packages/conan/`, requestBodyUploader), + u.route("PUT", apiProjectPattern+`/packages/conan/`, requestBodyUploader), // Generic Packages Repository - u.route("PUT", apiProjectPattern+`packages/generic/`, requestBodyUploader), + u.route("PUT", apiProjectPattern+`/packages/generic/`, requestBodyUploader), // NuGet Artifact Repository - u.route("PUT", apiProjectPattern+`packages/nuget/`, mimeMultipartUploader), + u.route("PUT", apiProjectPattern+`/packages/nuget/`, mimeMultipartUploader), // PyPI Artifact Repository - u.route("POST", apiProjectPattern+`packages/pypi`, mimeMultipartUploader), + u.route("POST", apiProjectPattern+`/packages/pypi`, mimeMultipartUploader), // Debian Artifact Repository - u.route("PUT", apiProjectPattern+`packages/debian/`, requestBodyUploader), + u.route("PUT", apiProjectPattern+`/packages/debian/`, requestBodyUploader), // Gem Artifact Repository - u.route("POST", apiProjectPattern+`packages/rubygems/`, requestBodyUploader), + u.route("POST", apiProjectPattern+`/packages/rubygems/`, requestBodyUploader), // Terraform Module Package Repository - u.route("PUT", apiProjectPattern+`packages/terraform/modules/`, requestBodyUploader), + u.route("PUT", apiProjectPattern+`/packages/terraform/modules/`, requestBodyUploader), // Helm Artifact Repository - u.route("POST", apiProjectPattern+`packages/helm/api/[^/]+/charts\z`, mimeMultipartUploader), + u.route("POST", apiProjectPattern+`/packages/helm/api/[^/]+/charts\z`, mimeMultipartUploader), // We are porting API to disk acceleration // we need to declare each routes until we have fixed all the routes on the rails codebase. // Overall status can be seen at https://gitlab.com/groups/gitlab-org/-/epics/1802#current-status - u.route("POST", apiProjectPattern+`wikis/attachments\z`, tempfileMultipartProxy), + u.route("POST", apiProjectPattern+`/wikis/attachments\z`, tempfileMultipartProxy), u.route("POST", apiPattern+`graphql\z`, tempfileMultipartProxy), u.route("POST", apiTopicPattern, tempfileMultipartProxy), u.route("PUT", apiTopicPattern, tempfileMultipartProxy), @@ -312,16 +312,20 @@ func configureRoutes(u *upstream) { u.route("POST", importPattern+`gitlab_group`, mimeMultipartUploader), // Issuable Metric image upload - u.route("POST", apiProjectPattern+`issues/[0-9]+/metric_images\z`, mimeMultipartUploader), + u.route("POST", apiProjectPattern+`/issues/[0-9]+/metric_images\z`, mimeMultipartUploader), // Alert Metric image upload - u.route("POST", apiProjectPattern+`alert_management_alerts/[0-9]+/metric_images\z`, mimeMultipartUploader), + u.route("POST", apiProjectPattern+`/alert_management_alerts/[0-9]+/metric_images\z`, mimeMultipartUploader), // Requirements Import via UI upload acceleration u.route("POST", projectPattern+`requirements_management/requirements/import_csv`, mimeMultipartUploader), // Uploads via API - u.route("POST", apiProjectPattern+`uploads\z`, mimeMultipartUploader), + u.route("POST", apiProjectPattern+`/uploads\z`, mimeMultipartUploader), + + // Project Avatar + u.route("POST", apiPattern+`v4/projects\z`, tempfileMultipartProxy), + u.route("PUT", apiProjectPattern+`\z`, tempfileMultipartProxy), // Explicitly proxy API requests u.route("", apiPattern, proxy), diff --git a/workhorse/upload_test.go b/workhorse/upload_test.go index 9947059770fbf84925ac24c855d745ecf854eb5a..970af3ca5ce4611f444fa51e1a65395ba408d0c9 100644 --- a/workhorse/upload_test.go +++ b/workhorse/upload_test.go @@ -122,6 +122,10 @@ func TestAcceleratedUpload(t *testing.T) { {"POST", `/example`, false}, {"POST", `/uploads/personal_snippet`, true}, {"POST", `/uploads/user`, true}, + {"POST", `/api/v4/projects`, false}, + {"PUT", `/api/v4/projects/group%2Fproject`, false}, + {"PUT", `/api/v4/projects/group%2Fsubgroup%2Fproject`, false}, + {"PUT", `/api/v4/projects/39`, false}, {"POST", `/api/v4/projects/1/uploads`, true}, {"POST", `/api/v4/projects/group%2Fproject/uploads`, true}, {"POST", `/api/v4/projects/group%2Fsubgroup%2Fproject/uploads`, true},