Skip to content

Protected packages: Batch load matching protection rules

Gerardo Navarro requested to merge gitlab-community/gitlab:413641-gerardo-navarro-protected-packages-show-protected-packages-in-package-registry-list-refactor-include-batch-loader into master
  • Please check this box if this contribution uses AI-generated content (including content generated by GitLab Duo features) as outlined in the GitLab DCO & CLA

What does this MR do and why?

  • Relates to #437926 and #437928 (closed)
  • In a previous MR, we identified a N+1 issue when requesting the data to display the label "protected" in the package overview list; when calculating the field protectionRuleExists as part of a Graphql query, then multiple queries were executed, see !146206 (comment 1871347682) see #437928 (comment 1891842044)
  • By adding the batch loader as part of this MR, only a single database query is executed
  • @10io gave some tips on how to integrate this in GraphQL API

🛠 with ❤ at Siemens

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.

MR Checklist (@gerardo-navarro)

DB queries

WITH "package_names_and_types_cte" AS MATERIALIZED (
  SELECT *
  FROM unnest(ARRAY['@flightjs/flight-gerardo-protected','@flightjs/flight-gerardo'],
    ARRAY[2,2]) AS x(package_name, package_type)
)
SELECT package_names_and_types_cte."package_type",
       package_names_and_types_cte."package_name",
       EXISTS(
         SELECT 1 FROM "packages_protection_rules"
         WHERE "packages_protection_rules"."project_id" = 7
           AND "packages_protection_rules"."package_type" = package_names_and_types_cte."package_type"
           AND (package_names_and_types_cte."package_name" ILIKE REPLACE(REPLACE(REPLACE(package_name_pattern, '%', '\%'), '_', '\_'), '*', '%'))
       ) AS protected
FROM package_names_and_types_cte;
1st SQL query approach (deprecated)

The following query corresponds to this commit.

SELECT
  COUNT("packages_protection_rules"."id") AS "count_id",
  "package_name" AS "package_name" FROM "packages_protection_rules"
RIGHT JOIN unnest(ARRAY['@flightjs/flight-gerardo-protected','@flightjs/flight-gerardo']::varchar[]) AS unnested_package_names(package_name)
ON packages_protection_rules.project_id = 7
  AND unnested_package_names.package_name ILIKE REPLACE(REPLACE(REPLACE(package_name_pattern, '%', '\%'), '_', '\_'), '*', '%')
GROUP BY "package_name";

Screenshots or screen recordings

Before this MR, the backend was individually checking if a protection rule exists for each package (package_name) in the package overview list, see console output below.

  Packages::Protection::Rule Exists? (1.3ms)  SELECT 1 AS one FROM "packages_protection_rules" INNER JOIN "projects" ON "packages_protection_rules"."project_id" = "projects"."id" WHERE "projects"."id" = 7 AND "packages_protection_rules"."package_type" = 2 AND ('@flightjs/flight-gerardo-protected' ILIKE REPLACE(REPLACE(REPLACE(package_name_pattern,
                        '%', '\%'),
                '_', '\_'),
        '*', '%')
) LIMIT 1 /*application:web,correlation_id:01HZ79EQNQTHA47M81H8A4KVQW,endpoint_id:GraphqlController#execute,db_config_name:main,line:/app/graphql/types/packages/package_base_type.rb:51:in `protection_rule_exists'*/
  ↳ app/graphql/types/packages/package_base_type.rb:51:in `protection_rule_exists'
  Packages::Protection::Rule Exists? (0.5ms)  SELECT 1 AS one FROM "packages_protection_rules" INNER JOIN "projects" ON "packages_protection_rules"."project_id" = "projects"."id" WHERE "projects"."id" = 7 AND "packages_protection_rules"."package_type" = 2 AND ('@flightjs/flight-gerardo' ILIKE REPLACE(REPLACE(REPLACE(package_name_pattern,
                        '%', '\%'),
                '_', '\_'),
        '*', '%')
) LIMIT 1 /*application:web,correlation_id:01HZ79EQNQTHA47M81H8A4KVQW,endpoint_id:GraphqlController#execute,db_config_name:main,line:/app/graphql/types/packages/package_base_type.rb:51:in `protection_rule_exists'*/
  ↳ app/graphql/types/packages/package_base_type.rb:51:in `protection_rule_exists'

After this MR, the backend just executes a single datbase query that checks several packages (package_names) at the same time, see console output below.

  SQL (3.1ms)  WITH "package_names_and_types_cte" AS MATERIALIZED (SELECT * FROM unnest(ARRAY['@flightjs/flight-gerardo-protected','@flightjs/flight-gerardo'], ARRAY[2,2]) AS x(package_name, package_type)) SELECT package_names_and_types_cte."package_type", package_names_and_types_cte."package_name", EXISTS(SELECT 1 FROM "packages_protection_rules" WHERE "packages_protection_rules"."project_id" = 7 AND "packages_protection_rules"."package_type" = package_names_and_types_cte."package_type" AND (package_names_and_types_cte."package_name" ILIKE REPLACE(REPLACE(REPLACE(package_name_pattern,
                        '%', '\%'),
                '_', '\_'),
        '*', '%')
)) AS protected FROM package_names_and_types_cte /*application:web,correlation_id:01HZW20G6Y40C4PE1FR0KSMM6Z,endpoint_id:GraphqlController#execute,db_config_name:main,line:/app/models/packages/protection/rule.rb:77:in `for_push_exists_for_multiple_packages'*/
  ↳ app/models/packages/protection/rule.rb:77:in `for_push_exists_for_multiple_packages'

How to set up and validate locally

  1. Enable the feature flag :packages_protected_packages 
Feature.enable(:packages_protected_packages)
  1. Go to the package registry setting of a project: http://gdk.test:3000/flightjs/Flight/-/settings/packages_and_registries
  2. Create a new package protection rule with package name pattern @flightjs/npm_package_* (<= this will match seeded packages that are already in the package registry)
  3. Go to the package registry of the project: http://gdk.test:3000/flightjs/Flight/-/packages
  4. You should see the package entries with the label "protected"
  5. Go to the logs of the rails-web server and look for anaylize the sql queries => you should see that the N+1 issue (multiple queries) has been resolved and replaced by only a single query.

💿 Database analysis

Edited by David Fernandez

Merge request reports