Refactor smells

* reduce no/resolve_repo smell
* reduce input/ssh/connect smell
* reduce source/http load smell
* reduce node/worker smell
* reduce source/csv smell
* reduce output/http smell

Get's code climate from B to A, so I'm sure it's super duper important.

6 files changed, 141 insertions, 131 deletions

diff --git a/lib/oxidized/input/ssh.rb b/lib/oxidized/input/ssh.rb
index d321a11..82335f9 100644
--- a/lib/oxidized/input/ssh.rb
+++ b/lib/oxidized/input/ssh.rb
@@ -21,36 +21,10 @@ module Oxidized
       @output      = ''
       @pty_options = { term: "vt100" }
       @node.model.cfg['ssh'].each { |cb| instance_exec(&cb) }
-      secure = Oxidized.config.input.ssh.secure
       @log = File.open(Oxidized::Config::Log + "/#{@node.ip}-ssh", 'w') if Oxidized.config.input.debug?
-      port = vars(:ssh_port) || 22
-
-      ssh_opts = {
-        port:         port.to_i,
-        paranoid:     secure,
-        keepalive:    true,
-        password:     @node.auth[:password], :timeout => Oxidized.config.timeout,
-        number_of_password_prompts: 0,
-      }
-
-      auth_methods = vars(:auth_methods) || %w(none publickey password)
-      ssh_opts[:auth_methods] = auth_methods
-      Oxidized.logger.debug "AUTH METHODS::#{auth_methods}"
-
-      if proxy_host = vars(:ssh_proxy)
-        proxy_command =  "ssh "
-        proxy_command += "-o StrictHostKeyChecking=no " unless secure
-        proxy_command += "#{proxy_host} -W %h:%p"
-        proxy = Net::SSH::Proxy::Command.new(proxy_command)
-        ssh_opts[:proxy] = proxy
-      end
-
-      ssh_opts[:keys] = vars(:ssh_keys).is_a?(Array) ? vars(:ssh_keys) : [vars(:ssh_keys)] if vars(:ssh_keys)
-      ssh_opts[:kex]  = vars(:ssh_kex).split(/,\s*/) if vars(:ssh_kex)
-      ssh_opts[:encryption] = vars(:ssh_encryption).split(/,\s*/) if vars(:ssh_encryption)
 
       Oxidized.logger.debug "lib/oxidized/input/ssh.rb: Connecting to #{@node.name}"
-      @ssh = Net::SSH.start(@node.ip, @node.auth[:username], ssh_opts)
+      @ssh = Net::SSH.start(@node.ip, @node.auth[:username], get_ssh_opts)
       unless @exec
         shell_open @ssh
         begin
@@ -142,5 +116,34 @@ module Oxidized
         end
       end
     end
+
+    def get_ssh_opts
+      ssh_opts = {
+        port:         (vars(:ssh_port) || 22).to_i,
+        paranoid:     secure,
+        keepalive:    true,
+        password:     @node.auth[:password], timeout: Oxidized.config.timeout,
+                      number_of_password_prompts: 0
+      }
+
+      auth_methods = vars(:auth_methods) || %w(none publickey password)
+      ssh_opts[:auth_methods] = auth_methods
+      Oxidized.logger.debug "AUTH METHODS::#{auth_methods}"
+
+      if proxy_host = vars(:ssh_proxy)
+        proxy_command =  "ssh "
+        proxy_command += "-o StrictHostKeyChecking=no " unless Oxidized.config.input.ssh.secure?
+        proxy_command += "#{proxy_host} -W %h:%p"
+        proxy = Net::SSH::Proxy::Command.new(proxy_command)
+        ssh_opts[:proxy] = proxy
+      end
+
+      ssh_opts[:keys]       = [vars(:ssh_keys)].flatten if vars(:ssh_keys)
+      ssh_opts[:kex]        = vars(:ssh_kex).split(/,\s*/) if vars(:ssh_kex)
+      ssh_opts[:encryption] = vars(:ssh_encryption).split(/,\s*/) if vars(:ssh_encryption)
+      ssh_opts[:verbose]    = Logger::DEBUG if Oxidized.config.input.debug?
+
+      ssh_opts
+    end
   end
 end
diff --git a/lib/oxidized/node.rb b/lib/oxidized/node.rb
index 300221e..bc1a586 100644
--- a/lib/oxidized/node.rb
+++ b/lib/oxidized/node.rb
@@ -173,32 +173,18 @@ module Oxidized
     end
 
     def resolve_repo opt
-      if is_git? opt
-        remote_repo = Oxidized.config.output.git.repo
-
-        if remote_repo.is_a?(::String)
-          if Oxidized.config.output.git.single_repo? || @group.nil?
-            remote_repo
-          else
-            File.join(File.dirname(remote_repo), @group + '.git')
-          end
-        else
-          remote_repo[@group]
-        end
-      elsif is_gitcrypt? opt
-        remote_repo = Oxidized.config.output.gitcrypt.repo
-
-        if remote_repo.is_a?(::String)
-          if Oxidized.config.output.gitcrypt.single_repo? || @group.nil?
-            remote_repo
-          else
-            File.join(File.dirname(remote_repo), @group + '.git')
-          end
+      type = git_type opt
+      return nil unless type
+
+      remote_repo = Oxidized.config.output.send(type).repo
+      if remote_repo.is_a?(::String)
+        if Oxidized.config.output.send(type).single_repo? || @group.nil?
+          remote_repo
         else
-          remote_repo[@group]
+          File.join(File.dirname(remote_repo), @group + '.git')
         end
       else
-        return
+        remote_repo[@group]
       end
     end
 
@@ -237,12 +223,11 @@ module Oxidized
       value
     end
 
-    def is_git? opt
-      (opt[:output] || Oxidized.config.output.default) == 'git'
+    def git_type
+      type = opt[:output] || Oxidized.config.output.default
+      return nil unless type[0..2] == "git"
+      type
     end
 
-    def is_gitcrypt? opt
-      (opt[:output] || Oxidized.config.output.default) == 'gitcrypt'
-    end
   end
 end
diff --git a/lib/oxidized/output/http.rb b/lib/oxidized/output/http.rb
index 0467261..756b9ad 100644
--- a/lib/oxidized/output/http.rb
+++ b/lib/oxidized/output/http.rb
@@ -14,28 +14,19 @@ module Oxidized
         raise NoConfig, 'no output http config, edit ~/.config/oxidized/config'
       end
     end
+
     require "net/http"
     require "uri"
     require "json"
+
     def store node, outputs, opt = {}
       @commitref = nil
-      json = JSON.pretty_generate(
-        {
-          'msg' => opt[:msg],
-          'user' => opt[:user],
-          'email' => opt[:email],
-          'group' => opt[:group],
-          'node' => node,
-          'config' => outputs.to_cfg,
-          # actually we need to also iterate outputs, for other types like in gitlab. But most people don't use 'type' functionality.
-        }
-      )
       uri = URI.parse @cfg.url
       http = Net::HTTP.new uri.host, uri.port
       # http.use_ssl = true if uri.scheme = 'https'
       req = Net::HTTP::Post.new(uri.request_uri, initheader = { 'Content-Type' => 'application/json' })
       req.basic_auth @cfg.user, @cfg.password
-      req.body = json
+      req.body = generate_json(node, outputs, opt)
       response = http.request req
 
       case response.code.to_i
@@ -50,5 +41,21 @@ module Oxidized
         Oxidized.logger.info "Configuration http backup for #{node} failed status: #{response.body}"
       end
     end
+
+    private
+
+    def generate_json node, outputs, opt
+      JSON.pretty_generate(
+        {
+          'msg'    => opt[:msg],
+          'user'   => opt[:user],
+          'email'  => opt[:email],
+          'group'  => opt[:group],
+          'node'   => node,
+          'config' => outputs.to_cfg,
+          # actually we need to also iterate outputs, for other types like in gitlab. But most people don't use 'type' functionality.
+        }
+      )
+    end
   end
 end
diff --git a/lib/oxidized/source/csv.rb b/lib/oxidized/source/csv.rb
index 7b771f6..769b1f7 100644
--- a/lib/oxidized/source/csv.rb
+++ b/lib/oxidized/source/csv.rb
@@ -20,14 +20,7 @@ module Oxidized
 
     def load _node_want = nil
       nodes = []
-      file = File.expand_path(@cfg.file)
-      file = if @cfg.gpg?
-               crypto = GPGME::Crypto.new password: @cfg.gpg_password
-               file = crypto.decrypt(File.open(file)).to_s
-             else
-               open(file)
-             end
-      file.each_line do |line|
+      get_file.each_line do |line|
         next if line.match(/^\s*#/)
         data = line.chomp.split(@cfg.delimiter, -1)
         next if data.empty?
@@ -49,5 +42,17 @@ module Oxidized
       end
       nodes
     end
+
+    private
+
+    def get_file
+      file = File.expand_path(@cfg.file)
+      if @cfg.gpg?
+        crypto = GPGME::Crypto.new password: @cfg.gpg_password
+        crypto.decrypt(File.open(file)).to_s
+      else
+        open(file)
+      end
+    end
   end
 end
diff --git a/lib/oxidized/source/http.rb b/lib/oxidized/source/http.rb
index bf1e74c..36fa764 100644
--- a/lib/oxidized/source/http.rb
+++ b/lib/oxidized/source/http.rb
@@ -17,28 +17,7 @@ module Oxidized
 
     def load node_want = nil
       nodes = []
-      uri = URI.parse(@cfg.url)
-      http = Net::HTTP.new(uri.host, uri.port)
-      http.use_ssl = true if uri.scheme == 'https'
-      http.verify_mode = OpenSSL::SSL::VERIFY_NONE unless @cfg.secure
-
-      # map headers
-      headers = {}
-      @cfg.headers.each do |header, value|
-        headers[header] = value
-      end
-
-      req_uri = uri.request_uri
-      if node_want
-        req_uri = "#{req_uri}/#{node_want}"
-      end
-      request = Net::HTTP::Get.new(req_uri, headers)
-      if (@cfg.user? && @cfg.pass?)
-        request.basic_auth(@cfg.user, @cfg.pass)
-      end
-
-      response = http.request(request)
-      data = JSON.parse(response.body)
+      data = JSON.parse(get_http)
       data = string_navigate(data, @cfg.hosts_location) if @cfg.hosts_location?
       data.each do |node|
         next if node.empty?
@@ -72,5 +51,28 @@ module Oxidized
       end
       object
     end
+
+    def get_http
+      uri = URI.parse(@cfg.url)
+      http = Net::HTTP.new(uri.host, uri.port)
+      http.use_ssl = true if uri.scheme == 'https'
+      http.verify_mode = OpenSSL::SSL::VERIFY_NONE unless @cfg.secure
+
+      # map headers
+      headers = {}
+      @cfg.headers.each do |header, value|
+        headers[header] = value
+      end
+
+      req_uri = uri.request_uri
+      if node_want
+        req_uri = "#{req_uri}/#{node_want}"
+      end
+      request = Net::HTTP::Get.new(req_uri, headers)
+      if (@cfg.user? && @cfg.pass?)
+        request.basic_auth(@cfg.user, @cfg.pass)
+      end
+      http.request(request).body
+    end
   end
 end
diff --git a/lib/oxidized/worker.rb b/lib/oxidized/worker.rb
index 74b046d..06e8689 100644
--- a/lib/oxidized/worker.rb
+++ b/lib/oxidized/worker.rb
@@ -36,48 +36,17 @@ module Oxidized
       Oxidized.logger.debug("lib/oxidized/worker.rb: #{@jobs.size} jobs running in parallel") unless @jobs.empty?
     end
 
+
     def process job
       node = job.node
       node.last = job
       node.stats.add job
       @jobs.duration job.time
       node.running = false
-
       if job.status == :success
-        @jobs_done += 1 # needed for :nodes_done hook
-        Oxidized.Hooks.handle :node_success, :node => node,
-                                             :job => job
-        msg = "update #{node.name}"
-        msg += " from #{node.from}" if node.from
-        msg += " with message '#{node.msg}'" if node.msg
-        output = node.output.new
-        if output.store node.name, job.config,
-                        :msg => msg, :email => node.email, :user => node.user, :group => node.group
-          node.modified
-          Oxidized.logger.info "Configuration updated for #{node.group}/#{node.name}"
-          Oxidized.Hooks.handle :post_store, :node => node,
-                                             :job => job,
-                                             :commitref => output.commitref
-        end
-        node.reset
+        process_success node, job
       else
-        msg = "#{node.name} status #{job.status}"
-        if node.retry < Oxidized.config.retries
-          node.retry += 1
-          msg += ", retry attempt #{node.retry}"
-          @nodes.next node.name
-        else
-          # Only increment the @jobs_done when we give up retries for a node (or success).
-          # As it would otherwise cause @jobs_done to be incremented with generic retries.
-          # This would cause :nodes_done hook to desync from running at the end of the nodelist and
-          # be fired when the @jobs_done > @nodes.count (could be mid-cycle on the next cycle).
-          @jobs_done += 1
-          msg += ", retries exhausted, giving up"
-          node.retry = 0
-          Oxidized.Hooks.handle :node_fail, :node => node,
-                                            :job => job
-        end
-        Oxidized.logger.warn msg
+        process_failure node, job
       end
     rescue NodeNotFound
       Oxidized.logger.warn "#{node.name} not found, removed while collecting?"
@@ -85,6 +54,45 @@ module Oxidized
 
     private
 
+    def process_success node, job
+      @jobs_done += 1 # needed for :nodes_done hook
+      Oxidized.Hooks.handle :node_success, node: node,
+                                           job: job
+      msg = "update #{node.name}"
+      msg += " from #{node.from}" if node.from
+      msg += " with message '#{node.msg}'" if node.msg
+      output = node.output.new
+      if output.store node.name, job.config,
+                      msg: msg, email: node.email, user: node.user, group: node.group
+        node.modified
+        Oxidized.logger.info "Configuration updated for #{node.group}/#{node.name}"
+        Oxidized.Hooks.handle :post_store, node: node,
+                                           job: job,
+                                           commitref: output.commitref
+      end
+      node.reset
+    end
+
+    def process_failure node, job
+      msg = "#{node.name} status #{job.status}"
+      if node.retry < Oxidized.config.retries
+        node.retry += 1
+        msg += ", retry attempt #{node.retry}"
+        @nodes.next node.name
+      else
+        # Only increment the @jobs_done when we give up retries for a node (or success).
+        # As it would otherwise cause @jobs_done to be incremented with generic retries.
+        # This would cause :nodes_done hook to desync from running at the end of the nodelist and
+        # be fired when the @jobs_done > @nodes.count (could be mid-cycle on the next cycle).
+        @jobs_done += 1
+        msg += ", retries exhausted, giving up"
+        node.retry = 0
+        Oxidized.Hooks.handle :node_fail, :node => node,
+                                          :job => job
+      end
+      Oxidized.logger.warn msg
+    end
+
     def is_cycle_finished?
       if @jobs_done > @nodes.count
         true