From 487c503b2185617485520b6fabe41f3e15ddb903 Mon Sep 17 00:00:00 2001 From: John Bintz Date: Tue, 18 May 2010 17:43:23 -0400 Subject: [PATCH] more reek cleanup --- Rakefile | 2 +- config/config.reek | 4 ++ lib/apache.rb | 1 - lib/apache/apachify.rb | 55 ++++++++++++++++++++++++ lib/apache/logging.rb | 18 +++++--- lib/apache/master.rb | 13 ++---- lib/apache/rewrites.rb | 95 ++++++++++++++++-------------------------- 7 files changed, 113 insertions(+), 75 deletions(-) create mode 100644 config/config.reek diff --git a/Rakefile b/Rakefile index 27928d4..3414fbd 100644 --- a/Rakefile +++ b/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 diff --git a/config/config.reek b/config/config.reek new file mode 100644 index 0000000..06694b2 --- /dev/null +++ b/config/config.reek @@ -0,0 +1,4 @@ +--- +FeatureEnvy: + exclude: + - Apache::Logging#handle_log diff --git a/lib/apache.rb b/lib/apache.rb index 03ebe1e..d49c9b4 100644 --- a/lib/apache.rb +++ b/lib/apache.rb @@ -1,2 +1 @@ require 'apache/config' -require 'apache/apachify' diff --git a/lib/apache/apachify.rb b/lib/apache/apachify.rb index 454a085..2493736 100644 --- a/lib/apache/apachify.rb +++ b/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 diff --git a/lib/apache/logging.rb b/lib/apache/logging.rb index 932455a..125a1a4 100644 --- a/lib/apache/logging.rb +++ b/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) - writable? path - self << "#{tag} #{[real_path, opts].flatten * " "}" + def handle_log(info) + writable? (path = info[:path]) + + real_path = (info[:real_path] || path).quoteize + + self << "#{info[:tag]} #{[real_path, info[:additional_options]].flatten * " "}" end end end diff --git a/lib/apache/master.rb b/lib/apache/master.rb index 6dd8815..f03d38d 100644 --- a/lib/apache/master.rb +++ b/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}" - case value - when String, Symbol - output += " #{value.quoteize}" - when Array - output += " #{value.first.quoteize} #{value.last}" - end - self << output + self << "Header set #{key.quoteize} #{value.headerize}" end end diff --git a/lib/apache/rewrites.rb b/lib/apache/rewrites.rb index 1e610e5..872c5ae 100644 --- a/lib/apache/rewrites.rb +++ b/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 }) - replace_placeholders(from, opts) + from.gsub(@from, @to.gsub(/\$([0-9])/) { |match| '\\' + $1 }).replace_placeholderize(opts) end def match?(from, opts = {}) - replace_placeholders(from, 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{%\{[^\}]+\}}, '') + from.replace_placeholderize(opts)[@from] 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| - 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 + @options = options.rewrite_rule_optionify.rewrite_option_listify 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| - ok = false if !c.test(from, opts) + @conditions.each do |cond| + return false if !cond.test(from, opts) end - if ok - super(from, opts) - else - false - end + super(from, opts) 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| - case opt - when :or - 'OR' - when :case_insensitive - 'NC' - when :no_vary - 'NV' - end - end - - @options = (!options.empty?) ? "[#{options * ','}]" : nil + @options = opts.rewrite_cond_optionify.rewrite_option_listify 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