Merge pull request aeolus-incubator#42 from jistr/config_loading

Config loading refactored

lib/aeolus/cli.rb

@@ -3,13 +3,3 @@ module Aeolus
 
 module Aeolus::Cli
 end
-
-class Aeolus::Cli::Error < StandardError
-  attr_reader :message
-
-  def initialize(message)
-    @message  = message
-  end
-end
-
-class Aeolus::Cli::ConfigError < Aeolus::Cli::Error; end

lib/aeolus/cli/common_cli.rb

@@ -1,5 +1,6 @@
 require 'active_resource'
 require 'thor'
+require 'aeolus/cli/config'
 require 'aeolus/cli/formatting'
 require 'aeolus/client/base'
 require 'aeolus/client/provider_type'
@@ -12,10 +13,12 @@ class Aeolus::Cli::CommonCli < Thor
     :desc => "FORMAT can be 'human' or 'machine'"
 
   attr_accessor :output_format
+  attr_accessor :config
 
-  def initialize(*args)
-    super
-    load_aeolus_config(options)
+  def initialize(args = [], opts = {}, cfg = {})
+    super(args, opts, cfg)
+    validate_config = cfg[:current_task] && cfg[:current_task].name != 'help'
+    load_config(options, validate_config)
     set_output_format(options)
   end
 
@@ -64,77 +67,10 @@ def method_option_sort_by
     end
   end
 
-  def load_aeolus_config(options)
-    # set logging defaults
-    ActiveResource::Base.logger = Logger.new(STDOUT)
-    ActiveResource::Base.logger.level = Logger::WARN
-
-    # locate the config file if one exists
-    config_fname = nil
-    if ENV.has_key?("AEOLUS_CLI_CONF")
-      config_fname = ENV["AEOLUS_CLI_CONF"]
-      if !is_file?(config_fname)
-        raise Aeolus::Cli::ConfigError.new(
-          "environment variable AEOLUS_CLI_CONF with value "+
-          "'#{ ENV['AEOLUS_CLI_CONF']}' does not point to an existing file")
-      end
-    else
-      ["~/.aeolus-cli","/etc/aeolus-cli"].each do |fname|
-        if is_file?(fname)
-          config_fname = fname
-          break
-        end
-      end
-    end
-
-    # load the config file if we have one
-    if config_fname != nil
-      @config = YAML::load(File.open(File.expand_path(config_fname)))
-      if @config.has_key?(:conductor)
-        [:url, :password, :username].each do |key|
-          raise Aeolus::Cli::ConfigError.new \
-          ("Error in configuration file: #{key} is missing"
-           ) unless @config[:conductor].has_key?(key)
-        end
-        Aeolus::Client::Base.site = @config[:conductor][:url]
-        Aeolus::Client::Base.user = @config[:conductor][:username]
-        Aeolus::Client::Base.password = @config[:conductor][:password]
-      else
-        raise Aeolus::Cli::ConfigError.new("Error in configuration file")
-      end
-      if @config.has_key?(:logging)
-        if  @config[:logging].has_key?(:logfile)
-          if @config[:logging][:logfile].upcase == "STDOUT"
-            ActiveResource::Base.logger = Logger.new(STDOUT)
-          elsif @config[:logging][:logfile].upcase == "STDERR"
-            ActiveResource::Base.logger = Logger.new(STDERR)
-          else
-            ActiveResource::Base.logger =
-              Logger.new(@config[:logging][:logfile])
-          end
-        end
-        if  @config[:logging].has_key?(:level)
-          log_level = @config[:logging][:level]
-          if ! ['DEBUG','WARN','INFO','ERROR','FATAL'].include?(log_level)
-            raise Aeolus::Cli::ConfigError.new \
-            ("log level specified in configuration #{log_level}, is not valid."+
-             ".  shoud be one of DEBUG, WARN, INFO, ERROR or FATAL")
-          else
-            ActiveResource::Base.logger.level = eval("Logger::#{log_level}")
-          end
-        end
-      end
-    end
-    # allow overrides from command line
-    if options[:conductor_url]
-      Aeolus::Client::Base.site = options[:conductor_url]
-    end
-    if options[:username]
-      Aeolus::Client::Base.user = options[:username]
-    end
-    if options[:password]
-      Aeolus::Client::Base.password = options[:password]
-    end
+  def load_config(options, validate)
+    @config = Aeolus::Cli::Config.load_config(options)
+    @config.validate! if validate
+    @config.push
   end
 
   # Set output format (human vs. machine)
@@ -189,14 +125,4 @@ def provider_type_hash
     deltacloud_driver_to_provider_type
   end
 
-  # TODO: Consider ripping all this file-related stuff into a module or
-  # class for better encapsulation and testability
-  def is_file?(path)
-    full_path = File.expand_path(path)
-    if File.exist?(full_path) && !File.directory?(full_path)
-      return true
-    end
-    false
-  end
-
 end

lib/aeolus/cli/config.rb

@@ -0,0 +1,89 @@
+require 'active_support/core_ext/hash/deep_merge'
+require 'aeolus/cli/errors'
+
+# push_config requirements
+require 'aeolus/client/base'
+require 'logger'
+
+module Aeolus::Cli
+  class Config < Hash
+    DEFAULT_CONFIG_FILE =
+      File.expand_path(File.join(File.dirname(__FILE__),
+                                 '..', '..', '..', # thor-cli gem dir
+                                 'templates', 'default_config.yml'))
+
+    def push
+      ActiveResource::Base.logger = case self[:logging][:file]
+                                    when /\Astdout\Z/i
+                                      Logger.new(STDOUT)
+                                    when /\Astderr\Z/i
+                                      Logger.new(STDERR)
+                                    else
+                                      Logger.new(File.expand_path(self[:logging][:file]))
+                                    end
+      ActiveResource::Base.logger.level = Logger.const_get(self[:logging][:level].upcase)
+
+      Aeolus::Client::Base.site = self[:conductor][:url]
+      Aeolus::Client::Base.user = self[:conductor][:username]
+      Aeolus::Client::Base.password = self[:conductor][:password]
+    end
+
+    def validate!
+      error_class = Aeolus::Cli::ConfigError
+      required_attributes = {
+        '--conductor-url' => self[:conductor][:url],
+        '--username'      => self[:conductor][:username],
+        '--password'      => self[:conductor][:password],
+      }
+      required_attributes.each do |name, value|
+        raise error_class, "Setting #{name} is required." unless value
+      end
+      true
+    end
+
+
+    class << self
+      def new_from_hash(hash)
+        self.new.merge!(hash)
+      end
+
+      def load_config(options)
+        config_hash = {}
+        config_hash.deep_merge!(config_file_hash(DEFAULT_CONFIG_FILE))
+        config_hash.deep_merge!(config_file_hash(config_file_to_load)) if config_file_to_load
+        config_hash.deep_merge!(options_hash(options))
+        self.new_from_hash(config_hash)
+      end
+
+      private
+
+      def config_file_to_load
+        env_config = ENV['AEOLUS_CLI_CONF']
+        home_config = "#{ENV['HOME']}/.aeolus-cli"
+        global_config = "/etc/aeolus-cli"
+
+        return env_config    if env_config
+        return home_config   if File.exists?(home_config)
+        return global_config if File.exists?(global_config)
+        nil
+      end
+
+      def config_file_hash(file_name)
+        unless File.file?(file_name)
+          raise Aeolus::Cli::ConfigError.new("Config file '#{file_name}' does not exist.")
+        end
+
+        YAML::load(File.read(File.expand_path(file_name)))
+      end
+
+      def options_hash(options)
+        hash = {}
+        hash.deep_merge!({ :conductor => { :url => options[:conductor_url] } }) if options[:conductor_url]
+        hash.deep_merge!({ :conductor => { :username => options[:username] } }) if options[:username]
+        hash.deep_merge!({ :conductor => { :password => options[:password] } }) if options[:password]
+        hash
+      end
+    end
+  end
+
+end

lib/aeolus/cli/errors.rb

@@ -0,0 +1,9 @@
+class Aeolus::Cli::Error < StandardError
+  attr_reader :message
+
+  def initialize(message)
+    @message  = message
+  end
+end
+
+class Aeolus::Cli::ConfigError < Aeolus::Cli::Error; end

lib/aeolus/client/base.rb

@@ -1,10 +1,6 @@
 require 'aeolus/client'
 require 'logger'
 
-# FIXME: get rid of these when refactoring config loading:
-ActiveResource::Base.logger = Logger.new(STDOUT)
-ActiveResource::Base.logger.level = Logger::INFO
-
 class Aeolus::Client::Base < ActiveResource::Base
   self.timeout = 600
   self.format = :xml

spec/aeolus/cli/common_cli_spec.rb

@@ -1,7 +1,39 @@
 require 'aeolus/cli/common_cli'
 
 describe Aeolus::Cli::CommonCli do
-  let(:common_cli) { Aeolus::Cli::CommonCli.new() }
+  let(:initialize_args) { [] }
+  let(:common_cli) { Aeolus::Cli::CommonCli.new(*initialize_args) }
+
+  context "#initialize" do
+    subject { common_cli }
+
+    context "config" do
+      let(:config) do
+        double('config').tap { |c| c.should_receive(:push) }
+      end
+
+      before do
+        Aeolus::Cli::Config.should_receive(:new_from_hash).and_return(config)
+      end
+
+      its(:config) { should == config }
+
+      context "when running non-help task" do
+        let(:initialize_args) do
+          [
+            [],
+            {},
+            { :current_task => double('list task', :name => 'list') },
+          ]
+        end
+
+        it "validates the config" do
+          config.should_receive(:validate!)
+          subject # run the constructor
+        end
+      end
+    end
+  end
 
   context "#resource_fields" do
     context "non-empty fields" do