Fix a Gradle bug with snapshot versions

Where the first upload (`maven-metadata.xml` file with verison) would
not create properly the package with the version set.
Co-Authored-By: Sara Ahbabou <sahbabou@gitlab.com>

app/services/packages/maven/find_or_create_package_service.rb

@@ -3,13 +3,14 @@ module Packages
   module Maven
     class FindOrCreatePackageService < BaseService
       MAVEN_METADATA_FILE = 'maven-metadata.xml'.freeze
+      SNAPSHOT_TERM = '-SNAPSHOT'.freeze
 
       def execute
-        package = ::Packages::Maven::PackageFinder
-          .new(params[:path], current_user, project: project).execute
+        package =
+          ::Packages::Maven::PackageFinder.new(params[:path], current_user, project: project)
+                                          .execute
 
         unless package
-          if params[:file_name] == MAVEN_METADATA_FILE
           # Maven uploads several files during `mvn deploy` in next order:
           #   - my-company/my-app/1.0-SNAPSHOT/my-app.jar
           #   - my-company/my-app/1.0-SNAPSHOT/my-app.pom
@@ -17,7 +18,18 @@ module Packages
           #   - my-company/my-app/maven-metadata.xml
           #
           # The last xml file does not have VERSION in URL because it contains
-            # information about all versions.
+          # information about all versions. When uploading such file, we create
+          # a package with a version set to `nil`. The xml file with a version
+          # is only created and uploaded for snapshot versions.
+          #
+          # Gradle has a different upload order:
+          #   - my-company/my-app/1.0-SNAPSHOT/maven-metadata.xml
+          #   - my-company/my-app/1.0-SNAPSHOT/my-app.jar
+          #   - my-company/my-app/1.0-SNAPSHOT/my-app.pom
+          #   - my-company/my-app/maven-metadata.xml
+          #
+          # The first upload has to create the proper package (the one with the version set).
+          if params[:file_name] == MAVEN_METADATA_FILE && !params[:path]&.ends_with?(SNAPSHOT_TERM)
             package_name, version = params[:path], nil
           else
             package_name, _, version = params[:path].rpartition('/')
@@ -30,8 +42,9 @@ module Packages
             build: params[:build]
           }
 
-          package = ::Packages::Maven::CreatePackageService
-            .new(project, current_user, package_params).execute
+          package =
+            ::Packages::Maven::CreatePackageService.new(project, current_user, package_params)
+                                                   .execute
         end
 
         package

changelogs/unreleased/38338-gradle-packages-group.yml

+---
+title: Fix a Gradle bug where a package without a version would be created and thus not displayed on the UI.
+merge_request: 38338
+author:
+type: fixed

spec/factories/packages.rb

@@ -10,7 +10,7 @@ FactoryBot.define do
       maven_metadatum
 
       after :build do |package|
-        package.maven_metadatum.path = "#{package.name}/#{package.version}"
+        package.maven_metadatum.path = package.version? ? "#{package.name}/#{package.version}" : package.name
       end
 
       after :create do |package|

spec/services/packages/maven/find_or_create_package_service_spec.rb

@@ -4,34 +4,77 @@ require 'spec_helper'
 RSpec.describe Packages::Maven::FindOrCreatePackageService do
   let_it_be(:project) { create(:project) }
   let_it_be(:user) { create(:user) }
-  let_it_be(:app_name) { 'my-app' }
-  let_it_be(:version) { '1.0-SNAPSHOT' }
-  let_it_be(:path) { "my/company/app/#{app_name}" }
-  let_it_be(:path_with_version) { "#{path}/#{version}" }
-  let_it_be(:params) do
-    {
-      path: path_with_version,
-      name: path,
-      version: version
-    }
-  end
+
+  let(:app_name) { 'my-app' }
+  let(:path) { "sandbox/test/app/#{app_name}" }
+  let(:version) { '1.0.0' }
+  let(:file_name) { 'test.jar' }
+  let(:param_path) { "#{path}/#{version}" }
 
   describe '#execute' do
-    subject { described_class.new(project, user, params).execute }
+    using RSpec::Parameterized::TableSyntax
+
+    subject { described_class.new(project, user, { path: param_path, file_name: file_name }).execute }
+
+    RSpec.shared_examples 'reuse existing package' do
+      it { expect { subject}.not_to change { Packages::Package.count } }
+
+      it { is_expected.to eq(existing_package) }
+    end
+
+    RSpec.shared_examples 'create package' do
+      it { expect { subject}.to change { Packages::Package.count }.by(1) }
+
+      it 'sets the proper name and version' do
+        pkg = subject
+
+        expect(pkg.name).to eq(path)
+        expect(pkg.version).to eq(version)
+      end
+    end
 
-    context 'without any existing package' do
-      it 'creates a package' do
-        expect { subject }.to change { Packages::Package.count }.by(1)
+    context 'path with version' do
+      # Note that "path with version" and "file type maven metadata xml" only exists for snapshot versions
+      # In other words, we will never have an metadata xml upload on a path with version for a non snapshot version
+      where(:package_exist, :file_type, :snapshot_version, :shared_example_name) do
+        true  | :jar       | false | 'reuse existing package'
+        false | :jar       | false | 'create package'
+        true  | :jar       | true  | 'reuse existing package'
+        false | :jar       | true  | 'create package'
+        true  | :maven_xml | true  | 'reuse existing package'
+        false | :maven_xml | true  | 'create package'
+      end
+
+      with_them do
+        let(:version) { snapshot_version ? '1.0-SNAPSHOT' : '1.0.0' }
+        let(:file_name) { file_type == :maven_xml ? 'maven-metadata.xml' : 'test.jar' }
+
+        let!(:existing_package) do
+          if package_exist
+            create(:maven_package, name: path, version: version, project: project)
+          end
+        end
+
+        it_behaves_like params[:shared_example_name]
       end
     end
 
-    context 'with an existing package' do
-      let_it_be(:existing_package) { create(:maven_package, name: path, version: version, project: project) }
+    context 'path without version' do
+      let(:param_path) { path }
+      let(:version) { nil }
 
-      it { is_expected.to eq existing_package }
-      it "doesn't create a new package" do
-        expect { subject }
-          .to not_change { Packages::Package.count }
+      context 'maven-metadata.xml file' do
+        let(:file_name) { 'maven-metadata.xml' }
+
+        context 'with existing package' do
+          let!(:existing_package) { create(:maven_package, name: path, version: version, project: project) }
+
+          it_behaves_like 'reuse existing package'
+        end
+
+        context 'without existing package' do
+          it_behaves_like 'create package'
+        end
       end
     end
   end