Skip to content

Improve readability of the cleanup tags service

🍰 Context

In !69459 (merged), we modified the cleanup tags service to add caching capabilities.

In a nutshell, that service receives a container image and a set of parameters. The parameters received are used in different filters that are applied to the list of tags. At the end of the #execute method, we will have the list of tags that needs to be destroyed. For that, they are simply sent to the delete tags service.

During the !69459 (merged), we noticed that the overall readability of the class was not that great. We opened #340278 (closed) as a follow up for that.

This MR tries to improve the situation.

🔬 What does this MR do and why?

  • Improve the readability of the cleanup tags service by:
    • Stopping inheriting from BaseService.
      • This service has the focus on a container repository. BaseService is better geared for services focusing on projects or groups.
    • Using an instance variable for the container repository (@container_repository)
      • This way, we don't need to pass it around all the internal methods. Each private method can access it directly.
    • Pushing the instance variable idea, so that we do the same for the list of tags we build.
      • Same argument here. This way, there is no need to pass around the tags list, each private method can see it directly.
      • Additional benefit here, we can use "in-place" methods (eg. #reverse! instead of #reverse) which avoids cloning the array of tags. That's a minor ~performance bump.
    • Using a internal structure for all the counts we need to track.
      • Use that structure right before returning the service response to pass the counts fields in the response.
    • Unifying the params keys access with functions
      • We had a mix between dedicated functions and direct reads. Now, we have dedicated functions for all the fields we need to read.
  • Update the clients of that service
  • Update the related specs

No feature changes are done here. The service is functionally the same. This is more about removing a ~"technical debt". As such, I don't think we need a changelog here.

🖼 Screenshots or screen recordings

No feature changes here

How to set up and validate locally

To see this service in action, we can re-use the same set up as in !69459 (merged)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by David Fernandez

Merge request reports