Split the releases list cache by include_html_description

José M. Requena Plensrequested to merge
gitlab-community/gitlab-org/gitlab:jmrp-releases-cache-html-description into master

What does this MR do and why?

GET /projects/:id/releases caches each rendered release for five minutes, under a key made of the release's cache key and the user. The include_html_description parameter decides whether a rendered release carries description_html, but the key does not include it, so whichever request renders a release first decides what every later request in those five minutes gets:

  • a request with include_html_description=true that follows one without it gets no description_html;
  • a request without the parameter that follows one with it gets description_html anyway.

This MR adds the parameter to the cache key, so each variant is cached on its own. I read the parameter once as a boolean (== true) and pass the same value to the cache key and to the entity, so an omitted parameter and false, which render the same release, share one cache entry. lib/api/merge_request_diffs.rb already adds a request parameter, unidiff, to its cache context.

The cache came first, in !60194 (merged) (13.12), and !62802 (merged) (14.0) added include_html_description to the list and passed it to the entity without changing the cache key. Only the list is cached: GET /projects/:id/releases/:tag_name and the group releases list present the entity directly, so they are not affected.

I picked this up because gitlab-mcp-server, an MCP server for GitLab that I maintain, passes include_html_description through on its release list action, so its answers depend on which variant the cache happens to hold.

Closes #588980 (closed)

References

Screenshots or screen recordings

Two requests to the list by the same user within five minutes, on a project with two releases:

Requests, in order description_html in the second response on master On this branch
without the parameter, then with include_html_description=true absent from both releases present on both releases
with include_html_description=true, then without it present on both releases absent from both releases

These are the results of the two new request specs, run against master and against this branch.

Evidence, file and line on master at a20857c3
  • lib/api/releases.rb:107-122: the list calls present_cached with cache_context: ->(_) { "user:{#{current_user&.id}}" } (line 118), expires_in: 5.minutes and include_html_description: declared_params[:include_html_description] (line 121).
  • lib/api/entities/release.rb:8: description_html is exposed only if: ->(_, options) { options[:include_html_description] }.
  • lib/gitlab/cache/helpers.rb:45-48: the key of each cached object is the presenter's name, the object's cache_key and what the context returns, joined with :. The presenter arguments, include_html_description among them, are not part of it.
  • spec/requests/api/releases_spec.rb:21: the list's examples run with :use_clean_rails_redis_caching, so the cache is real in these specs and the new examples reproduce the defect on master:
    • when the releases were cached by a request without include_html_description includes description_html field fails with actual collection contained: [nil, nil].
    • when the releases were cached by a request with include_html_description does not include description_html field fails with expected collection contained: [nil, nil], because both releases come back with their rendered description_html.
    • caches releases with and without description_html separately fails with expected: 4 times and received: 2 times, since the second variant reads the first one's entries instead of rendering its own.
  • The same example also fails on this branch if the key takes the raw parameter instead of == true (received: 5 times), because an omitted parameter and false then get keys of their own.

How to set up and validate locally

  1. In a project with at least one release, request the list without the parameter: curl --header "PRIVATE-TOKEN: <token>" "http://gdk.test:3000/api/v4/projects/<id>/releases".
  2. Within five minutes, request it again with the parameter: curl --header "PRIVATE-TOKEN: <token>" "http://gdk.test:3000/api/v4/projects/<id>/releases?include_html_description=true".
  3. On master the releases in the second response have no description_html. On this branch they have it.

bin/rspec spec/requests/api/releases_spec.rb passes on this branch, including the three new examples and the existing examples that count how often a release is rendered. The three new examples fail on master. RuboCop and both commit message linters report nothing.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports