From 6c70c02c83b3a03c88df4286130be8882f6d201c Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 7 Aug 2008 23:31:43 -0700 Subject: Freeze memoized results when instance is frozen instead of immediately so you can memoize mutable objects --- activesupport/lib/active_support/memoizable.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/activesupport/lib/active_support/memoizable.rb b/activesupport/lib/active_support/memoizable.rb index e6049d9496..b5adc2330c 100644 --- a/activesupport/lib/active_support/memoizable.rb +++ b/activesupport/lib/active_support/memoizable.rb @@ -14,7 +14,7 @@ module ActiveSupport methods.each do |method| if method.to_s =~ /^_unmemoized_(.*)/ begin - __send__($1) + __send__($1).freeze rescue ArgumentError end end @@ -41,7 +41,7 @@ module ActiveSupport if !reload && defined? #{memoized_ivar} #{memoized_ivar} else - #{memoized_ivar} = #{original_method}.freeze + #{memoized_ivar} = #{original_method} end end else @@ -52,7 +52,7 @@ module ActiveSupport if !reload && #{memoized_ivar} && #{memoized_ivar}.has_key?(args) #{memoized_ivar}[args] else - #{memoized_ivar}[args] = #{original_method}(*args).freeze + #{memoized_ivar}[args] = #{original_method}(*args) end end end -- cgit v1.2.3 From e43d1c226d09afe49b25f5e3a351c4c10371933a Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 7 Aug 2008 23:34:36 -0700 Subject: Inherit these from AbstractRequest instead --- actionpack/lib/action_controller/rack_process.rb | 50 ++---------------------- 1 file changed, 4 insertions(+), 46 deletions(-) diff --git a/actionpack/lib/action_controller/rack_process.rb b/actionpack/lib/action_controller/rack_process.rb index 7e0a6b091e..aca38bde3d 100644 --- a/actionpack/lib/action_controller/rack_process.rb +++ b/actionpack/lib/action_controller/rack_process.rb @@ -3,7 +3,7 @@ require 'action_controller/session/cookie_store' module ActionController #:nodoc: class RackRequest < AbstractRequest #:nodoc: - attr_accessor :env, :session_options + attr_accessor :session_options attr_reader :cgi class SessionFixationAttempt < StandardError #:nodoc: @@ -15,7 +15,7 @@ module ActionController #:nodoc: :session_path => "/", # available to all paths in app :session_key => "_session_id", :cookie_only => true - } unless const_defined?(:DEFAULT_SESSION_OPTIONS) + } def initialize(env, session_options = DEFAULT_SESSION_OPTIONS) @session_options = session_options @@ -37,28 +37,14 @@ module ActionController #:nodoc: end end - # The request body is an IO input stream. If the RAW_POST_DATA environment - # variable is already set, wrap it in a StringIO. - def body - if raw_post = env['RAW_POST_DATA'] - StringIO.new(raw_post) - else - @env['rack.input'] - end + def body_stream #:nodoc: + @env['rack.input'] end def key?(key) @env.key?(key) end - def query_parameters - @query_parameters ||= self.class.parse_query_parameters(query_string) - end - - def request_parameters - @request_parameters ||= parse_formatted_request_parameters - end - def cookies return {} unless @env["HTTP_COOKIE"] @@ -70,34 +56,6 @@ module ActionController #:nodoc: @env["rack.request.cookie_hash"] end - def host_with_port_without_standard_port_handling - if forwarded = @env["HTTP_X_FORWARDED_HOST"] - forwarded.split(/,\s?/).last - elsif http_host = @env['HTTP_HOST'] - http_host - elsif server_name = @env['SERVER_NAME'] - server_name - else - "#{env['SERVER_ADDR']}:#{env['SERVER_PORT']}" - end - end - - def host - host_with_port_without_standard_port_handling.sub(/:\d+$/, '') - end - - def port - if host_with_port_without_standard_port_handling =~ /:(\d+)$/ - $1.to_i - else - standard_port - end - end - - def remote_addr - @env['REMOTE_ADDR'] - end - def server_port @env['SERVER_PORT'].to_i end -- cgit v1.2.3 From b7529ed1cc7cfd8df5fd1b069e2881d39d3d984c Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 7 Aug 2008 23:43:12 -0700 Subject: Simplifying usage of ETags and Last-Modified and conditional GET requests --- actionpack/lib/action_controller/cgi_process.rb | 45 +------ actionpack/lib/action_controller/headers.rb | 30 ++--- actionpack/lib/action_controller/request.rb | 144 +++++++++++++++++------ actionpack/lib/action_controller/response.rb | 54 +++++---- actionpack/lib/action_controller/test_process.rb | 14 +-- actionpack/lib/action_view/renderable.rb | 6 +- actionpack/test/controller/render_test.rb | 11 +- 7 files changed, 169 insertions(+), 135 deletions(-) diff --git a/actionpack/lib/action_controller/cgi_process.rb b/actionpack/lib/action_controller/cgi_process.rb index 8bc5e4c3a7..0ca27b30db 100644 --- a/actionpack/lib/action_controller/cgi_process.rb +++ b/actionpack/lib/action_controller/cgi_process.rb @@ -43,7 +43,7 @@ module ActionController #:nodoc: :session_path => "/", # available to all paths in app :session_key => "_session_id", :cookie_only => true - } unless const_defined?(:DEFAULT_SESSION_OPTIONS) + } def initialize(cgi, session_options = {}) @cgi = cgi @@ -61,53 +61,14 @@ module ActionController #:nodoc: end end - # The request body is an IO input stream. If the RAW_POST_DATA environment - # variable is already set, wrap it in a StringIO. - def body - if raw_post = env['RAW_POST_DATA'] - raw_post.force_encoding(Encoding::BINARY) if raw_post.respond_to?(:force_encoding) - StringIO.new(raw_post) - else - @cgi.stdinput - end - end - - def query_parameters - @query_parameters ||= self.class.parse_query_parameters(query_string) - end - - def request_parameters - @request_parameters ||= parse_formatted_request_parameters + def body_stream #:nodoc: + @cgi.stdinput end def cookies @cgi.cookies.freeze end - def host_with_port_without_standard_port_handling - if forwarded = env["HTTP_X_FORWARDED_HOST"] - forwarded.split(/,\s?/).last - elsif http_host = env['HTTP_HOST'] - http_host - elsif server_name = env['SERVER_NAME'] - server_name - else - "#{env['SERVER_ADDR']}:#{env['SERVER_PORT']}" - end - end - - def host - host_with_port_without_standard_port_handling.sub(/:\d+$/, '') - end - - def port - if host_with_port_without_standard_port_handling =~ /:(\d+)$/ - $1.to_i - else - standard_port - end - end - def session unless defined?(@session) if @session_options == false diff --git a/actionpack/lib/action_controller/headers.rb b/actionpack/lib/action_controller/headers.rb index 7239438c49..139669c66f 100644 --- a/actionpack/lib/action_controller/headers.rb +++ b/actionpack/lib/action_controller/headers.rb @@ -1,31 +1,33 @@ +require 'active_support/memoizable' + module ActionController module Http class Headers < ::Hash - - def initialize(constructor = {}) - if constructor.is_a?(Hash) + extend ActiveSupport::Memoizable + + def initialize(*args) + if args.size == 1 && args[0].is_a?(Hash) super() - update(constructor) + update(args[0]) else - super(constructor) + super end end - + def [](header_name) if include?(header_name) - super + super else - super(normalize_header(header_name)) + super(env_name(header_name)) end end - - + private - # Takes an HTTP header name and returns it in the - # format - def normalize_header(header_name) + # Converts a HTTP header name to an environment variable name. + def env_name(header_name) "HTTP_#{header_name.upcase.gsub(/-/, '_')}" end + memoize :env_name end end -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index c55788a531..3c1521d8b1 100644 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -2,18 +2,22 @@ require 'tempfile' require 'stringio' require 'strscan' -module ActionController - # HTTP methods which are accepted by default. - ACCEPTED_HTTP_METHODS = Set.new(%w( get head put post delete options )) +require 'active_support/memoizable' +module ActionController # CgiRequest and TestRequest provide concrete implementations. class AbstractRequest + extend ActiveSupport::Memoizable + def self.relative_url_root=(*args) ActiveSupport::Deprecation.warn( "ActionController::AbstractRequest.relative_url_root= has been renamed." + "You can now set it with config.action_controller.relative_url_root=", caller) end + HTTP_METHODS = %w(get head put post delete options) + HTTP_METHOD_LOOKUP = HTTP_METHODS.inject({}) { |h, m| h[m] = h[m.upcase] = m.to_sym; h } + # The hash of environment variables for this request, # such as { 'RAILS_ENV' => 'production' }. attr_reader :env @@ -21,15 +25,12 @@ module ActionController # The true HTTP request method as a lowercase symbol, such as :get. # UnknownHttpMethod is raised for invalid methods not listed in ACCEPTED_HTTP_METHODS. def request_method - @request_method ||= begin - method = ((@env['REQUEST_METHOD'] == 'POST' && !parameters[:_method].blank?) ? parameters[:_method].to_s : @env['REQUEST_METHOD']).downcase - if ACCEPTED_HTTP_METHODS.include?(method) - method.to_sym - else - raise UnknownHttpMethod, "#{method}, accepted HTTP methods are #{ACCEPTED_HTTP_METHODS.to_a.to_sentence}" - end - end + method = @env['REQUEST_METHOD'] + method = parameters[:_method] if method == 'POST' && !parameters[:_method].blank? + + HTTP_METHOD_LOOKUP[method] || raise(UnknownHttpMethod, "#{method}, accepted HTTP methods are #{HTTP_METHODS.to_sentence}") end + memoize :request_method # The HTTP request method as a lowercase symbol, such as :get. # Note, HEAD is returned as :get since the two are functionally @@ -67,33 +68,59 @@ module ActionController # Provides access to the request's HTTP headers, for example: # request.headers["Content-Type"] # => "text/plain" def headers - @headers ||= ActionController::Http::Headers.new(@env) + ActionController::Http::Headers.new(@env) end + memoize :headers def content_length - @content_length ||= env['CONTENT_LENGTH'].to_i + @env['CONTENT_LENGTH'].to_i end + memoize :content_length # The MIME type of the HTTP request, such as Mime::XML. # # For backward compatibility, the post format is extracted from the # X-Post-Data-Format HTTP header if present. def content_type - @content_type ||= Mime::Type.lookup(content_type_without_parameters) + Mime::Type.lookup(content_type_without_parameters) end + memoize :content_type # Returns the accepted MIME type for the request def accepts - @accepts ||= - begin - header = @env['HTTP_ACCEPT'].to_s.strip + header = @env['HTTP_ACCEPT'].to_s.strip - if header.empty? - [content_type, Mime::ALL].compact - else - Mime::Type.parse(header) - end - end + if header.empty? + [content_type, Mime::ALL].compact + else + Mime::Type.parse(header) + end + end + memoize :accepts + + def if_modified_since + if since = env['HTTP_IF_MODIFIED_SINCE'] + Time.rfc2822(since) + end + end + memoize :if_modified_since + + def if_none_match + env['HTTP_IF_NONE_MATCH'] + end + + def not_modified?(modified_at) + if_modified_since && modified_at && if_modified_since >= modified_at + end + + def etag_matches?(etag) + if_none_match && if_none_match == etag + end + + # Check response freshness (Last-Modified and ETag) against request + # If-Modified-Since and If-None-Match conditions. + def fresh?(response) + not_modified?(response.last_modified) || etag_matches?(response.etag) end # Returns the Mime type for the format used in the request. @@ -102,7 +129,7 @@ module ActionController # GET /posts/5.xhtml | request.format => Mime::HTML # GET /posts/5 | request.format => Mime::HTML or MIME::JS, or request.accepts.first depending on the value of ActionController::Base.use_accept_header def format - @format ||= begin + @format ||= if parameters[:format] Mime::Type.lookup_by_extension(parameters[:format]) elsif ActionController::Base.use_accept_header @@ -112,7 +139,6 @@ module ActionController else Mime::Type.lookup_by_extension("html") end - end end @@ -200,42 +226,63 @@ EOM @env['REMOTE_ADDR'] end + memoize :remote_ip # Returns the lowercase name of the HTTP server software. def server_software (@env['SERVER_SOFTWARE'] && /^([a-zA-Z]+)/ =~ @env['SERVER_SOFTWARE']) ? $1.downcase : nil end + memoize :server_software # Returns the complete URL used for this request def url protocol + host_with_port + request_uri end + memoize :url # Return 'https://' if this is an SSL request and 'http://' otherwise. def protocol ssl? ? 'https://' : 'http://' end + memoize :protocol # Is this an SSL request? def ssl? @env['HTTPS'] == 'on' || @env['HTTP_X_FORWARDED_PROTO'] == 'https' end + def host_with_port_without_standard_port_handling + if forwarded = env["HTTP_X_FORWARDED_HOST"] + forwarded.split(/,\s?/).last + else + env['HTTP_HOST'] || env['SERVER_NAME'] || "#{env['SERVER_ADDR']}:#{env['SERVER_PORT']}" + end + end + memoize :host_with_port_without_standard_port_handling + # Returns the host for this request, such as example.com. def host + host_with_port_without_standard_port_handling.sub(/:\d+\Z/, '') end + memoize :host # Returns a host:port string for this request, such as example.com or # example.com:8080. def host_with_port - @host_with_port ||= host + port_string + "#{host}#{port_string}" end + memoize :host_with_port # Returns the port number of this request as an integer. def port - @port_as_int ||= @env['SERVER_PORT'].to_i + if host_with_port_without_standard_port_handling =~ /:(\d+)$/ + $1.to_i + else + standard_port + end end + memoize :port # Returns the standard port number for this request's protocol def standard_port @@ -248,7 +295,7 @@ EOM # Returns a port suffix like ":8080" if the port number of this request # is not the default HTTP port 80 or HTTPS port 443. def port_string - (port == standard_port) ? '' : ":#{port}" + ":#{port}" unless port == standard_port end # Returns the domain part of a host, such as rubyonrails.org in "www.rubyonrails.org". You can specify @@ -276,6 +323,7 @@ EOM @env['QUERY_STRING'] || '' end end + memoize :query_string # Return the request URI, accounting for server idiosyncrasies. # WEBrick includes the full URL. IIS leaves REQUEST_URI blank. @@ -300,6 +348,7 @@ EOM end end end + memoize :request_uri # Returns the interpreted path to requested resource after all the installation directory of this application was taken into account def path @@ -345,19 +394,41 @@ EOM @path_parameters ||= {} end + # The request body is an IO input stream. If the RAW_POST_DATA environment + # variable is already set, wrap it in a StringIO. + def body + if raw_post = env['RAW_POST_DATA'] + raw_post.force_encoding(Encoding::BINARY) if raw_post.respond_to?(:force_encoding) + StringIO.new(raw_post) + else + body_stream + end + end - #-- - # Must be implemented in the concrete request - #++ + def remote_addr + @env['REMOTE_ADDR'] + end - # The request body is an IO input stream. - def body + def referrer + @env['HTTP_REFERER'] + end + alias referer referrer + + + def query_parameters + @query_parameters ||= self.class.parse_query_parameters(query_string) end - def query_parameters #:nodoc: + def request_parameters + @request_parameters ||= parse_formatted_request_parameters end - def request_parameters #:nodoc: + + #-- + # Must be implemented in the concrete request + #++ + + def body_stream #:nodoc: end def cookies #:nodoc: @@ -384,8 +455,9 @@ EOM # The raw content type string with its parameters stripped off. def content_type_without_parameters - @content_type_without_parameters ||= self.class.extract_content_type_without_parameters(content_type_with_parameters) + self.class.extract_content_type_without_parameters(content_type_with_parameters) end + memoize :content_type_without_parameters private def content_type_from_legacy_post_data_format_header diff --git a/actionpack/lib/action_controller/response.rb b/actionpack/lib/action_controller/response.rb index da352b6993..a85fad0d39 100644 --- a/actionpack/lib/action_controller/response.rb +++ b/actionpack/lib/action_controller/response.rb @@ -37,12 +37,20 @@ module ActionController # :nodoc: attr_accessor :body # The headers of the response, as a Hash. It maps header names to header values. attr_accessor :headers - attr_accessor :session, :cookies, :assigns, :template, :redirected_to, :redirected_to_method_params, :layout + attr_accessor :session, :cookies, :assigns, :template, :layout + attr_accessor :redirected_to, :redirected_to_method_params def initialize @body, @headers, @session, @assigns = "", DEFAULT_HEADERS.merge("cookie" => []), [], [] end + def status; headers['Status'] end + def status=(status) headers['Status'] = status end + + def location; headers['Location'] end + def location=(url) headers['Location'] = url end + + # Sets the HTTP response's content MIME type. For example, in the controller # you could write this: # @@ -70,35 +78,29 @@ module ActionController # :nodoc: charset.blank? ? nil : charset.strip.split("=")[1] end - def redirect(to_url, response_status) - self.headers["Status"] = response_status - self.headers["Location"] = to_url + def last_modified + Time.rfc2822(headers['Last-Modified']) + end - self.body = "You are being redirected." + def last_modified=(utc_time) + headers['Last-Modified'] = utc_time.httpdate end - def prepare! - handle_conditional_get! - convert_content_type! - set_content_length! + def etag; headers['ETag'] end + def etag=(etag) + headers['ETag'] = %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(etag))}") end - # Sets the Last-Modified response header. Returns whether it's older than - # the If-Modified-Since request header. - def last_modified!(utc_time) - headers['Last-Modified'] ||= utc_time.httpdate - if request && since = request.headers['HTTP_IF_MODIFIED_SINCE'] - utc_time <= Time.rfc2822(since) - end + def redirect(url, status) + self.status = status + self.location = url + self.body = "You are being redirected." end - # Sets the ETag response header. Returns whether it matches the - # If-None-Match request header. - def etag!(tag) - headers['ETag'] ||= %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(tag))}") - if request && request.headers['HTTP_IF_NONE_MATCH'] == headers['ETag'] - true - end + def prepare! + handle_conditional_get! + convert_content_type! + set_content_length! end private @@ -106,15 +108,15 @@ module ActionController # :nodoc: if nonempty_ok_response? set_conditional_cache_control! - if etag!(body) - headers['Status'] = '304 Not Modified' + self.etag ||= body + if request && request.etag_matches?(etag) + self.status = '304 Not Modified' self.body = '' end end end def nonempty_ok_response? - status = headers['Status'] ok = !status || status[0..2] == '200' ok && body.is_a?(String) && !body.empty? end diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index 66675aaa13..42e79fe819 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -23,7 +23,7 @@ module ActionController #:nodoc: class TestRequest < AbstractRequest #:nodoc: attr_accessor :cookies, :session_options - attr_accessor :query_parameters, :request_parameters, :path, :session, :env + attr_accessor :query_parameters, :request_parameters, :path, :session attr_accessor :host, :user_agent def initialize(query_parameters = nil, request_parameters = nil, session = nil) @@ -42,7 +42,7 @@ module ActionController #:nodoc: end # Wraps raw_post in a StringIO. - def body + def body_stream #:nodoc: StringIO.new(raw_post) end @@ -54,7 +54,7 @@ module ActionController #:nodoc: def port=(number) @env["SERVER_PORT"] = number.to_i - @port_as_int = nil + port(true) end def action=(action_name) @@ -83,10 +83,6 @@ module ActionController #:nodoc: @env['REMOTE_ADDR'] = addr end - def remote_addr - @env['REMOTE_ADDR'] - end - def request_uri @request_uri || super end @@ -120,10 +116,6 @@ module ActionController #:nodoc: self.query_parameters = {} self.path_parameters = {} @request_method, @accepts, @content_type = nil, nil, nil - end - - def referer - @env["HTTP_REFERER"] end private diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index 5fe1ca86f3..89ac500717 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -31,10 +31,10 @@ module ActionView view.send(:evaluate_assigns) view.send(:set_controller_content_type, mime_type) if respond_to?(:mime_type) - view.send(:execute, method(local_assigns), local_assigns) + view.send(:execute, method_name(local_assigns), local_assigns) end - def method(local_assigns) + def method_name(local_assigns) if local_assigns && local_assigns.any? local_assigns_keys = "locals_#{local_assigns.keys.map { |k| k.to_s }.sort.join('_')}" end @@ -44,7 +44,7 @@ module ActionView private # Compile and evaluate the template's code (if necessary) def compile(local_assigns) - render_symbol = method(local_assigns) + render_symbol = method_name(local_assigns) @@mutex.synchronize do if recompile?(render_symbol) diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 76832f5713..fd042794fa 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -15,9 +15,14 @@ class TestController < ActionController::Base end def conditional_hello - etag! [:foo, 123] - last_modified! Time.now.utc.beginning_of_day - render :action => 'hello_world' unless performed? + response.last_modified = Time.now.utc.beginning_of_day + response.etag = [:foo, 123] + + if request.fresh?(response) + head :not_modified + else + render :action => 'hello_world' + end end def render_hello_world -- cgit v1.2.3 From c24a7cdd230ee0005add73b9d57bc859ddb57f83 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Fri, 8 Aug 2008 02:29:37 -0700 Subject: Don't shadow host method --- actionpack/lib/action_controller/rack_process.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/rack_process.rb b/actionpack/lib/action_controller/rack_process.rb index aca38bde3d..dcbcf8bc1d 100644 --- a/actionpack/lib/action_controller/rack_process.rb +++ b/actionpack/lib/action_controller/rack_process.rb @@ -30,7 +30,7 @@ module ActionController #:nodoc: SERVER_NAME SERVER_PROTOCOL HTTP_ACCEPT HTTP_ACCEPT_CHARSET HTTP_ACCEPT_ENCODING - HTTP_ACCEPT_LANGUAGE HTTP_CACHE_CONTROL HTTP_FROM HTTP_HOST + HTTP_ACCEPT_LANGUAGE HTTP_CACHE_CONTROL HTTP_FROM HTTP_NEGOTIATE HTTP_PRAGMA HTTP_REFERER HTTP_USER_AGENT ].each do |env| define_method(env.sub(/^HTTP_/n, '').downcase) do @env[env] -- cgit v1.2.3 From ba2d61dd8160237b2141ae24cf20db9b5301eb9d Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Fri, 8 Aug 2008 02:31:12 -0700 Subject: Update tests for request memoization --- actionpack/lib/action_controller/request.rb | 25 ++++--- actionpack/lib/action_controller/test_process.rb | 24 ++++-- actionpack/test/controller/caching_test.rb | 2 +- actionpack/test/controller/cgi_test.rb | 2 +- actionpack/test/controller/content_type_test.rb | 8 +- actionpack/test/controller/mime_responds_test.rb | 62 ++++++++-------- actionpack/test/controller/rack_test.rb | 23 +++--- actionpack/test/controller/render_test.rb | 26 +++---- actionpack/test/controller/request_test.rb | 95 +++++++++++++----------- 9 files changed, 146 insertions(+), 121 deletions(-) diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 3c1521d8b1..90bced14e6 100644 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -252,18 +252,17 @@ EOM @env['HTTPS'] == 'on' || @env['HTTP_X_FORWARDED_PROTO'] == 'https' end - def host_with_port_without_standard_port_handling + def raw_host_with_port if forwarded = env["HTTP_X_FORWARDED_HOST"] forwarded.split(/,\s?/).last else env['HTTP_HOST'] || env['SERVER_NAME'] || "#{env['SERVER_ADDR']}:#{env['SERVER_PORT']}" end end - memoize :host_with_port_without_standard_port_handling # Returns the host for this request, such as example.com. def host - host_with_port_without_standard_port_handling.sub(/:\d+\Z/, '') + raw_host_with_port.sub(/:\d+$/, '') end memoize :host @@ -276,7 +275,7 @@ EOM # Returns the port number of this request as an integer. def port - if host_with_port_without_standard_port_handling =~ /:(\d+)$/ + if raw_host_with_port =~ /:(\d+)$/ $1.to_i else standard_port @@ -295,7 +294,7 @@ EOM # Returns a port suffix like ":8080" if the port number of this request # is not the default HTTP port 80 or HTTPS port 443. def port_string - ":#{port}" unless port == standard_port + port == standard_port ? '' : ":#{port}" end # Returns the domain part of a host, such as rubyonrails.org in "www.rubyonrails.org". You can specify @@ -333,16 +332,17 @@ EOM (%r{^\w+\://[^/]+(/.*|$)$} =~ uri) ? $1 : uri else # Construct IIS missing REQUEST_URI from SCRIPT_NAME and PATH_INFO. - script_filename = @env['SCRIPT_NAME'].to_s.match(%r{[^/]+$}) - uri = @env['PATH_INFO'] - uri = uri.sub(/#{script_filename}\//, '') unless script_filename.nil? - unless (env_qs = @env['QUERY_STRING']).nil? || env_qs.empty? - uri << '?' << env_qs + uri = @env['PATH_INFO'].to_s + + if script_filename = @env['SCRIPT_NAME'].to_s.match(%r{[^/]+$}) + uri = uri.sub(/#{script_filename}\//, '') end - if uri.nil? + env_qs = @env['QUERY_STRING'].to_s + uri += "?#{env_qs}" unless env_qs.empty? + + if uri.blank? @env.delete('REQUEST_URI') - uri else @env['REQUEST_URI'] = uri end @@ -358,6 +358,7 @@ EOM path.sub!(%r/^#{ActionController::Base.relative_url_root}/, '') path || '' end + memoize :path # Read the request body. This is useful for web services that need to # work with raw requests directly. diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index 42e79fe819..636e1c38b3 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -68,6 +68,8 @@ module ActionController #:nodoc: @env["REQUEST_URI"] = value @request_uri = nil @path = nil + request_uri(true) + path(true) end def request_uri=(uri) @@ -77,17 +79,26 @@ module ActionController #:nodoc: def accept=(mime_types) @env["HTTP_ACCEPT"] = Array(mime_types).collect { |mime_types| mime_types.to_s }.join(",") + accepts(true) + end + + def if_modified_since=(last_modified) + @env["HTTP_IF_MODIFIED_SINCE"] = last_modified + end + + def if_none_match=(etag) + @env["HTTP_IF_NONE_MATCH"] = etag end def remote_addr=(addr) @env['REMOTE_ADDR'] = addr end - def request_uri + def request_uri(*args) @request_uri || super end - def path + def path(*args) @path || super end @@ -440,10 +451,13 @@ module ActionController #:nodoc: end def method_missing(selector, *args) - return @controller.send!(selector, *args) if ActionController::Routing::Routes.named_routes.helpers.include?(selector) - return super + if ActionController::Routing::Routes.named_routes.helpers.include?(selector) + @controller.send(selector, *args) + else + super + end end - + # Shortcut for ActionController::TestUploadedFile.new(Test::Unit::TestCase.fixture_path + path, type): # # post :change_avatar, :avatar => fixture_file_upload('/files/spongebob.png', 'image/png') diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 47a0fcf99d..b6cdd116e5 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -109,7 +109,7 @@ class PageCachingTest < Test::Unit::TestCase uses_mocha("should_cache_ok_at_custom_path") do def test_should_cache_ok_at_custom_path - @request.expects(:path).returns("/index.html") + @request.stubs(:path).returns("/index.html") get :ok assert_response :ok assert File.exist?("#{FILE_STORE_PATH}/index.html") diff --git a/actionpack/test/controller/cgi_test.rb b/actionpack/test/controller/cgi_test.rb index 8ca70f8595..813171857a 100644 --- a/actionpack/test/controller/cgi_test.rb +++ b/actionpack/test/controller/cgi_test.rb @@ -75,7 +75,7 @@ class CgiRequestTest < BaseCgiTest assert_equal "rubyonrails.org:8080", @request.host_with_port @request_hash['HTTP_X_FORWARDED_HOST'] = "www.firsthost.org, www.secondhost.org" - assert_equal "www.secondhost.org", @request.host + assert_equal "www.secondhost.org", @request.host(true) end def test_http_host_with_default_port_overrides_server_port diff --git a/actionpack/test/controller/content_type_test.rb b/actionpack/test/controller/content_type_test.rb index d457d13aef..e1bc46bb56 100644 --- a/actionpack/test/controller/content_type_test.rb +++ b/actionpack/test/controller/content_type_test.rb @@ -128,23 +128,23 @@ class AcceptBasedContentTypeTest < ActionController::TestCase def test_render_default_content_types_for_respond_to - @request.env["HTTP_ACCEPT"] = Mime::HTML.to_s + @request.accept = Mime::HTML.to_s get :render_default_content_types_for_respond_to assert_equal Mime::HTML, @response.content_type - @request.env["HTTP_ACCEPT"] = Mime::JS.to_s + @request.accept = Mime::JS.to_s get :render_default_content_types_for_respond_to assert_equal Mime::JS, @response.content_type end def test_render_default_content_types_for_respond_to_with_template - @request.env["HTTP_ACCEPT"] = Mime::XML.to_s + @request.accept = Mime::XML.to_s get :render_default_content_types_for_respond_to assert_equal Mime::XML, @response.content_type end def test_render_default_content_types_for_respond_to_with_overwrite - @request.env["HTTP_ACCEPT"] = Mime::RSS.to_s + @request.accept = Mime::RSS.to_s get :render_default_content_types_for_respond_to assert_equal Mime::XML, @response.content_type end diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 1701431858..0d508eb8df 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -177,7 +177,7 @@ class MimeControllerTest < Test::Unit::TestCase end def test_html - @request.env["HTTP_ACCEPT"] = "text/html" + @request.accept = "text/html" get :js_or_html assert_equal 'HTML', @response.body @@ -189,7 +189,7 @@ class MimeControllerTest < Test::Unit::TestCase end def test_all - @request.env["HTTP_ACCEPT"] = "*/*" + @request.accept = "*/*" get :js_or_html assert_equal 'HTML', @response.body # js is not part of all @@ -201,13 +201,13 @@ class MimeControllerTest < Test::Unit::TestCase end def test_xml - @request.env["HTTP_ACCEPT"] = "application/xml" + @request.accept = "application/xml" get :html_xml_or_rss assert_equal 'XML', @response.body end def test_js_or_html - @request.env["HTTP_ACCEPT"] = "text/javascript, text/html" + @request.accept = "text/javascript, text/html" get :js_or_html assert_equal 'JS', @response.body @@ -232,7 +232,7 @@ class MimeControllerTest < Test::Unit::TestCase 'JSON' => %w(application/json text/x-json) }.each do |body, content_types| content_types.each do |content_type| - @request.env['HTTP_ACCEPT'] = content_type + @request.accept = content_type get :json_or_yaml assert_equal body, @response.body end @@ -240,7 +240,7 @@ class MimeControllerTest < Test::Unit::TestCase end def test_js_or_anything - @request.env["HTTP_ACCEPT"] = "text/javascript, */*" + @request.accept = "text/javascript, */*" get :js_or_html assert_equal 'JS', @response.body @@ -252,34 +252,34 @@ class MimeControllerTest < Test::Unit::TestCase end def test_using_defaults - @request.env["HTTP_ACCEPT"] = "*/*" + @request.accept = "*/*" get :using_defaults assert_equal "text/html", @response.content_type assert_equal 'Hello world!', @response.body - @request.env["HTTP_ACCEPT"] = "text/javascript" + @request.accept = "text/javascript" get :using_defaults assert_equal "text/javascript", @response.content_type assert_equal '$("body").visualEffect("highlight");', @response.body - @request.env["HTTP_ACCEPT"] = "application/xml" + @request.accept = "application/xml" get :using_defaults assert_equal "application/xml", @response.content_type assert_equal "

Hello world!

\n", @response.body end def test_using_defaults_with_type_list - @request.env["HTTP_ACCEPT"] = "*/*" + @request.accept = "*/*" get :using_defaults_with_type_list assert_equal "text/html", @response.content_type assert_equal 'Hello world!', @response.body - @request.env["HTTP_ACCEPT"] = "text/javascript" + @request.accept = "text/javascript" get :using_defaults_with_type_list assert_equal "text/javascript", @response.content_type assert_equal '$("body").visualEffect("highlight");', @response.body - @request.env["HTTP_ACCEPT"] = "application/xml" + @request.accept = "application/xml" get :using_defaults_with_type_list assert_equal "application/xml", @response.content_type assert_equal "

Hello world!

\n", @response.body @@ -298,55 +298,55 @@ class MimeControllerTest < Test::Unit::TestCase end def test_synonyms - @request.env["HTTP_ACCEPT"] = "application/javascript" + @request.accept = "application/javascript" get :js_or_html assert_equal 'JS', @response.body - @request.env["HTTP_ACCEPT"] = "application/x-xml" + @request.accept = "application/x-xml" get :html_xml_or_rss assert_equal "XML", @response.body end def test_custom_types - @request.env["HTTP_ACCEPT"] = "application/crazy-xml" + @request.accept = "application/crazy-xml" get :custom_type_handling assert_equal "application/crazy-xml", @response.content_type assert_equal 'Crazy XML', @response.body - @request.env["HTTP_ACCEPT"] = "text/html" + @request.accept = "text/html" get :custom_type_handling assert_equal "text/html", @response.content_type assert_equal 'HTML', @response.body end def test_xhtml_alias - @request.env["HTTP_ACCEPT"] = "application/xhtml+xml,application/xml" + @request.accept = "application/xhtml+xml,application/xml" get :html_or_xml assert_equal 'HTML', @response.body end def test_firefox_simulation - @request.env["HTTP_ACCEPT"] = "text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5" + @request.accept = "text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5" get :html_or_xml assert_equal 'HTML', @response.body end def test_handle_any - @request.env["HTTP_ACCEPT"] = "*/*" + @request.accept = "*/*" get :handle_any assert_equal 'HTML', @response.body - @request.env["HTTP_ACCEPT"] = "text/javascript" + @request.accept = "text/javascript" get :handle_any assert_equal 'Either JS or XML', @response.body - @request.env["HTTP_ACCEPT"] = "text/xml" + @request.accept = "text/xml" get :handle_any assert_equal 'Either JS or XML', @response.body end def test_handle_any_any - @request.env["HTTP_ACCEPT"] = "*/*" + @request.accept = "*/*" get :handle_any_any assert_equal 'HTML', @response.body end @@ -357,31 +357,31 @@ class MimeControllerTest < Test::Unit::TestCase end def test_handle_any_any_explicit_html - @request.env["HTTP_ACCEPT"] = "text/html" + @request.accept = "text/html" get :handle_any_any assert_equal 'HTML', @response.body end def test_handle_any_any_javascript - @request.env["HTTP_ACCEPT"] = "text/javascript" + @request.accept = "text/javascript" get :handle_any_any assert_equal 'Whatever you ask for, I got it', @response.body end def test_handle_any_any_xml - @request.env["HTTP_ACCEPT"] = "text/xml" + @request.accept = "text/xml" get :handle_any_any assert_equal 'Whatever you ask for, I got it', @response.body end def test_rjs_type_skips_layout - @request.env["HTTP_ACCEPT"] = "text/javascript" + @request.accept = "text/javascript" get :all_types_with_layout assert_equal 'RJS for all_types_with_layout', @response.body end def test_html_type_with_layout - @request.env["HTTP_ACCEPT"] = "text/html" + @request.accept = "text/html" get :all_types_with_layout assert_equal '
HTML for all_types_with_layout
', @response.body end @@ -460,7 +460,7 @@ class MimeControllerTest < Test::Unit::TestCase end def test_format_with_custom_response_type_and_request_headers - @request.env["HTTP_ACCEPT"] = "text/iphone" + @request.accept = "text/iphone" get :iphone_with_html_response_type assert_equal '
Hello iPhone future from iPhone!
', @response.body assert_equal "text/html", @response.content_type @@ -470,7 +470,7 @@ class MimeControllerTest < Test::Unit::TestCase get :iphone_with_html_response_type_without_layout assert_equal '
Hello future from Firefox!
', @response.body - @request.env["HTTP_ACCEPT"] = "text/iphone" + @request.accept = "text/iphone" assert_raises(ActionView::MissingTemplate) { get :iphone_with_html_response_type_without_layout } end end @@ -522,7 +522,7 @@ class MimeControllerLayoutsTest < Test::Unit::TestCase get :index assert_equal '
Hello Firefox
', @response.body - @request.env["HTTP_ACCEPT"] = "text/iphone" + @request.accept = "text/iphone" get :index assert_equal 'Hello iPhone', @response.body end @@ -533,7 +533,7 @@ class MimeControllerLayoutsTest < Test::Unit::TestCase get :index assert_equal 'Super Firefox', @response.body - @request.env["HTTP_ACCEPT"] = "text/iphone" + @request.accept = "text/iphone" get :index assert_equal '
Super iPhone
', @response.body end diff --git a/actionpack/test/controller/rack_test.rb b/actionpack/test/controller/rack_test.rb index ab8bbc3bf9..d1650de1fc 100644 --- a/actionpack/test/controller/rack_test.rb +++ b/actionpack/test/controller/rack_test.rb @@ -64,58 +64,61 @@ end class RackRequestTest < BaseRackTest def test_proxy_request - assert_equal 'glu.ttono.us', @request.host_with_port + assert_equal 'glu.ttono.us', @request.host_with_port(true) end def test_http_host @env.delete "HTTP_X_FORWARDED_HOST" @env['HTTP_HOST'] = "rubyonrails.org:8080" - assert_equal "rubyonrails.org:8080", @request.host_with_port + assert_equal "rubyonrails.org", @request.host(true) + assert_equal "rubyonrails.org:8080", @request.host_with_port(true) @env['HTTP_X_FORWARDED_HOST'] = "www.firsthost.org, www.secondhost.org" - assert_equal "www.secondhost.org", @request.host + assert_equal "www.secondhost.org", @request.host(true) end def test_http_host_with_default_port_overrides_server_port @env.delete "HTTP_X_FORWARDED_HOST" @env['HTTP_HOST'] = "rubyonrails.org" - assert_equal "rubyonrails.org", @request.host_with_port + assert_equal "rubyonrails.org", @request.host_with_port(true) end def test_host_with_port_defaults_to_server_name_if_no_host_headers @env.delete "HTTP_X_FORWARDED_HOST" @env.delete "HTTP_HOST" - assert_equal "glu.ttono.us:8007", @request.host_with_port + assert_equal "glu.ttono.us:8007", @request.host_with_port(true) end def test_host_with_port_falls_back_to_server_addr_if_necessary @env.delete "HTTP_X_FORWARDED_HOST" @env.delete "HTTP_HOST" @env.delete "SERVER_NAME" - assert_equal "207.7.108.53:8007", @request.host_with_port + assert_equal "207.7.108.53", @request.host(true) + assert_equal 8007, @request.port(true) + assert_equal "207.7.108.53:8007", @request.host_with_port(true) end def test_host_with_port_if_http_standard_port_is_specified @env['HTTP_X_FORWARDED_HOST'] = "glu.ttono.us:80" - assert_equal "glu.ttono.us", @request.host_with_port + assert_equal "glu.ttono.us", @request.host_with_port(true) end def test_host_with_port_if_https_standard_port_is_specified @env['HTTP_X_FORWARDED_PROTO'] = "https" @env['HTTP_X_FORWARDED_HOST'] = "glu.ttono.us:443" - assert_equal "glu.ttono.us", @request.host_with_port + assert_equal "glu.ttono.us", @request.host_with_port(true) end def test_host_if_ipv6_reference @env.delete "HTTP_X_FORWARDED_HOST" @env['HTTP_HOST'] = "[2001:1234:5678:9abc:def0::dead:beef]" - assert_equal "[2001:1234:5678:9abc:def0::dead:beef]", @request.host + assert_equal "[2001:1234:5678:9abc:def0::dead:beef]", @request.host(true) end def test_host_if_ipv6_reference_with_port @env.delete "HTTP_X_FORWARDED_HOST" @env['HTTP_HOST'] = "[2001:1234:5678:9abc:def0::dead:beef]:8008" - assert_equal "[2001:1234:5678:9abc:def0::dead:beef]", @request.host + assert_equal "[2001:1234:5678:9abc:def0::dead:beef]", @request.host(true) end def test_cgi_environment_variables diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index fd042794fa..1b9b12acc6 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -433,7 +433,7 @@ class RenderTest < Test::Unit::TestCase end def test_should_render_formatted_html_erb_template_with_faulty_accepts_header - @request.env["HTTP_ACCEPT"] = "image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, appliction/x-shockwave-flash, */*" + @request.accept = "image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, appliction/x-shockwave-flash, */*" get :formatted_xml_erb assert_equal 'passed formatted html erb', @response.body end @@ -495,16 +495,16 @@ class EtagRenderTest < Test::Unit::TestCase end def test_render_against_etag_request_should_304_when_match - @request.headers["HTTP_IF_NONE_MATCH"] = etag_for("hello david") + @request.if_none_match = etag_for("hello david") get :render_hello_world_from_variable - assert_equal "304 Not Modified", @response.headers['Status'] + assert_equal "304 Not Modified", @response.status assert @response.body.empty? end def test_render_against_etag_request_should_200_when_no_match - @request.headers["HTTP_IF_NONE_MATCH"] = etag_for("hello somewhere else") + @request.if_none_match = etag_for("hello somewhere else") get :render_hello_world_from_variable - assert_equal "200 OK", @response.headers['Status'] + assert_equal "200 OK", @response.status assert !@response.body.empty? end @@ -513,13 +513,13 @@ class EtagRenderTest < Test::Unit::TestCase expected_etag = etag_for('hello david') assert_equal expected_etag, @response.headers['ETag'] - @request.headers["HTTP_IF_NONE_MATCH"] = expected_etag + @request.if_none_match = expected_etag get :render_hello_world_from_variable - assert_equal "304 Not Modified", @response.headers['Status'] + assert_equal "304 Not Modified", @response.status - @request.headers["HTTP_IF_NONE_MATCH"] = "\"diftag\"" + @request.if_none_match = "\"diftag\"" get :render_hello_world_from_variable - assert_equal "200 OK", @response.headers['Status'] + assert_equal "200 OK", @response.status end def render_with_404_shouldnt_have_etag @@ -562,17 +562,17 @@ class LastModifiedRenderTest < Test::Unit::TestCase end def test_request_not_modified - @request.headers["HTTP_IF_MODIFIED_SINCE"] = @last_modified + @request.if_modified_since = @last_modified get :conditional_hello - assert_equal "304 Not Modified", @response.headers['Status'] + assert_equal "304 Not Modified", @response.status assert @response.body.blank?, @response.body assert_equal @last_modified, @response.headers['Last-Modified'] end def test_request_modified - @request.headers["HTTP_IF_MODIFIED_SINCE"] = 'Thu, 16 Jul 2008 00:00:00 GMT' + @request.if_modified_since = 'Thu, 16 Jul 2008 00:00:00 GMT' get :conditional_hello - assert_equal "200 OK", @response.headers['Status'] + assert_equal "200 OK", @response.status assert !@response.body.blank? assert_equal @last_modified, @response.headers['Last-Modified'] end diff --git a/actionpack/test/controller/request_test.rb b/actionpack/test/controller/request_test.rb index 7db5264840..226c1ac018 100644 --- a/actionpack/test/controller/request_test.rb +++ b/actionpack/test/controller/request_test.rb @@ -15,57 +15,57 @@ class RequestTest < Test::Unit::TestCase assert_equal '0.0.0.0', @request.remote_ip @request.remote_addr = '1.2.3.4' - assert_equal '1.2.3.4', @request.remote_ip + assert_equal '1.2.3.4', @request.remote_ip(true) @request.env['HTTP_CLIENT_IP'] = '2.3.4.5' - assert_equal '1.2.3.4', @request.remote_ip + assert_equal '1.2.3.4', @request.remote_ip(true) @request.remote_addr = '192.168.0.1' - assert_equal '2.3.4.5', @request.remote_ip + assert_equal '2.3.4.5', @request.remote_ip(true) @request.env.delete 'HTTP_CLIENT_IP' @request.remote_addr = '1.2.3.4' @request.env['HTTP_X_FORWARDED_FOR'] = '3.4.5.6' - assert_equal '1.2.3.4', @request.remote_ip + assert_equal '1.2.3.4', @request.remote_ip(true) @request.remote_addr = '127.0.0.1' @request.env['HTTP_X_FORWARDED_FOR'] = '3.4.5.6' - assert_equal '3.4.5.6', @request.remote_ip + assert_equal '3.4.5.6', @request.remote_ip(true) @request.env['HTTP_X_FORWARDED_FOR'] = 'unknown,3.4.5.6' - assert_equal '3.4.5.6', @request.remote_ip + assert_equal '3.4.5.6', @request.remote_ip(true) @request.env['HTTP_X_FORWARDED_FOR'] = '172.16.0.1,3.4.5.6' - assert_equal '3.4.5.6', @request.remote_ip + assert_equal '3.4.5.6', @request.remote_ip(true) @request.env['HTTP_X_FORWARDED_FOR'] = '192.168.0.1,3.4.5.6' - assert_equal '3.4.5.6', @request.remote_ip + assert_equal '3.4.5.6', @request.remote_ip(true) @request.env['HTTP_X_FORWARDED_FOR'] = '10.0.0.1,3.4.5.6' - assert_equal '3.4.5.6', @request.remote_ip + assert_equal '3.4.5.6', @request.remote_ip(true) @request.env['HTTP_X_FORWARDED_FOR'] = '10.0.0.1, 10.0.0.1, 3.4.5.6' - assert_equal '3.4.5.6', @request.remote_ip + assert_equal '3.4.5.6', @request.remote_ip(true) @request.env['HTTP_X_FORWARDED_FOR'] = '127.0.0.1,3.4.5.6' - assert_equal '3.4.5.6', @request.remote_ip + assert_equal '3.4.5.6', @request.remote_ip(true) @request.env['HTTP_X_FORWARDED_FOR'] = 'unknown,192.168.0.1' - assert_equal 'unknown', @request.remote_ip + assert_equal 'unknown', @request.remote_ip(true) @request.env['HTTP_X_FORWARDED_FOR'] = '9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4' - assert_equal '3.4.5.6', @request.remote_ip + assert_equal '3.4.5.6', @request.remote_ip(true) @request.env['HTTP_CLIENT_IP'] = '8.8.8.8' e = assert_raises(ActionController::ActionControllerError) { - @request.remote_ip + @request.remote_ip(true) } assert_match /IP spoofing attack/, e.message assert_match /HTTP_X_FORWARDED_FOR="9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4"/, e.message assert_match /HTTP_CLIENT_IP="8.8.8.8"/, e.message @request.env['HTTP_X_FORWARDED_FOR'] = '8.8.8.8, 9.9.9.9' - assert_equal '8.8.8.8', @request.remote_ip + assert_equal '8.8.8.8', @request.remote_ip(true) @request.env.delete 'HTTP_CLIENT_IP' @request.env.delete 'HTTP_X_FORWARDED_FOR' @@ -168,58 +168,58 @@ class RequestTest < Test::Unit::TestCase ActionController::Base.relative_url_root = nil # The following tests are for when REQUEST_URI is not supplied (as in IIS) - @request.set_REQUEST_URI nil @request.env['PATH_INFO'] = "/path/of/some/uri?mapped=1" @request.env['SCRIPT_NAME'] = nil #"/path/dispatch.rb" + @request.set_REQUEST_URI nil assert_equal "/path/of/some/uri?mapped=1", @request.request_uri assert_equal "/path/of/some/uri", @request.path ActionController::Base.relative_url_root = '/path' - @request.set_REQUEST_URI nil @request.env['PATH_INFO'] = "/path/of/some/uri?mapped=1" @request.env['SCRIPT_NAME'] = "/path/dispatch.rb" - assert_equal "/path/of/some/uri?mapped=1", @request.request_uri - assert_equal "/of/some/uri", @request.path + @request.set_REQUEST_URI nil + assert_equal "/path/of/some/uri?mapped=1", @request.request_uri(true) + assert_equal "/of/some/uri", @request.path(true) ActionController::Base.relative_url_root = nil - @request.set_REQUEST_URI nil @request.env['PATH_INFO'] = "/path/of/some/uri" @request.env['SCRIPT_NAME'] = nil + @request.set_REQUEST_URI nil assert_equal "/path/of/some/uri", @request.request_uri assert_equal "/path/of/some/uri", @request.path - @request.set_REQUEST_URI nil @request.env['PATH_INFO'] = "/" + @request.set_REQUEST_URI nil assert_equal "/", @request.request_uri assert_equal "/", @request.path - @request.set_REQUEST_URI nil @request.env['PATH_INFO'] = "/?m=b" + @request.set_REQUEST_URI nil assert_equal "/?m=b", @request.request_uri assert_equal "/", @request.path - @request.set_REQUEST_URI nil @request.env['PATH_INFO'] = "/" @request.env['SCRIPT_NAME'] = "/dispatch.cgi" + @request.set_REQUEST_URI nil assert_equal "/", @request.request_uri assert_equal "/", @request.path ActionController::Base.relative_url_root = '/hieraki' - @request.set_REQUEST_URI nil @request.env['PATH_INFO'] = "/hieraki/" @request.env['SCRIPT_NAME'] = "/hieraki/dispatch.cgi" + @request.set_REQUEST_URI nil assert_equal "/hieraki/", @request.request_uri assert_equal "/", @request.path ActionController::Base.relative_url_root = nil @request.set_REQUEST_URI '/hieraki/dispatch.cgi' ActionController::Base.relative_url_root = '/hieraki' - assert_equal "/dispatch.cgi", @request.path + assert_equal "/dispatch.cgi", @request.path(true) ActionController::Base.relative_url_root = nil @request.set_REQUEST_URI '/hieraki/dispatch.cgi' ActionController::Base.relative_url_root = '/foo' - assert_equal "/hieraki/dispatch.cgi", @request.path + assert_equal "/hieraki/dispatch.cgi", @request.path(true) ActionController::Base.relative_url_root = nil # This test ensures that Rails uses REQUEST_URI over PATH_INFO @@ -227,8 +227,8 @@ class RequestTest < Test::Unit::TestCase @request.env['REQUEST_URI'] = "/some/path" @request.env['PATH_INFO'] = "/another/path" @request.env['SCRIPT_NAME'] = "/dispatch.cgi" - assert_equal "/some/path", @request.request_uri - assert_equal "/some/path", @request.path + assert_equal "/some/path", @request.request_uri(true) + assert_equal "/some/path", @request.path(true) end def test_host_with_default_port @@ -244,13 +244,13 @@ class RequestTest < Test::Unit::TestCase end def test_server_software - assert_equal nil, @request.server_software + assert_equal nil, @request.server_software(true) @request.env['SERVER_SOFTWARE'] = 'Apache3.422' - assert_equal 'apache', @request.server_software + assert_equal 'apache', @request.server_software(true) @request.env['SERVER_SOFTWARE'] = 'lighttpd(1.1.4)' - assert_equal 'lighttpd', @request.server_software + assert_equal 'lighttpd', @request.server_software(true) end def test_xml_http_request @@ -280,44 +280,44 @@ class RequestTest < Test::Unit::TestCase def test_symbolized_request_methods [:get, :post, :put, :delete].each do |method| - set_request_method_to method + self.request_method = method assert_equal method, @request.method end end def test_invalid_http_method_raises_exception - set_request_method_to :random_method assert_raises(ActionController::UnknownHttpMethod) do - @request.method + self.request_method = :random_method end end def test_allow_method_hacking_on_post - set_request_method_to :post + self.request_method = :post [:get, :head, :options, :put, :post, :delete].each do |method| - @request.instance_eval { @parameters = { :_method => method } ; @request_method = nil } + @request.instance_eval { @parameters = { :_method => method.to_s } ; @request_method = nil } + @request.request_method(true) assert_equal(method == :head ? :get : method, @request.method) end end def test_invalid_method_hacking_on_post_raises_exception - set_request_method_to :post + self.request_method = :post @request.instance_eval { @parameters = { :_method => :random_method } ; @request_method = nil } assert_raises(ActionController::UnknownHttpMethod) do - @request.method + @request.request_method(true) end end def test_restrict_method_hacking @request.instance_eval { @parameters = { :_method => 'put' } } [:get, :put, :delete].each do |method| - set_request_method_to method + self.request_method = method assert_equal method, @request.method end end - def test_head_masquarading_as_get - set_request_method_to :head + def test_head_masquerading_as_get + self.request_method = :head assert_equal :get, @request.method assert @request.get? assert @request.head? @@ -339,9 +339,16 @@ class RequestTest < Test::Unit::TestCase end def test_nil_format - @request.instance_eval { @parameters = { :format => nil } } + ActionController::Base.use_accept_header, old = + false, ActionController::Base.use_accept_header + + @request.instance_eval { @parameters = {} } @request.env["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" + assert @request.xhr? assert_equal Mime::JS, @request.format + + ensure + ActionController::Base.use_accept_header = old end def test_content_type @@ -384,9 +391,9 @@ class RequestTest < Test::Unit::TestCase end protected - def set_request_method_to(method) + def request_method=(method) @request.env['REQUEST_METHOD'] = method.to_s.upcase - @request.instance_eval { @request_method = nil } + @request.request_method(true) end end -- cgit v1.2.3 From c7375d74d9fff3219d4ea389ba9e36a90afe9d33 Mon Sep 17 00:00:00 2001 From: Michalis Polakis Date: Mon, 11 Aug 2008 14:53:24 +0100 Subject: Alias subquery used in calculations, to provide better compatibility with databases such as MonetDB Signed-off-by: Michael Koziarski Signed-off-by: Tom Ward [#796 state:committed] --- activerecord/lib/active_record/calculations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 2ca1a0aaa3..246f87b7a9 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -211,7 +211,7 @@ module ActiveRecord sql << " ORDER BY #{options[:order]} " if options[:order] add_limit!(sql, options, scope) - sql << ')' if use_workaround + sql << ') AS #{aggregate_alias}_subquery' if use_workaround sql end -- cgit v1.2.3 From 81c12d1f6359eb5e52b376f1f3552097a144cc8b Mon Sep 17 00:00:00 2001 From: Trevor Turk Date: Mon, 11 Aug 2008 21:17:14 -0500 Subject: move logging of protected attribute removal into log_protected_attribute_removal method Signed-off-by: Michael Koziarski [#804 status:committed] --- activerecord/lib/active_record/base.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 29c2995334..e5b6e3a02f 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2594,7 +2594,7 @@ module ActiveRecord #:nodoc: removed_attributes = attributes.keys - safe_attributes.keys if removed_attributes.any? - logger.debug "WARNING: Can't mass-assign these protected attributes: #{removed_attributes.join(', ')}" + log_protected_attribute_removal(removed_attributes) end safe_attributes @@ -2609,6 +2609,10 @@ module ActiveRecord #:nodoc: end end + def log_protected_attribute_removal(*attributes) + logger.debug "WARNING: Can't mass-assign these protected attributes: #{attributes.join(', ')}" + end + # The primary key and inheritance column can never be set by mass-assignment for security reasons. def attributes_protected_by_default default = [ self.class.primary_key, self.class.inheritance_column ] -- cgit v1.2.3 From 992fda16ed662f028700d63a8dcbd1837f1d58ab Mon Sep 17 00:00:00 2001 From: Tom Lea Date: Mon, 11 Aug 2008 18:16:58 +0100 Subject: Serialized attributes will now always be saved even with partial_updates turned on. Signed-off-by: Michael Koziarski [#788 state:committed] --- activerecord/lib/active_record/dirty.rb | 4 +++- activerecord/test/cases/dirty_test.rb | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/dirty.rb b/activerecord/lib/active_record/dirty.rb index 4ce0356457..63bf8c8f5b 100644 --- a/activerecord/lib/active_record/dirty.rb +++ b/activerecord/lib/active_record/dirty.rb @@ -134,7 +134,9 @@ module ActiveRecord def update_with_dirty if partial_updates? - update_without_dirty(changed) + # Serialized attributes should always be written in case they've been + # changed in place. + update_without_dirty(changed | self.class.serialized_attributes.keys) else update_without_dirty end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index e5e022050d..feb47a15a8 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -191,6 +191,18 @@ class DirtyTest < ActiveRecord::TestCase assert !pirate.changed? end + def test_save_should_store_serialized_attributes_even_with_partial_updates + with_partial_updates(Topic) do + topic = Topic.create!(:content => {:a => "a"}) + topic.content[:b] = "b" + #assert topic.changed? # Known bug, will fail + topic.save! + assert_equal "b", topic.content[:b] + topic.reload + assert_equal "b", topic.content[:b] + end + end + private def with_partial_updates(klass, on = true) old = klass.partial_updates? -- cgit v1.2.3 From 08b0cf07dbc639c8609118eaeb34330d5168e8b2 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 12 Aug 2008 17:03:06 -0700 Subject: Update changelog for conditional GET utility methods --- actionpack/CHANGELOG | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 177c6a354e..6f65d4003d 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -7,8 +7,14 @@ * Update Prototype to 1.6.0.2 #599 [Patrick Joyce] * Conditional GET utility methods. [Jeremy Kemper] - * etag!([:admin, post, current_user]) sets the ETag response header and returns head(:not_modified) if it matches the If-None-Match request header. - * last_modified!(post.updated_at) sets Last-Modified and returns head(:not_modified) if it's no later than If-Modified-Since. + response.last_modified = @post.updated_at + response.etag = [:admin, @post, current_user] + + if request.fresh?(response) + head :not_modified + else + # render ... + end * All 2xx requests are considered successful [Josh Peek] -- cgit v1.2.3 From 1b127fcdea28d6c6ab2f2c6370125c8817ba99d6 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 12 Aug 2008 20:18:03 -0700 Subject: Set asset-cached file ctime and mtime to the max mtime of the combined files. Allows for consistent ETag generation without having a shared filesystem. --- actionpack/lib/action_view/helpers/asset_tag_helper.rb | 5 +++++ actionpack/test/template/asset_tag_helper_test.rb | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 1e44b166d9..c2b4f51c9c 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -618,6 +618,11 @@ module ActionView def write_asset_file_contents(joined_asset_path, asset_paths) FileUtils.mkdir_p(File.dirname(joined_asset_path)) File.open(joined_asset_path, "w+") { |cache| cache.write(join_asset_file_contents(asset_paths)) } + + # Set mtime to the latest of the combined files to allow for + # consistent ETag without a shared filesystem. + mt = asset_paths.map { |p| File.mtime(File.join(ASSETS_DIR, p)) }.max + File.utime(mt, mt, joined_asset_path) end def collect_asset_files(*path) diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 8410e82c3c..7e40a55dc5 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -425,7 +425,8 @@ class AssetTagHelperTest < ActionView::TestCase stylesheet_link_tag(:all, :cache => true) ) - assert File.exist?(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'all.css')) + expected = Dir["#{ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR}/*.css"].map { |p| File.mtime(p) }.max + assert_equal expected, File.mtime(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'all.css')) assert_dom_equal( %(), -- cgit v1.2.3 From a5aad2e81febfa1a8d9fea0faffb5a3b4535982b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Wed, 13 Aug 2008 06:18:01 +0300 Subject: Fixed Time/Date object serialization Time/Date objects used to be converted to_s instead of to_uaml which made them unserializable. --- activerecord/lib/active_record/base.rb | 11 +++++++++-- activerecord/test/cases/base_test.rb | 6 ++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index e5b6e3a02f..2c4ead081d 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2626,8 +2626,15 @@ module ActiveRecord #:nodoc: quoted = {} connection = self.class.connection attribute_names.each do |name| - if column = column_for_attribute(name) - quoted[name] = connection.quote(read_attribute(name), column) unless !include_primary_key && column.primary + if (column = column_for_attribute(name)) && (include_primary_key || !column.primary) + value = read_attribute(name) + + # We need explicit to_yaml because quote() does not properly convert Time/Date fields to YAML. + if value && self.class.serialized_attributes.has_key?(name) && (value.acts_like?(:date) || value.acts_like?(:time)) + value = value.to_yaml + end + + quoted[name] = connection.quote(value, column) end end include_readonly_attributes ? quoted : remove_readonly_attributes(quoted) diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 0f9eda4d09..36d30ade5e 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1361,6 +1361,12 @@ class BasicsTest < ActiveRecord::TestCase assert_equal(myobj, topic.content) end + def test_serialized_time_attribute + myobj = Time.local(2008,1,1,1,0) + topic = Topic.create("content" => myobj).reload + assert_equal(myobj, topic.content) + end + def test_nil_serialized_attribute_with_class_constraint myobj = MyObject.new('value1', 'value2') topic = Topic.new -- cgit v1.2.3 From 282b42021306f73d4cc8f8a06713da9a55ed044b Mon Sep 17 00:00:00 2001 From: Jeffrey Hardy Date: Wed, 13 Aug 2008 00:51:47 -0400 Subject: Account for the possibility of a nil options argument to CompressedMemCacheStore#read/#write --- .../lib/active_support/cache/compressed_mem_cache_store.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/activesupport/lib/active_support/cache/compressed_mem_cache_store.rb b/activesupport/lib/active_support/cache/compressed_mem_cache_store.rb index 9470ac9f66..0bff6cf9ad 100644 --- a/activesupport/lib/active_support/cache/compressed_mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/compressed_mem_cache_store.rb @@ -1,14 +1,14 @@ module ActiveSupport module Cache class CompressedMemCacheStore < MemCacheStore - def read(name, options = {}) - if value = super(name, options.merge(:raw => true)) + def read(name, options = nil) + if value = super(name, (options || {}).merge(:raw => true)) Marshal.load(ActiveSupport::Gzip.decompress(value)) end end - def write(name, value, options = {}) - super(name, ActiveSupport::Gzip.compress(Marshal.dump(value)), options.merge(:raw => true)) + def write(name, value, options = nil) + super(name, ActiveSupport::Gzip.compress(Marshal.dump(value)), (options || {}).merge(:raw => true)) end end end -- cgit v1.2.3 From 1ee9e3fa5c924bef4aba3d53796f48f5badbd06f Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Wed, 13 Aug 2008 13:36:39 +0200 Subject: Fix ActiveRecord::NamedScope::Scope#respond_to? [#818 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/named_scope.rb | 6 +++++- activerecord/test/cases/named_scope_test.rb | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index d5a1c5fe08..0902018155 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -103,7 +103,7 @@ module ActiveRecord attr_reader :proxy_scope, :proxy_options [].methods.each do |m| - unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|find|count|sum|average|maximum|minimum|paginate|first|last|empty?|any?)/ + unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|find|count|sum|average|maximum|minimum|paginate|first|last|empty?|any?|respond_to?)/ delegate m, :to => :proxy_found end end @@ -140,6 +140,10 @@ module ActiveRecord @found ? @found.empty? : count.zero? end + def respond_to?(method) + super || @proxy_scope.respond_to?(method) + end + def any? if block_given? proxy_found.any? { |*block_args| yield(*block_args) } diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 7bd712e11e..bd6ec23853 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -45,6 +45,12 @@ class NamedScopeTest < ActiveRecord::TestCase assert_equal Topic.average(:replies_count), Topic.base.average(:replies_count) end + def test_scope_should_respond_to_own_methods_and_methods_of_the_proxy + assert Topic.approved.respond_to?(:proxy_found) + assert Topic.approved.respond_to?(:count) + assert Topic.approved.respond_to?(:length) + end + def test_subclasses_inherit_scopes assert Topic.scopes.include?(:base) -- cgit v1.2.3 From 2561be005b1180883ccf1d0f7ad38858d9348064 Mon Sep 17 00:00:00 2001 From: Josh Susser Date: Tue, 12 Aug 2008 22:38:45 -0700 Subject: Refactor Filter predicate methods to use inheritance. [#815 state:resolved] Signed-off-by: Pratik Naik --- actionpack/lib/action_controller/filters.rb | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb index 10dc0cc45b..1d7236f18a 100644 --- a/actionpack/lib/action_controller/filters.rb +++ b/actionpack/lib/action_controller/filters.rb @@ -109,16 +109,17 @@ module ActionController #:nodoc: update_options! options end + # override these to return true in appropriate subclass def before? - self.class == BeforeFilter + false end def after? - self.class == AfterFilter + false end def around? - self.class == AroundFilter + false end # Make sets of strings from :only/:except options @@ -170,6 +171,10 @@ module ActionController #:nodoc: :around end + def around? + true + end + def call(controller, &block) if should_run_callback?(controller) method = filter_responds_to_before_and_after? ? around_proc : self.method @@ -212,6 +217,10 @@ module ActionController #:nodoc: :before end + def before? + true + end + def call(controller, &block) super if controller.send!(:performed?) @@ -224,6 +233,10 @@ module ActionController #:nodoc: def type :after end + + def after? + true + end end # Filters enable controllers to run shared pre- and post-processing code for its actions. These filters can be used to do -- cgit v1.2.3 From 04248c62086b188ae354ed90ae40c832b79fd19c Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 13 Aug 2008 19:04:56 -0500 Subject: Ensure templates are rendered if all the parts are already processed --- actionmailer/lib/action_mailer/base.rb | 5 +++-- actionmailer/test/fixtures/test_mailer/signed_up.erb | 3 --- actionmailer/test/fixtures/test_mailer/signed_up.html.erb | 3 +++ 3 files changed, 6 insertions(+), 5 deletions(-) delete mode 100644 actionmailer/test/fixtures/test_mailer/signed_up.erb create mode 100644 actionmailer/test/fixtures/test_mailer/signed_up.html.erb diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 1583fb4066..72c94529b5 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -470,8 +470,9 @@ module ActionMailer #:nodoc: # also render a "normal" template (without the content type). If a # normal template exists (or if there were no implicit parts) we render # it. - template = template_root["#{mailer_name}/#{@template}"] - @body = render_message(@template, @body) if template + template_exists = @parts.empty? + template_exists ||= template_root["#{mailer_name}/#{@template}"] + @body = render_message(@template, @body) if template_exists # Finally, if there are other message parts and a textual body exists, # we shift it onto the front of the parts and set the body to nil (so diff --git a/actionmailer/test/fixtures/test_mailer/signed_up.erb b/actionmailer/test/fixtures/test_mailer/signed_up.erb deleted file mode 100644 index a85d5fa442..0000000000 --- a/actionmailer/test/fixtures/test_mailer/signed_up.erb +++ /dev/null @@ -1,3 +0,0 @@ -Hello there, - -Mr. <%= @recipient %> \ No newline at end of file diff --git a/actionmailer/test/fixtures/test_mailer/signed_up.html.erb b/actionmailer/test/fixtures/test_mailer/signed_up.html.erb new file mode 100644 index 0000000000..a85d5fa442 --- /dev/null +++ b/actionmailer/test/fixtures/test_mailer/signed_up.html.erb @@ -0,0 +1,3 @@ +Hello there, + +Mr. <%= @recipient %> \ No newline at end of file -- cgit v1.2.3 From 3b9324e62f770f1a0a457f7ad5fe6a3287ecae1f Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 13 Aug 2008 19:15:35 -0500 Subject: Fix rendering partials at the top level [#795 state:resolved] --- actionpack/lib/action_view/partials.rb | 2 +- actionpack/test/fixtures/_top_level_partial.html.erb | 1 + actionpack/test/fixtures/_top_level_partial_only.erb | 1 + actionpack/test/template/render_test.rb | 18 ++++++++++++++++++ 4 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 actionpack/test/fixtures/_top_level_partial.html.erb create mode 100644 actionpack/test/fixtures/_top_level_partial_only.erb diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index eb74d4a4c7..894b88534c 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -146,7 +146,7 @@ module ActionView def find_partial_path(partial_path) if partial_path.include?('/') - "#{File.dirname(partial_path)}/_#{File.basename(partial_path)}" + File.join(File.dirname(partial_path), "_#{File.basename(partial_path)}") elsif respond_to?(:controller) "#{controller.class.controller_path}/_#{partial_path}" else diff --git a/actionpack/test/fixtures/_top_level_partial.html.erb b/actionpack/test/fixtures/_top_level_partial.html.erb new file mode 100644 index 0000000000..0b1c2e46e0 --- /dev/null +++ b/actionpack/test/fixtures/_top_level_partial.html.erb @@ -0,0 +1 @@ +top level partial html \ No newline at end of file diff --git a/actionpack/test/fixtures/_top_level_partial_only.erb b/actionpack/test/fixtures/_top_level_partial_only.erb new file mode 100644 index 0000000000..44f25b61d0 --- /dev/null +++ b/actionpack/test/fixtures/_top_level_partial_only.erb @@ -0,0 +1 @@ +top level partial \ No newline at end of file diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index c37bac95f1..31cfdce531 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -19,6 +19,10 @@ class ViewRenderTest < Test::Unit::TestCase assert_equal "Hello world!", @view.render("test/hello_world") end + def test_render_file_at_top_level + assert_equal 'Elastica', @view.render('/shared') + end + def test_render_file_with_full_path template_path = File.join(File.dirname(__FILE__), '../fixtures/test/hello_world.erb') assert_equal "Hello world!", @view.render(:file => template_path) @@ -47,6 +51,20 @@ class ViewRenderTest < Test::Unit::TestCase assert_equal "only partial", @view.render(:partial => "test/partial_only") end + def test_render_partial_with_format + assert_equal 'partial html', @view.render(:partial => 'test/partial') + end + + def test_render_partial_at_top_level + # file fixtures/_top_level_partial_only.erb (not fixtures/test) + assert_equal 'top level partial', @view.render(:partial => '/top_level_partial_only') + end + + def test_render_partial_with_format_at_top_level + # file fixtures/_top_level_partial.html.erb (not fixtures/test, with format extension) + assert_equal 'top level partial html', @view.render(:partial => '/top_level_partial') + end + def test_render_partial_with_locals assert_equal "5", @view.render(:partial => "test/counter", :locals => { :counter_counter => 5 }) end -- cgit v1.2.3 From 3284fbb86629f398ba2634dd9369bc65beb7d6ae Mon Sep 17 00:00:00 2001 From: "S. Brent Faulkner" Date: Wed, 13 Aug 2008 19:19:00 -0500 Subject: Use current umask when testing the expected file mode [#823 state:resolved] Signed-off-by: Joshua Peek --- activesupport/test/core_ext/file_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/activesupport/test/core_ext/file_test.rb b/activesupport/test/core_ext/file_test.rb index 63b470684f..eedc6b592b 100644 --- a/activesupport/test/core_ext/file_test.rb +++ b/activesupport/test/core_ext/file_test.rb @@ -29,7 +29,7 @@ class AtomicWriteTest < Test::Unit::TestCase assert File.exist?(file_name) end assert File.exist?(file_name) - assert_equal "100755", file_mode + assert_equal 0100755, file_mode assert_equal contents, File.read(file_name) File.atomic_write(file_name, Dir.pwd) do |file| @@ -37,7 +37,7 @@ class AtomicWriteTest < Test::Unit::TestCase assert File.exist?(file_name) end assert File.exist?(file_name) - assert_equal "100755", file_mode + assert_equal 0100755, file_mode assert_equal contents, File.read(file_name) ensure File.unlink(file_name) rescue nil @@ -50,7 +50,7 @@ class AtomicWriteTest < Test::Unit::TestCase assert !File.exist?(file_name) end assert File.exist?(file_name) - assert_equal "100644", file_mode + assert_equal 0100666 ^ File.umask, file_mode assert_equal contents, File.read(file_name) ensure File.unlink(file_name) rescue nil @@ -62,6 +62,6 @@ class AtomicWriteTest < Test::Unit::TestCase end def file_mode - sprintf("%o", File.stat(file_name).mode) + File.stat(file_name).mode end end -- cgit v1.2.3 From 3fc9a67c04bade858e7ac7eb8cd94eec6a63ec27 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 13 Aug 2008 17:22:38 -0700 Subject: memoize_ and unmemoize_all --- activesupport/lib/active_support/memoizable.rb | 51 +++++++++++++++++--------- activesupport/test/memoizable_test.rb | 15 ++++++++ 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/activesupport/lib/active_support/memoizable.rb b/activesupport/lib/active_support/memoizable.rb index b5adc2330c..5aa73bce52 100644 --- a/activesupport/lib/active_support/memoizable.rb +++ b/activesupport/lib/active_support/memoizable.rb @@ -10,25 +10,37 @@ module ActiveSupport end def freeze_with_memoizable - unless frozen? - methods.each do |method| - if method.to_s =~ /^_unmemoized_(.*)/ - begin - __send__($1).freeze - rescue ArgumentError - end + memoize_all unless frozen? + freeze_without_memoizable + end + + def memoize_all + methods.each do |m| + if m.to_s =~ /^_unmemoized_(.*)/ + if method(m).arity == 0 + __send__($1) + else + ivar = :"@_memoized_#{$1}" + instance_variable_set(ivar, {}) end end end + end - freeze_without_memoizable + def unmemoize_all + methods.each do |m| + if m.to_s =~ /^_unmemoized_(.*)/ + ivar = :"@_memoized_#{$1}" + instance_variable_get(ivar).clear if instance_variable_defined?(ivar) + end + end end end def memoize(*symbols) symbols.each do |symbol| - original_method = "_unmemoized_#{symbol}" - memoized_ivar = "@_memoized_#{symbol.to_s.sub(/\?\Z/, '_query').sub(/!\Z/, '_bang')}" + original_method = :"_unmemoized_#{symbol}" + memoized_ivar = :"@_memoized_#{symbol.to_s.sub(/\?\Z/, '_query').sub(/!\Z/, '_bang')}" class_eval <<-EOS, __FILE__, __LINE__ include Freezable @@ -38,21 +50,24 @@ module ActiveSupport if instance_method(:#{symbol}).arity == 0 def #{symbol}(reload = false) - if !reload && defined? #{memoized_ivar} - #{memoized_ivar} - else - #{memoized_ivar} = #{original_method} + if reload || !defined?(#{memoized_ivar}) || #{memoized_ivar}.empty? + #{memoized_ivar} = [#{original_method}] end + #{memoized_ivar}[0] end else def #{symbol}(*args) - #{memoized_ivar} ||= {} + #{memoized_ivar} ||= {} unless frozen? reload = args.pop if args.last == true || args.last == :reload - if !reload && #{memoized_ivar} && #{memoized_ivar}.has_key?(args) - #{memoized_ivar}[args] + if #{memoized_ivar} + if !reload && #{memoized_ivar}.has_key?(args) + #{memoized_ivar}[args] + elsif #{memoized_ivar} + #{memoized_ivar}[args] = #{original_method}(*args) + end else - #{memoized_ivar}[args] = #{original_method}(*args) + #{original_method}(*args) end end end diff --git a/activesupport/test/memoizable_test.rb b/activesupport/test/memoizable_test.rb index 01631dc5e1..99d9240ea1 100644 --- a/activesupport/test/memoizable_test.rb +++ b/activesupport/test/memoizable_test.rb @@ -119,6 +119,21 @@ uses_mocha 'Memoizable' do assert_equal 3, @calculator.counter end + def test_unmemoize_all + assert_equal 1, @calculator.counter + + assert @calculator.instance_variable_get(:@_memoized_counter).any? + @calculator.unmemoize_all + assert @calculator.instance_variable_get(:@_memoized_counter).empty? + + assert_equal 2, @calculator.counter + end + + def test_memoize_all + @calculator.memoize_all + assert @calculator.instance_variable_defined?(:@_memoized_counter) + end + def test_memoization_cache_is_different_for_each_instance assert_equal 1, @calculator.counter assert_equal 2, @calculator.counter(:reload) -- cgit v1.2.3 From c7e09a8fb234d80f06fcd70b9263e28e42c4378b Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 13 Aug 2008 17:23:07 -0700 Subject: TestRequest#recycle! uses unmemoize_all to reset cached request method, accepts, etc. --- actionpack/lib/action_controller/test_process.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index 636e1c38b3..0c705207e3 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -120,13 +120,13 @@ module ActionController #:nodoc: end end @parameters = nil # reset TestRequest#parameters to use the new path_parameters - end - + end + def recycle! self.request_parameters = {} self.query_parameters = {} self.path_parameters = {} - @request_method, @accepts, @content_type = nil, nil, nil + unmemoize_all end private -- cgit v1.2.3 From b8b30985d525fd15b6c16d29fc115e83e3ee5037 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 13 Aug 2008 20:57:26 -0500 Subject: Marshal FileStore values --- .../lib/active_support/cache/file_store.rb | 4 ++-- activesupport/test/caching_test.rb | 27 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 7b6ca39091..437679cc05 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -9,13 +9,13 @@ module ActiveSupport def read(name, options = nil) super - File.open(real_file_path(name), 'rb') { |f| f.read } rescue nil + File.open(real_file_path(name), 'rb') { |f| Marshal.load(f) } rescue nil end def write(name, value, options = nil) super ensure_cache_path(File.dirname(real_file_path(name))) - File.atomic_write(real_file_path(name), cache_path) { |f| f.write(value) } + File.atomic_write(real_file_path(name), cache_path) { |f| Marshal.dump(value, f) } rescue => e RAILS_DEFAULT_LOGGER.error "Couldn't create cache directory: #{name} (#{e.message})" if RAILS_DEFAULT_LOGGER end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index f3220d27aa..c5f7fb7fdd 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -70,3 +70,30 @@ uses_mocha 'high-level cache store tests' do end end end + +class FileStoreTest < Test::Unit::TestCase + def setup + @cache = ActiveSupport::Cache.lookup_store(:file_store, Dir.pwd) + end + + def test_should_read_and_write_strings + @cache.write('foo', 'bar') + assert_equal 'bar', @cache.read('foo') + ensure + File.delete("foo.cache") + end + + def test_should_read_and_write_hash + @cache.write('foo', {:a => "b"}) + assert_equal({:a => "b"}, @cache.read('foo')) + ensure + File.delete("foo.cache") + end + + def test_should_read_and_write_nil + @cache.write('foo', nil) + assert_equal nil, @cache.read('foo') + ensure + File.delete("foo.cache") + end +end -- cgit v1.2.3 From 8cb14ee1203c9ed380c4b192e8730757a52d43cb Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 13 Aug 2008 21:30:46 -0500 Subject: Ensure results returned by a memoized method are immutable --- activesupport/lib/active_support/memoizable.rb | 4 ++-- activesupport/test/memoizable_test.rb | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/activesupport/lib/active_support/memoizable.rb b/activesupport/lib/active_support/memoizable.rb index 5aa73bce52..6506238ac0 100644 --- a/activesupport/lib/active_support/memoizable.rb +++ b/activesupport/lib/active_support/memoizable.rb @@ -51,7 +51,7 @@ module ActiveSupport if instance_method(:#{symbol}).arity == 0 def #{symbol}(reload = false) if reload || !defined?(#{memoized_ivar}) || #{memoized_ivar}.empty? - #{memoized_ivar} = [#{original_method}] + #{memoized_ivar} = [#{original_method}.freeze] end #{memoized_ivar}[0] end @@ -64,7 +64,7 @@ module ActiveSupport if !reload && #{memoized_ivar}.has_key?(args) #{memoized_ivar}[args] elsif #{memoized_ivar} - #{memoized_ivar}[args] = #{original_method}(*args) + #{memoized_ivar}[args] = #{original_method}(*args).freeze end else #{original_method}(*args) diff --git a/activesupport/test/memoizable_test.rb b/activesupport/test/memoizable_test.rb index 99d9240ea1..135d56f14a 100644 --- a/activesupport/test/memoizable_test.rb +++ b/activesupport/test/memoizable_test.rb @@ -110,6 +110,11 @@ uses_mocha 'Memoizable' do assert_equal 1, @person.age_calls end + def test_memorized_results_are_immutable + assert_equal "Josh", @person.name + assert_raise(ActiveSupport::FrozenObjectError) { @person.name.gsub!("Josh", "Gosh") } + end + def test_reloadable counter = @calculator.counter assert_equal 1, @calculator.counter -- cgit v1.2.3 From f1f4e84a7ef88d941f6508673bb448de640d6f77 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 14 Aug 2008 12:23:29 -0700 Subject: Fix asset file paths with dangling queries in mtime check --- actionpack/lib/action_view/helpers/asset_tag_helper.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index c2b4f51c9c..623ed1e8df 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -612,7 +612,7 @@ module ActionView end def join_asset_file_contents(paths) - paths.collect { |path| File.read(File.join(ASSETS_DIR, path.split("?").first)) }.join("\n\n") + paths.collect { |path| File.read(asset_file_path(path)) }.join("\n\n") end def write_asset_file_contents(joined_asset_path, asset_paths) @@ -621,10 +621,14 @@ module ActionView # Set mtime to the latest of the combined files to allow for # consistent ETag without a shared filesystem. - mt = asset_paths.map { |p| File.mtime(File.join(ASSETS_DIR, p)) }.max + mt = asset_paths.map { |p| File.mtime(asset_file_path(p)) }.max File.utime(mt, mt, joined_asset_path) end + def asset_file_path(path) + File.join(ASSETS_DIR, path.split('?').first) + end + def collect_asset_files(*path) dir = path.first -- cgit v1.2.3 From 8aad8cb3906fce40fa583839fec7f8591fab8ec7 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 14 Aug 2008 21:45:14 -0700 Subject: Set cache control to require revalidation if cache freshness response headers are set. Don't set Content-Length header if 304 status. --- actionpack/lib/action_controller/response.rb | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/actionpack/lib/action_controller/response.rb b/actionpack/lib/action_controller/response.rb index a85fad0d39..7b57f499bb 100644 --- a/actionpack/lib/action_controller/response.rb +++ b/actionpack/lib/action_controller/response.rb @@ -79,7 +79,13 @@ module ActionController # :nodoc: end def last_modified - Time.rfc2822(headers['Last-Modified']) + if last = headers['Last-Modified'] + Time.httpdate(last) + end + end + + def last_modified? + headers.include?('Last-Modified') end def last_modified=(utc_time) @@ -87,6 +93,7 @@ module ActionController # :nodoc: end def etag; headers['ETag'] end + def etag?; headers.include?('ETag') end def etag=(etag) headers['ETag'] = %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(etag))}") end @@ -98,22 +105,22 @@ module ActionController # :nodoc: end def prepare! + set_content_length! handle_conditional_get! convert_content_type! - set_content_length! end private def handle_conditional_get! if nonempty_ok_response? - set_conditional_cache_control! - self.etag ||= body if request && request.etag_matches?(etag) self.status = '304 Not Modified' self.body = '' end end + + set_conditional_cache_control! if etag? || last_modified? end def nonempty_ok_response? @@ -142,7 +149,9 @@ module ActionController # :nodoc: # Don't set the Content-Length for block-based bodies as that would mean reading it all into memory. Not nice # for, say, a 2GB streaming file. def set_content_length! - self.headers["Content-Length"] = body.size unless body.respond_to?(:call) + unless body.respond_to?(:call) || (status && status[0..2] == '304') + self.headers["Content-Length"] ||= body.size + end end end end -- cgit v1.2.3 From aad7cac6add2fa01cebbb36e9f546292d632c9ea Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 15 Aug 2008 09:27:07 -0500 Subject: Fixed problems with the logger used if the logging string included %'s [#840 state:resolved] (Jamis Buck) --- activeresource/CHANGELOG | 2 ++ activeresource/lib/active_resource/connection.rb | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/activeresource/CHANGELOG b/activeresource/CHANGELOG index 673e20de28..b62aa91b45 100644 --- a/activeresource/CHANGELOG +++ b/activeresource/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Fixed problems with the logger used if the logging string included %'s [#840 state:resolved] (Jamis Buck) + * Fixed Base#exists? to check status code as integer [#299 state:resolved] (Wes Oldenbeuving) diff --git a/activeresource/lib/active_resource/connection.rb b/activeresource/lib/active_resource/connection.rb index 0c4ea432d7..a4a61b61b5 100644 --- a/activeresource/lib/active_resource/connection.rb +++ b/activeresource/lib/active_resource/connection.rb @@ -140,7 +140,7 @@ module ActiveResource logger.info "#{method.to_s.upcase} #{site.scheme}://#{site.host}:#{site.port}#{path}" if logger result = nil time = Benchmark.realtime { result = http.send(method, path, *arguments) } - logger.info "--> #{result.code} #{result.message} (#{result.body ? result.body.length : 0}b %.2fs)" % time if logger + logger.info "--> %d %s (%d %.2fs)" % [result.code, result.message, result.body ? result.body.length : 0, time] if logger handle_response(result) rescue Timeout::Error => e raise TimeoutError.new(e.message) -- cgit v1.2.3 From e3523f1d33c3cf53f1a65e520be5e937e9c68c1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Fri, 15 Aug 2008 18:39:11 +0300 Subject: Fixed validates_uniqueness_of with decimal columns Only use special case-sensitive comparison operators for text columns in validates_uniqueness_of as mysql can fail at decimal comparisons with the BINARY operator. --- activerecord/lib/active_record/validations.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index e7a9676394..b7e6394748 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -629,12 +629,11 @@ module ActiveRecord if value.nil? comparison_operator = "IS ?" - else + elsif is_text_column comparison_operator = "#{connection.case_sensitive_equality_operator} ?" - - if is_text_column - value = value.to_s - end + value = value.to_s + else + comparison_operator = "= ?" end sql_attribute = "#{record.class.quoted_table_name}.#{connection.quote_column_name(attr_name)}" -- cgit v1.2.3 From b3c9d53b348f586ed223ec5de9f525faee6f564d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Mon, 2 Jun 2008 16:48:06 +0300 Subject: Use type_condition method for hmt STI condition --- .../lib/active_record/associations/has_many_through_association.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index e1bfff5923..24b02efc35 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -237,7 +237,7 @@ module ActiveRecord end def build_sti_condition - "#{@reflection.through_reflection.quoted_table_name}.#{@reflection.through_reflection.klass.inheritance_column} = #{@reflection.klass.quote_value(@reflection.through_reflection.klass.sti_name)}" + @reflection.through_reflection.klass.send(:type_condition) end alias_method :sql_conditions, :conditions -- cgit v1.2.3 From 8f4d3957a6986fe450cfd9058bb92ae1d6e5e745 Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Fri, 15 Aug 2008 13:51:57 -0700 Subject: Don't raise exception when comparing ActiveRecord::Reflection. [#842 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/reflection.rb | 2 +- activerecord/test/cases/reflection_test.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 3f74c03714..935b1939d8 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -109,7 +109,7 @@ module ActiveRecord # Returns +true+ if +self+ and +other_aggregation+ have the same +name+ attribute, +active_record+ attribute, # and +other_aggregation+ has an options hash assigned to it. def ==(other_aggregation) - name == other_aggregation.name && other_aggregation.options && active_record == other_aggregation.active_record + other_aggregation.kind_of?(self.class) && name == other_aggregation.name && other_aggregation.options && active_record == other_aggregation.active_record end def sanitized_conditions #:nodoc: diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 723062e3b8..4b86e32dbf 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -166,6 +166,10 @@ class ReflectionTest < ActiveRecord::TestCase assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size end + def test_reflection_should_not_raise_error_when_compared_to_other_object + assert_nothing_raised { Firm.reflections[:clients] == Object.new } + end + private def assert_reflection(klass, association, options) assert reflection = klass.reflect_on_association(association) -- cgit v1.2.3 From 2b69a636c431d62a85b2896d87b69cb13e2b8af0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Sat, 16 Aug 2008 02:24:29 +0300 Subject: Fixed STI type condition for eager loading of associations Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations.rb | 6 ++---- activerecord/lib/active_record/base.rb | 7 ++++--- .../test/cases/associations/cascaded_eager_loading_test.rb | 12 ++++++++++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 4e33dfe69f..b72fdb305f 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -2099,10 +2099,8 @@ module ActiveRecord else "" end || '' - join << %(AND %s.%s = %s ) % [ - connection.quote_table_name(aliased_table_name), - connection.quote_column_name(klass.inheritance_column), - klass.quote_value(klass.sti_name)] unless klass.descends_from_active_record? + join << %(AND %s) % [ + klass.send(:type_condition, aliased_table_name)] unless klass.descends_from_active_record? [through_reflection, reflection].each do |ref| join << "AND #{interpolate_sql(sanitize_sql(ref.options[:conditions]))} " if ref && ref.options[:conditions] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2c4ead081d..6eb4d42d51 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1609,10 +1609,11 @@ module ActiveRecord #:nodoc: sql << "WHERE #{merged_conditions} " unless merged_conditions.blank? end - def type_condition + def type_condition(table_alias=nil) + quoted_table_alias = self.connection.quote_table_name(table_alias || table_name) quoted_inheritance_column = connection.quote_column_name(inheritance_column) - type_condition = subclasses.inject("#{quoted_table_name}.#{quoted_inheritance_column} = '#{sti_name}' ") do |condition, subclass| - condition << "OR #{quoted_table_name}.#{quoted_inheritance_column} = '#{subclass.sti_name}' " + type_condition = subclasses.inject("#{quoted_table_alias}.#{quoted_inheritance_column} = '#{sti_name}' ") do |condition, subclass| + condition << "OR #{quoted_table_alias}.#{quoted_inheritance_column} = '#{subclass.sti_name}' " end " (#{type_condition}) " diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index 1f8a1090eb..8c9ae8a031 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -68,6 +68,18 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end end + def test_eager_association_loading_with_has_many_sti_and_subclasses + silly = SillyReply.new(:title => "gaga", :content => "boo-boo", :parent_id => 1) + silly.parent_id = 1 + assert silly.save + + topics = Topic.find(:all, :include => :replies, :order => 'topics.id, replies_topics.id') + assert_no_queries do + assert_equal 2, topics[0].replies.size + assert_equal 0, topics[1].replies.size + end + end + def test_eager_association_loading_with_belongs_to_sti replies = Reply.find(:all, :include => :topic, :order => 'topics.id') assert replies.include?(topics(:second)) -- cgit v1.2.3 From 8cfdcdb35db6e2f6fd5a72a38f4352beab148af1 Mon Sep 17 00:00:00 2001 From: Nathan Witmer Date: Sat, 16 Aug 2008 13:38:01 -0600 Subject: Updated has_and_belongs_to_many association to fix :finder_sql interpolation. [#848 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations/association_proxy.rb | 4 ---- .../associations/has_and_belongs_to_many_association.rb | 4 +--- .../associations/has_and_belongs_to_many_associations_test.rb | 7 +++++++ 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 77fc827e11..981be3b1a9 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -131,10 +131,6 @@ module ActiveRecord records.map { |record| record.quoted_id }.join(',') end - def interpolate_sql_options!(options, *keys) - keys.each { |key| options[key] &&= interpolate_sql(options[key]) } - end - def interpolate_sql(sql, record = nil) @owner.send(:interpolate_sql, sql, record) end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index d516d54151..e7e433b6b6 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -70,10 +70,8 @@ module ActiveRecord end def construct_sql - interpolate_sql_options!(@reflection.options, :finder_sql) - if @reflection.options[:finder_sql] - @finder_sql = @reflection.options[:finder_sql] + @finder_sql = interpolate_sql(@reflection.options[:finder_sql]) else @finder_sql = "#{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.primary_key_name} = #{owner_quoted_id} " @finder_sql << " AND (#{conditions})" if conditions diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index f71b122ff0..dfd82534ff 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -450,6 +450,13 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal developers(:david), active_record.developers_with_finder_sql.find(developers(:david).id), "Ruby find" end + def test_find_in_association_with_custom_finder_sql_and_multiple_interpolations + # interpolate once: + assert_equal [developers(:david), developers(:poor_jamis), developers(:jamis)], projects(:active_record).developers_with_finder_sql, "first interpolation" + # interpolate again, for a different project id + assert_equal [developers(:david)], projects(:action_controller).developers_with_finder_sql, "second interpolation" + end + def test_find_in_association_with_custom_finder_sql_and_string_id assert_equal developers(:david), projects(:active_record).developers_with_finder_sql.find(developers(:david).id.to_s), "SQL find" end -- cgit v1.2.3 From 96607996eaa826b5299780c7c9142e6e0157d198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Sun, 17 Aug 2008 00:20:55 +0300 Subject: Test for eager loading of STI subclasses from htm associations Signed-off-by: Pratik Naik --- activerecord/test/cases/associations/join_model_test.rb | 7 +++++++ activerecord/test/models/author.rb | 3 +++ 2 files changed, 10 insertions(+) diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 9e79d9c8a1..7a0427aabc 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -694,6 +694,13 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert ! david.categories.include?(category) end + def test_has_many_through_goes_through_all_sti_classes + sub_sti_post = SubStiPost.create!(:title => 'test', :body => 'test', :author_id => 1) + new_comment = sub_sti_post.comments.create(:body => 'test') + + assert_equal [9, 10, new_comment.id], authors(:david).sti_post_comments.map(&:id).sort + end + private # create dynamic Post models to allow different dependency options def find_post_with_dependency(post_id, association, association_name, dependency) diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 136dc39cf3..c6aa0293c2 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -32,6 +32,9 @@ class Author < ActiveRecord::Base has_many :special_posts has_many :special_post_comments, :through => :special_posts, :source => :comments + has_many :sti_posts, :class_name => 'StiPost' + has_many :sti_post_comments, :through => :sti_posts, :source => :comments + has_many :special_nonexistant_posts, :class_name => "SpecialPost", :conditions => "posts.body = 'nonexistant'" has_many :special_nonexistant_post_comments, :through => :special_nonexistant_posts, :source => :comments, :conditions => "comments.post_id = 0" has_many :nonexistant_comments, :through => :posts -- cgit v1.2.3 From 894f9ccc5354c29e102f316f388d1dc0b98ef080 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 17 Aug 2008 19:04:01 -0500 Subject: Use RackRequest as a mock instead of StubCGI into RequestTest --- actionpack/test/controller/request_test.rb | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/actionpack/test/controller/request_test.rb b/actionpack/test/controller/request_test.rb index 226c1ac018..045dab4141 100644 --- a/actionpack/test/controller/request_test.rb +++ b/actionpack/test/controller/request_test.rb @@ -397,7 +397,6 @@ class RequestTest < Test::Unit::TestCase end end - class UrlEncodedRequestParameterParsingTest < Test::Unit::TestCase def setup @query_string = "action=create_customer&full_name=David%20Heinemeier%20Hansson&customerId=1" @@ -509,7 +508,6 @@ class UrlEncodedRequestParameterParsingTest < Test::Unit::TestCase ) end - def test_request_hash_parsing query = { "note[viewers][viewer][][type]" => ["User", "Group"], @@ -521,7 +519,6 @@ class UrlEncodedRequestParameterParsingTest < Test::Unit::TestCase assert_equal(expected, ActionController::AbstractRequest.parse_request_parameters(query)) end - def test_parse_params input = { "customers[boston][first][name]" => [ "David" ], @@ -704,7 +701,6 @@ class UrlEncodedRequestParameterParsingTest < Test::Unit::TestCase end end - class MultipartRequestParameterParsingTest < Test::Unit::TestCase FIXTURE_PATH = File.dirname(__FILE__) + '/../fixtures/multipart' @@ -852,20 +848,20 @@ class XmlParamsParsingTest < Test::Unit::TestCase private def parse_body(body) - env = { 'CONTENT_TYPE' => 'application/xml', + env = { 'rack.input' => StringIO.new(body), + 'CONTENT_TYPE' => 'application/xml', 'CONTENT_LENGTH' => body.size.to_s } - cgi = ActionController::Integration::Session::StubCGI.new(env, body) - ActionController::CgiRequest.new(cgi).request_parameters + ActionController::RackRequest.new(env).request_parameters end end class LegacyXmlParamsParsingTest < XmlParamsParsingTest private def parse_body(body) - env = { 'HTTP_X_POST_DATA_FORMAT' => 'xml', - 'CONTENT_LENGTH' => body.size.to_s } - cgi = ActionController::Integration::Session::StubCGI.new(env, body) - ActionController::CgiRequest.new(cgi).request_parameters + env = { 'rack.input' => StringIO.new(body), + 'HTTP_X_POST_DATA_FORMAT' => 'xml', + 'CONTENT_LENGTH' => body.size.to_s } + ActionController::RackRequest.new(env).request_parameters end end @@ -884,9 +880,9 @@ class JsonParamsParsingTest < Test::Unit::TestCase private def parse_body(body,content_type) - env = { 'CONTENT_TYPE' => content_type, + env = { 'rack.input' => StringIO.new(body), + 'CONTENT_TYPE' => content_type, 'CONTENT_LENGTH' => body.size.to_s } - cgi = ActionController::Integration::Session::StubCGI.new(env, body) - ActionController::CgiRequest.new(cgi).request_parameters + ActionController::RackRequest.new(env).request_parameters end end -- cgit v1.2.3 From b8e930aa016e26471046d3f7d7ca1c10103791e7 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 17 Aug 2008 19:09:38 -0500 Subject: Merge RackProcess#normalize_headers logic into AbstractResponse#prepare! --- actionpack/lib/action_controller/rack_process.rb | 87 ++++++++++++++---------- actionpack/test/controller/rack_test.rb | 35 ++++++---- 2 files changed, 75 insertions(+), 47 deletions(-) diff --git a/actionpack/lib/action_controller/rack_process.rb b/actionpack/lib/action_controller/rack_process.rb index dcbcf8bc1d..221613b854 100644 --- a/actionpack/lib/action_controller/rack_process.rb +++ b/actionpack/lib/action_controller/rack_process.rb @@ -143,23 +143,26 @@ end_msg end class RackResponse < AbstractResponse #:nodoc: - attr_accessor :status - def initialize(request) - @request = request + @cgi = request.cgi @writer = lambda { |x| @body << x } @block = nil super() end + # Retrieve status from instance variable if has already been delete + def status + @status || super + end + def out(output = $stdout, &block) @block = block - normalize_headers(@headers) - if [204, 304].include?(@status.to_i) - @headers.delete "Content-Type" - [status, @headers.to_hash, []] + @status = headers.delete("Status") + if [204, 304].include?(status.to_i) + headers.delete("Content-Type") + [status, headers.to_hash, []] else - [status, @headers.to_hash, self] + [status, headers.to_hash, self] end end alias to_a out @@ -191,43 +194,57 @@ end_msg @block == nil && @body.empty? end - private - def normalize_headers(options = "text/html") - if options.is_a?(String) - headers['Content-Type'] = options unless headers['Content-Type'] - else - headers['Content-Length'] = options.delete('Content-Length').to_s if options['Content-Length'] + def prepare! + super - headers['Content-Type'] = options.delete('type') || "text/html" - headers['Content-Type'] += "; charset=" + options.delete('charset') if options['charset'] + convert_language! + convert_expires! + set_status! + set_cookies! + end - headers['Content-Language'] = options.delete('language') if options['language'] - headers['Expires'] = options.delete('expires') if options['expires'] + private + def convert_language! + headers["Content-Language"] = headers.delete("language") if headers["language"] + end - @status = options.delete('Status') || "200 OK" + def convert_expires! + headers["Expires"] = headers.delete("") if headers["expires"] + end - # Convert 'cookie' header to 'Set-Cookie' headers. - # Because Set-Cookie header can appear more the once in the response body, - # we store it in a line break separated string that will be translated to - # multiple Set-Cookie header by the handler. - if cookie = options.delete('cookie') - cookies = [] + def convert_content_type! + super + headers['Content-Type'] = headers.delete('type') || "text/html" + headers['Content-Type'] += "; charset=" + headers.delete('charset') if headers['charset'] + end - case cookie - when Array then cookie.each { |c| cookies << c.to_s } - when Hash then cookie.each { |_, c| cookies << c.to_s } - else cookies << cookie.to_s - end + def set_content_length! + super + headers["Content-Length"] = headers["Content-Length"].to_s if headers["Content-Length"] + end - @request.cgi.output_cookies.each { |c| cookies << c.to_s } if @request.cgi.output_cookies + def set_status! + self.status ||= "200 OK" + end - headers['Set-Cookie'] = [headers['Set-Cookie'], cookies].flatten.compact + def set_cookies! + # Convert 'cookie' header to 'Set-Cookie' headers. + # Because Set-Cookie header can appear more the once in the response body, + # we store it in a line break separated string that will be translated to + # multiple Set-Cookie header by the handler. + if cookie = headers.delete('cookie') + cookies = [] + + case cookie + when Array then cookie.each { |c| cookies << c.to_s } + when Hash then cookie.each { |_, c| cookies << c.to_s } + else cookies << cookie.to_s end - options.each { |k,v| headers[k] = v } - end + @cgi.output_cookies.each { |c| cookies << c.to_s } if @cgi.output_cookies - "" + headers['Set-Cookie'] = [headers['Set-Cookie'], cookies].flatten.compact + end end end diff --git a/actionpack/test/controller/rack_test.rb b/actionpack/test/controller/rack_test.rb index d1650de1fc..30a1144aad 100644 --- a/actionpack/test/controller/rack_test.rb +++ b/actionpack/test/controller/rack_test.rb @@ -236,10 +236,17 @@ class RackResponseTest < BaseRackTest def test_simple_output @response.body = "Hello, World!" + @response.prepare! status, headers, body = @response.out(@output) assert_equal "200 OK", status - assert_equal({"Content-Type" => "text/html", "Cache-Control" => "no-cache", "Set-Cookie" => []}, headers) + assert_equal({ + "Content-Type" => "text/html", + "Cache-Control" => "private, max-age=0, must-revalidate", + "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"', + "Set-Cookie" => [], + "Content-Length" => "13" + }, headers) parts = [] body.each { |part| parts << part } @@ -250,6 +257,7 @@ class RackResponseTest < BaseRackTest @response.body = Proc.new do |response, output| 5.times { |n| output.write(n) } end + @response.prepare! status, headers, body = @response.out(@output) assert_equal "200 OK", status @@ -265,13 +273,16 @@ class RackResponseTest < BaseRackTest @request.cgi.send :instance_variable_set, '@output_cookies', [cookie] @response.body = "Hello, World!" + @response.prepare! status, headers, body = @response.out(@output) assert_equal "200 OK", status assert_equal({ "Content-Type" => "text/html", - "Cache-Control" => "no-cache", - "Set-Cookie" => ["name=Josh; path="] + "Cache-Control" => "private, max-age=0, must-revalidate", + "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"', + "Set-Cookie" => ["name=Josh; path="], + "Content-Length" => "13" }, headers) parts = [] @@ -285,18 +296,18 @@ class RackResponseHeadersTest < BaseRackTest super @response = ActionController::RackResponse.new(@request) @output = StringIO.new('') - @response.headers['Status'] = 200 + @response.headers['Status'] = "200 OK" end def test_content_type [204, 304].each do |c| - @response.headers['Status'] = c - assert !response_headers.has_key?("Content-Type") + @response.headers['Status'] = c.to_s + assert !response_headers.has_key?("Content-Type"), "#{c} should not have Content-Type header" end [200, 302, 404, 500].each do |c| - @response.headers['Status'] = c - assert response_headers.has_key?("Content-Type") + @response.headers['Status'] = c.to_s + assert response_headers.has_key?("Content-Type"), "#{c} did not have Content-Type header" end end @@ -305,8 +316,8 @@ class RackResponseHeadersTest < BaseRackTest end private - - def response_headers - @response.out(@output)[1] - end + def response_headers + @response.prepare! + @response.out(@output)[1] + end end -- cgit v1.2.3 From f245658495865eff0882589b3e159704e48e1820 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 17 Aug 2008 19:13:49 -0500 Subject: Use Response status accessor instead of the Status header --- actionpack/lib/action_controller/test_process.rb | 34 ++++++++++++------------ 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index 0c705207e3..c6b1470070 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -138,7 +138,7 @@ module ActionController #:nodoc: @host = "test.host" @request_uri = "/" @user_agent = "Rails Testing" - self.remote_addr = "0.0.0.0" + self.remote_addr = "0.0.0.0" @env["SERVER_PORT"] = 80 @env['REQUEST_METHOD'] = "GET" end @@ -160,16 +160,16 @@ module ActionController #:nodoc: module TestResponseBehavior #:nodoc: # The response code of the request def response_code - headers['Status'][0,3].to_i rescue 0 + status[0,3].to_i rescue 0 end - + # Returns a String to ensure compatibility with Net::HTTPResponse def code - headers['Status'].to_s.split(' ')[0] + status.to_s.split(' ')[0] end def message - headers['Status'].to_s.split(' ',2)[1] + status.to_s.split(' ',2)[1] end # Was the response successful? @@ -246,11 +246,11 @@ module ActionController #:nodoc: # Does the specified template object exist? def has_template_object?(name=nil) - !template_objects[name].nil? + !template_objects[name].nil? end # Returns the response cookies, converted to a Hash of (name => CGI::Cookie) pairs - # + # # assert_equal ['AuthorOfNewPage'], r.cookies['author'].value def cookies headers['cookie'].inject({}) { |hash, cookie| hash[cookie.name] = cookie; hash } @@ -322,7 +322,7 @@ module ActionController #:nodoc: # # Usage example, within a functional test: # post :change_avatar, :avatar => ActionController::TestUploadedFile.new(Test::Unit::TestCase.fixture_path + '/files/spongebob.png', 'image/png') - # + # # Pass a true third parameter to ensure the uploaded file is opened in binary mode (only required for Windows): # post :change_avatar, :avatar => ActionController::TestUploadedFile.new(Test::Unit::TestCase.fixture_path + '/files/spongebob.png', 'image/png', :binary) require 'tempfile' @@ -403,13 +403,13 @@ module ActionController #:nodoc: end alias xhr :xml_http_request - def assigns(key = nil) - if key.nil? - @response.template.assigns - else - @response.template.assigns[key.to_s] - end - end + def assigns(key = nil) + if key.nil? + @response.template.assigns + else + @response.template.assigns[key.to_s] + end + end def session @response.session @@ -468,7 +468,7 @@ module ActionController #:nodoc: # post :change_avatar, :avatar => fixture_file_upload('/files/spongebob.png', 'image/png', :binary) def fixture_file_upload(path, mime_type = nil, binary = false) ActionController::TestUploadedFile.new( - Test::Unit::TestCase.respond_to?(:fixture_path) ? Test::Unit::TestCase.fixture_path + path : path, + Test::Unit::TestCase.respond_to?(:fixture_path) ? Test::Unit::TestCase.fixture_path + path : path, mime_type, binary ) @@ -476,7 +476,7 @@ module ActionController #:nodoc: # A helper to make it easier to test different route configurations. # This method temporarily replaces ActionController::Routing::Routes - # with a new RouteSet instance. + # with a new RouteSet instance. # # The new instance is yielded to the passed block. Typically the block # will create some routes using map.draw { map.connect ... }: -- cgit v1.2.3 From dbb0abfb7e9eb9a63b721a38625e3eff66ced49d Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 17 Aug 2008 19:18:18 -0500 Subject: More integration test coverage --- actionpack/test/controller/integration_test.rb | 32 +++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 475e13897b..a709343a94 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -265,6 +265,10 @@ class IntegrationProcessTest < ActionController::IntegrationTest cookies["cookie_3"] = "chocolate" render :text => "Gone", :status => 410 end + + def redirect + redirect_to :action => "get" + end end def test_get @@ -274,6 +278,9 @@ class IntegrationProcessTest < ActionController::IntegrationTest assert_equal "OK", status_message assert_equal "200 OK", response.headers["Status"] assert_equal ["200 OK"], headers["status"] + assert_response 200 + assert_response :success + assert_response :ok assert_equal [], response.headers["cookie"] assert_equal [], headers["cookie"] assert_equal({}, cookies) @@ -290,6 +297,9 @@ class IntegrationProcessTest < ActionController::IntegrationTest assert_equal "Created", status_message assert_equal "201 Created", response.headers["Status"] assert_equal ["201 Created"], headers["status"] + assert_response 201 + assert_response :success + assert_response :created assert_equal [], response.headers["cookie"] assert_equal [], headers["cookie"] assert_equal({}, cookies) @@ -308,6 +318,8 @@ class IntegrationProcessTest < ActionController::IntegrationTest assert_equal "Gone", status_message assert_equal "410 Gone", response.headers["Status"] assert_equal ["410 Gone"], headers["status"] + assert_response 410 + assert_response :gone assert_equal nil, response.headers["Set-Cookie"] assert_equal ["cookie_1=; path=/", "cookie_3=chocolate; path=/"], headers['set-cookie'] assert_equal [[], ["chocolate"]], response.headers["cookie"] @@ -317,14 +329,28 @@ class IntegrationProcessTest < ActionController::IntegrationTest end end + def test_redirect + with_test_route_set do + get '/redirect' + assert_equal 302, status + assert_equal "Found", status_message + assert_equal "302 Found", response.headers["Status"] + assert_equal ["302 Found"], headers["status"] + assert_response 302 + assert_response :redirect + assert_response :found + assert_equal "You are being redirected.", response.body + assert_kind_of HTML::Document, html_document + assert_equal 1, request_count + end + end + private def with_test_route_set with_routing do |set| set.draw do |map| map.with_options :controller => "IntegrationProcessTest::Integration" do |c| - c.connect '/get', :action => "get" - c.connect '/post', :action => "post" - c.connect '/cookie_monster', :action => "cookie_monster" + c.connect "/:action" end end yield -- cgit v1.2.3 From 38c7d73e73d569211c4dfadf96fc295a925b7c9c Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Sun, 17 Aug 2008 19:29:24 -0500 Subject: pass yielded arguments to block for ActionView::Base#render with :layout [#847 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_view/base.rb | 23 +++++-------- actionpack/lib/action_view/partials.rb | 38 ++++++++++++++++++++-- actionpack/lib/action_view/renderable.rb | 9 ++++- actionpack/test/controller/layout_test.rb | 19 +++++------ actionpack/test/controller/new_render_test.rb | 9 +++++ .../test/_layout_for_block_with_args.html.erb | 3 ++ .../using_layout_around_block_with_args.html.erb | 1 + 7 files changed, 74 insertions(+), 28 deletions(-) create mode 100644 actionpack/test/fixtures/test/_layout_for_block_with_args.html.erb create mode 100644 actionpack/test/fixtures/test/using_layout_around_block_with_args.html.erb diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index ad59d92086..f7f9f70298 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -246,12 +246,18 @@ module ActionView #:nodoc: if partial_layout = options.delete(:layout) if block_given? - wrap_content_for_layout capture(&block) do + begin + @_proc_for_layout = block concat(render(options.merge(:partial => partial_layout))) + ensure + @_proc_for_layout = nil end else - wrap_content_for_layout render(options) do + begin + original_content_for_layout, @content_for_layout = @content_for_layout, render(options) render(options.merge(:partial => partial_layout)) + ensure + @content_for_layout = original_content_for_layout end end elsif options[:file] @@ -367,13 +373,6 @@ module ActionView #:nodoc: InlineTemplate.new(text, type).render(self, local_assigns) end - def wrap_content_for_layout(content) - original_content_for_layout, @content_for_layout = @content_for_layout, content - yield - ensure - @content_for_layout = original_content_for_layout - end - # Evaluate the local assigns and pushes them to the view. def evaluate_assigns unless @assigns_added @@ -392,11 +391,5 @@ module ActionView #:nodoc: controller.response.content_type ||= content_type end end - - def execute(method, local_assigns = {}) - send(method, local_assigns) do |*names| - instance_variable_get "@content_for_#{names.first || 'layout'}" - end - end end end diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index 894b88534c..b661a62677 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -68,7 +68,7 @@ module ActionView # # <%# app/views/users/_editor.html.erb &> #
- # Deadline: $<%= user.deadline %> + # Deadline: <%= user.deadline %> # <%= yield %> #
# @@ -82,7 +82,7 @@ module ActionView # # Here's the editor: #
- # Deadline: $<%= user.deadline %> + # Deadline: <%= user.deadline %> # Name: <%= user.name %> #
# @@ -101,6 +101,40 @@ module ActionView # # # As you can see, the :locals hash is shared between both the partial and its layout. + # + # If you pass arguments to "yield" then this will be passed to the block. One way to use this is to pass + # an array to layout and treat it as an enumerable. + # + # <%# app/views/users/_user.html.erb &> + #
+ # Budget: $<%= user.budget %> + # <%= yield user %> + #
+ # + # <%# app/views/users/index.html.erb &> + # <% render :layout => @users do |user| %> + # Title: <%= user.title %> + # <% end %> + # + # This will render the layout for each user and yield to the block, passing the user, each time. + # + # You can also yield multiple times in one layout and use block arguments to differentiate the sections. + # + # <%# app/views/users/_user.html.erb &> + #
+ # <%= yield user, :header %> + # Budget: $<%= user.budget %> + # <%= yield user, :footer %> + #
+ # + # <%# app/views/users/index.html.erb &> + # <% render :layout => @users do |user, section| %> + # <%- case section when :header -%> + # Title: <%= user.title %> + # <%- when :footer -%> + # Deadline: <%= user.deadline %> + # <%- end -%> + # <% end %> module Partials extend ActiveSupport::Memoizable diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index 89ac500717..a28689dc69 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -31,7 +31,14 @@ module ActionView view.send(:evaluate_assigns) view.send(:set_controller_content_type, mime_type) if respond_to?(:mime_type) - view.send(:execute, method_name(local_assigns), local_assigns) + + view.send(method_name(local_assigns), local_assigns) do |*names| + if proc = view.instance_variable_get("@_proc_for_layout") + view.capture(*names, &proc) + else + view.instance_variable_get("@content_for_#{names.first || 'layout'}") + end + end end def method_name(local_assigns) diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index 72c01a9102..71f110f241 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -41,19 +41,19 @@ class LayoutAutoDiscoveryTest < Test::Unit::TestCase @request.host = "www.nextangle.com" end - + def test_application_layout_is_default_when_no_controller_match @controller = ProductController.new get :hello assert_equal 'layout_test.rhtml hello.rhtml', @response.body end - + def test_controller_name_layout_name_match @controller = ItemController.new get :hello assert_equal 'item.rhtml hello.rhtml', @response.body end - + def test_third_party_template_library_auto_discovers_layout ThirdPartyTemplateLibraryController.view_paths.reload! @controller = ThirdPartyTemplateLibraryController.new @@ -63,14 +63,14 @@ class LayoutAutoDiscoveryTest < Test::Unit::TestCase assert_response :success assert_equal 'Mab', @response.body end - + def test_namespaced_controllers_auto_detect_layouts @controller = ControllerNameSpace::NestedController.new get :hello assert_equal 'layouts/controller_name_space/nested', @controller.active_layout assert_equal 'controller_name_space/nested.rhtml hello.rhtml', @response.body end - + def test_namespaced_controllers_auto_detect_layouts @controller = MultipleExtensions.new get :hello @@ -115,7 +115,7 @@ class ExemptFromLayoutTest < Test::Unit::TestCase def test_rhtml_exempt_from_layout_status_should_prevent_layout_render ActionController::Base.exempt_from_layout :rhtml - + assert @controller.send!(:template_exempt_from_layout?, 'test.rhtml') assert @controller.send!(:template_exempt_from_layout?, 'hello.rhtml') @@ -156,19 +156,19 @@ class LayoutSetInResponseTest < Test::Unit::TestCase get :hello assert_equal 'layouts/layout_test', @response.layout end - + def test_layout_set_when_set_in_controller @controller = HasOwnLayoutController.new get :hello assert_equal 'layouts/item', @response.layout end - + def test_layout_set_when_using_render @controller = SetsLayoutInRenderController.new get :hello assert_equal 'layouts/third_party_template_library', @response.layout end - + def test_layout_is_not_set_when_none_rendered @controller = RendersNoLayoutController.new get :hello @@ -249,4 +249,3 @@ class LayoutSymlinkedIsRenderedTest < Test::Unit::TestCase assert_equal "layouts/symlinked/symlinked_layout", @response.layout end end - \ No newline at end of file diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb index be99350cd2..82919b7777 100644 --- a/actionpack/test/controller/new_render_test.rb +++ b/actionpack/test/controller/new_render_test.rb @@ -435,6 +435,10 @@ class NewRenderTestController < ActionController::Base render :action => "using_layout_around_block" end + def render_using_layout_around_block_with_args + render :action => "using_layout_around_block_with_args" + end + def render_using_layout_around_block_in_main_layout_and_within_content_for_layout render :action => "using_layout_around_block" end @@ -969,4 +973,9 @@ EOS get :render_using_layout_around_block_in_main_layout_and_within_content_for_layout assert_equal "Before (Anthony)\nInside from first block in layout\nAfter\nBefore (David)\nInside from block\nAfter\nBefore (Ramm)\nInside from second block in layout\nAfter\n", @response.body end + + def test_using_layout_around_block_with_args + get :render_using_layout_around_block_with_args + assert_equal "Before\narg1arg2\nAfter", @response.body + end end diff --git a/actionpack/test/fixtures/test/_layout_for_block_with_args.html.erb b/actionpack/test/fixtures/test/_layout_for_block_with_args.html.erb new file mode 100644 index 0000000000..307533208d --- /dev/null +++ b/actionpack/test/fixtures/test/_layout_for_block_with_args.html.erb @@ -0,0 +1,3 @@ +Before +<%= yield 'arg1', 'arg2' %> +After \ No newline at end of file diff --git a/actionpack/test/fixtures/test/using_layout_around_block_with_args.html.erb b/actionpack/test/fixtures/test/using_layout_around_block_with_args.html.erb new file mode 100644 index 0000000000..71b1f30ad0 --- /dev/null +++ b/actionpack/test/fixtures/test/using_layout_around_block_with_args.html.erb @@ -0,0 +1 @@ +<% render(:layout => "layout_for_block_with_args") do |*args| %><%= args.join %><% end %> \ No newline at end of file -- cgit v1.2.3 From 7fbe226de5c63565fcca67d01687d83009ab9886 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 7 Aug 2008 01:22:03 -0700 Subject: Ruby 1.9 and GC::Profiler updates --- .../lib/active_support/testing/performance.rb | 66 ++++++++++++++++++++-- 1 file changed, 60 insertions(+), 6 deletions(-) diff --git a/activesupport/lib/active_support/testing/performance.rb b/activesupport/lib/active_support/testing/performance.rb index 70a7f84023..3ad7289571 100644 --- a/activesupport/lib/active_support/testing/performance.rb +++ b/activesupport/lib/active_support/testing/performance.rb @@ -25,7 +25,7 @@ module ActiveSupport def self.included(base) base.class_inheritable_hash :profile_options - base.profile_options = DEFAULTS.dup + base.profile_options = DEFAULTS end def full_test_name @@ -34,6 +34,7 @@ module ActiveSupport def run(result) return if method_name =~ /^default_test$/ + self.profile_options ||= DEFAULTS yield(self.class::STARTED, name) @_result = result @@ -43,8 +44,6 @@ module ActiveSupport if klass = Metrics[metric_name.to_sym] run_profile(klass.new) result.add_run - else - $stderr.puts '%20s: unsupported' % metric_name.to_s end end @@ -164,7 +163,14 @@ module ActiveSupport end class Profiler < Performer + def initialize(*args) + super + @supported = @metric.measure_mode rescue false + end + def run + return unless @supported + RubyProf.measure_mode = @metric.measure_mode RubyProf.start RubyProf.pause @@ -173,7 +179,17 @@ module ActiveSupport @total = @data.threads.values.sum(0) { |method_infos| method_infos.sort.last.total_time } end + def report + if @supported + super + else + '%20s: unsupported' % @metric.name + end + end + def record + return unless @supported + klasses = profile_options[:formats].map { |f| RubyProf.const_get("#{f.to_s.camelize}Printer") }.compact klasses.each do |klass| @@ -202,8 +218,7 @@ module ActiveSupport module Metrics def self.[](name) - klass = const_get(name.to_s.camelize) - klass if klass::Mode + const_get(name.to_s.camelize) rescue NameError nil end @@ -250,6 +265,16 @@ module ActiveSupport ensure GC.disable_stats end + elsif defined?(GC::Profiler) + def with_gc_stats + GC.start + GC.disable + GC::Profiler.enable + yield + ensure + GC::Profiler.disable + GC.enable + end else def with_gc_stats yield @@ -310,7 +335,7 @@ module ActiveSupport RubyProf.measure_memory / 1024.0 end - # Ruby 1.8 + adymo patch + # Ruby 1.8 + railsbench patch elsif GC.respond_to?(:allocated_size) def measure GC.allocated_size / 1024.0 @@ -322,11 +347,27 @@ module ActiveSupport GC.heap_info['heap_current_memory'] / 1024.0 end + # Ruby 1.9 with total_malloc_allocated_size patch + elsif GC.respond_to?(:malloc_total_allocated_size) + def measure + GC.total_malloc_allocated_size / 1024.0 + end + # Ruby 1.9 unpatched elsif GC.respond_to?(:malloc_allocated_size) def measure GC.malloc_allocated_size / 1024.0 end + + # Ruby 1.9 + GC profiler patch + elsif defined?(GC::Profiler) + def measure + GC.enable + GC.start + kb = GC::Profiler.data.last[:HEAP_USE_SIZE] / 1024.0 + GC.disable + kb + end end def format(measurement) @@ -341,10 +382,23 @@ module ActiveSupport def measure RubyProf.measure_allocations end + + # Ruby 1.8 + railsbench patch elsif ObjectSpace.respond_to?(:allocated_objects) def measure ObjectSpace.allocated_objects end + + # Ruby 1.9 + GC profiler patch + elsif defined?(GC::Profiler) + def measure + GC.enable + GC.start + last = GC::Profiler.data.last + count = last[:HEAP_LIVE_OBJECTS] + last[:HEAP_FREE_OBJECTS] + GC.disable + count + end end def format(measurement) -- cgit v1.2.3 From cd8e653d5b18e6d3c3acc9930832f8e23945e392 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 11 Aug 2008 10:25:57 -0700 Subject: Performance: freeze cached rows instead of duping --- activerecord/lib/active_record/base.rb | 2 +- .../connection_adapters/abstract/query_cache.rb | 17 ++++------------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 6eb4d42d51..5357255bad 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -612,7 +612,7 @@ module ActiveRecord #:nodoc: # Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date] # > [#"The Cheap Man Buys Twice"}>, ...] def find_by_sql(sql) - connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) } + connection.select_all(sanitize_sql(sql), "#{name} Load").map { |record| instantiate(record) } end # Checks whether a record exists in the database that matches conditions given. These conditions diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 2afd6064ad..81a2e56b34 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -72,21 +72,12 @@ module ActiveRecord private def cache_sql(sql) - result = - if @query_cache.has_key?(sql) - log_info(sql, "CACHE", 0.0) - @query_cache[sql] - else - @query_cache[sql] = yield - end - - if Array === result - result.collect { |row| row.dup } + if @query_cache.has_key?(sql) + log_info(sql, "CACHE", 0.0) + @query_cache[sql] else - result.duplicable? ? result.dup : result + @query_cache[sql] = yield.freeze end - rescue TypeError - result end end end -- cgit v1.2.3 From a4da8175a2c989104de1a38e43d5ddfb0f89b055 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 18 Aug 2008 20:14:56 -0500 Subject: Replace MemoryStore mutex with a monitor to avoid issues with nested calls --- .../lib/active_support/cache/memory_store.rb | 36 ++++++++++++++-------- activesupport/test/caching_test.rb | 24 +++++++++++++++ 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/activesupport/lib/active_support/cache/memory_store.rb b/activesupport/lib/active_support/cache/memory_store.rb index a44f877414..7ac6f35357 100644 --- a/activesupport/lib/active_support/cache/memory_store.rb +++ b/activesupport/lib/active_support/cache/memory_store.rb @@ -3,51 +3,63 @@ module ActiveSupport class MemoryStore < Store def initialize @data = {} - @mutex = Mutex.new + @guard = Monitor.new end def fetch(key, options = {}) - @mutex.synchronize do + @guard.synchronize do super end end def read(name, options = nil) - super - @data[name] + @guard.synchronize do + super + @data[name] + end end def write(name, value, options = nil) - super - @data[name] = value + @guard.synchronize do + super + @data[name] = value + end end def delete(name, options = nil) - @data.delete(name) + @guard.synchronize do + @data.delete(name) + end end def delete_matched(matcher, options = nil) - @data.delete_if { |k,v| k =~ matcher } + @guard.synchronize do + @data.delete_if { |k,v| k =~ matcher } + end end def exist?(name,options = nil) - @data.has_key?(name) + @guard.synchronize do + @data.has_key?(name) + end end def increment(key, amount = 1) - @mutex.synchronize do + @guard.synchronize do super end end def decrement(key, amount = 1) - @mutex.synchronize do + @guard.synchronize do super end end def clear - @data.clear + @guard.synchronize do + @data.clear + end end end end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index c5f7fb7fdd..ce27b464f8 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -97,3 +97,27 @@ class FileStoreTest < Test::Unit::TestCase File.delete("foo.cache") end end + +class MemoryStoreTest < Test::Unit::TestCase + def setup + @cache = ActiveSupport::Cache.lookup_store(:memory_store) + end + + def test_should_read_and_write + @cache.write('foo', 'bar') + assert_equal 'bar', @cache.read('foo') + end + + def test_fetch_without_cache_miss + @cache.write('foo', 'bar') + assert_equal 'bar', @cache.fetch('foo') { 'baz' } + end + + def test_fetch_with_cache_miss + assert_equal 'baz', @cache.fetch('foo') { 'baz' } + end + + def test_fetch_with_forced_cache_miss + @cache.fetch('foo', :force => true) { 'bar' } + end +end -- cgit v1.2.3 From c1a8690d582c08777055caf449c03f85b4c8aa4b Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 18 Aug 2008 22:38:58 -0500 Subject: Consistently use the framework's configured logger and avoid reverting to RAILS_DEFAULT_LOGGER unless necessary. --- actionpack/lib/action_controller/dispatcher.rb | 4 ++-- actionpack/lib/action_view/base.rb | 3 ++- actionpack/lib/action_view/paths.rb | 2 +- actionpack/lib/action_view/renderable.rb | 2 +- activerecord/test/connections/native_mysql/connection.rb | 4 +--- activeresource/lib/active_resource/connection.rb | 16 ++++++++-------- activesupport/lib/active_support/cache/file_store.rb | 4 ++-- .../lib/active_support/core_ext/kernel/debugger.rb | 8 ++++---- activesupport/lib/active_support/dependencies.rb | 10 +++++++--- railties/lib/fcgi_handler.rb | 4 +--- railties/lib/initializer.rb | 15 ++++++++++----- 11 files changed, 39 insertions(+), 33 deletions(-) diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb index 835d8e834e..7e46f572fe 100644 --- a/actionpack/lib/action_controller/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatcher.rb @@ -24,7 +24,7 @@ module ActionController to_prepare(:activerecord_instantiate_observers) { ActiveRecord::Base.instantiate_observers } end - after_dispatch :flush_logger if defined?(RAILS_DEFAULT_LOGGER) && RAILS_DEFAULT_LOGGER.respond_to?(:flush) + after_dispatch :flush_logger if Base.logger && Base.logger.respond_to?(:flush) end # Backward-compatible class method takes CGI-specific args. Deprecated @@ -142,7 +142,7 @@ module ActionController end def flush_logger - RAILS_DEFAULT_LOGGER.flush + Base.logger.flush end protected diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index f7f9f70298..62a01b33dd 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -169,6 +169,7 @@ module ActionView #:nodoc: class << self delegate :erb_trim_mode=, :to => 'ActionView::TemplateHandlers::ERB' + delegate :logger, :to => 'ActionController::Base' end def self.cache_template_loading=(*args) @@ -328,7 +329,7 @@ module ActionView #:nodoc: else template = Template.new(template_path, view_paths) - if self.class.warn_cache_misses && logger = ActionController::Base.logger + if self.class.warn_cache_misses && logger logger.debug "[PERFORMANCE] Rendering a template that was " + "not found in view path. Templates outside the view path are " + "not cached and result in expensive disk operations. Move this " + diff --git a/actionpack/lib/action_view/paths.rb b/actionpack/lib/action_view/paths.rb index d97f963540..6a118a1cfa 100644 --- a/actionpack/lib/action_view/paths.rb +++ b/actionpack/lib/action_view/paths.rb @@ -3,7 +3,7 @@ module ActionView #:nodoc: def self.type_cast(obj) if obj.is_a?(String) if Base.warn_cache_misses && defined?(Rails) && Rails.initialized? - Rails.logger.debug "[PERFORMANCE] Processing view path during a " + + Base.logger.debug "[PERFORMANCE] Processing view path during a " + "request. This an expense disk operation that should be done at " + "boot. You can manually process this view path with " + "ActionView::Base.process_view_paths(#{obj.inspect}) and set it " + diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index a28689dc69..c011f21550 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -72,7 +72,7 @@ module ActionView end_src begin - logger = ActionController::Base.logger + logger = Base.logger logger.debug "Compiling template #{render_symbol}" if logger ActionView::Base::CompiledTemplates.module_eval(source, filename, 0) diff --git a/activerecord/test/connections/native_mysql/connection.rb b/activerecord/test/connections/native_mysql/connection.rb index 1fab444e58..140e06d631 100644 --- a/activerecord/test/connections/native_mysql/connection.rb +++ b/activerecord/test/connections/native_mysql/connection.rb @@ -2,9 +2,7 @@ print "Using native MySQL\n" require_dependency 'models/course' require 'logger' -RAILS_DEFAULT_LOGGER = Logger.new('debug.log') -RAILS_DEFAULT_LOGGER.level = Logger::DEBUG -ActiveRecord::Base.logger = RAILS_DEFAULT_LOGGER +ActiveRecord::Base.logger = Logger.new("debug.log") # GRANT ALL PRIVILEGES ON activerecord_unittest.* to 'rails'@'localhost'; # GRANT ALL PRIVILEGES ON activerecord_unittest2.* to 'rails'@'localhost'; diff --git a/activeresource/lib/active_resource/connection.rb b/activeresource/lib/active_resource/connection.rb index a4a61b61b5..d64fb79f1e 100644 --- a/activeresource/lib/active_resource/connection.rb +++ b/activeresource/lib/active_resource/connection.rb @@ -28,24 +28,24 @@ module ActiveResource # 3xx Redirection class Redirection < ConnectionError # :nodoc: - def to_s; response['Location'] ? "#{super} => #{response['Location']}" : super; end - end + def to_s; response['Location'] ? "#{super} => #{response['Location']}" : super; end + end # 4xx Client Error class ClientError < ConnectionError; end # :nodoc: - + # 400 Bad Request class BadRequest < ClientError; end # :nodoc - + # 401 Unauthorized class UnauthorizedAccess < ClientError; end # :nodoc - + # 403 Forbidden class ForbiddenAccess < ClientError; end # :nodoc - + # 404 Not Found class ResourceNotFound < ClientError; end # :nodoc: - + # 409 Conflict class ResourceConflict < ClientError; end # :nodoc: @@ -201,7 +201,7 @@ module ActiveResource end def logger #:nodoc: - ActiveResource::Base.logger + Base.logger end end end diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 437679cc05..659bde64f0 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -17,7 +17,7 @@ module ActiveSupport ensure_cache_path(File.dirname(real_file_path(name))) File.atomic_write(real_file_path(name), cache_path) { |f| Marshal.dump(value, f) } rescue => e - RAILS_DEFAULT_LOGGER.error "Couldn't create cache directory: #{name} (#{e.message})" if RAILS_DEFAULT_LOGGER + logger.error "Couldn't create cache directory: #{name} (#{e.message})" if logger end def delete(name, options = nil) @@ -67,4 +67,4 @@ module ActiveSupport end end end -end \ No newline at end of file +end diff --git a/activesupport/lib/active_support/core_ext/kernel/debugger.rb b/activesupport/lib/active_support/core_ext/kernel/debugger.rb index c74d6cf884..4007a647be 100644 --- a/activesupport/lib/active_support/core_ext/kernel/debugger.rb +++ b/activesupport/lib/active_support/core_ext/kernel/debugger.rb @@ -2,12 +2,12 @@ module Kernel unless respond_to?(:debugger) # Starts a debugging session if ruby-debug has been loaded (call script/server --debugger to do load it). def debugger - RAILS_DEFAULT_LOGGER.info "\n***** Debugger requested, but was not available: Start server with --debugger to enable *****\n" + Rails.logger.info "\n***** Debugger requested, but was not available: Start server with --debugger to enable *****\n" end end - + def breakpoint - RAILS_DEFAULT_LOGGER.info "\n***** The 'breakpoint' command has been renamed 'debugger' -- please change *****\n" + Rails.logger.info "\n***** The 'breakpoint' command has been renamed 'debugger' -- please change *****\n" debugger end -end \ No newline at end of file +end diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index a3f5f799a2..3d871eec11 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -39,6 +39,10 @@ module ActiveSupport #:nodoc: mattr_accessor :explicitly_unloadable_constants self.explicitly_unloadable_constants = [] + # The logger is used for generating information on the action run-time (including benchmarking) if available. + # Can be set to nil for no logging. Compatible with both Ruby's own Logger and Log4r loggers. + mattr_accessor :logger + # Set to true to enable logging of const_missing and file loads mattr_accessor :log_activity self.log_activity = false @@ -584,7 +588,7 @@ module ActiveSupport #:nodoc: protected def log_call(*args) - if defined?(RAILS_DEFAULT_LOGGER) && RAILS_DEFAULT_LOGGER && log_activity + if logger && log_activity arg_str = args.collect { |arg| arg.inspect } * ', ' /in `([a-z_\?\!]+)'/ =~ caller(1).first selector = $1 || '' @@ -593,8 +597,8 @@ module ActiveSupport #:nodoc: end def log(msg) - if defined?(RAILS_DEFAULT_LOGGER) && RAILS_DEFAULT_LOGGER && log_activity - RAILS_DEFAULT_LOGGER.debug "Dependencies: #{msg}" + if logger && log_activity + logger.debug "Dependencies: #{msg}" end end end diff --git a/railties/lib/fcgi_handler.rb b/railties/lib/fcgi_handler.rb index 722aa1940c..1bb55b9275 100644 --- a/railties/lib/fcgi_handler.rb +++ b/railties/lib/fcgi_handler.rb @@ -18,7 +18,6 @@ class RailsFCGIHandler attr_accessor :log_file_path attr_accessor :gc_request_period - # Initialize and run the FastCGI instance, passing arguments through to new. def self.process!(*args, &block) new(*args, &block).process! @@ -68,7 +67,6 @@ class RailsFCGIHandler end end - protected def process_each_request(provider) cgi = nil @@ -197,7 +195,7 @@ class RailsFCGIHandler # close resources as they won't be closed by # the OS when using exec logger.close rescue nil - RAILS_DEFAULT_LOGGER.close rescue nil + Rails.logger.close rescue nil exec(command_line) end diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 6576cd368b..70c6a629ec 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -33,7 +33,11 @@ module Rails end def logger - RAILS_DEFAULT_LOGGER + if defined?(RAILS_DEFAULT_LOGGER) + RAILS_DEFAULT_LOGGER + else + nil + end end def root @@ -403,7 +407,7 @@ Run `rake gems:install` to install the missing gems. # +STDERR+, with a log level of +WARN+. def initialize_logger # if the environment has explicitly defined a logger, use it - return if defined?(RAILS_DEFAULT_LOGGER) + return if Rails.logger unless logger = configuration.logger begin @@ -431,10 +435,11 @@ Run `rake gems:install` to install the missing gems. # RAILS_DEFAULT_LOGGER. def initialize_framework_logging for framework in ([ :active_record, :action_controller, :action_mailer ] & configuration.frameworks) - framework.to_s.camelize.constantize.const_get("Base").logger ||= RAILS_DEFAULT_LOGGER + framework.to_s.camelize.constantize.const_get("Base").logger ||= Rails.logger end - RAILS_CACHE.logger ||= RAILS_DEFAULT_LOGGER + ActiveSupport::Dependencies.logger ||= Rails.logger + Rails.cache.logger ||= Rails.logger end # Sets +ActionController::Base#view_paths+ and +ActionMailer::Base#template_root+ @@ -531,7 +536,7 @@ Run `rake gems:install` to install the missing gems. return unless configuration.frameworks.include?(:action_controller) require 'dispatcher' unless defined?(::Dispatcher) Dispatcher.define_dispatcher_callbacks(configuration.cache_classes) - Dispatcher.new(RAILS_DEFAULT_LOGGER).send :run_callbacks, :prepare_dispatch + Dispatcher.new(Rails.logger).send :run_callbacks, :prepare_dispatch end def disable_dependency_loading -- cgit v1.2.3 From 96ab01e8f2b5a4475453acf60f9cf9bd8cd98483 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 18 Aug 2008 23:33:46 -0500 Subject: Maintain a seperate buffer for each thread --- .../lib/active_support/buffered_logger.rb | 19 ++++++++------ activesupport/test/buffered_logger_test.rb | 29 +++++++++++++++++++--- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/activesupport/lib/active_support/buffered_logger.rb b/activesupport/lib/active_support/buffered_logger.rb index cedc1afe7f..6553d72b4f 100644 --- a/activesupport/lib/active_support/buffered_logger.rb +++ b/activesupport/lib/active_support/buffered_logger.rb @@ -33,11 +33,10 @@ module ActiveSupport attr_accessor :level attr_reader :auto_flushing - attr_reader :buffer def initialize(log, level = DEBUG) @level = level - @buffer = [] + @buffer = {} @auto_flushing = 1 @guard = Mutex.new @@ -60,9 +59,7 @@ module ActiveSupport # If a newline is necessary then create a new message ending with a newline. # Ensures that the original message is not mutated. message = "#{message}\n" unless message[-1] == ?\n - @guard.synchronize do - buffer << message - end + buffer << message auto_flush message end @@ -96,8 +93,8 @@ module ActiveSupport def flush @guard.synchronize do unless buffer.empty? - old_buffer = @buffer - @buffer = [] + old_buffer = buffer + clear_buffer @log.write(old_buffer.join) end end @@ -113,5 +110,13 @@ module ActiveSupport def auto_flush flush if buffer.size >= @auto_flushing end + + def buffer + @buffer[Thread.current] ||= [] + end + + def clear_buffer + @buffer[Thread.current] = [] + end end end diff --git a/activesupport/test/buffered_logger_test.rb b/activesupport/test/buffered_logger_test.rb index 97649518b7..6319c09210 100644 --- a/activesupport/test/buffered_logger_test.rb +++ b/activesupport/test/buffered_logger_test.rb @@ -70,7 +70,7 @@ class BufferedLoggerTest < Test::Unit::TestCase end @logger.flush - assert !@output.string.empty?, @logger.buffer.size + assert !@output.string.empty?, @logger.send(:buffer).size end define_method "test_disabling_auto_flush_with_#{disable.inspect}_should_flush_at_max_buffer_size_as_failsafe" do @@ -83,10 +83,10 @@ class BufferedLoggerTest < Test::Unit::TestCase end @logger.info 'there it is.' - assert !@output.string.empty?, @logger.buffer.size + assert !@output.string.empty?, @logger.send(:buffer).size end end - + def test_should_know_if_its_loglevel_is_below_a_given_level ActiveSupport::BufferedLogger::Severity.constants.each do |level| @logger.level = ActiveSupport::BufferedLogger::Severity.const_get(level) - 1 @@ -105,7 +105,7 @@ class BufferedLoggerTest < Test::Unit::TestCase @logger.info 'there it is.' assert !@output.string.empty?, @output.string end - + def test_should_create_the_log_directory_if_it_doesnt_exist tmp_directory = File.join(File.dirname(__FILE__), "tmp") log_file = File.join(tmp_directory, "development.log") @@ -115,4 +115,25 @@ class BufferedLoggerTest < Test::Unit::TestCase ensure FileUtils.rm_rf(tmp_directory) end + + def test_logger_should_maintain_separate_buffers_for_each_thread + @logger.auto_flushing = false + + a = Thread.new do + @logger.info("a"); Thread.pass; + @logger.info("b"); Thread.pass; + @logger.info("c"); @logger.flush + end + + b = Thread.new do + @logger.info("x"); Thread.pass; + @logger.info("y"); Thread.pass; + @logger.info("z"); @logger.flush + end + + a.join + b.join + + assert_equal "a\nb\nc\nx\ny\nz\n", @output.string + end end -- cgit v1.2.3 From e9ae2b2f4cae3e9ba4fc8ce91de951d18879af8b Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 19 Aug 2008 00:18:26 -0500 Subject: Added rack logger middleware that tails the environment log --- railties/lib/rails/rack.rb | 1 + railties/lib/rails/rack/logger.rb | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 railties/lib/rails/rack/logger.rb diff --git a/railties/lib/rails/rack.rb b/railties/lib/rails/rack.rb index abcd0741bf..b4f32c2d95 100644 --- a/railties/lib/rails/rack.rb +++ b/railties/lib/rails/rack.rb @@ -1,5 +1,6 @@ module Rails module Rack + autoload :Logger, "rails/rack/logger" autoload :Static, "rails/rack/static" end end diff --git a/railties/lib/rails/rack/logger.rb b/railties/lib/rails/rack/logger.rb new file mode 100644 index 0000000000..89d02e45a9 --- /dev/null +++ b/railties/lib/rails/rack/logger.rb @@ -0,0 +1,28 @@ +module Rails + module Rack + class Logger + EnvironmentLog = "#{File.expand_path(Rails.root)}/log/#{Rails.env}.log" + + def initialize(app, log = nil) + @app = app + @path = Pathname.new(log || EnvironmentLog).cleanpath + @cursor = ::File.size(@path) + @last_checked = Time.now + end + + def call(env) + response = @app.call(env) + ::File.open(@path, 'r') do |f| + f.seek @cursor + if f.mtime > @last_checked + contents = f.read + @last_checked = f.mtime + @cursor += contents.length + print contents + end + end + response + end + end + end +end -- cgit v1.2.3 From 977317da55b6ba9bbb1326392516480902145c5b Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 19 Aug 2008 16:29:02 -0500 Subject: hack around CGI session close --- actionpack/lib/action_controller/rack_process.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/rack_process.rb b/actionpack/lib/action_controller/rack_process.rb index 221613b854..69bfc549c1 100644 --- a/actionpack/lib/action_controller/rack_process.rb +++ b/actionpack/lib/action_controller/rack_process.rb @@ -156,6 +156,10 @@ end_msg end def out(output = $stdout, &block) + # Nasty hack because CGI sessions are closed after the normal + # prepare! statement + set_cookies! + @block = block @status = headers.delete("Status") if [204, 304].include?(status.to_i) @@ -200,7 +204,7 @@ end_msg convert_language! convert_expires! set_status! - set_cookies! + # set_cookies! end private -- cgit v1.2.3 From bd7edcf286421b090b8de5674422a7316ed043a9 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 19 Aug 2008 16:46:15 -0500 Subject: Removed config.ru template from app generator --- .../lib/rails_generator/generators/applications/app/app_generator.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/railties/lib/rails_generator/generators/applications/app/app_generator.rb b/railties/lib/rails_generator/generators/applications/app/app_generator.rb index 98fe163455..80e8eabfd3 100644 --- a/railties/lib/rails_generator/generators/applications/app/app_generator.rb +++ b/railties/lib/rails_generator/generators/applications/app/app_generator.rb @@ -46,7 +46,6 @@ class AppGenerator < Rails::Generator::Base # Root m.file "fresh_rakefile", "Rakefile" m.file "README", "README" - m.file "config.ru", "config.ru" # Application m.template "helpers/application.rb", "app/controllers/application.rb", :assigns => { :app_name => @app_name, :app_secret => md5.hexdigest } -- cgit v1.2.3 From 6e4ea66dc0d41ef611e2b2187e940321f5dc748a Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Tue, 19 Aug 2008 16:07:17 -0600 Subject: Make AbstractRequest.if_modified_sense return nil if the header could not be parsed --- actionpack/CHANGELOG | 2 ++ actionpack/lib/action_controller/request.rb | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 6f65d4003d..db3378d9d9 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Made AbstractRequest.if_modified_sense return nil if the header could not be parsed [Jamis Buck] + * Added back ActionController::Base.allow_concurrency flag [Josh Peek] * AbstractRequest.relative_url_root is no longer automatically configured by a HTTP header. It can now be set in your configuration environment with config.action_controller.relative_url_root [Josh Peek] diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 90bced14e6..364e6201cc 100644 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -100,7 +100,7 @@ module ActionController def if_modified_since if since = env['HTTP_IF_MODIFIED_SINCE'] - Time.rfc2822(since) + Time.rfc2822(since) rescue nil end end memoize :if_modified_since -- cgit v1.2.3 From 5df8ff1d6b6c35a7b5924f95ad12693984323513 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 19 Aug 2008 17:14:17 -0500 Subject: Touch file with git revision when freezing edge --- railties/lib/tasks/framework.rake | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/railties/lib/tasks/framework.rake b/railties/lib/tasks/framework.rake index 71aea09867..66ab78c3b2 100644 --- a/railties/lib/tasks/framework.rake +++ b/railties/lib/tasks/framework.rake @@ -43,9 +43,12 @@ namespace :rails do require 'open-uri' version = ENV["RELEASE"] || "edge" target = "rails_#{version}.zip" + commits = "http://github.com/api/v1/yaml/rails/rails/commits/master" url = "http://dev.rubyonrails.org/archives/#{target}" chdir 'vendor' do + latest_revision = YAML.load(open(commits))["commits"].first["id"] + puts "Downloading Rails from #{url}" File.open('rails.zip', 'wb') do |dst| open url do |src| @@ -61,6 +64,8 @@ namespace :rails do %w(rails.zip rails/Rakefile rails/cleanlogs.sh rails/pushgems.rb rails/release.rb).each do |goner| rm_f goner end + + touch "rails/REVISION_#{latest_revision}" end puts 'Updating current scripts, javascripts, and configuration settings' -- cgit v1.2.3 From 71c4ff07ab4313c1f4781d59ad2f4528f5875665 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 19 Aug 2008 18:53:46 -0500 Subject: Delegate xhr helper method to integration session --- actionpack/lib/action_controller/integration.rb | 2 +- actionpack/test/controller/integration_test.rb | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_controller/integration.rb b/actionpack/lib/action_controller/integration.rb index 1d2b81355c..ca5923de2f 100644 --- a/actionpack/lib/action_controller/integration.rb +++ b/actionpack/lib/action_controller/integration.rb @@ -451,7 +451,7 @@ EOF end %w(get post put head delete cookies assigns - xml_http_request get_via_redirect post_via_redirect).each do |method| + xml_http_request xhr get_via_redirect post_via_redirect).each do |method| define_method(method) do |*args| reset! unless @integration_session # reset the html_document variable, but only for new get/post calls diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index a709343a94..b878962df3 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -253,7 +253,10 @@ class IntegrationProcessTest < ActionController::IntegrationTest session :off def get - render :text => "OK", :status => 200 + respond_to do |format| + format.html { render :text => "OK", :status => 200 } + format.js { render :text => "JS OK", :status => 200 } + end end def post @@ -345,6 +348,20 @@ class IntegrationProcessTest < ActionController::IntegrationTest end end + def test_xml_http_request_get + with_test_route_set do + xhr :get, '/get' + assert_equal 200, status + assert_equal "OK", status_message + assert_equal "200 OK", response.headers["Status"] + assert_equal ["200 OK"], headers["status"] + assert_response 200 + assert_response :success + assert_response :ok + assert_equal "JS OK", response.body + end + end + private def with_test_route_set with_routing do |set| -- cgit v1.2.3 From a8ece12fe2ac7838407954453e0d31af6186a5db Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Tue, 19 Aug 2008 19:09:04 -0500 Subject: Return nil instead of a space when passing an empty collection or nil to 'render :partial' [#791 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_controller/base.rb | 9 ++++++--- actionpack/lib/action_view/base.rb | 2 +- actionpack/lib/action_view/partials.rb | 2 +- actionpack/test/controller/render_test.rb | 2 +- actionpack/test/template/render_test.rb | 8 ++++++++ 5 files changed, 17 insertions(+), 6 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 0fdbcbd26f..09414e7702 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -939,8 +939,7 @@ module ActionController #:nodoc: render_for_text(generator.to_s, options[:status]) elsif options[:nothing] - # Safari doesn't pass the headers of the return if the response is zero length - render_for_text(" ", options[:status]) + render_for_text(nil, options[:status]) else render_for_file(default_template_name, options[:status], true) @@ -1154,7 +1153,11 @@ module ActionController #:nodoc: response.body ||= '' response.body << text.to_s else - response.body = text.is_a?(Proc) ? text : text.to_s + response.body = case text + when Proc then text + when nil then " " # Safari doesn't pass the headers of the return if the response is zero length + else text.to_s + end end end diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 62a01b33dd..46bacbcbc1 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -263,7 +263,7 @@ module ActionView #:nodoc: end elsif options[:file] render_file(options[:file], nil, options[:locals]) - elsif options[:partial] && options[:collection] + elsif options[:partial] && options.has_key?(:collection) render_partial_collection(options[:partial], options[:collection], options[:spacer_template], options[:locals], options[:as]) elsif options[:partial] render_partial(options[:partial], options[:object], options[:locals]) diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index b661a62677..074ba5a2b5 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -161,7 +161,7 @@ module ActionView end def render_partial_collection(partial_path, collection, partial_spacer_template = nil, local_assigns = {}, as = nil) #:nodoc: - return " " if collection.empty? + return nil if collection.blank? local_assigns = local_assigns ? local_assigns.clone : {} spacer = partial_spacer_template ? render(:partial => partial_spacer_template) : '' diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 1b9b12acc6..3008f5ca03 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -329,7 +329,7 @@ class RenderTest < Test::Unit::TestCase def test_render_text_with_nil get :render_text_with_nil assert_response 200 - assert_equal '', @response.body + assert_equal ' ', @response.body end def test_render_text_with_false diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 31cfdce531..25bbc263dd 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -87,6 +87,14 @@ class ViewRenderTest < Test::Unit::TestCase @view.render(:partial => "test/local_inspector", :collection => [ Customer.new("mary") ]) end + def test_render_partial_with_empty_collection_should_return_nil + assert_nil @view.render(:partial => "test/customer", :collection => []) + end + + def test_render_partial_with_nil_collection_should_return_nil + assert_nil @view.render(:partial => "test/customer", :collection => nil) + end + # TODO: The reason for this test is unclear, improve documentation def test_render_partial_and_fallback_to_layout assert_equal "Before (Josh)\n\nAfter", @view.render(:partial => "test/layout_for_partial", :locals => { :name => "Josh" }) -- cgit v1.2.3 From 5de340e79f1d11973b7c7bbec82f320fc92b9c99 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 19 Aug 2008 19:20:10 -0500 Subject: Ensure objects cached with MemoryStore are immutable --- activesupport/lib/active_support/cache/memory_store.rb | 2 +- activesupport/test/caching_test.rb | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/cache/memory_store.rb b/activesupport/lib/active_support/cache/memory_store.rb index 7ac6f35357..f3e4b8c13b 100644 --- a/activesupport/lib/active_support/cache/memory_store.rb +++ b/activesupport/lib/active_support/cache/memory_store.rb @@ -22,7 +22,7 @@ module ActiveSupport def write(name, value, options = nil) @guard.synchronize do super - @data[name] = value + @data[name] = value.freeze end end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index ce27b464f8..9ea9389448 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -120,4 +120,10 @@ class MemoryStoreTest < Test::Unit::TestCase def test_fetch_with_forced_cache_miss @cache.fetch('foo', :force => true) { 'bar' } end + + def test_store_objects_should_be_immutable + @cache.write('foo', 'bar') + assert_raise(ActiveSupport::FrozenObjectError) { @cache.read('foo').gsub!(/.*/, 'baz') } + assert_equal 'bar', @cache.read('foo') + end end -- cgit v1.2.3 From 6f530de9443a4e2ab3d193624ff6ecd3c2b06de2 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 19 Aug 2008 20:15:51 -0500 Subject: Test coverage for integration testing with parameters --- actionpack/test/controller/integration_test.rb | 32 ++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index b878962df3..a5373912f5 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -259,6 +259,10 @@ class IntegrationProcessTest < ActionController::IntegrationTest end end + def get_with_params + render :text => "foo: #{params[:foo]}", :status => 200 + end + def post render :text => "Created", :status => 201 end @@ -362,6 +366,34 @@ class IntegrationProcessTest < ActionController::IntegrationTest end end + def test_get_with_query_string + with_test_route_set do + get '/get_with_params?foo=bar' + assert_equal '/get_with_params?foo=bar', request.env["REQUEST_URI"] + assert_equal '/get_with_params?foo=bar', request.request_uri + assert_equal nil, request.env["QUERY_STRING"] + assert_equal 'foo=bar', request.query_string + assert_equal 'bar', request.parameters['foo'] + + assert_equal 200, status + assert_equal "foo: bar", response.body + end + end + + def test_get_with_parameters + with_test_route_set do + get '/get_with_params', :foo => "bar" + assert_equal '/get_with_params', request.env["REQUEST_URI"] + assert_equal '/get_with_params', request.request_uri + assert_equal 'foo=bar', request.env["QUERY_STRING"] + assert_equal 'foo=bar', request.query_string + assert_equal 'bar', request.parameters['foo'] + + assert_equal 200, status + assert_equal "foo: bar", response.body + end + end + private def with_test_route_set with_routing do |set| -- cgit v1.2.3 From 3a2ff17af66dfb135ead212de458e7f6860c8004 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 19 Aug 2008 20:22:27 -0500 Subject: Don't shadow query string method --- actionpack/lib/action_controller/rack_process.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/rack_process.rb b/actionpack/lib/action_controller/rack_process.rb index 69bfc549c1..1ace16da07 100644 --- a/actionpack/lib/action_controller/rack_process.rb +++ b/actionpack/lib/action_controller/rack_process.rb @@ -25,7 +25,7 @@ module ActionController #:nodoc: end %w[ AUTH_TYPE GATEWAY_INTERFACE PATH_INFO - PATH_TRANSLATED QUERY_STRING REMOTE_HOST + PATH_TRANSLATED REMOTE_HOST REMOTE_IDENT REMOTE_USER SCRIPT_NAME SERVER_NAME SERVER_PROTOCOL @@ -37,6 +37,15 @@ module ActionController #:nodoc: end end + def query_string + qs = super + if !qs.blank? + qs + else + @env['QUERY_STRING'] + end + end + def body_stream #:nodoc: @env['rack.input'] end -- cgit v1.2.3 From a74dbe6c528a46f11e709386c282ead8049840ba Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 20 Aug 2008 13:22:36 -0500 Subject: Improve test coverage for integration tests cookie header --- actionpack/test/controller/integration_test.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index a5373912f5..47ff2dbbd3 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -329,7 +329,10 @@ class IntegrationProcessTest < ActionController::IntegrationTest assert_response :gone assert_equal nil, response.headers["Set-Cookie"] assert_equal ["cookie_1=; path=/", "cookie_3=chocolate; path=/"], headers['set-cookie'] - assert_equal [[], ["chocolate"]], response.headers["cookie"] + assert_equal [ + CGI::Cookie::new("name" => "cookie_1", "value" => ""), + CGI::Cookie::new("name" => "cookie_3", "value" => "chocolate") + ], response.headers["cookie"] assert_equal [], headers["cookie"] assert_equal({"cookie_1"=>"", "cookie_2"=>"oatmeal", "cookie_3"=>"chocolate"}, cookies) assert_equal "Gone", response.body -- cgit v1.2.3 From 47cd8b81cc19345aa68323755c78ab03f8176590 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 20 Aug 2008 13:37:18 -0500 Subject: Switched integration test runner to use Rack processor instead of CGI --- actionpack/CHANGELOG | 2 + actionpack/lib/action_controller/integration.rb | 76 +++++++++++-------------- actionpack/test/controller/integration_test.rb | 2 +- 3 files changed, 35 insertions(+), 45 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index db3378d9d9..b471be1df6 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Switched integration test runner to use Rack processor instead of CGI [Josh Peek] + * Made AbstractRequest.if_modified_sense return nil if the header could not be parsed [Jamis Buck] * Added back ActionController::Base.allow_concurrency flag [Josh Peek] diff --git a/actionpack/lib/action_controller/integration.rb b/actionpack/lib/action_controller/integration.rb index ca5923de2f..198a22e8dc 100644 --- a/actionpack/lib/action_controller/integration.rb +++ b/actionpack/lib/action_controller/integration.rb @@ -228,21 +228,6 @@ module ActionController end private - class StubCGI < CGI #:nodoc: - attr_accessor :stdinput, :stdoutput, :env_table - - def initialize(env, stdinput = nil) - self.env_table = env - self.stdoutput = StringIO.new - - super - - stdinput.set_encoding(Encoding::BINARY) if stdinput.respond_to?(:set_encoding) - stdinput.force_encoding(Encoding::BINARY) if stdinput.respond_to?(:force_encoding) - @stdinput = stdinput.is_a?(IO) ? stdinput : StringIO.new(stdinput || '') - end - end - # Tailors the session based on the given URI, setting the HTTPS value # and the hostname. def interpret_uri(path) @@ -290,9 +275,8 @@ module ActionController ActionController::Base.clear_last_instantiation! - cgi = StubCGI.new(env, data) - ActionController::Dispatcher.dispatch(cgi, ActionController::CgiRequest::DEFAULT_SESSION_OPTIONS, cgi.stdoutput) - @result = cgi.stdoutput.string + env['rack.input'] = data.is_a?(IO) ? data : StringIO.new(data || '') + @status, @headers, result_body = ActionController::Dispatcher.new.call(env) @request_count += 1 @controller = ActionController::Base.last_instantiation @@ -306,32 +290,34 @@ module ActionController @html_document = nil - parse_result - return status - rescue MultiPartNeededException - boundary = "----------XnJLe9ZIbbGUYtzPQJ16u1" - status = process(method, path, multipart_body(parameters, boundary), (headers || {}).merge({"CONTENT_TYPE" => "multipart/form-data; boundary=#{boundary}"})) - return status - end + # Inject status back in for backwords compatibility with CGI + @headers['Status'] = @status - # Parses the result of the response and extracts the various values, - # like cookies, status, headers, etc. - def parse_result - response_headers, result_body = @result.split(/\r\n\r\n/, 2) + @status, @status_message = @status.split(/ /) + @status = @status.to_i - @headers = Hash.new { |h,k| h[k] = [] } - response_headers.to_s.each_line do |line| - key, value = line.strip.split(/:\s*/, 2) - @headers[key.downcase] << value + cgi_headers = Hash.new { |h,k| h[k] = [] } + @headers.each do |key, value| + cgi_headers[key.downcase] << value end + cgi_headers['set-cookie'] = cgi_headers['set-cookie'].first + @headers = cgi_headers - (@headers['set-cookie'] || [] ).each do |string| - name, value = string.match(/^([^=]*)=([^;]*);/)[1,2] + @response.headers['cookie'] ||= [] + (@headers['set-cookie'] || []).each do |cookie| + name, value = cookie.match(/^([^=]*)=([^;]*);/)[1,2] @cookies[name] = value + + # Fake CGI cookie header + # DEPRECATE: Use response.headers["Set-Cookie"] instead + @response.headers['cookie'] << CGI::Cookie::new("name" => name, "value" => value) end - @status, @status_message = @headers["status"].first.to_s.split(/ /) - @status = @status.to_i + return status + rescue MultiPartNeededException + boundary = "----------XnJLe9ZIbbGUYtzPQJ16u1" + status = process(method, path, multipart_body(parameters, boundary), (headers || {}).merge({"CONTENT_TYPE" => "multipart/form-data; boundary=#{boundary}"})) + return status end # Encode the cookies hash in a format suitable for passing to a @@ -344,13 +330,15 @@ module ActionController # Get a temporary URL writer object def generic_url_rewriter - cgi = StubCGI.new('REQUEST_METHOD' => "GET", - 'QUERY_STRING' => "", - "REQUEST_URI" => "/", - "HTTP_HOST" => host, - "SERVER_PORT" => https? ? "443" : "80", - "HTTPS" => https? ? "on" : "off") - ActionController::UrlRewriter.new(ActionController::CgiRequest.new(cgi), {}) + env = { + 'REQUEST_METHOD' => "GET", + 'QUERY_STRING' => "", + "REQUEST_URI" => "/", + "HTTP_HOST" => host, + "SERVER_PORT" => https? ? "443" : "80", + "HTTPS" => https? ? "on" : "off" + } + ActionController::UrlRewriter.new(ActionController::RackRequest.new(env), {}) end def name_with_prefix(prefix, name) diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 47ff2dbbd3..c986941140 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -327,7 +327,7 @@ class IntegrationProcessTest < ActionController::IntegrationTest assert_equal ["410 Gone"], headers["status"] assert_response 410 assert_response :gone - assert_equal nil, response.headers["Set-Cookie"] + assert_equal ["cookie_1=; path=/", "cookie_3=chocolate; path=/"], response.headers["Set-Cookie"] assert_equal ["cookie_1=; path=/", "cookie_3=chocolate; path=/"], headers['set-cookie'] assert_equal [ CGI::Cookie::new("name" => "cookie_1", "value" => ""), -- cgit v1.2.3 From f388725bd61d8ae246c0c8e42eaec1a2be4620ac Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 21 Aug 2008 00:28:25 -0500 Subject: Partial revert of 2681685 premature TypeArray abstraction --- actionpack/lib/action_view/paths.rb | 26 ++++++++++++- activesupport/lib/active_support.rb | 1 - activesupport/lib/active_support/typed_array.rb | 31 --------------- activesupport/test/typed_array_test.rb | 51 ------------------------- 4 files changed, 25 insertions(+), 84 deletions(-) delete mode 100644 activesupport/lib/active_support/typed_array.rb delete mode 100644 activesupport/test/typed_array_test.rb diff --git a/actionpack/lib/action_view/paths.rb b/actionpack/lib/action_view/paths.rb index 6a118a1cfa..d6bf2137af 100644 --- a/actionpack/lib/action_view/paths.rb +++ b/actionpack/lib/action_view/paths.rb @@ -1,5 +1,5 @@ module ActionView #:nodoc: - class PathSet < ActiveSupport::TypedArray #:nodoc: + class PathSet < Array #:nodoc: def self.type_cast(obj) if obj.is_a?(String) if Base.warn_cache_misses && defined?(Rails) && Rails.initialized? @@ -15,6 +15,30 @@ module ActionView #:nodoc: end end + def initialize(*args) + super(*args).map! { |obj| self.class.type_cast(obj) } + end + + def <<(obj) + super(self.class.type_cast(obj)) + end + + def concat(array) + super(array.map! { |obj| self.class.type_cast(obj) }) + end + + def insert(index, obj) + super(index, self.class.type_cast(obj)) + end + + def push(*objs) + super(*objs.map { |obj| self.class.type_cast(obj) }) + end + + def unshift(*objs) + super(*objs.map { |obj| self.class.type_cast(obj) }) + end + class Path #:nodoc: def self.eager_load_templates! @eager_load_templates = true diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb index 51067e910e..1df911a3f2 100644 --- a/activesupport/lib/active_support.rb +++ b/activesupport/lib/active_support.rb @@ -39,7 +39,6 @@ require 'active_support/cache' require 'active_support/dependencies' require 'active_support/deprecation' -require 'active_support/typed_array' require 'active_support/ordered_hash' require 'active_support/ordered_options' require 'active_support/option_merger' diff --git a/activesupport/lib/active_support/typed_array.rb b/activesupport/lib/active_support/typed_array.rb deleted file mode 100644 index 1a4d8a8faf..0000000000 --- a/activesupport/lib/active_support/typed_array.rb +++ /dev/null @@ -1,31 +0,0 @@ -module ActiveSupport - class TypedArray < Array - def self.type_cast(obj) - obj - end - - def initialize(*args) - super(*args).map! { |obj| self.class.type_cast(obj) } - end - - def <<(obj) - super(self.class.type_cast(obj)) - end - - def concat(array) - super(array.map! { |obj| self.class.type_cast(obj) }) - end - - def insert(index, obj) - super(index, self.class.type_cast(obj)) - end - - def push(*objs) - super(*objs.map { |obj| self.class.type_cast(obj) }) - end - - def unshift(*objs) - super(*objs.map { |obj| self.class.type_cast(obj) }) - end - end -end diff --git a/activesupport/test/typed_array_test.rb b/activesupport/test/typed_array_test.rb deleted file mode 100644 index 023f3a1b84..0000000000 --- a/activesupport/test/typed_array_test.rb +++ /dev/null @@ -1,51 +0,0 @@ -require 'abstract_unit' - -class TypedArrayTest < Test::Unit::TestCase - class StringArray < ActiveSupport::TypedArray - def self.type_cast(obj) - obj.to_s - end - end - - def setup - @array = StringArray.new - end - - def test_string_array_initialize - assert_equal ["1", "2", "3"], StringArray.new([1, "2", :"3"]) - end - - def test_string_array_append - @array << 1 - @array << "2" - @array << :"3" - assert_equal ["1", "2", "3"], @array - end - - def test_string_array_concat - @array.concat([1, "2"]) - @array.concat([:"3"]) - assert_equal ["1", "2", "3"], @array - end - - def test_string_array_insert - @array.insert(0, 1) - @array.insert(1, "2") - @array.insert(2, :"3") - assert_equal ["1", "2", "3"], @array - end - - def test_string_array_push - @array.push(1) - @array.push("2") - @array.push(:"3") - assert_equal ["1", "2", "3"], @array - end - - def test_string_array_unshift - @array.unshift(:"3") - @array.unshift("2") - @array.unshift(1) - assert_equal ["1", "2", "3"], @array - end -end -- cgit v1.2.3 From 6be8251ec86b74b92e7bd922fafe9224281d2d26 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 21 Aug 2008 00:51:06 -0500 Subject: Simplified and renamed CallbackChain union method to replace_or_append! --- actionpack/lib/action_controller/dispatcher.rb | 2 +- activesupport/lib/active_support/callbacks.rb | 21 +++++++++++++-------- activesupport/test/callbacks_test.rb | 8 ++++---- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb index 7e46f572fe..bdae5f9d86 100644 --- a/actionpack/lib/action_controller/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatcher.rb @@ -44,7 +44,7 @@ module ActionController def to_prepare(identifier = nil, &block) @prepare_dispatch_callbacks ||= ActiveSupport::Callbacks::CallbackChain.new callback = ActiveSupport::Callbacks::Callback.new(:prepare_dispatch, block, :identifier => identifier) - @prepare_dispatch_callbacks | callback + @prepare_dispatch_callbacks.replace_or_append!(callback) end # If the block raises, send status code as a last-ditch response. diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 9c59b7ac76..7b905930bb 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -96,15 +96,12 @@ module ActiveSupport end end - def |(chain) - if chain.is_a?(CallbackChain) - chain.each { |callback| self | callback } + # TODO: Decompose into more Array like behavior + def replace_or_append!(chain) + if index = index(chain) + self[index] = chain else - if (found_callback = find(chain)) && (index = index(chain)) - self[index] = chain - else - self << chain - end + self << chain end self end @@ -157,6 +154,14 @@ module ActiveSupport self.class.new(@kind, @method, @options.dup) end + def hash + if @identifier + @identifier.hash + else + @method.hash + end + end + def call(*args, &block) evaluate_method(method, *args, &block) if should_run_callback?(*args) rescue LocalJumpError diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 7f71ca2262..25b8eecef5 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -134,10 +134,10 @@ class CallbackChainTest < Test::Unit::TestCase assert_equal :bacon, @chain.find(:bacon).method end - def test_union - assert_equal [:bacon, :lettuce, :tomato], (@chain | Callback.new(:make, :bacon)).map(&:method) - assert_equal [:bacon, :lettuce, :tomato, :turkey], (@chain | CallbackChain.build(:make, :bacon, :lettuce, :tomato, :turkey)).map(&:method) - assert_equal [:bacon, :lettuce, :tomato, :turkey, :mayo], (@chain | Callback.new(:make, :mayo)).map(&:method) + def test_replace_or_append + assert_equal [:bacon, :lettuce, :tomato], (@chain.replace_or_append!(Callback.new(:make, :bacon))).map(&:method) + assert_equal [:bacon, :lettuce, :tomato, :turkey], (@chain.replace_or_append!(Callback.new(:make, :turkey))).map(&:method) + assert_equal [:bacon, :lettuce, :tomato, :turkey, :mayo], (@chain.replace_or_append!(Callback.new(:make, :mayo))).map(&:method) end def test_delete -- cgit v1.2.3 From 2415652660242d6b0da97119c562ecff82928575 Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Thu, 21 Aug 2008 12:37:19 +0100 Subject: Support find_all on named scopes. [#730 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/named_scope.rb | 2 +- activerecord/test/cases/named_scope_test.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 0902018155..26701548c2 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -103,7 +103,7 @@ module ActiveRecord attr_reader :proxy_scope, :proxy_options [].methods.each do |m| - unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|find|count|sum|average|maximum|minimum|paginate|first|last|empty?|any?|respond_to?)/ + unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|^find$|count|sum|average|maximum|minimum|paginate|first|last|empty?|any?|respond_to?)/ delegate m, :to => :proxy_found end end diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index bd6ec23853..7cc51f5d68 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -238,4 +238,8 @@ class NamedScopeTest < ActiveRecord::TestCase assert topic.approved assert_equal 'lifo', topic.author_name end + + def test_find_all_should_behave_like_select + assert_equal Topic.base.select(&:approved), Topic.base.find_all(&:approved) + end end -- cgit v1.2.3 From 7e4ea5f4a2fd25e06820689688e3db5a4851f8e0 Mon Sep 17 00:00:00 2001 From: Darragh Curran Date: Wed, 25 Jun 2008 11:15:26 +0100 Subject: Allow overriding id for feed and entry with atom_feed_builder. [#485 state:resolved] Signed-off-by: Pratik Naik --- .../lib/action_view/helpers/atom_feed_helper.rb | 24 +++++++------- actionpack/test/template/atom_feed_helper_test.rb | 37 +++++++++++++++++++--- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/actionpack/lib/action_view/helpers/atom_feed_helper.rb b/actionpack/lib/action_view/helpers/atom_feed_helper.rb index ebb1cb34bc..e65d5d1f60 100644 --- a/actionpack/lib/action_view/helpers/atom_feed_helper.rb +++ b/actionpack/lib/action_view/helpers/atom_feed_helper.rb @@ -17,7 +17,7 @@ module ActionView # # GET /posts.atom # def index # @posts = Post.find(:all) - # + # # respond_to do |format| # format.html # format.atom @@ -29,12 +29,12 @@ module ActionView # atom_feed do |feed| # feed.title("My great blog!") # feed.updated((@posts.first.created_at)) - # + # # for post in @posts # feed.entry(post) do |entry| # entry.title(post.title) # entry.content(post.body, :type => 'html') - # + # # entry.author do |author| # author.name("DHH") # end @@ -47,8 +47,9 @@ module ActionView # * :language: Defaults to "en-US". # * :root_url: The HTML alternative that this feed is doubling for. Defaults to / on the current host. # * :url: The URL for this feed. Defaults to the current URL. - # * :schema_date: The date at which the tag scheme for the feed was first used. A good default is the year you - # created the feed. See http://feedvalidator.org/docs/error/InvalidTAG.html for more information. If not specified, + # * :id: The id for this feed. Defaults to "tag:#{request.host},#{options[:schema_date]}:#{request.request_uri.split(".")[0]}" + # * :schema_date: The date at which the tag scheme for the feed was first used. A good default is the year you + # created the feed. See http://feedvalidator.org/docs/error/InvalidTAG.html for more information. If not specified, # 2005 is used (as an "I don't care" value). # # Other namespaces can be added to the root element: @@ -81,7 +82,7 @@ module ActionView else options[:schema_date] = "2005" # The Atom spec copyright date end - + xml = options[:xml] || eval("xml", block.binding) xml.instruct! @@ -89,10 +90,10 @@ module ActionView feed_opts.merge!(options).reject!{|k,v| !k.to_s.match(/^xml/)} xml.feed(feed_opts) do - xml.id("tag:#{request.host},#{options[:schema_date]}:#{request.request_uri.split(".")[0]}") + xml.id(options[:id] || "tag:#{request.host},#{options[:schema_date]}:#{request.request_uri.split(".")[0]}") xml.link(:rel => 'alternate', :type => 'text/html', :href => options[:root_url] || (request.protocol + request.host_with_port)) xml.link(:rel => 'self', :type => 'application/atom+xml', :href => options[:url] || request.url) - + yield AtomFeedBuilder.new(xml, self, options) end end @@ -102,7 +103,7 @@ module ActionView def initialize(xml, view, feed_options = {}) @xml, @view, @feed_options = xml, view, feed_options end - + # Accepts a Date or Time object and inserts it in the proper format. If nil is passed, current time in UTC is used. def updated(date_or_time = nil) @xml.updated((date_or_time || Time.now.utc).xmlschema) @@ -115,9 +116,10 @@ module ActionView # * :published: Time first published. Defaults to the created_at attribute on the record if one such exists. # * :updated: Time of update. Defaults to the updated_at attribute on the record if one such exists. # * :url: The URL for this entry. Defaults to the polymorphic_url for the record. + # * :id: The ID for this entry. Defaults to "tag:#{@view.request.host},#{@feed_options[:schema_date]}:#{record.class}/#{record.id}" def entry(record, options = {}) - @xml.entry do - @xml.id("tag:#{@view.request.host},#{@feed_options[:schema_date]}:#{record.class}/#{record.id}") + @xml.entry do + @xml.id(options[:id] || "tag:#{@view.request.host},#{@feed_options[:schema_date]}:#{record.class}/#{record.id}") if options[:published] || (record.respond_to?(:created_at) && record.created_at) @xml.published((options[:published] || record.created_at).xmlschema) diff --git a/actionpack/test/template/atom_feed_helper_test.rb b/actionpack/test/template/atom_feed_helper_test.rb index 9f7e5b4c6c..ef31ab2c76 100644 --- a/actionpack/test/template/atom_feed_helper_test.rb +++ b/actionpack/test/template/atom_feed_helper_test.rb @@ -67,6 +67,24 @@ class ScrollsController < ActionController::Base entry.content(scroll.body, :type => 'html') entry.tag!('app:edited', Time.now) + entry.author do |author| + author.name("DHH") + end + end + end + end + EOT + FEEDS["feed_with_overridden_ids"] = <<-EOT + atom_feed({:id => 'tag:test.rubyonrails.org,2008:test/'}) do |feed| + feed.title("My great blog!") + feed.updated((@scrolls.first.created_at)) + + for scroll in @scrolls + feed.entry(scroll, :id => "tag:test.rubyonrails.org,2008:"+scroll.id.to_s) do |entry| + entry.title(scroll.title) + entry.content(scroll.body, :type => 'html') + entry.tag!('app:edited', Time.now) + entry.author do |author| author.name("DHH") end @@ -79,7 +97,7 @@ class ScrollsController < ActionController::Base Scroll.new(1, "1", "Hello One", "Something COOL!", Time.utc(2007, 12, 12, 15), Time.utc(2007, 12, 12, 15)), Scroll.new(2, "2", "Hello Two", "Something Boring", Time.utc(2007, 12, 12, 15)), ] - + render :inline => FEEDS[params[:id]], :type => :builder end @@ -98,21 +116,21 @@ class AtomFeedTest < Test::Unit::TestCase @request.host = "www.nextangle.com" end - + def test_feed_should_use_default_language_if_none_is_given with_restful_routing(:scrolls) do get :index, :id => "defaults" assert_match %r{xml:lang="en-US"}, @response.body end end - + def test_feed_should_include_two_entries with_restful_routing(:scrolls) do get :index, :id => "defaults" assert_select "entry", 2 end end - + def test_entry_should_only_use_published_if_created_at_is_present with_restful_routing(:scrolls) do get :index, :id => "defaults" @@ -167,7 +185,16 @@ class AtomFeedTest < Test::Unit::TestCase end end - private + def test_feed_should_allow_overriding_ids + with_restful_routing(:scrolls) do + get :index, :id => "feed_with_overridden_ids" + assert_select "id", :text => "tag:test.rubyonrails.org,2008:test/" + assert_select "entry id", :text => "tag:test.rubyonrails.org,2008:1" + assert_select "entry id", :text => "tag:test.rubyonrails.org,2008:2" + end + end + +private def with_restful_routing(resources) with_routing do |set| set.draw do |map| -- cgit v1.2.3 From ea40f71431a821b2ddb37be6ea3ee7d8dac63b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ku=C5=BAma?= Date: Thu, 21 Aug 2008 12:55:35 +0200 Subject: Fix that has_one natural assignment to already associated record. [#854 state:resolved] Signed-off-by: Pratik Naik --- .../lib/active_record/associations/has_one_association.rb | 4 ++-- .../test/cases/associations/has_one_associations_test.rb | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index fdc0fa52c9..18733255d2 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -21,8 +21,8 @@ module ActiveRecord def replace(obj, dont_save = false) load_target - unless @target.nil? - if dependent? && !dont_save && @target != obj + unless @target.nil? || @target == obj + if dependent? && !dont_save @target.destroy unless @target.new_record? @owner.clear_association_cache else diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 99639849a5..ec06be5eba 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -79,6 +79,16 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_raises(ActiveRecord::RecordNotFound) { Account.find(old_account_id) } end + def test_natural_assignment_to_already_associated_record + company = companies(:first_firm) + account = accounts(:signals37) + assert_equal company.account, account + company.account = account + company.reload + account.reload + assert_equal company.account, account + end + def test_assignment_without_replacement apple = Firm.create("name" => "Apple") citibank = Account.create("credit_limit" => 10) -- cgit v1.2.3 From a970f916fb1e05376733e2d42d9bcc2b873af355 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Thu, 21 Aug 2008 15:45:06 +0100 Subject: Fix has_many#count_records. [#865 state:resolved] Signed-off-by: Pratik Naik --- .../lib/active_record/associations/has_many_association.rb | 7 +++++-- .../test/cases/associations/has_many_associations_test.rb | 12 ++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index e6fa15c173..ce62127505 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -35,8 +35,11 @@ module ActiveRecord else @reflection.klass.count(:conditions => @counter_sql, :include => @reflection.options[:include]) end - - @target = [] and loaded if count == 0 + + # If there's nothing in the database and @target has no new records + # we are certain the current target is an empty array. This is a + # documented side-effect of the method that may avoid an extra SELECT. + @target ||= [] and loaded if count == 0 if @reflection.options[:limit] count = [ @reflection.options[:limit], count ].min diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index b806e885e1..da3c8fb28e 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -395,6 +395,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 3, company.clients_of_firm.size end + def test_collection_size_twice_for_regressions + post = posts(:thinking) + assert_equal 0, post.readers.size + # This test needs a post that has no readers, we assert it to ensure it holds, + # but need to reload the post because the very call to #size hides the bug. + post.reload + post.readers.build + size1 = post.readers.size + size2 = post.readers.size + assert_equal size1, size2 + end + def test_build_many company = companies(:first_firm) new_clients = assert_no_queries { company.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}]) } -- cgit v1.2.3 From 49c0e1e594c95d7e8446ebabecc9147afa62de7d Mon Sep 17 00:00:00 2001 From: Philip Hallstrom Date: Thu, 21 Aug 2008 16:08:42 +0100 Subject: Fix generated WHERE IN query for named scopes. [#583 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/base.rb | 2 +- activerecord/test/cases/named_scope_test.rb | 5 +++++ activerecord/test/models/developer.rb | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 5357255bad..15c6bc1b4a 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1750,7 +1750,7 @@ module ActiveRecord #:nodoc: def attribute_condition(argument) case argument when nil then "IS ?" - when Array, ActiveRecord::Associations::AssociationCollection then "IN (?)" + when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::NamedScope::Scope then "IN (?)" when Range then "BETWEEN ? AND ?" else "= ?" end diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 7cc51f5d68..db31ddb293 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -4,6 +4,7 @@ require 'models/topic' require 'models/comment' require 'models/reply' require 'models/author' +require 'models/developer' class NamedScopeTest < ActiveRecord::TestCase fixtures :posts, :authors, :topics, :comments, :author_addresses @@ -242,4 +243,8 @@ class NamedScopeTest < ActiveRecord::TestCase def test_find_all_should_behave_like_select assert_equal Topic.base.select(&:approved), Topic.base.find_all(&:approved) end + + def test_should_use_where_in_query_for_named_scope + assert_equal Developer.find_all_by_name('Jamis'), Developer.find_all_by_id(Developer.jamises) + end end diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index 9f26cacdec..c08476f728 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -43,6 +43,8 @@ class Developer < ActiveRecord::Base has_many :audit_logs + named_scope :jamises, :conditions => {:name => 'Jamis'} + validates_inclusion_of :salary, :in => 50000..200000 validates_length_of :name, :within => 3..20 -- cgit v1.2.3 From 0d74e72e6de7b96e158950a449ea1ccce6f5b8d7 Mon Sep 17 00:00:00 2001 From: Miles Georgi Date: Sun, 17 Aug 2008 23:45:25 -0700 Subject: Fix postgres bug when change_column is called with invalid parameters. [#861 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tarmo Tänav Signed-off-by: Pratik Naik --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 856435517a..74da0d9c85 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -761,7 +761,8 @@ module ActiveRecord begin execute "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" - rescue ActiveRecord::StatementInvalid + rescue ActiveRecord::StatementInvalid => e + raise e if postgresql_version > 80000 # This is PostgreSQL 7.x, so we have to use a more arcane way of doing it. begin begin_db_transaction -- cgit v1.2.3 From 3724dafe71f4afb2ca9f4d7d2526b228aa6c05a3 Mon Sep 17 00:00:00 2001 From: Tom Lea Date: Thu, 21 Aug 2008 16:38:27 +0100 Subject: Fix incorrect signature for NamedScope#respond_to? [#852 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/named_scope.rb | 4 ++-- activerecord/test/cases/named_scope_test.rb | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 26701548c2..c99c4beca9 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -140,8 +140,8 @@ module ActiveRecord @found ? @found.empty? : count.zero? end - def respond_to?(method) - super || @proxy_scope.respond_to?(method) + def respond_to?(method, include_private = false) + super || @proxy_scope.respond_to?(method, include_private) end def any? diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index db31ddb293..6f6ea1cbe9 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -52,6 +52,11 @@ class NamedScopeTest < ActiveRecord::TestCase assert Topic.approved.respond_to?(:length) end + def test_respond_to_respects_include_private_parameter + assert !Topic.approved.respond_to?(:load_found) + assert Topic.approved.respond_to?(:load_found, true) + end + def test_subclasses_inherit_scopes assert Topic.scopes.include?(:base) -- cgit v1.2.3 From 8622787f8748434b4ceb2b925a35b17a38e1f2d6 Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Wed, 2 Jul 2008 21:27:42 -0400 Subject: Don't interpret decimals as table names in ActiveRecord::Associations::ClassMethods#references_eager_loaded_tables? [#532 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations.rb | 6 +++--- activerecord/test/cases/associations/eager_test.rb | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) mode change 100644 => 100755 activerecord/lib/active_record/associations.rb diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb old mode 100644 new mode 100755 index b72fdb305f..b9039ce996 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1679,19 +1679,19 @@ module ActiveRecord else all << cond end end - conditions.join(' ').scan(/([\.\w]+).?\./).flatten + conditions.join(' ').scan(/([\.a-zA-Z_]+).?\./).flatten end def order_tables(options) order = [options[:order], scope(:find, :order) ].join(", ") return [] unless order && order.is_a?(String) - order.scan(/([\.\w]+).?\./).flatten + order.scan(/([\.a-zA-Z_]+).?\./).flatten end def selects_tables(options) select = options[:select] return [] unless select && select.is_a?(String) - select.scan(/"?([\.\w]+)"?.?\./).flatten + select.scan(/"?([\.a-zA-Z_]+)"?.?\./).flatten end # Checks if the conditions reference a table other than the current model table diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 58506574f8..f37e18df35 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -559,6 +559,13 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_nothing_raised { Post.find(:all, :include => 'comments') } end + def test_eager_with_floating_point_numbers + assert_queries(2) do + # Before changes, the floating point numbers will be interpreted as table names and will cause this to run in one query + Comment.find :all, :conditions => "123.456 = 123.456", :include => :post + end + end + def test_preconfigured_includes_with_belongs_to author = posts(:welcome).author_with_posts assert_no_queries {assert_equal 5, author.posts.size} -- cgit v1.2.3