more reek cleanup

Rakefile

@@ -43,5 +43,5 @@ Rake::RDocTask.new do |rdoc|
 end
 
 task :reek do
-  system('reek lib/*')
+  system('reek -c config/config.reek lib/*')
 end

config/config.reek

@@ -0,0 +1,4 @@
+---
+FeatureEnvy:
+  exclude:
+    - Apache::Logging#handle_log

lib/apache.rb

@@ -1,2 +1 @@
 require 'apache/config'
-require 'apache/apachify'

lib/apache/apachify.rb

@@ -36,6 +36,17 @@ class String
   end
 
   alias :blockify :quoteize
+
+  def headerize
+    "#{self.quoteize}"
+  end
+
+  def replace_placeholderize(opts)
+    self.gsub(%r{%\{([^\}]+)\}}) do |match|
+      key = $1.downcase.to_sym
+      opts[key] || ''
+    end
+  end
 end
 
 # Ruby symbols
@@ -56,6 +67,10 @@ class Symbol
   def blockify
     self.to_s
   end
+
+  def headerize
+    "#{self.quoteize}"
+  end
 end
 
 # Ruby everything
@@ -84,4 +99,44 @@ class Array
   end
 
   alias :commentize :to_a
+
+  def headerize
+    "#{self.first.quoteize} #{self.last}"
+  end
+
+  def rewrite_cond_optionify
+    self.collect do |opt|
+      {
+        :or => 'OR',
+        :case_insensitive => 'NC',
+        :no_vary => 'NV'
+      }[opt]
+    end
+  end
+
+  def rewrite_option_listify
+    (!self.empty?) ? "[#{self * ','}]" : nil
+  end
+end
+
+# Ruby hashes
+class Hash
+  REWRITE_RULE_CONDITIONS = {
+    :last => 'L',
+    :forbidden => 'F',
+    :no_escape => 'NE',
+    :redirect => lambda { |val| val == true ? 'R' : "R=#{val}" },
+    :pass_through => 'PT',
+    :preserve_query_string => 'QSA',
+    :query_string_append => 'QSA',
+    :env => lambda { |val| "E=#{val}" }
+  }
+
+  def rewrite_rule_optionify
+    self.collect do |key, value|
+      what = REWRITE_RULE_CONDITIONS[key]
+      what = what.call(value) if what.kind_of? Proc
+      what
+    end.compact.sort
+  end
 end

lib/apache/logging.rb

@@ -19,11 +19,16 @@ module Apache
     [ :custom, :error, :script, :rewrite ].each do |type|
       class_eval <<-EOT
         def #{type}_log(*opts)
-          handle_log '#{type.to_s.capitalize}Log', opts.first, opts.first.quoteize, opts[1..-1]
+          handle_log :tag => '#{type.to_s.capitalize}Log',
+                     :path => opts.first,
+                     :additional_options => opts[1..-1]
         end
 
         def rotate_#{type}_log(*opts)
-          handle_log '#{type.to_s.capitalize}Log', opts.first, rotatelogs(*opts[0..1]).quoteize, opts[2..-1]
+          handle_log :tag => '#{type.to_s.capitalize}Log',
+                     :path => opts.first,
+                     :real_path => rotatelogs(*opts[0..1]),
+                     :additional_options => opts[2..-1]
         end
       EOT
     end
@@ -37,9 +42,12 @@ module Apache
     end
 
     private
-      def handle_log(tag, path, real_path, *opts)
+      def handle_log(info)
-        writable? path
+        writable? (path = info[:path])
-        self << "#{tag} #{[real_path, opts].flatten * " "}"
+
+        real_path = (info[:real_path] || path).quoteize
+
+        self << "#{info[:tag]} #{[real_path, info[:additional_options]].flatten * " "}"
       end
   end
 end

lib/apache/master.rb

@@ -23,9 +23,9 @@ module Apache
     #  runner('www', 'www-data') #=>
     #    User www
     #    Group www-data
-    def runner(user, group = nil)
+    def runner(user, group)
       user! user
-      group! group if group
+      group! group
     end
 
     # Enable Passenger on this server
@@ -94,14 +94,7 @@ module Apache
     #  Header set "Content-dispoaition" "attachment" env=only-for-downloads
     def set_header(hash)
       hash.each do |key, value|
-        output = "Header set #{key.quoteize}"
+        self << "Header set #{key.quoteize} #{value.headerize}"
-        case value
-          when String, Symbol
-            output += " #{value.quoteize}"
-          when Array
-            output += " #{value.first.quoteize} #{value.last}"
-        end
-        self << output
       end
     end
 

lib/apache/rewrites.rb

@@ -170,23 +170,11 @@ module Apache
   module RegularExpressionMatcher
     # Test this rewritable thing
     def test(from, opts = {})
-      from = from.gsub(@from, @to.gsub(/\$([0-9])/) { |match| '\\' + $1 })
+      from.gsub(@from, @to.gsub(/\$([0-9])/) { |match| '\\' + $1 }).replace_placeholderize(opts)
-      replace_placeholders(from, opts)
     end
 
     def match?(from, opts = {})
-      replace_placeholders(from, opts)[@from]
+      from.replace_placeholderize(opts)[@from]
-    end
-
-    # Replace the placeholders in this rewritable thing
-    def replace_placeholders(string, opts)
-      opts.each do |opt, value|
-        case value
-          when String
-            string = string.gsub('%{' + opt.to_s.upcase + '}', value)
-        end
-      end
-      string.gsub(%r{%\{[^\}]+\}}, '')
     end
   end
 
@@ -203,6 +191,8 @@ module Apache
     end
 
     def rule(from, to)
+      raise "from must be a Regexp" if (!from.kind_of?(Regexp) && require_regexp?)
+
       @from = from
       @to = to
     end
@@ -217,6 +207,7 @@ module Apache
 
     def stop_if_match?; false; end
     def forbidden?; false; end
+    def require_regexp?; false; end
   end
 
   # A RewriteRule definition
@@ -235,33 +226,23 @@ module Apache
     # Define the rule, passing in additional options
     #
     # rule %r{^/here}, '/there', { :last => true, :preserve_query_string => true }
+    #
+    # Options for the options hash are:
+    # * :last => true #=> [L]
+    # * :forbidden => true #=> [F]
+    # * :no_escape => true #=> [NE]
+    # * :redirect => true #=> [R]
+    # * :redirect => 302 #=> [R=302]
+    # * :pass_through => true #=> [PT]
+    # * :preserve_query_string => true #=> [QSA]
+    # * :query_string_append => true #=> [QSA]
+    # * :env => 'what' #=>  [E=what]
     def rule(from, to, options = {})
       super(from, to)
 
-      raise "from must be a Regexp" if !from.kind_of?(Regexp)
-
       @input_options = options
 
-      options = options.collect do |key, value|
+      @options = options.rewrite_rule_optionify.rewrite_option_listify
-        case key
-          when :last
-            'L'
-          when :forbidden
-            'F'
-          when :no_escape
-            'NE'
-          when :redirect
-            (value == true) ? 'R' : "R=#{value}"
-          when :pass_through
-            'PT'
-          when :preserve_query_string, :query_string_append
-            'QSA'
-          when :env
-            "E=#{value}"
-        end
-      end.compact.sort
-
-      @options = !options.empty? ? "[#{options * ','}]" : nil
     end
 
     # Add a RewriteCondition to this RewriteRule
@@ -287,22 +268,17 @@ module Apache
 
       result = super(from, opts) if match?(from, opts)
 
-      replace_placeholders(result, opts)
+      result.replace_placeholderize(opts)
     end
 
     def match?(from, opts = {})
       opts[:request_uri] = from
-      ok = true
 
-      @conditions.each do |c|
+      @conditions.each do |cond|
-        ok = false if !c.test(from, opts)
+        return false if !cond.test(from, opts)
       end
 
-      if ok
       super(from, opts)
-      else
-        false
-      end
     end
 
     def stop_if_match?
@@ -312,25 +288,32 @@ module Apache
     def forbidden?
       @input_options[:forbidden]
     end
+
+    def require_regexp?; true; end
   end
 
   # A permanent RedirectMatch
   class RedirectMatchPermanent < MatchableThing
     include RegularExpressionMatcher
 
+    # The Apache directive for this object.
     def tag; 'RedirectMatch permanent'; end
 
+    # Define a RedirectMatch rule.
     def rule(from, to)
       super(from, to)
 
       raise "from must be a Regexp" if !from.kind_of?(Regexp)
     end
 
+    # Convert this tag to a String.
     def to_s
       "#{tag} #{[@from.source, @to].quoteize.compact.flatten * " "}"
     end
 
+    # Stop rewrite testing if this object matches.
     def stop_if_match; true; end
+    def require_regexp?; true; end
   end
 
   # A RewriteCond
@@ -343,30 +326,26 @@ module Apache
     #
     #  rule "%{REQUEST_FILENAME}", "^/here", :case_insensitive #=>
     #    RewriteCond "%{REQUEST_FILENAME}" "^/here" [NC]
+    #
+    # Additional parameters can include the following:
+    # * :or #=> [OR]
+    # * :case_insensitive #=> [NC]
+    # * :no_vary #=> [NV]
     def rule(from, to, *opts)
       super(from, to)
 
-      options = opts.collect do |opt|
+      @options = opts.rewrite_cond_optionify.rewrite_option_listify
-        case opt
-          when :or
-            'OR'
-          when :case_insensitive
-            'NC'
-          when :no_vary
-            'NV'
-        end
-      end
-
-      @options = (!options.empty?) ? "[#{options * ','}]" : nil
     end
 
     alias :cond :rule
 
+    # Create a new RewriteCond
     def initialize
       super
       @options = nil
     end
 
+    # Convert this tag to a String.
     def to_s
       "#{tag} #{[@from.quoteize, @to.quoteize, @options].compact.flatten * " "}"
     end
@@ -374,7 +353,7 @@ module Apache
     # Test this RewriteCond
     def test(from, opts = {})
       super(from, opts)
-      source = replace_placeholders(@from, opts)
+      source = @from.replace_placeholderize(opts)
 
       to = @to
       reverse = false