Refactor manager, take2

Make all the add_* return falsy valuem and make all the callers raise,
this has benefit that the caller has more information, like it can tell
what node we were trying to load when model failed to load.

We were only failing to do this in two places in the code, source and
plugin.

4 files changed, 19 insertions, 20 deletions

diff --git a/lib/oxidized/hook.rb b/lib/oxidized/hook.rb
index 915299b..2eb6f87 100644
--- a/lib/oxidized/hook.rb
+++ b/lib/oxidized/hook.rb
@@ -37,7 +37,7 @@ module Oxidized
               "unknown event #{event}, available: #{Events.join ','}"
       end
 
-      Oxidized.mgr.add_hook hook_type
+      Oxidized.mgr.add_hook(hook_type) or raise("cannot load hook '#{hook_type}', not found")
       begin
         hook = Oxidized.mgr.hook.fetch(hook_type).new
       rescue KeyError
diff --git a/lib/oxidized/manager.rb b/lib/oxidized/manager.rb
index 8b34125..ee39e40 100644
--- a/lib/oxidized/manager.rb
+++ b/lib/oxidized/manager.rb
@@ -17,49 +17,48 @@ module Oxidized
         i = klass.new
         i.setup if i.respond_to? :setup
         { file => klass }
+      rescue LoadError
+        false
       end
     end
 
-    attr_reader :input, :output, :model, :source, :hook
+    attr_reader :input, :output, :source, :model, :hook
     def initialize
       @input  = {}
       @output = {}
-      @model  = {}
       @source = {}
-      @hook = {}
+      @model  = {}
+      @hook   = {}
     end
 
     def add_input name
-      @input.merge! Manager.load(Config::InputDir, name)
+      loader @input, Config::InputDir, "input", name
     end
 
     def add_output name
-      @output.merge! Manager.load(Config::OutputDir, name)
+      loader @output, Config::OutputDir, "output", name
     end
 
     def add_source name
-      return nil if @source.has_key? name
-      @source.merge! Manager.load(Config::SourceDir, name)
+      loader @source, Config::SourceDir, "source", name
     end
 
     def add_model name
-      @model.merge! local_load("model", name) ||
-                    Manager.load(Config::ModelDir, name)
+      loader @model, Config::ModelDir, "model", name
     end
 
     def add_hook name
-      return nil if @hook.has_key? name
-      @model.merge! local_load("hook", name) ||
-                    Manager.load(Config::HookDir, name)
+      loader @hook, Config::HookDir, "hook", name
     end
 
     private
 
-    # try to load locally defined file, instead of upstream provided
-    def local_load dir, name
-      dir = File.join(Config::Root, dir)
-      return false unless File.exist? File.join(dir, name + ".rb")
-      Manager.load dir, name
+    # if local version of file exists, load it, else load global - return falsy value if nothing loaded
+    def loader hash, global_dir, local_dir, name
+      dir = File.join(Config::Root, local_dir)
+      map = Manager.load(dir, name) if File.exist? File.join(dir, name + ".rb")
+      map = Manager.load(global_dir, name) unless map
+      hash.merge!(map) if map
     end
   end
 end
diff --git a/lib/oxidized/nodes.rb b/lib/oxidized/nodes.rb
index a159b48..3e0869a 100644
--- a/lib/oxidized/nodes.rb
+++ b/lib/oxidized/nodes.rb
@@ -10,7 +10,7 @@ module Oxidized
       with_lock do
         new = []
         @source = Oxidized.config.source.default
-        Oxidized.mgr.add_source @source
+        Oxidized.mgr.add_source(@source) or raise(MethodNotFound, "cannot load node source '#{@source}', not found")
         Oxidized.logger.info "lib/oxidized/nodes.rb: Loading nodes"
         nodes = Oxidized.mgr.source[@source].new.load node_want
         nodes.each do |node|
diff --git a/lib/oxidized/version.rb b/lib/oxidized/version.rb
index 5a03e39..397ae5e 100644
--- a/lib/oxidized/version.rb
+++ b/lib/oxidized/version.rb
@@ -1,6 +1,6 @@
 module Oxidized
   VERSION = '0.22.0'
-  VERSION_FULL = '0.22.0'
+  VERSION_FULL = '0.22.0-10-ge8bee7b'
   def self.version_set
     version_full = %x(git describe --tags).chop rescue ""
     version      = %x(git describe --tags --abbrev=0).chop rescue ""