Merge branch '276882-maven-dupe-fix' into 'master'

Fix the Maven duplicate setting logic

See merge request gitlab-org/gitlab!51524

app/services/packages/maven/find_or_create_package_service.rb

@@ -11,8 +11,13 @@ module Packages
                                           .execute
 
         unless Namespace::PackageSetting.duplicates_allowed?(package)
+          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
 
         unless package
           # Maven uploads several files during `mvn deploy` in next order:
@@ -54,6 +59,14 @@ module Packages
 
         ServiceResponse.success(payload: { package: package })
       end
+
+      private
+
+      def extname(filename)
+        return if filename.blank?
+
+        File.extname(filename)
+      end
     end
   end
 end

changelogs/unreleased/276882-maven-dupe-fix.yml

+---
+title: Fix behavior of maven_duplicates_allowed setting so new Maven packages can
+  be uploaded
+merge_request: 51524
+author:
+type: fixed

spec/requests/api/maven_packages_spec.rb

@@ -679,6 +679,15 @@ RSpec.describe API::MavenPackages do
           package_settings.update!(maven_duplicates_allowed: false)
         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
           expect { upload_file_with_token(params: params) }.not_to change { package.package_files.count }
 
@@ -686,17 +695,23 @@ RSpec.describe API::MavenPackages do
           expect(json_response['message']).to include('Duplicate package is not allowed')
         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
-            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
 
-          it 'stores the package file', :aggregate_failures do
-            expect { upload_file_with_token(params: params) }.to change { package.package_files.count }.by(1)
+          it_behaves_like 'storing the package file'
+        end
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(jar_file.file_name).to eq(file_upload.original_filename)
+        context 'when the package name matches the exception regex' do
+          before do
+            package_settings.update!(maven_duplicate_exception_regex: '.*')
           end
+
+          it_behaves_like 'storing the package file'
         end
       end
 

spec/services/packages/maven/find_or_create_package_service_spec.rb

@@ -111,6 +111,15 @@ RSpec.describe Packages::Maven::FindOrCreatePackageService do
         expect(subject.errors).to include('Duplicate package is not allowed')
       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
         before do
           package_settings.update!(maven_duplicate_exception_regex: '.*')