Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix order of caching + authorization middlewares #1661

Open
wants to merge 6 commits into
base: v8
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions Gemfile
Expand Up @@ -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'
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/octokit/client.rb
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions lib/octokit/connection.rb
Expand Up @@ -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?
Expand All @@ -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

Expand Down

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions spec/octokit/client/repositories_spec.rb
@@ -1,4 +1,6 @@
# frozen_string_literal: true
require "faraday-http-cache"
require "timecop"

describe Octokit::Client::Repositories do
before do
Expand Down Expand Up @@ -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
Expand Down
29 changes: 27 additions & 2 deletions spec/spec_helper.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down