Refactor NPM metadata endpoint
NOTE: Given the blast radius (NPM is the most-used package format) I thought of putting the code changes behind a feature flag. But because the code changes moving code into/out of modules, we'll need to do feature flag evaluation at the class level, which is against our backend feature flag guidelines. I ended up not gating the changes behind a feature flag. Suggestions on how to gate the changes behind a feature flag are much appreciated!
💣 Problem
The NPM metadata endpoint code for the project level has diverged from the code for the instance and group level. It is currently a Concern shared among the project, group, and instance endpoint classes. But because of the diverging behavior, there are a number of if endpoint_scope...
checks sprinkled throughout the endpoint code.
We could simplify the logic of the metadata endpoint if we define it separately on the project level and use a shared metadata endpoint for group and instance level, since it's still very similar.
🚑 Solution
- Copy the metadata endpoint definition from the concern into the project endpoint file,
lib/api/npm_project_packages.rb
- On the project endpoint:
- Remove the
if endpoint_scope
checks and delete the unused code branch
- Remove the
- Move the metadata endpoint definition from the concern into a new module, (
NpmNamespaceEndpoints
) and include this module inlib/api/npm_instance_packages.rb
andlib/api/npm_group_packages.rb
- Remove the
if endpoint_scope
checks and delete the unused code branch inNpmNamespaceEndpoints
- Remove the
The changes can be hard to review by looking at the diffs view. It might be easier to compare side by side the master
and MR versions of these files:
lib/api/concerns/packages/npm_endpoints.rb
-
lib/api/concerns/packages/npm_namespace_endpoints.rb
(new file introduced by MR) lib/api/npm_project_packages.rb
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots or screen recordings
No UI changes
How to set up and validate locally
Check for regressions in npm install
(uses the metadata endpoint) for all three levels:
NOTE: You'll need to install a package at the appropriate level first
Related to #422372 (closed)