From ea181e93f90ada5d74921cab7ba89e308b9ea1e3 Mon Sep 17 00:00:00 2001
From: Saku Ytti <saku@ytti.fi>
Date: Thu, 7 Jun 2018 10:08:44 +0300
Subject: Refactor manager

Main problem we're trying to solve is the silent failures. Requiring
files will never fail, one user had problem loading their souce http.rb,
which is obviously mandatory for them, but because it didn't fail, it
lead to a much harder to understand bug down the line, as we tried to
instansiate the 'nil' source.

Reason why I did it like this was laziness, because I didn't have to
care if or not the file exists, I always tried to load locally defined
models, and just return empty hash if I couldn't find any.

There may be other reasons and reliances that I've forgotten, but we'll
fix them as they come. Such is life without proper testing.
---
 lib/oxidized/manager.rb | 74 +++++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 40 deletions(-)

diff --git a/lib/oxidized/manager.rb b/lib/oxidized/manager.rb
index c4523f3..e4a1d7c 100644
--- a/lib/oxidized/manager.rb
+++ b/lib/oxidized/manager.rb
@@ -6,23 +6,20 @@ module Oxidized
   class Manager
     class << self
       def load dir, file
-        begin
-          require File.join dir, file + '.rb'
-          klass = nil
-          [Oxidized, Object].each do |mod|
-            klass = mod.constants.find { |const| const.to_s.downcase == file.downcase }
-            klass = mod.constants.find { |const| const.to_s.downcase == 'oxidized' + file.downcase } unless klass
-            klass = mod.const_get klass if klass
-            break if klass
-          end
-          i = klass.new
-          i.setup if i.respond_to? :setup
-          { file => klass }
-        rescue LoadError
-          {}
+        require File.join dir, file + '.rb'
+        klass = nil
+        [Oxidized, Object].each do |mod|
+          klass = mod.constants.find { |const| const.to_s.downcase == file.downcase }
+          klass = mod.constants.find { |const| const.to_s.downcase == 'oxidized' + file.downcase } unless klass
+          klass = mod.const_get klass if klass
+          break if klass
         end
+        i = klass.new
+        i.setup if i.respond_to? :setup
+        { file => klass }
       end
     end
+
     attr_reader :input, :output, :model, :source, :hook
     def initialize
       @input  = {}
@@ -32,40 +29,37 @@ module Oxidized
       @hook = {}
     end
 
-    def add_input method
-      method = Manager.load Config::InputDir, method
-      return false if method.empty?
-      @input.merge! method
+    def add_input name
+      @input.merge! Manager.load(Config::InputDir, name)
+    end
+
+    def add_output name
+      @output.merge! Manager.load(Config::OutputDir, name)
     end
 
-    def add_output method
-      method = Manager.load Config::OutputDir, method
-      return false if method.empty?
-      @output.merge! method
+    def add_source name
+      return nil if @source.has_key? name
+      @source.merge Manager.load(Config::SourceDir, name)
     end
 
-    def add_model _model
-      name = _model
-      _model = Manager.load File.join(Config::Root, 'model'), name
-      _model = Manager.load Config::ModelDir, name if _model.empty?
-      return false if _model.empty?
-      @model.merge! _model
+    def add_model name
+      @model.merge! local_load("model", name) ||
+                    Manager.load(Config::ModelDir, name)
     end
 
-    def add_source _source
-      return nil if @source.has_key? _source
-      _source = Manager.load Config::SourceDir, _source
-      return false if _source.empty?
-      @source.merge! _source
+    def add_hook name
+      return nil if @hook.has_key? name
+      @model.merge! local_load("hook", name) ||
+                    Manager.load(Config::HookDir, name)
     end
 
-    def add_hook _hook
-      return nil if @hook.has_key? _hook
-      name = _hook
-      _hook = Manager.load File.join(Config::Root, 'hook'), name
-      _hook = Manager.load Config::HookDir, name if _hook.empty?
-      return false if _hook.empty?
-      @hook.merge! _hook
+    private
+
+    # try to load locally defined file, instead of upstream provided
+    def local_load dir, file
+      dir = File.join(Config::Root, dir)
+      return false unless File.exist? File.join(dir, file)
+      Manager.load dir, file
     end
   end
 end
-- 
cgit v1.2.1