Fix gradle uploads and snapshot versions

What does this MR do?

Given the amount of comments and discussions on the related issue: #207920 (closed), we would like to present this MR's context precisely here.

🌲 Context

To upload maven package files to the GitLab Package Registry, two tools are supported:

• mvn
• gradle

We're going to take a look at what happens when a snapshot version (eg. 1.0-SNAPSHOT) is uploaded.

Here is the upload order for mvn:

1. my-company/my-app/1.0-SNAPSHOT/my-app.jar
2. my-company/my-app/1.0-SNAPSHOT/my-app.pom
3. my-company/my-app/1.0-SNAPSHOT/maven-metadata.xml
4. my-company/my-app/maven-metadata.xml

And here is the upload order for gradle:

1. my-company/my-app/1.0-SNAPSHOT/maven-metadata.xml
2. my-company/my-app/1.0-SNAPSHOT/my-app.jar
3. my-company/my-app/1.0-SNAPSHOT/my-app.pom
4. my-company/my-app/maven-metadata.xml

Take note that the maven-metadata.xml file with a version is the first upload for gradle.

🔥 Issue

When the first upload hits the GitLab Package registry, it goes through https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/packages/maven/find_or_create_package_service.rb#L12-34 to create the Packages::Package model.

The problem is given that the first upload is not same between mvn and gradle, the package model is not created with the same attributes.

Taking the above example, here is the package created by mvn:

• package name: my-company/my-app
• package version: 1.0-SNAPSHOT
• maven path: my-company/my-app/1.0-SNAPSHOT

Here is the package created by gradle:

• package name: my-company/my-app/1.0-SNAPSHOT
• package version: nil
• maven path: my-company/my-app/1.0-SNAPSHOT

The issue lies in this line: https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/packages/maven/find_or_create_package_service.rb#L21. This line exists to properly deal with the last upload, the maven-metadata.xml file without a version. This last upload has a maven path set to my-company/my-app and will thus create a second package without a version.

When mvn sends the first upload, the line is executed and versionless package is created but the maven path is my-company/my-app/1.0-SNAPSHOT. This path will match upload (2), (3) and these files will get appended to the versionless package model = 💥

In addition, versionless packages are not show in the UI and are not returned by the API

🚒 How this MR fixes the issue

• Note that https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/packages/maven/find_or_create_package_service.rb#L21 was designed to work only with the versionless maven-metadata.xml.
• We will update the condition (https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/packages/maven/find_or_create_package_service.rb#L12) to avoid entering the if block if there is a -SNAPSHOT version in the maven path.
• This way, the proper version 1.0-SNAPSHOT gets extracted and the proper package with the correct version is created.

🛠 Design choices

• We added a longer note to explain why this condition should only match versionless maven-metadata.xml uploads.
• We refactored the corresponding rspec so we can use a table based test.
  • This allows us to test several different conditions (maven path with version or not, snapshot version or not) in a concise way

Screenshots

Before this MR

UI at the project level

UI at the group level

API

$ curl --header "PRIVATE-TOKEN: XXXXXXX" "http://gitlab.local:8000/api/v4/projects/1/packages"
[]

-> 😿

With this MR

UI at the project level

UI at the group level

API

$ curl --header "PRIVATE-TOKEN: XXXXXXX" "http://gitlab.local:8000/api/v4/projects/1/packages"
[{"id":42,"name":"gl/pru/ghost","version":"2.2-SNAPSHOT","package_type":"maven","_links":{"web_path":"/gitlab-org/gitlab-test/-/packages/42","delete_api_path":"http://gitlab.local:8000/api/v4/projects/1/packages/42"},"created_at":"2020-08-07T12:56:09.238Z","tags":[],"versions":[]}]

Package details page

-> 🎉 The version is properly set on the package model 🎉

Does this MR meet the acceptance criteria?

Conformity

• Changelog entry
• Documentation (if required)
• Code review guidelines
• Merge request performance guidelines
• Style guides
• Database guides
• Separation of EE specific content

Availability and Testing

• Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
• Tested in all supported browsers
• Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

• Label as security and @ mention @gitlab-com/gl-security/appsec
• The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
• Security reports checked/validated by a reviewer from the AppSec team

🙏 Thanks

Thanks @sahbabou for her initial investigation and fix implementation 🚀