Permalink
Browse files

Refactoring on CredentialHandler

  • Loading branch information...
1 parent 00e1584 commit f63e7cf8e880eca8a46b7cad5b9db31639d74c50 @dklimkin dklimkin committed Aug 2, 2012
@@ -1,4 +1,4 @@
-0.7.4:
+0.8.0:
0.7.3:
- Compatibility with Savon -> 1.0.0 (not backward-compatible).
@@ -136,7 +136,10 @@ def authorize(parameters = {}, &block)
# - auth handler
#
def get_auth_handler()
- @auth_handler ||= create_auth_handler()
+ if @auth_handler.nil?
+ @auth_handler = create_auth_handler()
+ @credential_handler.set_auth_handler(@auth_handler)
+ end
return @auth_handler
end
@@ -1,6 +1,6 @@
# Encoding: utf-8
#
-# Authors:: api.sgomes@gmail.com (Sérgio Gomes)
+# Authors:: api.dklimkin@gmail.com (Danial Klimkin)
#
# Copyright:: Copyright 2010, Google Inc. All Rights Reserved.
#
@@ -27,6 +27,7 @@ class CredentialHandler
# Initializes CredentialHandler.
def initialize(config)
@config = config
+ @auth_handler = nil
load_from_config(config)
end
@@ -39,61 +40,52 @@ def credentials(credentials_override = nil)
# Set the credentials hash to a new one. Calculate difference, and call the
# AuthHandler callback appropriately.
- # TODO(dklimkin): re-write this with inject.
def credentials=(new_credentials)
# Find new and changed properties.
- diff = new_credentials.select do |key, value|
- value != @credentials[key]
+ diff = new_credentials.inject([]) do |result, (key, value)|
+ result << [key, value] if value != @credentials[key]
+ result
end
# Find removed properties.
- removed = @credentials.select do |key, value|
- new_credentials[key] == nil
- end
- if removed
- removed = removed.map { |entry| [entry[0], nil] }
- diff += removed
+ diff = @credentials.inject(diff) do |result, (key, value)|
+ result << [key, nil] unless new_credentials.include?(key)
+ result
end
# Set each property.
- diff.each do |entry|
- set_credential(entry[0], entry[1])
- end
+ diff.each { |entry| set_credential(entry[0], entry[1]) }
+
+ return nil
end
# Set a single credential to a new value. Call the AuthHandler callback
# appropriately.
def set_credential(credential, value)
@credentials[credential] = value
- # TODO(dklimkin): @auth_handler is never defined.
@auth_handler.property_changed(credential, value) if @auth_handler
end
- # Generates string for UserAgent to put into HTTP headers.
- def generate_http_user_agent(extra_ids = [], agent_app = nil)
- return generate_user_agent(extra_ids, agent_app)
- end
-
- # Generates string for UserAgent to put into SOAP headers.
- def generate_soap_user_agent(extra_ids = [], agent_app = nil)
- return generate_user_agent(extra_ids, agent_app)
- end
-
- private
-
- # Generates user-agent.
- def generate_user_agent(extra_ids, agent_app)
+ # Generates string for UserAgent to put into headers.
+ def generate_user_agent(extra_ids = [], agent_app = nil)
agent_app ||= $0
agent_data = extra_ids
- agent_data << "Common-Ruby/%s" % AdsCommon::ApiConfig::CLIENT_LIB_VERSION
- agent_data << "Savon/%s" % Savon::VERSION
+ agent_data << 'Common-Ruby/%s' % AdsCommon::ApiConfig::CLIENT_LIB_VERSION
+ agent_data << 'Savon/%s' % Savon::VERSION
ruby_engine = defined?(RUBY_ENGINE) ? RUBY_ENGINE : 'ruby'
agent_data << [ruby_engine, RUBY_VERSION].join('/')
- agent_data << "HTTPI/%s" % HTTPI::VERSION
+ agent_data << 'HTTPI/%s' % HTTPI::VERSION
agent_data << HTTPI::Adapter.use.to_s
- return "%s (%s)" % [agent_app, agent_data.join(', ')]
+ return '%s (%s)' % [agent_app, agent_data.join(', ')]
+ end
+
+ # Sets authorization handler to notify about credential changes.
+ def set_auth_handler(auth_handler)
+ @auth_handler = auth_handler
end
+ private
+
# Loads the credentials from the config data.
def load_from_config(config)
@credentials = config.read('authentication')
@@ -62,7 +62,7 @@ def prepare_request(request, soap)
soap.input[2] = {:xmlns => @namespace}
# Sets User-Agent in the HTTP header.
request.headers['User-Agent'] =
- @credential_handler.generate_http_user_agent()
+ @credential_handler.generate_user_agent()
generate_headers(request, soap)
end
@@ -21,6 +21,6 @@
module AdsCommon
module ApiConfig
- CLIENT_LIB_VERSION = '0.7.4'
+ CLIENT_LIB_VERSION = '0.8.0'
end
end
@@ -21,7 +21,7 @@
# Tests credential handler methods.
require 'logger'
-require 'test/unit'
+require 'minitest/mock'
require 'ads_common/config'
require 'ads_common/credential_handler'
@@ -54,9 +54,49 @@ def test_credentials_override()
end
def test_generate_user_agent_simple()
- result1 = @handler.generate_http_user_agent()
+ result1 = @handler.generate_user_agent()
assert_kind_of(String, result1)
- result2 = @handler.generate_soap_user_agent()
- assert_kind_of(String, result2)
+ end
+
+ def test_generate_user_agent_chained()
+ test_str = 'Tester/0.2.0'
+ result1 = @handler.generate_user_agent([test_str])
+ assert_kind_of(String, result1)
+ assert_match(/#{Regexp.escape(test_str)}/, result1)
+ end
+
+ def test_auth_handler_callback_once()
+ mock = MiniTest::Mock.new()
+ mock.expect(:property_changed, nil, [:foo, 'bar'])
+ @handler.set_auth_handler(mock)
+ @handler.set_credential(:foo, 'bar')
+ assert(mock.verify)
+ end
+
+ def test_auth_handler_callback_compare()
+ credentials = @handler.credentials
+
+ credentials[:foo] = 'bar'
+ credentials[:baz] = 42
+ mock1 = MiniTest::Mock.new()
+ mock1.expect(:property_changed, nil, [:foo, 'bar'])
+ mock1.expect(:property_changed, nil, [:baz, 42])
+ @handler.set_auth_handler(mock1)
+ @handler.credentials = credentials
+ assert(mock1.verify)
+
+ credentials.delete(:baz)
+ mock2 = MiniTest::Mock.new()
+ mock2.expect(:property_changed, nil, [:baz, nil])
+ @handler.set_auth_handler(mock2)
+ @handler.credentials = credentials
+ assert(mock2.verify)
+
+ credentials[:foo] = nil
+ mock3 = MiniTest::Mock.new()
+ mock3.expect(:property_changed, nil, [:foo, nil])
+ @handler.set_auth_handler(mock3)
+ @handler.credentials = credentials
+ assert(mock3.verify)
end
end
@@ -37,5 +37,5 @@ Gem::Specification.new do |s|
s.files = Dir.glob('{lib,test}/**/*') + Dir.glob('examples/v*/**/*') +
%w(COPYING README ChangeLog adwords_api.yml)
s.test_files = ['test/suite_unittests.rb']
- s.add_dependency('google-ads-common', '~> 0.7.3')
+ s.add_dependency('google-ads-common', '~> 0.8.0')
end
@@ -45,7 +45,7 @@ def credentials(credentials_override = nil)
validate_headers_for_server(result)
extra_headers = {
- 'userAgent' => generate_soap_user_agent(),
+ 'userAgent' => generate_user_agent(),
'developerToken' => result[:developer_token]
}
if !@use_mcc and result[:client_customer_id]
@@ -57,27 +57,15 @@ def credentials(credentials_override = nil)
return result
end
- # Generates string to user as user agent in HTTP headers.
- def generate_http_user_agent(extra_ids = [])
- extra_ids, agent_app = get_user_agent_data(extra_ids)
- super(extra_ids, agent_app)
- end
-
- # Generates string to user as user agent in SOAP headers.
- def generate_soap_user_agent(extra_ids = [])
- extra_ids, agent_app = get_user_agent_data(extra_ids)
+ # Generates string to use as user agent in headers.
+ def generate_user_agent(extra_ids = [])
+ agent_app = @config.read('authentication.user_agent')
+ extra_ids << ['AwApi-Ruby/%s' % AdwordsApi::ApiConfig::CLIENT_LIB_VERSION]
super(extra_ids, agent_app)
end
private
- # Returns agent name and data for user-agent string generation.
- def get_user_agent_data(extra_ids)
- agent_app = @config.read('authentication.user_agent')
- extra_ids << ['AwApi-Ruby/%s' % AdwordsApi::ApiConfig::CLIENT_LIB_VERSION]
- return [extra_ids, agent_app]
- end
-
# Validates that the right credentials are being used for the chosen
# environment.
#
@@ -51,7 +51,7 @@ def headers(url, cid)
'Content-Type' => 'application/x-www-form-urlencoded',
'Authorization' =>
@auth_handler.auth_string(credentials, HTTPI::Request.new(url)),
- 'User-Agent' => @credential_handler.generate_http_user_agent(),
+ 'User-Agent' => @credential_handler.generate_user_agent(),
'clientCustomerId' => credentials[:client_customer_id].to_s,
'developerToken' => credentials[:developer_token]
}
@@ -37,5 +37,5 @@ Gem::Specification.new do |s|
s.files = Dir.glob('{examples,lib,test}/**/*') +
%w(COPYING README ChangeLog dfp_api.yml)
s.test_files = Dir.glob('test/**/test_*.rb')
- s.add_dependency('google-ads-common', '~> 0.7.3')
+ s.add_dependency('google-ads-common', '~> 0.8.0')
end
@@ -30,33 +30,21 @@ def credentials(credentials_override = nil)
result = super(credentials_override)
validate_headers_for_server(result)
result[:extra_headers] = {
- 'applicationName' => generate_soap_user_agent(),
+ 'applicationName' => generate_user_agent(),
'networkCode' => result[:network_code]
}
return result
end
- # Generates string to user as user agent in HTTP headers.
- def generate_http_user_agent(extra_ids = [])
- extra_ids, agent_app = get_user_agent_data(extra_ids)
- super(extra_ids, agent_app)
- end
-
- # Generates string to user as user agent in SOAP headers.
- def generate_soap_user_agent(extra_ids = [])
- extra_ids, agent_app = get_user_agent_data(extra_ids)
+ # Generates string to use as user agent in headers.
+ def generate_user_agent(extra_ids = [])
+ agent_app = @config.read('authentication.application_name')
+ extra_ids << ["DfpApi-Ruby/%s" % DfpApi::ApiConfig::CLIENT_LIB_VERSION]
super(extra_ids, agent_app)
end
private
- # Returns agent name and data for user-agent string generation.
- def get_user_agent_data(extra_ids)
- agent_app = @config.read('authentication.application_name')
- extra_ids << ["DfpApi-Ruby/%s" % DfpApi::ApiConfig::CLIENT_LIB_VERSION]
- return [extra_ids, agent_app]
- end
-
# Validates that the right credentials are being used for the chosen
# environment.
# TODO(dklimkin): implement NetworkCode check.

0 comments on commit f63e7cf

Please sign in to comment.