Merge pull request #148 from mbacovsky/7566_mod_deps

Fixes #7566 - added support for dependeces among modules

doc/installation.md

@@ -29,10 +29,10 @@ Configuration is by default looked for in the following directories, loaded in t
  - custom location (file or directory) specified on command line - ```-c CONF_FILE_PATH```
 
 In each of these directories hammer is trying to load ```cli_config.yml``` and anything in
-the ```cli.modules.d``` subdirectory which is place for specific configuration of hammer modules.
+the ```cli.modules.d``` subdirectory which is place for specific configuration of hammer modules a.k.a. plugins.
 
 Later directories and files have precedence if they redefine the same option. Files from ```cli.modules.d```
-are loaded in alphabetical order.
+are loaded in alphabetical order. The modules are loaded in alphabetical order which can be overriden with explicit dependences set in module ```gemspec```.
 
 ### Manual installation
 The packaged version of hammer copies the template to `/etc/hammer` for you.
@@ -70,6 +70,7 @@ in ```~/.hammer/cli.modules.d/foreman.yml``` should look as follows:
     :password: 'changeme'
 ```
 
+
 Use the hammer
 --------------
 

lib/hammer_cli/exception_handler.rb

@@ -17,7 +17,9 @@ def mappings
         [RestClient::ResourceNotFound, :handle_not_found],
         [RestClient::Unauthorized, :handle_unauthorized],
         [ApipieBindings::DocLoadingError, :handle_apipie_docloading_error],
-        [ApipieBindings::MissingArgumentsError, :handle_apipie_missing_arguments_error]
+        [ApipieBindings::MissingArgumentsError, :handle_apipie_missing_arguments_error],
+        [HammerCLI::ModuleCircularDependency, :handle_generic_config_error],
+        [HammerCLI::ModuleDisabledButRequired, :handle_generic_config_error]
       ]
     end
 
@@ -103,8 +105,11 @@ def handle_apipie_missing_arguments_error(e)
       HammerCLI::EX_USAGE
     end
 
+    def handle_generic_config_error(e)
+      print_error e.message
+      log_full_error e
+      HammerCLI::EX_CONFIG
+    end
+
   end
 end
-
-
-

lib/hammer_cli/exceptions.rb

@@ -3,5 +3,7 @@ module HammerCLI
   class CommandConflict < StandardError; end
   class OperationNotSupportedError < StandardError; end
   class ModuleLoadingError < StandardError; end
+  class ModuleCircularDependency < StandardError; end
+  class ModuleDisabledButRequired < StandardError; end
 
 end

lib/hammer_cli/modules.rb

@@ -1,16 +1,67 @@
-
+require 'tsort'
 module HammerCLI
 
+  class ModulesList < Hash
+    include TSort
+
+    def tsort_each_node(&block)
+      each_key.sort.each(&block)
+    end
+
+    def tsort_each_child(node, &block)
+      fetch(node).each(&block)
+    end
+  end
+
   class Modules
 
     def self.names
+      # add dependencies
+      modules = find_dependencies({}, enabled_modules)
+      # sort with the deps in mind
+      ModulesList[modules].tsort
+    rescue TSort::Cyclic => e
+      raise HammerCLI::ModuleCircularDependency.new(_("Unable to load modules: Circular dependency detected (%s)") % e.message)
+    end
+
+
+    def self.find_dependencies(dependencies, module_list)
+      new_deps = []
+
+      # add inspected modules in current level (depth)
+      dependencies.merge(Hash[module_list.map{ |m| [m, []] }])
 
+      # lookup dependencies
+      module_list.each do |mod|
+        deps = dependencies_for(mod)
+        logger.debug(_("Module depenedency detected: %{mod} requires %{deps}") % { :mod => mod, :deps => deps.join(', ') }) unless deps.empty?
+        dependencies[mod] = deps # update deps
+        # check new/disabled deps
+        deps.each do |dep|
+          if !dependencies.has_key?(dep)
+            if HammerCLI::Settings.get(dep.gsub(/^hammer_cli_/, ''), :enable_module) == false
+              raise HammerCLI::ModuleDisabledButRequired.new(_("Module %{mod} depends on module %{dep} which is disabled in configuration") % { :mod => mod, :dep => dep })
+            end
+            new_deps << dep
+          end
+        end
+      end
+      dependencies = find_dependencies(dependencies, new_deps) unless new_deps.empty?
+      dependencies
+    end
+
+    def self.dependencies_for(module_name)
+      mod = Gem::Specification.find_by_name(module_name)
+      mod.dependencies.select{ |dep| dep.name =~ /^hammer_cli_.*/ }.map(&:name)
+    end
+
+    def self.enabled_modules
       # legacy modules config
       modules = HammerCLI::Settings.get(:modules) || []
       logger.warn _("Legacy configuration of modules detected. Check section about configuration in user manual") unless modules.empty?
 
       HammerCLI::Settings.dump.inject(modules) do |names, (mod_name, mod_config)|
-        if mod_config.kind_of?(Hash) && !mod_config[:enable_module].nil?
+        if is_module_config?(mod_config)
           mod = ["hammer_cli_#{mod_name}"]
           if mod_config[:enable_module]
             names += mod
@@ -67,6 +118,11 @@ def self.load_all
     end
 
     protected
+
+    def self.is_module_config?(config)
+      config.kind_of?(Hash) && !config[:enable_module].nil?
+    end
+
     def self.logger
       Logging.logger['Modules']
     end

test/unit/modules_test.rb

@@ -28,7 +28,8 @@ def self.version
 
   describe "names" do
     it "must return list of modules" do
-      HammerCLI::Modules.names.sort.must_equal ["hammer_cli_tom", "hammer_cli_jerry"].sort
+      HammerCLI::Modules.stubs(:dependencies_for).returns([])
+      HammerCLI::Modules.names.must_equal ["hammer_cli_jerry", "hammer_cli_tom"]
     end
 
     it "must return empty array by default" do
@@ -42,7 +43,51 @@ def self.version
         :tom => {},
         :modules => ['hammer_cli_tom', 'hammer_cli_jerry'],
       })
-      HammerCLI::Modules.names.sort.must_equal ["hammer_cli_tom", "hammer_cli_jerry"].sort
+      HammerCLI::Modules.stubs(:dependencies_for).returns([])
+      HammerCLI::Modules.names.must_equal ["hammer_cli_jerry", "hammer_cli_tom"]
+    end
+
+    it "must resolve module depndences" do
+      HammerCLI::Modules.stubs(:dependencies_for).returns([])
+      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_jerry').returns(['hammer_cli_tom'])
+      HammerCLI::Modules.names.must_equal ["hammer_cli_tom", "hammer_cli_jerry"]
+    end
+
+    it "must detect circular dependences" do
+      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_jerry').returns(['hammer_cli_tom'])
+      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_tom').returns(['hammer_cli_jerry'])
+      proc { HammerCLI::Modules.names }.must_raise HammerCLI::ModuleCircularDependency
+    end
+
+    it "must sort modules with dependency depth > 1" do
+      HammerCLI::Settings.clear
+      HammerCLI::Settings.load({
+        :tom => { :enable_module => true },
+        :jerry => { :enable_module => true },
+        :cherie => { :enable_module => true }
+      })
+      HammerCLI::Modules.stubs(:dependencies_for).returns([])
+      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_jerry').returns(['hammer_cli_tom'])
+      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_cherie').returns(['hammer_cli_jerry'])
+      HammerCLI::Modules.names.must_equal ["hammer_cli_tom", "hammer_cli_jerry", "hammer_cli_cherie"]
+    end
+
+    it "must handle dependency on module not mentioned in configuration" do
+      HammerCLI::Modules.stubs(:dependencies_for).returns([])
+      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_jerry').returns(['hammer_cli_tom', 'hammer_cli_cherie'])
+      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_cherie').returns(['hammer_cli_quacker'])
+      HammerCLI::Modules.names.must_equal ["hammer_cli_quacker", "hammer_cli_cherie", "hammer_cli_tom", "hammer_cli_jerry"]
+    end
+
+    it "must detect dependency on disabled module" do
+      HammerCLI::Settings.clear
+      HammerCLI::Settings.load({
+        :tom => { :enable_module => false },
+        :jerry => { :enable_module => true },
+      })
+      HammerCLI::Modules.stubs(:dependencies_for).returns([])
+      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_jerry').returns(['hammer_cli_tom'])
+      proc { HammerCLI::Modules.names }.must_raise HammerCLI::ModuleDisabledButRequired
     end
   end
 
@@ -63,6 +108,7 @@ def self.version
   describe "load all modules" do
 
     it "must call load for each module" do
+      HammerCLI::Modules.stubs(:dependencies_for).returns([])
       HammerCLI::Modules.expects(:load).with("hammer_cli_tom")
       HammerCLI::Modules.expects(:load).with("hammer_cli_jerry")
       HammerCLI::Modules.load_all