Merge branch '331297_fix_pagination_headers_issue' into 'master'

Fix missing pagination headers for Branches endpoint

See merge request gitlab-org/gitlab!64723

lib/api/helpers/caching.rb

@@ -14,6 +14,12 @@ module API
         race_condition_ttl: 5.seconds
       }.freeze
 
+      # @return Integer
+      VERSION = 1
+
+      # @return [Array]
+      PAGINATION_HEADERS = %w[X-Per-Page X-Page X-Next-Page X-Prev-Page Link X-Total X-Total-Pages].freeze
+
       # This is functionally equivalent to the standard `#present` used in
       # Grape endpoints, but the JSON for the object, or for each object of
       # a collection, will be cached.
@@ -72,15 +78,22 @@ module API
       # @param key [Object] any object that can be converted into a cache key
       # @param expires_in [ActiveSupport::Duration, Integer] an expiry time for the cache entry
       # @return [Gitlab::Json::PrecompiledJson]
-      def cache_action(key, **cache_opts)
-        json = cache.fetch(key, **apply_default_cache_options(cache_opts)) do
+      def cache_action(key, **custom_cache_opts)
+        cache_opts = apply_default_cache_options(custom_cache_opts)
+
+        json, cached_headers = cache.fetch([key, VERSION], **cache_opts) do
           response = yield
 
-          if response.is_a?(Gitlab::Json::PrecompiledJson)
-            response.to_s
-          else
-            Gitlab::Json.dump(response.as_json)
-          end
+          cached_body = response.is_a?(Gitlab::Json::PrecompiledJson) ? response.to_s : Gitlab::Json.dump(response.as_json)
+          cached_headers = header.slice(*PAGINATION_HEADERS)
+
+          [cached_body, cached_headers]
+        end
+
+        cached_headers.each do |key, value|
+          next if header.key?(key)
+
+          header key, value
         end
 
         body Gitlab::Json::PrecompiledJson.new(json)

spec/lib/api/helpers/caching_spec.rb

@@ -3,7 +3,7 @@
 require "spec_helper"
 
 RSpec.describe API::Helpers::Caching, :use_clean_rails_redis_caching do
-  subject(:instance) { Class.new.include(described_class).new }
+  subject(:instance) { Class.new.include(described_class, Grape::DSL::Headers).new }
 
   let_it_be(:project) { create(:project) }
   let_it_be(:user) { create(:user) }
@@ -81,7 +81,7 @@ RSpec.describe API::Helpers::Caching, :use_clean_rails_redis_caching do
       expected_kwargs = described_class::DEFAULT_CACHE_OPTIONS.merge(kwargs)
 
       expect(expensive_thing).to receive(:do_very_expensive_action).once
-      expect(instance.cache).to receive(:fetch).with(cache_key, **expected_kwargs).exactly(5).times.and_call_original
+      expect(instance.cache).to receive(:fetch).with([cache_key, 1], **expected_kwargs).exactly(5).times.and_call_original
 
       5.times { perform }
     end
@@ -95,6 +95,32 @@ RSpec.describe API::Helpers::Caching, :use_clean_rails_redis_caching do
 
       expect(nested_call.to_s).to eq(subject.to_s)
     end
+
+    context 'Cache for pagination headers' do
+      described_class::PAGINATION_HEADERS.each do |pagination_header|
+        context pagination_header do
+          before do
+            instance.header(pagination_header, 100)
+          end
+
+          it 'stores and recovers pagination headers from cache' do
+            expect { perform }.not_to change { instance.header[pagination_header] }
+
+            instance.header.delete(pagination_header)
+
+            expect { perform }.to change { instance.header[pagination_header] }.from(nil).to(100)
+          end
+
+          it 'prefers headers from request than from cache' do
+            expect { perform }.not_to change { instance.header[pagination_header] }
+
+            instance.header(pagination_header, 50)
+
+            expect { perform }.not_to change { instance.header[pagination_header] }.from(50)
+          end
+        end
+      end
+    end
   end
 
   describe "#cache_action_if" do

spec/requests/api/branches_spec.rb

@@ -75,6 +75,14 @@ RSpec.describe API::Branches do
 
             check_merge_status(json_response)
           end
+
+          it 'recovers pagination headers from cache between consecutive requests' do
+            2.times do
+              get api(route, current_user), params: base_params
+
+              expect(response.headers).to include('X-Page')
+            end
+          end
         end
 
         context 'with gitaly pagination params' do