Merge pull request #6 from cadenza-tech/refactoring

Refactoring

Author: w-masahiro-ct

lib/omniauth/strategies/qiita_v2.rb

@@ -61,31 +61,8 @@ class QiitaV2 < OmniAuth::Strategies::OAuth2
  hash
  end
 
- def raw_info # rubocop:disable Metrics/AbcSize
- @raw_info ||= begin
- access_token.get(USER_INFO_URL).parsed || {}
- rescue ::OAuth2::Error => e
- case e.response.status
- when 401
- log :error, '401 Unauthorized - Invalid access token'
- raise ::OmniAuth::NoSessionError.new('Invalid access token')
- when 403
- log :error, '403 Forbidden - Insufficient permissions'
- raise ::OmniAuth::NoSessionError.new('Insufficient permissions')
- when 404
- log :error, '404 Not Found - User not found'
- raise ::OmniAuth::NoSessionError.new('User not found')
- else
- log :error, "API Error: #{e.response.status} - #{e.message}"
- raise e
- end
- rescue ::Errno::ETIMEDOUT
- log :error, 'Connection timed out'
- raise ::OmniAuth::NoSessionError.new('Connection timed out')
- rescue ::SocketError => e
- log :error, "Network error: #{e.message}"
- raise ::OmniAuth::NoSessionError.new('Network error')
- end
+ def raw_info
+ @raw_info ||= access_token.get(USER_INFO_URL).parsed
  end
 
  def callback_url
@@ -105,7 +82,7 @@ def authorize_params
  protected
 
  def build_access_token
- client.get_token(base_params.merge(token_params_from_options).merge(deep_symbolize(options.auth_token_params)))
+ client.get_token(base_params.merge(token_params.to_hash(symbolize_keys: true)).merge(deep_symbolize(options.auth_token_params)))
  end
 
  private
@@ -128,17 +105,6 @@ def base_params
  code: request.params['code']
  }
  end
-
- def token_params_from_options
- return {} unless options.token_params
-
- if options.token_params.is_a?(Hash)
- params = options.token_params
- else
- params = options.token_params.to_hash
- end
- params.transform_keys(&:to_sym)
- end
  end
  end
 end

spec/omniauth/strategies/qiita_v2_spec.rb

@@ -211,108 +211,18 @@
  let(:access_token) { instance_double(OAuth2::AccessToken) }
  let(:response) { instance_double(OAuth2::Response, parsed: { 'id' => 'test123' }) }
 
- before { allow(strategy).to receive(:access_token).and_return(access_token) }
-
- context 'when API request is successful' do
- before { allow(access_token).to receive(:get).with('/api/v2/authenticated_user').and_return(response) }
-
- it 'fetches user info from API' do
- expect(strategy.raw_info).to eq({ 'id' => 'test123' })
- end
-
- it 'memoizes the result' do
- 2.times { strategy.raw_info }
- expect(access_token).to have_received(:get).once
- end
- end
-
- context 'when API returns 401 Unauthorized' do
- let(:error_response) { instance_double(OAuth2::Response, status: 401) }
- let(:oauth_error) { OAuth2::Error.new(error_response) }
-
- before do
- allow(access_token).to receive(:get).with('/api/v2/authenticated_user').and_raise(oauth_error)
- allow(strategy).to receive(:log)
- end
-
- it 'raises OmniAuth::NoSessionError' do
- expect { strategy.raw_info }.to raise_error(OmniAuth::NoSessionError, 'Invalid access token')
- end
-
- it 'logs the error' do
- expect { strategy.raw_info }.to raise_error(OmniAuth::NoSessionError)
- expect(strategy).to have_received(:log).with(:error, '401 Unauthorized - Invalid access token')
- end
- end
-
- context 'when API returns 403 Forbidden' do
- let(:error_response) { instance_double(OAuth2::Response, status: 403) }
- let(:oauth_error) { OAuth2::Error.new(error_response) }
-
- before do
- allow(access_token).to receive(:get).and_raise(oauth_error)
- allow(strategy).to receive(:log)
- end
-
- it 'raises OmniAuth::NoSessionError with appropriate message' do
- expect { strategy.raw_info }.to raise_error(OmniAuth::NoSessionError, 'Insufficient permissions')
- end
- end
-
- context 'when API returns 404 Not Found' do
- let(:error_response) { instance_double(OAuth2::Response, status: 404) }
- let(:oauth_error) { OAuth2::Error.new(error_response) }
-
- before do
- allow(access_token).to receive(:get).and_raise(oauth_error)
- allow(strategy).to receive(:log)
- end
-
- it 'raises OmniAuth::NoSessionError with appropriate message' do
- expect { strategy.raw_info }.to raise_error(OmniAuth::NoSessionError, 'User not found')
- end
- end
-
- context 'when connection times out' do
- before do
- allow(access_token).to receive(:get).and_raise(Errno::ETIMEDOUT)
- allow(strategy).to receive(:log)
- end
-
- it 'raises OmniAuth::NoSessionError' do
- expect { strategy.raw_info }.to raise_error(OmniAuth::NoSessionError, 'Connection timed out')
- end
+ before do
+ allow(strategy).to receive(:access_token).and_return(access_token)
+ allow(access_token).to receive(:get).with('/api/v2/authenticated_user').and_return(response)
  end
 
- context 'when network error occurs' do
- before do
- allow(access_token).to receive(:get).and_raise(SocketError.new('getaddrinfo: nodename nor servname provided'))
- allow(strategy).to receive(:log)
- end
-
- it 'raises OmniAuth::NoSessionError' do
- expect { strategy.raw_info }.to raise_error(OmniAuth::NoSessionError, 'Network error')
- end
+ it 'fetches user info from API' do
+ expect(strategy.raw_info).to eq({ 'id' => 'test123' })
  end
 
- context 'when API returns other errors' do
- let(:error_response) { instance_double(OAuth2::Response, status: 500) }
- let(:oauth_error) { OAuth2::Error.new(error_response) }
-
- before do
- allow(oauth_error).to receive(:message).and_return('Internal Server Error')
- allow(access_token).to receive(:get).with('/api/v2/authenticated_user').and_raise(oauth_error)
- allow(strategy).to receive(:log)
- end
-
- it 're-raises the original error' do
- expect { strategy.raw_info }.to raise_error(OAuth2::Error)
- end
-
- it 'logs the error with status and message' do
- expect { strategy.raw_info }.to raise_error(OAuth2::Error)
- expect(strategy).to have_received(:log).with(:error, 'API Error: 500 - Internal Server Error')
- end
+ it 'memoizes the result' do
+ 2.times { strategy.raw_info }
+ expect(access_token).to have_received(:get).once
  end
  end
 
@@ -434,6 +344,7 @@
  client_id: 'client_id',
  client_secret: 'secret',
  token_params: {},
+ token_options: {},
  auth_token_params: {}
  )
  )