From cdf60e46cc01e5f7b14e95a0b7d914516fcdcbc1 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 1 Aug 2009 18:15:52 -0700 Subject: [PATCH 01/38] SQLite: drop support for 'dbfile' option in favor of 'database.' --- activerecord/CHANGELOG | 2 ++ .../lib/active_record/connection_adapters/sqlite_adapter.rb | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 659de99873..5515cf6b1a 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* SQLite: drop support for 'dbfile' option in favor of 'database.' #2363 [Paul Hinze, Jeremy Kemper] + * Added :primary_key option to belongs_to associations. #765 [Szymon Nowak, Philip Hallstrom, Noel Rocha] # employees.company_name references companies.name Employee.belongs_to :company, :primary_key => 'name', :foreign_key => 'company_name' diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 5e5e30776a..c0f5046bff 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -27,7 +27,6 @@ module ActiveRecord private def parse_sqlite_config!(config) - config[:database] ||= config[:dbfile] # Require database. unless config[:database] raise ArgumentError, "No database file specified. Missing argument: database" From a606727606cc0725a39748dd9d310b2b064e3ca7 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 1 Aug 2009 18:34:41 -0700 Subject: [PATCH 02/38] Extract String#bytesize shim --- .../lib/active_support/core_ext/string/bytesize.rb | 5 +++++ .../lib/active_support/core_ext/string/interpolation.rb | 5 ++--- activesupport/test/core_ext/string_ext_test.rb | 7 +++++++ 3 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 activesupport/lib/active_support/core_ext/string/bytesize.rb diff --git a/activesupport/lib/active_support/core_ext/string/bytesize.rb b/activesupport/lib/active_support/core_ext/string/bytesize.rb new file mode 100644 index 0000000000..ed051b921e --- /dev/null +++ b/activesupport/lib/active_support/core_ext/string/bytesize.rb @@ -0,0 +1,5 @@ +unless '1.9'.respond_to?(:bytesize) + class String + alias :bytesize :size + end +end diff --git a/activesupport/lib/active_support/core_ext/string/interpolation.rb b/activesupport/lib/active_support/core_ext/string/interpolation.rb index d459c03d39..d9159b690a 100644 --- a/activesupport/lib/active_support/core_ext/string/interpolation.rb +++ b/activesupport/lib/active_support/core_ext/string/interpolation.rb @@ -6,6 +6,7 @@ =end if RUBY_VERSION < '1.9' + require 'active_support/core_ext/string/bytesize' # KeyError is raised by String#% when the string contains a named placeholder # that is not contained in the given arguments hash. Ruby 1.9 includes and @@ -24,8 +25,6 @@ if RUBY_VERSION < '1.9' # the meaning of the msgids using "named argument" instead of %s/%d style. class String - # For older ruby versions, such as ruby-1.8.5 - alias :bytesize :size unless instance_methods.find {|m| m.to_s == 'bytesize'} alias :interpolate_without_ruby_19_syntax :% # :nodoc: INTERPOLATION_PATTERN = Regexp.union( @@ -90,4 +89,4 @@ if RUBY_VERSION < '1.9' end end end -end \ No newline at end of file +end diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb index a23d3f6fef..1005a7e7ad 100644 --- a/activesupport/test/core_ext/string_ext_test.rb +++ b/activesupport/test/core_ext/string_ext_test.rb @@ -345,3 +345,10 @@ class TestGetTextString < Test::Unit::TestCase assert_raises(ArgumentError) { "%{name} %f" % [1.0, 2.0] } end end + +class StringBytesizeTest < Test::Unit::TestCase + def test_bytesize + assert_respond_to 'foo', :bytesize + assert_equal 3, 'foo'.bytesize + end +end From ec94c2550dae463e646a18316bfcdaded9d140c9 Mon Sep 17 00:00:00 2001 From: Sava Chankov Date: Sat, 1 Aug 2009 19:38:05 -0700 Subject: [PATCH 03/38] Ruby 1.9: fix Content-Length for multibyte send_data streaming [#2661 state:resolved] Signed-off-by: Jeremy Kemper --- actionpack/CHANGELOG | 2 ++ actionpack/lib/action_controller/base/streaming.rb | 4 +++- actionpack/test/controller/send_file_test.rb | 12 ++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index e758983d7a..aab26b411c 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Ruby 1.9: fix Content-Length for multibyte send_data streaming. #2661 [Sava Chankov] + * Ruby 1.9: ERB template encoding using a magic comment at the top of the file. [Jeremy Kemper] <%# encoding: utf-8 %> diff --git a/actionpack/lib/action_controller/base/streaming.rb b/actionpack/lib/action_controller/base/streaming.rb index 9ff4f25f43..f52810ff3a 100644 --- a/actionpack/lib/action_controller/base/streaming.rb +++ b/actionpack/lib/action_controller/base/streaming.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/string/bytesize' + module ActionController #:nodoc: # Methods for sending arbitrary data and for streaming files to the browser, # instead of rendering. @@ -142,7 +144,7 @@ module ActionController #:nodoc: # instead. See ActionController::Base#render for more information. def send_data(data, options = {}) #:doc: logger.info "Sending data #{options[:filename]}" if logger - send_file_headers! options.merge(:length => data.size) + send_file_headers! options.merge(:length => data.bytesize) @performed_render = false render :status => options[:status], :text => data end diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index ae32ee5649..154daad1b9 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -1,3 +1,4 @@ +# encoding: utf-8 require 'abstract_unit' module TestFileUtils @@ -22,6 +23,10 @@ class SendFileController < ActionController::Base def data send_data(file_data, options) end + + def multibyte_text_data + send_data("Кирилица\n祝您好運", options) + end end class SendFileTest < ActionController::TestCase @@ -163,4 +168,11 @@ class SendFileTest < ActionController::TestCase assert_equal 200, @response.status end end + + def test_send_data_content_length_header + @controller.headers = {} + @controller.options = { :type => :text, :filename => 'file_with_utf8_text' } + process('multibyte_text_data') + assert_equal '29', @controller.headers['Content-Length'] + end end From f2a35723c8876697d5a7ebfdf329cee54d8a39ac Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 1 Aug 2009 20:07:28 -0700 Subject: [PATCH 04/38] Ruby 1.9: fix encoding for test_file_stream --- actionpack/test/controller/send_file_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index 154daad1b9..f0c723eaa2 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -4,7 +4,7 @@ require 'abstract_unit' module TestFileUtils def file_name() File.basename(__FILE__) end def file_path() File.expand_path(__FILE__) end - def file_data() File.open(file_path, 'rb') { |f| f.read } end + def file_data() @data ||= File.open(file_path, 'rb') { |f| f.read } end end class SendFileController < ActionController::Base @@ -60,6 +60,7 @@ class SendFileTest < ActionController::TestCase require 'stringio' output = StringIO.new output.binmode + output.string.force_encoding(file_data.encoding) if output.string.respond_to?(:force_encoding) assert_nothing_raised { response.body_parts.each { |part| output << part.to_s } } assert_equal file_data, output.string end From 503ce1d01ce6c8eee9818f4e76a9f880bb1a291d Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 30 Jul 2009 21:00:39 -0700 Subject: [PATCH 05/38] Update cache_control to be a Hash of options that is used to build the header. * Significantly simplifies setting and modifying cache control in other areas --- .../action_controller/base/conditional_get.rb | 23 ++++-------------- .../lib/action_controller/base/streaming.rb | 2 +- .../lib/action_controller/testing/process.rb | 2 +- .../lib/action_dispatch/http/response.rb | 24 +++++++++++++++---- actionpack/test/controller/render_test.rb | 2 +- actionpack/test/controller/send_file_test.rb | 2 +- actionpack/test/dispatch/response_test.rb | 4 ++-- 7 files changed, 29 insertions(+), 30 deletions(-) diff --git a/actionpack/lib/action_controller/base/conditional_get.rb b/actionpack/lib/action_controller/base/conditional_get.rb index d287ec4994..6d35137428 100644 --- a/actionpack/lib/action_controller/base/conditional_get.rb +++ b/actionpack/lib/action_controller/base/conditional_get.rb @@ -29,11 +29,7 @@ module ActionController response.last_modified = options[:last_modified] if options[:last_modified] if options[:public] - cache_control = response.headers["Cache-Control"].split(",").map {|k| k.strip } - cache_control.delete("private") - cache_control.delete("no-cache") - cache_control << "public" - response.headers["Cache-Control"] = cache_control.join(', ') + response.cache_control[:public] = true end if request.fresh?(response) @@ -107,21 +103,10 @@ module ActionController # This method will overwrite an existing Cache-Control header. # See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html for more possibilities. def expires_in(seconds, options = {}) #:doc: - cache_control = response.headers["Cache-Control"].split(",").map {|k| k.strip } + response.cache_control.merge!(:max_age => seconds, :public => options.delete(:public)) + options.delete(:private) - cache_control << "max-age=#{seconds}" - cache_control.delete("no-cache") - if options[:public] - cache_control.delete("private") - cache_control << "public" - else - cache_control << "private" - end - - # This allows for additional headers to be passed through like 'max-stale' => 5.hours - cache_control += options.symbolize_keys.reject{|k,v| k == :public || k == :private }.map{ |k,v| v == true ? k.to_s : "#{k.to_s}=#{v.to_s}"} - - response.headers["Cache-Control"] = cache_control.join(', ') + response.cache_control[:extras] = options.map {|k,v| "#{k}=#{v}"} end # Sets a HTTP 1.1 Cache-Control header of "no-cache" so no caching should occur by the browser or diff --git a/actionpack/lib/action_controller/base/streaming.rb b/actionpack/lib/action_controller/base/streaming.rb index f52810ff3a..f0317c6e99 100644 --- a/actionpack/lib/action_controller/base/streaming.rb +++ b/actionpack/lib/action_controller/base/streaming.rb @@ -181,7 +181,7 @@ module ActionController #:nodoc: # after it displays the "open/save" dialog, which means that if you # hit "open" the file isn't there anymore when the application that # is called for handling the download is run, so let's workaround that - headers['Cache-Control'] = 'private' if headers['Cache-Control'] == 'no-cache' + response.cache_control[:public] ||= false end end end diff --git a/actionpack/lib/action_controller/testing/process.rb b/actionpack/lib/action_controller/testing/process.rb index e7c64d0942..d32d5562e8 100644 --- a/actionpack/lib/action_controller/testing/process.rb +++ b/actionpack/lib/action_controller/testing/process.rb @@ -52,7 +52,7 @@ module ActionController #:nodoc: class TestResponse < ActionDispatch::TestResponse def recycle! @status = 200 - @header = Rack::Utils::HeaderHash.new(DEFAULT_HEADERS) + @header = Rack::Utils::HeaderHash.new @writer = lambda { |x| @body << x } @block = nil @length = 0 diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index e58b4b5e19..32cfb5ae44 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -32,8 +32,8 @@ module ActionDispatch # :nodoc: # end # end class Response < Rack::Response - DEFAULT_HEADERS = { "Cache-Control" => "no-cache" } attr_accessor :request + attr_reader :cache_control attr_writer :header alias_method :headers=, :header= @@ -42,7 +42,8 @@ module ActionDispatch # :nodoc: def initialize super - @header = Rack::Utils::HeaderHash.new(DEFAULT_HEADERS) + @cache_control = {} + @header = Rack::Utils::HeaderHash.new end # The response code of the request @@ -196,7 +197,7 @@ module ActionDispatch # :nodoc: private def handle_conditional_get! - if etag? || last_modified? + if etag? || last_modified? || !cache_control.empty? set_conditional_cache_control! elsif nonempty_ok_response? self.etag = body @@ -207,6 +208,8 @@ module ActionDispatch # :nodoc: end set_conditional_cache_control! + else + headers["Cache-Control"] = "no-cache" end end @@ -220,9 +223,20 @@ module ActionDispatch # :nodoc: end def set_conditional_cache_control! - if headers['Cache-Control'] == DEFAULT_HEADERS['Cache-Control'] - headers['Cache-Control'] = 'private, max-age=0, must-revalidate' + if cache_control.empty? + cache_control.merge!(:public => false, :max_age => 0, :must_revalidate => true) end + + public_cache, max_age, must_revalidate, extras = + cache_control.values_at(:public, :max_age, :must_revalidate, :extras) + + options = [] + options << "max-age=#{max_age}" if max_age + options << (public_cache ? "public" : "private") + options << "must-revalidate" if must_revalidate + options.concat(extras) if extras + + headers["Cache-Control"] = options.join(", ") end def convert_content_type! diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index acb0c895e0..d0fa67c945 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -1331,7 +1331,7 @@ class EtagRenderTest < ActionController::TestCase def test_render_200_should_set_etag get :render_hello_world_from_variable assert_equal etag_for("hello david"), @response.headers['ETag'] - assert_equal "private, max-age=0, must-revalidate", @response.headers['Cache-Control'] + assert_equal "max-age=0, private, must-revalidate", @response.headers['Cache-Control'] end def test_render_against_etag_request_should_304_when_match diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index f0c723eaa2..0afebac68c 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -129,7 +129,7 @@ class SendFileTest < ActionController::TestCase # test overriding Cache-Control: no-cache header to fix IE open/save dialog @controller.headers = { 'Cache-Control' => 'no-cache' } @controller.send(:send_file_headers!, options) - h = @controller.headers + @controller.response.prepare! assert_equal 'private', h['Cache-Control'] end diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 2ddc6cb2b5..2b1540c678 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -13,7 +13,7 @@ class ResponseTest < ActiveSupport::TestCase assert_equal 200, status assert_equal({ "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "private, max-age=0, must-revalidate", + "Cache-Control" => "max-age=0, private, must-revalidate", "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"', "Set-Cookie" => "", "Content-Length" => "13" @@ -32,7 +32,7 @@ class ResponseTest < ActiveSupport::TestCase assert_equal 200, status assert_equal({ "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "private, max-age=0, must-revalidate", + "Cache-Control" => "max-age=0, private, must-revalidate", "ETag" => '"ebb5e89e8a94e9dd22abf5d915d112b2"', "Set-Cookie" => "", "Content-Length" => "8" From b53f00690173797a39ff46e55dd25c20581c3d00 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 30 Jul 2009 21:32:24 -0700 Subject: [PATCH 06/38] Remove legacy processing and content_length * convert_content_type! is handled by assign_default_content_type_and_charset! * set_content_length! should be handled by the endpoint server. Otherwise each middleware that modifies the body has to do the expensive work of recalculating content_length. * convert_language! appears to be legacy. There are no tests for this * convert_cookies! should be handled by the new HeaderHash in Rack * Use an integer for .status's internal representation to avoid needing to do String manipulation just to find out the status --- .../lib/action_dispatch/http/response.rb | 50 ++++--------------- .../request/multipart_params_parsing_test.rb | 2 - actionpack/test/dispatch/response_test.rb | 6 +-- actionpack/test/new_base/base_test.rb | 3 -- 4 files changed, 12 insertions(+), 49 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 32cfb5ae44..03d1780b77 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -46,18 +46,22 @@ module ActionDispatch # :nodoc: @header = Rack::Utils::HeaderHash.new end + def status=(status) + @status = status.to_i + end + # The response code of the request def response_code - status.to_s[0,3].to_i rescue 0 + @status end # Returns a String to ensure compatibility with Net::HTTPResponse def code - status.to_s.split(' ')[0] + @status.to_s end def message - status.to_s.split(' ',2)[1] || StatusCodes::STATUS_CODES[response_code] + StatusCodes::STATUS_CODES[@status] end alias_method :status_message, :message @@ -143,10 +147,7 @@ module ActionDispatch # :nodoc: def prepare! assign_default_content_type_and_charset! handle_conditional_get! - set_content_length! - convert_content_type! - convert_language! - convert_cookies! + self["Set-Cookie"] ||= "" end def each(&callback) @@ -203,7 +204,7 @@ module ActionDispatch # :nodoc: self.etag = body if request && request.etag_matches?(etag) - self.status = '304 Not Modified' + self.status = 304 self.body = [] end @@ -214,7 +215,7 @@ module ActionDispatch # :nodoc: end def nonempty_ok_response? - ok = !status || status.to_s[0..2] == '200' + ok = !@status || @status == 200 ok && string_body? end @@ -238,36 +239,5 @@ module ActionDispatch # :nodoc: headers["Cache-Control"] = options.join(", ") end - - def convert_content_type! - headers['Content-Type'] ||= "text/html" - headers['Content-Type'] += "; charset=" + headers.delete('charset') if headers['charset'] - end - - # Don't set the Content-Length for block-based bodies as that would mean - # reading it all into memory. Not nice for, say, a 2GB streaming file. - def set_content_length! - if status && status.to_s[0..2] == '204' - headers.delete('Content-Length') - elsif length = headers['Content-Length'] - headers['Content-Length'] = length.to_s - elsif string_body? && (!status || status.to_s[0..2] != '304') - headers["Content-Length"] = Rack::Utils.bytesize(body).to_s - end - end - - def convert_language! - headers["Content-Language"] = headers.delete("language") if headers["language"] - end - - def convert_cookies! - headers['Set-Cookie'] = - if header = headers['Set-Cookie'] - header = header.split("\n") if header.respond_to?(:to_str) - header.compact - else - [] - end - end end end diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 9e008a9ae8..301080842e 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -120,8 +120,6 @@ class MultipartParamsParsingTest < ActionController::IntegrationTest fixture = FIXTURE_PATH + "/mona_lisa.jpg" params = { :uploaded_data => fixture_file_upload(fixture, "image/jpg") } post '/read', params - expected_length = 'File: '.length + File.size(fixture) - assert_equal expected_length, response.content_length end end diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 2b1540c678..256ed06a45 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -15,8 +15,7 @@ class ResponseTest < ActiveSupport::TestCase "Content-Type" => "text/html; charset=utf-8", "Cache-Control" => "max-age=0, private, must-revalidate", "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"', - "Set-Cookie" => "", - "Content-Length" => "13" + "Set-Cookie" => "" }, headers) parts = [] @@ -34,8 +33,7 @@ class ResponseTest < ActiveSupport::TestCase "Content-Type" => "text/html; charset=utf-8", "Cache-Control" => "max-age=0, private, must-revalidate", "ETag" => '"ebb5e89e8a94e9dd22abf5d915d112b2"', - "Set-Cookie" => "", - "Content-Length" => "8" + "Set-Cookie" => "" }, headers) end diff --git a/actionpack/test/new_base/base_test.rb b/actionpack/test/new_base/base_test.rb index d9d552f9e5..1b2e917ced 100644 --- a/actionpack/test/new_base/base_test.rb +++ b/actionpack/test/new_base/base_test.rb @@ -34,7 +34,6 @@ module Dispatching assert_body "success" assert_status 200 assert_content_type "text/html; charset=utf-8" - assert_header "Content-Length", "7" end # :api: plugin @@ -42,7 +41,6 @@ module Dispatching get "/dispatching/simple/modify_response_body" assert_body "success" - assert_header "Content-Length", "7" # setting the body manually sets the content length end # :api: plugin @@ -50,7 +48,6 @@ module Dispatching get "/dispatching/simple/modify_response_body_twice" assert_body "success!" - assert_header "Content-Length", "8" end test "controller path" do From 9b68877897a44d4484cde40117abc42c9b029154 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 2 Aug 2009 21:06:35 -0500 Subject: [PATCH 07/38] Track generated attribute methods in a separate module --- .../lib/active_record/attribute_methods.rb | 44 ++++++------------- .../active_record/attribute_methods/read.rb | 4 +- .../attribute_methods/time_zone_conversion.rb | 4 +- .../active_record/attribute_methods/write.rb | 2 +- 4 files changed, 19 insertions(+), 35 deletions(-) diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 5cb536af1f..211b77f514 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -75,19 +75,18 @@ module ActiveRecord @@attribute_method_regexp.match(method_name) end - # Contains the names of the generated attribute methods. def generated_methods #:nodoc: - @generated_methods ||= Set.new - end - - def generated_methods? - !generated_methods.empty? + @generated_methods ||= begin + mod = Module.new + include mod + mod + end end # Generates all the attribute related methods for columns in the database # accessors, mutators and query methods. def define_attribute_methods - return if generated_methods? + return unless generated_methods.instance_methods.empty? columns_hash.keys.each do |name| attribute_method_suffixes.each do |suffix| method_name = "#{name}#{suffix}" @@ -96,7 +95,7 @@ module ActiveRecord if respond_to?(generate_method) send(generate_method, name) else - evaluate_attribute_method("def #{method_name}(*args); send(:attribute#{suffix}, '#{name}', *args); end", method_name) + generated_methods.module_eval("def #{method_name}(*args); send(:attribute#{suffix}, '#{name}', *args); end", __FILE__, __LINE__) end end end @@ -104,8 +103,9 @@ module ActiveRecord end def undefine_attribute_methods - generated_methods.each { |name| undef_method(name) } - @generated_methods = nil + generated_methods.module_eval do + instance_methods.each { |m| undef_method(m) } + end end # Checks whether the method is defined in the model or any of its subclasses @@ -129,22 +129,6 @@ module ActiveRecord def attribute_method_suffixes @@attribute_method_suffixes ||= [] end - - # Evaluate the definition for an attribute related method - def evaluate_attribute_method(method_definition, method_name) - generated_methods << method_name.to_s - - begin - class_eval(method_definition, __FILE__, __LINE__) - rescue SyntaxError => err - generated_methods.delete(method_name.to_s) - if logger - logger.warn "Exception occurred during reader method compilation." - logger.warn "Maybe #{method_name} is not a valid Ruby identifier?" - logger.warn err.message - end - end - end end # Allows access to the object attributes, which are held in the @attributes hash, as though they @@ -160,10 +144,10 @@ module ActiveRecord # If we haven't generated any methods yet, generate them, then # see if we've created the method we're looking for. - if !self.class.generated_methods? + if self.class.generated_methods.instance_methods.empty? self.class.define_attribute_methods guard_private_attribute_method!(method_name, args) - if self.class.generated_methods.include?(method_name) + if self.class.generated_methods.instance_methods.include?(method_name) return self.send(method_id, *args, &block) end end @@ -190,9 +174,9 @@ module ActiveRecord # If we're here than we haven't found among non-private methods # but found among all methods. Which means that given method is private. return false - elsif !self.class.generated_methods? + elsif self.class.generated_methods.instance_methods.empty? self.class.define_attribute_methods - if self.class.generated_methods.include?(method_name) + if self.class.generated_methods.instance_methods.include?(method_name) return true end end diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index bea332ef26..6c0bf072e9 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -51,7 +51,7 @@ module ActiveRecord private # Define read method for serialized attribute. def define_read_method_for_serialized_attribute(attr_name) - evaluate_attribute_method "def #{attr_name}; unserialize_attribute('#{attr_name}'); end", attr_name + generated_methods.module_eval("def #{attr_name}; unserialize_attribute('#{attr_name}'); end", __FILE__, __LINE__) end # Define an attribute reader method. Cope with nil column. @@ -66,7 +66,7 @@ module ActiveRecord if cache_attribute?(attr_name) access_code = "@attributes_cache['#{attr_name}'] ||= (#{access_code})" end - evaluate_attribute_method "def #{symbol}; #{access_code}; end", symbol + generated_methods.module_eval("def #{symbol}; #{access_code}; end", __FILE__, __LINE__) end end diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 9e2c6174c6..4438c7543e 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -25,7 +25,7 @@ module ActiveRecord @attributes_cache['#{attr_name}'] = time.acts_like?(:time) ? time.in_time_zone : time end EOV - evaluate_attribute_method method_body, attr_name + generated_methods.module_eval(method_body, __FILE__, __LINE__) else super end @@ -44,7 +44,7 @@ module ActiveRecord write_attribute(:#{attr_name}, time) end EOV - evaluate_attribute_method method_body, "#{attr_name}=" + generated_methods.module_eval(method_body, __FILE__, __LINE__) else super end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index 497e72ee4a..c75745c00d 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -10,7 +10,7 @@ module ActiveRecord module ClassMethods protected def define_attribute_method=(attr_name) - evaluate_attribute_method "def #{attr_name}=(new_value); write_attribute('#{attr_name}', new_value); end", "#{attr_name}=" + generated_methods.module_eval("def #{attr_name}=(new_value); write_attribute('#{attr_name}', new_value); end", __FILE__, __LINE__) end end From 6f97ad07ded847f29159baf71050c63f04282170 Mon Sep 17 00:00:00 2001 From: Geoff Buesing Date: Mon, 3 Aug 2009 21:54:40 -0500 Subject: [PATCH 08/38] quoted_date converts time-like objects to ActiveRecord::Base.default_timezone before serialization. This allows you to use Time.now in find conditions and have it correctly be serialized as the current time in UTC when default_timezone == :utc [#2946 state:resolved] --- activerecord/CHANGELOG | 2 + .../connection_adapters/abstract/quoting.rb | 7 +- activerecord/test/cases/base_test.rb | 69 +++++++++++++++++++ activerecord/test/cases/finder_test.rb | 50 ++++++++++++++ 4 files changed, 127 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 5515cf6b1a..9adc6b887f 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* quoted_date converts time-like objects to ActiveRecord::Base.default_timezone before serialization. This allows you to use Time.now in find conditions and have it correctly be serialized as the current time in UTC when default_timezone == :utc. #2946 [Geoff Buesing] + * SQLite: drop support for 'dbfile' option in favor of 'database.' #2363 [Paul Hinze, Jeremy Kemper] * Added :primary_key option to belongs_to associations. #765 [Szymon Nowak, Philip Hallstrom, Noel Rocha] diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index 720fba29e9..8649f96498 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -60,7 +60,12 @@ module ActiveRecord end def quoted_date(value) - value.to_s(:db) + if value.acts_like?(:time) + zone_conversion_method = ActiveRecord::Base.default_timezone == :utc ? :getutc : :getlocal + value.respond_to?(zone_conversion_method) ? value.send(zone_conversion_method) : value + else + value + end.to_s(:db) end def quoted_string_prefix diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 7169d93841..82eba81549 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -464,6 +464,60 @@ class BasicsTest < ActiveRecord::TestCase end end + def test_preserving_time_objects_with_local_time_conversion_to_default_timezone_utc + with_env_tz 'America/New_York' do + with_active_record_default_timezone :utc do + time = Time.local(2000) + topic = Topic.create('written_on' => time) + saved_time = Topic.find(topic.id).written_on + assert_equal time, saved_time + assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "EST"], time.to_a + assert_equal [0, 0, 5, 1, 1, 2000, 6, 1, false, "UTC"], saved_time.to_a + end + end + end + + def test_preserving_time_objects_with_time_with_zone_conversion_to_default_timezone_utc + with_env_tz 'America/New_York' do + with_active_record_default_timezone :utc do + Time.use_zone 'Central Time (US & Canada)' do + time = Time.zone.local(2000) + topic = Topic.create('written_on' => time) + saved_time = Topic.find(topic.id).written_on + assert_equal time, saved_time + assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "CST"], time.to_a + assert_equal [0, 0, 6, 1, 1, 2000, 6, 1, false, "UTC"], saved_time.to_a + end + end + end + end + + def test_preserving_time_objects_with_utc_time_conversion_to_default_timezone_local + with_env_tz 'America/New_York' do + time = Time.utc(2000) + topic = Topic.create('written_on' => time) + saved_time = Topic.find(topic.id).written_on + assert_equal time, saved_time + assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "UTC"], time.to_a + assert_equal [0, 0, 19, 31, 12, 1999, 5, 365, false, "EST"], saved_time.to_a + end + end + + def test_preserving_time_objects_with_time_with_zone_conversion_to_default_timezone_local + with_env_tz 'America/New_York' do + with_active_record_default_timezone :local do + Time.use_zone 'Central Time (US & Canada)' do + time = Time.zone.local(2000) + topic = Topic.create('written_on' => time) + saved_time = Topic.find(topic.id).written_on + assert_equal time, saved_time + assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "CST"], time.to_a + assert_equal [0, 0, 1, 1, 1, 2000, 6, 1, false, "EST"], saved_time.to_a + end + end + end + end + def test_custom_mutator topic = Topic.find(1) # This mutator is protected in the class definition @@ -2115,4 +2169,19 @@ class BasicsTest < ActiveRecord::TestCase def test_dup assert !Minimalistic.new.freeze.dup.frozen? end + + protected + def with_env_tz(new_tz = 'US/Eastern') + old_tz, ENV['TZ'] = ENV['TZ'], new_tz + yield + ensure + old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ') + end + + def with_active_record_default_timezone(zone) + old_zone, ActiveRecord::Base.default_timezone = ActiveRecord::Base.default_timezone, zone + yield + ensure + ActiveRecord::Base.default_timezone = old_zone + end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index d8f5695a0f..55ef0d45eb 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -423,6 +423,42 @@ class FinderTest < ActiveRecord::TestCase assert_equal customers(:david), found_customer end + def test_condition_utc_time_interpolation_with_default_timezone_local + with_env_tz 'America/New_York' do + with_active_record_default_timezone :local do + topic = Topic.first + assert_equal topic, Topic.find(:first, :conditions => ['written_on = ?', topic.written_on.getutc]) + end + end + end + + def test_hash_condition_utc_time_interpolation_with_default_timezone_local + with_env_tz 'America/New_York' do + with_active_record_default_timezone :local do + topic = Topic.first + assert_equal topic, Topic.find(:first, :conditions => {:written_on => topic.written_on.getutc}) + end + end + end + + def test_condition_local_time_interpolation_with_default_timezone_utc + with_env_tz 'America/New_York' do + with_active_record_default_timezone :utc do + topic = Topic.first + assert_equal topic, Topic.find(:first, :conditions => ['written_on = ?', topic.written_on.getlocal]) + end + end + end + + def test_hash_condition_local_time_interpolation_with_default_timezone_utc + with_env_tz 'America/New_York' do + with_active_record_default_timezone :utc do + topic = Topic.first + assert_equal topic, Topic.find(:first, :conditions => {:written_on => topic.written_on.getlocal}) + end + end + end + def test_bind_variables assert_kind_of Firm, Company.find(:first, :conditions => ["name = ?", "37signals"]) assert_nil Company.find(:first, :conditions => ["name = ?", "37signals!"]) @@ -1087,4 +1123,18 @@ class FinderTest < ActiveRecord::TestCase ActiveRecord::Base.send(:replace_bind_variables, statement, vars) end end + + def with_env_tz(new_tz = 'US/Eastern') + old_tz, ENV['TZ'] = ENV['TZ'], new_tz + yield + ensure + old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ') + end + + def with_active_record_default_timezone(zone) + old_zone, ActiveRecord::Base.default_timezone = ActiveRecord::Base.default_timezone, zone + yield + ensure + ActiveRecord::Base.default_timezone = old_zone + end end From 55d1d12c32a1b99f3f07d2346b49a63650ba2e9d Mon Sep 17 00:00:00 2001 From: Matt Ganderup Date: Mon, 1 Jun 2009 10:24:15 -0700 Subject: [PATCH 09/38] fallback_string_to_date sets Date._parse comp arg to true, so that strings with two-digit years, e.g. '1/1/09', are interpreted as modern years [#2019 state:resolved] --- activerecord/CHANGELOG | 2 ++ .../connection_adapters/abstract/schema_definitions.rb | 2 +- activerecord/test/cases/date_time_test.rb | 6 ++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 9adc6b887f..3f350be3e9 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* fallback_string_to_date sets Date._parse comp arg to true, so that strings with two-digit years, e.g. '1/1/09', are interpreted as modern years #2019 [Matt Ganderup] + * quoted_date converts time-like objects to ActiveRecord::Base.default_timezone before serialization. This allows you to use Time.now in find conditions and have it correctly be serialized as the current time in UTC when default_timezone == :utc. #2946 [Geoff Buesing] * SQLite: drop support for 'dbfile' option in favor of 'database.' #2363 [Paul Hinze, Jeremy Kemper] diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 24c734cddb..f4e8fe6a38 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -201,7 +201,7 @@ module ActiveRecord end def fallback_string_to_date(string) - new_date(*::Date._parse(string, false).values_at(:year, :mon, :mday)) + new_date(*::Date._parse(string, true).values_at(:year, :mon, :mday)) end def fallback_string_to_time(string) diff --git a/activerecord/test/cases/date_time_test.rb b/activerecord/test/cases/date_time_test.rb index 36e1caa0b6..f7527c0c04 100644 --- a/activerecord/test/cases/date_time_test.rb +++ b/activerecord/test/cases/date_time_test.rb @@ -34,4 +34,10 @@ class DateTimeTest < ActiveRecord::TestCase topic.bonus_time = '' assert_nil topic.bonus_time end + + def test_two_digit_year + topic = Topic.new + topic.last_read = '1/1/09' + assert_equal Date.new(2009,1,1), topic.last_read + end end From aad5a30bf25d8a3167afd685fc91c99f4f09cc57 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 4 Aug 2009 11:03:57 -0500 Subject: [PATCH 10/38] Add simple support for ActiveModel's StateMachine for ActiveRecord --- activemodel/lib/active_model/state_machine.rb | 7 +--- .../lib/active_model/state_machine/event.rb | 12 +++--- .../lib/active_model/state_machine/machine.rb | 29 ++++++------- .../state_machine/state_transition.rb | 2 +- activerecord/lib/active_record.rb | 1 + .../lib/active_record/state_machine.rb | 24 +++++++++++ activerecord/test/cases/state_machine_test.rb | 42 +++++++++++++++++++ activerecord/test/models/traffic_light.rb | 27 ++++++++++++ activerecord/test/schema/schema.rb | 7 ++++ 9 files changed, 122 insertions(+), 29 deletions(-) create mode 100644 activerecord/lib/active_record/state_machine.rb create mode 100644 activerecord/test/cases/state_machine_test.rb create mode 100644 activerecord/test/models/traffic_light.rb diff --git a/activemodel/lib/active_model/state_machine.rb b/activemodel/lib/active_model/state_machine.rb index 1172d31ea3..527794b34d 100644 --- a/activemodel/lib/active_model/state_machine.rb +++ b/activemodel/lib/active_model/state_machine.rb @@ -5,12 +5,9 @@ module ActiveModel autoload :State, 'active_model/state_machine/state' autoload :StateTransition, 'active_model/state_machine/state_transition' - class InvalidTransition < Exception - end + extend ActiveSupport::Concern - def self.included(base) - require 'active_model/state_machine/machine' - base.extend ClassMethods + class InvalidTransition < Exception end module ClassMethods diff --git a/activemodel/lib/active_model/state_machine/event.rb b/activemodel/lib/active_model/state_machine/event.rb index 3eb656b6d6..30e9601dc2 100644 --- a/activemodel/lib/active_model/state_machine/event.rb +++ b/activemodel/lib/active_model/state_machine/event.rb @@ -1,5 +1,3 @@ -require 'active_model/state_machine/state_transition' - module ActiveModel module StateMachine class Event @@ -53,12 +51,12 @@ module ActiveModel self end - private - def transitions(trans_opts) - Array(trans_opts[:from]).each do |s| - @transitions << StateTransition.new(trans_opts.merge({:from => s.to_sym})) + private + def transitions(trans_opts) + Array(trans_opts[:from]).each do |s| + @transitions << StateTransition.new(trans_opts.merge({:from => s.to_sym})) + end end - end end end end diff --git a/activemodel/lib/active_model/state_machine/machine.rb b/activemodel/lib/active_model/state_machine/machine.rb index a5ede021b1..777531213e 100644 --- a/activemodel/lib/active_model/state_machine/machine.rb +++ b/activemodel/lib/active_model/state_machine/machine.rb @@ -1,6 +1,3 @@ -require 'active_model/state_machine/state' -require 'active_model/state_machine/event' - module ActiveModel module StateMachine class Machine @@ -57,22 +54,22 @@ module ActiveModel "@#{@name}_current_state" end - private - def state(name, options = {}) - @states << (state_index[name] ||= State.new(name, :machine => self)).update(options) - end + private + def state(name, options = {}) + @states << (state_index[name] ||= State.new(name, :machine => self)).update(options) + end - def event(name, options = {}, &block) - (@events[name] ||= Event.new(self, name)).update(options, &block) - end + def event(name, options = {}, &block) + (@events[name] ||= Event.new(self, name)).update(options, &block) + end - def event_fired_callback - @event_fired_callback ||= (@name == :default ? '' : "#{@name}_") + 'event_fired' - end + def event_fired_callback + @event_fired_callback ||= (@name == :default ? '' : "#{@name}_") + 'event_fired' + end - def event_failed_callback - @event_failed_callback ||= (@name == :default ? '' : "#{@name}_") + 'event_failed' - end + def event_failed_callback + @event_failed_callback ||= (@name == :default ? '' : "#{@name}_") + 'event_failed' + end end end end diff --git a/activemodel/lib/active_model/state_machine/state_transition.rb b/activemodel/lib/active_model/state_machine/state_transition.rb index f9df998ea4..b0c5504de7 100644 --- a/activemodel/lib/active_model/state_machine/state_transition.rb +++ b/activemodel/lib/active_model/state_machine/state_transition.rb @@ -18,7 +18,7 @@ module ActiveModel true end end - + def execute(obj, *args) case @on_transition when Symbol, String diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 5e9ce0600a..68b0251982 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -65,6 +65,7 @@ module ActiveRecord autoload :SchemaDumper, 'active_record/schema_dumper' autoload :Serialization, 'active_record/serialization' autoload :SessionStore, 'active_record/session_store' + autoload :StateMachine, 'active_record/state_machine' autoload :TestCase, 'active_record/test_case' autoload :Timestamp, 'active_record/timestamp' autoload :Transactions, 'active_record/transactions' diff --git a/activerecord/lib/active_record/state_machine.rb b/activerecord/lib/active_record/state_machine.rb new file mode 100644 index 0000000000..aebd03344a --- /dev/null +++ b/activerecord/lib/active_record/state_machine.rb @@ -0,0 +1,24 @@ +module ActiveRecord + module StateMachine #:nodoc: + extend ActiveSupport::Concern + include ActiveModel::StateMachine + + included do + before_validation :set_initial_state + validates_presence_of :state + end + + protected + def write_state(state_machine, state) + update_attributes! :state => state.to_s + end + + def read_state(state_machine) + self.state.to_sym + end + + def set_initial_state + self.state ||= self.class.state_machine.initial_state.to_s + end + end +end diff --git a/activerecord/test/cases/state_machine_test.rb b/activerecord/test/cases/state_machine_test.rb new file mode 100644 index 0000000000..5d13668bab --- /dev/null +++ b/activerecord/test/cases/state_machine_test.rb @@ -0,0 +1,42 @@ +require 'cases/helper' +require 'models/traffic_light' + +class StateMachineTest < ActiveRecord::TestCase + def setup + @light = TrafficLight.create! + end + + test "states initial state" do + assert @light.off? + assert_equal :off, @light.current_state + end + + test "transition to a valid state" do + @light.reset + assert @light.red? + assert_equal :red, @light.current_state + + @light.green_on + assert @light.green? + assert_equal :green, @light.current_state + end + + test "transition does not persist state" do + @light.reset + assert_equal :red, @light.current_state + @light.reload + assert_equal "off", @light.state + end + + test "transition does persists state" do + @light.reset! + assert_equal :red, @light.current_state + @light.reload + assert_equal "red", @light.state + end + + test "transition to an invalid state" do + assert_raise(ActiveModel::StateMachine::InvalidTransition) { @light.yellow_on } + assert_equal :off, @light.current_state + end +end diff --git a/activerecord/test/models/traffic_light.rb b/activerecord/test/models/traffic_light.rb new file mode 100644 index 0000000000..f8cfddbef9 --- /dev/null +++ b/activerecord/test/models/traffic_light.rb @@ -0,0 +1,27 @@ +class TrafficLight < ActiveRecord::Base + include ActiveRecord::StateMachine + + state_machine do + state :off + + state :red + state :green + state :yellow + + event :red_on do + transitions :to => :red, :from => [:yellow] + end + + event :green_on do + transitions :to => :green, :from => [:red] + end + + event :yellow_on do + transitions :to => :yellow, :from => [:green] + end + + event :reset do + transitions :to => :red, :from => [:off] + end + end +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 2b7d3856b7..1e47cdbaf6 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -448,6 +448,13 @@ ActiveRecord::Schema.define do t.integer :pet_id, :integer end + create_table :traffic_lights, :force => true do |t| + t.string :location + t.string :state + t.datetime :created_at + t.datetime :updated_at + end + create_table :treasures, :force => true do |t| t.column :name, :string t.column :looter_id, :integer From c30a0ce3c8f88baebd369180a6e221706e2b5cbf Mon Sep 17 00:00:00 2001 From: Paul Gillard Date: Tue, 4 Aug 2009 16:19:19 -0500 Subject: [PATCH 11/38] Modified ActiveRecord::AttributeMethods to allow classes to specify attribute method prefixes and/or suffixes. Previously only suffixes were allowed. Signed-off-by: Joshua Peek --- .../lib/active_record/attribute_methods.rb | 154 +++++++++++++----- .../active_record/attribute_methods/read.rb | 2 +- .../attribute_methods/time_zone_conversion.rb | 4 +- .../active_record/attribute_methods/write.rb | 2 +- .../test/cases/attribute_methods_test.rb | 90 ++++++++-- 5 files changed, 198 insertions(+), 54 deletions(-) diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 211b77f514..89a92cd7f7 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -4,10 +4,62 @@ module ActiveRecord module AttributeMethods #:nodoc: extend ActiveSupport::Concern + class AttributeMethodMatcher + attr_reader :prefix, :suffix + + AttributeMethodMatch = Struct.new(:prefix, :base, :suffix) + + def initialize(options = {}) + options.symbolize_keys! + @prefix, @suffix = options[:prefix] || '', options[:suffix] || '' + @regex = /^(#{Regexp.escape(@prefix)})(.+?)(#{Regexp.escape(@suffix)})$/ + end + + def match(method_name) + if matchdata = @regex.match(method_name) + AttributeMethodMatch.new(matchdata[1], matchdata[2], matchdata[3]) + else + nil + end + end + end + # Declare and check for suffixed attribute methods. module ClassMethods + # Declares a method available for all attributes with the given prefix. + # Uses +method_missing+ and respond_to? to rewrite the method. + # + # #{prefix}#{attr}(*args, &block) + # + # to + # + # #{prefix}attribute(#{attr}, *args, &block) + # + # An #{prefix}attribute instance method must exist and accept at least + # the +attr+ argument. + # + # For example: + # + # class Person < ActiveRecord::Base + # attribute_method_prefix 'clear_' + # + # private + # def clear_attribute(attr) + # ... + # end + # end + # + # person = Person.find(1) + # person.name # => 'Gem' + # person.clear_name + # person.name # => '' + def attribute_method_prefix(*prefixes) + attribute_method_matchers.concat(prefixes.map { |prefix| AttributeMethodMatcher.new :prefix => prefix }) + undefine_attribute_methods + end + # Declares a method available for all attributes with the given suffix. - # Uses +method_missing+ and respond_to? to rewrite the method + # Uses +method_missing+ and respond_to? to rewrite the method. # # #{attr}#{suffix}(*args, &block) # @@ -21,24 +73,59 @@ module ActiveRecord # For example: # # class Person < ActiveRecord::Base - # attribute_method_suffix '_changed?' + # attribute_method_suffix '_short?' # # private - # def attribute_changed?(attr) + # def attribute_short?(attr) # ... # end # end # # person = Person.find(1) - # person.name_changed? # => false - # person.name = 'Hubert' - # person.name_changed? # => true + # person.name # => 'Gem' + # person.name_short? # => true def attribute_method_suffix(*suffixes) - attribute_method_suffixes.concat(suffixes) - rebuild_attribute_method_regexp + attribute_method_matchers.concat(suffixes.map { |suffix| AttributeMethodMatcher.new :suffix => suffix }) undefine_attribute_methods end + # Declares a method available for all attributes with the given prefix + # and suffix. Uses +method_missing+ and respond_to? to rewrite + # the method. + # + # #{prefix}#{attr}#{suffix}(*args, &block) + # + # to + # + # #{prefix}attribute#{suffix}(#{attr}, *args, &block) + # + # An #{prefix}attribute#{suffix} instance method must exist and + # accept at least the +attr+ argument. + # + # For example: + # + # class Person < ActiveRecord::Base + # attribute_method_affix :prefix => 'reset_', :suffix => '_to_default!' + # + # private + # def reset_attribute_to_default!(attr) + # ... + # end + # end + # + # person = Person.find(1) + # person.name # => 'Gem' + # person.reset_name_to_default! + # person.name # => 'Gemma' + def attribute_method_affix(*affixes) + attribute_method_matchers.concat(affixes.map { |affix| AttributeMethodMatcher.new :prefix => affix[:prefix], :suffix => affix[:suffix] }) + undefine_attribute_methods + end + + def matching_attribute_methods(method_name) + attribute_method_matchers.collect { |method| method.match(method_name) }.compact + end + # Defines an "attribute" method (like +inheritance_column+ or # +table_name+). A new (class) method will be created with the # given name. If a value is specified, the new method will @@ -69,12 +156,6 @@ module ActiveRecord end end - # Returns MatchData if method_name is an attribute method. - def match_attribute_method?(method_name) - rebuild_attribute_method_regexp unless defined?(@@attribute_method_regexp) && @@attribute_method_regexp - @@attribute_method_regexp.match(method_name) - end - def generated_methods #:nodoc: @generated_methods ||= begin mod = Module.new @@ -88,14 +169,15 @@ module ActiveRecord def define_attribute_methods return unless generated_methods.instance_methods.empty? columns_hash.keys.each do |name| - attribute_method_suffixes.each do |suffix| - method_name = "#{name}#{suffix}" + attribute_method_matchers.each do |method| + method_name = "#{method.prefix}#{name}#{method.suffix}" unless instance_method_already_implemented?(method_name) - generate_method = "define_attribute_method#{suffix}" + generate_method = "define_method_#{method.prefix}attribute#{method.suffix}" + if respond_to?(generate_method) send(generate_method, name) else - generated_methods.module_eval("def #{method_name}(*args); send(:attribute#{suffix}, '#{name}', *args); end", __FILE__, __LINE__) + generated_methods.module_eval("def #{method_name}(*args); send(:#{method.prefix}attribute#{method.suffix}, '#{name}', *args); end", __FILE__, __LINE__) end end end @@ -120,17 +202,20 @@ module ActiveRecord end private - # Suffixes a, ?, c become regexp /(a|\?|c)$/ - def rebuild_attribute_method_regexp - suffixes = attribute_method_suffixes.map { |s| Regexp.escape(s) } - @@attribute_method_regexp = /(#{suffixes.join('|')})$/.freeze - end - - def attribute_method_suffixes - @@attribute_method_suffixes ||= [] + # Default to *=, *? and *_before_type_cast + def attribute_method_matchers + @@attribute_method_matchers ||= [] end end + # Returns a struct representing the matching attribute method. + # The struct's attributes are prefix, base and suffix. + def match_attribute_method?(method_name) + self.class.matching_attribute_methods(method_name).find do |match| + match.base == 'id' || @attributes.include?(match.base) + end + end + # Allows access to the object attributes, which are held in the @attributes hash, as though they # were first-class methods. So a Person class with a name attribute can use Person#name and # Person#name= and never directly use the attributes hash -- except for multiple assigns with @@ -152,12 +237,9 @@ module ActiveRecord end end - if md = self.class.match_attribute_method?(method_name) - attribute_name, method_type = md.pre_match, md.to_s - if attribute_name == 'id' || @attributes.include?(attribute_name) - guard_private_attribute_method!(method_name, args) - return __send__("attribute#{method_type}", attribute_name, *args, &block) - end + if match = match_attribute_method?(method_name) + guard_private_attribute_method!(method_name, args) + return __send__("#{match.prefix}attribute#{match.suffix}", match.base, *args, &block) end super end @@ -171,7 +253,7 @@ module ActiveRecord if super return true elsif !include_private_methods && super(method, true) - # If we're here than we haven't found among non-private methods + # If we're here then we haven't found among non-private methods # but found among all methods. Which means that given method is private. return false elsif self.class.generated_methods.instance_methods.empty? @@ -179,10 +261,8 @@ module ActiveRecord if self.class.generated_methods.instance_methods.include?(method_name) return true end - end - - if md = self.class.match_attribute_method?(method_name) - return true if md.pre_match == 'id' || @attributes.include?(md.pre_match) + elsif match_attribute_method?(method_name) + return true end super end diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 6c0bf072e9..90acb769a9 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -36,7 +36,7 @@ module ActiveRecord end protected - def define_attribute_method(attr_name) + def define_method_attribute(attr_name) if self.serialized_attributes[attr_name] define_read_method_for_serialized_attribute(attr_name) else diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 4438c7543e..b9cfe59971 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -15,7 +15,7 @@ module ActiveRecord protected # Defined for all +datetime+ and +timestamp+ attributes when +time_zone_aware_attributes+ are enabled. # This enhanced read method automatically converts the UTC time stored in the database to the time zone stored in Time.zone. - def define_attribute_method(attr_name) + def define_method_attribute(attr_name) if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name]) method_body = <<-EOV def #{attr_name}(reload = false) @@ -33,7 +33,7 @@ module ActiveRecord # Defined for all +datetime+ and +timestamp+ attributes when +time_zone_aware_attributes+ are enabled. # This enhanced write method will automatically convert the time passed to it to the zone stored in Time.zone. - def define_attribute_method=(attr_name) + def define_method_attribute=(attr_name) if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name]) method_body = <<-EOV def #{attr_name}=(time) diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index c75745c00d..79118855cf 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -9,7 +9,7 @@ module ActiveRecord module ClassMethods protected - def define_attribute_method=(attr_name) + def define_method_attribute=(attr_name) generated_methods.module_eval("def #{attr_name}=(new_value); write_attribute('#{attr_name}', new_value); end", __FILE__, __LINE__) end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 168b617fbc..a5f4a67200 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -4,40 +4,90 @@ require 'models/minimalistic' class AttributeMethodsTest < ActiveRecord::TestCase fixtures :topics + def setup - @old_suffixes = ActiveRecord::Base.send(:attribute_method_suffixes).dup + @old_matchers = ActiveRecord::Base.send(:attribute_method_matchers).dup @target = Class.new(ActiveRecord::Base) @target.table_name = 'topics' end def teardown - ActiveRecord::Base.send(:attribute_method_suffixes).clear - ActiveRecord::Base.attribute_method_suffix *@old_suffixes + ActiveRecord::Base.send(:attribute_method_matchers).clear + ActiveRecord::Base.send(:attribute_method_matchers).concat(@old_matchers) end - def test_match_attribute_method_query_returns_match_data - assert_not_nil md = @target.match_attribute_method?('title=') - assert_equal 'title', md.pre_match - assert_equal ['='], md.captures + def test_match_attribute_method_query_returns_default_match_data + topic = @target.new(:title => 'Budget') + assert_not_nil match = topic.match_attribute_method?('title=') + assert_equal '', match.prefix + assert_equal 'title', match.base + assert_equal '=', match.suffix + end + + def test_match_attribute_method_query_returns_match_data_for_prefixes + topic = @target.new(:title => 'Budget') + %w(default_ title_).each do |prefix| + @target.class_eval "def #{prefix}attribute(*args) args end" + @target.attribute_method_prefix prefix - %w(_hello_world ist! _maybe?).each do |suffix| + assert_not_nil match = topic.match_attribute_method?("#{prefix}title") + assert_equal prefix, match.prefix + assert_equal 'title', match.base + assert_equal '', match.suffix + end + end + + def test_match_attribute_method_query_returns_match_data_for_suffixes + topic = @target.new(:title => 'Budget') + %w(_default _title_default it! _candidate= _maybe?).each do |suffix| @target.class_eval "def attribute#{suffix}(*args) args end" @target.attribute_method_suffix suffix - assert_not_nil md = @target.match_attribute_method?("title#{suffix}") - assert_equal 'title', md.pre_match - assert_equal [suffix], md.captures + assert_not_nil match = topic.match_attribute_method?("title#{suffix}") + assert_equal '', match.prefix + assert_equal 'title', match.base + assert_equal suffix, match.suffix end end + + def test_match_attribute_method_query_returns_match_data_for_affixes + topic = @target.new(:title => 'Budget') + [['mark_', '_for_update'], ['reset_', '!'], ['default_', '_value?']].each do |prefix, suffix| + @target.class_eval "def #{prefix}attribute#{suffix}(*args) args end" + @target.attribute_method_affix({ :prefix => prefix, :suffix => suffix }) - def test_declared_attribute_method_affects_respond_to_and_method_missing + assert_not_nil match = topic.match_attribute_method?("#{prefix}title#{suffix}") + assert_equal prefix, match.prefix + assert_equal 'title', match.base + assert_equal suffix, match.suffix + end + end + + def test_undeclared_attribute_method_does_not_affect_respond_to_and_method_missing topic = @target.new(:title => 'Budget') assert topic.respond_to?('title') assert_equal 'Budget', topic.title assert !topic.respond_to?('title_hello_world') assert_raise(NoMethodError) { topic.title_hello_world } + end - %w(_hello_world _it! _candidate= able?).each do |suffix| + def test_declared_prefixed_attribute_method_affects_respond_to_and_method_missing + topic = @target.new(:title => 'Budget') + %w(default_ title_).each do |prefix| + @target.class_eval "def #{prefix}attribute(*args) args end" + @target.attribute_method_prefix prefix + + meth = "#{prefix}title" + assert topic.respond_to?(meth) + assert_equal ['title'], topic.send(meth) + assert_equal ['title', 'a'], topic.send(meth, 'a') + assert_equal ['title', 1, 2, 3], topic.send(meth, 1, 2, 3) + end + end + + def test_declared_suffixed_attribute_method_affects_respond_to_and_method_missing + topic = @target.new(:title => 'Budget') + %w(_default _title_default _it! _candidate= able?).each do |suffix| @target.class_eval "def attribute#{suffix}(*args) args end" @target.attribute_method_suffix suffix @@ -49,6 +99,20 @@ class AttributeMethodsTest < ActiveRecord::TestCase end end + def test_declared_affixed_attribute_method_affects_respond_to_and_method_missing + topic = @target.new(:title => 'Budget') + [['mark_', '_for_update'], ['reset_', '!'], ['default_', '_value?']].each do |prefix, suffix| + @target.class_eval "def #{prefix}attribute#{suffix}(*args) args end" + @target.attribute_method_affix({ :prefix => prefix, :suffix => suffix }) + + meth = "#{prefix}title#{suffix}" + assert topic.respond_to?(meth) + assert_equal ['title'], topic.send(meth) + assert_equal ['title', 'a'], topic.send(meth, 'a') + assert_equal ['title', 1, 2, 3], topic.send(meth, 1, 2, 3) + end + end + def test_should_unserialize_attributes_for_frozen_records myobj = {:value1 => :value2} topic = Topic.create("content" => myobj) From bada18dc36e3875dea1814ffaab1e8d1ac24b521 Mon Sep 17 00:00:00 2001 From: Paul Gillard Date: Tue, 4 Aug 2009 16:23:08 -0500 Subject: [PATCH 12/38] Added reset_attribute! method to ActiveRecord::AttributeMethods::Dirty which will reset an attribute to its original value should it have changed. Signed-off-by: Joshua Peek --- .../active_record/attribute_methods/dirty.rb | 41 +++++++++++++------ activerecord/test/cases/dirty_test.rb | 10 +++++ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index b88c84938d..9ec1fbeee1 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -3,17 +3,17 @@ module ActiveRecord # Track unsaved attribute changes. # # A newly instantiated object is unchanged: - # person = Person.find_by_name('uncle bob') + # person = Person.find_by_name('Uncle Bob') # person.changed? # => false # # Change the name: # person.name = 'Bob' # person.changed? # => true # person.name_changed? # => true - # person.name_was # => 'uncle bob' - # person.name_change # => ['uncle bob', 'Bob'] + # person.name_was # => 'Uncle Bob' + # person.name_change # => ['Uncle Bob', 'Bob'] # person.name = 'Bill' - # person.name_change # => ['uncle bob', 'Bill'] + # person.name_change # => ['Uncle Bob', 'Bill'] # # Save the changes: # person.save @@ -26,21 +26,33 @@ module ActiveRecord # person.name_change # => nil # # Which attributes have changed? - # person.name = 'bob' + # person.name = 'Bob' # person.changed # => ['name'] - # person.changes # => { 'name' => ['Bill', 'bob'] } + # person.changes # => { 'name' => ['Bill', 'Bob'] } + # + # Resetting an attribute returns it to its original state: + # person.reset_name! # => 'Bill' + # person.changed? # => false + # person.name_changed? # => false + # person.name # => 'Bill' # # Before modifying an attribute in-place: # person.name_will_change! - # person.name << 'by' - # person.name_change # => ['uncle bob', 'uncle bobby'] + # person.name << 'y' + # person.name_change # => ['Bill', 'Billy'] module Dirty extend ActiveSupport::Concern - DIRTY_SUFFIXES = ['_changed?', '_change', '_will_change!', '_was'] + DIRTY_AFFIXES = [ + { :suffix => '_changed?' }, + { :suffix => '_change' }, + { :suffix => '_will_change!' }, + { :suffix => '_was' }, + { :prefix => 'reset_', :suffix => '!' } + ] included do - attribute_method_suffix *DIRTY_SUFFIXES + attribute_method_affix *DIRTY_AFFIXES alias_method_chain :save, :dirty alias_method_chain :save!, :dirty @@ -118,6 +130,11 @@ module ActiveRecord attribute_changed?(attr) ? changed_attributes[attr] : __send__(attr) end + # Handle reset_*! for +method_missing+. + def reset_attribute!(attr) + self[attr] = changed_attributes[attr] if attribute_changed?(attr) + end + # Handle *_will_change! for +method_missing+. def attribute_will_change!(attr) changed_attributes[attr] = clone_attribute_value(:read_attribute, attr) @@ -175,9 +192,9 @@ module ActiveRecord def alias_attribute_with_dirty(new_name, old_name) alias_attribute_without_dirty(new_name, old_name) - DIRTY_SUFFIXES.each do |suffix| + DIRTY_AFFIXES.each do |affixes| module_eval <<-STR, __FILE__, __LINE__+1 - def #{new_name}#{suffix}; self.#{old_name}#{suffix}; end # def subject_changed?; self.title_changed?; end + def #{affixes[:prefix]}#{new_name}#{affixes[:suffix]}; self.#{affixes[:prefix]}#{old_name}#{affixes[:suffix]}; end # def reset_subject!; self.reset_title!; end STR end end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index ac95bac4ad..1441421a80 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -62,6 +62,16 @@ class DirtyTest < ActiveRecord::TestCase assert_equal parrot.name_change, parrot.title_change end + def test_reset_attribute! + pirate = Pirate.create!(:catchphrase => 'Yar!') + pirate.catchphrase = 'Ahoy!' + + pirate.reset_catchphrase! + assert_equal "Yar!", pirate.catchphrase + assert_equal Hash.new, pirate.changes + assert !pirate.catchphrase_changed? + end + def test_nullable_number_not_marked_as_changed_if_new_value_is_blank pirate = Pirate.new From 64eecdd131c93d7d6c8ef9c6a7ae6b9d76c72a8b Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 4 Aug 2009 16:28:44 -0500 Subject: [PATCH 13/38] whitespace --- activerecord/lib/active_record/attribute_methods.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 89a92cd7f7..be275f5cb6 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -6,7 +6,7 @@ module ActiveRecord class AttributeMethodMatcher attr_reader :prefix, :suffix - + AttributeMethodMatch = Struct.new(:prefix, :base, :suffix) def initialize(options = {}) @@ -121,11 +121,11 @@ module ActiveRecord attribute_method_matchers.concat(affixes.map { |affix| AttributeMethodMatcher.new :prefix => affix[:prefix], :suffix => affix[:suffix] }) undefine_attribute_methods end - + def matching_attribute_methods(method_name) attribute_method_matchers.collect { |method| method.match(method_name) }.compact end - + # Defines an "attribute" method (like +inheritance_column+ or # +table_name+). A new (class) method will be created with the # given name. If a value is specified, the new method will @@ -173,7 +173,7 @@ module ActiveRecord method_name = "#{method.prefix}#{name}#{method.suffix}" unless instance_method_already_implemented?(method_name) generate_method = "define_method_#{method.prefix}attribute#{method.suffix}" - + if respond_to?(generate_method) send(generate_method, name) else @@ -215,7 +215,7 @@ module ActiveRecord match.base == 'id' || @attributes.include?(match.base) end end - + # Allows access to the object attributes, which are held in the @attributes hash, as though they # were first-class methods. So a Person class with a name attribute can use Person#name and # Person#name= and never directly use the attributes hash -- except for multiple assigns with From f8d3c72c39ad209abca7f3613f91fb3a03805261 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 4 Aug 2009 23:36:05 -0500 Subject: [PATCH 14/38] Extract generic attribute method generation to AMo --- activemodel/lib/active_model.rb | 1 + .../lib/active_model/attribute_methods.rb | 267 ++++++++++++++++++ .../lib/active_record/attribute_methods.rb | 251 +--------------- .../active_record/attribute_methods/read.rb | 4 +- .../attribute_methods/time_zone_conversion.rb | 4 +- .../active_record/attribute_methods/write.rb | 2 +- activerecord/lib/active_record/base.rb | 5 - .../test/cases/attribute_methods_test.rb | 47 --- activerecord/test/cases/finder_test.rb | 2 +- 9 files changed, 284 insertions(+), 299 deletions(-) create mode 100644 activemodel/lib/active_model/attribute_methods.rb diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb index 2de19597b1..9bb4cf8b54 100644 --- a/activemodel/lib/active_model.rb +++ b/activemodel/lib/active_model.rb @@ -26,6 +26,7 @@ $:.unshift(activesupport_path) if File.directory?(activesupport_path) require 'active_support' module ActiveModel + autoload :AttributeMethods, 'active_model/attribute_methods' autoload :Conversion, 'active_model/conversion' autoload :DeprecatedErrorMethods, 'active_model/deprecated_error_methods' autoload :Errors, 'active_model/errors' diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb new file mode 100644 index 0000000000..de80559036 --- /dev/null +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -0,0 +1,267 @@ +module ActiveModel + class MissingAttributeError < NoMethodError + end + + module AttributeMethods + extend ActiveSupport::Concern + + # Declare and check for suffixed attribute methods. + module ClassMethods + # Defines an "attribute" method (like +inheritance_column+ or + # +table_name+). A new (class) method will be created with the + # given name. If a value is specified, the new method will + # return that value (as a string). Otherwise, the given block + # will be used to compute the value of the method. + # + # The original method will be aliased, with the new name being + # prefixed with "original_". This allows the new method to + # access the original value. + # + # Example: + # + # class A < ActiveRecord::Base + # define_attr_method :primary_key, "sysid" + # define_attr_method( :inheritance_column ) do + # original_inheritance_column + "_id" + # end + # end + def define_attr_method(name, value=nil, &block) + sing = metaclass + sing.send :alias_method, "original_#{name}", name + if block_given? + sing.send :define_method, name, &block + else + # use eval instead of a block to work around a memory leak in dev + # mode in fcgi + sing.class_eval "def #{name}; #{value.to_s.inspect}; end" + end + end + + # Declares a method available for all attributes with the given prefix. + # Uses +method_missing+ and respond_to? to rewrite the method. + # + # #{prefix}#{attr}(*args, &block) + # + # to + # + # #{prefix}attribute(#{attr}, *args, &block) + # + # An #{prefix}attribute instance method must exist and accept at least + # the +attr+ argument. + # + # For example: + # + # class Person < ActiveRecord::Base + # attribute_method_prefix 'clear_' + # + # private + # def clear_attribute(attr) + # ... + # end + # end + # + # person = Person.find(1) + # person.name # => 'Gem' + # person.clear_name + # person.name # => '' + def attribute_method_prefix(*prefixes) + attribute_method_matchers.concat(prefixes.map { |prefix| AttributeMethodMatcher.new :prefix => prefix }) + undefine_attribute_methods + end + + # Declares a method available for all attributes with the given suffix. + # Uses +method_missing+ and respond_to? to rewrite the method. + # + # #{attr}#{suffix}(*args, &block) + # + # to + # + # attribute#{suffix}(#{attr}, *args, &block) + # + # An attribute#{suffix} instance method must exist and accept at least + # the +attr+ argument. + # + # For example: + # + # class Person < ActiveRecord::Base + # attribute_method_suffix '_short?' + # + # private + # def attribute_short?(attr) + # ... + # end + # end + # + # person = Person.find(1) + # person.name # => 'Gem' + # person.name_short? # => true + def attribute_method_suffix(*suffixes) + attribute_method_matchers.concat(suffixes.map { |suffix| AttributeMethodMatcher.new :suffix => suffix }) + undefine_attribute_methods + end + + # Declares a method available for all attributes with the given prefix + # and suffix. Uses +method_missing+ and respond_to? to rewrite + # the method. + # + # #{prefix}#{attr}#{suffix}(*args, &block) + # + # to + # + # #{prefix}attribute#{suffix}(#{attr}, *args, &block) + # + # An #{prefix}attribute#{suffix} instance method must exist and + # accept at least the +attr+ argument. + # + # For example: + # + # class Person < ActiveRecord::Base + # attribute_method_affix :prefix => 'reset_', :suffix => '_to_default!' + # + # private + # def reset_attribute_to_default!(attr) + # ... + # end + # end + # + # person = Person.find(1) + # person.name # => 'Gem' + # person.reset_name_to_default! + # person.name # => 'Gemma' + def attribute_method_affix(*affixes) + attribute_method_matchers.concat(affixes.map { |affix| AttributeMethodMatcher.new :prefix => affix[:prefix], :suffix => affix[:suffix] }) + undefine_attribute_methods + end + + def define_attribute_methods(attr_names) + return if attribute_methods_generated? + attr_names.each do |name| + attribute_method_matchers.each do |method| + method_name = "#{method.prefix}#{name}#{method.suffix}" + unless instance_method_already_implemented?(method_name) + generate_method = "define_method_#{method.prefix}attribute#{method.suffix}" + + if respond_to?(generate_method) + send(generate_method, name) + else + generated_attribute_methods.module_eval("def #{method_name}(*args); send(:#{method.prefix}attribute#{method.suffix}, '#{name}', *args); end", __FILE__, __LINE__) + end + end + end + end + end + + def undefine_attribute_methods + generated_attribute_methods.module_eval do + instance_methods.each { |m| undef_method(m) } + end + @attribute_methods_generated = nil + end + + def generated_attribute_methods #:nodoc: + @generated_attribute_methods ||= begin + @attribute_methods_generated = true + mod = Module.new + include mod + mod + end + end + + def attribute_methods_generated? + @attribute_methods_generated ? true : false + end + + protected + def instance_method_already_implemented?(method_name) + method_defined?(method_name) + end + + private + class AttributeMethodMatcher + attr_reader :prefix, :suffix + + AttributeMethodMatch = Struct.new(:prefix, :base, :suffix) + + def initialize(options = {}) + options.symbolize_keys! + @prefix, @suffix = options[:prefix] || '', options[:suffix] || '' + @regex = /^(#{Regexp.escape(@prefix)})(.+?)(#{Regexp.escape(@suffix)})$/ + end + + def match(method_name) + if matchdata = @regex.match(method_name) + AttributeMethodMatch.new(matchdata[1], matchdata[2], matchdata[3]) + else + nil + end + end + end + + def attribute_method_matchers #:nodoc: + @@attribute_method_matchers ||= [] + end + end + + # Allows access to the object attributes, which are held in the @attributes hash, as though they + # were first-class methods. So a Person class with a name attribute can use Person#name and + # Person#name= and never directly use the attributes hash -- except for multiple assigns with + # ActiveRecord#attributes=. A Milestone class can also ask Milestone#completed? to test that + # the completed attribute is not +nil+ or 0. + # + # It's also possible to instantiate related objects, so a Client class belonging to the clients + # table with a +master_id+ foreign key can instantiate master through Client#master. + def method_missing(method_id, *args, &block) + method_name = method_id.to_s + if match = match_attribute_method?(method_name) + guard_private_attribute_method!(method_name, args) + return __send__("#{match.prefix}attribute#{match.suffix}", match.base, *args, &block) + end + super + end + + # A Person object with a name attribute can ask person.respond_to?(:name), + # person.respond_to?(:name=), and person.respond_to?(:name?) + # which will all return +true+. + alias :respond_to_without_attributes? :respond_to? + def respond_to?(method, include_private_methods = false) + if super + return true + elsif !include_private_methods && super(method, true) + # If we're here then we haven't found among non-private methods + # but found among all methods. Which means that given method is private. + return false + elsif match_attribute_method?(method.to_s) + return true + end + super + end + + protected + def attribute_method?(attr_name) + attributes.include?(attr_name) + end + + private + # Returns a struct representing the matching attribute method. + # The struct's attributes are prefix, base and suffix. + def match_attribute_method?(method_name) + self.class.send(:attribute_method_matchers).each do |method| + if (match = method.match(method_name)) && attribute_method?(match.base) + return match + end + end + nil + end + + # prevent method_missing from calling private methods with #send + def guard_private_attribute_method!(method_name, args) + if self.class.private_method_defined?(method_name) + raise NoMethodError.new("Attempt to call private method", method_name, args) + end + end + + def missing_attribute(attr_name, stack) + raise ActiveModel::MissingAttributeError, "missing attribute: #{attr_name}", stack + end + end +end diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index be275f5cb6..ab7ad34b9e 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -3,191 +3,13 @@ require 'active_support/core_ext/enumerable' module ActiveRecord module AttributeMethods #:nodoc: extend ActiveSupport::Concern + include ActiveModel::AttributeMethods - class AttributeMethodMatcher - attr_reader :prefix, :suffix - - AttributeMethodMatch = Struct.new(:prefix, :base, :suffix) - - def initialize(options = {}) - options.symbolize_keys! - @prefix, @suffix = options[:prefix] || '', options[:suffix] || '' - @regex = /^(#{Regexp.escape(@prefix)})(.+?)(#{Regexp.escape(@suffix)})$/ - end - - def match(method_name) - if matchdata = @regex.match(method_name) - AttributeMethodMatch.new(matchdata[1], matchdata[2], matchdata[3]) - else - nil - end - end - end - - # Declare and check for suffixed attribute methods. module ClassMethods - # Declares a method available for all attributes with the given prefix. - # Uses +method_missing+ and respond_to? to rewrite the method. - # - # #{prefix}#{attr}(*args, &block) - # - # to - # - # #{prefix}attribute(#{attr}, *args, &block) - # - # An #{prefix}attribute instance method must exist and accept at least - # the +attr+ argument. - # - # For example: - # - # class Person < ActiveRecord::Base - # attribute_method_prefix 'clear_' - # - # private - # def clear_attribute(attr) - # ... - # end - # end - # - # person = Person.find(1) - # person.name # => 'Gem' - # person.clear_name - # person.name # => '' - def attribute_method_prefix(*prefixes) - attribute_method_matchers.concat(prefixes.map { |prefix| AttributeMethodMatcher.new :prefix => prefix }) - undefine_attribute_methods - end - - # Declares a method available for all attributes with the given suffix. - # Uses +method_missing+ and respond_to? to rewrite the method. - # - # #{attr}#{suffix}(*args, &block) - # - # to - # - # attribute#{suffix}(#{attr}, *args, &block) - # - # An attribute#{suffix} instance method must exist and accept at least - # the +attr+ argument. - # - # For example: - # - # class Person < ActiveRecord::Base - # attribute_method_suffix '_short?' - # - # private - # def attribute_short?(attr) - # ... - # end - # end - # - # person = Person.find(1) - # person.name # => 'Gem' - # person.name_short? # => true - def attribute_method_suffix(*suffixes) - attribute_method_matchers.concat(suffixes.map { |suffix| AttributeMethodMatcher.new :suffix => suffix }) - undefine_attribute_methods - end - - # Declares a method available for all attributes with the given prefix - # and suffix. Uses +method_missing+ and respond_to? to rewrite - # the method. - # - # #{prefix}#{attr}#{suffix}(*args, &block) - # - # to - # - # #{prefix}attribute#{suffix}(#{attr}, *args, &block) - # - # An #{prefix}attribute#{suffix} instance method must exist and - # accept at least the +attr+ argument. - # - # For example: - # - # class Person < ActiveRecord::Base - # attribute_method_affix :prefix => 'reset_', :suffix => '_to_default!' - # - # private - # def reset_attribute_to_default!(attr) - # ... - # end - # end - # - # person = Person.find(1) - # person.name # => 'Gem' - # person.reset_name_to_default! - # person.name # => 'Gemma' - def attribute_method_affix(*affixes) - attribute_method_matchers.concat(affixes.map { |affix| AttributeMethodMatcher.new :prefix => affix[:prefix], :suffix => affix[:suffix] }) - undefine_attribute_methods - end - - def matching_attribute_methods(method_name) - attribute_method_matchers.collect { |method| method.match(method_name) }.compact - end - - # Defines an "attribute" method (like +inheritance_column+ or - # +table_name+). A new (class) method will be created with the - # given name. If a value is specified, the new method will - # return that value (as a string). Otherwise, the given block - # will be used to compute the value of the method. - # - # The original method will be aliased, with the new name being - # prefixed with "original_". This allows the new method to - # access the original value. - # - # Example: - # - # class A < ActiveRecord::Base - # define_attr_method :primary_key, "sysid" - # define_attr_method( :inheritance_column ) do - # original_inheritance_column + "_id" - # end - # end - def define_attr_method(name, value=nil, &block) - sing = metaclass - sing.send :alias_method, "original_#{name}", name - if block_given? - sing.send :define_method, name, &block - else - # use eval instead of a block to work around a memory leak in dev - # mode in fcgi - sing.class_eval "def #{name}; #{value.to_s.inspect}; end" - end - end - - def generated_methods #:nodoc: - @generated_methods ||= begin - mod = Module.new - include mod - mod - end - end - # Generates all the attribute related methods for columns in the database # accessors, mutators and query methods. def define_attribute_methods - return unless generated_methods.instance_methods.empty? - columns_hash.keys.each do |name| - attribute_method_matchers.each do |method| - method_name = "#{method.prefix}#{name}#{method.suffix}" - unless instance_method_already_implemented?(method_name) - generate_method = "define_method_#{method.prefix}attribute#{method.suffix}" - - if respond_to?(generate_method) - send(generate_method, name) - else - generated_methods.module_eval("def #{method_name}(*args); send(:#{method.prefix}attribute#{method.suffix}, '#{name}', *args); end", __FILE__, __LINE__) - end - end - end - end - end - - def undefine_attribute_methods - generated_methods.module_eval do - instance_methods.each { |m| undef_method(m) } - end + super(columns_hash.keys) end # Checks whether the method is defined in the model or any of its subclasses @@ -200,83 +22,30 @@ module ActiveRecord raise DangerousAttributeError, "#{method_name} is defined by ActiveRecord" if @@_defined_activerecord_methods.include?(method_name) @_defined_class_methods.include?(method_name) end - - private - # Default to *=, *? and *_before_type_cast - def attribute_method_matchers - @@attribute_method_matchers ||= [] - end end - # Returns a struct representing the matching attribute method. - # The struct's attributes are prefix, base and suffix. - def match_attribute_method?(method_name) - self.class.matching_attribute_methods(method_name).find do |match| - match.base == 'id' || @attributes.include?(match.base) - end - end - - # Allows access to the object attributes, which are held in the @attributes hash, as though they - # were first-class methods. So a Person class with a name attribute can use Person#name and - # Person#name= and never directly use the attributes hash -- except for multiple assigns with - # ActiveRecord#attributes=. A Milestone class can also ask Milestone#completed? to test that - # the completed attribute is not +nil+ or 0. - # - # It's also possible to instantiate related objects, so a Client class belonging to the clients - # table with a +master_id+ foreign key can instantiate master through Client#master. def method_missing(method_id, *args, &block) - method_name = method_id.to_s - # If we haven't generated any methods yet, generate them, then # see if we've created the method we're looking for. - if self.class.generated_methods.instance_methods.empty? + if !self.class.attribute_methods_generated? self.class.define_attribute_methods + method_name = method_id.to_s guard_private_attribute_method!(method_name, args) - if self.class.generated_methods.instance_methods.include?(method_name) + if self.class.generated_attribute_methods.instance_methods.include?(method_name) return self.send(method_id, *args, &block) end end - - if match = match_attribute_method?(method_name) - guard_private_attribute_method!(method_name, args) - return __send__("#{match.prefix}attribute#{match.suffix}", match.base, *args, &block) - end super end - # A Person object with a name attribute can ask person.respond_to?(:name), - # person.respond_to?(:name=), and person.respond_to?(:name?) - # which will all return +true+. - alias :respond_to_without_attributes? :respond_to? - def respond_to?(method, include_private_methods = false) - method_name = method.to_s - if super - return true - elsif !include_private_methods && super(method, true) - # If we're here then we haven't found among non-private methods - # but found among all methods. Which means that given method is private. - return false - elsif self.class.generated_methods.instance_methods.empty? - self.class.define_attribute_methods - if self.class.generated_methods.instance_methods.include?(method_name) - return true - end - elsif match_attribute_method?(method_name) - return true - end + def respond_to?(*args) + self.class.define_attribute_methods super end - private - # prevent method_missing from calling private methods with #send - def guard_private_attribute_method!(method_name, args) - if self.class.private_method_defined?(method_name) - raise NoMethodError.new("Attempt to call private method", method_name, args) - end - end - - def missing_attribute(attr_name, stack) - raise ActiveRecord::MissingAttributeError, "missing attribute: #{attr_name}", stack + protected + def attribute_method?(attr_name) + attr_name == 'id' || attributes.include?(attr_name) end end end diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 90acb769a9..0b7d6d9094 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -51,7 +51,7 @@ module ActiveRecord private # Define read method for serialized attribute. def define_read_method_for_serialized_attribute(attr_name) - generated_methods.module_eval("def #{attr_name}; unserialize_attribute('#{attr_name}'); end", __FILE__, __LINE__) + generated_attribute_methods.module_eval("def #{attr_name}; unserialize_attribute('#{attr_name}'); end", __FILE__, __LINE__) end # Define an attribute reader method. Cope with nil column. @@ -66,7 +66,7 @@ module ActiveRecord if cache_attribute?(attr_name) access_code = "@attributes_cache['#{attr_name}'] ||= (#{access_code})" end - generated_methods.module_eval("def #{symbol}; #{access_code}; end", __FILE__, __LINE__) + generated_attribute_methods.module_eval("def #{symbol}; #{access_code}; end", __FILE__, __LINE__) end end diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index b9cfe59971..a8e3e28a7a 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -25,7 +25,7 @@ module ActiveRecord @attributes_cache['#{attr_name}'] = time.acts_like?(:time) ? time.in_time_zone : time end EOV - generated_methods.module_eval(method_body, __FILE__, __LINE__) + generated_attribute_methods.module_eval(method_body, __FILE__, __LINE__) else super end @@ -44,7 +44,7 @@ module ActiveRecord write_attribute(:#{attr_name}, time) end EOV - generated_methods.module_eval(method_body, __FILE__, __LINE__) + generated_attribute_methods.module_eval(method_body, __FILE__, __LINE__) else super end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index 79118855cf..e31acac050 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -10,7 +10,7 @@ module ActiveRecord module ClassMethods protected def define_method_attribute=(attr_name) - generated_methods.module_eval("def #{attr_name}=(new_value); write_attribute('#{attr_name}', new_value); end", __FILE__, __LINE__) + generated_attribute_methods.module_eval("def #{attr_name}=(new_value); write_attribute('#{attr_name}', new_value); end", __FILE__, __LINE__) end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index ce93ea8eee..e358564ead 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -148,11 +148,6 @@ module ActiveRecord #:nodoc: class DangerousAttributeError < ActiveRecordError end - # Raised when you've tried to access a column which wasn't loaded by your finder. - # Typically this is because :select has been specified. - class MissingAttributeError < NoMethodError - end - # Raised when unknown attributes are supplied via mass assignment. class UnknownAttributeError < NoMethodError end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index a5f4a67200..ab8768ea3e 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -16,53 +16,6 @@ class AttributeMethodsTest < ActiveRecord::TestCase ActiveRecord::Base.send(:attribute_method_matchers).concat(@old_matchers) end - def test_match_attribute_method_query_returns_default_match_data - topic = @target.new(:title => 'Budget') - assert_not_nil match = topic.match_attribute_method?('title=') - assert_equal '', match.prefix - assert_equal 'title', match.base - assert_equal '=', match.suffix - end - - def test_match_attribute_method_query_returns_match_data_for_prefixes - topic = @target.new(:title => 'Budget') - %w(default_ title_).each do |prefix| - @target.class_eval "def #{prefix}attribute(*args) args end" - @target.attribute_method_prefix prefix - - assert_not_nil match = topic.match_attribute_method?("#{prefix}title") - assert_equal prefix, match.prefix - assert_equal 'title', match.base - assert_equal '', match.suffix - end - end - - def test_match_attribute_method_query_returns_match_data_for_suffixes - topic = @target.new(:title => 'Budget') - %w(_default _title_default it! _candidate= _maybe?).each do |suffix| - @target.class_eval "def attribute#{suffix}(*args) args end" - @target.attribute_method_suffix suffix - - assert_not_nil match = topic.match_attribute_method?("title#{suffix}") - assert_equal '', match.prefix - assert_equal 'title', match.base - assert_equal suffix, match.suffix - end - end - - def test_match_attribute_method_query_returns_match_data_for_affixes - topic = @target.new(:title => 'Budget') - [['mark_', '_for_update'], ['reset_', '!'], ['default_', '_value?']].each do |prefix, suffix| - @target.class_eval "def #{prefix}attribute#{suffix}(*args) args end" - @target.attribute_method_affix({ :prefix => prefix, :suffix => suffix }) - - assert_not_nil match = topic.match_attribute_method?("#{prefix}title#{suffix}") - assert_equal prefix, match.prefix - assert_equal 'title', match.base - assert_equal suffix, match.suffix - end - end - def test_undeclared_attribute_method_does_not_affect_respond_to_and_method_missing topic = @target.new(:title => 'Budget') assert topic.respond_to?('title') diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 55ef0d45eb..893fc34c36 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -251,7 +251,7 @@ class FinderTest < ActiveRecord::TestCase def test_find_only_some_columns topic = Topic.find(1, :select => "author_name") - assert_raise(ActiveRecord::MissingAttributeError) {topic.title} + assert_raise(ActiveModel::MissingAttributeError) {topic.title} assert_equal "David", topic.author_name assert !topic.attribute_present?("title") #assert !topic.respond_to?("title") From bfafe8c4055bcb8dcf7440015d95a32c9773f40b Mon Sep 17 00:00:00 2001 From: Geoff Buesing Date: Wed, 5 Aug 2009 08:19:21 -0500 Subject: [PATCH 15/38] Revert "fallback_string_to_date sets Date._parse comp arg to true, so that strings with two-digit years, e.g. '1/1/09', are interpreted as modern years" [#2019 state:wontfix] This reverts commit 55d1d12c32a1b99f3f07d2346b49a63650ba2e9d. --- activerecord/CHANGELOG | 2 -- .../connection_adapters/abstract/schema_definitions.rb | 2 +- activerecord/test/cases/date_time_test.rb | 6 ------ 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 3f350be3e9..9adc6b887f 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,7 +1,5 @@ *Edge* -* fallback_string_to_date sets Date._parse comp arg to true, so that strings with two-digit years, e.g. '1/1/09', are interpreted as modern years #2019 [Matt Ganderup] - * quoted_date converts time-like objects to ActiveRecord::Base.default_timezone before serialization. This allows you to use Time.now in find conditions and have it correctly be serialized as the current time in UTC when default_timezone == :utc. #2946 [Geoff Buesing] * SQLite: drop support for 'dbfile' option in favor of 'database.' #2363 [Paul Hinze, Jeremy Kemper] diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index f4e8fe6a38..24c734cddb 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -201,7 +201,7 @@ module ActiveRecord end def fallback_string_to_date(string) - new_date(*::Date._parse(string, true).values_at(:year, :mon, :mday)) + new_date(*::Date._parse(string, false).values_at(:year, :mon, :mday)) end def fallback_string_to_time(string) diff --git a/activerecord/test/cases/date_time_test.rb b/activerecord/test/cases/date_time_test.rb index f7527c0c04..36e1caa0b6 100644 --- a/activerecord/test/cases/date_time_test.rb +++ b/activerecord/test/cases/date_time_test.rb @@ -34,10 +34,4 @@ class DateTimeTest < ActiveRecord::TestCase topic.bonus_time = '' assert_nil topic.bonus_time end - - def test_two_digit_year - topic = Topic.new - topic.last_read = '1/1/09' - assert_equal Date.new(2009,1,1), topic.last_read - end end From 64268a0b06d32567c6e88b7293f332b79e10414b Mon Sep 17 00:00:00 2001 From: Matthew Rudy Jacobs Date: Wed, 5 Aug 2009 15:58:55 +0100 Subject: [PATCH 16/38] Make sure javascript_include_tag/stylesheet_link_tag does not append ".js" or ".css" onto external urls [#1664 state:resolved] Signed-off-by: Pratik Naik --- actionpack/CHANGELOG | 2 ++ .../action_view/helpers/asset_tag_helper.rb | 24 +++++++++++-------- .../test/template/asset_tag_helper_test.rb | 8 +++++-- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index aab26b411c..75de1fe2a6 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Make sure javascript_include_tag/stylesheet_link_tag does not append ".js" or ".css" onto external urls. #1664 [Matthew Rudy Jacobs] + * Ruby 1.9: fix Content-Length for multibyte send_data streaming. #2661 [Sava Chankov] * Ruby 1.9: ERB template encoding using a magic comment at the top of the file. [Jeremy Kemper] diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 081003bcf3..c71840d41f 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -171,7 +171,7 @@ module ActionView end # Computes the path to a javascript asset in the public javascripts directory. - # If the +source+ filename has no extension, .js will be appended. + # If the +source+ filename has no extension, .js will be appended (except for explicit URIs) # Full paths from the document root will be passed through. # Used internally by javascript_include_tag to build the script path. # @@ -179,7 +179,7 @@ module ActionView # javascript_path "xmlhr" # => /javascripts/xmlhr.js # javascript_path "dir/xmlhr.js" # => /javascripts/dir/xmlhr.js # javascript_path "/dir/xmlhr" # => /dir/xmlhr.js - # javascript_path "http://www.railsapplication.com/js/xmlhr" # => http://www.railsapplication.com/js/xmlhr.js + # javascript_path "http://www.railsapplication.com/js/xmlhr" # => http://www.railsapplication.com/js/xmlhr # javascript_path "http://www.railsapplication.com/js/xmlhr.js" # => http://www.railsapplication.com/js/xmlhr.js def javascript_path(source) compute_public_path(source, 'javascripts', 'js') @@ -337,7 +337,7 @@ module ActionView end # Computes the path to a stylesheet asset in the public stylesheets directory. - # If the +source+ filename has no extension, .css will be appended. + # If the +source+ filename has no extension, .css will be appended (except for explicit URIs). # Full paths from the document root will be passed through. # Used internally by +stylesheet_link_tag+ to build the stylesheet path. # @@ -345,8 +345,8 @@ module ActionView # stylesheet_path "style" # => /stylesheets/style.css # stylesheet_path "dir/style.css" # => /stylesheets/dir/style.css # stylesheet_path "/dir/style.css" # => /dir/style.css - # stylesheet_path "http://www.railsapplication.com/css/style" # => http://www.railsapplication.com/css/style.css - # stylesheet_path "http://www.railsapplication.com/css/style.js" # => http://www.railsapplication.com/css/style.css + # stylesheet_path "http://www.railsapplication.com/css/style" # => http://www.railsapplication.com/css/style + # stylesheet_path "http://www.railsapplication.com/css/style.css" # => http://www.railsapplication.com/css/style.css def stylesheet_path(source) compute_public_path(source, 'stylesheets', 'css') end @@ -629,11 +629,11 @@ module ActionView has_request = @controller.respond_to?(:request) source_ext = File.extname(source)[1..-1] - if ext && (source_ext.blank? || (ext != source_ext && File.exist?(File.join(ASSETS_DIR, dir, "#{source}.#{ext}")))) + if ext && !is_uri?(source) && (source_ext.blank? || (ext != source_ext && File.exist?(File.join(ASSETS_DIR, dir, "#{source}.#{ext}")))) source += ".#{ext}" end - unless source =~ %r{^[-a-z]+://} + unless is_uri?(source) source = "/#{dir}/#{source}" unless source[0] == ?/ source = rewrite_asset_path(source) @@ -645,10 +645,10 @@ module ActionView end end - if include_host && source !~ %r{^[-a-z]+://} + if include_host && !is_uri?(source) host = compute_asset_host(source) - if has_request && !host.blank? && host !~ %r{^[-a-z]+://} + if has_request && !host.blank? && !is_uri?(host) host = "#{@controller.request.protocol}#{host}" end @@ -658,6 +658,10 @@ module ActionView end end + def is_uri?(path) + path =~ %r{^[-a-z]+://} + end + # Pick an asset host for this source. Returns +nil+ if no host is set, # the host if no wildcard is set, the host interpolated with the # numbers 0-3 if it contains %d (the number is the source hash mod 4), @@ -798,7 +802,7 @@ module ActionView end def asset_file_path!(path) - unless path =~ %r{^[-a-z]+://} + unless is_uri?(path) absolute_path = asset_file_path(path) raise(Errno::ENOENT, "Asset file not found at '#{absolute_path}'" ) unless File.exist?(absolute_path) return absolute_path diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 08963946ab..28f9d48671 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -83,7 +83,10 @@ class AssetTagHelperTest < ActionView::TestCase %(javascript_include_tag(:all)) => %(\n\n\n\n\n\n\n), %(javascript_include_tag(:all, :recursive => true)) => %(\n\n\n\n\n\n\n\n), %(javascript_include_tag(:defaults, "bank")) => %(\n\n\n\n\n), - %(javascript_include_tag("bank", :defaults)) => %(\n\n\n\n\n) + %(javascript_include_tag("bank", :defaults)) => %(\n\n\n\n\n), + + %(javascript_include_tag("http://example.com/all")) => %(), + %(javascript_include_tag("http://example.com/all.js")) => %(), } StylePathToTag = { @@ -111,7 +114,8 @@ class AssetTagHelperTest < ActionView::TestCase %(stylesheet_link_tag(:all, :media => "all")) => %(\n\n), %(stylesheet_link_tag("random.styles", "/elsewhere/file")) => %(\n), - %(stylesheet_link_tag("http://www.example.com/styles/style")) => %() + %(stylesheet_link_tag("http://www.example.com/styles/style")) => %(), + %(stylesheet_link_tag("http://www.example.com/styles/style.css")) => %(), } ImagePathToTag = { From cfd421daa2b04216e27d666361eb4053020e027d Mon Sep 17 00:00:00 2001 From: James Hill Date: Wed, 5 Aug 2009 11:44:44 -0500 Subject: [PATCH 17/38] Allow validations to use values from custom readers [#2936 state:resolved] Signed-off-by: Joshua Peek --- activemodel/lib/active_model/errors.rb | 6 ++--- activemodel/lib/active_model/validations.rb | 24 ++++++++++++++++++- .../validations/presence_validation_test.rb | 14 +++++++++++ activemodel/test/cases/validations_test.rb | 14 +++++++++++ activemodel/test/models/custom_reader.rb | 17 +++++++++++++ 5 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 activemodel/test/models/custom_reader.rb diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index a4cf700231..a47d89ac0e 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -68,7 +68,7 @@ module ActiveModel # Will add an error message to each of the attributes in +attributes+ that is empty. def add_on_empty(attributes, custom_message = nil) [attributes].flatten.each do |attribute| - value = @base.send(attribute) + value = @base.instance_eval { read_attribute_for_validation(attribute) } is_empty = value.respond_to?(:empty?) ? value.empty? : false add(attribute, :empty, :default => custom_message) unless !value.nil? && !is_empty end @@ -77,7 +77,7 @@ module ActiveModel # Will add an error message to each of the attributes in +attributes+ that is blank (using Object#blank?). def add_on_blank(attributes, custom_message = nil) [attributes].flatten.each do |attribute| - value = @base.send(attribute) + value = @base.instance_eval { read_attribute_for_validation(attribute) } add(attribute, :blank, :default => custom_message) if value.blank? end end @@ -146,7 +146,7 @@ module ActiveModel defaults = defaults.compact.flatten << :"messages.#{message}" key = defaults.shift - value = @base.send(attribute) + value = @base.instance_eval { read_attribute_for_validation(attribute) } options = { :default => defaults, :model => @base.class.name.humanize, diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index 54a869396d..0fca96e5cc 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -66,7 +66,7 @@ module ActiveModel # Declare the validation. send(validation_method(options[:on]), options) do |record| attrs.each do |attr| - value = record.send(attr) + value = record.instance_eval { read_attribute_for_validation(attr) } next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank]) yield record, attr, value end @@ -95,6 +95,28 @@ module ActiveModel def invalid? !valid? end + + protected + # Hook method defining how an attribute value should be retieved. By default this is assumed + # to be an instance named after the attribute. Override this method in subclasses should you + # need to retrieve the value for a given attribute differently e.g. + # class MyClass + # include ActiveModel::Validations + # + # def initialize(data = {}) + # @data = data + # end + # + # private + # + # def read_attribute_for_validation(key) + # @data[key] + # end + # end + # + def read_attribute_for_validation(key) + send(key) + end end end diff --git a/activemodel/test/cases/validations/presence_validation_test.rb b/activemodel/test/cases/validations/presence_validation_test.rb index aa5bdf1e62..bb6fb91774 100644 --- a/activemodel/test/cases/validations/presence_validation_test.rb +++ b/activemodel/test/cases/validations/presence_validation_test.rb @@ -54,4 +54,18 @@ class PresenceValidationTest < ActiveModel::TestCase assert p.valid? end end + + def test_validates_presence_of_for_ruby_class_with_custom_reader + repair_validations(Person) do + CustomReader.validates_presence_of :karma + + p = CustomReader.new + assert p.invalid? + + assert_equal ["can't be blank"], p.errors[:karma] + + p[:karma] = "Cold" + assert p.valid? + end + end end diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb index 8c89494247..0b340e68bf 100644 --- a/activemodel/test/cases/validations_test.rb +++ b/activemodel/test/cases/validations_test.rb @@ -5,6 +5,7 @@ require 'cases/tests_database' require 'models/topic' require 'models/reply' require 'models/developer' +require 'models/custom_reader' class ValidationsTest < ActiveModel::TestCase include ActiveModel::TestsDatabase @@ -97,6 +98,19 @@ class ValidationsTest < ActiveModel::TestCase assert_equal %w(gotcha gotcha), t.errors[:title] assert_equal %w(gotcha gotcha), t.errors[:content] end + + def test_validates_each_custom_reader + hits = 0 + CustomReader.validates_each(:title, :content, [:title, :content]) do |record, attr| + record.errors.add attr, 'gotcha' + hits += 1 + end + t = CustomReader.new("title" => "valid", "content" => "whatever") + assert !t.valid? + assert_equal 4, hits + assert_equal %w(gotcha gotcha), t.errors[:title] + assert_equal %w(gotcha gotcha), t.errors[:content] + end def test_validate_block Topic.validate { |topic| topic.errors.add("title", "will never be valid") } diff --git a/activemodel/test/models/custom_reader.rb b/activemodel/test/models/custom_reader.rb new file mode 100644 index 0000000000..7ac70e6167 --- /dev/null +++ b/activemodel/test/models/custom_reader.rb @@ -0,0 +1,17 @@ +class CustomReader + include ActiveModel::Validations + + def initialize(data = {}) + @data = data + end + + def []=(key, value) + @data[key] = value + end + + private + + def read_attribute_for_validation(key) + @data[key] + end +end \ No newline at end of file From b7052b8dc33d5769e94dfcec62d439c2dd18a8b9 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Wed, 5 Aug 2009 23:00:17 +0100 Subject: [PATCH 18/38] Dont require thin as the thin rack adapter is now upstream Signed-off-by: Pratik Naik --- railties/lib/commands/server.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/railties/lib/commands/server.rb b/railties/lib/commands/server.rb index 01dd33fa8c..823916b1dc 100644 --- a/railties/lib/commands/server.rb +++ b/railties/lib/commands/server.rb @@ -3,13 +3,6 @@ require 'action_controller' require 'fileutils' require 'optparse' -# TODO: Push Thin adapter upstream so we don't need worry about requiring it -begin - require_library_or_gem 'thin' -rescue Exception - # Thin not available -end - options = { :Port => 3000, :Host => "0.0.0.0", From 5ce3831faf684aea75948ce4602b6b9de361c11e Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Thu, 6 Aug 2009 00:11:28 +0100 Subject: [PATCH 19/38] Use send instead of instance_eval --- activemodel/lib/active_model/errors.rb | 6 +++--- activemodel/lib/active_model/validations.rb | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index a47d89ac0e..7a3001174f 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -68,7 +68,7 @@ module ActiveModel # Will add an error message to each of the attributes in +attributes+ that is empty. def add_on_empty(attributes, custom_message = nil) [attributes].flatten.each do |attribute| - value = @base.instance_eval { read_attribute_for_validation(attribute) } + value = @base.send(:read_attribute_for_validation, attribute) is_empty = value.respond_to?(:empty?) ? value.empty? : false add(attribute, :empty, :default => custom_message) unless !value.nil? && !is_empty end @@ -77,7 +77,7 @@ module ActiveModel # Will add an error message to each of the attributes in +attributes+ that is blank (using Object#blank?). def add_on_blank(attributes, custom_message = nil) [attributes].flatten.each do |attribute| - value = @base.instance_eval { read_attribute_for_validation(attribute) } + value = @base.send(:read_attribute_for_validation, attribute) add(attribute, :blank, :default => custom_message) if value.blank? end end @@ -146,7 +146,7 @@ module ActiveModel defaults = defaults.compact.flatten << :"messages.#{message}" key = defaults.shift - value = @base.instance_eval { read_attribute_for_validation(attribute) } + value = @base.send(:read_attribute_for_validation, attribute) options = { :default => defaults, :model => @base.class.name.humanize, diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index 0fca96e5cc..7d49e60790 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -66,7 +66,7 @@ module ActiveModel # Declare the validation. send(validation_method(options[:on]), options) do |record| attrs.each do |attr| - value = record.instance_eval { read_attribute_for_validation(attr) } + value = record.send(:read_attribute_for_validation, attr) next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank]) yield record, attr, value end From 230d43fbf5dee569ea031c8c394ba9ce70804cae Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Thu, 6 Aug 2009 08:34:23 +0900 Subject: [PATCH 20/38] Ruby 1.9.2 compat: Array#* uses to_str instead of to_s to join values since Ruby 1.9.2 [#2959 state:committed] Signed-off-by: Jeremy Kemper --- .../connection_adapters/abstract/schema_definitions.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 24c734cddb..f346e3ebc8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -277,7 +277,6 @@ module ActiveRecord add_column_options!(column_sql, column_options) unless type.to_sym == :primary_key column_sql end - alias to_s :to_sql private @@ -508,7 +507,7 @@ module ActiveRecord # concatenated together. This string can then be prepended and appended to # to generate the final SQL to create the table. def to_sql - @columns * ', ' + @columns.map(&:to_sql) * ', ' end private From f0945409d935cdd3cb783a728d68414e7ca02dfc Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 6 Aug 2009 19:43:28 -0300 Subject: [PATCH 21/38] replace _render_*_from_controller with render_* as they are intended to be public --- actionpack/examples/very_simple.rb | 2 +- actionpack/lib/action_controller/abstract/renderer.rb | 8 ++++---- actionpack/lib/action_view/context.rb | 4 ++-- actionpack/lib/action_view/render/partials.rb | 2 +- actionpack/lib/action_view/render/rendering.rb | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/actionpack/examples/very_simple.rb b/actionpack/examples/very_simple.rb index 8d01cb39bd..e0f2d2bde8 100644 --- a/actionpack/examples/very_simple.rb +++ b/actionpack/examples/very_simple.rb @@ -23,7 +23,7 @@ class Kaigi < ActionController::Http DEFAULT_LAYOUT = Object.new.tap {|l| def l.render(*) yield end } - def _render_template_from_controller(template, layout = DEFAULT_LAYOUT, options = {}, partial = false) + def render_template(template, layout = DEFAULT_LAYOUT, options = {}, partial = false) ret = template.render(self, {}) layout.render(self, {}) { ret } end diff --git a/actionpack/lib/action_controller/abstract/renderer.rb b/actionpack/lib/action_controller/abstract/renderer.rb index 41b7d47458..f3e3903a3b 100644 --- a/actionpack/lib/action_controller/abstract/renderer.rb +++ b/actionpack/lib/action_controller/abstract/renderer.rb @@ -19,11 +19,11 @@ module AbstractController # The view class must have the following methods: # View.for_controller[controller] Create a new ActionView instance for a # controller - # View#_render_partial_from_controller[options] + # View#render_partial[options] # - responsible for setting options[:_template] # - Returns String with the rendered partial # options:: see _render_partial in ActionView::Base - # View#_render_template_from_controller[template, layout, options, partial] + # View#render_template[template, layout, options, partial] # - Returns String with the rendered template # template:: The template to render # layout:: The layout to render around the template @@ -55,7 +55,7 @@ module AbstractController def render_to_body(options = {}) # TODO: Refactor so we can just use the normal template logic for this if options[:_partial_object] - _action_view._render_partial_from_controller(options) + _action_view.render_partial(options) else _determine_template(options) _render_template(options) @@ -77,7 +77,7 @@ module AbstractController # _layout:: The layout to wrap the template in (optional) # _partial:: Whether or not the template to be rendered is a partial def _render_template(options) - _action_view._render_template_from_controller(options[:_template], options[:_layout], options, options[:_partial]) + _action_view.render_template(options[:_template], options[:_layout], options, options[:_partial]) end # The list of view paths for this controller. See ActionView::ViewPathSet for diff --git a/actionpack/lib/action_view/context.rb b/actionpack/lib/action_view/context.rb index f212fe25eb..df078a7151 100644 --- a/actionpack/lib/action_view/context.rb +++ b/actionpack/lib/action_view/context.rb @@ -12,11 +12,11 @@ module ActionView # # Context.for_controller[controller] Create a new ActionView instance for a # controller - # Context#_render_partial_from_controller[options] + # Context#render_partial[options] # - responsible for setting options[:_template] # - Returns String with the rendered partial # options:: see _render_partial in ActionView::Base - # Context#_render_template_from_controller[template, layout, options, partial] + # Context#render_template[template, layout, options, partial] # - Returns String with the rendered template # template:: The template to render # layout:: The layout to render around the template diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index ccb14d513a..8bdff0f3b9 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -177,7 +177,7 @@ module ActionView attr_accessor :_partial end - def _render_partial_from_controller(*args) + def render_partial(*args) @assigns_added = false _render_partial(*args) end diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 162e38c484..86a59dd1bc 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -123,7 +123,7 @@ module ActionView layout ? _render_content_with_layout(text, layout, options[:locals]) : text end - def _render_template_from_controller(*args) + def render_template(*args) @assigns_added = nil _render_template_with_layout(*args) end From af375a5eb3aba149590be1636480e1c3976c124f Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 6 Aug 2009 19:45:40 -0300 Subject: [PATCH 22/38] Replace _action_view with view_context to reflect that it is public and that it does not need to be an ActionView instance --- actionpack/examples/very_simple.rb | 2 +- actionpack/lib/action_controller/abstract/renderer.rb | 8 ++++---- actionpack/lib/action_controller/base/compatibility.rb | 4 ++-- actionpack/lib/action_controller/base/render_options.rb | 2 +- actionpack/lib/action_controller/caching/actions.rb | 3 +-- actionpack/test/abstract_unit.rb | 2 +- 6 files changed, 10 insertions(+), 11 deletions(-) diff --git a/actionpack/examples/very_simple.rb b/actionpack/examples/very_simple.rb index e0f2d2bde8..995e40adb9 100644 --- a/actionpack/examples/very_simple.rb +++ b/actionpack/examples/very_simple.rb @@ -13,7 +13,7 @@ class Kaigi < ActionController::Http before_filter :set_name append_view_path "views" - def _action_view + def view_context self end diff --git a/actionpack/lib/action_controller/abstract/renderer.rb b/actionpack/lib/action_controller/abstract/renderer.rb index f3e3903a3b..fe556281ab 100644 --- a/actionpack/lib/action_controller/abstract/renderer.rb +++ b/actionpack/lib/action_controller/abstract/renderer.rb @@ -31,8 +31,8 @@ module AbstractController # partial:: Whether or not the template to render is a partial # # Override this method in a to change the default behavior. - def _action_view - @_action_view ||= ActionView::Base.for_controller(self) + def view_context + @_view_context ||= ActionView::Base.for_controller(self) end # Mostly abstracts the fact that calling render twice is a DoubleRenderError. @@ -55,7 +55,7 @@ module AbstractController def render_to_body(options = {}) # TODO: Refactor so we can just use the normal template logic for this if options[:_partial_object] - _action_view.render_partial(options) + view_context.render_partial(options) else _determine_template(options) _render_template(options) @@ -77,7 +77,7 @@ module AbstractController # _layout:: The layout to wrap the template in (optional) # _partial:: Whether or not the template to be rendered is a partial def _render_template(options) - _action_view.render_template(options[:_template], options[:_layout], options, options[:_partial]) + view_context.render_template(options[:_template], options[:_layout], options, options[:_partial]) end # The list of view paths for this controller. See ActionView::ViewPathSet for diff --git a/actionpack/lib/action_controller/base/compatibility.rb b/actionpack/lib/action_controller/base/compatibility.rb index 13813ffd17..23e7b1b3af 100644 --- a/actionpack/lib/action_controller/base/compatibility.rb +++ b/actionpack/lib/action_controller/base/compatibility.rb @@ -72,7 +72,7 @@ module ActionController # TODO: Remove this after we flip def template - @template ||= _action_view + @template ||= view_context end def process_action(*) @@ -141,7 +141,7 @@ module ActionController end def view_paths - _action_view.view_paths + view_context.view_paths end end end diff --git a/actionpack/lib/action_controller/base/render_options.rb b/actionpack/lib/action_controller/base/render_options.rb index fc9a02626f..65ee09883e 100644 --- a/actionpack/lib/action_controller/base/render_options.rb +++ b/actionpack/lib/action_controller/base/render_options.rb @@ -85,7 +85,7 @@ module ActionController register_renderer :update def _render_update(proc, options) - generator = ActionView::Helpers::PrototypeHelper::JavaScriptGenerator.new(_action_view, &proc) + generator = ActionView::Helpers::PrototypeHelper::JavaScriptGenerator.new(view_context, &proc) self.content_type = Mime::JS self.response_body = generator.to_s end diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index d8a1662acc..cb0c3a1384 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -129,8 +129,7 @@ module ActionController #:nodoc: end def content_for_layout(controller) - # TODO: Remove this when new base is merged in - template = controller.respond_to?(:template) ? controller.template : controller._action_view + template = controller.view_context template.layout && template.instance_variable_get('@cached_content_for_layout') end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 6e71b85645..7f373ea7c5 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -78,7 +78,7 @@ module ActionController def assert_template(options = {}, message = nil) validate_request! - hax = @controller._action_view.instance_variable_get(:@_rendered) + hax = @controller.view_context.instance_variable_get(:@_rendered) case options when NilClass, String From 52798fd479d4acbf823d093b03bdd1acf8e86b62 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 6 Aug 2009 19:50:22 -0300 Subject: [PATCH 23/38] rename ActionController::Http to ActionController::Metal at Josh's suggestion --- actionpack/examples/minimal.rb | 8 +++----- actionpack/examples/very_simple.rb | 2 +- actionpack/lib/action_controller.rb | 2 +- actionpack/lib/action_controller/abstract/base.rb | 2 +- actionpack/lib/action_controller/base/base.rb | 2 +- .../lib/action_controller/base/{http.rb => metal.rb} | 6 +++--- 6 files changed, 10 insertions(+), 12 deletions(-) rename actionpack/lib/action_controller/base/{http.rb => metal.rb} (92%) diff --git a/actionpack/examples/minimal.rb b/actionpack/examples/minimal.rb index 9eb92cd8e7..7106149fa2 100644 --- a/actionpack/examples/minimal.rb +++ b/actionpack/examples/minimal.rb @@ -45,11 +45,9 @@ end OK = [200, {}, []] MetalPostController = lambda { OK } -if ActionController.const_defined?(:Http) - class HttpPostController < ActionController::Http - def index - self.response_body = '' - end +class HttpPostController < ActionController::Metal + def index + self.response_body = '' end end diff --git a/actionpack/examples/very_simple.rb b/actionpack/examples/very_simple.rb index 995e40adb9..422c4c3298 100644 --- a/actionpack/examples/very_simple.rb +++ b/actionpack/examples/very_simple.rb @@ -3,7 +3,7 @@ $:.push "rails/actionpack/lib" require "action_controller" -class Kaigi < ActionController::Http +class Kaigi < ActionController::Metal include AbstractController::Callbacks include ActionController::RackConvenience include ActionController::Renderer diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 32572c93c0..b6bf620ffe 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -2,7 +2,7 @@ module ActionController autoload :Base, "action_controller/base/base" autoload :ConditionalGet, "action_controller/base/conditional_get" autoload :HideActions, "action_controller/base/hide_actions" - autoload :Http, "action_controller/base/http" + autoload :Metal, "action_controller/base/metal" autoload :Layouts, "action_controller/base/layouts" autoload :RackConvenience, "action_controller/base/rack_convenience" autoload :Rails2Compatibility, "action_controller/base/compatibility" diff --git a/actionpack/lib/action_controller/abstract/base.rb b/actionpack/lib/action_controller/abstract/base.rb index ca00e66349..b93e6ce634 100644 --- a/actionpack/lib/action_controller/abstract/base.rb +++ b/actionpack/lib/action_controller/abstract/base.rb @@ -30,7 +30,7 @@ module AbstractController # instance methods on that abstract class. Public instance methods of # a controller would normally be considered action methods, so we # are removing those methods on classes declared as abstract - # (ActionController::Http and ActionController::Base are defined + # (ActionController::Metal and ActionController::Base are defined # as abstract) def internal_methods controller = self diff --git a/actionpack/lib/action_controller/base/base.rb b/actionpack/lib/action_controller/base/base.rb index e541d24e31..9d9f735e27 100644 --- a/actionpack/lib/action_controller/base/base.rb +++ b/actionpack/lib/action_controller/base/base.rb @@ -1,5 +1,5 @@ module ActionController - class Base < Http + class Base < Metal abstract! include AbstractController::Benchmarker diff --git a/actionpack/lib/action_controller/base/http.rb b/actionpack/lib/action_controller/base/metal.rb similarity index 92% rename from actionpack/lib/action_controller/base/http.rb rename to actionpack/lib/action_controller/base/metal.rb index 3efd1b656f..e7d776b63e 100644 --- a/actionpack/lib/action_controller/base/http.rb +++ b/actionpack/lib/action_controller/base/metal.rb @@ -1,13 +1,13 @@ require 'action_controller/abstract' module ActionController - # ActionController::Http provides a way to get a valid Rack application from a controller. + # ActionController::Metal provides a way to get a valid Rack application from a controller. # # In AbstractController, dispatching is triggered directly by calling #process on a new controller. - # ActionController::Http provides an #action method that returns a valid Rack application for a + # ActionController::Metal provides an #action method that returns a valid Rack application for a # given action. Other rack builders, such as Rack::Builder, Rack::URLMap, and the Rails router, # can dispatch directly to the action returned by FooController.action(:index). - class Http < AbstractController::Base + class Metal < AbstractController::Base abstract! # :api: public From bd6b61be88dfe6eb1ff1dcc5c17542d804a842c7 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 6 Aug 2009 19:52:11 -0300 Subject: [PATCH 24/38] Rename /base to /metal and make base.rb and metal.rb top-level to reflect their module locations --- actionpack/lib/action_controller.rb | 72 +++++++++---------- .../lib/action_controller/{base => }/base.rb | 0 .../lib/action_controller/{base => }/metal.rb | 0 .../{base => metal}/compatibility.rb | 0 .../{base => metal}/conditional_get.rb | 0 .../{base => metal}/cookies.rb | 0 .../{base => metal}/exceptions.rb | 0 .../filter_parameter_logging.rb | 0 .../{base => metal}/flash.rb | 0 .../{base => metal}/helpers.rb | 0 .../{base => metal}/hide_actions.rb | 0 .../{base => metal}/http_authentication.rb | 0 .../{base => metal}/layouts.rb | 0 .../{base => metal}/mime_responds.rb | 0 .../{base => metal}/rack_convenience.rb | 0 .../{base => metal}/redirector.rb | 0 .../{base => metal}/render_options.rb | 0 .../{base => metal}/renderer.rb | 0 .../request_forgery_protection.rb | 0 .../{base => metal}/rescuable.rb | 0 .../{base => metal}/session.rb | 0 .../{base => metal}/session_management.rb | 0 .../{base => metal}/streaming.rb | 0 .../{base => metal}/testing.rb | 0 .../{base => metal}/url_for.rb | 0 .../{base => metal}/verification.rb | 0 26 files changed, 36 insertions(+), 36 deletions(-) rename actionpack/lib/action_controller/{base => }/base.rb (100%) rename actionpack/lib/action_controller/{base => }/metal.rb (100%) rename actionpack/lib/action_controller/{base => metal}/compatibility.rb (100%) rename actionpack/lib/action_controller/{base => metal}/conditional_get.rb (100%) rename actionpack/lib/action_controller/{base => metal}/cookies.rb (100%) rename actionpack/lib/action_controller/{base => metal}/exceptions.rb (100%) rename actionpack/lib/action_controller/{base => metal}/filter_parameter_logging.rb (100%) rename actionpack/lib/action_controller/{base => metal}/flash.rb (100%) rename actionpack/lib/action_controller/{base => metal}/helpers.rb (100%) rename actionpack/lib/action_controller/{base => metal}/hide_actions.rb (100%) rename actionpack/lib/action_controller/{base => metal}/http_authentication.rb (100%) rename actionpack/lib/action_controller/{base => metal}/layouts.rb (100%) rename actionpack/lib/action_controller/{base => metal}/mime_responds.rb (100%) rename actionpack/lib/action_controller/{base => metal}/rack_convenience.rb (100%) rename actionpack/lib/action_controller/{base => metal}/redirector.rb (100%) rename actionpack/lib/action_controller/{base => metal}/render_options.rb (100%) rename actionpack/lib/action_controller/{base => metal}/renderer.rb (100%) rename actionpack/lib/action_controller/{base => metal}/request_forgery_protection.rb (100%) rename actionpack/lib/action_controller/{base => metal}/rescuable.rb (100%) rename actionpack/lib/action_controller/{base => metal}/session.rb (100%) rename actionpack/lib/action_controller/{base => metal}/session_management.rb (100%) rename actionpack/lib/action_controller/{base => metal}/streaming.rb (100%) rename actionpack/lib/action_controller/{base => metal}/testing.rb (100%) rename actionpack/lib/action_controller/{base => metal}/url_for.rb (100%) rename actionpack/lib/action_controller/{base => metal}/verification.rb (100%) diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index b6bf620ffe..2b2c7ef725 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -1,56 +1,56 @@ module ActionController - autoload :Base, "action_controller/base/base" - autoload :ConditionalGet, "action_controller/base/conditional_get" - autoload :HideActions, "action_controller/base/hide_actions" - autoload :Metal, "action_controller/base/metal" - autoload :Layouts, "action_controller/base/layouts" - autoload :RackConvenience, "action_controller/base/rack_convenience" - autoload :Rails2Compatibility, "action_controller/base/compatibility" - autoload :Redirector, "action_controller/base/redirector" - autoload :Renderer, "action_controller/base/renderer" - autoload :RenderOptions, "action_controller/base/render_options" - autoload :Renderers, "action_controller/base/render_options" - autoload :Rescue, "action_controller/base/rescuable" - autoload :Testing, "action_controller/base/testing" - autoload :UrlFor, "action_controller/base/url_for" - autoload :Session, "action_controller/base/session" - autoload :Helpers, "action_controller/base/helpers" + autoload :Base, "action_controller/base" + autoload :ConditionalGet, "action_controller/metal/conditional_get" + autoload :HideActions, "action_controller/metal/hide_actions" + autoload :Metal, "action_controller/metal" + autoload :Layouts, "action_controller/metal/layouts" + autoload :RackConvenience, "action_controller/metal/rack_convenience" + autoload :Rails2Compatibility, "action_controller/metal/compatibility" + autoload :Redirector, "action_controller/metal/redirector" + autoload :Renderer, "action_controller/metal/renderer" + autoload :RenderOptions, "action_controller/metal/render_options" + autoload :Renderers, "action_controller/metal/render_options" + autoload :Rescue, "action_controller/metal/rescuable" + autoload :Testing, "action_controller/metal/testing" + autoload :UrlFor, "action_controller/metal/url_for" + autoload :Session, "action_controller/metal/session" + autoload :Helpers, "action_controller/metal/helpers" # Ported modules # require 'action_controller/routing' autoload :Caching, 'action_controller/caching' autoload :Dispatcher, 'action_controller/dispatch/dispatcher' autoload :Integration, 'action_controller/testing/integration' - autoload :MimeResponds, 'action_controller/base/mime_responds' + autoload :MimeResponds, 'action_controller/metal/mime_responds' autoload :PolymorphicRoutes, 'action_controller/routing/generation/polymorphic_routes' autoload :RecordIdentifier, 'action_controller/record_identifier' autoload :Resources, 'action_controller/routing/resources' - autoload :SessionManagement, 'action_controller/base/session_management' + autoload :SessionManagement, 'action_controller/metal/session_management' autoload :TestCase, 'action_controller/testing/test_case' autoload :TestProcess, 'action_controller/testing/process' autoload :UrlRewriter, 'action_controller/routing/generation/url_rewriter' autoload :UrlWriter, 'action_controller/routing/generation/url_rewriter' - autoload :Verification, 'action_controller/base/verification' - autoload :Flash, 'action_controller/base/flash' - autoload :RequestForgeryProtection, 'action_controller/base/request_forgery_protection' - autoload :Streaming, 'action_controller/base/streaming' - autoload :HttpAuthentication, 'action_controller/base/http_authentication' - autoload :FilterParameterLogging, 'action_controller/base/filter_parameter_logging' + autoload :Verification, 'action_controller/metal/verification' + autoload :Flash, 'action_controller/metal/flash' + autoload :RequestForgeryProtection, 'action_controller/metal/request_forgery_protection' + autoload :Streaming, 'action_controller/metal/streaming' + autoload :HttpAuthentication, 'action_controller/metal/http_authentication' + autoload :FilterParameterLogging, 'action_controller/metal/filter_parameter_logging' autoload :Translation, 'action_controller/translation' - autoload :Cookies, 'action_controller/base/cookies' + autoload :Cookies, 'action_controller/metal/cookies' - autoload :ActionControllerError, 'action_controller/base/exceptions' - autoload :SessionRestoreError, 'action_controller/base/exceptions' - autoload :RenderError, 'action_controller/base/exceptions' - autoload :RoutingError, 'action_controller/base/exceptions' - autoload :MethodNotAllowed, 'action_controller/base/exceptions' - autoload :NotImplemented, 'action_controller/base/exceptions' - autoload :UnknownController, 'action_controller/base/exceptions' - autoload :MissingFile, 'action_controller/base/exceptions' - autoload :RenderError, 'action_controller/base/exceptions' - autoload :SessionOverflowError, 'action_controller/base/exceptions' - autoload :UnknownHttpMethod, 'action_controller/base/exceptions' + autoload :ActionControllerError, 'action_controller/metal/exceptions' + autoload :SessionRestoreError, 'action_controller/metal/exceptions' + autoload :RenderError, 'action_controller/metal/exceptions' + autoload :RoutingError, 'action_controller/metal/exceptions' + autoload :MethodNotAllowed, 'action_controller/metal/exceptions' + autoload :NotImplemented, 'action_controller/metal/exceptions' + autoload :UnknownController, 'action_controller/metal/exceptions' + autoload :MissingFile, 'action_controller/metal/exceptions' + autoload :RenderError, 'action_controller/metal/exceptions' + autoload :SessionOverflowError, 'action_controller/metal/exceptions' + autoload :UnknownHttpMethod, 'action_controller/metal/exceptions' autoload :Routing, 'action_controller/routing' end diff --git a/actionpack/lib/action_controller/base/base.rb b/actionpack/lib/action_controller/base.rb similarity index 100% rename from actionpack/lib/action_controller/base/base.rb rename to actionpack/lib/action_controller/base.rb diff --git a/actionpack/lib/action_controller/base/metal.rb b/actionpack/lib/action_controller/metal.rb similarity index 100% rename from actionpack/lib/action_controller/base/metal.rb rename to actionpack/lib/action_controller/metal.rb diff --git a/actionpack/lib/action_controller/base/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb similarity index 100% rename from actionpack/lib/action_controller/base/compatibility.rb rename to actionpack/lib/action_controller/metal/compatibility.rb diff --git a/actionpack/lib/action_controller/base/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb similarity index 100% rename from actionpack/lib/action_controller/base/conditional_get.rb rename to actionpack/lib/action_controller/metal/conditional_get.rb diff --git a/actionpack/lib/action_controller/base/cookies.rb b/actionpack/lib/action_controller/metal/cookies.rb similarity index 100% rename from actionpack/lib/action_controller/base/cookies.rb rename to actionpack/lib/action_controller/metal/cookies.rb diff --git a/actionpack/lib/action_controller/base/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb similarity index 100% rename from actionpack/lib/action_controller/base/exceptions.rb rename to actionpack/lib/action_controller/metal/exceptions.rb diff --git a/actionpack/lib/action_controller/base/filter_parameter_logging.rb b/actionpack/lib/action_controller/metal/filter_parameter_logging.rb similarity index 100% rename from actionpack/lib/action_controller/base/filter_parameter_logging.rb rename to actionpack/lib/action_controller/metal/filter_parameter_logging.rb diff --git a/actionpack/lib/action_controller/base/flash.rb b/actionpack/lib/action_controller/metal/flash.rb similarity index 100% rename from actionpack/lib/action_controller/base/flash.rb rename to actionpack/lib/action_controller/metal/flash.rb diff --git a/actionpack/lib/action_controller/base/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb similarity index 100% rename from actionpack/lib/action_controller/base/helpers.rb rename to actionpack/lib/action_controller/metal/helpers.rb diff --git a/actionpack/lib/action_controller/base/hide_actions.rb b/actionpack/lib/action_controller/metal/hide_actions.rb similarity index 100% rename from actionpack/lib/action_controller/base/hide_actions.rb rename to actionpack/lib/action_controller/metal/hide_actions.rb diff --git a/actionpack/lib/action_controller/base/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb similarity index 100% rename from actionpack/lib/action_controller/base/http_authentication.rb rename to actionpack/lib/action_controller/metal/http_authentication.rb diff --git a/actionpack/lib/action_controller/base/layouts.rb b/actionpack/lib/action_controller/metal/layouts.rb similarity index 100% rename from actionpack/lib/action_controller/base/layouts.rb rename to actionpack/lib/action_controller/metal/layouts.rb diff --git a/actionpack/lib/action_controller/base/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb similarity index 100% rename from actionpack/lib/action_controller/base/mime_responds.rb rename to actionpack/lib/action_controller/metal/mime_responds.rb diff --git a/actionpack/lib/action_controller/base/rack_convenience.rb b/actionpack/lib/action_controller/metal/rack_convenience.rb similarity index 100% rename from actionpack/lib/action_controller/base/rack_convenience.rb rename to actionpack/lib/action_controller/metal/rack_convenience.rb diff --git a/actionpack/lib/action_controller/base/redirector.rb b/actionpack/lib/action_controller/metal/redirector.rb similarity index 100% rename from actionpack/lib/action_controller/base/redirector.rb rename to actionpack/lib/action_controller/metal/redirector.rb diff --git a/actionpack/lib/action_controller/base/render_options.rb b/actionpack/lib/action_controller/metal/render_options.rb similarity index 100% rename from actionpack/lib/action_controller/base/render_options.rb rename to actionpack/lib/action_controller/metal/render_options.rb diff --git a/actionpack/lib/action_controller/base/renderer.rb b/actionpack/lib/action_controller/metal/renderer.rb similarity index 100% rename from actionpack/lib/action_controller/base/renderer.rb rename to actionpack/lib/action_controller/metal/renderer.rb diff --git a/actionpack/lib/action_controller/base/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb similarity index 100% rename from actionpack/lib/action_controller/base/request_forgery_protection.rb rename to actionpack/lib/action_controller/metal/request_forgery_protection.rb diff --git a/actionpack/lib/action_controller/base/rescuable.rb b/actionpack/lib/action_controller/metal/rescuable.rb similarity index 100% rename from actionpack/lib/action_controller/base/rescuable.rb rename to actionpack/lib/action_controller/metal/rescuable.rb diff --git a/actionpack/lib/action_controller/base/session.rb b/actionpack/lib/action_controller/metal/session.rb similarity index 100% rename from actionpack/lib/action_controller/base/session.rb rename to actionpack/lib/action_controller/metal/session.rb diff --git a/actionpack/lib/action_controller/base/session_management.rb b/actionpack/lib/action_controller/metal/session_management.rb similarity index 100% rename from actionpack/lib/action_controller/base/session_management.rb rename to actionpack/lib/action_controller/metal/session_management.rb diff --git a/actionpack/lib/action_controller/base/streaming.rb b/actionpack/lib/action_controller/metal/streaming.rb similarity index 100% rename from actionpack/lib/action_controller/base/streaming.rb rename to actionpack/lib/action_controller/metal/streaming.rb diff --git a/actionpack/lib/action_controller/base/testing.rb b/actionpack/lib/action_controller/metal/testing.rb similarity index 100% rename from actionpack/lib/action_controller/base/testing.rb rename to actionpack/lib/action_controller/metal/testing.rb diff --git a/actionpack/lib/action_controller/base/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb similarity index 100% rename from actionpack/lib/action_controller/base/url_for.rb rename to actionpack/lib/action_controller/metal/url_for.rb diff --git a/actionpack/lib/action_controller/base/verification.rb b/actionpack/lib/action_controller/metal/verification.rb similarity index 100% rename from actionpack/lib/action_controller/base/verification.rb rename to actionpack/lib/action_controller/metal/verification.rb From 70d779aaea719e747b242ad88f1489bf19c6795e Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 6 Aug 2009 20:03:59 -0300 Subject: [PATCH 25/38] Update _render_options to reflect the fact that they're public --- .../lib/action_controller/metal/render_options.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/actionpack/lib/action_controller/metal/render_options.rb b/actionpack/lib/action_controller/metal/render_options.rb index 65ee09883e..c6a965472f 100644 --- a/actionpack/lib/action_controller/metal/render_options.rb +++ b/actionpack/lib/action_controller/metal/render_options.rb @@ -13,7 +13,7 @@ module ActionController <<-RUBY_EVAL if options.key?(:#{r}) _process_options(options) - return _render_#{r}(options[:#{r}], options) + return render_#{r}(options[:#{r}], options) end RUBY_EVAL end @@ -52,7 +52,7 @@ module ActionController extend RenderOption register_renderer :json - def _render_json(json, options) + def render_json(json, options) json = ActiveSupport::JSON.encode(json) unless json.respond_to?(:to_str) json = "#{options[:callback]}(#{json})" unless options[:callback].blank? self.content_type ||= Mime::JSON @@ -64,9 +64,9 @@ module ActionController extend RenderOption register_renderer :js - def _render_js(js, options) + def render_js(js, options) self.content_type ||= Mime::JS - self.response_body = js + self.response_body = js.respond_to?(:to_js) ? js.to_js : js end end @@ -74,7 +74,7 @@ module ActionController extend RenderOption register_renderer :xml - def _render_xml(xml, options) + def render_xml(xml, options) self.content_type ||= Mime::XML self.response_body = xml.respond_to?(:to_xml) ? xml.to_xml : xml end @@ -84,7 +84,7 @@ module ActionController extend RenderOption register_renderer :update - def _render_update(proc, options) + def render_update(proc, options) generator = ActionView::Helpers::PrototypeHelper::JavaScriptGenerator.new(view_context, &proc) self.content_type = Mime::JS self.response_body = generator.to_s From 16c01224cbe503ab1f4d0f02955b16662be00116 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 6 Aug 2009 20:05:14 -0300 Subject: [PATCH 26/38] ActionController::Metal#to_rack converted to #to_a to match normal rack convention --- actionpack/lib/action_controller/metal.rb | 4 ++-- actionpack/lib/action_controller/metal/rack_convenience.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index e7d776b63e..82d05414b9 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -72,11 +72,11 @@ module ActionController def call(name, env) @_env = env process(name) - to_rack + to_a end # :api: private - def to_rack + def to_a [status, headers, response_body] end diff --git a/actionpack/lib/action_controller/metal/rack_convenience.rb b/actionpack/lib/action_controller/metal/rack_convenience.rb index 805157b0e3..5fac445dab 100644 --- a/actionpack/lib/action_controller/metal/rack_convenience.rb +++ b/actionpack/lib/action_controller/metal/rack_convenience.rb @@ -20,7 +20,7 @@ module ActionController end # :api: private - def to_rack + def to_a @_response.prepare! @_response.to_a end From 71638e6760bed0445e5fefc185924b07076fef47 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 6 Aug 2009 22:51:24 -0300 Subject: [PATCH 27/38] Move AbstractController to a top-level component --- actionpack/lib/abstract_controller.rb | 16 ++++++++++++++++ .../abstract => abstract_controller}/base.rb | 0 .../benchmarker.rb | 0 .../callbacks.rb | 0 .../exceptions.rb | 0 .../abstract => abstract_controller}/helpers.rb | 0 .../abstract => abstract_controller}/layouts.rb | 0 .../abstract => abstract_controller}/logger.rb | 0 .../abstract => abstract_controller}/renderer.rb | 2 +- actionpack/lib/action_controller.rb | 2 +- actionpack/lib/action_controller/abstract.rb | 16 ---------------- actionpack/lib/action_controller/metal.rb | 2 -- .../test/abstract_controller/test_helper.rb | 2 +- actionpack/test/abstract_unit.rb | 1 - 14 files changed, 19 insertions(+), 22 deletions(-) create mode 100644 actionpack/lib/abstract_controller.rb rename actionpack/lib/{action_controller/abstract => abstract_controller}/base.rb (100%) rename actionpack/lib/{action_controller/abstract => abstract_controller}/benchmarker.rb (100%) rename actionpack/lib/{action_controller/abstract => abstract_controller}/callbacks.rb (100%) rename actionpack/lib/{action_controller/abstract => abstract_controller}/exceptions.rb (100%) rename actionpack/lib/{action_controller/abstract => abstract_controller}/helpers.rb (100%) rename actionpack/lib/{action_controller/abstract => abstract_controller}/layouts.rb (100%) rename actionpack/lib/{action_controller/abstract => abstract_controller}/logger.rb (100%) rename actionpack/lib/{action_controller/abstract => abstract_controller}/renderer.rb (99%) delete mode 100644 actionpack/lib/action_controller/abstract.rb diff --git a/actionpack/lib/abstract_controller.rb b/actionpack/lib/abstract_controller.rb new file mode 100644 index 0000000000..f020abaa45 --- /dev/null +++ b/actionpack/lib/abstract_controller.rb @@ -0,0 +1,16 @@ +require "active_support/core_ext/module/attr_internal" +require "active_support/core_ext/module/delegation" + +module AbstractController + autoload :Base, "abstract_controller/base" + autoload :Benchmarker, "abstract_controller/benchmarker" + autoload :Callbacks, "abstract_controller/callbacks" + autoload :Helpers, "abstract_controller/helpers" + autoload :Layouts, "abstract_controller/layouts" + autoload :Logger, "abstract_controller/logger" + autoload :Renderer, "abstract_controller/renderer" + # === Exceptions + autoload :ActionNotFound, "abstract_controller/exceptions" + autoload :DoubleRenderError, "abstract_controller/exceptions" + autoload :Error, "abstract_controller/exceptions" +end diff --git a/actionpack/lib/action_controller/abstract/base.rb b/actionpack/lib/abstract_controller/base.rb similarity index 100% rename from actionpack/lib/action_controller/abstract/base.rb rename to actionpack/lib/abstract_controller/base.rb diff --git a/actionpack/lib/action_controller/abstract/benchmarker.rb b/actionpack/lib/abstract_controller/benchmarker.rb similarity index 100% rename from actionpack/lib/action_controller/abstract/benchmarker.rb rename to actionpack/lib/abstract_controller/benchmarker.rb diff --git a/actionpack/lib/action_controller/abstract/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb similarity index 100% rename from actionpack/lib/action_controller/abstract/callbacks.rb rename to actionpack/lib/abstract_controller/callbacks.rb diff --git a/actionpack/lib/action_controller/abstract/exceptions.rb b/actionpack/lib/abstract_controller/exceptions.rb similarity index 100% rename from actionpack/lib/action_controller/abstract/exceptions.rb rename to actionpack/lib/abstract_controller/exceptions.rb diff --git a/actionpack/lib/action_controller/abstract/helpers.rb b/actionpack/lib/abstract_controller/helpers.rb similarity index 100% rename from actionpack/lib/action_controller/abstract/helpers.rb rename to actionpack/lib/abstract_controller/helpers.rb diff --git a/actionpack/lib/action_controller/abstract/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb similarity index 100% rename from actionpack/lib/action_controller/abstract/layouts.rb rename to actionpack/lib/abstract_controller/layouts.rb diff --git a/actionpack/lib/action_controller/abstract/logger.rb b/actionpack/lib/abstract_controller/logger.rb similarity index 100% rename from actionpack/lib/action_controller/abstract/logger.rb rename to actionpack/lib/abstract_controller/logger.rb diff --git a/actionpack/lib/action_controller/abstract/renderer.rb b/actionpack/lib/abstract_controller/renderer.rb similarity index 99% rename from actionpack/lib/action_controller/abstract/renderer.rb rename to actionpack/lib/abstract_controller/renderer.rb index fe556281ab..4e368a099a 100644 --- a/actionpack/lib/action_controller/abstract/renderer.rb +++ b/actionpack/lib/abstract_controller/renderer.rb @@ -1,4 +1,4 @@ -require "action_controller/abstract/logger" +require "abstract_controller/logger" module AbstractController module Renderer diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 2b2c7ef725..22c2c4f499 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -56,7 +56,7 @@ module ActionController end autoload :HTML, 'action_controller/vendor/html-scanner' -autoload :AbstractController, 'action_controller/abstract' +autoload :AbstractController, 'abstract_controller' autoload :Rack, 'action_dispatch' autoload :ActionDispatch, 'action_dispatch' diff --git a/actionpack/lib/action_controller/abstract.rb b/actionpack/lib/action_controller/abstract.rb deleted file mode 100644 index d0eba253b8..0000000000 --- a/actionpack/lib/action_controller/abstract.rb +++ /dev/null @@ -1,16 +0,0 @@ -require "active_support/core_ext/module/attr_internal" -require "active_support/core_ext/module/delegation" - -module AbstractController - autoload :Base, "action_controller/abstract/base" - autoload :Benchmarker, "action_controller/abstract/benchmarker" - autoload :Callbacks, "action_controller/abstract/callbacks" - autoload :Helpers, "action_controller/abstract/helpers" - autoload :Layouts, "action_controller/abstract/layouts" - autoload :Logger, "action_controller/abstract/logger" - autoload :Renderer, "action_controller/abstract/renderer" - # === Exceptions - autoload :ActionNotFound, "action_controller/abstract/exceptions" - autoload :DoubleRenderError, "action_controller/abstract/exceptions" - autoload :Error, "action_controller/abstract/exceptions" -end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 82d05414b9..5333ca497c 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -1,5 +1,3 @@ -require 'action_controller/abstract' - module ActionController # ActionController::Metal provides a way to get a valid Rack application from a controller. # diff --git a/actionpack/test/abstract_controller/test_helper.rb b/actionpack/test/abstract_controller/test_helper.rb index 915054f7fb..ba4302d914 100644 --- a/actionpack/test/abstract_controller/test_helper.rb +++ b/actionpack/test/abstract_controller/test_helper.rb @@ -6,7 +6,7 @@ require 'rubygems' require 'test/unit' require 'active_support' require 'active_support/test_case' -require 'action_controller/abstract' +require 'abstract_controller' require 'action_view' require 'action_view/base' require 'action_dispatch' diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 7f373ea7c5..b062a71442 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -13,7 +13,6 @@ require 'test/unit' require 'active_support' require 'active_support/test_case' -require 'action_controller/abstract' require 'action_controller' require 'fixture_template' require 'action_controller/testing/process' From 0435178ff896e9b3854cb53cbd9fbd33a8f78209 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 6 Aug 2009 22:57:42 -0300 Subject: [PATCH 28/38] Remove file that doesn't seem to be used anymore --- actionpack/lib/action_controller/old_base.rb | 84 -------------------- 1 file changed, 84 deletions(-) delete mode 100644 actionpack/lib/action_controller/old_base.rb diff --git a/actionpack/lib/action_controller/old_base.rb b/actionpack/lib/action_controller/old_base.rb deleted file mode 100644 index dd22bfd617..0000000000 --- a/actionpack/lib/action_controller/old_base.rb +++ /dev/null @@ -1,84 +0,0 @@ -#-- -# Copyright (c) 2004-2009 David Heinemeier Hansson -# -# Permission is hereby granted, free of charge, to any person obtaining -# a copy of this software and associated documentation files (the -# "Software"), to deal in the Software without restriction, including -# without limitation the rights to use, copy, modify, merge, publish, -# distribute, sublicense, and/or sell copies of the Software, and to -# permit persons to whom the Software is furnished to do so, subject to -# the following conditions: -# -# The above copyright notice and this permission notice shall be -# included in all copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, -# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND -# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE -# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION -# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION -# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. -#++ - -activesupport_path = "#{File.dirname(__FILE__)}/../../activesupport/lib" -$:.unshift activesupport_path if File.directory?(activesupport_path) -require 'active_support' - -require File.join(File.dirname(__FILE__), "action_pack") - -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, Request, Response, UrlRewriter, UrlWriter] - [ActionDispatch::Http::Headers] - end - - autoload :Base, 'action_controller/base/base' - autoload :Benchmarking, 'action_controller/base/chained/benchmarking' - autoload :Caching, 'action_controller/caching' - autoload :Cookies, 'action_controller/base/cookies' - autoload :Dispatcher, 'action_controller/dispatch/dispatcher' - autoload :Filters, 'action_controller/base/chained/filters' - autoload :Flash, 'action_controller/base/chained/flash' - autoload :Helpers, 'action_controller/base/helpers' - autoload :HttpAuthentication, 'action_controller/base/http_authentication' - autoload :Integration, 'action_controller/testing/integration' - autoload :IntegrationTest, 'action_controller/testing/integration' - autoload :Layout, 'action_controller/base/layout' - autoload :MimeResponds, 'action_controller/base/mime_responds' - autoload :PolymorphicRoutes, 'action_controller/routing/generation/polymorphic_routes' - autoload :RecordIdentifier, 'action_controller/record_identifier' - autoload :Redirector, 'action_controller/base/redirect' - autoload :Renderer, 'action_controller/base/render' - autoload :RequestForgeryProtection, 'action_controller/base/request_forgery_protection' - autoload :Rescue, 'action_controller/base/rescue' - autoload :Resources, 'action_controller/routing/resources' - autoload :Responder, 'action_controller/base/responder' - autoload :Routing, 'action_controller/routing' - autoload :SessionManagement, 'action_controller/base/session_management' - autoload :Streaming, 'action_controller/base/streaming' - autoload :TestCase, 'action_controller/testing/test_case' - autoload :TestProcess, 'action_controller/testing/process' - autoload :Translation, 'action_controller/translation' - autoload :UrlEncodedPairParser, 'action_controller/dispatch/url_encoded_pair_parser' - autoload :UrlRewriter, 'action_controller/routing/generation/url_rewriter' - autoload :UrlWriter, 'action_controller/routing/generation/url_rewriter' - autoload :Verification, 'action_controller/base/verification' - autoload :FilterParameterLogging, 'action_controller/base/filter_parameter_logging' - - module Assertions - autoload :DomAssertions, 'action_controller/testing/assertions/dom' - autoload :ModelAssertions, 'action_controller/testing/assertions/model' - autoload :ResponseAssertions, 'action_controller/testing/assertions/response' - autoload :RoutingAssertions, 'action_controller/testing/assertions/routing' - autoload :SelectorAssertions, 'action_controller/testing/assertions/selector' - autoload :TagAssertions, 'action_controller/testing/assertions/tag' - end -end - -autoload :HTML, 'action_controller/vendor/html-scanner' - -require 'action_dispatch' -require 'action_view' From 4ac9d391d337d5a05a7aa93849405e21dd4bbf01 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 6 Aug 2009 23:42:11 -0300 Subject: [PATCH 29/38] Improve a path in _render_partial --- .../lib/action_view/helpers/form_helper.rb | 4 ++ actionpack/lib/action_view/render/partials.rb | 47 +++++++++---------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 2d1d19d5f3..6e2990e084 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -932,6 +932,10 @@ module ActionView attr_accessor :object_name, :object, :options + def self.model_name + @model_name ||= Struct.new(:partial_path).new(name.demodulize.underscore.sub!(/_builder$/, '')) + end + def initialize(object_name, object, template, options, proc) @nested_child_index = {} @object_name, @object, @template, @options, @proc = object_name, object, template, options, proc diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 8bdff0f3b9..581742e875 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -172,9 +172,9 @@ module ActionView module Partials extend ActiveSupport::Memoizable extend ActiveSupport::Concern - + included do - attr_accessor :_partial + attr_accessor :_partial end def render_partial(*args) @@ -185,22 +185,21 @@ module ActionView def _render_partial(options = {}) #:nodoc: options[:locals] ||= {} - case path = partial = options[:partial] - when *_array_like_objects + path = partial = options[:partial] + + if partial.respond_to?(:to_ary) return _render_partial_collection(partial, options) - else - if partial.is_a?(ActionView::Helpers::FormBuilder) - path = partial.class.to_s.demodulize.underscore.sub(/_builder$/, '') - options[:locals].merge!(path.to_sym => partial) - elsif !partial.is_a?(String) - options[:object] = object = partial - path = ActionController::RecordIdentifier.partial_path(object, controller_path) - end - _, _, prefix, object = parts = partial_parts(path, options) - parts[1] = {:formats => parts[1]} - template = find_by_parts(*parts) - _render_partial_object(template, options, (object unless object == true)) + elsif partial.is_a?(ActionView::Helpers::FormBuilder) + path = partial.class.model_name.partial_path + options[:locals].merge!(path.to_sym => partial) + elsif !partial.is_a?(String) + options[:object] = object = partial + path = ActionController::RecordIdentifier.partial_path(object, controller_path) end + + parts = partial_parts(path, options) + template = find_by_parts(*parts) + _render_partial_object(template, options, (parts[3] unless parts[3] == true)) end private @@ -222,7 +221,7 @@ module ActionView path = parts.join(".") prefix = segments[0..-1].join("/") prefix = prefix.blank? ? controller_path : prefix - parts = [path, formats, prefix] + parts = [path, {:formats => formats}, prefix] parts.push options[:object] || true end @@ -256,11 +255,11 @@ module ActionView else locals = (options[:locals] ||= {}) object ||= locals[:object] || locals[template.variable_name] - + _set_locals(object, locals, template, options) - + options[:_template] = template - + _render_template(template, locals) end end @@ -272,23 +271,23 @@ module ActionView def _render_partial_collection(collection, options = {}, passed_template = nil) #:nodoc: return nil if collection.blank? - + spacer = options[:spacer_template] ? _render_partial(:partial => options[:spacer_template]) : '' locals = (options[:locals] ||= {}) index, @_partial_path = 0, nil collection.map do |object| options[:_template] = template = passed_template || begin - _partial_path = + _partial_path = ActionController::RecordIdentifier.partial_path(object, controller_path) template = _pick_partial_template(_partial_path) end _set_locals(object, locals, template, options) locals[template.counter_name] = index - + index += 1 - + _render_template(template, locals) end.join(spacer) end From 70a440aa277676078e3b8baafe1101d1287e114f Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Fri, 7 Aug 2009 00:52:13 -0300 Subject: [PATCH 30/38] Clean up render @object a bit more. --- .../action_controller/record_identifier.rb | 15 ----- actionpack/lib/action_view/render/partials.rb | 25 ++++++-- .../test/controller/record_identifier_test.rb | 57 ------------------- 3 files changed, 19 insertions(+), 78 deletions(-) diff --git a/actionpack/lib/action_controller/record_identifier.rb b/actionpack/lib/action_controller/record_identifier.rb index b4408e4e1d..1165c3b7c5 100644 --- a/actionpack/lib/action_controller/record_identifier.rb +++ b/actionpack/lib/action_controller/record_identifier.rb @@ -36,21 +36,6 @@ module ActionController JOIN = '_'.freeze NEW = 'new'.freeze - # Returns plural/singular for a record or class. Example: - # - # partial_path(post) # => "posts/post" - # partial_path(Person) # => "people/person" - # partial_path(Person, "admin/games") # => "admin/people/person" - def partial_path(record_or_class, controller_path = nil) - name = model_name_from_record_or_class(record_or_class) - - if controller_path && controller_path.include?("/") - "#{File.dirname(controller_path)}/#{name.partial_path}" - else - name.partial_path - end - end - # The DOM class convention is to use the singular form of an object or class. Examples: # # dom_class(post) # => "post" diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 581742e875..6e5810d148 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -177,6 +177,12 @@ module ActionView attr_accessor :_partial end + module ClassMethods + def _partial_names + @_partial_names ||= ActiveSupport::ConcurrentHash.new + end + end + def render_partial(*args) @assigns_added = false _render_partial(*args) @@ -189,12 +195,9 @@ module ActionView if partial.respond_to?(:to_ary) return _render_partial_collection(partial, options) - elsif partial.is_a?(ActionView::Helpers::FormBuilder) - path = partial.class.model_name.partial_path - options[:locals].merge!(path.to_sym => partial) elsif !partial.is_a?(String) options[:object] = object = partial - path = ActionController::RecordIdentifier.partial_path(object, controller_path) + path = _partial_path(object) end parts = partial_parts(path, options) @@ -203,6 +206,17 @@ module ActionView end private + def _partial_path(object) + self.class._partial_names[[controller.class, object.class]] ||= begin + name = object.class.model_name + if controller_path && controller_path.include?("/") + File.join(File.dirname(controller_path), name.partial_path) + else + name.partial_path + end + end + end + def partial_parts(name, options) segments = name.split("/") parts = segments.pop.split(".") @@ -278,8 +292,7 @@ module ActionView index, @_partial_path = 0, nil collection.map do |object| options[:_template] = template = passed_template || begin - _partial_path = - ActionController::RecordIdentifier.partial_path(object, controller_path) + _partial_path = _partial_path(object) template = _pick_partial_template(_partial_path) end diff --git a/actionpack/test/controller/record_identifier_test.rb b/actionpack/test/controller/record_identifier_test.rb index 44e49ed3f8..6b6d154faa 100644 --- a/actionpack/test/controller/record_identifier_test.rb +++ b/actionpack/test/controller/record_identifier_test.rb @@ -54,24 +54,6 @@ class RecordIdentifierTest < Test::Unit::TestCase assert_equal "edit_#{@singular}_1", dom_id(@record, :edit) end - def test_partial_path - expected = "#{@plural}/#{@singular}" - assert_equal expected, partial_path(@record) - assert_equal expected, partial_path(Comment) - end - - def test_partial_path_with_namespaced_controller_path - expected = "admin/#{@plural}/#{@singular}" - assert_equal expected, partial_path(@record, "admin/posts") - assert_equal expected, partial_path(@klass, "admin/posts") - end - - def test_partial_path_with_not_namespaced_controller_path - expected = "#{@plural}/#{@singular}" - assert_equal expected, partial_path(@record, "posts") - assert_equal expected, partial_path(@klass, "posts") - end - def test_dom_class assert_equal @singular, dom_class(@record) end @@ -101,42 +83,3 @@ class RecordIdentifierTest < Test::Unit::TestCase RecordIdentifier.send(method, *args) end end - -class NestedRecordIdentifierTest < RecordIdentifierTest - def setup - @klass = Comment::Nested - @record = @klass.new - @singular = 'comment_nested' - @plural = 'comment_nesteds' - end - - def test_partial_path - expected = "comment/nesteds/nested" - assert_equal expected, partial_path(@record) - assert_equal expected, partial_path(Comment::Nested) - end - - def test_partial_path_with_namespaced_controller_path - expected = "admin/comment/nesteds/nested" - assert_equal expected, partial_path(@record, "admin/posts") - assert_equal expected, partial_path(@klass, "admin/posts") - end - - def test_partial_path_with_deeper_namespaced_controller_path - expected = "deeper/admin/comment/nesteds/nested" - assert_equal expected, partial_path(@record, "deeper/admin/posts") - assert_equal expected, partial_path(@klass, "deeper/admin/posts") - end - - def test_partial_path_with_even_deeper_namespaced_controller_path - expected = "even/more/deeper/admin/comment/nesteds/nested" - assert_equal expected, partial_path(@record, "even/more/deeper/admin/posts") - assert_equal expected, partial_path(@klass, "even/more/deeper/admin/posts") - end - - def test_partial_path_with_not_namespaced_controller_path - expected = "comment/nesteds/nested" - assert_equal expected, partial_path(@record, "posts") - assert_equal expected, partial_path(@klass, "posts") - end -end From 9b506484f1e72aeba2f1fd4af3e39cf450f1d4a9 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Fri, 7 Aug 2009 01:51:32 -0300 Subject: [PATCH 31/38] This is handled by the resolver now --- actionpack/lib/action_view/render/partials.rb | 29 ++++--------------- 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 6e5810d148..3e60dcf068 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -200,9 +200,12 @@ module ActionView path = _partial_path(object) end - parts = partial_parts(path, options) + parts = [path, {:formats => formats}] + parts.push path.include?(?/) ? nil : controller_path + parts.push true + template = find_by_parts(*parts) - _render_partial_object(template, options, (parts[3] unless parts[3] == true)) + _render_partial_object(template, options) end private @@ -217,28 +220,6 @@ module ActionView end end - def partial_parts(name, options) - segments = name.split("/") - parts = segments.pop.split(".") - - case parts.size - when 1 - parts - when 2, 3 - extension = parts.delete_at(1).to_sym - if formats.include?(extension) - self.formats.replace [extension] - end - parts.pop if parts.size == 2 - end - - path = parts.join(".") - prefix = segments[0..-1].join("/") - prefix = prefix.blank? ? controller_path : prefix - parts = [path, {:formats => formats}, prefix] - parts.push options[:object] || true - end - def _render_partial_with_block(layout, block, options) @_proc_for_layout = block concat(_render_partial(options.merge(:partial => layout))) From 8534c5bf193c6a234d55116d520a54a1b634a93e Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Fri, 7 Aug 2009 01:51:50 -0300 Subject: [PATCH 32/38] Start cleaning up partial path --- actionpack/lib/abstract_controller/renderer.rb | 4 ++-- .../lib/action_controller/metal/renderer.rb | 2 +- actionpack/lib/action_view/render/partials.rb | 10 ++++++---- actionpack/lib/action_view/render/rendering.rb | 16 +++++++++++----- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/actionpack/lib/abstract_controller/renderer.rb b/actionpack/lib/abstract_controller/renderer.rb index 4e368a099a..73e6b2a4dc 100644 --- a/actionpack/lib/abstract_controller/renderer.rb +++ b/actionpack/lib/abstract_controller/renderer.rb @@ -54,7 +54,7 @@ module AbstractController # :api: plugin def render_to_body(options = {}) # TODO: Refactor so we can just use the normal template logic for this - if options[:_partial_object] + if options.key?(:_partial_object) view_context.render_partial(options) else _determine_template(options) @@ -77,7 +77,7 @@ module AbstractController # _layout:: The layout to wrap the template in (optional) # _partial:: Whether or not the template to be rendered is a partial def _render_template(options) - view_context.render_template(options[:_template], options[:_layout], options, options[:_partial]) + view_context.render_template(options) end # The list of view paths for this controller. See ActionView::ViewPathSet for diff --git a/actionpack/lib/action_controller/metal/renderer.rb b/actionpack/lib/action_controller/metal/renderer.rb index 572da451ff..5a458764ad 100644 --- a/actionpack/lib/action_controller/metal/renderer.rb +++ b/actionpack/lib/action_controller/metal/renderer.rb @@ -57,7 +57,7 @@ module ActionController when true options[:_prefix] = _prefix when String - options[:_prefix] = _prefix unless partial.index('/') + options[:_prefix] = _prefix unless partial.include?(?/) options[:_template_name] = partial else options[:_partial_object] = true diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 3e60dcf068..89d954037b 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -183,12 +183,12 @@ module ActionView end end - def render_partial(*args) + def render_partial(options) @assigns_added = false - _render_partial(*args) + _render_partial(options) end - def _render_partial(options = {}) #:nodoc: + def _render_partial(options) #:nodoc: options[:locals] ||= {} path = partial = options[:partial] @@ -244,7 +244,9 @@ module ActionView array_like end - def _render_partial_object(template, options, object = nil) + def _render_partial_object(template, options) + object = options[:object] + if options.key?(:collection) _render_partial_collection(options.delete(:collection), options, template) else diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 86a59dd1bc..32c30e8f68 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -123,19 +123,25 @@ module ActionView layout ? _render_content_with_layout(text, layout, options[:locals]) : text end - def render_template(*args) + # This is the API to render a ViewContext's template from a controller. + # + # Internal Options: + # _template:: The Template object to render + # _layout:: The layout, if any, to wrap the Template in + # _partial:: true if the template is a partial + def render_template(options) @assigns_added = nil - _render_template_with_layout(*args) + template, layout, partial = options.values_at(:_template, :_layout, :_partial) + _render_template_with_layout(template, layout, options, partial) end - def _render_template_with_layout(template, layout = nil, options = {}, partial = false) + def _render_template_with_layout(template, layout = nil, options = {}, partial = nil) logger && logger.info("Rendering #{template.identifier}#{' (#{options[:status]})' if options[:status]}") locals = options[:locals] || {} content = if partial - object = partial unless partial == true - _render_partial_object(template, options, object) + _render_partial_object(template, options) else _render_template(template, locals) end From b3e199f6981b2fbf062fe668ff93b7dc56e98a38 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Fri, 7 Aug 2009 02:46:21 -0300 Subject: [PATCH 33/38] Some more AV refactoring: * remove no longer used _array_like_objects * _render_content_with_layout renamed to _render_content since layout it optional * remove check for optional layout before _render_content --- actionpack/lib/action_view/render/partials.rb | 13 ++----------- actionpack/lib/action_view/render/rendering.rb | 8 ++++---- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 89d954037b..7a791d03ce 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -229,19 +229,10 @@ module ActionView def _render_partial_with_layout(layout, options) if layout - prefix = controller && !layout.include?("/") ? controller.controller_path : nil + prefix = layout.include?(?/) ? nil : controller_path layout = find_by_parts(layout, {:formats => formats}, prefix, true) end - content = _render_partial(options) - return _render_content_with_layout(content, layout, options[:locals]) - end - - def _array_like_objects - array_like = [Array] - if defined?(ActiveRecord) - array_like.push(ActiveRecord::Associations::AssociationCollection, ActiveRecord::NamedScope::Scope) - end - array_like + _render_content(_render_partial(options), layout, options[:locals]) end def _render_partial_object(template, options) diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 32c30e8f68..527cef97c7 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -40,7 +40,7 @@ module ActionView end end - def _render_content_with_layout(content, layout, locals) + def _render_content(content, layout, locals) return content unless layout locals ||= {} @@ -116,11 +116,11 @@ module ActionView handler = Template.handler_class_for_extension(options[:type] || "erb") template = Template.new(options[:inline], "inline #{options[:inline].inspect}", handler, {}) content = _render_template(template, options[:locals] || {}) - layout ? _render_content_with_layout(content, layout, options[:locals]) : content + layout ? _render_content(content, layout, options[:locals]) : content end def _render_text(text, layout, options) - layout ? _render_content_with_layout(text, layout, options[:locals]) : text + layout ? _render_content(text, layout, options[:locals]) : text end # This is the API to render a ViewContext's template from a controller. @@ -146,7 +146,7 @@ module ActionView _render_template(template, locals) end - layout ? _render_content_with_layout(content, layout, locals) : content + _render_content(content, layout, locals) end end end \ No newline at end of file From 0612fd0f09977dece11a0325a0d7ee07c5cab35c Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Fri, 7 Aug 2009 03:18:45 -0300 Subject: [PATCH 34/38] Replace _render_template_with_layout with _render_template since the layout is optional --- actionmailer/lib/action_mailer/base.rb | 4 ++-- actionmailer/test/mail_service_test.rb | 10 +++++---- actionpack/lib/action_view/render/partials.rb | 4 ++-- .../lib/action_view/render/rendering.rb | 22 +++++++++++-------- actionpack/lib/action_view/test_case.rb | 4 ++-- actionpack/test/controller/logging_test.rb | 3 ++- actionpack/test/controller/render_test.rb | 3 ++- 7 files changed, 29 insertions(+), 21 deletions(-) diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index b5a0d0ab96..7b03a7f9ff 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -566,7 +566,7 @@ module ActionMailer #:nodoc: @template = initialize_template_class(body) layout = _pick_layout(layout, true) unless ActionController::Base.exempt_from_layout.include?(template.handler) - @template._render_template_with_layout(template, layout, {}) + @template._render_template(template, layout, {}) ensure @current_template_content_type = nil end @@ -592,7 +592,7 @@ module ActionMailer #:nodoc: !template || ActionController::Base.exempt_from_layout.include?(template.handler)) if template - @template._render_template_with_layout(template, layout, opts) + @template._render_template(template, layout, opts) elsif inline = opts[:inline] @template._render_inline(inline, layout, opts) end diff --git a/actionmailer/test/mail_service_test.rb b/actionmailer/test/mail_service_test.rb index 30d1b836c4..f2a8c2303c 100644 --- a/actionmailer/test/mail_service_test.rb +++ b/actionmailer/test/mail_service_test.rb @@ -573,12 +573,14 @@ class ActionMailerTest < Test::Unit::TestCase @info_contents, @debug_contents = "", "" end - def info(str) - @info_contents << str + def info(str = nil, &blk) + @info_contents << str if str + @info_contents << blk.call if block_given? end - def debug(str) - @debug_contents << str + def debug(str = nil, &blk) + @debug_contents << str if str + @debug_contents << blk.call if block_given? end end diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 7a791d03ce..98694788f8 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -248,7 +248,7 @@ module ActionView options[:_template] = template - _render_template(template, locals) + _render_single_template(template, locals) end end @@ -275,7 +275,7 @@ module ActionView index += 1 - _render_template(template, locals) + _render_single_template(template, locals) end.join(spacer) end diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 527cef97c7..911e480d50 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -27,7 +27,7 @@ module ActionView if file = options[:file] template = find_by_parts(file, {:formats => formats}) - _render_template_with_layout(template, layout, :locals => options[:locals]) + _render_template(template, layout, :locals => options[:locals]) elsif inline = options[:inline] _render_inline(inline, layout, options) elsif text = options[:text] @@ -54,7 +54,7 @@ module ActionView old_content, @_content_for[:layout] = @_content_for[:layout], content @cached_content_for_layout = @_content_for[:layout] - _render_template(layout, locals) + _render_single_template(layout, locals) ensure @_content_for[:layout] = old_content end @@ -97,9 +97,9 @@ module ActionView !@_content_for.key?(name) && @_proc_for_layout || @_default_layout end - def _render_template(template, local_assigns = {}) + def _render_single_template(template, locals = {}) with_template(template) do - template.render(self, local_assigns) do |*names| + template.render(self, locals) do |*names| capture(*names, &layout_proc(names.first)) end end @@ -115,7 +115,7 @@ module ActionView def _render_inline(inline, layout, options) handler = Template.handler_class_for_extension(options[:type] || "erb") template = Template.new(options[:inline], "inline #{options[:inline].inspect}", handler, {}) - content = _render_template(template, options[:locals] || {}) + content = _render_single_template(template, options[:locals] || {}) layout ? _render_content(content, layout, options[:locals]) : content end @@ -132,18 +132,22 @@ module ActionView def render_template(options) @assigns_added = nil template, layout, partial = options.values_at(:_template, :_layout, :_partial) - _render_template_with_layout(template, layout, options, partial) + _render_template(template, layout, options, partial) end - def _render_template_with_layout(template, layout = nil, options = {}, partial = nil) - logger && logger.info("Rendering #{template.identifier}#{' (#{options[:status]})' if options[:status]}") + def _render_template(template, layout = nil, options = {}, partial = nil) + logger && logger.info do + msg = "Rendering #{template.identifier}" + msg << " (#{options[:status]})" if options[:status] + msg + end locals = options[:locals] || {} content = if partial _render_partial_object(template, options) else - _render_template(template, locals) + _render_single_template(template, locals) end _render_content(content, layout, locals) diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 3f3951509a..71a4a88afe 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -9,8 +9,8 @@ module ActionView end attr_internal :rendered - alias_method :_render_template_without_template_tracking, :_render_template - def _render_template(template, local_assigns = {}) + alias_method :_render_template_without_template_tracking, :_render_single_template + def _render_single_template(template, local_assigns = {}) if template.respond_to?(:identifier) && template.present? @_rendered[:partials][template] += 1 if template.partial? @_rendered[:template] ||= [] diff --git a/actionpack/test/controller/logging_test.rb b/actionpack/test/controller/logging_test.rb index a7ed1b8665..98ffbc3813 100644 --- a/actionpack/test/controller/logging_test.rb +++ b/actionpack/test/controller/logging_test.rb @@ -17,9 +17,10 @@ class LoggingTest < ActionController::TestCase @level = Logger::DEBUG end - def method_missing(method, *args) + def method_missing(method, *args, &blk) @logged ||= [] @logged << args.first + @logged << blk.call if block_given? end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index d0fa67c945..947ffa9ea6 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -17,8 +17,9 @@ class MockLogger @logged = [] end - def method_missing(method, *args) + def method_missing(method, *args, &blk) @logged << args.first + @logged << blk.call if block_given? end end From 59e475e3a66fe647ea74f4daf472d82bc9b4535c Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Fri, 7 Aug 2009 03:48:28 -0300 Subject: [PATCH 35/38] Some more AV work: * rename _render_partial to _render_partial_unknown_type to reflect that for this call, we don't know the type. * Merge _render_partial_with_block and _render_partial_with_layout to _render_partial * TODO: Check to see if any more logic can be shared * TODO: See about streamlining block path so we can get rid of @_proc_for_layout * Remove @exempt_from_layout as it is no longer needed --- actionpack/lib/action_view/render/partials.rb | 33 ++++++++++--------- .../lib/action_view/render/rendering.rb | 18 ++++------ 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 98694788f8..8e094240d3 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -185,10 +185,10 @@ module ActionView def render_partial(options) @assigns_added = false - _render_partial(options) + _render_partial_unknown_type(options) end - def _render_partial(options) #:nodoc: + def _render_partial_unknown_type(options) #:nodoc: options[:locals] ||= {} path = partial = options[:partial] @@ -220,19 +220,21 @@ module ActionView end end - def _render_partial_with_block(layout, block, options) - @_proc_for_layout = block - concat(_render_partial(options.merge(:partial => layout))) - ensure - @_proc_for_layout = nil - end - - def _render_partial_with_layout(layout, options) - if layout - prefix = layout.include?(?/) ? nil : controller_path - layout = find_by_parts(layout, {:formats => formats}, prefix, true) + def _render_partial(layout, options, block = nil) + if block + begin + @_proc_for_layout = block + concat(_render_partial_unknown_type(options.merge(:partial => layout))) + ensure + @_proc_for_layout = nil + end + else + if layout + prefix = layout.include?(?/) ? nil : controller_path + layout = find_by_parts(layout, {:formats => formats}, prefix, true) + end + _render_content(_render_partial_unknown_type(options), layout, options[:locals]) end - _render_content(_render_partial(options), layout, options[:locals]) end def _render_partial_object(template, options) @@ -260,7 +262,8 @@ module ActionView def _render_partial_collection(collection, options = {}, passed_template = nil) #:nodoc: return nil if collection.blank? - spacer = options[:spacer_template] ? _render_partial(:partial => options[:spacer_template]) : '' + spacer = options[:spacer_template] ? + _render_partial_unknown_type(:partial => options[:spacer_template]) : '' locals = (options[:locals] ||= {}) index, @_partial_path = 0, nil diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 911e480d50..0a5953be88 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -10,24 +10,22 @@ module ActionView # # If no options hash is passed or :update specified, the default is to render a partial and use the second parameter # as the locals hash. - def render(options = {}, local_assigns = {}, &block) #:nodoc: - local_assigns ||= {} - - @exempt_from_layout = true - + def render(options = {}, locals = {}, &block) #:nodoc: case options + when String, NilClass + _render_partial_unknown_type(:partial => options, :locals => locals || {}) when Hash - options[:locals] ||= {} layout = options[:layout] - return _render_partial_with_layout(layout, options) if options.key?(:partial) - return _render_partial_with_block(layout, block, options) if block_given? + if options.key?(:partial) || block_given? + return _render_partial(layout, options, block) + end layout = find_by_parts(layout, {:formats => formats}) if layout if file = options[:file] template = find_by_parts(file, {:formats => formats}) - _render_template(template, layout, :locals => options[:locals]) + _render_template(template, layout, :locals => options[:locals] || {}) elsif inline = options[:inline] _render_inline(inline, layout, options) elsif text = options[:text] @@ -35,8 +33,6 @@ module ActionView end when :update update_page(&block) - when String, NilClass - _render_partial(:partial => options, :locals => local_assigns) end end From 493d84ce2f1e25081a394fd70ac4e23b6a2be682 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Fri, 7 Aug 2009 05:40:01 -0300 Subject: [PATCH 36/38] Modify various partial methods to carry along the block that can be passed in with render * _render_single_template, which renders a template without layout * _render_partial_unknown_type, which renders a partial of unknown type (the entry method for most partial rendering; supports strings, objects, and collections) * _render_partial_object, which renders a partial for a single object. * extracted _render_partial_path so it can be used to render the spacer without going through the public render :partial --- actionpack/lib/action_view/render/partials.rb | 61 +++++++++---------- .../lib/action_view/render/rendering.rb | 33 +++++++--- actionpack/lib/action_view/test_case.rb | 4 +- 3 files changed, 54 insertions(+), 44 deletions(-) diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 8e094240d3..3141eb20d5 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -188,27 +188,33 @@ module ActionView _render_partial_unknown_type(options) end - def _render_partial_unknown_type(options) #:nodoc: + def _render_partial_unknown_type(options, &block) #:nodoc: options[:locals] ||= {} path = partial = options[:partial] if partial.respond_to?(:to_ary) - return _render_partial_collection(partial, options) + return _render_partial_collection(partial, options, &block) elsif !partial.is_a?(String) options[:object] = object = partial path = _partial_path(object) end - parts = [path, {:formats => formats}] - parts.push path.include?(?/) ? nil : controller_path - parts.push true - - template = find_by_parts(*parts) - _render_partial_object(template, options) + _render_partial_path(path, options, &block) end private + def _render_partial_path(path, options, &block) + return '' if path.nil? + + parts = [path, {:formats => formats}] + parts.push path.include?(?/) ? nil : controller_path + parts.push true + + template = find_by_parts(*parts) + _render_partial_object(template, options, &block) + end + def _partial_path(object) self.class._partial_names[[controller.class, object.class]] ||= begin name = object.class.model_name @@ -220,14 +226,9 @@ module ActionView end end - def _render_partial(layout, options, block = nil) - if block - begin - @_proc_for_layout = block - concat(_render_partial_unknown_type(options.merge(:partial => layout))) - ensure - @_proc_for_layout = nil - end + def _render_partial(layout, options, &block) + if block_given? + concat(_render_partial_unknown_type(options.merge(:partial => layout), &block)) else if layout prefix = layout.include?(?/) ? nil : controller_path @@ -237,11 +238,11 @@ module ActionView end end - def _render_partial_object(template, options) + def _render_partial_object(template, options, &block) object = options[:object] if options.key?(:collection) - _render_partial_collection(options.delete(:collection), options, template) + _render_partial_collection(options.delete(:collection), options, template, &block) else locals = (options[:locals] ||= {}) object ||= locals[:object] || locals[template.variable_name] @@ -250,7 +251,7 @@ module ActionView options[:_template] = template - _render_single_template(template, locals) + _render_single_template(template, locals, &block) end end @@ -259,26 +260,23 @@ module ActionView locals[options[:as]] = object if options[:as] end - def _render_partial_collection(collection, options = {}, passed_template = nil) #:nodoc: + def _render_partial_collection(collection, options = {}, template = nil, &block) #:nodoc: return nil if collection.blank? - spacer = options[:spacer_template] ? - _render_partial_unknown_type(:partial => options[:spacer_template]) : '' + spacer = _render_partial_path(options[:spacer_template], {}) + + locals, index = options[:locals] || {}, 0 - locals = (options[:locals] ||= {}) - index, @_partial_path = 0, nil collection.map do |object| - options[:_template] = template = passed_template || begin - _partial_path = _partial_path(object) - template = _pick_partial_template(_partial_path) - end + tmp = template || _pick_partial_template(_partial_path(object)) + options[:_template] = tmp - _set_locals(object, locals, template, options) - locals[template.counter_name] = index + _set_locals(object, locals, tmp, options) + locals[tmp.counter_name] = index index += 1 - _render_single_template(template, locals) + _render_single_template(tmp, locals, &block) end.join(spacer) end @@ -286,6 +284,5 @@ module ActionView prefix = controller_path unless partial_path.include?('/') find_by_parts(partial_path, {:formats => formats}, prefix, true) end - memoize :_pick_partial_template end end diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 0a5953be88..a721ade4e1 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -18,7 +18,7 @@ module ActionView layout = options[:layout] if options.key?(:partial) || block_given? - return _render_partial(layout, options, block) + return _render_partial(layout, options, &block) end layout = find_by_parts(layout, {:formats => formats}) if layout @@ -56,10 +56,10 @@ module ActionView end end - # You can think of a layout as a method that is called with a block. This method - # returns the block that the layout is called with. If the user calls yield :some_name, - # the block, by default, returns content_for(:some_name). If the user calls yield, - # the default block returns content_for(:layout). + # You can think of a layout as a method that is called with a block. _layout_for + # returns the contents that are yielded to the layout. If the user calls yield + # :some_name, the block, by default, returns content_for(:some_name). If the user + # calls yield, the default block returns content_for(:layout). # # The user can override this default by passing a block to the layout. # @@ -88,15 +88,28 @@ module ActionView # In this case, the layout would receive the block passed into render :layout, # and the Struct specified in the layout would be passed into the block. The result # would be Hello David. - def layout_proc(name) - @_default_layout ||= proc { |*names| @_content_for[names.first || :layout] } - !@_content_for.key?(name) && @_proc_for_layout || @_default_layout + def _layout_for(names, &block) + with_output_buffer do + # This is due to the potentially ambiguous use of yield when + # a block is passed in to a template *and* there is a content_for() + # of the same name. Suggested solution: require explicit use of content_for + # in these ambiguous cases. + # + # We would be able to continue supporting yield in all non-ambiguous + # cases. Question: should we deprecate yield in favor of content_for + # and reserve yield for cases where there is a yield into a real block? + if @_content_for.key?(names.first) || !block_given? + return @_content_for[names.first || :layout] + else + return yield(names) + end + end end - def _render_single_template(template, locals = {}) + def _render_single_template(template, locals = {}, &block) with_template(template) do template.render(self, locals) do |*names| - capture(*names, &layout_proc(names.first)) + _layout_for(names, &block) end end rescue Exception => e diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 71a4a88afe..e51744d095 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -10,13 +10,13 @@ module ActionView attr_internal :rendered alias_method :_render_template_without_template_tracking, :_render_single_template - def _render_single_template(template, local_assigns = {}) + def _render_single_template(template, local_assigns, &block) if template.respond_to?(:identifier) && template.present? @_rendered[:partials][template] += 1 if template.partial? @_rendered[:template] ||= [] @_rendered[:template] << template end - _render_template_without_template_tracking(template, local_assigns) + _render_template_without_template_tracking(template, local_assigns, &block) end end From d94ba11295c74e5a661a78a93d6d0259ab01fa50 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Fri, 7 Aug 2009 06:32:17 -0300 Subject: [PATCH 37/38] Continue reworking the partial path. * TODO: Review ActionController calling render_template for certain partials. Might we be able to save logic by always delegating to AV's render_partial? --- actionpack/lib/action_view/render/partials.rb | 60 +++++-------------- .../lib/action_view/render/rendering.rb | 9 ++- 2 files changed, 22 insertions(+), 47 deletions(-) diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 3141eb20d5..b7b14e9007 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -185,10 +185,10 @@ module ActionView def render_partial(options) @assigns_added = false - _render_partial_unknown_type(options) + _render_partial(options) end - def _render_partial_unknown_type(options, &block) #:nodoc: + def _render_partial(options, &block) #:nodoc: options[:locals] ||= {} path = partial = options[:partial] @@ -200,21 +200,10 @@ module ActionView path = _partial_path(object) end - _render_partial_path(path, options, &block) + _render_partial_object(_pick_partial_template(path), options, &block) end private - def _render_partial_path(path, options, &block) - return '' if path.nil? - - parts = [path, {:formats => formats}] - parts.push path.include?(?/) ? nil : controller_path - parts.push true - - template = find_by_parts(*parts) - _render_partial_object(template, options, &block) - end - def _partial_path(object) self.class._partial_names[[controller.class, object.class]] ||= begin name = object.class.model_name @@ -226,62 +215,45 @@ module ActionView end end - def _render_partial(layout, options, &block) - if block_given? - concat(_render_partial_unknown_type(options.merge(:partial => layout), &block)) - else - if layout - prefix = layout.include?(?/) ? nil : controller_path - layout = find_by_parts(layout, {:formats => formats}, prefix, true) - end - _render_content(_render_partial_unknown_type(options), layout, options[:locals]) - end + def _render_partial_template(template, locals, object, options = {}, &block) + options[:_template] = template + locals[:object] = locals[template.variable_name] = object + locals[options[:as]] = object if options[:as] + + _render_single_template(template, locals, &block) end def _render_partial_object(template, options, &block) - object = options[:object] - if options.key?(:collection) _render_partial_collection(options.delete(:collection), options, template, &block) else locals = (options[:locals] ||= {}) - object ||= locals[:object] || locals[template.variable_name] + object = options[:object] || locals[:object] || locals[template.variable_name] - _set_locals(object, locals, template, options) - - options[:_template] = template - - _render_single_template(template, locals, &block) + _render_partial_template(template, locals, object, options, &block) end end - def _set_locals(object, locals, template, options) - locals[:object] = locals[template.variable_name] = object - locals[options[:as]] = object if options[:as] - end - def _render_partial_collection(collection, options = {}, template = nil, &block) #:nodoc: return nil if collection.blank? - spacer = _render_partial_path(options[:spacer_template], {}) + if options.key?(:spacer_template) + spacer = _render_partial(:partial => options[:spacer_template]) + end locals, index = options[:locals] || {}, 0 collection.map do |object| tmp = template || _pick_partial_template(_partial_path(object)) - options[:_template] = tmp - - _set_locals(object, locals, tmp, options) locals[tmp.counter_name] = index - index += 1 - _render_single_template(tmp, locals, &block) + _render_partial_template(tmp, locals, object, options, &block) end.join(spacer) end def _pick_partial_template(partial_path) #:nodoc: - prefix = controller_path unless partial_path.include?('/') + prefix = controller_path unless partial_path.include?(?/) find_by_parts(partial_path, {:formats => formats}, prefix, true) end end diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index a721ade4e1..39b8608c68 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -13,12 +13,15 @@ module ActionView def render(options = {}, locals = {}, &block) #:nodoc: case options when String, NilClass - _render_partial_unknown_type(:partial => options, :locals => locals || {}) + _render_partial(:partial => options, :locals => locals || {}) when Hash layout = options[:layout] - if options.key?(:partial) || block_given? - return _render_partial(layout, options, &block) + if block_given? + return concat(_render_partial(options.merge(:partial => layout), &block)) + elsif options.key?(:partial) + layout = _pick_partial_template(layout) if layout + return _render_content(_render_partial(options), layout, options[:locals]) end layout = find_by_parts(layout, {:formats => formats}) if layout From 606e950ccbd02a10f724c73543575a2a4e1ed8cb Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Fri, 7 Aug 2009 06:32:54 -0300 Subject: [PATCH 38/38] Whitespace --- actionpack/lib/action_view/render/rendering.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 39b8608c68..9c25fab6bb 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -16,16 +16,16 @@ module ActionView _render_partial(:partial => options, :locals => locals || {}) when Hash layout = options[:layout] - + if block_given? return concat(_render_partial(options.merge(:partial => layout), &block)) elsif options.key?(:partial) layout = _pick_partial_template(layout) if layout return _render_content(_render_partial(options), layout, options[:locals]) end - + layout = find_by_parts(layout, {:formats => formats}) if layout - + if file = options[:file] template = find_by_parts(file, {:formats => formats}) _render_template(template, layout, :locals => options[:locals] || {}) @@ -38,17 +38,17 @@ module ActionView update_page(&block) end end - + def _render_content(content, layout, locals) return content unless layout - + locals ||= {} if controller && layout @_layout = layout.identifier logger.info("Rendering template within #{layout.identifier}") if logger end - + begin old_content, @_content_for[:layout] = @_content_for[:layout], content @@ -161,7 +161,7 @@ module ActionView else _render_single_template(template, locals) end - + _render_content(content, layout, locals) end end