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

Fixes #8016 - api connection moved to context #227

Merged
merged 1 commit into from
Dec 9, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/hammer_cli/apipie.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
require File.join(File.dirname(__FILE__), './apipie/option_builder')
require File.join(File.dirname(__FILE__), './apipie/api_connection')
require File.join(File.dirname(__FILE__), './apipie/command')
27 changes: 27 additions & 0 deletions lib/hammer_cli/apipie/api_connection.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
require 'apipie_bindings'
module HammerCLI::Apipie
class ApiConnection < HammerCLI::AbstractConnector
attr_reader :api

def initialize(params, options = {})
@logger = options[:logger]
@api = ApipieBindings::API.new(params)
if options[:reload_cache]
@api.clean_cache
@logger.debug 'Apipie cache was cleared' unless @logger.nil?
end
end

def resources
@api.resources
end

def resource(resource_name)
@api.resource(resource_name)
end

def has_resource?(resource_name)
@api.has_resource?(resource_name)
end
end
end
27 changes: 1 addition & 26 deletions lib/hammer_cli/apipie/resource.rb
Original file line number Diff line number Diff line change
@@ -1,21 +1,5 @@
require 'apipie_bindings'
module HammerCLI::Apipie


class ApipieConnector < HammerCLI::AbstractConnector

attr_reader :api

def initialize(params)
@api = ApipieBindings::API.new(params)
if HammerCLI::Settings.get(:_params, :reload_cache) || HammerCLI::Settings.get(:reload_cache)
@api.clean_cache
Logging.logger['Init'].debug 'Apipie cache was cleared'
end
end
end


module Resource

def self.included(base)
Expand Down Expand Up @@ -44,12 +28,6 @@ def resource_config
{}
end

def connection_options
{
:connector => HammerCLI::Apipie::ApipieConnector
}
end

def connection_name(resource_class)
:apipie
end
Expand All @@ -70,10 +48,7 @@ def module_resource

def resource(resource=nil, action=nil)
unless resource.nil?
api = HammerCLI::Connection.create(
connection_name(resource),
resource_config,
connection_options).api
api = HammerCLI.context[:api_connection].get(connection_name(resource))
if api.has_resource?(resource)
@api_resource = api.resource(resource)
else
Expand Down
23 changes: 12 additions & 11 deletions lib/hammer_cli/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,39 +6,40 @@ def initialize(params={})
end

class Connection
def initialize(logger = nil)
@logger = logger
end

def self.drop(name)
def drop(name)
connections.delete(name)
end

def self.drop_all()
def drop_all()
connections.keys.each { |c| drop(c) }
end

def self.create(name, conector_params={}, options={})
def create(name, &create_connector_block)
unless connections[name]
Logging.logger['Connection'].debug "Registered: #{name}"
connector = options[:connector] || AbstractConnector

connections[name] = connector.new(conector_params)
connector = yield
@logger.debug("Registered: #{name}") if @logger
connections[name] = connector
end
connections[name]
end

def self.exist?(name)
def exist?(name)
!get(name).nil?
end

def self.get(name)
def get(name)
connections[name]
end

private

def self.connections
def connections
@connections_hash ||= {}
@connections_hash
end

end
end
5 changes: 3 additions & 2 deletions lib/hammer_cli/context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
module HammerCLI

def self.context
{
@context ||= {
:defaults => HammerCLI.defaults,
:is_tty? => HammerCLI.tty?
:is_tty? => HammerCLI.tty?,
:api_connection => HammerCLI::Connection.new(Logging.logger['Connection'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to move this to command initializer and add just the attributes missing? This way you could provide only part of the context to the command and the rest would be replaced with the defaults. The context would also be accessible with @context?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand. The context is passed to commands' constructors and is accessible as self.context or @context.
I don't think setting a default value for the api connection makes sense in this case, because we need one connection for the whole hammer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, sorry for the confusion.

}
end

Expand Down
2 changes: 1 addition & 1 deletion lib/hammer_cli/exception_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def handle_not_found(e)
end

def handle_unauthorized(e)
print_error _("Invalid username or password")
print_error e.message
log_full_error e
HammerCLI::EX_UNAUTHORIZED
end
Expand Down
6 changes: 6 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,9 @@

Logging.logger.root.appenders = Logging::Appenders['__test__'] || Logging::Appenders::StringIo.new('__test__')

HammerCLI.context[:api_connection].create(:apipie) do
HammerCLI::Apipie::ApiConnection.new({
:apidoc_cache_dir => 'test/unit/fixtures/apipie',
:apidoc_cache_name => 'architectures'
})
end
38 changes: 38 additions & 0 deletions test/unit/apipie/api_connection_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
require File.join(File.dirname(__FILE__), '../../test_helper')

describe HammerCLI::Apipie::ApiConnection do
let(:connection) { HammerCLI::Connection.new }

describe '#initialize' do

let(:empty_params) {{}}

def api_stub(params = {})
api_stub = stub()
ApipieBindings::API.expects(:new).with(params).returns(api_stub)
api_stub
end

it "passes attributes to apipie bindings" do
params = { :apidoc_cache_name => 'test.example.com' }

api_stub(params)
HammerCLI::Apipie::ApiConnection.new(params)
end

context "with :clear_cache => true" do
it "clears cache" do
api_stub.expects(:clean_cache)
HammerCLI::Apipie::ApiConnection.new(empty_params, :reload_cache => true)
end

it "logs message when logger is available" do
logger = stub()
logger.expects(:debug).with('Apipie cache was cleared')

api_stub.expects(:clean_cache)
HammerCLI::Apipie::ApiConnection.new(empty_params, :reload_cache => true, :logger => logger)
end
end
end
end
52 changes: 6 additions & 46 deletions test/unit/apipie/command_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,11 @@

describe HammerCLI::Apipie::Command do

class TestCommand < HammerCLI::Apipie::Command
def self.resource_config
{ :apidoc_cache_dir => 'test/unit/fixtures/apipie', :apidoc_cache_name => 'architectures' }
end
end

class ParentCommand < TestCommand
class ParentCommand < HammerCLI::Apipie::Command
action :show
end

class OptionCommand < TestCommand
class OptionCommand < HammerCLI::Apipie::Command
resource :architectures, :create

def option_name
Expand All @@ -23,10 +17,9 @@ def option_name
def option_operatingsystem_ids
nil
end

end

class CommandA < TestCommand
class CommandA < HammerCLI::Apipie::Command
resource :architectures, :index

class CommandB < ParentCommand
Expand All @@ -37,14 +30,10 @@ class CommandC < CommandA
end

let(:ctx) { { :adapter => :silent, :interactive => false } }
let(:cmd_class) { TestCommand.dup }
let(:cmd_class) { HammerCLI::Apipie::Command.dup }
let(:cmd) { cmd_class.new("", ctx) }
let(:cmd_run) { cmd.run([]) }

before :each do
HammerCLI::Connection.drop_all
end

context "setting resources" do

it "should set resource and action together" do
Expand Down Expand Up @@ -103,40 +92,11 @@ class CommandC < CommandA
end

context "resource defined" do

before :each do
HammerCLI::Connection.drop_all
ApipieBindings::API.any_instance.stubs(:call).returns([])
cmd.class.resource :architectures, :index
end

it "should perform a call to api when resource is defined" do
ctx[:defaults] = stub(:get_defaults => {})
cmd_run.must_equal 0
end
end

context "reload apipie cache" do

before :each do
HammerCLI::Connection.drop_all
ApipieBindings::API.any_instance.stubs(:call).returns([])
end

it "clears the cache on init when required from config" do
HammerCLI::Settings.load({ :reload_cache => true })
ApipieBindings::API.any_instance.expects(:clean_cache).returns(nil)
ApipieBindings::API.any_instance.expects(:call).returns([])
cmd.class.resource :architectures, :index
HammerCLI::Settings.load({ :reload_cache => false })
end

it "clears the cache on init when required from CLI" do
HammerCLI::Settings.load({ :_params => { :reload_cache => true }})
ApipieBindings::API.any_instance.expects(:clean_cache).returns(nil)
cmd.class.resource :architectures, :index
HammerCLI::Settings.load({ :_params => { :reload_cache => false }})
cmd_run.must_equal 0
end

end

context "options" do
Expand Down
Loading