Commit 22ab00f1 authored by Steve Abrams's avatar Steve Abrams Committed by Heinrich Lee Yu

Fix the Maven duplicate setting logic

When maven_duplicates_allowed is set to false
Maven package files of different formats are still
allowed to be uploaded to an existing package.
parent 2e0423a9
...@@ -11,7 +11,12 @@ module Packages ...@@ -11,7 +11,12 @@ module Packages
.execute .execute
unless Namespace::PackageSetting.duplicates_allowed?(package) unless Namespace::PackageSetting.duplicates_allowed?(package)
return ServiceResponse.error(message: 'Duplicate package is not allowed') files = package&.package_files || []
current_maven_files = files.map { |file| extname(file.file_name) }
if current_maven_files.compact.include?(extname(params[:file_name]))
return ServiceResponse.error(message: 'Duplicate package is not allowed')
end
end end
unless package unless package
...@@ -54,6 +59,14 @@ module Packages ...@@ -54,6 +59,14 @@ module Packages
ServiceResponse.success(payload: { package: package }) ServiceResponse.success(payload: { package: package })
end end
private
def extname(filename)
return if filename.blank?
File.extname(filename)
end
end end
end end
end end
---
title: Fix behavior of maven_duplicates_allowed setting so new Maven packages can
be uploaded
merge_request: 51524
author:
type: fixed
...@@ -679,6 +679,15 @@ RSpec.describe API::MavenPackages do ...@@ -679,6 +679,15 @@ RSpec.describe API::MavenPackages do
package_settings.update!(maven_duplicates_allowed: false) package_settings.update!(maven_duplicates_allowed: false)
end end
shared_examples 'storing the package file' do
it 'stores the file', :aggregate_failures do
expect { upload_file_with_token(params: params) }.to change { package.package_files.count }.by(1)
expect(response).to have_gitlab_http_status(:ok)
expect(jar_file.file_name).to eq(file_upload.original_filename)
end
end
it 'rejects the request', :aggregate_failures do it 'rejects the request', :aggregate_failures do
expect { upload_file_with_token(params: params) }.not_to change { package.package_files.count } expect { upload_file_with_token(params: params) }.not_to change { package.package_files.count }
...@@ -686,17 +695,23 @@ RSpec.describe API::MavenPackages do ...@@ -686,17 +695,23 @@ RSpec.describe API::MavenPackages do
expect(json_response['message']).to include('Duplicate package is not allowed') expect(json_response['message']).to include('Duplicate package is not allowed')
end end
context 'when the package name matches the exception regex' do context 'when uploading different non-duplicate files to the same package' do
let!(:package) { create(:maven_package, project: project, name: project.full_path) }
before do before do
package_settings.update!(maven_duplicate_exception_regex: '.*') package_file = package.package_files.find_by(file_name: 'my-app-1.0-20180724.124855-1.jar')
package_file.destroy!
end end
it 'stores the package file', :aggregate_failures do it_behaves_like 'storing the package file'
expect { upload_file_with_token(params: params) }.to change { package.package_files.count }.by(1) end
expect(response).to have_gitlab_http_status(:ok) context 'when the package name matches the exception regex' do
expect(jar_file.file_name).to eq(file_upload.original_filename) before do
package_settings.update!(maven_duplicate_exception_regex: '.*')
end end
it_behaves_like 'storing the package file'
end end
end end
......
...@@ -111,6 +111,15 @@ RSpec.describe Packages::Maven::FindOrCreatePackageService do ...@@ -111,6 +111,15 @@ RSpec.describe Packages::Maven::FindOrCreatePackageService do
expect(subject.errors).to include('Duplicate package is not allowed') expect(subject.errors).to include('Duplicate package is not allowed')
end end
context 'when uploading different non-duplicate files to the same package' do
before do
package_file = existing_package.package_files.find_by(file_name: 'my-app-1.0-20180724.124855-1.jar')
package_file.destroy!
end
it_behaves_like 'reuse existing package'
end
context 'when the package name matches the exception regex' do context 'when the package name matches the exception regex' do
before do before do
package_settings.update!(maven_duplicate_exception_regex: '.*') package_settings.update!(maven_duplicate_exception_regex: '.*')
......
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