Skip to content

Improve code readability in nuget search service

📻 Context

In !48356 (comment 475043648), we noticed that the nuget search service was using an OpenStruct to return an object with two functions: results and total_count. During that time, we updated the code to use simply Struct since the accessible fields are known in advance.

This led me to have Struct.new as a constant to avoid creating a new Struct each time the service is executed.

Using CONSTANT.new to create the returned object feels a bit strange and could be confusing to new readers.

We have two solutions here:

  1. Use a better CONSTANT name.
  2. Have a dedicated class.

I decided to use (2.) but I did not want to create a new file for this super tiny class because in short, it's a class with two accessors and that's it.

I ended up using an inner class within the service class. This is not used very often but it provides a good middleground:

  • It's still a dedicated class
  • It's not a new file within models or lib.

See #296086 (closed)

🤔 What does this MR do?

  • Replace the nuget search service result object with an inner class.
    • I used the include ActiveModel::Model so that I can initialize the object with a hash of attributes, similar to what you would do with a fully fledged ActiveRecord class.
  • No specs were added as the existing specs already check that the returned object has the proper functions.

No changelog or documentation was added/updated as this is a tiny refactoring that will not change the expected behavior.

📷 Screenshots (strongly suggested)

n / a

🍴 Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

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
Edited by David Fernandez

Merge request reports