Merge branch 'cat-api-releases-caching' into 'master'

API caching for the Releases endpoint [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!60194

app/models/releases/evidence.rb

@@ -5,7 +5,7 @@ module Releases
     include ShaAttribute
     include Presentable
 
-    belongs_to :release, inverse_of: :evidences
+    belongs_to :release, inverse_of: :evidences, touch: true
 
     default_scope { order(created_at: :asc) } # rubocop:disable Cop/DefaultScope
 

app/models/releases/link.rb

@@ -4,7 +4,7 @@ module Releases
   class Link < ApplicationRecord
     self.table_name = 'release_links'
 
-    belongs_to :release
+    belongs_to :release, touch: true
 
     # See https://gitlab.com/gitlab-org/gitlab/-/issues/218753
     # Regex modified to prevent catastrophic backtracking

config/feature_flags/development/api_caching_releases.yml

+---
+name: api_caching_releases
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60194
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/329861
+milestone: '13.12'
+type: development
+group: group::release
+default_enabled: false

lib/api/helpers/caching.rb

@@ -119,8 +119,11 @@ module API
         objs.flatten!
         map = multi_key_map(objs, context: context)
 
-        cache.fetch_multi(*map.keys, **kwargs) do |key|
-          yield map[key]
+        # TODO: `contextual_cache_key` should be constructed based on the guideline https://docs.gitlab.com/ee/development/redis.html#multi-key-commands.
+        Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
+          cache.fetch_multi(*map.keys, **kwargs) do |key|
+            yield map[key]
+          end
         end
       end
 

lib/api/releases.rb

@@ -33,7 +33,21 @@ module API
       get ':id/releases' do
         releases = ::ReleasesFinder.new(user_project, current_user, declared_params.slice(:order_by, :sort)).execute
 
-        present paginate(releases), with: Entities::Release, current_user: current_user
+        if Feature.enabled?(:api_caching_releases, user_project, default_enabled: :yaml)
+          # We cache the serialized payload per user in order to avoid repeated renderings.
+          # Since the cached result could contain sensitive information,
+          # it will expire in a short interval.
+          present_cached paginate(releases),
+                         with: Entities::Release,
+                         # `current_user` could be absent if the releases are publicly accesible.
+                         # We should not use `cache_key` for the user because the version/updated_at
+                         # context is unnecessary here.
+                         cache_context: -> (_) { "user:{#{current_user&.id}}" },
+                         expires_in: 5.minutes,
+                         current_user: current_user
+        else
+          present paginate(releases), with: Entities::Release, current_user: current_user
+        end
       end
 
       desc 'Get a single project release' do

spec/requests/api/releases_spec.rb

@@ -18,7 +18,7 @@ RSpec.describe API::Releases do
     project.add_developer(developer)
   end
 
-  describe 'GET /projects/:id/releases' do
+  describe 'GET /projects/:id/releases', :use_clean_rails_redis_caching do
     context 'when there are two releases' do
       let!(:release_1) do
         create(:release,
@@ -147,6 +147,60 @@ RSpec.describe API::Releases do
       end.not_to exceed_all_query_limit(control_count)
     end
 
+    it 'serializes releases for the first time and read cached data from the second time' do
+      create_list(:release, 2, project: project)
+
+      expect(API::Entities::Release)
+        .to receive(:represent).with(instance_of(Release), any_args)
+        .twice
+
+      5.times { get api("/projects/#{project.id}/releases", maintainer) }
+    end
+
+    it 'increments the cache key when link is updated' do
+      releases = create_list(:release, 2, project: project)
+
+      expect(API::Entities::Release)
+        .to receive(:represent).with(instance_of(Release), any_args)
+        .exactly(4).times
+
+      2.times { get api("/projects/#{project.id}/releases", maintainer) }
+
+      releases.each { |release| create(:release_link, release: release) }
+
+      3.times { get api("/projects/#{project.id}/releases", maintainer) }
+    end
+
+    it 'increments the cache key when evidence is updated' do
+      releases = create_list(:release, 2, project: project)
+
+      expect(API::Entities::Release)
+        .to receive(:represent).with(instance_of(Release), any_args)
+        .exactly(4).times
+
+      2.times { get api("/projects/#{project.id}/releases", maintainer) }
+
+      releases.each { |release| create(:evidence, release: release) }
+
+      3.times { get api("/projects/#{project.id}/releases", maintainer) }
+    end
+
+    context 'when api_caching_releases feature flag is disabled' do
+      before do
+        stub_feature_flags(api_caching_releases: false)
+      end
+
+      it 'serializes releases everytime' do
+        create_list(:release, 2, project: project)
+
+        expect(API::Entities::Release)
+          .to receive(:represent).with(kind_of(ActiveRecord::Relation), any_args)
+          .exactly(5).times
+
+        5.times { get api("/projects/#{project.id}/releases", maintainer) }
+      end
+    end
+
     context 'when tag does not exist in git repository' do
       let!(:release) { create(:release, project: project, tag: 'v1.1.5') }
 
@@ -230,6 +284,20 @@ RSpec.describe API::Releases do
         end
       end
     end
+
+    context 'when releases are public and request user is absent' do
+      let(:project) { create(:project, :repository, :public) }
+
+      it 'returns the releases' do
+        create(:release, project: project, tag: 'v0.1')
+
+        get api("/projects/#{project.id}/releases")
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response.count).to eq(1)
+        expect(json_response.first['tag_name']).to eq('v0.1')
+      end
+    end
   end
 
   describe 'GET /projects/:id/releases/:tag_name' do