fix: Corrects the order of caching + authorization middlewares

Fix order of caching + authorization middlewares

Author: nickfloyd

Gemfile

@@ -19,6 +19,7 @@ group :test do
   install_if -> { RUBY_VERSION >= '2.8' } do
     gem 'rexml', '>= 3.2.4'
   end
+  gem 'faraday-http-cache', '~> 2.5', '>= 2.5.1'
   gem 'json', '>= 2.3.0'
   gem 'jwt', '~> 2.2', '>= 2.2.1'
   gem 'mime-types', '~> 3.3', '>= 3.3.1'
@@ -29,6 +30,7 @@ group :test do
   gem 'rspec', '~> 3.9'
   gem 'simplecov', require: false
   gem 'test-queue'
+  gem 'timecop', '~> 0.9.8'
   gem 'vcr', '~> 6.1'
   gem 'webmock', '~> 3.8', '>= 3.8.2'
 end

lib/octokit/client.rb

@@ -261,13 +261,15 @@ def client_without_redirects(options = {})
       conn_opts[:proxy] = @proxy if @proxy
       conn_opts[:ssl] = { verify_mode: @ssl_verify_mode } if @ssl_verify_mode
       conn = Faraday.new(conn_opts) do |http|
+        http_cache_middleware = http.builder.handlers.delete(Faraday::HttpCache) if Faraday.const_defined?(:HttpCache)
         if basic_authenticated?
           http.request(*FARADAY_BASIC_AUTH_KEYS, @login, @password)
         elsif token_authenticated?
           http.request :authorization, 'token', @access_token
         elsif bearer_authenticated?
           http.request :authorization, 'Bearer', @bearer_token
         end
+        http.builder.handlers.push(http_cache_middleware) unless http_cache_middleware.nil?
         http.headers['accept'] = options[:accept] if options.key?(:accept)
       end
       conn.builder.delete(Octokit::Middleware::FollowRedirects)

lib/octokit/connection.rb

@@ -106,6 +106,7 @@ def agent
         http.headers[:accept] = default_media_type
         http.headers[:content_type] = 'application/json'
         http.headers[:user_agent] = user_agent
+        http_cache_middleware = http.builder.handlers.delete(Faraday::HttpCache) if Faraday.const_defined?(:HttpCache)
         if basic_authenticated?
           http.request(*FARADAY_BASIC_AUTH_KEYS, @login, @password)
         elsif token_authenticated?
@@ -115,6 +116,7 @@ def agent
         elsif application_authenticated?
           http.request(*FARADAY_BASIC_AUTH_KEYS, @client_id, @client_secret)
         end
+        http.builder.handlers.push(http_cache_middleware) unless http_cache_middleware.nil?
       end
     end
 

spec/cassettes/Octokit_Client_Repositories/_repositories/performs_requests_per_user_when_using_caching_middleware.json

@@ -1,4 +1,6 @@
 # frozen_string_literal: true
+require "faraday-http-cache"
+require "timecop"
 
 describe Octokit::Client::Repositories do
   before do
@@ -260,6 +262,22 @@
       expect(repositories).to be_kind_of Array
       assert_requested :get, github_url('/user/repos')
     end
+    it 'performs requests per user when using caching middleware' do
+      Timecop.freeze(VCR.current_cassette.originally_recorded_at || Time.now) do
+        client_with_caching = oauth_client_with_http_cache_middleware(access_token: test_github_token)
+        client_with_caching.repositories
+        client_with_caching.repositories
+
+        client_with_caching_two = oauth_client_with_http_cache_middleware(access_token: test_github_token_two)
+        client_with_caching_two.repositories
+        client_with_caching_two.repositories
+
+        client_with_caching.repositories
+        client_with_caching_two.repositories
+
+        assert_requested :get, github_url('/user/repos'), times: 2
+      end
+    end
   end # .repositories
 
   describe '.all_repositories', :vcr do

spec/octokit/client/repositories_spec.rb

@@ -52,6 +52,9 @@
   c.filter_sensitive_data('<<ACCESS_TOKEN>>') do
     test_github_token
   end
+  c.filter_sensitive_data('<<ACCESS_TOKEN>>') do
+    test_github_token_two
+  end
   c.filter_sensitive_data('<GITHUB_COLLABORATOR_TOKEN>') do
     test_github_collaborator_token
   end
@@ -176,6 +179,10 @@ def test_github_token
   ENV.fetch 'OCTOKIT_TEST_GITHUB_TOKEN', 'x' * 40
 end
 
+def test_github_token_two
+  ENV.fetch 'OCTOKIT_TEST_GITHUB_TOKEN_TWO', 'y' * 40
+end
+
 def test_github_collaborator_token
   ENV.fetch 'OCTOKIT_TEST_GITHUB_COLLABORATOR_TOKEN', 'x' * 40
 end
@@ -299,8 +306,8 @@ def basic_auth_client(login: test_github_login, password: test_github_password)
   Octokit::Client.new(login: login, password: password)
 end
 
-def oauth_client
-  Octokit::Client.new(access_token: test_github_token)
+def oauth_client(access_token: test_github_token)
+  Octokit::Client.new(access_token: access_token)
 end
 
 def enterprise_admin_client
@@ -321,6 +328,24 @@ def enterprise_admin_client
   client
 end
 
+def http_cache_middleware_store
+  @http_cache_middleware_store ||= Faraday::HttpCache::MemoryStore.new
+end
+
+def oauth_client_with_http_cache_middleware(access_token: test_github_token)
+  stack = Faraday::RackBuilder.new do |builder|
+    builder.use Faraday::HttpCache, serializer: Marshal, store: http_cache_middleware_store, shared_cache: false
+    builder.adapter Faraday.default_adapter
+  end
+
+  client = oauth_client(access_token: access_token)
+
+  client.configure do |c|
+    c.middleware = stack
+  end
+  client
+end
+
 def enterprise_management_console_client
   stack = Faraday::RackBuilder.new do |builder|
     builder.request :multipart