From 39de112e7b902176434a1d0ebc1d6741247e50de Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 8 Dec 2008 19:14:07 -0800 Subject: [PATCH 001/188] Use full path to environment --- railties/lib/commands/about.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/commands/about.rb b/railties/lib/commands/about.rb index 7f53ac8a2e..bc2cfcb948 100644 --- a/railties/lib/commands/about.rb +++ b/railties/lib/commands/about.rb @@ -1,3 +1,3 @@ -require 'environment' +require "#{RAILS_ROOT}/config/environment" require 'rails/info' puts Rails::Info From 9adcf951ea7e5342c913dda40594cbb382995e3b Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 8 Dec 2008 19:19:48 -0800 Subject: [PATCH 002/188] Fix failing test introduced by optional-format routes --- actionpack/test/controller/caching_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 7e7f488df6..ddf140ac3a 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -48,6 +48,7 @@ class PageCachingTest < ActionController::TestCase ActionController::Routing::Routes.draw do |map| map.main '', :controller => 'posts' + map.formatted_posts 'posts.:format', :controller => 'posts' map.resources :posts map.connect ':controller/:action/:id' end From e54f17920fddebdfa241a5d9253ad5810972af87 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 9 Dec 2008 11:13:02 +0100 Subject: [PATCH 003/188] Updated included memcache-client to the 1.5.0.5 version which includes fixes from fiveruns and 37signals to deal with failover and timeouts (Joshua Sierles) [#1535 state:committed] Signed-off-by: David Heinemeier Hansson --- activesupport/CHANGELOG | 2 + activesupport/lib/active_support/vendor.rb | 4 +- .../memcache.rb | 463 +++++++++--------- 3 files changed, 245 insertions(+), 224 deletions(-) rename activesupport/lib/active_support/vendor/{memcache-client-1.5.1 => memcache-client-1.5.0.5}/memcache.rb (69%) diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 0e796d802c..6fce65af4a 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *2.3.0 [Edge]* +* Updated included memcache-client to the 1.5.0.5 version which includes fixes from fiveruns and 37signals to deal with failover and timeouts #1535 [Joshua Sierles] + * Multibyte: add multibyte-safe Chars#ord rather than falling back to String#ord. #1483 [Jason Cheow] * I18n support for Array#to_sentence. Introduces support.array.words_connector, .two_words_connector, and .last_word_connector translation keys. #1397 [Akira Matsuda] diff --git a/activesupport/lib/active_support/vendor.rb b/activesupport/lib/active_support/vendor.rb index 463610722c..4525bba559 100644 --- a/activesupport/lib/active_support/vendor.rb +++ b/activesupport/lib/active_support/vendor.rb @@ -9,9 +9,9 @@ end require 'builder' begin - gem 'memcache-client', '~> 1.5.1' + gem 'memcache-client', '~> 1.5.0.5' rescue Gem::LoadError - $:.unshift "#{File.dirname(__FILE__)}/vendor/memcache-client-1.5.1" + $:.unshift "#{File.dirname(__FILE__)}/vendor/memcache-client-1.5.0.5" end begin diff --git a/activesupport/lib/active_support/vendor/memcache-client-1.5.1/memcache.rb b/activesupport/lib/active_support/vendor/memcache-client-1.5.0.5/memcache.rb similarity index 69% rename from activesupport/lib/active_support/vendor/memcache-client-1.5.1/memcache.rb rename to activesupport/lib/active_support/vendor/memcache-client-1.5.0.5/memcache.rb index 99c9af0398..e90ddf3359 100644 --- a/activesupport/lib/active_support/vendor/memcache-client-1.5.1/memcache.rb +++ b/activesupport/lib/active_support/vendor/memcache-client-1.5.0.5/memcache.rb @@ -26,36 +26,13 @@ # OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, # EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +$TESTING = defined?($TESTING) && $TESTING require 'socket' require 'thread' require 'timeout' require 'rubygems' - -class String - - ## - # Uses the ITU-T polynomial in the CRC32 algorithm. - - def crc32_ITU_T - n = length - r = 0xFFFFFFFF - - n.times do |i| - r ^= self[i] - 8.times do - if (r & 1) != 0 then - r = (r>>1) ^ 0xEDB88320 - else - r >>= 1 - end - end - end - - r ^ 0xFFFFFFFF - end - -end +require 'zlib' ## # A Ruby client library for memcached. @@ -69,7 +46,7 @@ class MemCache ## # The version of MemCache you are using. - VERSION = '1.5.0' + VERSION = '1.5.0.5' ## # Default options for the cache object. @@ -78,6 +55,7 @@ class MemCache :namespace => nil, :readonly => false, :multithread => false, + :failover => true } ## @@ -112,6 +90,10 @@ class MemCache attr_reader :servers + ## + # Whether this client should failover reads and writes to another server + + attr_accessor :failover ## # Accepts a list of +servers+ and a list of +opts+. +servers+ may be # omitted. See +servers=+ for acceptable server list arguments. @@ -148,6 +130,7 @@ class MemCache @namespace = opts[:namespace] @readonly = opts[:readonly] @multithread = opts[:multithread] + @failover = opts[:failover] @mutex = Mutex.new if @multithread @buckets = [] self.servers = servers @@ -182,7 +165,7 @@ class MemCache def servers=(servers) # Create the server objects. - @servers = servers.collect do |server| + @servers = Array(servers).collect do |server| case server when String host, port, weight = server.split ':', 3 @@ -212,15 +195,12 @@ class MemCache # 0. +key+ can not be decremented below 0. def decr(key, amount = 1) - server, cache_key = request_setup key - - if @multithread then - threadsafe_cache_decr server, cache_key, amount - else + raise MemCacheError, "Update of readonly cache" if @readonly + with_server(key) do |server, cache_key| cache_decr server, cache_key, amount end - rescue TypeError, SocketError, SystemCallError, IOError => err - handle_error server, err + rescue TypeError => err + handle_error nil, err end ## @@ -228,21 +208,14 @@ class MemCache # unmarshalled. def get(key, raw = false) - server, cache_key = request_setup key - - value = if @multithread then - threadsafe_cache_get server, cache_key - else - cache_get server, cache_key - end - - return nil if value.nil? - - value = Marshal.load value unless raw - - return value - rescue TypeError, SocketError, SystemCallError, IOError => err - handle_error server, err + with_server(key) do |server, cache_key| + value = cache_get server, cache_key + return nil if value.nil? + value = Marshal.load value unless raw + return value + end + rescue TypeError => err + handle_error nil, err end ## @@ -280,36 +253,29 @@ class MemCache server_keys.each do |server, keys_for_server| keys_for_server = keys_for_server.join ' ' - values = if @multithread then - threadsafe_cache_get_multi server, keys_for_server - else - cache_get_multi server, keys_for_server - end + values = cache_get_multi server, keys_for_server values.each do |key, value| results[cache_keys[key]] = Marshal.load value end end return results - rescue TypeError, SocketError, SystemCallError, IOError => err - handle_error server, err + rescue TypeError, IndexError => err + handle_error nil, err end ## - # Increments the value for +key+ by +amount+ and retruns the new value. + # Increments the value for +key+ by +amount+ and returns the new value. # +key+ must already exist. If +key+ is not an integer, it is assumed to be # 0. def incr(key, amount = 1) - server, cache_key = request_setup key - - if @multithread then - threadsafe_cache_incr server, cache_key, amount - else + raise MemCacheError, "Update of readonly cache" if @readonly + with_server(key) do |server, cache_key| cache_incr server, cache_key, amount end - rescue TypeError, SocketError, SystemCallError, IOError => err - handle_error server, err + rescue TypeError => err + handle_error nil, err end ## @@ -321,23 +287,23 @@ class MemCache def set(key, value, expiry = 0, raw = false) raise MemCacheError, "Update of readonly cache" if @readonly - server, cache_key = request_setup key - socket = server.socket + with_server(key) do |server, cache_key| - value = Marshal.dump value unless raw - command = "set #{cache_key} 0 #{expiry} #{value.size}\r\n#{value}\r\n" + value = Marshal.dump value unless raw + command = "set #{cache_key} 0 #{expiry} #{value.to_s.size}\r\n#{value}\r\n" - begin - @mutex.lock if @multithread - socket.write command - result = socket.gets - raise_on_error_response! result - result - rescue SocketError, SystemCallError, IOError => err - server.close - raise MemCacheError, err.message - ensure - @mutex.unlock if @multithread + with_socket_management(server) do |socket| + socket.write command + result = socket.gets + raise_on_error_response! result + + if result.nil? + server.close + raise MemCacheError, "lost connection to #{server.host}:#{server.port}" + end + + result + end end end @@ -351,23 +317,16 @@ class MemCache def add(key, value, expiry = 0, raw = false) raise MemCacheError, "Update of readonly cache" if @readonly - server, cache_key = request_setup key - socket = server.socket + with_server(key) do |server, cache_key| + value = Marshal.dump value unless raw + command = "add #{cache_key} 0 #{expiry} #{value.size}\r\n#{value}\r\n" - value = Marshal.dump value unless raw - command = "add #{cache_key} 0 #{expiry} #{value.size}\r\n#{value}\r\n" - - begin - @mutex.lock if @multithread - socket.write command - result = socket.gets - raise_on_error_response! result - result - rescue SocketError, SystemCallError, IOError => err - server.close - raise MemCacheError, err.message - ensure - @mutex.unlock if @multithread + with_socket_management(server) do |socket| + socket.write command + result = socket.gets + raise_on_error_response! result + result + end end end @@ -375,26 +334,15 @@ class MemCache # Removes +key+ from the cache in +expiry+ seconds. def delete(key, expiry = 0) - @mutex.lock if @multithread - - raise MemCacheError, "No active servers" unless active? - cache_key = make_cache_key key - server = get_server_for_key cache_key - - sock = server.socket - raise MemCacheError, "No connection to server" if sock.nil? - - begin - sock.write "delete #{cache_key} #{expiry}\r\n" - result = sock.gets - raise_on_error_response! result - result - rescue SocketError, SystemCallError, IOError => err - server.close - raise MemCacheError, err.message + raise MemCacheError, "Update of readonly cache" if @readonly + with_server(key) do |server, cache_key| + with_socket_management(server) do |socket| + socket.write "delete #{cache_key} #{expiry}\r\n" + result = socket.gets + raise_on_error_response! result + result + end end - ensure - @mutex.unlock if @multithread end ## @@ -403,21 +351,19 @@ class MemCache def flush_all raise MemCacheError, 'No active servers' unless active? raise MemCacheError, "Update of readonly cache" if @readonly + begin @mutex.lock if @multithread @servers.each do |server| - begin - sock = server.socket - raise MemCacheError, "No connection to server" if sock.nil? - sock.write "flush_all\r\n" - result = sock.gets + with_socket_management(server) do |socket| + socket.write "flush_all\r\n" + result = socket.gets raise_on_error_response! result result - rescue SocketError, SystemCallError, IOError => err - server.close - raise MemCacheError, err.message end end + rescue IndexError => err + handle_error nil, err ensure @mutex.unlock if @multithread end @@ -469,14 +415,13 @@ class MemCache server_stats = {} @servers.each do |server| - sock = server.socket - raise MemCacheError, "No connection to server" if sock.nil? + next unless server.alive? - value = nil - begin - sock.write "stats\r\n" + with_socket_management(server) do |socket| + value = nil + socket.write "stats\r\n" stats = {} - while line = sock.gets do + while line = socket.gets do raise_on_error_response! line break if line == "END\r\n" if line =~ /\ASTAT ([\w]+) ([\w\.\:]+)/ then @@ -498,12 +443,10 @@ class MemCache end end server_stats["#{server.host}:#{server.port}"] = stats - rescue SocketError, SystemCallError, IOError => err - server.close - raise MemCacheError, err.message end end + raise MemCacheError, "No active servers" if server_stats.empty? server_stats end @@ -520,7 +463,7 @@ class MemCache set key, value end - protected + protected unless $TESTING ## # Create a key for the cache, incorporating the namespace qualifier if @@ -537,7 +480,7 @@ class MemCache ## # Pick a server to handle the request based on a hash of the key. - def get_server_for_key(key) + def get_server_for_key(key, options = {}) raise ArgumentError, "illegal character in key #{key.inspect}" if key =~ /\s/ raise ArgumentError, "key too long #{key.inspect}" if key.length > 250 @@ -545,13 +488,17 @@ class MemCache return @servers.first if @servers.length == 1 hkey = hash_for key - - 20.times do |try| - server = @buckets[hkey % @buckets.nitems] - return server if server.alive? - hkey += hash_for "#{try}#{key}" + + if @failover + 20.times do |try| + server = @buckets[hkey % @buckets.compact.size] + return server if server.alive? + hkey += hash_for "#{try}#{key}" + end + else + return @buckets[hkey % @buckets.compact.size] end - + raise MemCacheError, "No servers available" end @@ -560,7 +507,7 @@ class MemCache # sketchy for down servers). def hash_for(key) - (key.crc32_ITU_T >> 16) & 0x7fff + (Zlib.crc32(key) >> 16) & 0x7fff end ## @@ -568,12 +515,13 @@ class MemCache # found. def cache_decr(server, cache_key, amount) - socket = server.socket - socket.write "decr #{cache_key} #{amount}\r\n" - text = socket.gets - raise_on_error_response! text - return nil if text == "NOT_FOUND\r\n" - return text.to_i + with_socket_management(server) do |socket| + socket.write "decr #{cache_key} #{amount}\r\n" + text = socket.gets + raise_on_error_response! text + return nil if text == "NOT_FOUND\r\n" + return text.to_i + end end ## @@ -581,52 +529,54 @@ class MemCache # miss. def cache_get(server, cache_key) - socket = server.socket - socket.write "get #{cache_key}\r\n" - keyline = socket.gets # "VALUE \r\n" + with_socket_management(server) do |socket| + socket.write "get #{cache_key}\r\n" + keyline = socket.gets # "VALUE \r\n" - if keyline.nil? then - server.close - raise MemCacheError, "lost connection to #{server.host}:#{server.port}" + if keyline.nil? then + server.close + raise MemCacheError, "lost connection to #{server.host}:#{server.port}" + end + + raise_on_error_response! keyline + return nil if keyline == "END\r\n" + + unless keyline =~ /(\d+)\r/ then + server.close + raise MemCacheError, "unexpected response #{keyline.inspect}" + end + value = socket.read $1.to_i + socket.read 2 # "\r\n" + socket.gets # "END\r\n" + return value end - - raise_on_error_response! keyline - return nil if keyline == "END\r\n" - - unless keyline =~ /(\d+)\r/ then - server.close - raise MemCacheError, "unexpected response #{keyline.inspect}" - end - value = socket.read $1.to_i - socket.read 2 # "\r\n" - socket.gets # "END\r\n" - return value end ## # Fetches +cache_keys+ from +server+ using a multi-get. def cache_get_multi(server, cache_keys) - values = {} - socket = server.socket - socket.write "get #{cache_keys}\r\n" + with_socket_management(server) do |socket| + values = {} + socket.write "get #{cache_keys}\r\n" - while keyline = socket.gets do - return values if keyline == "END\r\n" - raise_on_error_response! keyline + while keyline = socket.gets do + return values if keyline == "END\r\n" + raise_on_error_response! keyline - unless keyline =~ /\AVALUE (.+) (.+) (.+)/ then - server.close - raise MemCacheError, "unexpected response #{keyline.inspect}" + unless keyline =~ /\AVALUE (.+) (.+) (.+)/ then + server.close + raise MemCacheError, "unexpected response #{keyline.inspect}" + end + + key, data_length = $1, $3 + values[$1] = socket.read data_length.to_i + socket.read(2) # "\r\n" end - key, data_length = $1, $3 - values[$1] = socket.read data_length.to_i - socket.read(2) # "\r\n" + server.close + raise MemCacheError, "lost connection to #{server.host}:#{server.port}" # TODO: retry here too end - - server.close - raise MemCacheError, "lost connection to #{server.host}:#{server.port}" end ## @@ -634,18 +584,76 @@ class MemCache # found. def cache_incr(server, cache_key, amount) - socket = server.socket - socket.write "incr #{cache_key} #{amount}\r\n" - text = socket.gets - raise_on_error_response! text - return nil if text == "NOT_FOUND\r\n" - return text.to_i + with_socket_management(server) do |socket| + socket.write "incr #{cache_key} #{amount}\r\n" + text = socket.gets + raise_on_error_response! text + return nil if text == "NOT_FOUND\r\n" + return text.to_i + end + end + + ## + # Gets or creates a socket connected to the given server, and yields it + # to the block, wrapped in a mutex synchronization if @multithread is true. + # + # If a socket error (SocketError, SystemCallError, IOError) or protocol error + # (MemCacheError) is raised by the block, closes the socket, attempts to + # connect again, and retries the block (once). If an error is again raised, + # reraises it as MemCacheError. + # + # If unable to connect to the server (or if in the reconnect wait period), + # raises MemCacheError. Note that the socket connect code marks a server + # dead for a timeout period, so retrying does not apply to connection attempt + # failures (but does still apply to unexpectedly lost connections etc.). + + def with_socket_management(server, &block) + @mutex.lock if @multithread + retried = false + + begin + socket = server.socket + + # Raise an IndexError to show this server is out of whack. If were inside + # a with_server block, we'll catch it and attempt to restart the operation. + + raise IndexError, "No connection to server (#{server.status})" if socket.nil? + + block.call(socket) + + rescue SocketError => err + server.mark_dead(err.message) + handle_error(server, err) + + rescue MemCacheError, SocketError, SystemCallError, IOError => err + handle_error(server, err) if retried || socket.nil? + retried = true + retry + end + ensure + @mutex.unlock if @multithread + end + + def with_server(key) + retried = false + begin + server, cache_key = request_setup(key) + yield server, cache_key + rescue IndexError => e + if !retried && @servers.size > 1 + puts "Connection to server #{server.inspect} DIED! Retrying operation..." + retried = true + retry + end + handle_error(nil, e) + end end ## # Handles +error+ from +server+. def handle_error(server, error) + raise error if error.is_a?(MemCacheError) server.close if server new_error = MemCacheError.new error.message new_error.set_backtrace error.backtrace @@ -660,45 +668,15 @@ class MemCache raise MemCacheError, 'No active servers' unless active? cache_key = make_cache_key key server = get_server_for_key cache_key - raise MemCacheError, 'No connection to server' if server.socket.nil? return server, cache_key end - def threadsafe_cache_decr(server, cache_key, amount) # :nodoc: - @mutex.lock - cache_decr server, cache_key, amount - ensure - @mutex.unlock - end - - def threadsafe_cache_get(server, cache_key) # :nodoc: - @mutex.lock - cache_get server, cache_key - ensure - @mutex.unlock - end - - def threadsafe_cache_get_multi(socket, cache_keys) # :nodoc: - @mutex.lock - cache_get_multi socket, cache_keys - ensure - @mutex.unlock - end - - def threadsafe_cache_incr(server, cache_key, amount) # :nodoc: - @mutex.lock - cache_incr server, cache_key, amount - ensure - @mutex.unlock - end - def raise_on_error_response!(response) - if response =~ /\A(?:CLIENT_|SERVER_)?ERROR (.*)/ + if response =~ /\A(?:CLIENT_|SERVER_)?ERROR(.*)/ raise MemCacheError, $1.strip end end - ## # This class represents a memcached server instance. @@ -711,6 +689,13 @@ class MemCache CONNECT_TIMEOUT = 0.25 + ## + # The amount of time to wait for a response from a memcached server. + # If a response isn't received within this time limit, + # the server will be marked as down. + + SOCKET_TIMEOUT = 0.5 + ## # The amount of time to wait before attempting to re-establish a # connection with a server that is marked dead. @@ -795,9 +780,9 @@ class MemCache # Attempt to connect if not already connected. begin - @sock = timeout CONNECT_TIMEOUT do - TCPSocket.new @host, @port - end + + @sock = TCPTimeoutSocket.new @host, @port + if Socket.constants.include? 'TCP_NODELAY' then @sock.setsockopt Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1 end @@ -826,8 +811,6 @@ class MemCache @mutex.unlock if @multithread end - private - ## # Mark the server as dead and close its socket. @@ -836,8 +819,9 @@ class MemCache @sock = nil @retry = Time.now + RETRY_DELAY - @status = sprintf "DEAD: %s, will retry at %s", reason, @retry + @status = sprintf "%s:%s DEAD: %s, will retry at %s", @host, @port, reason, @retry end + end ## @@ -847,3 +831,38 @@ class MemCache end +# TCPSocket facade class which implements timeouts. +class TCPTimeoutSocket + def initialize(*args) + Timeout::timeout(MemCache::Server::CONNECT_TIMEOUT, SocketError) do + @sock = TCPSocket.new(*args) + @len = MemCache::Server::SOCKET_TIMEOUT.to_f || 0.5 + end + end + + def write(*args) + Timeout::timeout(@len, SocketError) do + @sock.write(*args) + end + end + + def gets(*args) + Timeout::timeout(@len, SocketError) do + @sock.gets(*args) + end + end + + def read(*args) + Timeout::timeout(@len, SocketError) do + @sock.read(*args) + end + end + + def _socket + @sock + end + + def method_missing(meth, *args) + @sock.__send__(meth, *args) + end +end \ No newline at end of file From c3fe6ebbfaedff8d6dd8f590bdd237fac174faac Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 9 Dec 2008 11:16:30 -0800 Subject: [PATCH 004/188] How'd that sneak in there? --- railties/test/gem_dependency_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/railties/test/gem_dependency_test.rb b/railties/test/gem_dependency_test.rb index 1d4f2b18b3..30fd899fea 100644 --- a/railties/test/gem_dependency_test.rb +++ b/railties/test/gem_dependency_test.rb @@ -134,7 +134,6 @@ uses_mocha "Plugin Tests" do dummy_gem.add_load_paths dummy_gem.load assert dummy_gem.loaded? - debugger assert_equal 2, dummy_gem.dependencies.size assert_nothing_raised do dummy_gem.dependencies.each do |g| From e8c4939fb3366472021c1af1331bfdfe5d7a5d75 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 9 Dec 2008 11:17:11 -0800 Subject: [PATCH 005/188] Benchmark.ms --- .../lib/action_controller/benchmarking.rb | 16 ++++++++-------- .../lib/action_controller/request_profiler.rb | 12 ++++++------ .../action_view/helpers/benchmark_helper.rb | 6 +++--- activerecord/lib/active_record/base.rb | 4 ++-- .../connection_adapters/abstract_adapter.rb | 10 +++++----- .../lib/active_resource/connection.rb | 4 ++-- activesupport/CHANGELOG | 2 ++ activesupport/lib/active_support/cache.rb | 4 ++-- .../lib/active_support/core_ext/benchmark.rb | 19 +++++++++++++------ 9 files changed, 43 insertions(+), 34 deletions(-) diff --git a/actionpack/lib/action_controller/benchmarking.rb b/actionpack/lib/action_controller/benchmarking.rb index fa572ebf3d..732f774fbc 100644 --- a/actionpack/lib/action_controller/benchmarking.rb +++ b/actionpack/lib/action_controller/benchmarking.rb @@ -23,8 +23,8 @@ module ActionController #:nodoc: def benchmark(title, log_level = Logger::DEBUG, use_silence = true) if logger && logger.level == log_level result = nil - seconds = Benchmark.realtime { result = use_silence ? silence { yield } : yield } - logger.add(log_level, "#{title} (#{('%.1f' % (seconds * 1000))}ms)") + ms = Benchmark.ms { result = use_silence ? silence { yield } : yield } + logger.add(log_level, "#{title} (#{('%.1f' % ms)}ms)") result else yield @@ -48,7 +48,7 @@ module ActionController #:nodoc: end render_output = nil - @view_runtime = Benchmark::realtime { render_output = render_without_benchmark(options, extra_options, &block) } + @view_runtime = Benchmark.ms { render_output = render_without_benchmark(options, extra_options, &block) } if Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected? @db_rt_before_render = db_runtime @@ -65,11 +65,11 @@ module ActionController #:nodoc: private def perform_action_with_benchmark if logger - seconds = [ Benchmark::measure{ perform_action_without_benchmark }.real, 0.0001 ].max + ms = [Benchmark.ms { perform_action_without_benchmark }, 0.01].max logging_view = defined?(@view_runtime) logging_active_record = Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected? - log_message = "Completed in #{sprintf("%.0f", seconds * 1000)}ms" + log_message = 'Completed in %.0fms' % ms if logging_view || logging_active_record log_message << " (" @@ -87,21 +87,21 @@ module ActionController #:nodoc: log_message << " [#{complete_request_uri rescue "unknown"}]" logger.info(log_message) - response.headers["X-Runtime"] = "#{sprintf("%.0f", seconds * 1000)}ms" + response.headers["X-Runtime"] = "%.0f" % ms else perform_action_without_benchmark end end def view_runtime - "View: %.0f" % (@view_runtime * 1000) + "View: %.0f" % @view_runtime end def active_record_runtime db_runtime = ActiveRecord::Base.connection.reset_runtime db_runtime += @db_rt_before_render if @db_rt_before_render db_runtime += @db_rt_after_render if @db_rt_after_render - "DB: %.0f" % (db_runtime * 1000) + "DB: %.0f" % db_runtime end end end diff --git a/actionpack/lib/action_controller/request_profiler.rb b/actionpack/lib/action_controller/request_profiler.rb index 70bb77e7ac..80cd55334f 100644 --- a/actionpack/lib/action_controller/request_profiler.rb +++ b/actionpack/lib/action_controller/request_profiler.rb @@ -20,7 +20,7 @@ module ActionController @quiet = true print ' ' - result = Benchmark.realtime do + ms = Benchmark.ms do n.times do |i| run(profiling) print_progress(i) @@ -28,7 +28,7 @@ module ActionController end puts - result + ms ensure @quiet = false end @@ -88,7 +88,7 @@ module ActionController puts 'Warming up once' elapsed = warmup(sandbox) - puts '%.2f sec, %d requests, %d req/sec' % [elapsed, sandbox.request_count, sandbox.request_count / elapsed] + puts '%.0f ms, %d requests, %d req/sec' % [elapsed, sandbox.request_count, 1000 * sandbox.request_count / elapsed] puts "\n#{options[:benchmark] ? 'Benchmarking' : 'Profiling'} #{options[:n]}x" options[:benchmark] ? benchmark(sandbox) : profile(sandbox) @@ -106,13 +106,13 @@ module ActionController def benchmark(sandbox, profiling = false) sandbox.request_count = 0 - elapsed = sandbox.benchmark(options[:n], profiling).to_f + elapsed = sandbox.benchmark(options[:n], profiling) count = sandbox.request_count.to_i - puts '%.2f sec, %d requests, %d req/sec' % [elapsed, count, count / elapsed] + puts '%.0f ms, %d requests, %d req/sec' % [elapsed, count, 1000 * count / elapsed] end def warmup(sandbox) - Benchmark.realtime { sandbox.run(false) } + Benchmark.ms { sandbox.run(false) } end def default_options diff --git a/actionpack/lib/action_view/helpers/benchmark_helper.rb b/actionpack/lib/action_view/helpers/benchmark_helper.rb index bd72cda700..372d24a22e 100644 --- a/actionpack/lib/action_view/helpers/benchmark_helper.rb +++ b/actionpack/lib/action_view/helpers/benchmark_helper.rb @@ -22,12 +22,12 @@ module ActionView # (:debug, :info, :warn, :error); the default value is :info. def benchmark(message = "Benchmarking", level = :info) if controller.logger - seconds = Benchmark.realtime { yield } - controller.logger.send(level, "#{message} (#{'%.1f' % (seconds * 1000)}ms)") + ms = Benchmark.ms { yield } + controller.logger.send(level, '%s (%.1fms)' % [message, ms]) else yield end end end end -end \ No newline at end of file +end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 5d614442c3..a23518b357 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1417,8 +1417,8 @@ module ActiveRecord #:nodoc: def benchmark(title, log_level = Logger::DEBUG, use_silence = true) if logger && logger.level <= log_level result = nil - seconds = Benchmark.realtime { result = use_silence ? silence { yield } : yield } - logger.add(log_level, "#{title} (#{'%.1f' % (seconds * 1000)}ms)") + ms = Benchmark.ms { result = use_silence ? silence { yield } : yield } + logger.add(log_level, '%s (%.1fms)' % [title, ms]) result else yield diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index cab77fc031..bfafcfb3ab 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -160,9 +160,9 @@ module ActiveRecord @open_transactions -= 1 end - def log_info(sql, name, seconds) + def log_info(sql, name, ms) if @logger && @logger.debug? - name = "#{name.nil? ? "SQL" : name} (#{sprintf("%.1f", seconds * 1000)}ms)" + name = '%s (%.1fms)' % [name || 'SQL', ms] @logger.debug(format_log_entry(name, sql.squeeze(' '))) end end @@ -171,9 +171,9 @@ module ActiveRecord def log(sql, name) if block_given? result = nil - seconds = Benchmark.realtime { result = yield } - @runtime += seconds - log_info(sql, name, seconds) + ms = Benchmark.ms { result = yield } + @runtime += ms + log_info(sql, name, ms) result else log_info(sql, name, 0) diff --git a/activeresource/lib/active_resource/connection.rb b/activeresource/lib/active_resource/connection.rb index 273fee3286..85103b53c5 100644 --- a/activeresource/lib/active_resource/connection.rb +++ b/activeresource/lib/active_resource/connection.rb @@ -146,8 +146,8 @@ module ActiveResource def request(method, path, *arguments) 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 "--> %d %s (%d %.2fs)" % [result.code, result.message, result.body ? result.body.length : 0, time] if logger + ms = Benchmark.ms { result = http.send(method, path, *arguments) } + logger.info "--> %d %s (%d %.0fms)" % [result.code, result.message, result.body ? result.body.length : 0, ms] if logger handle_response(result) rescue Timeout::Error => e raise TimeoutError.new(e.message) diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 0e796d802c..a74c645981 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *2.3.0 [Edge]* +* Add Benchmark.ms convenience method to benchmark realtime in milliseconds. [Jeremy Kemper] + * Multibyte: add multibyte-safe Chars#ord rather than falling back to String#ord. #1483 [Jason Cheow] * I18n support for Array#to_sentence. Introduces support.array.words_connector, .two_words_connector, and .last_word_connector translation keys. #1397 [Akira Matsuda] diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 10281d60eb..6a6c861458 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -143,13 +143,13 @@ module ActiveSupport log("miss", key, options) value = nil - seconds = Benchmark.realtime { value = yield } + ms = Benchmark.ms { value = yield } @logger_off = true write(key, value, options) @logger_off = false - log("write (will save #{'%.2f' % (seconds * 1000)}ms)", key, nil) + log('write (will save %.2fms)' % ms, key, nil) value end diff --git a/activesupport/lib/active_support/core_ext/benchmark.rb b/activesupport/lib/active_support/core_ext/benchmark.rb index 79ba165e3a..ae57b152e8 100644 --- a/activesupport/lib/active_support/core_ext/benchmark.rb +++ b/activesupport/lib/active_support/core_ext/benchmark.rb @@ -1,12 +1,19 @@ require 'benchmark' class << Benchmark - remove_method :realtime + # Earlier Ruby had a slower implementation. + if RUBY_VERSION < '1.8.7' + remove_method :realtime - def realtime - r0 = Time.now - yield - r1 = Time.now - r1.to_f - r0.to_f + def realtime + r0 = Time.now + yield + r1 = Time.now + r1.to_f - r0.to_f + end + end + + def ms + 1000 * realtime { yield } end end From 781e29be0a18b6c0a4de42edef5e9157429a6686 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 9 Dec 2008 11:39:45 -0800 Subject: [PATCH 006/188] Fix tests broken by switch to Pathname --- actionmailer/test/mail_service_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actionmailer/test/mail_service_test.rb b/actionmailer/test/mail_service_test.rb index e469302fe1..15a40552c9 100644 --- a/actionmailer/test/mail_service_test.rb +++ b/actionmailer/test/mail_service_test.rb @@ -967,13 +967,13 @@ end # uses_mocha class InheritableTemplateRootTest < Test::Unit::TestCase def test_attr expected = "#{File.dirname(__FILE__)}/fixtures/path.with.dots" - assert_equal expected, FunkyPathMailer.template_root + assert_equal expected, FunkyPathMailer.template_root.to_s sub = Class.new(FunkyPathMailer) sub.template_root = 'test/path' - assert_equal 'test/path', sub.template_root - assert_equal expected, FunkyPathMailer.template_root + assert_equal 'test/path', sub.template_root.to_s + assert_equal expected, FunkyPathMailer.template_root.to_s end end From 014b7994ac1a829845e2882f4c852bc28bd2aa53 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 9 Dec 2008 14:12:25 -0600 Subject: [PATCH 007/188] Explicitly require ERB Utils extensions from TagHelper --- actionpack/lib/action_view/helpers/tag_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/lib/action_view/helpers/tag_helper.rb b/actionpack/lib/action_view/helpers/tag_helper.rb index 3b301248ff..af8c4d5e21 100644 --- a/actionpack/lib/action_view/helpers/tag_helper.rb +++ b/actionpack/lib/action_view/helpers/tag_helper.rb @@ -1,4 +1,4 @@ -require 'erb' +require 'action_view/erb/util' require 'set' module ActionView From 355f41d8aafd75d76db25cdda4736e0052b0605c Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Tue, 9 Dec 2008 20:36:24 +0000 Subject: [PATCH 008/188] Rework ActiveSupport::OrderedHash to make lookups faster [#1352 state:committed] Signed-off-by: Jeremy Kemper --- activerecord/test/cases/calculations_test.rb | 5 +- .../lib/active_support/ordered_hash.rb | 59 ++++++++----------- activesupport/test/ordered_hash_test.rb | 10 ++++ 3 files changed, 36 insertions(+), 38 deletions(-) diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 8bd0dd0f6e..080f6a7007 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -171,8 +171,9 @@ class CalculationsTest < ActiveRecord::TestCase Account.expects(:columns).at_least_once.returns([column]) c = Account.count(:all, :group => :firm) - assert_equal Firm, c.first.first.class - assert_equal 1, c.first.last + first_key = c.keys.first + assert_equal Firm, first_key.class + assert_equal 1, c[first_key] end end diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index 5de94c67e0..1ed7737017 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -4,62 +4,49 @@ module ActiveSupport if RUBY_VERSION >= '1.9' OrderedHash = ::Hash else - class OrderedHash < Array #:nodoc: - def []=(key, value) - if pair = assoc(key) - pair.pop - pair << value - else - self << [key, value] - end - value + class OrderedHash < Hash #:nodoc: + def initialize(*args, &block) + super + @keys = [] end - def [](key) - pair = assoc(key) - pair ? pair.last : nil + def []=(key, value) + if !has_key?(key) + @keys << key + end + super end def delete(key) - pair = assoc(key) - pair ? array_index = index(pair) : nil - array_index ? delete_at(array_index).last : nil + array_index = has_key?(key) && index(key) + if array_index + @keys.delete_at(array_index) + end + super end def keys - collect { |key, value| key } + @keys end def values - collect { |key, value| value } + @keys.collect { |key| self[key] } end def to_hash - returning({}) do |hash| - each { |array| hash[array[0]] = array[1] } - end + Hash.new(self) end - def has_key?(k) - !assoc(k).nil? - end - - alias_method :key?, :has_key? - alias_method :include?, :has_key? - alias_method :member?, :has_key? - - def has_value?(v) - any? { |key, value| value == v } - end - - alias_method :value?, :has_value? - def each_key - each { |key, value| yield key } + @keys.each { |key| yield key } end def each_value - each { |key, value| yield value } + @keys.each { |key| yield self[key]} + end + + def each + keys.each {|key| yield [key, self[key]]} end end end diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb index 17dffbd624..094f9316d6 100644 --- a/activesupport/test/ordered_hash_test.rb +++ b/activesupport/test/ordered_hash_test.rb @@ -73,4 +73,14 @@ class OrderedHashTest < Test::Unit::TestCase @ordered_hash.each_value { |v| values << v } assert_equal @values, values end + + def test_each + values = [] + @ordered_hash.each {|key, value| values << value} + assert_equal @values, values + end + + def test_each_with_index + @ordered_hash.each_with_index { |pair, index| assert_equal [@keys[index], @values[index]], pair} + end end From 05f2183747c8e75c9e8bbaadb9573b4bdf41ecfc Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Tue, 2 Dec 2008 14:12:09 -0300 Subject: [PATCH 009/188] Fix: counter_cache should decrement on deleting associated records. [#1195 state:committed] Signed-off-by: Jeremy Kemper --- .../associations/association_collection.rb | 3 +++ .../has_many_associations_test.rb | 20 +++++++++++++++++++ .../has_many_through_associations_test.rb | 14 +++++++++++++ 3 files changed, 37 insertions(+) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 0ff91fbdf8..0ad58010b2 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -202,6 +202,9 @@ module ActiveRecord records.each do |record| @target.delete(record) + if respond_to?(:cached_counter_attribute_name) && @owner[cached_counter_attribute_name] + @owner.class.decrement_counter(cached_counter_attribute_name, @owner.send(@owner.class.primary_key)) + end callback(:after_remove, record) end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 816ceb6855..1f8b297e81 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -552,6 +552,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 0, companies(:first_firm).clients_of_firm(true).size end + def test_deleting_updates_counter_cache + post = Post.first + + post.comments.delete(post.comments.first) + post.reload + assert_equal post.comments(true).size, post.comments_count + + post.comments.delete(post.comments.first) + post.reload + assert_equal 0, post.comments_count + end + def test_deleting_before_save new_firm = Firm.new("name" => "A New Firm, Inc.") new_client = new_firm.clients_of_firm.build("name" => "Another Client") @@ -605,6 +617,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end end + def test_clearing_updates_counter_cache + post = Post.first + + post.comments.clear + post.reload + assert_equal 0, post.comments_count + end + def test_clearing_a_dependent_association_collection firm = companies(:first_firm) client_id = firm.dependent_clients_of_firm.first.id diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index a07f4bcbdd..ba3428a508 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -3,6 +3,9 @@ require 'models/post' require 'models/person' require 'models/reader' require 'models/comment' +require 'models/tag' +require 'models/tagging' +require 'models/author' class HasManyThroughAssociationsTest < ActiveRecord::TestCase fixtures :posts, :readers, :people, :comments, :authors @@ -84,6 +87,17 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:welcome).reload.people(true).empty? end + def test_deleting_updates_counter_cache + taggable = Tagging.first.taggable + taggable.taggings.push(Tagging.new) + taggable.reload + assert_equal 1, taggable.taggings_count + + taggable.taggings.delete(taggable.taggings.first) + taggable.reload + assert_equal 0, taggable.taggings_count + end + def test_replace_association assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)} From e4c0163f3288cf992b8a60666d79f20f74daeab8 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 10 Dec 2008 11:00:44 -0800 Subject: [PATCH 010/188] Fix ActionController autoloads --- actionpack/lib/action_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 5d5d6b8c9c..abc404afe7 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -38,7 +38,7 @@ module ActionController # TODO: Review explicit to see if they will automatically be handled by # the initilizer if they are really needed. def self.load_all! - [Base, CgiRequest, CgiResponse, RackRequest, RackRequest, Http::Headers, UrlRewriter, UrlWriter] + [Base, CGIHandler, CgiRequest, RackRequest, RackRequest, Http::Headers, UrlRewriter, UrlWriter] end autoload :AbstractRequest, 'action_controller/request' @@ -91,7 +91,6 @@ module ActionController # DEPRECATE: Remove CGI support autoload :CgiRequest, 'action_controller/cgi_process' - autoload :CgiResponse, 'action_controller/cgi_process' autoload :CGIHandler, 'action_controller/cgi_process' end From 953954302682e1ae2f56cfc2ed04debc737c206c Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 10 Dec 2008 11:01:04 -0800 Subject: [PATCH 011/188] Add ActiveRecord::VERSION autoload --- activerecord/lib/active_record.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 348e5b94af..1aaf456c0f 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -37,6 +37,8 @@ module ActiveRecord [Base, DynamicFinderMatch, ConnectionAdapters::AbstractAdapter] end + autoload :VERSION, 'active_record/version' + autoload :ActiveRecordError, 'active_record/base' autoload :ConnectionNotEstablished, 'active_record/base' From 96b815d7e81b9cef912ef94c96dca923fe43b0ba Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Wed, 10 Dec 2008 16:04:29 -0300 Subject: [PATCH 012/188] Fix test names collision. [#1549 state:committed] Signed-off-by: Jeremy Kemper --- activerecord/test/cases/validations_i18n_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/activerecord/test/cases/validations_i18n_test.rb b/activerecord/test/cases/validations_i18n_test.rb index f59e3f7001..e893a704f1 100644 --- a/activerecord/test/cases/validations_i18n_test.rb +++ b/activerecord/test/cases/validations_i18n_test.rb @@ -506,7 +506,7 @@ class ActiveRecordValidationsI18nTests < ActiveSupport::TestCase # validates_length_of :is w/o mocha - def test_validates_length_of_within_finds_custom_model_key_translation + def test_validates_length_of_is_finds_custom_model_key_translation I18n.backend.store_translations 'en', :activerecord => {:errors => {:models => {:topic => {:attributes => {:title => {:wrong_length => 'custom message'}}}}}} I18n.backend.store_translations 'en', :activerecord => {:errors => {:messages => {:wrong_length => 'global message'}}} @@ -515,7 +515,7 @@ class ActiveRecordValidationsI18nTests < ActiveSupport::TestCase assert_equal 'custom message', @topic.errors.on(:title) end - def test_validates_length_of_within_finds_global_default_translation + def test_validates_length_of_is_finds_global_default_translation I18n.backend.store_translations 'en', :activerecord => {:errors => {:messages => {:wrong_length => 'global message'}}} Topic.validates_length_of :title, :is => 5 @@ -525,7 +525,7 @@ class ActiveRecordValidationsI18nTests < ActiveSupport::TestCase # validates_uniqueness_of w/o mocha - def test_validates_length_of_within_finds_custom_model_key_translation + def test_validates_length_of_is_finds_custom_model_key_translation I18n.backend.store_translations 'en', :activerecord => {:errors => {:models => {:topic => {:attributes => {:title => {:wrong_length => 'custom message'}}}}}} I18n.backend.store_translations 'en', :activerecord => {:errors => {:messages => {:wrong_length => 'global message'}}} @@ -534,7 +534,7 @@ class ActiveRecordValidationsI18nTests < ActiveSupport::TestCase assert_equal 'custom message', @topic.errors.on(:title) end - def test_validates_length_of_within_finds_global_default_translation + def test_validates_length_of_is_finds_global_default_translation I18n.backend.store_translations 'en', :activerecord => {:errors => {:messages => {:wrong_length => 'global message'}}} Topic.validates_length_of :title, :is => 5 From aa5cdb0d47fb5484bfdde8244df7efeb2175bf3a Mon Sep 17 00:00:00 2001 From: Bruce Krysiak Date: Mon, 8 Dec 2008 16:38:04 -0800 Subject: [PATCH 013/188] Added a :camelize option to ActiveRecord and Hash to_xml serialization and from_xml deserialization Signed-off-by: Michael Koziarski --- .../serializers/xml_serializer.rb | 23 +++++++++++++------ .../test/cases/xml_serialization_test.rb | 7 ++++++ .../core_ext/hash/conversions.rb | 20 ++++++++++------ activesupport/test/core_ext/hash_ext_test.rb | 7 ++++++ 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/activerecord/lib/active_record/serializers/xml_serializer.rb b/activerecord/lib/active_record/serializers/xml_serializer.rb index d171b742f5..4749823b94 100644 --- a/activerecord/lib/active_record/serializers/xml_serializer.rb +++ b/activerecord/lib/active_record/serializers/xml_serializer.rb @@ -23,11 +23,12 @@ module ActiveRecord #:nodoc: # # # This behavior can be controlled with :only, :except, - # :skip_instruct, :skip_types and :dasherize. + # :skip_instruct, :skip_types, :dasherize and :camelize . # The :only and :except options are the same as for the # +attributes+ method. The default is to dasherize all column names, but you - # can disable this setting :dasherize to +false+. To not have the - # column type included in the XML output set :skip_types to +true+. + # can disable this setting :dasherize to +false+. Setting :camelize + # to +true+ will camelize all column names - this also overrides :dasherize. + # To not have the column type included in the XML output set :skip_types to +true+. # # For instance: # @@ -178,13 +179,22 @@ module ActiveRecord #:nodoc: def root root = (options[:root] || @record.class.to_s.underscore).to_s - dasherize? ? root.dasherize : root + reformat_name(root) end def dasherize? !options.has_key?(:dasherize) || options[:dasherize] end + def camelize? + options.has_key?(:camelize) && options[:camelize] + end + + def reformat_name(name) + name = name.camelize if camelize? + dasherize? ? name.dasherize : name + end + def serializable_attributes serializable_attribute_names.collect { |name| Attribute.new(name, @record) } end @@ -212,7 +222,7 @@ module ActiveRecord #:nodoc: def add_tag(attribute) builder.tag!( - dasherize? ? attribute.name.dasherize : attribute.name, + reformat_name(attribute.name), attribute.value.to_s, attribute.decorations(!options[:skip_types]) ) @@ -220,8 +230,7 @@ module ActiveRecord #:nodoc: def add_associations(association, records, opts) if records.is_a?(Enumerable) - tag = association.to_s - tag = tag.dasherize if dasherize? + tag = reformat_name(association.to_s) if records.empty? builder.tag!(tag, :type => :array) else diff --git a/activerecord/test/cases/xml_serialization_test.rb b/activerecord/test/cases/xml_serialization_test.rb index 63f48865cc..39c6ea820d 100644 --- a/activerecord/test/cases/xml_serialization_test.rb +++ b/activerecord/test/cases/xml_serialization_test.rb @@ -31,6 +31,13 @@ class XmlSerializationTest < ActiveRecord::TestCase assert_match %r{ 'xml_contact', :camelize => true + assert_match %r{^}, @xml + assert_match %r{$}, @xml + assert_match %r{ Builder::XmlMarkup.new(:indent => options[:indent]), :root => "hash" }) options[:builder].instruct! unless options.delete(:skip_instruct) - dasherize = !options.has_key?(:dasherize) || options[:dasherize] - root = dasherize ? options[:root].to_s.dasherize : options[:root].to_s + root = rename_key(options[:root].to_s, options) options[:builder].__send__(:method_missing, root) do each do |key, value| @@ -122,7 +121,7 @@ module ActiveSupport #:nodoc: else type_name = XML_TYPE_NAMES[value.class.name] - key = dasherize ? key.to_s.dasherize : key.to_s + key = rename_key(key.to_s, options) attributes = options[:skip_types] || value.nil? || type_name.nil? ? { } : { :type => type_name } if value.nil? @@ -142,9 +141,16 @@ module ActiveSupport #:nodoc: end + def rename_key(key, options = {}) + camelize = options.has_key?(:camelize) && options[:camelize] + dasherize = !options.has_key?(:dasherize) || options[:dasherize] + key = key.camelize if camelize + dasherize ? key.dasherize : key + end + module ClassMethods def from_xml(xml) - typecast_xml_value(undasherize_keys(XmlMini.parse(xml))) + typecast_xml_value(unrename_keys(XmlMini.parse(xml))) end private @@ -210,15 +216,15 @@ module ActiveSupport #:nodoc: end end - def undasherize_keys(params) + def unrename_keys(params) case params.class.to_s when "Hash" params.inject({}) do |h,(k,v)| - h[k.to_s.tr("-", "_")] = undasherize_keys(v) + h[k.to_s.underscore.tr("-", "_")] = unrename_keys(v) h end when "Array" - params.map { |v| undasherize_keys(v) } + params.map { |v| unrename_keys(v) } else params end diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 30cbba26b0..63ccb5a7da 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -403,6 +403,13 @@ class HashToXmlTest < Test::Unit::TestCase assert xml.include?(%(David)) end + def test_one_level_camelize_true + xml = { :name => "David", :street_name => "Paulina" }.to_xml(@xml_options.merge(:camelize => true)) + assert_equal "", xml.first(8) + assert xml.include?(%(Paulina)) + assert xml.include?(%(David)) + end + def test_one_level_with_types xml = { :name => "David", :street => "Paulina", :age => 26, :age_in_millis => 820497600000, :moved_on => Date.new(2005, 11, 15), :resident => :yes }.to_xml(@xml_options) assert_equal "", xml.first(8) From b30ae1974851b20ef430df9de17e6e79e5b25ad2 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 10 Dec 2008 14:48:12 -0800 Subject: [PATCH 014/188] Revert "Fix: counter_cache should decrement on deleting associated records." [#1196 state:open] This reverts commit 05f2183747c8e75c9e8bbaadb9573b4bdf41ecfc. --- .../associations/association_collection.rb | 3 --- .../has_many_associations_test.rb | 20 ------------------- .../has_many_through_associations_test.rb | 14 ------------- 3 files changed, 37 deletions(-) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 0ad58010b2..0ff91fbdf8 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -202,9 +202,6 @@ module ActiveRecord records.each do |record| @target.delete(record) - if respond_to?(:cached_counter_attribute_name) && @owner[cached_counter_attribute_name] - @owner.class.decrement_counter(cached_counter_attribute_name, @owner.send(@owner.class.primary_key)) - end callback(:after_remove, record) end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 1f8b297e81..816ceb6855 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -552,18 +552,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 0, companies(:first_firm).clients_of_firm(true).size end - def test_deleting_updates_counter_cache - post = Post.first - - post.comments.delete(post.comments.first) - post.reload - assert_equal post.comments(true).size, post.comments_count - - post.comments.delete(post.comments.first) - post.reload - assert_equal 0, post.comments_count - end - def test_deleting_before_save new_firm = Firm.new("name" => "A New Firm, Inc.") new_client = new_firm.clients_of_firm.build("name" => "Another Client") @@ -617,14 +605,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end end - def test_clearing_updates_counter_cache - post = Post.first - - post.comments.clear - post.reload - assert_equal 0, post.comments_count - end - def test_clearing_a_dependent_association_collection firm = companies(:first_firm) client_id = firm.dependent_clients_of_firm.first.id diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index ba3428a508..a07f4bcbdd 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -3,9 +3,6 @@ require 'models/post' require 'models/person' require 'models/reader' require 'models/comment' -require 'models/tag' -require 'models/tagging' -require 'models/author' class HasManyThroughAssociationsTest < ActiveRecord::TestCase fixtures :posts, :readers, :people, :comments, :authors @@ -87,17 +84,6 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:welcome).reload.people(true).empty? end - def test_deleting_updates_counter_cache - taggable = Tagging.first.taggable - taggable.taggings.push(Tagging.new) - taggable.reload - assert_equal 1, taggable.taggings_count - - taggable.taggings.delete(taggable.taggings.first) - taggable.reload - assert_equal 0, taggable.taggings_count - end - def test_replace_association assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)} From 9f69ff12d44c4d1e475fd6efede120ccedba3b3e Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Wed, 10 Dec 2008 22:36:58 +0000 Subject: [PATCH 015/188] Squash memory leak when calling flush with an empty buffer [#1552 state:committed] Signed-off-by: Jeremy Kemper --- activesupport/lib/active_support/buffered_logger.rb | 5 ++++- activesupport/test/buffered_logger_test.rb | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/buffered_logger.rb b/activesupport/lib/active_support/buffered_logger.rb index b2c863c893..445d8edf47 100644 --- a/activesupport/lib/active_support/buffered_logger.rb +++ b/activesupport/lib/active_support/buffered_logger.rb @@ -96,9 +96,12 @@ module ActiveSupport @guard.synchronize do unless buffer.empty? old_buffer = buffer - clear_buffer @log.write(old_buffer.join) end + + # Important to do this even if buffer was empty or else @buffer will + # accumulate empty arrays for each request where nothing was logged. + clear_buffer end end diff --git a/activesupport/test/buffered_logger_test.rb b/activesupport/test/buffered_logger_test.rb index 28dd34334f..e178ced06d 100644 --- a/activesupport/test/buffered_logger_test.rb +++ b/activesupport/test/buffered_logger_test.rb @@ -137,4 +137,10 @@ class BufferedLoggerTest < Test::Unit::TestCase assert @output.string.include?("a\nb\nc\n") assert @output.string.include?("x\ny\nz\n") end + + def test_flush_should_remove_empty_buffers + @logger.send :buffer + @logger.expects :clear_buffer + @logger.flush + end end From 455c7f9e37fda2969e52698b766413fc735eb488 Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Wed, 10 Dec 2008 20:57:19 +0000 Subject: [PATCH 016/188] Don't use the transaction instance method so that people with has_one/belongs_to :transaction aren't fubared [#1551 state:committed] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/transactions.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 27b5aca18f..0a27ea980e 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -147,7 +147,7 @@ module ActiveRecord end def save_with_transactions! #:nodoc: - rollback_active_record_state! { transaction { save_without_transactions! } } + rollback_active_record_state! { self.class.transaction { save_without_transactions! } } end # Reset id and @new_record if the transaction rolls back. @@ -175,7 +175,7 @@ module ActiveRecord # instance. def with_transaction_returning_status(method, *args) status = nil - transaction do + self.class.transaction do status = send(method, *args) raise ActiveRecord::Rollback unless status end From 75fa82418d54b36b6092767f2a2b5c1d5324441b Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 10 Dec 2008 18:07:02 -0600 Subject: [PATCH 017/188] Prefer Rails.logger over RAILS_DEFAULT_LOGGER --- actionpack/lib/action_controller/failsafe.rb | 4 ++-- activesupport/lib/active_support/dependencies.rb | 6 +++--- activesupport/lib/active_support/deprecation.rb | 2 +- railties/lib/commands/runner.rb | 4 +++- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/actionpack/lib/action_controller/failsafe.rb b/actionpack/lib/action_controller/failsafe.rb index bb6ef39470..1cd649b2e1 100644 --- a/actionpack/lib/action_controller/failsafe.rb +++ b/actionpack/lib/action_controller/failsafe.rb @@ -42,8 +42,8 @@ module ActionController end def failsafe_logger - if defined?(::RAILS_DEFAULT_LOGGER) && !::RAILS_DEFAULT_LOGGER.nil? - ::RAILS_DEFAULT_LOGGER + if defined? Rails && Rails.logger + Rails.logger else Logger.new($stderr) end diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 293450c180..23b7aee514 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -559,9 +559,9 @@ module ActiveSupport #:nodoc: # Old style environment.rb referenced this method directly. Please note, it doesn't # actually *do* anything any more. def self.root(*args) - if defined?(RAILS_DEFAULT_LOGGER) - RAILS_DEFAULT_LOGGER.warn "Your environment.rb uses the old syntax, it may not continue to work in future releases." - RAILS_DEFAULT_LOGGER.warn "For upgrade instructions please see: http://manuals.rubyonrails.com/read/book/19" + if defined? Rails && Rails.logger + Rails.logger.warn "Your environment.rb uses the old syntax, it may not continue to work in future releases." + Rails.logger.warn "For upgrade instructions please see: http://manuals.rubyonrails.com/read/book/19" end end end diff --git a/activesupport/lib/active_support/deprecation.rb b/activesupport/lib/active_support/deprecation.rb index 543e3b08d2..f18ea197a5 100644 --- a/activesupport/lib/active_support/deprecation.rb +++ b/activesupport/lib/active_support/deprecation.rb @@ -13,7 +13,7 @@ module ActiveSupport $stderr.puts callstack.join("\n ") if debug }, 'development' => Proc.new { |message, callstack| - logger = defined?(::RAILS_DEFAULT_LOGGER) ? ::RAILS_DEFAULT_LOGGER : Logger.new($stderr) + logger = defined? Rails ? Rails.logger : Logger.new($stderr) logger.warn message logger.debug callstack.join("\n ") if debug } diff --git a/railties/lib/commands/runner.rb b/railties/lib/commands/runner.rb index 2411c3d270..510128318a 100644 --- a/railties/lib/commands/runner.rb +++ b/railties/lib/commands/runner.rb @@ -48,5 +48,7 @@ begin eval(code_or_file) end ensure - RAILS_DEFAULT_LOGGER.flush if RAILS_DEFAULT_LOGGER + if defined? Rails + Rails.logger.flush if Rails.logger.respond_to?(:flush) + end end From 69387ce0169b95d3a170cfb1c66a7570b1746e37 Mon Sep 17 00:00:00 2001 From: Christos Zisopoulos Date: Wed, 10 Dec 2008 18:38:28 -0600 Subject: [PATCH 018/188] Fix for Integration::Session follow_redirect! headers['location'] bug with Rack [#1555 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_controller/integration.rb | 2 +- actionpack/test/controller/integration_test.rb | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/actionpack/lib/action_controller/integration.rb b/actionpack/lib/action_controller/integration.rb index eeabe5b845..0f0db03b6b 100644 --- a/actionpack/lib/action_controller/integration.rb +++ b/actionpack/lib/action_controller/integration.rb @@ -126,7 +126,7 @@ module ActionController # performed on the location header. def follow_redirect! raise "not a redirect! #{@status} #{@status_message}" unless redirect? - get(interpret_uri(headers['location'].first)) + get(interpret_uri(headers['location'])) status end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 4735b2927b..6a793c8bb6 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -30,14 +30,6 @@ class SessionTest < Test::Unit::TestCase assert_raise(RuntimeError) { @session.follow_redirect! } end - def test_follow_redirect_calls_get_and_returns_status - @session.stubs(:redirect?).returns(true) - @session.stubs(:headers).returns({"location" => ["www.google.com"]}) - @session.stubs(:status).returns(200) - @session.expects(:get) - assert_equal 200, @session.follow_redirect! - end - def test_request_via_redirect_uses_given_method path = "/somepath"; args = {:id => '1'}; headers = {"X-Test-Header" => "testvalue"} @session.expects(:put).with(path, args, headers) @@ -323,6 +315,10 @@ class IntegrationProcessTest < ActionController::IntegrationTest assert_equal "You are being redirected.", response.body assert_kind_of HTML::Document, html_document assert_equal 1, request_count + + follow_redirect! + assert_response :success + assert_equal "/get", path end end From 7394d12dc7c4bd6c1604e482042efab23f455794 Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Thu, 11 Dec 2008 14:12:06 +0100 Subject: [PATCH 019/188] Fixed ActiveSupport::OrderedHash #delete_if, #reject!, and #reject, which did not sync the @keys after the operation. This probably holds true for other mutating methods as well. Signed-off-by: Michael Koziarski --- .../lib/active_support/ordered_hash.rb | 25 +++++++++++++++++++ activesupport/test/ordered_hash_test.rb | 22 +++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index 1ed7737017..fa68db5604 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -25,6 +25,25 @@ module ActiveSupport super end + def delete_if + super + sync_keys! + self + end + + def reject! + super + sync_keys! + self + end + + def reject(&block) + dup.reject!(&block) + end + + alias_method :super_keys, :keys + private :super_keys + def keys @keys end @@ -48,6 +67,12 @@ module ActiveSupport def each keys.each {|key| yield [key, self[key]]} end + + private + + def sync_keys! + (@keys - super_keys).each { |k| @keys.delete(k) } + end end end end diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb index 094f9316d6..ca5fce6267 100644 --- a/activesupport/test/ordered_hash_test.rb +++ b/activesupport/test/ordered_hash_test.rb @@ -83,4 +83,24 @@ class OrderedHashTest < Test::Unit::TestCase def test_each_with_index @ordered_hash.each_with_index { |pair, index| assert_equal [@keys[index], @values[index]], pair} end -end + + def test_delete_if + (copy = @ordered_hash.dup).delete('pink') + assert_equal copy, @ordered_hash.delete_if { |k, _| k == 'pink' } + assert !@ordered_hash.keys.include?('pink') + end + + def test_reject! + (copy = @ordered_hash.dup).delete('pink') + @ordered_hash.reject! { |k, _| k == 'pink' } + assert_equal copy, @ordered_hash + assert !@ordered_hash.keys.include?('pink') + end + + def test_reject + copy = @ordered_hash.dup + new_ordered_hash = @ordered_hash.reject { |k, _| k == 'pink' } + assert_equal copy, @ordered_hash + assert !new_ordered_hash.keys.include?('pink') + end +end \ No newline at end of file From 7cfa6c535bc54f16a3fc7fa39969d4410de3e483 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 11 Dec 2008 10:17:29 -0600 Subject: [PATCH 020/188] Fixed template lookups from outside the rails root [#1557 state:resolved] --- actionpack/lib/action_controller/rescue.rb | 2 +- actionpack/lib/action_view/paths.rb | 4 ++++ actionpack/lib/action_view/template.rb | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index d7b0e96c93..24ee160ee8 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -39,7 +39,7 @@ module ActionController #:nodoc: } RESCUES_TEMPLATE_PATH = ActionView::PathSet::Path.new( - "#{File.dirname(__FILE__)}/templates", true) + File.join(File.dirname(__FILE__), "templates"), true) def self.included(base) #:nodoc: base.cattr_accessor :rescue_responses diff --git a/actionpack/lib/action_view/paths.rb b/actionpack/lib/action_view/paths.rb index 9c8b8ade1e..623b9ff6b0 100644 --- a/actionpack/lib/action_view/paths.rb +++ b/actionpack/lib/action_view/paths.rb @@ -57,6 +57,10 @@ module ActionView #:nodoc: end end + def to_str + path.to_str + end + def ==(path) to_str == path.to_str end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index e32b7688d0..93748638c3 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -105,7 +105,7 @@ module ActionView #:nodoc: def find_full_path(path, load_paths) load_paths = Array(load_paths) + [nil] load_paths.each do |load_path| - file = [load_path, path].compact.join('/') + file = load_path ? "#{load_path.to_str}/#{path}" : path return load_path, file if File.file?(file) end raise MissingTemplate.new(load_paths, path) From 5ede4ce188d29aef94af78f27d89169ac4ee54cd Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Thu, 11 Dec 2008 10:20:33 -0600 Subject: [PATCH 021/188] Fixed session related memory leak [#1558 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_controller/base.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index c2f0c1c4f6..13f2e9072e 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -1160,6 +1160,9 @@ module ActionController #:nodoc: def reset_session #:doc: request.reset_session @_session = request.session + #http://rails.lighthouseapp.com/projects/8994/tickets/1558-memory-problem-on-reset_session-in-around_filter#ticket-1558-1 + #MRI appears to have a GC related memory leak to do with the finalizer that is defined on CGI::Session + ObjectSpace.undefine_finalizer(@_session) response.session = @_session end From 49306ccacf01e36d444771d42321965616e226f0 Mon Sep 17 00:00:00 2001 From: mark Date: Thu, 11 Dec 2008 11:06:35 -0600 Subject: [PATCH 022/188] Add :partial option to assert_template [#1550 state:resolved] Signed-off-by: Joshua Peek --- .../assertions/response_assertions.rb | 56 ++++++++++++++----- .../lib/action_controller/test_process.rb | 4 +- actionpack/lib/action_view/renderable.rb | 5 -- actionpack/lib/action_view/test_case.rb | 20 +++++++ .../controller/action_pack_assertions_test.rb | 6 +- actionpack/test/controller/render_test.rb | 7 +++ 6 files changed, 75 insertions(+), 23 deletions(-) diff --git a/actionpack/lib/action_controller/assertions/response_assertions.rb b/actionpack/lib/action_controller/assertions/response_assertions.rb index 7ab24389b8..5976090273 100644 --- a/actionpack/lib/action_controller/assertions/response_assertions.rb +++ b/actionpack/lib/action_controller/assertions/response_assertions.rb @@ -16,7 +16,7 @@ module ActionController # ==== Examples # # # assert that the response was a redirection - # assert_response :redirect + # assert_response :redirect # # # assert that the response code was status code 401 (unauthorized) # assert_response 401 @@ -41,7 +41,7 @@ module ActionController end end - # Assert that the redirection options passed in match those of the redirect called in the latest action. + # Assert that the redirection options passed in match those of the redirect called in the latest action. # This match can be partial, such that assert_redirected_to(:controller => "weblog") will also # match the redirection of redirect_to(:controller => "weblog", :action => "show") and so on. # @@ -60,12 +60,12 @@ module ActionController clean_backtrace do assert_response(:redirect, message) return true if options == @response.redirected_to - + # Support partial arguments for hash redirections if options.is_a?(Hash) && @response.redirected_to.is_a?(Hash) return true if options.all? {|(key, value)| @response.redirected_to[key] == value} end - + redirected_to_after_normalisation = normalize_argument_to_redirection(@response.redirected_to) options_after_normalisation = normalize_argument_to_redirection(options) @@ -75,29 +75,59 @@ module ActionController end end - # Asserts that the request was rendered with the appropriate template file. + # Asserts that the request was rendered with the appropriate template file or partials # # ==== Examples # # # assert that the "new" view template was rendered # assert_template "new" # - def assert_template(expected = nil, message=nil) + # # assert that the "_customer" partial was rendered twice + # assert_template :partial => '_customer', :count => 2 + # + # # assert that no partials were rendered + # assert_template :partial => false + # + def assert_template(options = {}, message = nil) clean_backtrace do - rendered = @response.rendered_template.to_s - msg = build_message(message, "expecting but rendering with ", expected, rendered) - assert_block(msg) do - if expected.nil? - @response.rendered_template.blank? + case options + when NilClass, String + rendered = @response.rendered[:template].to_s + msg = build_message(message, + "expecting but rendering with ", + options, rendered) + assert_block(msg) do + if options.nil? + @response.rendered[:template].blank? + else + rendered.to_s.match(options) + end + end + when Hash + if expected_partial = options[:partial] + partials = @response.rendered[:partials] + if expected_count = options[:count] + found = partials.detect { |p, _| p.to_s.match(expected_partial) } + actual_count = found.nil? ? 0 : found.second + msg = build_message(message, + "expecting ? to be rendered ? time(s) but rendered ? time(s)", + expected_partial, expected_count, actual_count) + assert(actual_count == expected_count.to_i, msg) + else + msg = build_message(message, + "expecting partial but action rendered ", + options[:partial], partials.keys) + assert(partials.keys.any? { |p| p.to_s.match(expected_partial) }, msg) + end else - rendered.to_s.match(expected) + assert @response.rendered[:partials].empty?, + "Expected no partials to be rendered" end end end end private - # Proxy to to_param if the object will respond to it. def parameterize(value) value.respond_to?(:to_param) ? value.to_param : value diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index cd3914f011..78dc3827bc 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -221,8 +221,8 @@ module ActionController #:nodoc: # Returns the template of the file which was used to # render this response (or nil) - def rendered_template - template.instance_variable_get(:@_first_render) + def rendered + template.instance_variable_get(:@_rendered) end # A shortcut to the flash. Returns an empty hash if no session flash exists. diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index 7258ad04bf..7c0e62f1d7 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -28,11 +28,6 @@ module ActionView stack = view.instance_variable_get(:@_render_stack) stack.push(self) - # This is only used for TestResponse to set rendered_template - unless is_a?(InlineTemplate) || view.instance_variable_get(:@_first_render) - view.instance_variable_set(:@_first_render, self) - end - view.send(:_evaluate_assigns_and_ivars) view.send(:_set_controller_content_type, mime_type) if respond_to?(:mime_type) diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 4ab4ed233f..a5655843d2 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -1,4 +1,24 @@ module ActionView + class Base + alias_method :initialize_without_template_tracking, :initialize + def initialize(*args) + @_rendered = { :template => nil, :partials => Hash.new(0) } + initialize_without_template_tracking(*args) + end + end + + module Renderable + alias_method :render_without_template_tracking, :render + def render(view, local_assigns = {}) + if respond_to?(:path) && !is_a?(InlineTemplate) + rendered = view.instance_variable_get(:@_rendered) + rendered[:partials][self] += 1 if is_a?(RenderablePartial) + rendered[:template] ||= self + end + render_without_template_tracking(view, local_assigns) + end + end + class TestCase < ActiveSupport::TestCase include ActionController::TestCase::Assertions diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index ea56048f37..87c12ee4ee 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -326,11 +326,11 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase # check if we were rendered by a file-based template? def test_rendered_action process :nothing - assert_nil @response.rendered_template + assert_nil @response.rendered[:template] process :hello_world - assert @response.rendered_template - assert 'hello_world', @response.rendered_template.to_s + assert @response.rendered[:template] + assert 'hello_world', @response.rendered[:template].to_s end # check the redirection location diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index c5496a9af5..87733c2d33 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -1310,21 +1310,28 @@ class RenderTest < ActionController::TestCase def test_partial_collection_with_spacer get :partial_collection_with_spacer assert_equal "Hello: davidonly partialHello: mary", @response.body + assert_template :partial => 'test/_partial_only' + assert_template :partial => '_customer' end def test_partial_collection_shorthand_with_locals get :partial_collection_shorthand_with_locals assert_equal "Bonjour: davidBonjour: mary", @response.body + assert_template :partial => 'customers/_customer', :count => 2 + assert_template :partial => '_completely_fake_and_made_up_template_that_cannot_possibly_be_rendered', :count => 0 end def test_partial_collection_shorthand_with_different_types_of_records get :partial_collection_shorthand_with_different_types_of_records assert_equal "Bonjour bad customer: mark0Bonjour good customer: craig1Bonjour bad customer: john2Bonjour good customer: zach3Bonjour good customer: brandon4Bonjour bad customer: dan5", @response.body + assert_template :partial => 'good_customers/_good_customer', :count => 3 + assert_template :partial => 'bad_customers/_bad_customer', :count => 3 end def test_empty_partial_collection get :empty_partial_collection assert_equal " ", @response.body + assert_template :partial => false end def test_partial_with_hash_object From 6eed6cf5c6db0dbf96c1c2756eb946f54277a0cb Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Thu, 11 Dec 2008 15:19:03 +0000 Subject: [PATCH 023/188] Make delete_if/reject faster and fix other mutators [#1559 state:committed] Signed-off-by: Jeremy Kemper --- .../lib/active_support/ordered_hash.rb | 37 ++++++++++++------- activesupport/test/ordered_hash_test.rb | 37 +++++++++++++++++++ 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index fa68db5604..3def0be639 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -18,19 +18,13 @@ module ActiveSupport end def delete(key) - array_index = has_key?(key) && index(key) - if array_index - @keys.delete_at(array_index) + if has_key? key + index = @keys.index(key) + @keys.delete_at index end super end - def delete_if - super - sync_keys! - self - end - def reject! super sync_keys! @@ -41,9 +35,6 @@ module ActiveSupport dup.reject!(&block) end - alias_method :super_keys, :keys - private :super_keys - def keys @keys end @@ -68,10 +59,30 @@ module ActiveSupport keys.each {|key| yield [key, self[key]]} end + alias_method :each_pair, :each + + def clear + super + @keys.clear + self + end + + def shift + k = @keys.first + v = delete(k) + [k, v] + end + + def merge(other_hash) + result = dup + other_hash.each {|k,v| result[k]=v} + result + end + private def sync_keys! - (@keys - super_keys).each { |k| @keys.delete(k) } + @keys.delete_if {|k| !has_key?(k)} end end end diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb index ca5fce6267..0e2aa4543d 100644 --- a/activesupport/test/ordered_hash_test.rb +++ b/activesupport/test/ordered_hash_test.rb @@ -36,9 +36,11 @@ class OrderedHashTest < Test::Unit::TestCase @ordered_hash[key] = value assert_equal @keys.length + 1, @ordered_hash.length + assert_equal @ordered_hash.keys.length, @ordered_hash.length assert_equal value, @ordered_hash.delete(key) assert_equal @keys.length, @ordered_hash.length + assert_equal @ordered_hash.keys.length, @ordered_hash.length assert_nil @ordered_hash.delete(bad_key) end @@ -84,6 +86,17 @@ class OrderedHashTest < Test::Unit::TestCase @ordered_hash.each_with_index { |pair, index| assert_equal [@keys[index], @values[index]], pair} end + def test_each_pair + values = [] + keys = [] + @ordered_hash.each_pair do |key, value| + keys << key + values << value + end + assert_equal @values, values + assert_equal @keys, keys + end + def test_delete_if (copy = @ordered_hash.dup).delete('pink') assert_equal copy, @ordered_hash.delete_if { |k, _| k == 'pink' } @@ -103,4 +116,28 @@ class OrderedHashTest < Test::Unit::TestCase assert_equal copy, @ordered_hash assert !new_ordered_hash.keys.include?('pink') end + + def test_clear + @ordered_hash.clear + assert_equal [], @ordered_hash.keys + end + + def test_merge + other_hash = ActiveSupport::OrderedHash.new + other_hash['purple'] = '800080' + other_hash['violet'] = 'ee82ee' + merged = @ordered_hash.merge other_hash + assert_equal merged.length, @ordered_hash.length + other_hash.length + assert_equal @keys + ['purple', 'violet'], merged.keys + + @ordered_hash.merge! other_hash + assert_equal @ordered_hash, merged + assert_equal @ordered_hash.keys, merged.keys + end + + def test_shift + pair = @ordered_hash.shift + assert_equal [@keys.first, @values.first], pair + assert !@ordered_hash.keys.include?(pair.first) + end end \ No newline at end of file From 4dcd8f01afe5800baa67bbdf72832afb0d627755 Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Thu, 11 Dec 2008 15:19:03 +0000 Subject: [PATCH 024/188] Make delete_if/reject faster and fix other mutators [#1559 state:committed] Signed-off-by: Jeremy Kemper --- .../lib/active_support/ordered_hash.rb | 37 ++++++++++++------- activesupport/test/ordered_hash_test.rb | 37 +++++++++++++++++++ 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index fa68db5604..3def0be639 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -18,19 +18,13 @@ module ActiveSupport end def delete(key) - array_index = has_key?(key) && index(key) - if array_index - @keys.delete_at(array_index) + if has_key? key + index = @keys.index(key) + @keys.delete_at index end super end - def delete_if - super - sync_keys! - self - end - def reject! super sync_keys! @@ -41,9 +35,6 @@ module ActiveSupport dup.reject!(&block) end - alias_method :super_keys, :keys - private :super_keys - def keys @keys end @@ -68,10 +59,30 @@ module ActiveSupport keys.each {|key| yield [key, self[key]]} end + alias_method :each_pair, :each + + def clear + super + @keys.clear + self + end + + def shift + k = @keys.first + v = delete(k) + [k, v] + end + + def merge(other_hash) + result = dup + other_hash.each {|k,v| result[k]=v} + result + end + private def sync_keys! - (@keys - super_keys).each { |k| @keys.delete(k) } + @keys.delete_if {|k| !has_key?(k)} end end end diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb index ca5fce6267..0e2aa4543d 100644 --- a/activesupport/test/ordered_hash_test.rb +++ b/activesupport/test/ordered_hash_test.rb @@ -36,9 +36,11 @@ class OrderedHashTest < Test::Unit::TestCase @ordered_hash[key] = value assert_equal @keys.length + 1, @ordered_hash.length + assert_equal @ordered_hash.keys.length, @ordered_hash.length assert_equal value, @ordered_hash.delete(key) assert_equal @keys.length, @ordered_hash.length + assert_equal @ordered_hash.keys.length, @ordered_hash.length assert_nil @ordered_hash.delete(bad_key) end @@ -84,6 +86,17 @@ class OrderedHashTest < Test::Unit::TestCase @ordered_hash.each_with_index { |pair, index| assert_equal [@keys[index], @values[index]], pair} end + def test_each_pair + values = [] + keys = [] + @ordered_hash.each_pair do |key, value| + keys << key + values << value + end + assert_equal @values, values + assert_equal @keys, keys + end + def test_delete_if (copy = @ordered_hash.dup).delete('pink') assert_equal copy, @ordered_hash.delete_if { |k, _| k == 'pink' } @@ -103,4 +116,28 @@ class OrderedHashTest < Test::Unit::TestCase assert_equal copy, @ordered_hash assert !new_ordered_hash.keys.include?('pink') end + + def test_clear + @ordered_hash.clear + assert_equal [], @ordered_hash.keys + end + + def test_merge + other_hash = ActiveSupport::OrderedHash.new + other_hash['purple'] = '800080' + other_hash['violet'] = 'ee82ee' + merged = @ordered_hash.merge other_hash + assert_equal merged.length, @ordered_hash.length + other_hash.length + assert_equal @keys + ['purple', 'violet'], merged.keys + + @ordered_hash.merge! other_hash + assert_equal @ordered_hash, merged + assert_equal @ordered_hash.keys, merged.keys + end + + def test_shift + pair = @ordered_hash.shift + assert_equal [@keys.first, @values.first], pair + assert !@ordered_hash.keys.include?(pair.first) + end end \ No newline at end of file From f9a02b12d15bdbd3c2ed18b16b31b712a77027bc Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Mon, 15 Dec 2008 15:37:27 +0100 Subject: [PATCH 025/188] =?UTF-8?q?Added=20gem=20backtrace=20pretty=20prit?= =?UTF-8?q?ing=20(Juan=20Lupi=C3=B3n)=20[#1497=20state:committed]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- railties/lib/rails/backtrace_cleaner.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/railties/lib/rails/backtrace_cleaner.rb b/railties/lib/rails/backtrace_cleaner.rb index f344c6477d..94d34cda39 100644 --- a/railties/lib/rails/backtrace_cleaner.rb +++ b/railties/lib/rails/backtrace_cleaner.rb @@ -9,6 +9,8 @@ module Rails RAILS_NOISE = %w( script/server ) RUBY_NOISE = %w( rubygems/custom_require benchmark.rb ) + GEMS_DIR = Gem.default_dir + ALL_NOISE = VENDOR_DIRS + SERVER_DIRS + RAILS_NOISE + RUBY_NOISE def initialize @@ -16,6 +18,7 @@ module Rails add_filter { |line| line.sub(RAILS_ROOT, '') } add_filter { |line| line.sub(ERB_METHOD_SIG, '') } add_filter { |line| line.sub('./', '/') } # for tests + add_filter { |line| line.sub(/(#{GEMS_DIR})\/gems\/([a-z]+)-([0-9.]+)\/(.*)/, '\2 (\3) \4')} # http://gist.github.com/30430 add_silencer { |line| ALL_NOISE.any? { |dir| line.include?(dir) } } end end From 38412ecb5daa1826574ad0f132d23dc2ef4288bf Mon Sep 17 00:00:00 2001 From: Dan Pickett Date: Mon, 15 Dec 2008 11:47:39 -0600 Subject: [PATCH 026/188] Fixed ActionView::TestCase current url context [#1561 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_view/test_case.rb | 5 ++++- actionpack/test/view/test_case_test.rb | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 actionpack/test/view/test_case_test.rb diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index a5655843d2..1a9ef983a5 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -60,11 +60,14 @@ module ActionView end class TestController < ActionController::Base - attr_accessor :request, :response + attr_accessor :request, :response, :params def initialize @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new + + @params = {} + send(:initialize_current_url) end end diff --git a/actionpack/test/view/test_case_test.rb b/actionpack/test/view/test_case_test.rb new file mode 100644 index 0000000000..9124198b28 --- /dev/null +++ b/actionpack/test/view/test_case_test.rb @@ -0,0 +1,8 @@ +require 'abstract_unit' + +class TestCaseTest < ActionView::TestCase + def test_should_have_current_url + controller = TestController.new + assert_nothing_raised(NoMethodError){ controller.url_for({:controller => "foo", :action => "index"}) } + end +end From 7c18518105e98ccfd89fe64194ede27824dfe8b3 Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Mon, 15 Dec 2008 11:49:08 -0600 Subject: [PATCH 027/188] Properly parenthasize calls to defined?(Rails) in 75fa82418 [#1563 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_controller/failsafe.rb | 2 +- activesupport/lib/active_support/dependencies.rb | 2 +- activesupport/lib/active_support/deprecation.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_controller/failsafe.rb b/actionpack/lib/action_controller/failsafe.rb index 1cd649b2e1..b1e9957b49 100644 --- a/actionpack/lib/action_controller/failsafe.rb +++ b/actionpack/lib/action_controller/failsafe.rb @@ -42,7 +42,7 @@ module ActionController end def failsafe_logger - if defined? Rails && Rails.logger + if defined?(Rails) && Rails.logger Rails.logger else Logger.new($stderr) diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 23b7aee514..7ce9adec2c 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -559,7 +559,7 @@ module ActiveSupport #:nodoc: # Old style environment.rb referenced this method directly. Please note, it doesn't # actually *do* anything any more. def self.root(*args) - if defined? Rails && Rails.logger + if defined?(Rails) && Rails.logger Rails.logger.warn "Your environment.rb uses the old syntax, it may not continue to work in future releases." Rails.logger.warn "For upgrade instructions please see: http://manuals.rubyonrails.com/read/book/19" end diff --git a/activesupport/lib/active_support/deprecation.rb b/activesupport/lib/active_support/deprecation.rb index f18ea197a5..25b26e9c96 100644 --- a/activesupport/lib/active_support/deprecation.rb +++ b/activesupport/lib/active_support/deprecation.rb @@ -13,7 +13,7 @@ module ActiveSupport $stderr.puts callstack.join("\n ") if debug }, 'development' => Proc.new { |message, callstack| - logger = defined? Rails ? Rails.logger : Logger.new($stderr) + logger = defined?(Rails) ? Rails.logger : Logger.new($stderr) logger.warn message logger.debug callstack.join("\n ") if debug } From f36dafa492e3de66e624d81d6860f5f0536de6b0 Mon Sep 17 00:00:00 2001 From: Seth Fitzsimmons Date: Mon, 15 Dec 2008 12:00:55 -0600 Subject: [PATCH 028/188] Implement Mime::Type.=~ to match all synonyms against arg [#1573 state:resolved] Signed-off-by: Joshua Peek --- .../action_controller/assertions/selector_assertions.rb | 2 +- actionpack/lib/action_controller/mime_type.rb | 8 ++++++++ actionpack/test/controller/mime_type_test.rb | 8 ++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/assertions/selector_assertions.rb b/actionpack/lib/action_controller/assertions/selector_assertions.rb index e03fed7abb..248ca85994 100644 --- a/actionpack/lib/action_controller/assertions/selector_assertions.rb +++ b/actionpack/lib/action_controller/assertions/selector_assertions.rb @@ -587,7 +587,7 @@ module ActionController def response_from_page_or_rjs() content_type = @response.content_type - if content_type && content_type =~ /text\/javascript/ + if content_type && Mime::JS =~ content_type body = @response.body.dup root = HTML::Node.new(nil) diff --git a/actionpack/lib/action_controller/mime_type.rb b/actionpack/lib/action_controller/mime_type.rb index 6923a13f3f..43b3da8d35 100644 --- a/actionpack/lib/action_controller/mime_type.rb +++ b/actionpack/lib/action_controller/mime_type.rb @@ -176,6 +176,14 @@ module Mime end end + def =~(mime_type) + return false if mime_type.blank? + regexp = Regexp.new(mime_type.to_s) + (@synonyms + [ self ]).any? do |synonym| + synonym.to_s =~ regexp + end + end + # Returns true if Action Pack should check requests using this Mime Type for possible request forgery. See # ActionController::RequestForgeryProtection. def verify_request? diff --git a/actionpack/test/controller/mime_type_test.rb b/actionpack/test/controller/mime_type_test.rb index 21ae0419f1..9c4416ecf0 100644 --- a/actionpack/test/controller/mime_type_test.rb +++ b/actionpack/test/controller/mime_type_test.rb @@ -81,4 +81,12 @@ class MimeTypeTest < Test::Unit::TestCase assert verified.each { |type| assert Mime.const_get(type.to_s.upcase).verify_request?, "Verifiable Mime Type is not verified: #{type.inspect}" } assert unverified.each { |type| assert !Mime.const_get(type.to_s.upcase).verify_request?, "Nonverifiable Mime Type is verified: #{type.inspect}" } end + + def test_regexp_matcher + assert Mime::JS =~ "text/javascript" + assert Mime::JS =~ "application/javascript" + assert Mime::JS !~ "text/html" + assert !(Mime::JS !~ "text/javascript") + assert !(Mime::JS !~ "application/javascript") + end end From 4966076d35d5d9510590d87d90dae8daf79b2069 Mon Sep 17 00:00:00 2001 From: Seth Fitzsimmons Date: Mon, 15 Dec 2008 12:18:45 -0600 Subject: [PATCH 029/188] Use Mime::JS in place of explicit 'text/javascript' [#1573 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_controller/integration.rb | 4 +--- actionpack/lib/action_controller/test_process.rb | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_controller/integration.rb b/actionpack/lib/action_controller/integration.rb index 0f0db03b6b..212a293da0 100644 --- a/actionpack/lib/action_controller/integration.rb +++ b/actionpack/lib/action_controller/integration.rb @@ -227,9 +227,7 @@ module ActionController def xml_http_request(request_method, path, parameters = nil, headers = nil) headers ||= {} headers['X-Requested-With'] = 'XMLHttpRequest' - headers['Accept'] ||= 'text/javascript, text/html, application/xml, ' + - 'text/xml, */*' - + headers['Accept'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') process(request_method, path, parameters, headers) end alias xhr :xml_http_request diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index 78dc3827bc..c35c3c07e8 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -413,7 +413,7 @@ module ActionController #:nodoc: def xml_http_request(request_method, action, parameters = nil, session = nil, flash = nil) @request.env['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' - @request.env['HTTP_ACCEPT'] = 'text/javascript, text/html, application/xml, text/xml, */*' + @request.env['HTTP_ACCEPT'] = [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') returning __send__(request_method, action, parameters, session, flash) do @request.env.delete 'HTTP_X_REQUESTED_WITH' @request.env.delete 'HTTP_ACCEPT' From 262fef7ed57520b857605a0105fe7ba9265654f6 Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Sun, 14 Dec 2008 10:07:06 +0000 Subject: [PATCH 030/188] Make constantize look into ancestors [#410 state:resolved] Signed-off-by: Jeremy Kemper --- activesupport/lib/active_support/inflector.rb | 63 +++++++------------ activesupport/test/inflector_test.rb | 17 ++++- 2 files changed, 39 insertions(+), 41 deletions(-) diff --git a/activesupport/lib/active_support/inflector.rb b/activesupport/lib/active_support/inflector.rb index 4921b99677..9fb7540ea2 100644 --- a/activesupport/lib/active_support/inflector.rb +++ b/activesupport/lib/active_support/inflector.rb @@ -330,47 +330,30 @@ module ActiveSupport underscore(demodulize(class_name)) + (separate_class_name_and_id_with_underscore ? "_id" : "id") end - # Ruby 1.9 introduces an inherit argument for Module#const_get and - # #const_defined? and changes their default behavior. - if Module.method(:const_get).arity == 1 - # Tries to find a constant with the name specified in the argument string: - # - # "Module".constantize # => Module - # "Test::Unit".constantize # => Test::Unit - # - # The name is assumed to be the one of a top-level constant, no matter whether - # it starts with "::" or not. No lexical context is taken into account: - # - # C = 'outside' - # module M - # C = 'inside' - # C # => 'inside' - # "C".constantize # => 'outside', same as ::C - # end - # - # NameError is raised when the name is not in CamelCase or the constant is - # unknown. - def constantize(camel_cased_word) - names = camel_cased_word.split('::') - names.shift if names.empty? || names.first.empty? + # Tries to find a constant with the name specified in the argument string: + # + # "Module".constantize # => Module + # "Test::Unit".constantize # => Test::Unit + # + # The name is assumed to be the one of a top-level constant, no matter whether + # it starts with "::" or not. No lexical context is taken into account: + # + # C = 'outside' + # module M + # C = 'inside' + # C # => 'inside' + # "C".constantize # => 'outside', same as ::C + # end + # + # NameError is raised when the name is not in CamelCase or the constant is + # unknown. + def constantize(camel_cased_word) + names = camel_cased_word.split('::') + names.shift if names.empty? || names.first.empty? - constant = Object - names.each do |name| - constant = constant.const_defined?(name) ? constant.const_get(name) : constant.const_missing(name) - end - constant - end - else - def constantize(camel_cased_word) #:nodoc: - names = camel_cased_word.split('::') - names.shift if names.empty? || names.first.empty? - - constant = Object - names.each do |name| - constant = constant.const_get(name, false) || constant.const_missing(name) - end - constant - end + constant = Object + names.each { |name| constant = constant.const_get(name) } + constant end # Turns a number into an ordinal string used to denote the position in an diff --git a/activesupport/test/inflector_test.rb b/activesupport/test/inflector_test.rb index d8c93dc9ae..2c422ebe72 100644 --- a/activesupport/test/inflector_test.rb +++ b/activesupport/test/inflector_test.rb @@ -2,8 +2,21 @@ require 'abstract_unit' require 'inflector_test_cases' module Ace + module Extension + def self.included(base) + base.extend(ClassMethods) + end + + module ClassMethods + def mission_accomplished? + false + end + end + end + module Base class Case + include Extension end end end @@ -167,7 +180,9 @@ class InflectorTest < Test::Unit::TestCase end def test_constantize_does_lexical_lookup - assert_raises(NameError) { ActiveSupport::Inflector.constantize("Ace::Base::InflectorTest") } + assert_equal InflectorTest, ActiveSupport::Inflector.constantize("Ace::Base::InflectorTest") + assert_nothing_raised { Ace::Base::Case::ClassMethods } + assert_nothing_raised { assert_equal Ace::Base::Case::ClassMethods, ActiveSupport::Inflector.constantize("Ace::Base::Case::ClassMethods") } end def test_ordinal From a392f34fb4069ab847ff631130d023cdaf896735 Mon Sep 17 00:00:00 2001 From: "Hongli Lai (Phusion)" Date: Mon, 15 Dec 2008 14:47:19 -0600 Subject: [PATCH 031/188] Require mocha >= 0.9.3, older versions don't work anymore [#1579 state:resolved] Signed-off-by: Joshua Peek --- activerecord/test/cases/helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 2382bfe4fe..afba715448 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -24,6 +24,7 @@ end def uses_mocha(description) require 'rubygems' + gem 'mocha', '>= 0.9.3' require 'mocha' yield rescue LoadError From e8c1915416579a3840573ca2c80822d96cb31823 Mon Sep 17 00:00:00 2001 From: Nathan Weizenbaum Date: Mon, 15 Dec 2008 14:49:38 -0600 Subject: [PATCH 032/188] Auto-load template handlers based on unmatched extensions [#1540 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_view/template.rb | 10 +++----- .../lib/action_view/template_handlers.rb | 25 ++++++++++++++++++- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 93748638c3..8f4ca433c0 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -98,10 +98,6 @@ module ActionView #:nodoc: end private - def valid_extension?(extension) - Template.template_handler_extensions.include?(extension) - end - def find_full_path(path, load_paths) load_paths = Array(load_paths) + [nil] load_paths.each do |load_path| @@ -115,11 +111,11 @@ module ActionView #:nodoc: # [base_path, name, format, extension] def split(file) if m = file.match(/^(.*\/)?([^\.]+)\.?(\w+)?\.?(\w+)?\.?(\w+)?$/) - if valid_extension?(m[5]) # Multipart formats + if Template.valid_extension?(m[5]) # Multipart formats [m[1], m[2], "#{m[3]}.#{m[4]}", m[5]] - elsif valid_extension?(m[4]) # Single format + elsif Template.valid_extension?(m[4]) # Single format [m[1], m[2], m[3], m[4]] - elsif valid_extension?(m[3]) # No format + elsif Template.valid_extension?(m[3]) # No format [m[1], m[2], nil, m[3]] else # No extension [m[1], m[2], m[3], nil] diff --git a/actionpack/lib/action_view/template_handlers.rb b/actionpack/lib/action_view/template_handlers.rb index d06ddd5fb5..c50a51b0d1 100644 --- a/actionpack/lib/action_view/template_handlers.rb +++ b/actionpack/lib/action_view/template_handlers.rb @@ -28,6 +28,10 @@ module ActionView #:nodoc: @@template_handlers[extension.to_sym] = klass end + def valid_extension?(extension) + template_handler_extensions.include?(extension) || init_path_for_extension(extension) + end + def template_handler_extensions @@template_handlers.keys.map(&:to_s).sort end @@ -38,7 +42,26 @@ module ActionView #:nodoc: end def handler_class_for_extension(extension) - (extension && @@template_handlers[extension.to_sym]) || @@default_template_handlers + (extension && @@template_handlers[extension.to_sym] || autoload_handler_class(extension)) || + @@default_template_handlers end + + private + def autoload_handler_class(extension) + return if Gem.loaded_specs[extension] + return unless init_path = init_path_for_extension(extension) + Gem.activate(extension) + load(init_path) + handler_class_for_extension(extension) + end + + # Returns the path to the rails/init.rb file for the given extension, + # or nil if no gem provides it. + def init_path_for_extension(extension) + return unless spec = Gem.searcher.find(extension.to_s) + returning File.join(spec.full_gem_path, 'rails', 'init.rb') do |path| + return unless File.file?(path) + end + end end end From ed708307137c811d14e5fd2cb4ea550add381a82 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 15 Dec 2008 16:33:31 -0600 Subject: [PATCH 033/188] Switch to Rack based session stores. --- actionpack/lib/action_controller.rb | 15 +- actionpack/lib/action_controller/base.rb | 12 +- actionpack/lib/action_controller/cgi_ext.rb | 1 - .../lib/action_controller/cgi_ext/session.rb | 53 --- .../lib/action_controller/cgi_process.rb | 2 +- .../lib/action_controller/dispatcher.rb | 8 +- .../lib/action_controller/integration.rb | 4 +- .../lib/action_controller/middleware_stack.rb | 25 +- .../lib/action_controller/rack_process.rb | 139 +------ .../session/abstract_store.rb | 129 +++++++ .../session/active_record_store.rb | 350 ----------------- .../action_controller/session/cookie_store.rb | 348 +++++++++-------- .../action_controller/session/drb_server.rb | 32 -- .../action_controller/session/drb_store.rb | 35 -- .../session/mem_cache_store.rb | 123 ++---- .../action_controller/session_management.rb | 174 +++------ actionpack/test/abstract_unit.rb | 2 + .../activerecord/active_record_store_test.rb | 238 ++++++------ .../test/controller/integration_test.rb | 2 - .../controller/integration_upload_test.rb | 2 - actionpack/test/controller/rack_test.rb | 26 +- .../controller/session/cookie_store_test.rb | 354 +++++------------- .../session/mem_cache_store_test.rb | 233 ++++-------- .../test/controller/session_fixation_test.rb | 168 ++++----- .../controller/session_management_test.rb | 178 --------- actionpack/test/controller/webservice_test.rb | 2 - activerecord/lib/active_record.rb | 1 + .../lib/active_record/session_store.rb | 319 ++++++++++++++++ railties/lib/initializer.rb | 12 +- railties/lib/tasks/databases.rake | 2 +- railties/test/console_app_test.rb | 1 + 31 files changed, 1159 insertions(+), 1831 deletions(-) delete mode 100644 actionpack/lib/action_controller/cgi_ext/session.rb create mode 100644 actionpack/lib/action_controller/session/abstract_store.rb delete mode 100644 actionpack/lib/action_controller/session/active_record_store.rb delete mode 100755 actionpack/lib/action_controller/session/drb_server.rb delete mode 100644 actionpack/lib/action_controller/session/drb_store.rb delete mode 100644 actionpack/test/controller/session_management_test.rb create mode 100644 activerecord/lib/active_record/session_store.rb diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index abc404afe7..c170e4dd2a 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -89,20 +89,17 @@ module ActionController autoload :Headers, 'action_controller/headers' end + module Session + autoload :AbstractStore, 'action_controller/session/abstract_store' + autoload :CookieStore, 'action_controller/session/cookie_store' + autoload :MemCacheStore, 'action_controller/session/mem_cache_store' + end + # DEPRECATE: Remove CGI support autoload :CgiRequest, 'action_controller/cgi_process' autoload :CGIHandler, 'action_controller/cgi_process' end -class CGI - class Session - autoload :ActiveRecordStore, 'action_controller/session/active_record_store' - autoload :CookieStore, 'action_controller/session/cookie_store' - autoload :DRbStore, 'action_controller/session/drb_store' - autoload :MemCacheStore, 'action_controller/session/mem_cache_store' - end -end - autoload :Mime, 'action_controller/mime_type' autoload :HTML, 'action_controller/vendor/html-scanner' diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 13f2e9072e..0b32da55d5 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -164,8 +164,8 @@ module ActionController #:nodoc: # # Other options for session storage are: # - # * ActiveRecordStore - Sessions are stored in your database, which works better than PStore with multiple app servers and, - # unlike CookieStore, hides your session contents from the user. To use ActiveRecordStore, set + # * ActiveRecord::SessionStore - Sessions are stored in your database, which works better than PStore with multiple app servers and, + # unlike CookieStore, hides your session contents from the user. To use ActiveRecord::SessionStore, set # # config.action_controller.session_store = :active_record_store # @@ -1216,7 +1216,6 @@ module ActionController #:nodoc: def log_processing if logger && logger.info? log_processing_for_request_id - log_processing_for_session_id log_processing_for_parameters end end @@ -1229,13 +1228,6 @@ module ActionController #:nodoc: logger.info(request_id) end - def log_processing_for_session_id - if @_session && @_session.respond_to?(:session_id) && @_session.respond_to?(:dbman) && - !@_session.dbman.is_a?(CGI::Session::CookieStore) - logger.info " Session ID: #{@_session.session_id}" - end - end - def log_processing_for_parameters parameters = respond_to?(:filter_parameters) ? filter_parameters(params) : params.dup parameters = parameters.except!(:controller, :action, :format, :_method) diff --git a/actionpack/lib/action_controller/cgi_ext.rb b/actionpack/lib/action_controller/cgi_ext.rb index f3b8c08d8f..406b6f06d6 100644 --- a/actionpack/lib/action_controller/cgi_ext.rb +++ b/actionpack/lib/action_controller/cgi_ext.rb @@ -1,7 +1,6 @@ require 'action_controller/cgi_ext/stdinput' require 'action_controller/cgi_ext/query_extension' require 'action_controller/cgi_ext/cookie' -require 'action_controller/cgi_ext/session' class CGI #:nodoc: include ActionController::CgiExt::Stdinput diff --git a/actionpack/lib/action_controller/cgi_ext/session.rb b/actionpack/lib/action_controller/cgi_ext/session.rb deleted file mode 100644 index d3f85e3705..0000000000 --- a/actionpack/lib/action_controller/cgi_ext/session.rb +++ /dev/null @@ -1,53 +0,0 @@ -require 'digest/md5' -require 'cgi/session' -require 'cgi/session/pstore' - -class CGI #:nodoc: - # * Expose the CGI instance to session stores. - # * Don't require 'digest/md5' whenever a new session id is generated. - class Session #:nodoc: - def self.generate_unique_id(constant = nil) - ActiveSupport::SecureRandom.hex(16) - end - - # Make the CGI instance available to session stores. - attr_reader :cgi - attr_reader :dbman - alias_method :initialize_without_cgi_reader, :initialize - def initialize(cgi, options = {}) - @cgi = cgi - initialize_without_cgi_reader(cgi, options) - end - - private - # Create a new session id. - def create_new_id - @new_session = true - self.class.generate_unique_id - end - - # * Don't require 'digest/md5' whenever a new session is started. - class PStore #:nodoc: - def initialize(session, option={}) - dir = option['tmpdir'] || Dir::tmpdir - prefix = option['prefix'] || '' - id = session.session_id - md5 = Digest::MD5.hexdigest(id)[0,16] - path = dir+"/"+prefix+md5 - path.untaint - if File::exist?(path) - @hash = nil - else - unless session.new_session - raise CGI::Session::NoSession, "uninitialized session" - end - @hash = {} - end - @p = ::PStore.new(path) - @p.transaction do |p| - File.chmod(0600, p.path) - end - end - end - end -end diff --git a/actionpack/lib/action_controller/cgi_process.rb b/actionpack/lib/action_controller/cgi_process.rb index 5d6988e1b1..7e5e95e135 100644 --- a/actionpack/lib/action_controller/cgi_process.rb +++ b/actionpack/lib/action_controller/cgi_process.rb @@ -61,7 +61,7 @@ module ActionController #:nodoc: class CgiRequest #:nodoc: DEFAULT_SESSION_OPTIONS = { - :database_manager => CGI::Session::CookieStore, + :database_manager => nil, :prefix => "ruby_sess.", :session_path => "/", :session_key => "_session_id", diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb index 203f6b1683..c9a9264b6d 100644 --- a/actionpack/lib/action_controller/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatcher.rb @@ -45,8 +45,10 @@ module ActionController end cattr_accessor :middleware - self.middleware = MiddlewareStack.new - self.middleware.use "ActionController::Failsafe" + self.middleware = MiddlewareStack.new do |middleware| + middleware.use "ActionController::Failsafe" + middleware.use "ActionController::SessionManagement::Middleware" + end include ActiveSupport::Callbacks define_callbacks :prepare_dispatch, :before_dispatch, :after_dispatch @@ -89,7 +91,7 @@ module ActionController def _call(env) @request = RackRequest.new(env) - @response = RackResponse.new(@request) + @response = RackResponse.new dispatch end diff --git a/actionpack/lib/action_controller/integration.rb b/actionpack/lib/action_controller/integration.rb index 212a293da0..1b0543033b 100644 --- a/actionpack/lib/action_controller/integration.rb +++ b/actionpack/lib/action_controller/integration.rb @@ -489,8 +489,8 @@ EOF # By default, a single session is automatically created for you, but you # can use this method to open multiple sessions that ought to be tested # simultaneously. - def open_session - application = ActionController::Dispatcher.new + def open_session(application = nil) + application ||= ActionController::Dispatcher.new session = Integration::Session.new(application) # delegate the fixture accessors back to the test instance diff --git a/actionpack/lib/action_controller/middleware_stack.rb b/actionpack/lib/action_controller/middleware_stack.rb index 1864bed23a..a6597a6fec 100644 --- a/actionpack/lib/action_controller/middleware_stack.rb +++ b/actionpack/lib/action_controller/middleware_stack.rb @@ -4,7 +4,12 @@ module ActionController attr_reader :klass, :args, :block def initialize(klass, *args, &block) - @klass = klass.is_a?(Class) ? klass : klass.to_s.constantize + if klass.is_a?(Class) + @klass = klass + else + @klass = klass.to_s.constantize + end + @args = args @block = block end @@ -21,18 +26,28 @@ module ActionController end def inspect - str = @klass.to_s - @args.each { |arg| str += ", #{arg.inspect}" } + str = klass.to_s + args.each { |arg| str += ", #{arg.inspect}" } str end def build(app) - klass.new(app, *args, &block) + if block + klass.new(app, *args, &block) + else + klass.new(app, *args) + end end end + def initialize(*args, &block) + super(*args) + block.call(self) if block_given? + end + def use(*args, &block) - push(Middleware.new(*args, &block)) + middleware = Middleware.new(*args, &block) + push(middleware) end def build(app) diff --git a/actionpack/lib/action_controller/rack_process.rb b/actionpack/lib/action_controller/rack_process.rb index 568f893c6c..e783839f34 100644 --- a/actionpack/lib/action_controller/rack_process.rb +++ b/actionpack/lib/action_controller/rack_process.rb @@ -3,24 +3,12 @@ require 'action_controller/cgi_ext' module ActionController #:nodoc: class RackRequest < AbstractRequest #:nodoc: attr_accessor :session_options - attr_reader :cgi class SessionFixationAttempt < StandardError #:nodoc: end - DEFAULT_SESSION_OPTIONS = { - :database_manager => CGI::Session::CookieStore, # store data in cookie - :prefix => "ruby_sess.", # prefix session file names - :session_path => "/", # available to all paths in app - :session_key => "_session_id", - :cookie_only => true, - :session_http_only=> true - } - - def initialize(env, session_options = DEFAULT_SESSION_OPTIONS) - @session_options = session_options + def initialize(env) @env = env - @cgi = CGIWrapper.new(self) super() end @@ -66,87 +54,25 @@ module ActionController #:nodoc: @env['SERVER_SOFTWARE'].split("/").first end + def session_options + @env['rack.session.options'] ||= {} + end + + def session_options=(options) + @env['rack.session.options'] = options + end + def session - unless defined?(@session) - if @session_options == false - @session = Hash.new - else - stale_session_check! do - if cookie_only? && query_parameters[session_options_with_string_keys['session_key']] - raise SessionFixationAttempt - end - case value = session_options_with_string_keys['new_session'] - when true - @session = new_session - when false - begin - @session = CGI::Session.new(@cgi, session_options_with_string_keys) - # CGI::Session raises ArgumentError if 'new_session' == false - # and no session cookie or query param is present. - rescue ArgumentError - @session = Hash.new - end - when nil - @session = CGI::Session.new(@cgi, session_options_with_string_keys) - else - raise ArgumentError, "Invalid new_session option: #{value}" - end - @session['__valid_session'] - end - end - end - @session + @env['rack.session'] ||= {} end def reset_session - @session.delete if defined?(@session) && @session.is_a?(CGI::Session) - @session = new_session + @env['rack.session'] = {} end - - private - # Delete an old session if it exists then create a new one. - def new_session - if @session_options == false - Hash.new - else - CGI::Session.new(@cgi, session_options_with_string_keys.merge("new_session" => false)).delete rescue nil - CGI::Session.new(@cgi, session_options_with_string_keys.merge("new_session" => true)) - end - end - - def cookie_only? - session_options_with_string_keys['cookie_only'] - end - - def stale_session_check! - yield - rescue ArgumentError => argument_error - if argument_error.message =~ %r{undefined class/module ([\w:]*\w)} - begin - # Note that the regexp does not allow $1 to end with a ':' - $1.constantize - rescue LoadError, NameError => const_error - raise ActionController::SessionRestoreError, <<-end_msg -Session contains objects whose class definition isn\'t available. -Remember to require the classes for all objects kept in the session. -(Original exception: #{const_error.message} [#{const_error.class}]) -end_msg - end - - retry - else - raise - end - end - - def session_options_with_string_keys - @session_options_with_string_keys ||= DEFAULT_SESSION_OPTIONS.merge(@session_options).stringify_keys - end end class RackResponse < AbstractResponse #:nodoc: - def initialize(request) - @cgi = request.cgi + def initialize @writer = lambda { |x| @body << x } @block = nil super() @@ -247,49 +173,8 @@ end_msg else cookies << cookie.to_s 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 - - class CGIWrapper < ::CGI - attr_reader :output_cookies - - def initialize(request, *args) - @request = request - @args = *args - @input = request.body - - super *args - end - - def params - @params ||= @request.params - end - - def cookies - @request.cookies - end - - def query_string - @request.query_string - end - - # Used to wrap the normal args variable used inside CGI. - def args - @args - end - - # Used to wrap the normal env_table variable used inside CGI. - def env_table - @request.env - end - - # Used to wrap the normal stdinput variable used inside CGI. - def stdinput - @input - end - end end diff --git a/actionpack/lib/action_controller/session/abstract_store.rb b/actionpack/lib/action_controller/session/abstract_store.rb new file mode 100644 index 0000000000..b23bf062c4 --- /dev/null +++ b/actionpack/lib/action_controller/session/abstract_store.rb @@ -0,0 +1,129 @@ +require 'rack/utils' + +module ActionController + module Session + class AbstractStore + ENV_SESSION_KEY = 'rack.session'.freeze + ENV_SESSION_OPTIONS_KEY = 'rack.session.options'.freeze + + HTTP_COOKIE = 'HTTP_COOKIE'.freeze + SET_COOKIE = 'Set-Cookie'.freeze + + class SessionHash < Hash + def initialize(by, env) + @by = by + @env = env + @loaded = false + end + + def id + load! unless @loaded + @id + end + + def [](key) + load! unless @loaded + super + end + + def []=(key, value) + load! unless @loaded + super + end + + def to_hash + {}.replace(self) + end + + private + def load! + @id, session = @by.send(:load_session, @env) + replace(session) + @loaded = true + end + end + + DEFAULT_OPTIONS = { + :key => 'rack.session', + :path => '/', + :domain => nil, + :expire_after => nil, + :secure => false, + :httponly => true, + :cookie_only => true + } + + def initialize(app, options = {}) + @app = app + @default_options = DEFAULT_OPTIONS.merge(options) + @key = @default_options[:key] + @cookie_only = @default_options[:cookie_only] + end + + def call(env) + session = SessionHash.new(self, env) + original_session = session.dup + + env[ENV_SESSION_KEY] = session + env[ENV_SESSION_OPTIONS_KEY] = @default_options.dup + + response = @app.call(env) + + session = env[ENV_SESSION_KEY] + unless session == original_session + options = env[ENV_SESSION_OPTIONS_KEY] + sid = session.id + + unless set_session(env, sid, session.to_hash) + return response + end + + cookie = Rack::Utils.escape(@key) + '=' + Rack::Utils.escape(sid) + cookie << "; domain=#{options[:domain]}" if options[:domain] + cookie << "; path=#{options[:path]}" if options[:path] + if options[:expire_after] + expiry = Time.now + options[:expire_after] + cookie << "; expires=#{expiry.httpdate}" + end + cookie << "; Secure" if options[:secure] + cookie << "; HttpOnly" if options[:httponly] + + headers = response[1] + case a = headers[SET_COOKIE] + when Array + a << cookie + when String + headers[SET_COOKIE] = [a, cookie] + when nil + headers[SET_COOKIE] = cookie + end + end + + response + end + + private + def generate_sid + ActiveSupport::SecureRandom.hex(16) + end + + def load_session(env) + request = Rack::Request.new(env) + sid = request.cookies[@key] + unless @cookie_only + sid ||= request.params[@key] + end + sid, session = get_session(env, sid) + [sid, session] + end + + def get_session(env, sid) + raise '#get_session needs to be implemented.' + end + + def set_session(env, sid, session_data) + raise '#set_session needs to be implemented.' + end + end + end +end diff --git a/actionpack/lib/action_controller/session/active_record_store.rb b/actionpack/lib/action_controller/session/active_record_store.rb deleted file mode 100644 index fadf2a6b32..0000000000 --- a/actionpack/lib/action_controller/session/active_record_store.rb +++ /dev/null @@ -1,350 +0,0 @@ -require 'cgi' -require 'cgi/session' -require 'digest/md5' - -class CGI - class Session - attr_reader :data - - # Return this session's underlying Session instance. Useful for the DB-backed session stores. - def model - @dbman.model if @dbman - end - - - # A session store backed by an Active Record class. A default class is - # provided, but any object duck-typing to an Active Record Session class - # with text +session_id+ and +data+ attributes is sufficient. - # - # The default assumes a +sessions+ tables with columns: - # +id+ (numeric primary key), - # +session_id+ (text, or longtext if your session data exceeds 65K), and - # +data+ (text or longtext; careful if your session data exceeds 65KB). - # The +session_id+ column should always be indexed for speedy lookups. - # Session data is marshaled to the +data+ column in Base64 format. - # If the data you write is larger than the column's size limit, - # ActionController::SessionOverflowError will be raised. - # - # You may configure the table name, primary key, and data column. - # For example, at the end of config/environment.rb: - # CGI::Session::ActiveRecordStore::Session.table_name = 'legacy_session_table' - # CGI::Session::ActiveRecordStore::Session.primary_key = 'session_id' - # CGI::Session::ActiveRecordStore::Session.data_column_name = 'legacy_session_data' - # Note that setting the primary key to the +session_id+ frees you from - # having a separate +id+ column if you don't want it. However, you must - # set session.model.id = session.session_id by hand! A before filter - # on ApplicationController is a good place. - # - # Since the default class is a simple Active Record, you get timestamps - # for free if you add +created_at+ and +updated_at+ datetime columns to - # the +sessions+ table, making periodic session expiration a snap. - # - # You may provide your own session class implementation, whether a - # feature-packed Active Record or a bare-metal high-performance SQL - # store, by setting - # CGI::Session::ActiveRecordStore.session_class = MySessionClass - # You must implement these methods: - # self.find_by_session_id(session_id) - # initialize(hash_of_session_id_and_data) - # attr_reader :session_id - # attr_accessor :data - # save - # destroy - # - # The example SqlBypass class is a generic SQL session store. You may - # use it as a basis for high-performance database-specific stores. - class ActiveRecordStore - # The default Active Record class. - class Session < ActiveRecord::Base - ## - # :singleton-method: - # Customizable data column name. Defaults to 'data'. - cattr_accessor :data_column_name - self.data_column_name = 'data' - - before_save :marshal_data! - before_save :raise_on_session_data_overflow! - - class << self - # Don't try to reload ARStore::Session in dev mode. - def reloadable? #:nodoc: - false - end - - def data_column_size_limit - @data_column_size_limit ||= columns_hash[@@data_column_name].limit - end - - # Hook to set up sessid compatibility. - def find_by_session_id(session_id) - setup_sessid_compatibility! - find_by_session_id(session_id) - end - - def marshal(data) ActiveSupport::Base64.encode64(Marshal.dump(data)) if data end - def unmarshal(data) Marshal.load(ActiveSupport::Base64.decode64(data)) if data end - - def create_table! - connection.execute <<-end_sql - CREATE TABLE #{table_name} ( - id INTEGER PRIMARY KEY, - #{connection.quote_column_name('session_id')} TEXT UNIQUE, - #{connection.quote_column_name(@@data_column_name)} TEXT(255) - ) - end_sql - end - - def drop_table! - connection.execute "DROP TABLE #{table_name}" - end - - private - # Compatibility with tables using sessid instead of session_id. - def setup_sessid_compatibility! - # Reset column info since it may be stale. - reset_column_information - if columns_hash['sessid'] - def self.find_by_session_id(*args) - find_by_sessid(*args) - end - - define_method(:session_id) { sessid } - define_method(:session_id=) { |session_id| self.sessid = session_id } - else - def self.find_by_session_id(session_id) - find :first, :conditions => ["session_id #{attribute_condition(session_id)}", session_id] - end - end - end - end - - # Lazy-unmarshal session state. - def data - @data ||= self.class.unmarshal(read_attribute(@@data_column_name)) || {} - end - - attr_writer :data - - # Has the session been loaded yet? - def loaded? - !! @data - end - - private - - def marshal_data! - return false if !loaded? - write_attribute(@@data_column_name, self.class.marshal(self.data)) - end - - # Ensures that the data about to be stored in the database is not - # larger than the data storage column. Raises - # ActionController::SessionOverflowError. - def raise_on_session_data_overflow! - return false if !loaded? - limit = self.class.data_column_size_limit - if loaded? and limit and read_attribute(@@data_column_name).size > limit - raise ActionController::SessionOverflowError - end - end - end - - # A barebones session store which duck-types with the default session - # store but bypasses Active Record and issues SQL directly. This is - # an example session model class meant as a basis for your own classes. - # - # The database connection, table name, and session id and data columns - # are configurable class attributes. Marshaling and unmarshaling - # are implemented as class methods that you may override. By default, - # marshaling data is - # - # ActiveSupport::Base64.encode64(Marshal.dump(data)) - # - # and unmarshaling data is - # - # Marshal.load(ActiveSupport::Base64.decode64(data)) - # - # This marshaling behavior is intended to store the widest range of - # binary session data in a +text+ column. For higher performance, - # store in a +blob+ column instead and forgo the Base64 encoding. - class SqlBypass - ## - # :singleton-method: - # Use the ActiveRecord::Base.connection by default. - cattr_accessor :connection - - ## - # :singleton-method: - # The table name defaults to 'sessions'. - cattr_accessor :table_name - @@table_name = 'sessions' - - ## - # :singleton-method: - # The session id field defaults to 'session_id'. - cattr_accessor :session_id_column - @@session_id_column = 'session_id' - - ## - # :singleton-method: - # The data field defaults to 'data'. - cattr_accessor :data_column - @@data_column = 'data' - - class << self - - def connection - @@connection ||= ActiveRecord::Base.connection - end - - # Look up a session by id and unmarshal its data if found. - def find_by_session_id(session_id) - if record = @@connection.select_one("SELECT * FROM #{@@table_name} WHERE #{@@session_id_column}=#{@@connection.quote(session_id)}") - new(:session_id => session_id, :marshaled_data => record['data']) - end - end - - def marshal(data) ActiveSupport::Base64.encode64(Marshal.dump(data)) if data end - def unmarshal(data) Marshal.load(ActiveSupport::Base64.decode64(data)) if data end - - def create_table! - @@connection.execute <<-end_sql - CREATE TABLE #{table_name} ( - id INTEGER PRIMARY KEY, - #{@@connection.quote_column_name(session_id_column)} TEXT UNIQUE, - #{@@connection.quote_column_name(data_column)} TEXT - ) - end_sql - end - - def drop_table! - @@connection.execute "DROP TABLE #{table_name}" - end - end - - attr_reader :session_id - attr_writer :data - - # Look for normal and marshaled data, self.find_by_session_id's way of - # telling us to postpone unmarshaling until the data is requested. - # We need to handle a normal data attribute in case of a new record. - def initialize(attributes) - @session_id, @data, @marshaled_data = attributes[:session_id], attributes[:data], attributes[:marshaled_data] - @new_record = @marshaled_data.nil? - end - - def new_record? - @new_record - end - - # Lazy-unmarshal session state. - def data - unless @data - if @marshaled_data - @data, @marshaled_data = self.class.unmarshal(@marshaled_data) || {}, nil - else - @data = {} - end - end - @data - end - - def loaded? - !! @data - end - - def save - return false if !loaded? - marshaled_data = self.class.marshal(data) - - if @new_record - @new_record = false - @@connection.update <<-end_sql, 'Create session' - INSERT INTO #{@@table_name} ( - #{@@connection.quote_column_name(@@session_id_column)}, - #{@@connection.quote_column_name(@@data_column)} ) - VALUES ( - #{@@connection.quote(session_id)}, - #{@@connection.quote(marshaled_data)} ) - end_sql - else - @@connection.update <<-end_sql, 'Update session' - UPDATE #{@@table_name} - SET #{@@connection.quote_column_name(@@data_column)}=#{@@connection.quote(marshaled_data)} - WHERE #{@@connection.quote_column_name(@@session_id_column)}=#{@@connection.quote(session_id)} - end_sql - end - end - - def destroy - unless @new_record - @@connection.delete <<-end_sql, 'Destroy session' - DELETE FROM #{@@table_name} - WHERE #{@@connection.quote_column_name(@@session_id_column)}=#{@@connection.quote(session_id)} - end_sql - end - end - end - - - # The class used for session storage. Defaults to - # CGI::Session::ActiveRecordStore::Session. - cattr_accessor :session_class - self.session_class = Session - - # Find or instantiate a session given a CGI::Session. - def initialize(session, option = nil) - session_id = session.session_id - unless @session = ActiveRecord::Base.silence { @@session_class.find_by_session_id(session_id) } - unless session.new_session - raise CGI::Session::NoSession, 'uninitialized session' - end - @session = @@session_class.new(:session_id => session_id, :data => {}) - # session saving can be lazy again, because of improved component implementation - # therefore next line gets commented out: - # @session.save - end - end - - # Access the underlying session model. - def model - @session - end - - # Restore session state. The session model handles unmarshaling. - def restore - if @session - @session.data - end - end - - # Save session store. - def update - if @session - ActiveRecord::Base.silence { @session.save } - end - end - - # Save and close the session store. - def close - if @session - update - @session = nil - end - end - - # Delete and close the session store. - def delete - if @session - ActiveRecord::Base.silence { @session.destroy } - @session = nil - end - end - - protected - def logger - ActionController::Base.logger rescue nil - end - end - end -end diff --git a/actionpack/lib/action_controller/session/cookie_store.rb b/actionpack/lib/action_controller/session/cookie_store.rb index ea0ea4f841..f13c9290c0 100644 --- a/actionpack/lib/action_controller/session/cookie_store.rb +++ b/actionpack/lib/action_controller/session/cookie_store.rb @@ -1,163 +1,219 @@ -require 'cgi' -require 'cgi/session' +module ActionController + module Session + # This cookie-based session store is the Rails default. Sessions typically + # contain at most a user_id and flash message; both fit within the 4K cookie + # size limit. Cookie-based sessions are dramatically faster than the + # alternatives. + # + # If you have more than 4K of session data or don't want your data to be + # visible to the user, pick another session store. + # + # CookieOverflow is raised if you attempt to store more than 4K of data. + # + # A message digest is included with the cookie to ensure data integrity: + # a user cannot alter his +user_id+ without knowing the secret key + # included in the hash. New apps are generated with a pregenerated secret + # in config/environment.rb. Set your own for old apps you're upgrading. + # + # Session options: + # + # * :secret: An application-wide key string or block returning a + # string called per generated digest. The block is called with the + # CGI::Session instance as an argument. It's important that the secret + # is not vulnerable to a dictionary attack. Therefore, you should choose + # a secret consisting of random numbers and letters and more than 30 + # characters. Examples: + # + # :secret => '449fe2e7daee471bffae2fd8dc02313d' + # :secret => Proc.new { User.current_user.secret_key } + # + # * :digest: The message digest algorithm used to verify session + # integrity defaults to 'SHA1' but may be any digest provided by OpenSSL, + # such as 'MD5', 'RIPEMD160', 'SHA256', etc. + # + # To generate a secret key for an existing application, run + # "rake secret" and set the key in config/environment.rb. + # + # Note that changing digest or secret invalidates all existing sessions! + class CookieStore + # Cookies can typically store 4096 bytes. + MAX = 4096 + SECRET_MIN_LENGTH = 30 # characters -# This cookie-based session store is the Rails default. Sessions typically -# contain at most a user_id and flash message; both fit within the 4K cookie -# size limit. Cookie-based sessions are dramatically faster than the -# alternatives. -# -# If you have more than 4K of session data or don't want your data to be -# visible to the user, pick another session store. -# -# CookieOverflow is raised if you attempt to store more than 4K of data. -# TamperedWithCookie is raised if the data integrity check fails. -# -# A message digest is included with the cookie to ensure data integrity: -# a user cannot alter his +user_id+ without knowing the secret key included in -# the hash. New apps are generated with a pregenerated secret in -# config/environment.rb. Set your own for old apps you're upgrading. -# -# Session options: -# -# * :secret: An application-wide key string or block returning a string -# called per generated digest. The block is called with the CGI::Session -# instance as an argument. It's important that the secret is not vulnerable to -# a dictionary attack. Therefore, you should choose a secret consisting of -# random numbers and letters and more than 30 characters. Examples: -# -# :secret => '449fe2e7daee471bffae2fd8dc02313d' -# :secret => Proc.new { User.current_user.secret_key } -# -# * :digest: The message digest algorithm used to verify session -# integrity defaults to 'SHA1' but may be any digest provided by OpenSSL, -# such as 'MD5', 'RIPEMD160', 'SHA256', etc. -# -# To generate a secret key for an existing application, run -# "rake secret" and set the key in config/environment.rb. -# -# Note that changing digest or secret invalidates all existing sessions! -class CGI::Session::CookieStore - # Cookies can typically store 4096 bytes. - MAX = 4096 - SECRET_MIN_LENGTH = 30 # characters + DEFAULT_OPTIONS = { + :domain => nil, + :path => "/", + :expire_after => nil + }.freeze - # Raised when storing more than 4K of session data. - class CookieOverflow < StandardError; end + ENV_SESSION_KEY = "rack.session".freeze + ENV_SESSION_OPTIONS_KEY = "rack.session.options".freeze + HTTP_SET_COOKIE = "Set-Cookie".freeze - # Raised when the cookie fails its integrity check. - class TamperedWithCookie < StandardError; end + # Raised when storing more than 4K of session data. + class CookieOverflow < StandardError; end - # Called from CGI::Session only. - def initialize(session, options = {}) - # The session_key option is required. - if options['session_key'].blank? - raise ArgumentError, 'A session_key is required to write a cookie containing the session data. Use config.action_controller.session = { :session_key => "_myapp_session", :secret => "some secret phrase" } in config/environment.rb' - end + def initialize(app, options = {}) + options = options.dup - # The secret option is required. - ensure_secret_secure(options['secret']) + @app = app - # Keep the session and its secret on hand so we can read and write cookies. - @session, @secret = session, options['secret'] + # The session_key option is required. + ensure_session_key(options[:key]) + @key = options.delete(:key).freeze - # Message digest defaults to SHA1. - @digest = options['digest'] || 'SHA1' + # The secret option is required. + ensure_secret_secure(options[:secret]) + @secret = options.delete(:secret).freeze - # Default cookie options derived from session settings. - @cookie_options = { - 'name' => options['session_key'], - 'path' => options['session_path'], - 'domain' => options['session_domain'], - 'expires' => options['session_expires'], - 'secure' => options['session_secure'], - 'http_only' => options['session_http_only'] - } + @digest = options.delete(:digest) || 'SHA1' + @verifier = verifier_for(@secret, @digest) - # Set no_hidden and no_cookies since the session id is unused and we - # set our own data cookie. - options['no_hidden'] = true - options['no_cookies'] = true - end + @default_options = DEFAULT_OPTIONS.merge(options).freeze - # To prevent users from using something insecure like "Password" we make sure that the - # secret they've provided is at least 30 characters in length. - def ensure_secret_secure(secret) - # There's no way we can do this check if they've provided a proc for the - # secret. - return true if secret.is_a?(Proc) - - if secret.blank? - raise ArgumentError, %Q{A secret is required to generate an integrity hash for cookie session data. Use config.action_controller.session = { :session_key => "_myapp_session", :secret => "some secret phrase of at least #{SECRET_MIN_LENGTH} characters" } in config/environment.rb} - end - - if secret.length < SECRET_MIN_LENGTH - raise ArgumentError, %Q{Secret should be something secure, like "#{CGI::Session.generate_unique_id}". The value you provided, "#{secret}", is shorter than the minimum length of #{SECRET_MIN_LENGTH} characters} - end - end - - # Restore session data from the cookie. - def restore - @original = read_cookie - @data = unmarshal(@original) || {} - end - - # Wait until close to write the session data cookie. - def update; end - - # Write the session data cookie if it was loaded and has changed. - def close - if defined?(@data) && !@data.blank? - updated = marshal(@data) - raise CookieOverflow if updated.size > MAX - write_cookie('value' => updated) unless updated == @original - end - end - - # Delete the session data by setting an expired cookie with no data. - def delete - @data = nil - clear_old_cookie_value - write_cookie('value' => nil, 'expires' => 1.year.ago) - end - - private - # Marshal a session hash into safe cookie data. Include an integrity hash. - def marshal(session) - verifier.generate(session) - end - - # Unmarshal cookie data to a hash and verify its integrity. - def unmarshal(cookie) - if cookie - verifier.verify(cookie) + freeze end - rescue ActiveSupport::MessageVerifier::InvalidSignature - delete - raise TamperedWithCookie - end - # Read the session data cookie. - def read_cookie - @session.cgi.cookies[@cookie_options['name']].first - end + class SessionHash < Hash + def initialize(middleware, env) + @middleware = middleware + @env = env + @loaded = false + end - # CGI likes to make you hack. - def write_cookie(options) - cookie = CGI::Cookie.new(@cookie_options.merge(options)) - @session.cgi.send :instance_variable_set, '@output_cookies', [cookie] - end + def [](key) + load! unless @loaded + super + end - # Clear cookie value so subsequent new_session doesn't reload old data. - def clear_old_cookie_value - @session.cgi.cookies[@cookie_options['name']].clear - end - - def verifier - if @secret.respond_to?(:call) - key = @secret.call - else - key = @secret + def []=(key, value) + load! unless @loaded + super + end + + def to_hash + {}.replace(self) + end + + private + def load! + replace(@middleware.send(:load_session, @env)) + @loaded = true + end end - ActiveSupport::MessageVerifier.new(key, @digest) + + def call(env) + session_data = SessionHash.new(self, env) + original_value = session_data.dup + + env[ENV_SESSION_KEY] = session_data + env[ENV_SESSION_OPTIONS_KEY] = @default_options.dup + + status, headers, body = @app.call(env) + + unless env[ENV_SESSION_KEY] == original_value + session_data = marshal(env[ENV_SESSION_KEY].to_hash) + + raise CookieOverflow if session_data.size > MAX + + options = env[ENV_SESSION_OPTIONS_KEY] + cookie = Hash.new + cookie[:value] = session_data + unless options[:expire_after].nil? + cookie[:expires] = Time.now + options[:expire_after] + end + + cookie = build_cookie(@key, cookie.merge(options)) + case headers[HTTP_SET_COOKIE] + when Array + headers[HTTP_SET_COOKIE] << cookie + when String + headers[HTTP_SET_COOKIE] = [headers[HTTP_SET_COOKIE], cookie] + when nil + headers[HTTP_SET_COOKIE] = cookie + end + end + + [status, headers, body] + end + + private + # Should be in Rack::Utils soon + def build_cookie(key, value) + case value + when Hash + domain = "; domain=" + value[:domain] if value[:domain] + path = "; path=" + value[:path] if value[:path] + # According to RFC 2109, we need dashes here. + # N.B.: cgi.rb uses spaces... + expires = "; expires=" + value[:expires].clone.gmtime. + strftime("%a, %d-%b-%Y %H:%M:%S GMT") if value[:expires] + secure = "; secure" if value[:secure] + httponly = "; httponly" if value[:httponly] + value = value[:value] + end + value = [value] unless Array === value + cookie = Rack::Utils.escape(key) + "=" + + value.map { |v| Rack::Utils.escape(v) }.join("&") + + "#{domain}#{path}#{expires}#{secure}#{httponly}" + end + + def load_session(env) + request = Rack::Request.new(env) + session_data = request.cookies[@key] + unmarshal(session_data) || {} + end + + # Marshal a session hash into safe cookie data. Include an integrity hash. + def marshal(session) + @verifier.generate(session) + end + + # Unmarshal cookie data to a hash and verify its integrity. + def unmarshal(cookie) + @verifier.verify(cookie) if cookie + rescue ActiveSupport::MessageVerifier::InvalidSignature + nil + end + + def ensure_session_key(key) + if key.blank? + raise ArgumentError, 'A session_key is required to write a ' + + 'cookie containing the session data. Use ' + + 'config.action_controller.session = { :session_key => ' + + '"_myapp_session", :secret => "some secret phrase" } in ' + + 'config/environment.rb' + end + end + + # To prevent users from using something insecure like "Password" we make sure that the + # secret they've provided is at least 30 characters in length. + def ensure_secret_secure(secret) + # There's no way we can do this check if they've provided a proc for the + # secret. + return true if secret.is_a?(Proc) + + if secret.blank? + raise ArgumentError, "A secret is required to generate an " + + "integrity hash for cookie session data. Use " + + "config.action_controller.session = { :session_key => " + + "\"_myapp_session\", :secret => \"some secret phrase of at " + + "least #{SECRET_MIN_LENGTH} characters\" } " + + "in config/environment.rb" + end + + if secret.length < SECRET_MIN_LENGTH + raise ArgumentError, "Secret should be something secure, " + + "like \"#{ActiveSupport::SecureRandom.hex(16)}\". The value you " + + "provided, \"#{secret}\", is shorter than the minimum length " + + "of #{SECRET_MIN_LENGTH} characters" + end + end + + def verifier_for(secret, digest) + key = secret.respond_to?(:call) ? secret.call : secret + ActiveSupport::MessageVerifier.new(key, digest) + end end + end end diff --git a/actionpack/lib/action_controller/session/drb_server.rb b/actionpack/lib/action_controller/session/drb_server.rb deleted file mode 100755 index 2caa27f62a..0000000000 --- a/actionpack/lib/action_controller/session/drb_server.rb +++ /dev/null @@ -1,32 +0,0 @@ -#!/usr/bin/env ruby - -# This is a really simple session storage daemon, basically just a hash, -# which is enabled for DRb access. - -require 'drb' - -session_hash = Hash.new -session_hash.instance_eval { @mutex = Mutex.new } - -class < 'rack:session', + :memcache_server => 'localhost:11211' + }.merge(@default_options) + + @pool = options[:cache] || MemCache.new(@default_options[:memcache_server], @default_options) + unless @pool.servers.any? { |s| s.alive? } + raise "#{self} unable to find server during initialization." end - @cache = options['cache'] || MemCache.new('localhost') - @expires = options['expires'] || 0 - @session_key = "session:#{id}" - @session_data = {} - # Add this key to the store if haven't done so yet - unless @cache.get(@session_key) - @cache.add(@session_key, @session_data, @expires) + @mutex = Mutex.new + + super + end + + private + def get_session(env, sid) + sid ||= generate_sid + begin + session = @pool.get(sid) || {} + rescue MemCache::MemCacheError, Errno::ECONNREFUSED + session = {} + end + [sid, session] end - end - - # Restore session state from the session's memcache entry. - # - # Returns the session state as a hash. - def restore - @session_data = @cache[@session_key] || {} - end - - # Save session state to the session's memcache entry. - def update - @cache.set(@session_key, @session_data, @expires) - end - - # Update and close the session's memcache entry. - def close - update - end - - # Delete the session's memcache entry. - def delete - @cache.delete(@session_key) - @session_data = {} - end - - def data - @session_data - end + def set_session(env, sid, session_data) + options = env['rack.session.options'] + expiry = options[:expire_after] || 0 + @pool.set(sid, session_data, expiry) + return true + rescue MemCache::MemCacheError, Errno::ECONNREFUSED + return false + end end end end diff --git a/actionpack/lib/action_controller/session_management.rb b/actionpack/lib/action_controller/session_management.rb index 60a9aec39c..dd5001d328 100644 --- a/actionpack/lib/action_controller/session_management.rb +++ b/actionpack/lib/action_controller/session_management.rb @@ -3,8 +3,29 @@ module ActionController #:nodoc: def self.included(base) base.class_eval do extend ClassMethods - alias_method_chain :process, :session_management_support - alias_method_chain :process_cleanup, :session_management_support + end + end + + class Middleware + DEFAULT_OPTIONS = { + :path => "/", + :key => "_session_id", + :httponly => true, + }.freeze + + def self.new(app) + cgi_options = ActionController::Base.session_options + options = cgi_options.symbolize_keys + options = DEFAULT_OPTIONS.merge(options) + options[:path] = options.delete(:session_path) + options[:key] = options.delete(:session_key) + options[:httponly] = options.delete(:session_http_only) + + if store = ActionController::Base.session_store + store.new(app, options) + else # Sessions disabled + lambda { |env| app.call(env) } + end end end @@ -12,144 +33,45 @@ module ActionController #:nodoc: # Set the session store to be used for keeping the session data between requests. # By default, sessions are stored in browser cookies (:cookie_store), # but you can also specify one of the other included stores (:active_record_store, - # :p_store, :drb_store, :mem_cache_store, or - # :memory_store) or your own custom class. + # :mem_cache_store, or your own custom class. def session_store=(store) - ActionController::CgiRequest::DEFAULT_SESSION_OPTIONS[:database_manager] = - store.is_a?(Symbol) ? CGI::Session.const_get(store == :drb_store ? "DRbStore" : store.to_s.camelize) : store + if store == :active_record_store + self.session_store = ActiveRecord::SessionStore + else + @@session_store = store.is_a?(Symbol) ? + Session.const_get(store.to_s.camelize) : + store + end end # Returns the session store class currently used. def session_store - ActionController::CgiRequest::DEFAULT_SESSION_OPTIONS[:database_manager] + if defined? @@session_store + @@session_store + else + Session::CookieStore + end + end + + def session=(options = {}) + self.session_store = nil if options.delete(:disabled) + session_options.merge!(options) end # Returns the hash used to configure the session. Example use: # # ActionController::Base.session_options[:session_secure] = true # session only available over HTTPS def session_options - ActionController::CgiRequest::DEFAULT_SESSION_OPTIONS + @session_options ||= {} end - - # Specify how sessions ought to be managed for a subset of the actions on - # the controller. Like filters, you can specify :only and - # :except clauses to restrict the subset, otherwise options - # apply to all actions on this controller. - # - # The session options are inheritable, as well, so if you specify them in - # a parent controller, they apply to controllers that extend the parent. - # - # Usage: - # - # # turn off session management for all actions. - # session :off - # - # # turn off session management for all actions _except_ foo and bar. - # session :off, :except => %w(foo bar) - # - # # turn off session management for only the foo and bar actions. - # session :off, :only => %w(foo bar) - # - # # the session will only work over HTTPS, but only for the foo action - # session :only => :foo, :session_secure => true - # - # # the session by default uses HttpOnly sessions for security reasons. - # # this can be switched off. - # session :only => :foo, :session_http_only => false - # - # # the session will only be disabled for 'foo', and only if it is - # # requested as a web service - # session :off, :only => :foo, - # :if => Proc.new { |req| req.parameters[:ws] } - # - # # the session will be disabled for non html/ajax requests - # session :off, - # :if => Proc.new { |req| !(req.format.html? || req.format.js?) } - # - # # turn the session back on, useful when it was turned off in the - # # application controller, and you need it on in another controller - # session :on - # - # All session options described for ActionController::Base.process_cgi - # are valid arguments. + def session(*args) - options = args.extract_options! - - options[:disabled] = false if args.delete(:on) - options[:disabled] = true if !args.empty? - options[:only] = [*options[:only]].map { |o| o.to_s } if options[:only] - options[:except] = [*options[:except]].map { |o| o.to_s } if options[:except] - if options[:only] && options[:except] - raise ArgumentError, "only one of either :only or :except are allowed" - end - - write_inheritable_array(:session_options, [options]) - end - - # So we can declare session options in the Rails initializer. - alias_method :session=, :session - - def cached_session_options #:nodoc: - @session_options ||= read_inheritable_attribute(:session_options) || [] - end - - def session_options_for(request, action) #:nodoc: - if (session_options = cached_session_options).empty? - {} - else - options = {} - - action = action.to_s - session_options.each do |opts| - next if opts[:if] && !opts[:if].call(request) - if opts[:only] && opts[:only].include?(action) - options.merge!(opts) - elsif opts[:except] && !opts[:except].include?(action) - options.merge!(opts) - elsif !opts[:only] && !opts[:except] - options.merge!(opts) - end - end - - if options.empty? then options - else - options.delete :only - options.delete :except - options.delete :if - options[:disabled] ? false : options - end - end + ActiveSupport::Deprecation.warn( + "Disabling sessions for a single controller has been deprecated. " + + "Sessions are now lazy loaded. So if you don't access them, " + + "consider them off. You can still modify the session cookie " + + "options with request.session_options.", caller) end end - - def process_with_session_management_support(request, response, method = :perform_action, *arguments) #:nodoc: - set_session_options(request) - process_without_session_management_support(request, response, method, *arguments) - end - - private - def set_session_options(request) - request.session_options = self.class.session_options_for(request, request.parameters["action"] || "index") - end - - def process_cleanup_with_session_management_support - clear_persistent_model_associations - process_cleanup_without_session_management_support - end - - # Clear cached associations in session data so they don't overflow - # the database field. Only applies to ActiveRecordStore since there - # is not a standard way to iterate over session data. - def clear_persistent_model_associations #:doc: - if defined?(@_session) && @_session.respond_to?(:data) - session_data = @_session.data - - if session_data && session_data.respond_to?(:each_value) - session_data.each_value do |obj| - obj.clear_association_cache if obj.respond_to?(:clear_association_cache) - end - end - end - end end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 51697fda2f..2ba056e60f 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -30,6 +30,8 @@ ActiveSupport::Deprecation.debug = true ActionController::Base.logger = nil ActionController::Routing::Routes.reload rescue nil +ActionController::Base.session_store = nil + FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures') ActionController::Base.view_paths = FIXTURE_LOAD_PATH ActionController::Base.view_paths.load diff --git a/actionpack/test/activerecord/active_record_store_test.rb b/actionpack/test/activerecord/active_record_store_test.rb index 677d434f9c..6a75e6050d 100644 --- a/actionpack/test/activerecord/active_record_store_test.rb +++ b/actionpack/test/activerecord/active_record_store_test.rb @@ -1,140 +1,128 @@ -# These tests exercise CGI::Session::ActiveRecordStore, so you're going to -# need AR in a sibling directory to AP and have SQLite installed. require 'active_record_unit' -module CommonActiveRecordStoreTests - def test_basics - s = session_class.new(:session_id => '1234', :data => { 'foo' => 'bar' }) - assert_equal 'bar', s.data['foo'] - assert s.save - assert_equal 'bar', s.data['foo'] +class ActiveRecordStoreTest < ActionController::IntegrationTest + DispatcherApp = ActionController::Dispatcher.new + SessionApp = ActiveRecord::SessionStore.new(DispatcherApp, + :key => '_session_id') + SessionAppWithFixation = ActiveRecord::SessionStore.new(DispatcherApp, + :key => '_session_id', :cookie_only => false) - assert_not_nil t = session_class.find_by_session_id('1234') - assert_not_nil t.data - assert_equal 'bar', t.data['foo'] + class TestController < ActionController::Base + def no_session_access + head :ok + end + + def set_session_value + session[:foo] = params[:foo] || "bar" + head :ok + end + + def get_session_value + render :text => "foo: #{session[:foo].inspect}" + end + + def rescue_action(e) raise end end - def test_reload_same_session - @new_session.update - reloaded = CGI::Session.new(CGI.new, 'session_id' => @new_session.session_id, 'database_manager' => CGI::Session::ActiveRecordStore) - assert_equal 'bar', reloaded['foo'] + def setup + ActiveRecord::SessionStore.session_class.create_table! + @integration_session = open_session(SessionApp) end - def test_tolerates_close_close - assert_nothing_raised do - @new_session.close - @new_session.close + def teardown + ActiveRecord::SessionStore.session_class.drop_table! + end + + def test_setting_and_getting_session_value + with_test_route_set do + get '/set_session_value' + assert_response :success + assert cookies['_session_id'] + + get '/get_session_value' + assert_response :success + assert_equal 'foo: "bar"', response.body + + get '/set_session_value', :foo => "baz" + assert_response :success + assert cookies['_session_id'] + + get '/get_session_value' + assert_response :success + assert_equal 'foo: "baz"', response.body end end -end -class ActiveRecordStoreTest < ActiveRecordTestCase - include CommonActiveRecordStoreTests - - def session_class - CGI::Session::ActiveRecordStore::Session - end - - def session_id_column - "session_id" - end - - def setup - session_class.create_table! - - ENV['REQUEST_METHOD'] = 'GET' - ENV['REQUEST_URI'] = '/' - CGI::Session::ActiveRecordStore.session_class = session_class - - @cgi = CGI.new - @new_session = CGI::Session.new(@cgi, 'database_manager' => CGI::Session::ActiveRecordStore, 'new_session' => true) - @new_session['foo'] = 'bar' - end - -# this test only applies for eager session saving -# def test_another_instance -# @another = CGI::Session.new(@cgi, 'session_id' => @new_session.session_id, 'database_manager' => CGI::Session::ActiveRecordStore) -# assert_equal @new_session.session_id, @another.session_id -# end - - def test_model_attribute - assert_kind_of CGI::Session::ActiveRecordStore::Session, @new_session.model - assert_equal({ 'foo' => 'bar' }, @new_session.model.data) - end - - def test_save_unloaded_session - c = session_class.connection - bogus_class = c.quote(ActiveSupport::Base64.encode64("\004\010o:\vBlammo\000")) - c.insert("INSERT INTO #{session_class.table_name} ('#{session_id_column}', 'data') VALUES ('abcdefghijklmnop', #{bogus_class})") - - sess = session_class.find_by_session_id('abcdefghijklmnop') - assert_not_nil sess - assert !sess.loaded? - - # because the session is not loaded, the save should be a no-op. If it - # isn't, this'll try and unmarshall the bogus class, and should get an error. - assert_nothing_raised { sess.save } - end - - def teardown - session_class.drop_table! - end -end - -class ColumnLimitTest < ActiveRecordTestCase - def setup - @session_class = CGI::Session::ActiveRecordStore::Session - @session_class.create_table! - end - - def teardown - @session_class.drop_table! - end - - def test_protection_from_data_larger_than_column - # Can't test this unless there is a limit - return unless limit = @session_class.data_column_size_limit - too_big = ':(' * limit - s = @session_class.new(:session_id => '666', :data => {'foo' => too_big}) - s.data - assert_raise(ActionController::SessionOverflowError) { s.save } - end -end - -class DeprecatedActiveRecordStoreTest < ActiveRecordStoreTest - def session_id_column - "sessid" - end - - def setup - session_class.connection.execute 'create table old_sessions (id integer primary key, sessid text unique, data text)' - session_class.table_name = 'old_sessions' - session_class.send :setup_sessid_compatibility! - - ENV['REQUEST_METHOD'] = 'GET' - CGI::Session::ActiveRecordStore.session_class = session_class - - @new_session = CGI::Session.new(CGI.new, 'database_manager' => CGI::Session::ActiveRecordStore, 'new_session' => true) - @new_session['foo'] = 'bar' - end - - def teardown - session_class.connection.execute 'drop table old_sessions' - session_class.table_name = 'sessions' - end -end - -class SqlBypassActiveRecordStoreTest < ActiveRecordStoreTest - def session_class - unless defined? @session_class - @session_class = CGI::Session::ActiveRecordStore::SqlBypass - @session_class.connection = CGI::Session::ActiveRecordStore::Session.connection + def test_getting_nil_session_value + with_test_route_set do + get '/get_session_value' + assert_response :success + assert_equal 'foo: nil', response.body end - @session_class end - def test_model_attribute - assert_kind_of CGI::Session::ActiveRecordStore::SqlBypass, @new_session.model - assert_equal({ 'foo' => 'bar' }, @new_session.model.data) + def test_prevents_session_fixation + with_test_route_set do + get '/set_session_value' + assert_response :success + assert cookies['_session_id'] + + get '/get_session_value' + assert_response :success + assert_equal 'foo: "bar"', response.body + session_id = cookies['_session_id'] + assert session_id + + reset! + + get '/set_session_value', :_session_id => session_id, :foo => "baz" + assert_response :success + assert_equal nil, cookies['_session_id'] + + get '/get_session_value', :_session_id => session_id + assert_response :success + assert_equal 'foo: nil', response.body + assert_equal nil, cookies['_session_id'] + end end + + def test_allows_session_fixation + @integration_session = open_session(SessionAppWithFixation) + + with_test_route_set do + get '/set_session_value' + assert_response :success + assert cookies['_session_id'] + + get '/get_session_value' + assert_response :success + assert_equal 'foo: "bar"', response.body + session_id = cookies['_session_id'] + assert session_id + + reset! + @integration_session = open_session(SessionAppWithFixation) + + get '/set_session_value', :_session_id => session_id, :foo => "baz" + assert_response :success + assert_equal session_id, cookies['_session_id'] + + get '/get_session_value', :_session_id => session_id + assert_response :success + assert_equal 'foo: "baz"', response.body + assert_equal session_id, cookies['_session_id'] + end + end + + private + def with_test_route_set + with_routing do |set| + set.draw do |map| + map.with_options :controller => "active_record_store_test/test" do |c| + c.connect "/:action" + end + end + yield + end + end end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 6a793c8bb6..fd985a9a46 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -231,8 +231,6 @@ end class IntegrationProcessTest < ActionController::IntegrationTest class IntegrationController < ActionController::Base - session :off - def get respond_to do |format| format.html { render :text => "OK", :status => 200 } diff --git a/actionpack/test/controller/integration_upload_test.rb b/actionpack/test/controller/integration_upload_test.rb index b1dd6a6341..39d2e164e4 100644 --- a/actionpack/test/controller/integration_upload_test.rb +++ b/actionpack/test/controller/integration_upload_test.rb @@ -6,8 +6,6 @@ unless defined? ApplicationController end class UploadTestController < ActionController::Base - session :off - def update SessionUploadTest.last_request_type = ActionController::Base.param_parsers[request.content_type] render :text => "got here" diff --git a/actionpack/test/controller/rack_test.rb b/actionpack/test/controller/rack_test.rb index 641ef9626e..e2ec686c41 100644 --- a/actionpack/test/controller/rack_test.rb +++ b/actionpack/test/controller/rack_test.rb @@ -229,7 +229,7 @@ end class RackResponseTest < BaseRackTest def setup super - @response = ActionController::RackResponse.new(@request) + @response = ActionController::RackResponse.new end def test_simple_output @@ -265,34 +265,12 @@ class RackResponseTest < BaseRackTest body.each { |part| parts << part } assert_equal ["0", "1", "2", "3", "4"], parts end - - def test_set_session_cookie - cookie = CGI::Cookie.new({"name" => "name", "value" => "Josh"}) - @request.cgi.send :instance_variable_set, '@output_cookies', [cookie] - - @response.body = "Hello, World!" - @response.prepare! - - status, headers, body = @response.out - assert_equal "200 OK", status - assert_equal({ - "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "private, max-age=0, must-revalidate", - "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"', - "Set-Cookie" => ["name=Josh; path="], - "Content-Length" => "13" - }, headers) - - parts = [] - body.each { |part| parts << part } - assert_equal ["Hello, World!"], parts - end end class RackResponseHeadersTest < BaseRackTest def setup super - @response = ActionController::RackResponse.new(@request) + @response = ActionController::RackResponse.new @response.headers['Status'] = "200 OK" end diff --git a/actionpack/test/controller/session/cookie_store_test.rb b/actionpack/test/controller/session/cookie_store_test.rb index b5f14acc1f..8098059d46 100644 --- a/actionpack/test/controller/session/cookie_store_test.rb +++ b/actionpack/test/controller/session/cookie_store_test.rb @@ -1,298 +1,146 @@ require 'abstract_unit' require 'stringio' +class CookieStoreTest < ActionController::IntegrationTest + SessionKey = '_myapp_session' + SessionSecret = 'b3c631c314c0bbca50c1b2843150fe33' -class CGI::Session::CookieStore - def ensure_secret_secure_with_test_hax(secret) - if secret == CookieStoreTest.default_session_options['secret'] - return true - else - ensure_secret_secure_without_test_hax(secret) + DispatcherApp = ActionController::Dispatcher.new + CookieStoreApp = ActionController::Session::CookieStore.new(DispatcherApp, + :key => SessionKey, :secret => SessionSecret) + + SignedBar = "BAh7BjoIZm9vIghiYXI%3D--" + + "fef868465920f415f2c0652d6910d3af288a0367" + + class TestController < ActionController::Base + def no_session_access + head :ok end - end - alias_method_chain :ensure_secret_secure, :test_hax -end - -# Expose for tests. -class CGI - attr_reader :output_cookies, :output_hidden - - class Session - attr_reader :dbman - - class CookieStore - attr_reader :data, :original, :cookie_options + def set_session_value + session[:foo] = "bar" + head :ok end - end -end -class CookieStoreTest < Test::Unit::TestCase - def self.default_session_options - { 'database_manager' => CGI::Session::CookieStore, - 'session_key' => '_myapp_session', - 'secret' => 'Keep it secret; keep it safe.', - 'no_cookies' => true, - 'no_hidden' => true, - 'session_http_only' => true - } - end + def get_session_value + render :text => "foo: #{session[:foo].inspect}" + end - def self.cookies - { :empty => ['BAgw--0686dcaccc01040f4bd4f35fe160afe9bc04c330', {}], - :a_one => ['BAh7BiIGYWkG--5689059497d7f122a7119f171aef81dcfd807fec', { 'a' => 1 }], - :typical => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7BiILbm90aWNlIgxIZXkgbm93--9d20154623b9eeea05c62ab819be0e2483238759', { 'user_id' => 123, 'flash' => { 'notice' => 'Hey now' }}], - :flashed => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA==--bf9785a666d3c4ac09f7fe3353496b437546cfbf', { 'user_id' => 123, 'flash' => {} }] - } + def raise_data_overflow + session[:foo] = 'bye!' * 1024 + head :ok + end + def rescue_action(e) raise end end def setup - ENV.delete('HTTP_COOKIE') + @integration_session = open_session(CookieStoreApp) end def test_raises_argument_error_if_missing_session_key - [nil, ''].each do |blank| - assert_raise(ArgumentError, blank.inspect) { new_session 'session_key' => blank } - end + assert_raise(ArgumentError, nil.inspect) { + ActionController::Session::CookieStore.new(nil, + :key => nil, :secret => SessionSecret) + } + + assert_raise(ArgumentError, ''.inspect) { + ActionController::Session::CookieStore.new(nil, + :key => '', :secret => SessionSecret) + } end def test_raises_argument_error_if_missing_secret - [nil, ''].each do |blank| - assert_raise(ArgumentError, blank.inspect) { new_session 'secret' => blank } - end + assert_raise(ArgumentError, nil.inspect) { + ActionController::Session::CookieStore.new(nil, + :key => SessionKey, :secret => nil) + } + + assert_raise(ArgumentError, ''.inspect) { + ActionController::Session::CookieStore.new(nil, + :key => SessionKey, :secret => '') + } end def test_raises_argument_error_if_secret_is_probably_insecure - ["password", "secret", "12345678901234567890123456789"].each do |blank| - assert_raise(ArgumentError, blank.inspect) { new_session 'secret' => blank } - end + assert_raise(ArgumentError, "password".inspect) { + ActionController::Session::CookieStore.new(nil, + :key => SessionKey, :secret => "password") + } + + assert_raise(ArgumentError, "secret".inspect) { + ActionController::Session::CookieStore.new(nil, + :key => SessionKey, :secret => "secret") + } + + assert_raise(ArgumentError, "12345678901234567890123456789".inspect) { + ActionController::Session::CookieStore.new(nil, + :key => SessionKey, :secret => "12345678901234567890123456789") + } end - def test_reconfigures_session_to_omit_id_cookie_and_hidden_field - new_session do |session| - assert_equal true, @options['no_hidden'] - assert_equal true, @options['no_cookies'] - end + def test_setting_session_value + with_test_route_set do + get '/set_session_value' + assert_response :success + assert_equal ["_myapp_session=#{SignedBar}; path=/"], + headers['Set-Cookie'] + end end - def test_restore_unmarshals_missing_cookie_as_empty_hash - new_session do |session| - assert_nil session.dbman.data - assert_nil session['test'] - assert_equal Hash.new, session.dbman.data - end + def test_getting_session_value + with_test_route_set do + cookies[SessionKey] = SignedBar + get '/get_session_value' + assert_response :success + assert_equal 'foo: "bar"', response.body + end end - def test_restore_unmarshals_good_cookies - cookies(:empty, :a_one, :typical).each do |value, expected| - set_cookie! value - new_session do |session| - assert_nil session['lazy loads the data hash'] - assert_equal expected, session.dbman.data - end - end - end - - def test_restore_deletes_tampered_cookies - set_cookie! 'a--b' - new_session do |session| - assert_raise(CGI::Session::CookieStore::TamperedWithCookie) { session['fail'] } - assert_cookie_deleted session - end - end - - def test_close_doesnt_write_cookie_if_data_is_blank - new_session do |session| - assert_no_cookies session - session.close - assert_no_cookies session - end - end - - def test_close_doesnt_write_cookie_if_data_is_unchanged - set_cookie! cookie_value(:typical) - new_session do |session| - assert_no_cookies session - session['user_id'] = session['user_id'] - session.close - assert_no_cookies session + def test_disregards_tampered_sessions + with_test_route_set do + cookies[SessionKey] = "BAh7BjoIZm9vIghiYXI%3D--123456780" + get '/get_session_value' + assert_response :success + assert_equal 'foo: nil', response.body end end def test_close_raises_when_data_overflows - set_cookie! cookie_value(:empty) - new_session do |session| - session['overflow'] = 'bye!' * 1024 - assert_raise(CGI::Session::CookieStore::CookieOverflow) { session.close } - assert_no_cookies session + with_test_route_set do + assert_raise(ActionController::Session::CookieStore::CookieOverflow) { + get '/raise_data_overflow' + } end end - def test_close_marshals_and_writes_cookie - set_cookie! cookie_value(:typical) - new_session do |session| - assert_no_cookies session - session['flash'] = {} - assert_no_cookies session - session.close - assert_equal 1, session.cgi.output_cookies.size - cookie = session.cgi.output_cookies.first - assert_cookie cookie, cookie_value(:flashed) - assert_http_only_cookie cookie - assert_secure_cookie cookie, false + def test_doesnt_write_session_cookie_if_session_is_not_accessed + with_test_route_set do + get '/no_session_access' + assert_response :success + assert_equal [], headers['Set-Cookie'] end end - def test_writes_non_secure_cookie_by_default - set_cookie! cookie_value(:typical) - new_session do |session| - session['flash'] = {} - session.close - cookie = session.cgi.output_cookies.first - assert_secure_cookie cookie,false - end - end - - def test_writes_secure_cookie - set_cookie! cookie_value(:typical) - new_session('session_secure'=>true) do |session| - session['flash'] = {} - session.close - cookie = session.cgi.output_cookies.first - assert_secure_cookie cookie - end - end - - def test_http_only_cookie_by_default - set_cookie! cookie_value(:typical) - new_session do |session| - session['flash'] = {} - session.close - cookie = session.cgi.output_cookies.first - assert_http_only_cookie cookie - end - end - - def test_overides_http_only_cookie - set_cookie! cookie_value(:typical) - new_session('session_http_only'=>false) do |session| - session['flash'] = {} - session.close - cookie = session.cgi.output_cookies.first - assert_http_only_cookie cookie, false - end - end - - def test_delete_writes_expired_empty_cookie_and_sets_data_to_nil - set_cookie! cookie_value(:typical) - new_session do |session| - assert_no_cookies session - session.delete - assert_cookie_deleted session - - # @data is set to nil so #close doesn't send another cookie. - session.close - assert_cookie_deleted session - end - end - - def test_new_session_doesnt_reuse_deleted_cookie_data - set_cookie! cookie_value(:typical) - - new_session do |session| - assert_not_nil session['user_id'] - session.delete - - # Start a new session using the same CGI instance. - post_delete_session = CGI::Session.new(session.cgi, self.class.default_session_options) - assert_nil post_delete_session['user_id'] + def test_doesnt_write_session_cookie_if_session_is_unchanged + with_test_route_set do + cookies[SessionKey] = "BAh7BjoIZm9vIghiYXI%3D--" + + "fef868465920f415f2c0652d6910d3af288a0367" + get '/no_session_access' + assert_response :success + assert_equal [], headers['Set-Cookie'] end end private - def assert_no_cookies(session) - assert_nil session.cgi.output_cookies, session.cgi.output_cookies.inspect - end - - def assert_cookie_deleted(session, message = 'Expected session deletion cookie to be set') - assert_equal 1, session.cgi.output_cookies.size - cookie = session.cgi.output_cookies.first - assert_cookie cookie, nil, 1.year.ago.to_date, "#{message}: #{cookie.name} => #{cookie.value}" - end - - def assert_cookie(cookie, value = nil, expires = nil, message = nil) - assert_equal '_myapp_session', cookie.name, message - assert_equal [value].compact, cookie.value, message - assert_equal expires, cookie.expires ? cookie.expires.to_date : cookie.expires, message - end - - def assert_secure_cookie(cookie,value=true) - assert cookie.secure==value - end - - def assert_http_only_cookie(cookie,value=true) - assert cookie.http_only==value - end - - def cookies(*which) - self.class.cookies.values_at(*which) - end - - def cookie_value(which) - self.class.cookies[which].first - end - - def set_cookie!(value) - ENV['HTTP_COOKIE'] = "_myapp_session=#{value}" - end - - def new_session(options = {}) - with_cgi do |cgi| - assert_nil cgi.output_hidden, "Output hidden params should be empty: #{cgi.output_hidden.inspect}" - assert_nil cgi.output_cookies, "Output cookies should be empty: #{cgi.output_cookies.inspect}" - - @options = self.class.default_session_options.merge(options) - session = CGI::Session.new(cgi, @options) - ObjectSpace.undefine_finalizer(session) - - assert_nil cgi.output_hidden, "Output hidden params should be empty: #{cgi.output_hidden.inspect}" - assert_nil cgi.output_cookies, "Output cookies should be empty: #{cgi.output_cookies.inspect}" - - yield session if block_given? - session + def with_test_route_set + with_routing do |set| + set.draw do |map| + map.with_options :controller => "cookie_store_test/test" do |c| + c.connect "/:action" + end + end + yield end end - - def with_cgi - ENV['REQUEST_METHOD'] = 'GET' - ENV['HTTP_HOST'] = 'example.com' - ENV['QUERY_STRING'] = '' - - cgi = CGI.new('query', StringIO.new('')) - yield cgi if block_given? - cgi - end -end - - -class CookieStoreWithBlockAsSecretTest < CookieStoreTest - def self.default_session_options - CookieStoreTest.default_session_options.merge 'secret' => Proc.new { 'Keep it secret; keep it safe.' } - end -end - - -class CookieStoreWithMD5DigestTest < CookieStoreTest - def self.default_session_options - CookieStoreTest.default_session_options.merge 'digest' => 'MD5' - end - - def self.cookies - { :empty => ['BAgw--0415cc0be9579b14afc22ee2d341aa21', {}], - :a_one => ['BAh7BiIGYWkG--5a0ed962089cc6600ff44168a5d59bc8', { 'a' => 1 }], - :typical => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7BiILbm90aWNlIgxIZXkgbm93--f426763f6ef435b3738b493600db8d64', { 'user_id' => 123, 'flash' => { 'notice' => 'Hey now' }}], - :flashed => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA==--0af9156650dab044a53a91a4ddec2c51', { 'user_id' => 123, 'flash' => {} }], - :double_escaped => [CGI.escape('BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA%3D%3D--0af9156650dab044a53a91a4ddec2c51'), { 'user_id' => 123, 'flash' => {} }] } - end end diff --git a/actionpack/test/controller/session/mem_cache_store_test.rb b/actionpack/test/controller/session/mem_cache_store_test.rb index 9ab927a01f..52e31b78da 100644 --- a/actionpack/test/controller/session/mem_cache_store_test.rb +++ b/actionpack/test/controller/session/mem_cache_store_test.rb @@ -1,178 +1,81 @@ require 'abstract_unit' -class CGI::Session - def cache - dbman.instance_variable_get(:@cache) - end -end - - -uses_mocha 'MemCacheStore tests' do -if defined? MemCache::MemCacheError - -class MemCacheStoreTest < Test::Unit::TestCase - SESSION_KEY_RE = /^session:[0-9a-z]+/ - CONN_TEST_KEY = 'connection_test' - MULTI_TEST_KEY = '0123456789' - TEST_DATA = 'Hello test' - - def self.get_mem_cache_if_available - begin - require 'memcache' - cache = MemCache.new('127.0.0.1') - # Test availability of the connection - cache.set(CONN_TEST_KEY, 1) - unless cache.get(CONN_TEST_KEY) == 1 - puts 'Warning: memcache server available but corrupted.' - return nil - end - rescue LoadError, MemCache::MemCacheError - return nil +# You need to start a memcached server inorder to run these tests +class MemCacheStoreTest < ActionController::IntegrationTest + class TestController < ActionController::Base + def no_session_access + head :ok end - return cache + + def set_session_value + session[:foo] = "bar" + head :ok + end + + def get_session_value + render :text => "foo: #{session[:foo].inspect}" + end + + def rescue_action(e) raise end end - CACHE = get_mem_cache_if_available + begin + DispatcherApp = ActionController::Dispatcher.new + MemCacheStoreApp = ActionController::Session::MemCacheStore.new( + DispatcherApp, :key => '_session_id') - def test_initialization - assert_raise(ArgumentError) { new_session('session_id' => '!invalid_id') } - new_session do |s| - assert_equal Hash.new, s.cache.get('session:' + s.session_id) + def setup + @integration_session = open_session(MemCacheStoreApp) end + + def test_setting_and_getting_session_value + with_test_route_set do + get '/set_session_value' + assert_response :success + assert cookies['_session_id'] + + get '/get_session_value' + assert_response :success + assert_equal 'foo: "bar"', response.body + end + end + + def test_getting_nil_session_value + with_test_route_set do + get '/get_session_value' + assert_response :success + assert_equal 'foo: nil', response.body + end + end + + def test_prevents_session_fixation + with_test_route_set do + get '/get_session_value' + assert_response :success + assert_equal 'foo: nil', response.body + session_id = cookies['_session_id'] + + reset! + + get '/set_session_value', :_session_id => session_id + assert_response :success + assert_equal nil, cookies['_session_id'] + end + end + rescue LoadError, RuntimeError + $stderr.puts "Skipping MemCacheStoreTest tests. Start memcached and try again." end - - def test_storage - d = rand(0xffff) - new_session do |s| - session_key = 'session:' + s.session_id - unless CACHE - s.cache.expects(:get).with(session_key) \ - .returns(:test => d) - s.cache.expects(:set).with(session_key, - has_entry(:test, d), - 0) - end - s[:test] = d - s.close - assert_equal d, s.cache.get(session_key)[:test] - assert_equal d, s[:test] - end - end - - def test_deletion - new_session do |s| - session_key = 'session:' + s.session_id - unless CACHE - s.cache.expects(:delete) - s.cache.expects(:get).with(session_key) \ - .returns(nil) - end - s[:test] = rand(0xffff) - s.delete - assert_nil s.cache.get(session_key) - end - end - - - def test_other_session_retrieval - new_session do |sa| - unless CACHE - sa.cache.expects(:set).with('session:' + sa.session_id, - has_entry(:test, TEST_DATA), - 0) - end - sa[:test] = TEST_DATA - sa.close - new_session('session_id' => sa.session_id) do |sb| - unless CACHE - sb.cache.expects(:[]).with('session:' + sb.session_id) \ - .returns(:test => TEST_DATA) - end - assert_equal(TEST_DATA, sb[:test]) - end - end - end - - - def test_multiple_sessions - s_slots = Array.new(10) - operation = :write - last_data = nil - reads = writes = 0 - 50.times do - current = rand(10) - s_slots[current] ||= new_session('session_id' => MULTI_TEST_KEY, - 'new_session' => true) - s = s_slots[current] - case operation - when :write - last_data = rand(0xffff) - unless CACHE - s.cache.expects(:set).with('session:' + MULTI_TEST_KEY, - { :test => last_data }, - 0) - end - s[:test] = last_data - s.close - writes += 1 - when :read - # Make CGI::Session#[] think there was no data retrieval yet. - # Normally, the session caches the data during its lifetime. - s.instance_variable_set(:@data, nil) - unless CACHE - s.cache.expects(:[]).with('session:' + MULTI_TEST_KEY) \ - .returns(:test => last_data) - end - d = s[:test] - assert_equal(last_data, d, "OK reads: #{reads}, OK writes: #{writes}") - reads += 1 - end - operation = rand(5) == 0 ? :write : :read - end - end - - - private - def obtain_session_options - options = { 'database_manager' => CGI::Session::MemCacheStore, - 'session_key' => '_test_app_session' - } - # if don't have running memcache server we use mock instead - unless CACHE - options['cache'] = c = mock - c.stubs(:[]).with(regexp_matches(SESSION_KEY_RE)) - c.stubs(:get).with(regexp_matches(SESSION_KEY_RE)) \ - .returns(Hash.new) - c.stubs(:add).with(regexp_matches(SESSION_KEY_RE), - instance_of(Hash), - 0) + def with_test_route_set + with_routing do |set| + set.draw do |map| + map.with_options :controller => "mem_cache_store_test/test" do |c| + c.connect "/:action" + end + end + yield + end end - options - end - - - def new_session(options = {}) - with_cgi do |cgi| - @options = obtain_session_options.merge(options) - session = CGI::Session.new(cgi, @options) - yield session if block_given? - return session - end - end - - def with_cgi - ENV['REQUEST_METHOD'] = 'GET' - ENV['HTTP_HOST'] = 'example.com' - ENV['QUERY_STRING'] = '' - - cgi = CGI.new('query', StringIO.new('')) - yield cgi if block_given? - cgi - end end - -end # defined? MemCache -end # uses_mocha diff --git a/actionpack/test/controller/session_fixation_test.rb b/actionpack/test/controller/session_fixation_test.rb index e8dc8bd295..9e5b45dc3d 100644 --- a/actionpack/test/controller/session_fixation_test.rb +++ b/actionpack/test/controller/session_fixation_test.rb @@ -1,84 +1,84 @@ -require 'abstract_unit' - -class SessionFixationTest < ActionController::IntegrationTest - class TestController < ActionController::Base - session :session_key => '_myapp_session_id', - :secret => CGI::Session.generate_unique_id, - :except => :default_session_key - - session :cookie_only => false, - :only => :allow_session_fixation - - def default_session_key - render :text => "default_session_key" - end - - def custom_session_key - render :text => "custom_session_key: #{params[:id]}" - end - - def allow_session_fixation - render :text => "allow_session_fixation" - end - - def rescue_action(e) raise end - end - - def setup - @controller = TestController.new - end - - def test_should_be_able_to_make_a_successful_request - with_test_route_set do - assert_nothing_raised do - get '/custom_session_key', :id => "1" - end - assert_equal 'custom_session_key: 1', @controller.response.body - assert_not_nil @controller.session - end - end - - def test_should_catch_session_fixation_attempt - with_test_route_set do - assert_raises(ActionController::RackRequest::SessionFixationAttempt) do - get '/custom_session_key', :_myapp_session_id => "42" - end - assert_nil @controller.session - end - end - - def test_should_not_catch_session_fixation_attempt_when_cookie_only_setting_is_disabled - with_test_route_set do - assert_nothing_raised do - get '/allow_session_fixation', :_myapp_session_id => "42" - end - assert !@controller.response.body.blank? - assert_not_nil @controller.session - end - end - - def test_should_catch_session_fixation_attempt_with_default_session_key - # using the default session_key is not possible with cookie store - ActionController::Base.session_store = :p_store - - with_test_route_set do - assert_raises ActionController::RackRequest::SessionFixationAttempt do - get '/default_session_key', :_session_id => "42" - end - assert_nil @controller.response - assert_nil @controller.session - end - end - - private - def with_test_route_set - with_routing do |set| - set.draw do |map| - map.with_options :controller => "session_fixation_test/test" do |c| - c.connect "/:action" - end - end - yield - end - end -end +# require 'abstract_unit' +# +# class SessionFixationTest < ActionController::IntegrationTest +# class TestController < ActionController::Base +# session :session_key => '_myapp_session_id', +# :secret => CGI::Session.generate_unique_id, +# :except => :default_session_key +# +# session :cookie_only => false, +# :only => :allow_session_fixation +# +# def default_session_key +# render :text => "default_session_key" +# end +# +# def custom_session_key +# render :text => "custom_session_key: #{params[:id]}" +# end +# +# def allow_session_fixation +# render :text => "allow_session_fixation" +# end +# +# def rescue_action(e) raise end +# end +# +# def setup +# @controller = TestController.new +# end +# +# def test_should_be_able_to_make_a_successful_request +# with_test_route_set do +# assert_nothing_raised do +# get '/custom_session_key', :id => "1" +# end +# assert_equal 'custom_session_key: 1', @controller.response.body +# assert_not_nil @controller.session +# end +# end +# +# def test_should_catch_session_fixation_attempt +# with_test_route_set do +# assert_raises(ActionController::RackRequest::SessionFixationAttempt) do +# get '/custom_session_key', :_myapp_session_id => "42" +# end +# assert_nil @controller.session +# end +# end +# +# def test_should_not_catch_session_fixation_attempt_when_cookie_only_setting_is_disabled +# with_test_route_set do +# assert_nothing_raised do +# get '/allow_session_fixation', :_myapp_session_id => "42" +# end +# assert !@controller.response.body.blank? +# assert_not_nil @controller.session +# end +# end +# +# def test_should_catch_session_fixation_attempt_with_default_session_key +# # using the default session_key is not possible with cookie store +# ActionController::Base.session_store = :p_store +# +# with_test_route_set do +# assert_raises ActionController::RackRequest::SessionFixationAttempt do +# get '/default_session_key', :_session_id => "42" +# end +# assert_nil @controller.response +# assert_nil @controller.session +# end +# end +# +# private +# def with_test_route_set +# with_routing do |set| +# set.draw do |map| +# map.with_options :controller => "session_fixation_test/test" do |c| +# c.connect "/:action" +# end +# end +# yield +# end +# end +# end diff --git a/actionpack/test/controller/session_management_test.rb b/actionpack/test/controller/session_management_test.rb deleted file mode 100644 index 592b0b549d..0000000000 --- a/actionpack/test/controller/session_management_test.rb +++ /dev/null @@ -1,178 +0,0 @@ -require 'abstract_unit' - -class SessionManagementTest < Test::Unit::TestCase - class SessionOffController < ActionController::Base - session :off - - def show - render :text => "done" - end - - def tell - render :text => "done" - end - end - - class SessionOffOnController < ActionController::Base - session :off - session :on, :only => :tell - - def show - render :text => "done" - end - - def tell - render :text => "done" - end - end - - class TestController < ActionController::Base - session :off, :only => :show - session :session_secure => true, :except => :show - session :off, :only => :conditional, - :if => Proc.new { |r| r.parameters[:ws] } - - def show - render :text => "done" - end - - def tell - render :text => "done" - end - - def conditional - render :text => ">>>#{params[:ws]}<<<" - end - end - - class SpecializedController < SessionOffController - session :disabled => false, :only => :something - - def something - render :text => "done" - end - - def another - render :text => "done" - end - end - - class AssociationCachingTestController < ActionController::Base - class ObjectWithAssociationCache - def initialize - @cached_associations = false - end - - def fetch_associations - @cached_associations = true - end - - def clear_association_cache - @cached_associations = false - end - - def has_cached_associations? - @cached_associations - end - end - - def show - session[:object] = ObjectWithAssociationCache.new - session[:object].fetch_associations - if session[:object].has_cached_associations? - render :text => "has cached associations" - else - render :text => "does not have cached associations" - end - end - - def tell - if session[:object] - if session[:object].has_cached_associations? - render :text => "has cached associations" - else - render :text => "does not have cached associations" - end - else - render :text => "there is no object" - end - end - end - - - def setup - @request, @response = ActionController::TestRequest.new, - ActionController::TestResponse.new - end - - def test_session_off_globally - @controller = SessionOffController.new - get :show - assert_equal false, @request.session_options - get :tell - assert_equal false, @request.session_options - end - - def test_session_off_then_on_globally - @controller = SessionOffOnController.new - get :show - assert_equal false, @request.session_options - get :tell - assert_instance_of Hash, @request.session_options - assert_equal false, @request.session_options[:disabled] - end - - def test_session_off_conditionally - @controller = TestController.new - get :show - assert_equal false, @request.session_options - get :tell - assert_instance_of Hash, @request.session_options - assert @request.session_options[:session_secure] - end - - def test_controller_specialization_overrides_settings - @controller = SpecializedController.new - get :something - assert_instance_of Hash, @request.session_options - get :another - assert_equal false, @request.session_options - end - - def test_session_off_with_if - @controller = TestController.new - get :conditional - assert_instance_of Hash, @request.session_options - get :conditional, :ws => "ws" - assert_equal false, @request.session_options - end - - def test_session_store_setting - ActionController::Base.session_store = :drb_store - assert_equal CGI::Session::DRbStore, ActionController::Base.session_store - - if Object.const_defined?(:ActiveRecord) - ActionController::Base.session_store = :active_record_store - assert_equal CGI::Session::ActiveRecordStore, ActionController::Base.session_store - end - end - - def test_process_cleanup_with_session_management_support - @controller = AssociationCachingTestController.new - get :show - assert_equal "has cached associations", @response.body - get :tell - assert_equal "does not have cached associations", @response.body - end - - def test_session_is_enabled - @controller = TestController.new - get :show - assert_nothing_raised do - assert_equal false, @controller.session_enabled? - end - - get :tell - assert @controller.session_enabled? - end -end diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb index 4c44ea4205..e89d6bb960 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -2,8 +2,6 @@ require 'abstract_unit' class WebServiceTest < ActionController::IntegrationTest class TestController < ActionController::Base - session :off - def assign_parameters if params[:full] render :text => dump_params_keys diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 1aaf456c0f..c428366a04 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -60,6 +60,7 @@ module ActiveRecord autoload :Schema, 'active_record/schema' autoload :SchemaDumper, 'active_record/schema_dumper' autoload :Serialization, 'active_record/serialization' + autoload :SessionStore, 'active_record/session_store' autoload :TestCase, 'active_record/test_case' autoload :Timestamp, 'active_record/timestamp' autoload :Transactions, 'active_record/transactions' diff --git a/activerecord/lib/active_record/session_store.rb b/activerecord/lib/active_record/session_store.rb new file mode 100644 index 0000000000..bd198c03b2 --- /dev/null +++ b/activerecord/lib/active_record/session_store.rb @@ -0,0 +1,319 @@ +module ActiveRecord + # A session store backed by an Active Record class. A default class is + # provided, but any object duck-typing to an Active Record Session class + # with text +session_id+ and +data+ attributes is sufficient. + # + # The default assumes a +sessions+ tables with columns: + # +id+ (numeric primary key), + # +session_id+ (text, or longtext if your session data exceeds 65K), and + # +data+ (text or longtext; careful if your session data exceeds 65KB). + # The +session_id+ column should always be indexed for speedy lookups. + # Session data is marshaled to the +data+ column in Base64 format. + # If the data you write is larger than the column's size limit, + # ActionController::SessionOverflowError will be raised. + # + # You may configure the table name, primary key, and data column. + # For example, at the end of config/environment.rb: + # ActiveRecord::SessionStore::Session.table_name = 'legacy_session_table' + # ActiveRecord::SessionStore::Session.primary_key = 'session_id' + # ActiveRecord::SessionStore::Session.data_column_name = 'legacy_session_data' + # Note that setting the primary key to the +session_id+ frees you from + # having a separate +id+ column if you don't want it. However, you must + # set session.model.id = session.session_id by hand! A before filter + # on ApplicationController is a good place. + # + # Since the default class is a simple Active Record, you get timestamps + # for free if you add +created_at+ and +updated_at+ datetime columns to + # the +sessions+ table, making periodic session expiration a snap. + # + # You may provide your own session class implementation, whether a + # feature-packed Active Record or a bare-metal high-performance SQL + # store, by setting + # ActiveRecord::SessionStore.session_class = MySessionClass + # You must implement these methods: + # self.find_by_session_id(session_id) + # initialize(hash_of_session_id_and_data) + # attr_reader :session_id + # attr_accessor :data + # save + # destroy + # + # The example SqlBypass class is a generic SQL session store. You may + # use it as a basis for high-performance database-specific stores. + class SessionStore < ActionController::Session::AbstractStore + # The default Active Record class. + class Session < ActiveRecord::Base + ## + # :singleton-method: + # Customizable data column name. Defaults to 'data'. + cattr_accessor :data_column_name + self.data_column_name = 'data' + + before_save :marshal_data! + before_save :raise_on_session_data_overflow! + + class << self + # Don't try to reload ARStore::Session in dev mode. + def reloadable? #:nodoc: + false + end + + def data_column_size_limit + @data_column_size_limit ||= columns_hash[@@data_column_name].limit + end + + # Hook to set up sessid compatibility. + def find_by_session_id(session_id) + setup_sessid_compatibility! + find_by_session_id(session_id) + end + + def marshal(data) + ActiveSupport::Base64.encode64(Marshal.dump(data)) if data + end + + def unmarshal(data) + Marshal.load(ActiveSupport::Base64.decode64(data)) if data + end + + def create_table! + connection.execute <<-end_sql + CREATE TABLE #{table_name} ( + id INTEGER PRIMARY KEY, + #{connection.quote_column_name('session_id')} TEXT UNIQUE, + #{connection.quote_column_name(@@data_column_name)} TEXT(255) + ) + end_sql + end + + def drop_table! + connection.execute "DROP TABLE #{table_name}" + end + + private + # Compatibility with tables using sessid instead of session_id. + def setup_sessid_compatibility! + # Reset column info since it may be stale. + reset_column_information + if columns_hash['sessid'] + def self.find_by_session_id(*args) + find_by_sessid(*args) + end + + define_method(:session_id) { sessid } + define_method(:session_id=) { |session_id| self.sessid = session_id } + else + def self.find_by_session_id(session_id) + find :first, :conditions => ["session_id #{attribute_condition(session_id)}", session_id] + end + end + end + end + + # Lazy-unmarshal session state. + def data + @data ||= self.class.unmarshal(read_attribute(@@data_column_name)) || {} + end + + attr_writer :data + + # Has the session been loaded yet? + def loaded? + !!@data + end + + private + def marshal_data! + return false if !loaded? + write_attribute(@@data_column_name, self.class.marshal(self.data)) + end + + # Ensures that the data about to be stored in the database is not + # larger than the data storage column. Raises + # ActionController::SessionOverflowError. + def raise_on_session_data_overflow! + return false if !loaded? + limit = self.class.data_column_size_limit + if loaded? and limit and read_attribute(@@data_column_name).size > limit + raise ActionController::SessionOverflowError + end + end + end + + # A barebones session store which duck-types with the default session + # store but bypasses Active Record and issues SQL directly. This is + # an example session model class meant as a basis for your own classes. + # + # The database connection, table name, and session id and data columns + # are configurable class attributes. Marshaling and unmarshaling + # are implemented as class methods that you may override. By default, + # marshaling data is + # + # ActiveSupport::Base64.encode64(Marshal.dump(data)) + # + # and unmarshaling data is + # + # Marshal.load(ActiveSupport::Base64.decode64(data)) + # + # This marshaling behavior is intended to store the widest range of + # binary session data in a +text+ column. For higher performance, + # store in a +blob+ column instead and forgo the Base64 encoding. + class SqlBypass + ## + # :singleton-method: + # Use the ActiveRecord::Base.connection by default. + cattr_accessor :connection + + ## + # :singleton-method: + # The table name defaults to 'sessions'. + cattr_accessor :table_name + @@table_name = 'sessions' + + ## + # :singleton-method: + # The session id field defaults to 'session_id'. + cattr_accessor :session_id_column + @@session_id_column = 'session_id' + + ## + # :singleton-method: + # The data field defaults to 'data'. + cattr_accessor :data_column + @@data_column = 'data' + + class << self + def connection + @@connection ||= ActiveRecord::Base.connection + end + + # Look up a session by id and unmarshal its data if found. + def find_by_session_id(session_id) + if record = @@connection.select_one("SELECT * FROM #{@@table_name} WHERE #{@@session_id_column}=#{@@connection.quote(session_id)}") + new(:session_id => session_id, :marshaled_data => record['data']) + end + end + + def marshal(data) + ActiveSupport::Base64.encode64(Marshal.dump(data)) if data + end + + def unmarshal(data) + Marshal.load(ActiveSupport::Base64.decode64(data)) if data + end + + def create_table! + @@connection.execute <<-end_sql + CREATE TABLE #{table_name} ( + id INTEGER PRIMARY KEY, + #{@@connection.quote_column_name(session_id_column)} TEXT UNIQUE, + #{@@connection.quote_column_name(data_column)} TEXT + ) + end_sql + end + + def drop_table! + @@connection.execute "DROP TABLE #{table_name}" + end + end + + attr_reader :session_id + attr_writer :data + + # Look for normal and marshaled data, self.find_by_session_id's way of + # telling us to postpone unmarshaling until the data is requested. + # We need to handle a normal data attribute in case of a new record. + def initialize(attributes) + @session_id, @data, @marshaled_data = attributes[:session_id], attributes[:data], attributes[:marshaled_data] + @new_record = @marshaled_data.nil? + end + + def new_record? + @new_record + end + + # Lazy-unmarshal session state. + def data + unless @data + if @marshaled_data + @data, @marshaled_data = self.class.unmarshal(@marshaled_data) || {}, nil + else + @data = {} + end + end + @data + end + + def loaded? + !!@data + end + + def save + return false if !loaded? + marshaled_data = self.class.marshal(data) + + if @new_record + @new_record = false + @@connection.update <<-end_sql, 'Create session' + INSERT INTO #{@@table_name} ( + #{@@connection.quote_column_name(@@session_id_column)}, + #{@@connection.quote_column_name(@@data_column)} ) + VALUES ( + #{@@connection.quote(session_id)}, + #{@@connection.quote(marshaled_data)} ) + end_sql + else + @@connection.update <<-end_sql, 'Update session' + UPDATE #{@@table_name} + SET #{@@connection.quote_column_name(@@data_column)}=#{@@connection.quote(marshaled_data)} + WHERE #{@@connection.quote_column_name(@@session_id_column)}=#{@@connection.quote(session_id)} + end_sql + end + end + + def destroy + unless @new_record + @@connection.delete <<-end_sql, 'Destroy session' + DELETE FROM #{@@table_name} + WHERE #{@@connection.quote_column_name(@@session_id_column)}=#{@@connection.quote(session_id)} + end_sql + end + end + end + + # The class used for session storage. Defaults to + # ActiveRecord::SessionStore::Session + cattr_accessor :session_class + self.session_class = Session + + SESSION_RECORD_KEY = 'rack.session.record'.freeze + + private + def get_session(env, sid) + Base.silence do + sid ||= generate_sid + session = @@session_class.find_by_session_id(sid) + session ||= @@session_class.new(:session_id => sid, :data => {}) + env[SESSION_RECORD_KEY] = session + [sid, session.data] + end + end + + def set_session(env, sid, session_data) + Base.silence do + record = env[SESSION_RECORD_KEY] + record.data = session_data + return false unless record.save + + session_data = record.data + if session_data && session_data.respond_to?(:each_value) + session_data.each_value do |obj| + obj.clear_association_cache if obj.respond_to?(:clear_association_cache) + end + end + end + + return true + end + end +end diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 06a3332c42..56e8ce95ab 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -39,7 +39,7 @@ module Rails nil end end - + def backtrace_cleaner @@backtrace_cleaner ||= begin # Relies on ActiveSupport, so we have to lazy load to postpone definition until AS has been loaded @@ -148,7 +148,6 @@ module Rails initialize_dependency_mechanism initialize_whiny_nils - initialize_temporary_session_directory initialize_time_zone initialize_i18n @@ -501,13 +500,6 @@ Run `rake gems:install` to install the missing gems. require('active_support/whiny_nil') if configuration.whiny_nils end - def initialize_temporary_session_directory - if configuration.frameworks.include?(:action_controller) - session_path = "#{configuration.root_path}/tmp/sessions/" - ActionController::Base.session_options[:tmpdir] = File.exist?(session_path) ? session_path : Dir::tmpdir - end - end - # Sets the default value for Time.zone, and turns on ActiveRecord::Base#time_zone_aware_attributes. # If assigned value cannot be matched to a TimeZone, an exception will be raised. def initialize_time_zone @@ -529,7 +521,7 @@ Run `rake gems:install` to install the missing gems. end end - # Set the i18n configuration from config.i18n but special-case for the load_path which should be + # Set the i18n configuration from config.i18n but special-case for the load_path which should be # appended to what's already set instead of overwritten. def initialize_i18n configuration.i18n.each do |setting, value| diff --git a/railties/lib/tasks/databases.rake b/railties/lib/tasks/databases.rake index 3a576063fa..68ffefae0b 100644 --- a/railties/lib/tasks/databases.rake +++ b/railties/lib/tasks/databases.rake @@ -380,7 +380,7 @@ namespace :db do end namespace :sessions do - desc "Creates a sessions migration for use with CGI::Session::ActiveRecordStore" + desc "Creates a sessions migration for use with ActiveRecord::SessionStore" task :create => :environment do raise "Task unavailable to this database (no migration support)" unless ActiveRecord::Base.connection.supports_migrations? require 'rails_generator' diff --git a/railties/test/console_app_test.rb b/railties/test/console_app_test.rb index 6cfc907b80..cbaf230594 100644 --- a/railties/test/console_app_test.rb +++ b/railties/test/console_app_test.rb @@ -4,6 +4,7 @@ require 'action_controller' # console_app uses 'action_controller/integration' unless defined? ApplicationController class ApplicationController < ActionController::Base; end + ActionController::Base.session_store = nil end require 'dispatcher' From 43ac42c46a462e1453b1b97da00e11bff7bba55d Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 15 Dec 2008 19:25:31 -0600 Subject: [PATCH 034/188] Clear empty nil values in session hash before saving --- .../session/abstract_store.rb | 4 ++- .../action_controller/session/cookie_store.rb | 25 +++---------------- 2 files changed, 6 insertions(+), 23 deletions(-) diff --git a/actionpack/lib/action_controller/session/abstract_store.rb b/actionpack/lib/action_controller/session/abstract_store.rb index b23bf062c4..c6dd865fad 100644 --- a/actionpack/lib/action_controller/session/abstract_store.rb +++ b/actionpack/lib/action_controller/session/abstract_store.rb @@ -32,7 +32,9 @@ module ActionController end def to_hash - {}.replace(self) + h = {}.replace(self) + h.delete_if { |k,v| v.nil? } + h end private diff --git a/actionpack/lib/action_controller/session/cookie_store.rb b/actionpack/lib/action_controller/session/cookie_store.rb index f13c9290c0..f4089bfa8b 100644 --- a/actionpack/lib/action_controller/session/cookie_store.rb +++ b/actionpack/lib/action_controller/session/cookie_store.rb @@ -74,30 +74,11 @@ module ActionController freeze end - class SessionHash < Hash - def initialize(middleware, env) - @middleware = middleware - @env = env - @loaded = false - end - - def [](key) - load! unless @loaded - super - end - - def []=(key, value) - load! unless @loaded - super - end - - def to_hash - {}.replace(self) - end - + class SessionHash < AbstractStore::SessionHash private def load! - replace(@middleware.send(:load_session, @env)) + session = @by.send(:load_session, @env) + replace(session) @loaded = true end end From 19be3d35b38b6685789d8d343617d465a3652717 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Dec 2008 18:20:18 -0800 Subject: [PATCH 035/188] Revert "Make constantize look into ancestors" [#410 state:open] This reverts commit 262fef7ed57520b857605a0105fe7ba9265654f6. --- activesupport/lib/active_support/inflector.rb | 63 ++++++++++++------- activesupport/test/inflector_test.rb | 17 +---- 2 files changed, 41 insertions(+), 39 deletions(-) diff --git a/activesupport/lib/active_support/inflector.rb b/activesupport/lib/active_support/inflector.rb index 9fb7540ea2..4921b99677 100644 --- a/activesupport/lib/active_support/inflector.rb +++ b/activesupport/lib/active_support/inflector.rb @@ -330,30 +330,47 @@ module ActiveSupport underscore(demodulize(class_name)) + (separate_class_name_and_id_with_underscore ? "_id" : "id") end - # Tries to find a constant with the name specified in the argument string: - # - # "Module".constantize # => Module - # "Test::Unit".constantize # => Test::Unit - # - # The name is assumed to be the one of a top-level constant, no matter whether - # it starts with "::" or not. No lexical context is taken into account: - # - # C = 'outside' - # module M - # C = 'inside' - # C # => 'inside' - # "C".constantize # => 'outside', same as ::C - # end - # - # NameError is raised when the name is not in CamelCase or the constant is - # unknown. - def constantize(camel_cased_word) - names = camel_cased_word.split('::') - names.shift if names.empty? || names.first.empty? + # Ruby 1.9 introduces an inherit argument for Module#const_get and + # #const_defined? and changes their default behavior. + if Module.method(:const_get).arity == 1 + # Tries to find a constant with the name specified in the argument string: + # + # "Module".constantize # => Module + # "Test::Unit".constantize # => Test::Unit + # + # The name is assumed to be the one of a top-level constant, no matter whether + # it starts with "::" or not. No lexical context is taken into account: + # + # C = 'outside' + # module M + # C = 'inside' + # C # => 'inside' + # "C".constantize # => 'outside', same as ::C + # end + # + # NameError is raised when the name is not in CamelCase or the constant is + # unknown. + def constantize(camel_cased_word) + names = camel_cased_word.split('::') + names.shift if names.empty? || names.first.empty? - constant = Object - names.each { |name| constant = constant.const_get(name) } - constant + constant = Object + names.each do |name| + constant = constant.const_defined?(name) ? constant.const_get(name) : constant.const_missing(name) + end + constant + end + else + def constantize(camel_cased_word) #:nodoc: + names = camel_cased_word.split('::') + names.shift if names.empty? || names.first.empty? + + constant = Object + names.each do |name| + constant = constant.const_get(name, false) || constant.const_missing(name) + end + constant + end end # Turns a number into an ordinal string used to denote the position in an diff --git a/activesupport/test/inflector_test.rb b/activesupport/test/inflector_test.rb index 2c422ebe72..d8c93dc9ae 100644 --- a/activesupport/test/inflector_test.rb +++ b/activesupport/test/inflector_test.rb @@ -2,21 +2,8 @@ require 'abstract_unit' require 'inflector_test_cases' module Ace - module Extension - def self.included(base) - base.extend(ClassMethods) - end - - module ClassMethods - def mission_accomplished? - false - end - end - end - module Base class Case - include Extension end end end @@ -180,9 +167,7 @@ class InflectorTest < Test::Unit::TestCase end def test_constantize_does_lexical_lookup - assert_equal InflectorTest, ActiveSupport::Inflector.constantize("Ace::Base::InflectorTest") - assert_nothing_raised { Ace::Base::Case::ClassMethods } - assert_nothing_raised { assert_equal Ace::Base::Case::ClassMethods, ActiveSupport::Inflector.constantize("Ace::Base::Case::ClassMethods") } + assert_raises(NameError) { ActiveSupport::Inflector.constantize("Ace::Base::InflectorTest") } end def test_ordinal From 95c839bd2aca1576a33a6503f0dd67266d53a353 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 15 Dec 2008 20:33:21 -0600 Subject: [PATCH 036/188] Session objects are always a hash, so we need to ensure a flash hash is always assigned to the session --- actionpack/lib/action_controller/flash.rb | 55 +++++++++-------------- 1 file changed, 20 insertions(+), 35 deletions(-) diff --git a/actionpack/lib/action_controller/flash.rb b/actionpack/lib/action_controller/flash.rb index 62fa381a6f..6378a7f25b 100644 --- a/actionpack/lib/action_controller/flash.rb +++ b/actionpack/lib/action_controller/flash.rb @@ -31,51 +31,50 @@ module ActionController #:nodoc: alias_method_chain :reset_session, :flash end end - - + class FlashNow #:nodoc: def initialize(flash) @flash = flash end - + def []=(k, v) @flash[k] = v @flash.discard(k) v end - + def [](k) @flash[k] end end - + class FlashHash < Hash def initialize #:nodoc: super @used = {} end - + def []=(k, v) #:nodoc: keep(k) super end - + def update(h) #:nodoc: h.keys.each { |k| keep(k) } super end - + alias :merge! :update - + def replace(h) #:nodoc: @used = {} super end - + # Sets a flash that will not be available to the next action, only to the current. # # flash.now[:message] = "Hello current action" - # + # # This method enables you to use the flash as a central messaging system in your app. # When you need to pass an object to the next action, you use the standard flash assign ([]=). # When you need to pass an object to the current action, you use now, and your object will @@ -85,7 +84,7 @@ module ActionController #:nodoc: def now FlashNow.new(self) end - + # Keeps either the entire current flash or a specific flash entry available for the next action: # # flash.keep # keeps the entire flash @@ -93,7 +92,7 @@ module ActionController #:nodoc: def keep(k = nil) use(k, false) end - + # Marks the entire flash or a single flash entry to be discarded by the end of the current action: # # flash.discard # discard the entire flash at the end of the current action @@ -101,12 +100,12 @@ module ActionController #:nodoc: def discard(k = nil) use(k) end - + # Mark for removal entries that were kept, and delete unkept ones. # # This method is called automatically by filters, so you generally don't need to care about it. def sweep #:nodoc: - keys.each do |k| + keys.each do |k| unless @used[k] use(k) else @@ -118,7 +117,7 @@ module ActionController #:nodoc: # clean up after keys that could have been left over by calling reject! or shift on the flash (@used.keys - keys).each{ |k| @used.delete(k) } end - + private # Used internally by the keep and discard methods # use() # marks the entire flash as used @@ -139,32 +138,18 @@ module ActionController #:nodoc: def reset_session_with_flash reset_session_without_flash remove_instance_variable(:@_flash) - flash(:refresh) end - - # Access the contents of the flash. Use flash["notice"] to read a notice you put there or - # flash["notice"] = "hello" to put a new one. - # Note that if sessions are disabled only flash.now will work. - def flash(refresh = false) #:doc: - if !defined?(@_flash) || refresh - @_flash = - if session.is_a?(Hash) - # don't put flash in session if disabled - FlashHash.new - else - # otherwise, session is a CGI::Session or a TestSession - # so make sure it gets retrieved from/saved to session storage after request processing - session["flash"] ||= FlashHash.new - end - end - @_flash + # Access the contents of the flash. Use flash["notice"] to + # read a notice you put there or flash["notice"] = "hello" + # to put a new one. + def flash #:doc: + @_flash ||= session["flash"] ||= FlashHash.new end private def assign_shortcuts_with_flash(request, response) #:nodoc: assign_shortcuts_without_flash(request, response) - flash(:refresh) flash.sweep if @_session end end From 9a733f6c640cb3c4d474ecf44dd62ab73e351fd5 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 16 Dec 2008 00:04:04 -0600 Subject: [PATCH 037/188] Don't write nil values to default session options hash --- .../lib/action_controller/session_management.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_controller/session_management.rb b/actionpack/lib/action_controller/session_management.rb index dd5001d328..a9989d8198 100644 --- a/actionpack/lib/action_controller/session_management.rb +++ b/actionpack/lib/action_controller/session_management.rb @@ -17,9 +17,15 @@ module ActionController #:nodoc: cgi_options = ActionController::Base.session_options options = cgi_options.symbolize_keys options = DEFAULT_OPTIONS.merge(options) - options[:path] = options.delete(:session_path) - options[:key] = options.delete(:session_key) - options[:httponly] = options.delete(:session_http_only) + if options.has_key?(:session_path) + options[:path] = options.delete(:session_path) + end + if options.has_key?(:session_key) + options[:key] = options.delete(:session_key) + end + if options.has_key?(:session_http_only) + options[:httponly] = options.delete(:session_http_only) + end if store = ActionController::Base.session_store store.new(app, options) From 7c090509994faa19fcbd534aa78324b70b659627 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 16 Dec 2008 01:00:48 -0600 Subject: [PATCH 038/188] Lazy load flash access --- actionpack/lib/action_controller/flash.rb | 22 +++++++++++-------- .../lib/action_controller/test_process.rb | 2 +- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/actionpack/lib/action_controller/flash.rb b/actionpack/lib/action_controller/flash.rb index 6378a7f25b..9856dbed2a 100644 --- a/actionpack/lib/action_controller/flash.rb +++ b/actionpack/lib/action_controller/flash.rb @@ -27,8 +27,8 @@ module ActionController #:nodoc: def self.included(base) base.class_eval do include InstanceMethods - alias_method_chain :assign_shortcuts, :flash - alias_method_chain :reset_session, :flash + alias_method_chain :perform_action, :flash + alias_method_chain :reset_session, :flash end end @@ -135,22 +135,26 @@ module ActionController #:nodoc: module InstanceMethods #:nodoc: protected + def perform_action_with_flash + perform_action_without_flash + remove_instance_variable(:@_flash) if defined? @_flash + end + def reset_session_with_flash reset_session_without_flash - remove_instance_variable(:@_flash) + remove_instance_variable(:@_flash) if defined? @_flash end # Access the contents of the flash. Use flash["notice"] to # read a notice you put there or flash["notice"] = "hello" # to put a new one. def flash #:doc: - @_flash ||= session["flash"] ||= FlashHash.new - end + unless defined? @_flash + @_flash = session["flash"] ||= FlashHash.new + @_flash.sweep + end - private - def assign_shortcuts_with_flash(request, response) #:nodoc: - assign_shortcuts_without_flash(request, response) - flash.sweep if @_session + @_flash end end end diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index c35c3c07e8..c613d6b862 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -232,7 +232,7 @@ module ActionController #:nodoc: # Do we have a flash? def has_flash? - !session['flash'].empty? + !flash.empty? end # Do we have a flash that has contents? From 9e2b4a10f7f091868b3c3701efb4c04048455706 Mon Sep 17 00:00:00 2001 From: "Hongli Lai (Phusion" Date: Mon, 15 Dec 2008 21:36:33 +0100 Subject: [PATCH 039/188] Do not output an ETag header if response body is blank or when sending files with send_file(... :xsendfile => true) [#1578 state:committed] Signed-off-by: David Heinemeier Hansson --- actionpack/CHANGELOG | 2 ++ actionpack/lib/action_controller/response.rb | 6 +++++- actionpack/test/controller/render_test.rb | 9 +++++++++ actionpack/test/controller/send_file_test.rb | 1 + 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 352c4253f4..1584ae5835 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *2.3.0 [Edge]* +* Fixed that send_file shouldn't set an etag #1578 [Hongli Lai] + * Allow users to opt out of the spoofing checks in Request#remote_ip. Useful for sites whose traffic regularly triggers false positives. [Darren Boyd] * Deprecated formatted_polymorphic_url. [Jeremy Kemper] diff --git a/actionpack/lib/action_controller/response.rb b/actionpack/lib/action_controller/response.rb index 559c38efd0..4c37f09215 100644 --- a/actionpack/lib/action_controller/response.rb +++ b/actionpack/lib/action_controller/response.rb @@ -115,7 +115,11 @@ module ActionController # :nodoc: end def etag=(etag) - headers['ETag'] = %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(etag))}") + if etag.blank? + headers.delete('ETag') + else + headers['ETag'] = %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(etag))}") + end end def redirect(url, status) diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 87733c2d33..a6721fd903 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -208,6 +208,10 @@ class TestController < ActionController::Base def greeting # let's just rely on the template end + + def blank_response + render :text => ' ' + end def layout_test render :action => "hello_world" @@ -1380,6 +1384,11 @@ class EtagRenderTest < ActionController::TestCase @request.host = "www.nextangle.com" @expected_bang_etag = etag_for(expand_key([:foo, 123])) end + + def test_render_blank_body_shouldnt_set_etag + get :blank_response + assert !@response.etag? + end def test_render_200_should_set_etag get :render_hello_world_from_variable diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index c003abf094..ffbaa457f8 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -69,6 +69,7 @@ class SendFileTest < Test::Unit::TestCase assert_equal @controller.file_path, response.headers['X-Sendfile'] assert response.body.blank? + assert !response.etag? end def test_data From 46c7dd234807b2d24c8c742acb18c633b69e385d Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Wed, 10 Dec 2008 00:53:32 +0100 Subject: [PATCH 040/188] normalize author names in changelogs [#1495 state:committed] Signed-off-by: David Heinemeier Hansson --- actionmailer/CHANGELOG | 16 +- actionpack/CHANGELOG | 748 ++++++++++++++++++------------------- activerecord/CHANGELOG | 784 +++++++++++++++++++-------------------- activeresource/CHANGELOG | 42 +-- activesupport/CHANGELOG | 176 ++++----- railties/CHANGELOG | 390 +++++++++---------- 6 files changed, 1078 insertions(+), 1078 deletions(-) diff --git a/actionmailer/CHANGELOG b/actionmailer/CHANGELOG index 5dc7a33f55..64124e06a8 100644 --- a/actionmailer/CHANGELOG +++ b/actionmailer/CHANGELOG @@ -12,7 +12,7 @@ *2.2.0 [RC1] (October 24th, 2008)* -* Add layout functionality to mailers [Pratik] +* Add layout functionality to mailers [Pratik Naik] Mailer layouts behaves just like controller layouts, except layout names need to have '_mailer' postfix for them to be automatically picked up. @@ -24,7 +24,7 @@ * Less verbose mail logging: just recipients for :info log level; the whole email for :debug only. #8000 [iaddict, Tarmo Tänav] -* Updated TMail to version 1.2.1 [raasdnil] +* Updated TMail to version 1.2.1 [Mikel Lindsaar] * Fixed that you don't have to call super in ActionMailer::TestCase#setup #10406 [jamesgolick] @@ -36,7 +36,7 @@ *2.0.1* (December 7th, 2007) -* Update ActionMailer so it treats ActionView the same way that ActionController does. Closes #10244 [rick] +* Update ActionMailer so it treats ActionView the same way that ActionController does. Closes #10244 [Rick Olson] * Pass the template_root as an array as ActionView's view_path * Request templates with the "#{mailer_name}/#{action}" as opposed to just "#{action}" @@ -45,11 +45,11 @@ * Update README to use new smtp settings configuration API. Closes #10060 [psq] -* Allow ActionMailer subclasses to individually set their delivery method (so two subclasses can have different delivery methods) #10033 [zdennis] +* Allow ActionMailer subclasses to individually set their delivery method (so two subclasses can have different delivery methods) #10033 [Zach Dennis] -* Update TMail to v1.1.0. Use an updated version of TMail if available. [mikel] +* Update TMail to v1.1.0. Use an updated version of TMail if available. [Mikel Lindsaar] -* Introduce a new base test class for testing Mailers. ActionMailer::TestCase [Koz] +* Introduce a new base test class for testing Mailers. ActionMailer::TestCase [Michael Koziarski] * Fix silent failure of rxml templates. #9879 [jstewart] @@ -84,7 +84,7 @@ *1.3.2* (February 5th, 2007) -* Deprecate server_settings renaming it to smtp_settings, add sendmail_settings to allow you to override the arguments to and location of the sendmail executable. [Koz] +* Deprecate server_settings renaming it to smtp_settings, add sendmail_settings to allow you to override the arguments to and location of the sendmail executable. [Michael Koziarski] *1.3.1* (January 16th, 2007) @@ -104,7 +104,7 @@ * Tighten rescue clauses. #5985 [james@grayproductions.net] -* Automatically included ActionController::UrlWriter, such that URL generation can happen within ActionMailer controllers. [DHH] +* Automatically included ActionController::UrlWriter, such that URL generation can happen within ActionMailer controllers. [David Heinemeier Hansson] * Replace Reloadable with Reloadable::Deprecated. [Nicholas Seckar] diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 1584ae5835..639cf14cd1 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -6,19 +6,19 @@ * Deprecated formatted_polymorphic_url. [Jeremy Kemper] -* Added the option to declare an asset_host as an object that responds to call (see http://github.com/dhh/asset-hosting-with-minimum-ssl for an example) [DHH] +* Added the option to declare an asset_host as an object that responds to call (see http://github.com/dhh/asset-hosting-with-minimum-ssl for an example) [David Heinemeier Hansson] -* Added support for multiple routes.rb files (useful for plugin engines). This also means that draw will no longer clear the route set, you have to do that by hand (shouldn't make a difference to you unless you're doing some funky stuff) [DHH] +* Added support for multiple routes.rb files (useful for plugin engines). This also means that draw will no longer clear the route set, you have to do that by hand (shouldn't make a difference to you unless you're doing some funky stuff) [David Heinemeier Hansson] * Dropped formatted_* routes in favor of just passing in :format as an option. This cuts resource routes generation in half #1359 [aaronbatalion] -* Remove support for old double-encoded cookies from the cookie store. These values haven't been generated since before 2.1.0, and any users who have visited the app in the intervening 6 months will have had their cookie upgraded. [Koz] +* Remove support for old double-encoded cookies from the cookie store. These values haven't been generated since before 2.1.0, and any users who have visited the app in the intervening 6 months will have had their cookie upgraded. [Michael Koziarski] * Allow helpers directory to be overridden via ActionController::Base.helpers_dir #1424 [Sam Pohlenz] * Remove deprecated ActionController::Base#assign_default_content_type_and_charset -* Changed the default of ActionView#render to assume partials instead of files when not given an options hash [DHH]. Examples: +* Changed the default of ActionView#render to assume partials instead of files when not given an options hash [David Heinemeier Hansson]. Examples: # Instead of <%= render :partial => "account" %> <%= render "account" %> @@ -34,9 +34,9 @@ # <%= render :partial => "posts/post", :collection => @posts %> <%= render(@posts) %> -* Remove deprecated render_component. Please use the plugin from http://github.com/rails/render_component/tree/master [Pratik] +* Remove deprecated render_component. Please use the plugin from http://github.com/rails/render_component/tree/master [Pratik Naik] -* Fixed RedCloth and BlueCloth shouldn't preload. Instead just assume that they're available if you want to use textilize and markdown and let autoload require them [DHH] +* Fixed RedCloth and BlueCloth shouldn't preload. Instead just assume that they're available if you want to use textilize and markdown and let autoload require them [David Heinemeier Hansson] *2.2.2 (November 21st, 2008)* @@ -53,7 +53,7 @@ product.resources :images, :except => :destroy end -* Added render :js for people who want to render inline JavaScript replies without using RJS [DHH] +* Added render :js for people who want to render inline JavaScript replies without using RJS [David Heinemeier Hansson] * Fixed that polymorphic_url should compact given array #1317 [hiroshi] @@ -63,9 +63,9 @@ * Fix regression bug that made date_select and datetime_select raise a Null Pointer Exception when a nil date/datetime was passed and only month and year were displayed #1289 [Bernardo Padua/Tor Erik] -* Simplified the logging format for parameters (don't include controller, action, and format as duplicates) [DHH] +* Simplified the logging format for parameters (don't include controller, action, and format as duplicates) [David Heinemeier Hansson] -* Remove the logging of the Session ID when the session store is CookieStore [DHH] +* Remove the logging of the Session ID when the session store is CookieStore [David Heinemeier Hansson] * Fixed regex in redirect_to to fully support URI schemes #1247 [Seth Fitzsimmons] @@ -76,7 +76,7 @@ * Fix incorrect closing CDATA delimiter and that HTML::Node.parse would blow up on unclosed CDATA sections [packagethief] -* Added stale? and fresh_when methods to provide a layer of abstraction above request.fresh? and friends [DHH]. Example: +* Added stale? and fresh_when methods to provide a layer of abstraction above request.fresh? and friends [David Heinemeier Hansson]. Example: class ArticlesController < ApplicationController def show_with_respond_to_block @@ -126,13 +126,13 @@ * Fixed FormTagHelper#submit_tag with :disable_with option wouldn't submit the button's value when was clicked #633 [Jose Fernandez] -* Stopped logging template compiles as it only clogs up the log [DHH] +* Stopped logging template compiles as it only clogs up the log [David Heinemeier Hansson] -* Changed the X-Runtime header to report in milliseconds [DHH] +* Changed the X-Runtime header to report in milliseconds [David Heinemeier Hansson] -* Changed BenchmarkHelper#benchmark to report in milliseconds [DHH] +* Changed BenchmarkHelper#benchmark to report in milliseconds [David Heinemeier Hansson] -* Changed logging format to be millisecond based and skip misleading stats [DHH]. Went from: +* Changed logging format to be millisecond based and skip misleading stats [David Heinemeier Hansson]. Went from: Completed in 0.10000 (4 reqs/sec) | Rendering: 0.04000 (40%) | DB: 0.00400 (4%) | 200 OK [http://example.com] @@ -156,7 +156,7 @@ * Added button_to_remote helper. #3641 [Donald Piret, Tarmo Tänav] -* Deprecate render_component. Please use render_component plugin from http://github.com/rails/render_component/tree/master [Pratik] +* Deprecate render_component. Please use render_component plugin from http://github.com/rails/render_component/tree/master [Pratik Naik] * Routes may be restricted to lists of HTTP methods instead of a single method or :any. #407 [Brennan Dunn, Gaius Centus Novus] map.resource :posts, :collection => { :search => [:get, :post] } @@ -190,7 +190,7 @@ * All 2xx requests are considered successful [Josh Peek] -* Fixed that AssetTagHelper#compute_public_path shouldn't cache the asset_host along with the source or per-request proc's won't run [DHH] +* Fixed that AssetTagHelper#compute_public_path shouldn't cache the asset_host along with the source or per-request proc's won't run [David Heinemeier Hansson] * Removed config.action_view.cache_template_loading, use config.cache_classes instead [Josh Peek] @@ -253,7 +253,7 @@ * Replaced TemplateFinder abstraction with ViewLoadPaths [Josh Peek] -* Added block-call style to link_to [Sam Stephenson/DHH]. Example: +* Added block-call style to link_to [Sam Stephenson/David Heinemeier Hansson]. Example: <% link_to(@profile) do %> <%= @profile.name %> -- Check it out!! @@ -284,30 +284,30 @@ * Added session(:on) to turn session management back on in a controller subclass if the superclass turned it off (Peter Jones) [#136] -* Change the request forgery protection to go by Content-Type instead of request.format so that you can't bypass it by POSTing to "#{request.uri}.xml" [rick] +* Change the request forgery protection to go by Content-Type instead of request.format so that you can't bypass it by POSTing to "#{request.uri}.xml" [Rick Olson] * InstanceTag#default_time_from_options with hash args uses Time.current as default; respects hash settings when time falls in system local spring DST gap [Geoff Buesing] * select_date defaults to Time.zone.today when config.time_zone is set [Geoff Buesing] * Fixed that TextHelper#text_field would corrypt when raw HTML was used as the value (mchenryc, Kevin Glowacz) [#80] -* Added ActionController::TestCase#rescue_action_in_public! to control whether the action under test should use the regular rescue_action path instead of simply raising the exception inline (great for error testing) [DHH] +* Added ActionController::TestCase#rescue_action_in_public! to control whether the action under test should use the regular rescue_action path instead of simply raising the exception inline (great for error testing) [David Heinemeier Hansson] -* Reduce number of instance variables being copied from controller to view. [Pratik] +* Reduce number of instance variables being copied from controller to view. [Pratik Naik] * select_datetime and select_time default to Time.zone.now when config.time_zone is set [Geoff Buesing] * datetime_select defaults to Time.zone.now when config.time_zone is set [Geoff Buesing] -* Remove ActionController::Base#view_controller_internals flag. [Pratik] +* Remove ActionController::Base#view_controller_internals flag. [Pratik Naik] * Add conditional options to caches_page method. [Paul Horsfall] -* Move missing template logic to ActionView. [Pratik] +* Move missing template logic to ActionView. [Pratik Naik] -* Introduce ActionView::InlineTemplate class. [Pratik] +* Introduce ActionView::InlineTemplate class. [Pratik Naik] -* Automatically parse posted JSON content for Mime::JSON requests. [rick] +* Automatically parse posted JSON content for Mime::JSON requests. [Rick Olson] POST /posts {"post": {"title": "Breaking News"}} @@ -317,14 +317,14 @@ # ... end -* add json_escape ERB util to escape html entities in json strings that are output in HTML pages. [rick] +* add json_escape ERB util to escape html entities in json strings that are output in HTML pages. [Rick Olson] * Provide a helper proxy to access helper methods from outside views. Closes #10839 [Josh Peek] e.g. ApplicationController.helpers.simple_format(text) * Improve documentation. [Xavier Noria, leethal, jerome] -* Ensure RJS redirect_to doesn't html-escapes string argument. Closes #8546 [josh, eventualbuddha, Pratik] +* Ensure RJS redirect_to doesn't html-escapes string argument. Closes #8546 [Josh Peek, eventualbuddha, Pratik Naik] * Support render :partial => collection of heterogeneous elements. #11491 [Zach Dennis] @@ -336,17 +336,17 @@ * Fixed HTML::Tokenizer (used in sanitize helper) didn't handle unclosed CDATA tags #10071 [esad, packagethief] -* Improve documentation. [Radar, Jan De Poorter, chuyeow, xaviershay, danger, miloops, Xavier Noria, Sunny Ripert] +* Improve documentation. [Ryan Bigg, Jan De Poorter, Cheah Chu Yeow, Xavier Shay, Jack Danger Canty, Emilio Tagua, Xavier Noria, Sunny Ripert] * Fixed that FormHelper#radio_button would produce invalid ids #11298 [harlancrystal] -* Added :confirm option to submit_tag #11415 [miloops] +* Added :confirm option to submit_tag #11415 [Emilio Tagua] * Fixed NumberHelper#number_with_precision to properly round in a way that works equally on Mac, Windows, Linux (closes #11409, #8275, #10090, #8027) [zhangyuanyi] -* Allow the #simple_format text_helper to take an html_options hash for each paragraph. #2448 [Francois Beausoleil, thechrisoshow] +* Allow the #simple_format text_helper to take an html_options hash for each paragraph. #2448 [François Beausoleil, Chris O'Sullivan] -* Fix regression from filter refactoring where re-adding a skipped filter resulted in it being called twice. [rick] +* Fix regression from filter refactoring where re-adding a skipped filter resulted in it being called twice. [Rick Olson] * Refactor filters to use Active Support callbacks. #11235 [Josh Peek] @@ -362,43 +362,43 @@ * Fix nested parameter hash parsing bug. #10797 [thomas.lee] -* Allow using named routes in ActionController::TestCase before any request has been made. Closes #11273 [alloy] +* Allow using named routes in ActionController::TestCase before any request has been made. Closes #11273 [Eloy Duran] -* Fixed that sweepers defined by cache_sweeper will be added regardless of the perform_caching setting. Instead, control whether the sweeper should be run with the perform_caching setting. This makes testing easier when you want to turn perform_caching on/off [DHH] +* Fixed that sweepers defined by cache_sweeper will be added regardless of the perform_caching setting. Instead, control whether the sweeper should be run with the perform_caching setting. This makes testing easier when you want to turn perform_caching on/off [David Heinemeier Hansson] * Make MimeResponds::Responder#any work without explicit types. Closes #11140 [jaw6] * Better error message for type conflicts when parsing params. Closes #7962 [spicycode, matt] -* Remove unused ActionController::Base.template_class. Closes #10787 [Pratik] +* Remove unused ActionController::Base.template_class. Closes #10787 [Pratik Naik] -* Moved template handlers related code from ActionView::Base to ActionView::Template. [Pratik] +* Moved template handlers related code from ActionView::Base to ActionView::Template. [Pratik Naik] -* Tests for div_for and content_tag_for helpers. Closes #11223 [thechrisoshow] +* Tests for div_for and content_tag_for helpers. Closes #11223 [Chris O'Sullivan] * Allow file uploads in Integration Tests. Closes #11091 [RubyRedRick] -* Refactor partial rendering into a PartialTemplate class. [Pratik] +* Refactor partial rendering into a PartialTemplate class. [Pratik Naik] -* Added that requests with JavaScript as the priority mime type in the accept header and no format extension in the parameters will be treated as though their format was :js when it comes to determining which template to render. This makes it possible for JS requests to automatically render action.js.rjs files without an explicit respond_to block [DHH] +* Added that requests with JavaScript as the priority mime type in the accept header and no format extension in the parameters will be treated as though their format was :js when it comes to determining which template to render. This makes it possible for JS requests to automatically render action.js.rjs files without an explicit respond_to block [David Heinemeier Hansson] -* Tests for distance_of_time_in_words with TimeWithZone instances. Closes #10914 [ernesto.jimenez] +* Tests for distance_of_time_in_words with TimeWithZone instances. Closes #10914 [Ernesto Jimenez] * Remove support for multivalued (e.g., '&'-delimited) cookies. [Jamis Buck] * Fix problem with render :partial collections, records, and locals. #11057 [lotswholetime] -* Added support for naming concrete classes in sweeper declarations [DHH] +* Added support for naming concrete classes in sweeper declarations [David Heinemeier Hansson] -* Remove ERB trim variables from trace template in case ActionView::Base.erb_trim_mode is changed in the application. #10098 [tpope, kampers] +* Remove ERB trim variables from trace template in case ActionView::Base.erb_trim_mode is changed in the application. #10098 [Tim Pope, Chris Kampmeier] -* Fix typo in form_helper documentation. #10650 [xaviershay, kampers] +* Fix typo in form_helper documentation. #10650 [Xavier Shay, Chris Kampmeier] * Fix bug with setting Request#format= after the getter has cached the value. #10889 [cch1] -* Correct inconsistencies in RequestForgeryProtection docs. #11032 [mislav] +* Correct inconsistencies in RequestForgeryProtection docs. #11032 [Mislav Marohnić] -* Introduce a Template class to ActionView. #11024 [lifofifo] +* Introduce a Template class to ActionView. #11024 [Pratik Naik] * Introduce the :index option for form_for and fields_for to simplify multi-model forms (see http://railscasts.com/episodes/75). #9883 [rmm5t] @@ -414,7 +414,7 @@ e.g. map.dashboard '/dashboard', :controller=>'dashboard' map.root :dashboard -* Handle corner case with image_tag when passed 'messed up' image names. #9018 [duncanbeevers, mpalmer] +* Handle corner case with image_tag when passed 'messed up' image names. #9018 [Duncan Beevers, mpalmer] * Add label_tag helper for generating elements. #10802 [DefV] @@ -422,15 +422,15 @@ * Performance: optimize route recognition. Large speedup for apps with many resource routes. #10835 [oleganza] -* Make render :partial recognise form builders and use the _form partial. #10814 [djanowski] +* Make render :partial recognise form builders and use the _form partial. #10814 [Damian Janowski] * Allow users to declare other namespaces when using the atom feed helpers. #10304 [david.calavera] * Introduce send_file :x_sendfile => true to send an X-Sendfile response header. [Jeremy Kemper] -* Fixed ActionView::Helpers::ActiveRecordHelper::form for when protect_from_forgery is used #10739 [jeremyevans] +* Fixed ActionView::Helpers::ActiveRecordHelper::form for when protect_from_forgery is used #10739 [Jeremy Evans] -* Provide nicer access to HTTP Headers. Instead of request.env["HTTP_REFERRER"] you can now use request.headers["Referrer"]. [Koz] +* Provide nicer access to HTTP Headers. Instead of request.env["HTTP_REFERRER"] you can now use request.headers["Referrer"]. [Michael Koziarski] * UrlWriter respects relative_url_root. #10748 [Cheah Chu Yeow] @@ -440,26 +440,26 @@ * assert_response failures include the exception message. #10688 [Seth Rasmussen] -* All fragment cache keys are now by default prefixed with the "views/" namespace [DHH] +* All fragment cache keys are now by default prefixed with the "views/" namespace [David Heinemeier Hansson] -* Moved the caching stores from ActionController::Caching::Fragments::* to ActiveSupport::Cache::*. If you're explicitly referring to a store, like ActionController::Caching::Fragments::MemoryStore, you need to update that reference with ActiveSupport::Cache::MemoryStore [DHH] +* Moved the caching stores from ActionController::Caching::Fragments::* to ActiveSupport::Cache::*. If you're explicitly referring to a store, like ActionController::Caching::Fragments::MemoryStore, you need to update that reference with ActiveSupport::Cache::MemoryStore [David Heinemeier Hansson] -* Deprecated ActionController::Base.fragment_cache_store for ActionController::Base.cache_store [DHH] +* Deprecated ActionController::Base.fragment_cache_store for ActionController::Base.cache_store [David Heinemeier Hansson] -* Made fragment caching in views work for rjs and builder as well #6642 [zsombor] +* Made fragment caching in views work for rjs and builder as well #6642 [Dee Zsombor] * Fixed rendering of partials with layout when done from site layout #9209 [antramm] -* Fix atom_feed_helper to comply with the atom spec. Closes #10672 [xaviershay] +* Fix atom_feed_helper to comply with the atom spec. Closes #10672 [Xavier Shay] * The tags created do not contain a date (http://feedvalidator.org/docs/error/InvalidTAG.html) * IDs are not guaranteed unique * A default self link was not provided, contrary to the documentation * NOTE: This changes tags for existing atom entries, but at least they validate now. -* Correct indentation in tests. Closes #10671 [l.guidi] +* Correct indentation in tests. Closes #10671 [Luca Guidi] -* Fix that auto_link looks for ='s in url paths (Amazon urls have them). Closes #10640 [bgreenlee] +* Fix that auto_link looks for ='s in url paths (Amazon urls have them). Closes #10640 [Brad Greenlee] * Ensure that test case setup is run even if overridden. #10382 [Josh Peek] @@ -476,7 +476,7 @@ * Added OPTIONS to list of default accepted HTTP methods #10449 [holoway] -* Added option to pass proc to ActionController::Base.asset_host for maximum configurability #10521 [chuyeow]. Example: +* Added option to pass proc to ActionController::Base.asset_host for maximum configurability #10521 [Cheah Chu Yeow]. Example: ActionController::Base.asset_host = Proc.new { |source| if source.starts_with?('/images') @@ -505,45 +505,45 @@ * Fixed send_file/binary_content for testing #8044 [tolsen] -* When a NonInferrableControllerError is raised, make the proposed fix clearer in the error message. Closes #10199 [danger] +* When a NonInferrableControllerError is raised, make the proposed fix clearer in the error message. Closes #10199 [Jack Danger Canty] * Update Prototype to 1.6.0.1. [sam] * Update script.aculo.us to 1.8.0.1. [madrobby] -* Add 'disabled' attribute to