diff options
| author | Pratik Naik <pratiknaik@gmail.com> | 2008-09-10 19:06:59 +0100 |
|---|---|---|
| committer | Pratik Naik <pratiknaik@gmail.com> | 2008-09-10 19:06:59 +0100 |
| commit | 6d1be5f1eb83fb693ffd00e1967c1b3ca1c9ece3 (patch) | |
| tree | 059a4da48e9d4ec113c7df2bf915e83244ea3820 | |
| parent | d0a2b849f469469a1b189b4d077a95f35e31d65a (diff) | |
| parent | c19c0e7872e65094b14bc08e24d19eebd9d1562a (diff) | |
| download | rails-6d1be5f1eb83fb693ffd00e1967c1b3ca1c9ece3.tar.gz rails-6d1be5f1eb83fb693ffd00e1967c1b3ca1c9ece3.tar.bz2 rails-6d1be5f1eb83fb693ffd00e1967c1b3ca1c9ece3.zip | |
Merge commit 'mainstream/master'
56 files changed, 640 insertions, 333 deletions
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 96e514e0db..bfe435550b 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -466,7 +466,7 @@ module ActionMailer #:nodoc: template = template_root["#{mailer_name}/#{File.basename(path)}"] # Skip unless template has a multipart format - next unless template.multipart? + next unless template && template.multipart? @parts << Part.new( :content_type => template.content_type, diff --git a/actionmailer/test/fixtures/test_mailer/implicitly_multipart_example.text.html.erb~ b/actionmailer/test/fixtures/test_mailer/implicitly_multipart_example.text.html.erb~ new file mode 100644 index 0000000000..946d99ede5 --- /dev/null +++ b/actionmailer/test/fixtures/test_mailer/implicitly_multipart_example.text.html.erb~ @@ -0,0 +1,10 @@ +<html> + <body> + HTML formatted message to <strong><%= @recipient %></strong>. + </body> +</html> +<html> + <body> + HTML formatted message to <strong><%= @recipient %></strong>. + </body> +</html> diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index cb61c8d2dd..3743e3e8fe 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,11 @@ *Edge* +* Added FormTagHelper#image_submit_tag confirm option #784 [Alastair Brunton] + +* Fixed FormTagHelper#submit_tag with :disable_with option wouldn't submit the button's value when was clicked #633 [Jose Fernandez] + +* Stopped logging template compiles as it only clogs up the log [DHH] + * Changed the X-Runtime header to report in milliseconds [DHH] * Changed BenchmarkHelper#benchmark to report in milliseconds [DHH] diff --git a/actionpack/Rakefile b/actionpack/Rakefile index 1880f21f67..0ee9382e89 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -25,14 +25,16 @@ task :default => [ :test ] desc "Run all unit tests" task :test => [:test_action_pack, :test_active_record_integration] -Rake::TestTask.new(:test_action_pack) { |t| +Rake::TestTask.new(:test_action_pack) do |t| t.libs << "test" -# make sure we include the tests in alphabetical order as on some systems -# this will not happen automatically and the tests (as a whole) will error - t.test_files=Dir.glob( "test/[cft]*/**/*_test.rb" ).sort -# t.pattern = 'test/*/*_test.rb' + + # make sure we include the tests in alphabetical order as on some systems + # this will not happen automatically and the tests (as a whole) will error + t.test_files = Dir.glob( "test/[cft]*/**/*_test.rb" ).sort + t.verbose = true -} + #t.warning = true +end desc 'ActiveRecord Integration Tests' Rake::TestTask.new(:test_active_record_integration) do |t| diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index e58071d4af..2efd0dad2e 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -54,6 +54,7 @@ require 'action_controller/rack_process' require 'action_controller/record_identifier' require 'action_controller/request_forgery_protection' require 'action_controller/headers' +require 'action_controller/translation' require 'action_view' @@ -74,4 +75,5 @@ ActionController::Base.class_eval do include ActionController::Components include ActionController::RecordIdentifier include ActionController::RequestForgeryProtection + include ActionController::Translation end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 670a049497..457b9e85bc 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -434,7 +434,11 @@ module ActionController #:nodoc: # render("test/template") will be looked up in the view load paths array and the closest match will be # returned. def view_paths - @view_paths || superclass.view_paths + if defined? @view_paths + @view_paths + else + superclass.view_paths + end end def view_paths=(value) @@ -449,7 +453,7 @@ module ActionController #:nodoc: # ArticleController.prepend_view_path(["views/default", "views/custom"]) # def prepend_view_path(path) - @view_paths = superclass.view_paths.dup if @view_paths.nil? + @view_paths = superclass.view_paths.dup if !defined?(@view_paths) || @view_paths.nil? @view_paths.unshift(*path) end diff --git a/actionpack/lib/action_controller/benchmarking.rb b/actionpack/lib/action_controller/benchmarking.rb index 746894497c..fa572ebf3d 100644 --- a/actionpack/lib/action_controller/benchmarking.rb +++ b/actionpack/lib/action_controller/benchmarking.rb @@ -76,7 +76,8 @@ module ActionController #:nodoc: log_message << view_runtime if logging_view if logging_active_record - log_message << ", " + active_record_runtime + ")" + log_message << ", " if logging_view + log_message << active_record_runtime + ")" else ")" end diff --git a/actionpack/lib/action_controller/routing/recognition_optimisation.rb b/actionpack/lib/action_controller/routing/recognition_optimisation.rb index 6d54d0334c..4935432d87 100644 --- a/actionpack/lib/action_controller/routing/recognition_optimisation.rb +++ b/actionpack/lib/action_controller/routing/recognition_optimisation.rb @@ -134,6 +134,9 @@ module ActionController def write_recognize_optimized! tree = segment_tree(routes) body = generate_code(tree) + + remove_recognize_optimized! + instance_eval %{ def recognize_optimized(path, env) segments = to_plain_segments(path) @@ -147,6 +150,25 @@ module ActionController end }, __FILE__, __LINE__ end + + def clear_recognize_optimized! + remove_recognize_optimized! + + class << self + def recognize_optimized(path, environment) + write_recognize_optimized! + recognize_optimized(path, environment) + end + end + end + + def remove_recognize_optimized! + if respond_to?(:recognize_optimized) + class << self + remove_method :recognize_optimized + end + end + end end end end diff --git a/actionpack/lib/action_controller/routing/route_set.rb b/actionpack/lib/action_controller/routing/route_set.rb index 9d48f289db..ff448490e9 100644 --- a/actionpack/lib/action_controller/routing/route_set.rb +++ b/actionpack/lib/action_controller/routing/route_set.rb @@ -195,7 +195,7 @@ module ActionController self.routes = [] self.named_routes = NamedRouteCollection.new - write_recognize_optimized! + clear_recognize_optimized! end # Subclasses and plugins may override this method to specify a different @@ -217,7 +217,7 @@ module ActionController @routes_by_controller = nil # This will force routing/recognition_optimization.rb # to refresh optimisations. - @compiled_recognize_optimized = nil + clear_recognize_optimized! end def install_helpers(destinations = [ActionController::Base, ActionView::Base], regenerate_code = false) diff --git a/actionpack/lib/action_controller/translation.rb b/actionpack/lib/action_controller/translation.rb new file mode 100644 index 0000000000..9bb63cdb15 --- /dev/null +++ b/actionpack/lib/action_controller/translation.rb @@ -0,0 +1,13 @@ +module ActionController + module Translation + def translate(*args) + I18n.translate *args + end + alias :t :translate + + def localize(*args) + I18n.localize *args + end + alias :l :localize + end +end
\ No newline at end of file diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 6c4da9067d..8c00670087 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -278,9 +278,9 @@ module ActionView #:nodoc: # the same name but differing formats. See +Request#template_format+ # for more details. def template_format - return @template_format if @template_format - - if controller && controller.respond_to?(:request) + if defined? @template_format + @template_format + elsif controller && controller.respond_to?(:request) @template_format = controller.request.template_format else @template_format = :html @@ -366,7 +366,9 @@ module ActionView #:nodoc: end else begin - original_content_for_layout, @content_for_layout = @content_for_layout, render(options) + original_content_for_layout = @content_for_layout if defined?(@content_for_layout) + @content_for_layout = render(options) + if (options[:inline] || options[:file] || options[:text]) @cached_content_for_layout = @content_for_layout render(:file => partial_layout, :locals => local_assigns) diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb index e5777b71d7..294c22521e 100644 --- a/actionpack/lib/action_view/helpers/form_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb @@ -116,7 +116,7 @@ module ActionView # Creates a label field # - # ==== Options + # ==== Options # * Creates standard HTML attributes for the tag. # # ==== Examples @@ -351,19 +351,16 @@ module ActionView disable_with = "this.value='#{disable_with}'" disable_with << ";#{options.delete('onclick')}" if options['onclick'] - options["onclick"] = [ - "this.setAttribute('originalValue', this.value)", - "this.disabled=true", - disable_with, - "result = (this.form.onsubmit ? (this.form.onsubmit() ? this.form.submit() : false) : this.form.submit())", - "if (result == false) { this.value = this.getAttribute('originalValue'); this.disabled = false }", - "return result;", - ].join(";") + options["onclick"] = "if (window.hiddenCommit) { window.hiddenCommit.setAttribute('value', this.value); }" + options["onclick"] << "else { hiddenCommit = this.cloneNode(false);hiddenCommit.setAttribute('type', 'hidden');this.form.appendChild(hiddenCommit); }" + options["onclick"] << "this.setAttribute('originalValue', this.value);this.disabled = true;#{disable_with};" + options["onclick"] << "result = (this.form.onsubmit ? (this.form.onsubmit() ? this.form.submit() : false) : this.form.submit());" + options["onclick"] << "if (result == false) { this.value = this.getAttribute('originalValue');this.disabled = false; }return result;" end if confirm = options.delete("confirm") options["onclick"] ||= '' - options["onclick"] += "return #{confirm_javascript_function(confirm)};" + options["onclick"] << "return #{confirm_javascript_function(confirm)};" end tag :input, { "type" => "submit", "name" => "commit", "value" => value }.update(options.stringify_keys) @@ -374,6 +371,9 @@ module ActionView # <tt>source</tt> is passed to AssetTagHelper#image_path # # ==== Options + # * <tt>:confirm => 'question?'</tt> - This will add a JavaScript confirm + # prompt with the question specified. If the user accepts, the form is + # processed normally, otherwise no action is taken. # * <tt>:disabled</tt> - If set to true, the user will not be able to use this input. # * Any other key creates standard HTML options for the tag. # @@ -390,6 +390,13 @@ module ActionView # image_submit_tag("agree.png", :disabled => true, :class => "agree-disagree-button") # # => <input class="agree-disagree-button" disabled="disabled" src="/images/agree.png" type="image" /> def image_submit_tag(source, options = {}) + options.stringify_keys! + + if confirm = options.delete("confirm") + options["onclick"] ||= '' + options["onclick"] += "return #{confirm_javascript_function(confirm)};" + end + tag :input, { "type" => "image", "src" => path_to_image(source) }.update(options.stringify_keys) end diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index d960335220..0134bc988f 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -1,8 +1,7 @@ module ActionView - module Renderable - # NOTE: The template that this mixin is beening include into is frozen - # So you can not set or modify any instance variables - + # NOTE: The template that this mixin is being included into is frozen + # so you cannot set or modify any instance variables + module Renderable #:nodoc: extend ActiveSupport::Memoizable def self.included(base) @@ -33,10 +32,11 @@ module ActionView view.send(:_set_controller_content_type, mime_type) if respond_to?(:mime_type) view.send(method_name(local_assigns), local_assigns) do |*names| - if proc = view.instance_variable_get("@_proc_for_layout") + ivar = :@_proc_for_layout + if view.instance_variable_defined?(ivar) and proc = view.instance_variable_get(ivar) view.capture(*names, &proc) - else - view.instance_variable_get("@content_for_#{names.first || 'layout'}") + elsif view.instance_variable_defined?(ivar = :"@content_for_#{names.first || :layout}") + view.instance_variable_get(ivar) end end end @@ -72,12 +72,9 @@ module ActionView end_src begin - logger = defined?(ActionController) && Base.logger - logger.debug "Compiling template #{render_symbol}" if logger - ActionView::Base::CompiledTemplates.module_eval(source, filename, 0) rescue Exception => e # errors from template code - if logger + if logger = defined?(ActionController) && Base.logger logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}" logger.debug "Function body: #{source}" logger.debug "Backtrace: #{e.backtrace.join("\n")}" diff --git a/actionpack/lib/action_view/renderable_partial.rb b/actionpack/lib/action_view/renderable_partial.rb index 123a9aebbc..d92ff1b8d3 100644 --- a/actionpack/lib/action_view/renderable_partial.rb +++ b/actionpack/lib/action_view/renderable_partial.rb @@ -1,8 +1,7 @@ module ActionView - module RenderablePartial - # NOTE: The template that this mixin is beening include into is frozen - # So you can not set or modify any instance variables - + # NOTE: The template that this mixin is being included into is frozen + # so you cannot set or modify any instance variables + module RenderablePartial #:nodoc: extend ActiveSupport::Memoizable def variable_name @@ -30,10 +29,13 @@ module ActionView local_assigns[variable_name] if view.respond_to?(:controller) - object ||= ActiveSupport::Deprecation::DeprecatedObjectProxy.new( - view.controller.instance_variable_get("@#{variable_name}"), - "@#{variable_name} will no longer be implicitly assigned to #{variable_name}" - ) + ivar = :"@#{variable_name}" + object ||= + if view.controller.instance_variable_defined?(ivar) + ActiveSupport::Deprecation::DeprecatedObjectProxy.new( + view.controller.instance_variable_get(ivar), + "#{ivar} will no longer be implicitly assigned to #{variable_name}") + end end # Ensure correct object is reassigned to other accessors diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 8bb1c49cbd..1eb26a7cfd 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -1375,6 +1375,36 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do x.send(:foo_with_requirement_url, "I am Against the requirements") end end + + def test_routes_changed_correctly_after_clear + ActionController::Base.optimise_named_routes = true + rs = ::ActionController::Routing::RouteSet.new + rs.draw do |r| + r.connect 'ca', :controller => 'ca', :action => "aa" + r.connect 'cb', :controller => 'cb', :action => "ab" + r.connect 'cc', :controller => 'cc', :action => "ac" + r.connect ':controller/:action/:id' + r.connect ':controller/:action/:id.:format' + end + + hash = rs.recognize_path "/cc" + + assert_not_nil hash + assert_equal %w(cc ac), [hash[:controller], hash[:action]] + + rs.draw do |r| + r.connect 'cb', :controller => 'cb', :action => "ab" + r.connect 'cc', :controller => 'cc', :action => "ac" + r.connect ':controller/:action/:id' + r.connect ':controller/:action/:id.:format' + end + + hash = rs.recognize_path "/cc" + + assert_not_nil hash + assert_equal %w(cc ac), [hash[:controller], hash[:action]] + + end end class RouteTest < Test::Unit::TestCase diff --git a/actionpack/test/controller/translation_test.rb b/actionpack/test/controller/translation_test.rb new file mode 100644 index 0000000000..0bf61a6556 --- /dev/null +++ b/actionpack/test/controller/translation_test.rb @@ -0,0 +1,26 @@ +require 'abstract_unit' + +# class TranslatingController < ActionController::Base +# end + +class TranslationControllerTest < Test::Unit::TestCase + def setup + @controller = ActionController::Base.new + end + + def test_action_controller_base_responds_to_translate + assert @controller.respond_to?(:translate) + end + + def test_action_controller_base_responds_to_t + assert @controller.respond_to?(:t) + end + + def test_action_controller_base_responds_to_localize + assert @controller.respond_to?(:localize) + end + + def test_action_controller_base_responds_to_l + assert @controller.respond_to?(:l) + end +end
\ No newline at end of file diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb index 9b41ff8179..6473d011d5 100644 --- a/actionpack/test/template/form_tag_helper_test.rb +++ b/actionpack/test/template/form_tag_helper_test.rb @@ -223,14 +223,14 @@ class FormTagHelperTest < ActionView::TestCase def test_submit_tag assert_dom_equal( - %(<input name='commit' type='submit' value='Save' onclick="this.setAttribute('originalValue', this.value);this.disabled=true;this.value='Saving...';alert('hello!');result = (this.form.onsubmit ? (this.form.onsubmit() ? this.form.submit() : false) : this.form.submit());if (result == false) { this.value = this.getAttribute('originalValue'); this.disabled = false };return result;" />), + %(<input name='commit' type='submit' value='Save' onclick="if (window.hiddenCommit) { window.hiddenCommit.setAttribute('value', this.value); }else { hiddenCommit = this.cloneNode(false);hiddenCommit.setAttribute('type', 'hidden');this.form.appendChild(hiddenCommit); }this.setAttribute('originalValue', this.value);this.disabled = true;this.value='Saving...';alert('hello!');result = (this.form.onsubmit ? (this.form.onsubmit() ? this.form.submit() : false) : this.form.submit());if (result == false) { this.value = this.getAttribute('originalValue');this.disabled = false; }return result;" />), submit_tag("Save", :disable_with => "Saving...", :onclick => "alert('hello!')") ) end def test_submit_tag_with_no_onclick_options assert_dom_equal( - %(<input name='commit' type='submit' value='Save' onclick="this.setAttribute('originalValue', this.value);this.disabled=true;this.value='Saving...';result = (this.form.onsubmit ? (this.form.onsubmit() ? this.form.submit() : false) : this.form.submit());if (result == false) { this.value = this.getAttribute('originalValue'); this.disabled = false };return result;" />), + %(<input name='commit' type='submit' value='Save' onclick="if (window.hiddenCommit) { window.hiddenCommit.setAttribute('value', this.value); }else { hiddenCommit = this.cloneNode(false);hiddenCommit.setAttribute('type', 'hidden');this.form.appendChild(hiddenCommit); }this.setAttribute('originalValue', this.value);this.disabled = true;this.value='Saving...';result = (this.form.onsubmit ? (this.form.onsubmit() ? this.form.submit() : false) : this.form.submit());if (result == false) { this.value = this.getAttribute('originalValue');this.disabled = false; }return result;" />), submit_tag("Save", :disable_with => "Saving...") ) end @@ -241,6 +241,13 @@ class FormTagHelperTest < ActionView::TestCase submit_tag("Save", :confirm => "Are you sure?") ) end + + def test_image_submit_tag_with_confirmation + assert_dom_equal( + %(<input type="image" src="/images/save.gif" onclick="return confirm('Are you sure?');"/>), + image_submit_tag("save.gif", :confirm => "Are you sure?") + ) + end def test_pass assert_equal 1, 1 diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index f6b9913790..5bcc7ad3a9 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,9 @@ *Edge* +* Added find_last_by dynamic finder #762 [miloops] + +* Internal API: configurable association options and build_association method for reflections so plugins may extend and override. #985 [Hongli Lai] + * Changed benchmarks to be reported in milliseconds [DHH] * Connection pooling. #936 [Nick Sieger] @@ -16,24 +20,6 @@ * Fixed that create database statements would always include "DEFAULT NULL" (Nick Sieger) [#334] -* Add :accessible option to associations for allowing (opt-in) mass assignment. #474. [David Dollar] Example : - - class Post < ActiveRecord::Base - belongs_to :author, :accessible => true - has_many :comments, :accessible => true - end - - post = Post.create({ - :title => 'Accessible Attributes', - :author => { :name => 'David Dollar' }, - :comments => [ - { :body => 'First Post!' }, - { :body => 'Nested Hashes are great!' } - ] - }) - - post.comments << { :body => 'Another Comment' } - * Add :tokenizer option to validates_length_of to specify how to split up the attribute string. #507. [David Lowenfels] Example : # Ensure essay contains at least 100 words. diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb index a5d3a50ef1..9eee7f2d98 100644 --- a/activerecord/lib/active_record/aggregations.rb +++ b/activerecord/lib/active_record/aggregations.rb @@ -10,10 +10,10 @@ module ActiveRecord end unless self.new_record? end - # Active Record implements aggregation through a macro-like class method called +composed_of+ for representing attributes + # Active Record implements aggregation through a macro-like class method called +composed_of+ for representing attributes # as value objects. It expresses relationships like "Account [is] composed of Money [among other things]" or "Person [is] - # composed of [an] address". Each call to the macro adds a description of how the value objects are created from the - # attributes of the entity object (when the entity is initialized either as a new object or from finding an existing object) + # composed of [an] address". Each call to the macro adds a description of how the value objects are created from the + # attributes of the entity object (when the entity is initialized either as a new object or from finding an existing object) # and how it can be turned back into attributes (when the entity is saved to the database). Example: # # class Customer < ActiveRecord::Base @@ -30,10 +30,10 @@ module ActiveRecord # class Money # include Comparable # attr_reader :amount, :currency - # EXCHANGE_RATES = { "USD_TO_DKK" => 6 } - # - # def initialize(amount, currency = "USD") - # @amount, @currency = amount, currency + # EXCHANGE_RATES = { "USD_TO_DKK" => 6 } + # + # def initialize(amount, currency = "USD") + # @amount, @currency = amount, currency # end # # def exchange_to(other_currency) @@ -56,19 +56,19 @@ module ActiveRecord # # class Address # attr_reader :street, :city - # def initialize(street, city) - # @street, @city = street, city + # def initialize(street, city) + # @street, @city = street, city # end # - # def close_to?(other_address) - # city == other_address.city + # def close_to?(other_address) + # city == other_address.city # end # # def ==(other_address) # city == other_address.city && street == other_address.street # end # end - # + # # Now it's possible to access attributes from the database through the value objects instead. If you choose to name the # composition the same as the attribute's name, it will be the only way to access that attribute. That's the case with our # +balance+ attribute. You interact with the value objects just like you would any other attribute, though: @@ -87,8 +87,8 @@ module ActiveRecord # customer.address_city = "Copenhagen" # customer.address # => Address.new("Hyancintvej", "Copenhagen") # customer.address = Address.new("May Street", "Chicago") - # customer.address_street # => "May Street" - # customer.address_city # => "Chicago" + # customer.address_street # => "May Street" + # customer.address_city # => "Chicago" # # == Writing value objects # @@ -103,12 +103,51 @@ module ActiveRecord # returns a new value object instead of changing its own values. Active Record won't persist value objects that have been # changed through means other than the writer method. # - # The immutable requirement is enforced by Active Record by freezing any object assigned as a value object. Attempting to + # The immutable requirement is enforced by Active Record by freezing any object assigned as a value object. Attempting to # change it afterwards will result in a ActiveSupport::FrozenObjectError. - # + # # Read more about value objects on http://c2.com/cgi/wiki?ValueObject and on the dangers of not keeping value objects # immutable on http://c2.com/cgi/wiki?ValueObjectsShouldBeImmutable # + # == Custom constructors and converters + # + # By default value objects are initialized by calling the <tt>new</tt> constructor of the value class passing each of the + # mapped attributes, in the order specified by the <tt>:mapping</tt> option, as arguments. If the value class doesn't support + # this convention then +composed_of+ allows a custom constructor to be specified. + # + # When a new value is assigned to the value object the default assumption is that the new value is an instance of the value + # class. Specifying a custom converter allows the new value to be automatically converted to an instance of value class if + # necessary. + # + # For example, the NetworkResource model has +network_address+ and +cidr_range+ attributes that should be aggregated using the + # NetAddr::CIDR value class (http://netaddr.rubyforge.org). The constructor for the value class is called +create+ and it + # expects a CIDR address string as a parameter. New values can be assigned to the value object using either another + # NetAddr::CIDR object, a string or an array. The <tt>:constructor</tt> and <tt>:converter</tt> options can be used to + # meet these requirements: + # + # class NetworkResource < ActiveRecord::Base + # composed_of :cidr, + # :class_name => 'NetAddr::CIDR', + # :mapping => [ %w(network_address network), %w(cidr_range bits) ], + # :allow_nil => true, + # :constructor => Proc.new { |network_address, cidr_range| NetAddr::CIDR.create("#{network_address}/#{cidr_range}") }, + # :converter => Proc.new { |value| NetAddr::CIDR.create(value.is_a?(Array) ? value.join('/') : value) } + # end + # + # # This calls the :constructor + # network_resource = NetworkResource.new(:network_address => '192.168.0.1', :cidr_range => 24) + # + # # These assignments will both use the :converter + # network_resource.cidr = [ '192.168.2.1', 8 ] + # network_resource.cidr = '192.168.0.1/24' + # + # # This assignment won't use the :converter as the value is already an instance of the value class + # network_resource.cidr = NetAddr::CIDR.create('192.168.2.1/8') + # + # # Saving and then reloading will use the :constructor on reload + # network_resource.save + # network_resource.reload + # # == Finding records by a value object # # Once a +composed_of+ relationship is specified for a model, records can be loaded from the database by specifying an instance @@ -122,47 +161,71 @@ module ActiveRecord # <tt>composed_of :address</tt> adds <tt>address</tt> and <tt>address=(new_address)</tt> methods. # # Options are: - # * <tt>:class_name</tt> - specify the class name of the association. Use it only if that name can't be inferred + # * <tt>:class_name</tt> - Specifies the class name of the association. Use it only if that name can't be inferred # from the part id. So <tt>composed_of :address</tt> will by default be linked to the Address class, but # if the real class name is CompanyAddress, you'll have to specify it with this option. - # * <tt>:mapping</tt> - specifies a number of mapping arrays (attribute, parameter) that bind an attribute name - # to a constructor parameter on the value class. - # * <tt>:allow_nil</tt> - specifies that the aggregate object will not be instantiated when all mapped - # attributes are +nil+. Setting the aggregate class to +nil+ has the effect of writing +nil+ to all mapped attributes. + # * <tt>:mapping</tt> - Specifies the mapping of entity attributes to attributes of the value object. Each mapping + # is represented as an array where the first item is the name of the entity attribute and the second item is the + # name the attribute in the value object. The order in which mappings are defined determine the order in which + # attributes are sent to the value class constructor. + # * <tt>:allow_nil</tt> - Specifies that the value object will not be instantiated when all mapped + # attributes are +nil+. Setting the value object to +nil+ has the effect of writing +nil+ to all mapped attributes. # This defaults to +false+. - # - # An optional block can be passed to convert the argument that is passed to the writer method into an instance of - # <tt>:class_name</tt>. The block will only be called if the argument is not already an instance of <tt>:class_name</tt>. + # * <tt>:constructor</tt> - A symbol specifying the name of the constructor method or a Proc that is called to + # initialize the value object. The constructor is passed all of the mapped attributes, in the order that they + # are defined in the <tt>:mapping option</tt>, as arguments and uses them to instantiate a <tt>:class_name</tt> object. + # The default is <tt>:new</tt>. + # * <tt>:converter</tt> - A symbol specifying the name of a class method of <tt>:class_name</tt> or a Proc that is + # called when a new value is assigned to the value object. The converter is passed the single value that is used + # in the assignment and is only called if the new value is not an instance of <tt>:class_name</tt>. # # Option examples: # composed_of :temperature, :mapping => %w(reading celsius) - # composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) {|balance| balance.to_money } + # composed_of :balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| balance.to_money } # composed_of :address, :mapping => [ %w(address_street street), %w(address_city city) ] # composed_of :gps_location # composed_of :gps_location, :allow_nil => true + # composed_of :ip_address, + # :class_name => 'IPAddr', + # :mapping => %w(ip to_i), + # :constructor => Proc.new { |ip| IPAddr.new(ip, Socket::AF_INET) }, + # :converter => Proc.new { |ip| ip.is_a?(Integer) ? IPAddr.new(ip, Socket::AF_INET) : IPAddr.new(ip.to_s) } # def composed_of(part_id, options = {}, &block) - options.assert_valid_keys(:class_name, :mapping, :allow_nil) + options.assert_valid_keys(:class_name, :mapping, :allow_nil, :constructor, :converter) name = part_id.id2name - class_name = options[:class_name] || name.camelize - mapping = options[:mapping] || [ name, name ] + class_name = options[:class_name] || name.camelize + mapping = options[:mapping] || [ name, name ] mapping = [ mapping ] unless mapping.first.is_a?(Array) - allow_nil = options[:allow_nil] || false + allow_nil = options[:allow_nil] || false + constructor = options[:constructor] || :new + converter = options[:converter] || block + + ActiveSupport::Deprecation.warn('The conversion block has been deprecated, use the :converter option instead.', caller) if block_given? + + reader_method(name, class_name, mapping, allow_nil, constructor) + writer_method(name, class_name, mapping, allow_nil, converter) - reader_method(name, class_name, mapping, allow_nil) - writer_method(name, class_name, mapping, allow_nil, block) - create_reflection(:composed_of, part_id, options, self) end private - def reader_method(name, class_name, mapping, allow_nil) + def reader_method(name, class_name, mapping, allow_nil, constructor) module_eval do define_method(name) do |*args| force_reload = args.first || false if (instance_variable_get("@#{name}").nil? || force_reload) && (!allow_nil || mapping.any? {|pair| !read_attribute(pair.first).nil? }) - instance_variable_set("@#{name}", class_name.constantize.new(*mapping.collect {|pair| read_attribute(pair.first)})) + attrs = mapping.collect {|pair| read_attribute(pair.first)} + object = case constructor + when Symbol + class_name.constantize.send(constructor, *attrs) + when Proc, Method + constructor.call(*attrs) + else + raise ArgumentError, 'Constructor must be a symbol denoting the constructor method to call or a Proc to be invoked.' + end + instance_variable_set("@#{name}", object) end instance_variable_get("@#{name}") end @@ -170,14 +233,23 @@ module ActiveRecord end - def writer_method(name, class_name, mapping, allow_nil, conversion) + def writer_method(name, class_name, mapping, allow_nil, converter) module_eval do define_method("#{name}=") do |part| if part.nil? && allow_nil mapping.each { |pair| self[pair.first] = nil } instance_variable_set("@#{name}", nil) else - part = conversion.call(part) unless part.is_a?(class_name.constantize) || conversion.nil? + unless part.is_a?(class_name.constantize) || converter.nil? + part = case converter + when Symbol + class_name.constantize.send(converter, part) + when Proc, Method + converter.call(part) + else + raise ArgumentError, 'Converter must be a symbol denoting the converter method to call or a Proc to be invoked.' + end + end mapping.each { |pair| self[pair.first] = part.send(pair.last) } instance_variable_set("@#{name}", part.freeze) end diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 6405071354..aca2d770fc 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -739,9 +739,6 @@ module ActiveRecord # If true, all the associated objects are readonly through the association. # [:validate] # If false, don't validate the associated objects when saving the parent object. true by default. - # [:accessible] - # Mass assignment is allowed for this assocation (similar to <tt>ActiveRecord::Base#attr_accessible</tt>). - # Option examples: # has_many :comments, :order => "posted_on" # has_many :comments, :include => :author @@ -855,8 +852,6 @@ module ActiveRecord # If true, the associated object is readonly through the association. # [:validate] # If false, don't validate the associated object when saving the parent object. +false+ by default. - # [:accessible] - # Mass assignment is allowed for this assocation (similar to <tt>ActiveRecord::Base#attr_accessible</tt>). # # Option examples: # has_one :credit_card, :dependent => :destroy # destroys the associated credit card @@ -973,8 +968,6 @@ module ActiveRecord # If true, the associated object is readonly through the association. # [:validate] # If false, don't validate the associated objects when saving the parent object. +false+ by default. - # [:accessible] - # Mass assignment is allowed for this assocation (similar to <tt>ActiveRecord::Base#attr_accessible</tt>). # # Option examples: # belongs_to :firm, :foreign_key => "client_of" @@ -1190,8 +1183,6 @@ module ActiveRecord # If true, all the associated objects are readonly through the association. # [:validate] # If false, don't validate the associated objects when saving the parent object. +true+ by default. - # [:accessible<] - # Mass assignment is allowed for this assocation (similar to <tt>ActiveRecord::Base#attr_accessible</tt>). # # Option examples: # has_and_belongs_to_many :projects @@ -1266,8 +1257,6 @@ module ActiveRecord association = association_proxy_class.new(self, reflection) end - new_value = reflection.klass.new(new_value) if reflection.options[:accessible] && new_value.is_a?(Hash) - if association_proxy_class == HasOneThroughAssociation association.create_through_record(new_value) self.send(reflection.name, new_value) @@ -1509,29 +1498,35 @@ module ActiveRecord end end - def create_has_many_reflection(association_id, options, &extension) - options.assert_valid_keys( - :class_name, :table_name, :foreign_key, :primary_key, - :dependent, - :select, :conditions, :include, :order, :group, :limit, :offset, - :as, :through, :source, :source_type, - :uniq, - :finder_sql, :counter_sql, - :before_add, :after_add, :before_remove, :after_remove, - :extend, :readonly, - :validate, :accessible - ) + mattr_accessor :valid_keys_for_has_many_association + @@valid_keys_for_has_many_association = [ + :class_name, :table_name, :foreign_key, :primary_key, + :dependent, + :select, :conditions, :include, :order, :group, :limit, :offset, + :as, :through, :source, :source_type, + :uniq, + :finder_sql, :counter_sql, + :before_add, :after_add, :before_remove, :after_remove, + :extend, :readonly, + :validate + ] + def create_has_many_reflection(association_id, options, &extension) + options.assert_valid_keys(valid_keys_for_has_many_association) options[:extend] = create_extension_modules(association_id, extension, options[:extend]) create_reflection(:has_many, association_id, options, self) end - def create_has_one_reflection(association_id, options) - options.assert_valid_keys( - :class_name, :foreign_key, :remote, :select, :conditions, :order, :include, :dependent, :counter_cache, :extend, :as, :readonly, :validate, :primary_key, :accessible - ) + mattr_accessor :valid_keys_for_has_one_association + @@valid_keys_for_has_one_association = [ + :class_name, :foreign_key, :remote, :select, :conditions, :order, + :include, :dependent, :counter_cache, :extend, :as, :readonly, + :validate, :primary_key + ] + def create_has_one_reflection(association_id, options) + options.assert_valid_keys(valid_keys_for_has_one_association) create_reflection(:has_one, association_id, options, self) end @@ -1542,12 +1537,15 @@ module ActiveRecord create_reflection(:has_one, association_id, options, self) end - def create_belongs_to_reflection(association_id, options) - options.assert_valid_keys( - :class_name, :foreign_key, :foreign_type, :remote, :select, :conditions, :include, :dependent, - :counter_cache, :extend, :polymorphic, :readonly, :validate, :accessible - ) + mattr_accessor :valid_keys_for_belongs_to_association + @@valid_keys_for_belongs_to_association = [ + :class_name, :foreign_key, :foreign_type, :remote, :select, :conditions, + :include, :dependent, :counter_cache, :extend, :polymorphic, :readonly, + :validate + ] + def create_belongs_to_reflection(association_id, options) + options.assert_valid_keys(valid_keys_for_belongs_to_association) reflection = create_reflection(:belongs_to, association_id, options, self) if options[:polymorphic] @@ -1565,7 +1563,7 @@ module ActiveRecord :finder_sql, :delete_sql, :insert_sql, :before_add, :after_add, :before_remove, :after_remove, :extend, :readonly, - :validate, :accessible + :validate ) options[:extend] = create_extension_modules(association_id, extension, options[:extend]) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 168443e092..8de528f638 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -110,8 +110,6 @@ module ActiveRecord @owner.transaction do flatten_deeper(records).each do |record| - record = @reflection.klass.new(record) if @reflection.options[:accessible] && record.is_a?(Hash) - raise_on_type_mismatch(record) add_record_to_target_with_callbacks(record) do |r| result &&= insert_record(record) unless @owner.new_record? @@ -286,10 +284,6 @@ module ActiveRecord # Replace this collection with +other_array+ # This will perform a diff and delete/add only records that have changed. def replace(other_array) - other_array.map! do |val| - val.is_a?(Hash) ? @reflection.klass.new(val) : val - end if @reflection.options[:accessible] - other_array.each { |val| raise_on_type_mismatch(val) } load_target @@ -377,7 +371,9 @@ module ActiveRecord def create_record(attrs) attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash) ensure_owner_is_not_new - record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) { @reflection.klass.new(attrs) } + record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) do + @reflection.build_association(attrs) + end if block_given? add_record_to_target_with_callbacks(record) { |*block_args| yield(*block_args) } else @@ -387,7 +383,7 @@ module ActiveRecord def build_record(attrs) attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash) - record = @reflection.klass.new(attrs) + record = @reflection.build_association(attrs) if block_given? add_record_to_target_with_callbacks(record) { |*block_args| yield(*block_args) } else diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 7c28cbdd07..f05c6be075 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -2,11 +2,11 @@ module ActiveRecord module Associations class BelongsToAssociation < AssociationProxy #:nodoc: def create(attributes = {}) - replace(@reflection.klass.create(attributes)) + replace(@reflection.create_association(attributes)) end def build(attributes = {}) - replace(@reflection.klass.new(attributes)) + replace(@reflection.build_association(attributes)) end def replace(record) diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 84fa900f46..ebd2bf768c 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -10,14 +10,14 @@ module ActiveRecord def create!(attrs = nil) @reflection.klass.transaction do - self << (object = attrs ? @reflection.klass.send(:with_scope, :create => attrs) { @reflection.klass.create! } : @reflection.klass.create!) + self << (object = attrs ? @reflection.klass.send(:with_scope, :create => attrs) { @reflection.create_association! } : @reflection.create_association!) object end end def create(attrs = nil) @reflection.klass.transaction do - self << (object = attrs ? @reflection.klass.send(:with_scope, :create => attrs) { @reflection.klass.create } : @reflection.klass.create) + self << (object = attrs ? @reflection.klass.send(:with_scope, :create => attrs) { @reflection.create_association } : @reflection.create_association) object end end @@ -47,8 +47,9 @@ module ActiveRecord return false unless record.save end end - klass = @reflection.through_reflection.klass - @owner.send(@reflection.through_reflection.name).proxy_target << klass.send(:with_scope, :create => construct_join_attributes(record)) { klass.create! } + through_reflection = @reflection.through_reflection + klass = through_reflection.klass + @owner.send(@reflection.through_reflection.name).proxy_target << klass.send(:with_scope, :create => construct_join_attributes(record)) { through_reflection.create_association! } end # TODO - add dependent option support diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 18733255d2..c92ef5c2c9 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -7,15 +7,21 @@ module ActiveRecord end def create(attrs = {}, replace_existing = true) - new_record(replace_existing) { |klass| klass.create(attrs) } + new_record(replace_existing) do |reflection| + reflection.create_association(attrs) + end end def create!(attrs = {}, replace_existing = true) - new_record(replace_existing) { |klass| klass.create!(attrs) } + new_record(replace_existing) do |reflection| + reflection.create_association!(attrs) + end end def build(attrs = {}, replace_existing = true) - new_record(replace_existing) { |klass| klass.new(attrs) } + new_record(replace_existing) do |reflection| + reflection.build_association(attrs) + end end def replace(obj, dont_save = false) @@ -91,7 +97,9 @@ module ActiveRecord # instance. Otherwise, if the target has not previously been loaded # elsewhere, the instance we create will get orphaned. load_target if replace_existing - record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) { yield @reflection.klass } + record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) do + yield @reflection + end if replace_existing replace(record, true) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 907495a416..fc6d762fcd 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -274,7 +274,7 @@ module ActiveRecord #:nodoc: # == Dynamic attribute-based finders # # Dynamic attribute-based finders are a cleaner way of getting (and/or creating) objects by simple queries without turning to SQL. They work by - # appending the name of an attribute to <tt>find_by_</tt> or <tt>find_all_by_</tt>, so you get finders like <tt>Person.find_by_user_name</tt>, + # appending the name of an attribute to <tt>find_by_</tt>, <tt>find_last_by_</tt>, or <tt>find_all_by_</tt>, so you get finders like <tt>Person.find_by_user_name</tt>, # <tt>Person.find_all_by_last_name</tt>, and <tt>Payment.find_by_transaction_id</tt>. So instead of writing # <tt>Person.find(:first, :conditions => ["user_name = ?", user_name])</tt>, you just do <tt>Person.find_by_user_name(user_name)</tt>. # And instead of writing <tt>Person.find(:all, :conditions => ["last_name = ?", last_name])</tt>, you just do <tt>Person.find_all_by_last_name(last_name)</tt>. @@ -287,6 +287,7 @@ module ActiveRecord #:nodoc: # It's even possible to use all the additional parameters to find. For example, the full interface for <tt>Payment.find_all_by_amount</tt> # is actually <tt>Payment.find_all_by_amount(amount, options)</tt>. And the full interface to <tt>Person.find_by_user_name</tt> is # actually <tt>Person.find_by_user_name(user_name, options)</tt>. So you could call <tt>Payment.find_all_by_amount(50, :order => "created_on")</tt>. + # Also you may call <tt>Payment.find_last_by_amount(amount, options)</tt> returning the last record matching that amount and options. # # The same dynamic finder style can be used to create the object if it doesn't already exist. This dynamic finder is called with # <tt>find_or_create_by_</tt> and will return the object if it already exists and otherwise creates it, then returns it. Protected attributes won't be set unless they are given in a block. For example: @@ -768,10 +769,24 @@ module ActiveRecord #:nodoc: # :order => 'created_at', :limit => 5 ) def update_all(updates, conditions = nil, options = {}) sql = "UPDATE #{quoted_table_name} SET #{sanitize_sql_for_assignment(updates)} " + scope = scope(:find) - add_conditions!(sql, conditions, scope) - add_order!(sql, options[:order], nil) - add_limit!(sql, options, nil) + + select_sql = "" + add_conditions!(select_sql, conditions, scope) + + if options.has_key?(:limit) || (scope && scope[:limit]) + # Only take order from scope if limit is also provided by scope, this + # is useful for updating a has_many association with a limit. + add_order!(select_sql, options[:order], scope) + + add_limit!(select_sql, options, scope) + sql.concat(connection.limited_update_conditions(select_sql, quoted_table_name, connection.quote_column_name(primary_key))) + else + add_order!(select_sql, options[:order], nil) + sql.concat(select_sql) + end + connection.update(sql, "#{name} Update") end diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index a675af4787..80992dd34b 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -217,7 +217,7 @@ module ActiveRecord sql << " ORDER BY #{options[:order]} " if options[:order] add_limit!(sql, options, scope) - sql << ') AS #{aggregate_alias}_subquery' if use_workaround + sql << ") AS #{aggregate_alias}_subquery" if use_workaround sql end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index aaf9e2e73f..8fc89de22b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -153,6 +153,10 @@ module ActiveRecord "=" end + def limited_update_conditions(where_sql, quoted_table_name, quoted_primary_key) + "WHERE #{quoted_primary_key} IN (SELECT #{quoted_primary_key} FROM #{quoted_table_name} #{where_sql})" + end + protected # Returns an array of record hashes with the column names as keys and # column values as values. diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index f1d13698c3..c2a0fb72bf 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -523,6 +523,10 @@ module ActiveRecord "= BINARY" end + def limited_update_conditions(where_sql, quoted_table_name, quoted_primary_key) + where_sql + end + private def connect @connection.reconnect = true if @connection.respond_to?(:reconnect=) diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb index b105b919f5..f4a5712981 100644 --- a/activerecord/lib/active_record/dynamic_finder_match.rb +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -8,7 +8,8 @@ module ActiveRecord def initialize(method) @finder = :find_initial case method.to_s - when /^find_(all_by|by)_([_a-zA-Z]\w*)$/ + when /^find_(all_by|last_by|by)_([_a-zA-Z]\w*)$/ + @finder = :find_last if $1 == 'last_by' @finder = :find_every if $1 == 'all_by' names = $2 when /^find_by_([_a-zA-Z]\w*)\!$/ diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 935b1939d8..a1b498eceb 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -129,10 +129,45 @@ module ActiveRecord # Holds all the meta-data about an association as it was specified in the Active Record class. class AssociationReflection < MacroReflection #:nodoc: + # Returns the target association's class: + # + # class Author < ActiveRecord::Base + # has_many :books + # end + # + # Author.reflect_on_association(:books).klass + # # => Book + # + # <b>Note:</b> do not call +klass.new+ or +klass.create+ to instantiate + # a new association object. Use +build_association+ or +create_association+ + # instead. This allows plugins to hook into association object creation. def klass @klass ||= active_record.send(:compute_type, class_name) end + # Returns a new, unsaved instance of the associated class. +options+ will + # be passed to the class's constructor. + def build_association(*options) + klass.new(*options) + end + + # Creates a new instance of the associated class, and immediates saves it + # with ActiveRecord::Base#save. +options+ will be passed to the class's + # creation method. Returns the newly created object. + def create_association(*options) + klass.create(*options) + end + + # Creates a new instance of the associated class, and immediates saves it + # with ActiveRecord::Base#save!. +options+ will be passed to the class's + # creation method. If the created record doesn't pass validations, then an + # exception will be raised. + # + # Returns the newly created object. + def create_association!(*options) + klass.create!(*options) + end + def table_name @table_name ||= klass.table_name end diff --git a/activerecord/test/cases/aggregations_test.rb b/activerecord/test/cases/aggregations_test.rb index 75d1f27e07..b6656c8631 100644 --- a/activerecord/test/cases/aggregations_test.rb +++ b/activerecord/test/cases/aggregations_test.rb @@ -107,6 +107,41 @@ class AggregationsTest < ActiveRecord::TestCase customers(:david).gps_location = nil assert_equal nil, customers(:david).gps_location end + + def test_custom_constructor + assert_equal 'Barney GUMBLE', customers(:barney).fullname.to_s + assert_kind_of Fullname, customers(:barney).fullname + end + + def test_custom_converter + customers(:barney).fullname = 'Barnoit Gumbleau' + assert_equal 'Barnoit GUMBLEAU', customers(:barney).fullname.to_s + assert_kind_of Fullname, customers(:barney).fullname + end +end + +class DeprecatedAggregationsTest < ActiveRecord::TestCase + class Person < ActiveRecord::Base; end + + def test_conversion_block_is_deprecated + assert_deprecated 'conversion block has been deprecated' do + Person.composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) { |balance| balance.to_money } + end + end + + def test_conversion_block_used_when_converter_option_is_nil + Person.composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) { |balance| balance.to_money } + assert_raise(NoMethodError) { Person.new.balance = 5 } + end + + def test_converter_option_overrides_conversion_block + Person.composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| Money.new(balance) }) { |balance| balance.to_money } + + person = Person.new + assert_nothing_raised { person.balance = 5 } + assert_equal 5, person.balance.amount + assert_kind_of Money, person.balance + end end class OverridingAggregationsTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 0b2731ecd7..056a29438a 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -189,114 +189,6 @@ class AssociationProxyTest < ActiveRecord::TestCase end end - def test_belongs_to_mass_assignment - post_attributes = { :title => 'Associations', :body => 'Are They Accessible?' } - author_attributes = { :name => 'David Dollar' } - - assert_no_difference 'Author.count' do - assert_raise(ActiveRecord::AssociationTypeMismatch) do - Post.create(post_attributes.merge({:author => author_attributes})) - end - end - - assert_difference 'Author.count' do - post = Post.create(post_attributes.merge({:creatable_author => author_attributes})) - assert_equal post.creatable_author.name, author_attributes[:name] - end - end - - def test_has_one_mass_assignment - post_attributes = { :title => 'Associations', :body => 'Are They Accessible?' } - comment_attributes = { :body => 'Setter Takes Hash' } - - assert_no_difference 'Comment.count' do - assert_raise(ActiveRecord::AssociationTypeMismatch) do - Post.create(post_attributes.merge({:uncreatable_comment => comment_attributes})) - end - end - - assert_difference 'Comment.count' do - post = Post.create(post_attributes.merge({:creatable_comment => comment_attributes})) - assert_equal post.creatable_comment.body, comment_attributes[:body] - end - end - - def test_has_many_mass_assignment - post = posts(:welcome) - post_attributes = { :title => 'Associations', :body => 'Are They Accessible?' } - comment_attributes = { :body => 'Setter Takes Hash' } - - assert_no_difference 'Comment.count' do - assert_raise(ActiveRecord::AssociationTypeMismatch) do - Post.create(post_attributes.merge({:comments => [comment_attributes]})) - end - assert_raise(ActiveRecord::AssociationTypeMismatch) do - post.comments << comment_attributes - end - end - - assert_difference 'Comment.count' do - post = Post.create(post_attributes.merge({:creatable_comments => [comment_attributes]})) - assert_equal post.creatable_comments.last.body, comment_attributes[:body] - end - - assert_difference 'Comment.count' do - post.creatable_comments << comment_attributes - assert_equal post.comments.last.body, comment_attributes[:body] - end - - post.creatable_comments = [comment_attributes, comment_attributes] - assert_equal post.creatable_comments.count, 2 - end - - def test_has_and_belongs_to_many_mass_assignment - post = posts(:welcome) - post_attributes = { :title => 'Associations', :body => 'Are They Accessible?' } - category_attributes = { :name => 'Accessible Association', :type => 'Category' } - - assert_no_difference 'Category.count' do - assert_raise(ActiveRecord::AssociationTypeMismatch) do - Post.create(post_attributes.merge({:categories => [category_attributes]})) - end - assert_raise(ActiveRecord::AssociationTypeMismatch) do - post.categories << category_attributes - end - end - - assert_difference 'Category.count' do - post = Post.create(post_attributes.merge({:creatable_categories => [category_attributes]})) - assert_equal post.creatable_categories.last.name, category_attributes[:name] - end - - assert_difference 'Category.count' do - post.creatable_categories << category_attributes - assert_equal post.creatable_categories.last.name, category_attributes[:name] - end - - post.creatable_categories = [category_attributes, category_attributes] - assert_equal post.creatable_categories.count, 2 - end - - def test_association_proxy_setter_can_take_hash - special_comment_attributes = { :body => 'Setter Takes Hash' } - - post = posts(:welcome) - post.creatable_comment = { :body => 'Setter Takes Hash' } - - assert_equal post.creatable_comment.body, special_comment_attributes[:body] - end - - def test_association_collection_can_take_hash - post_attributes = { :title => 'Setter Takes', :body => 'Hash' } - david = authors(:david) - - post = (david.posts << post_attributes).last - assert_equal post.title, post_attributes[:title] - - david.posts = [post_attributes, post_attributes] - assert_equal david.posts.count, 2 - end - def setup_dangling_association josh = Author.create(:name => "Josh") p = Post.create(:title => "New on Edge", :body => "More cool stuff!", :author => josh) diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 67358fedd6..bda6f346f0 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -76,7 +76,7 @@ class TopicWithProtectedContentAndAccessibleAuthorName < ActiveRecord::Base end class BasicsTest < ActiveRecord::TestCase - fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations, :categories + fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations, :categories, :posts def test_table_exists assert !NonExistentTable.table_exists? @@ -664,10 +664,21 @@ class BasicsTest < ActiveRecord::TestCase end end - def test_update_all_ignores_order_limit_from_association - author = Author.find(1) + def test_update_all_ignores_order_without_limit_from_association + author = authors(:david) assert_nothing_raised do - assert_equal author.posts_with_comments_and_categories.length, author.posts_with_comments_and_categories.update_all("body = 'bulk update!'") + assert_equal author.posts_with_comments_and_categories.length, author.posts_with_comments_and_categories.update_all([ "body = ?", "bulk update!" ]) + end + end + + def test_update_all_with_order_and_limit_updates_subset_only + author = authors(:david) + assert_nothing_raised do + assert_equal 1, author.posts_sorted_by_id_limited.size + assert_equal 2, author.posts_sorted_by_id_limited.find(:all, :limit => 2).size + assert_equal 1, author.posts_sorted_by_id_limited.update_all([ "body = ?", "bulk update!" ]) + assert_equal "bulk update!", posts(:welcome).body + assert_not_equal "bulk update!", posts(:thinking).body end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 2ce49ed76f..1cbc5182d6 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -197,11 +197,11 @@ class FinderTest < ActiveRecord::TestCase first = Topic.find(:first, :conditions => "title = 'The First Topic!'") assert_nil(first) end - + def test_first assert_equal topics(:second).title, Topic.first(:conditions => "title = 'The Second Topic of the day'").title end - + def test_first_failing assert_nil Topic.first(:conditions => "title = 'The Second Topic of the day!'") end @@ -418,7 +418,7 @@ class FinderTest < ActiveRecord::TestCase def test_named_bind_variables assert_equal '1', bind(':a', :a => 1) # ' ruby-mode assert_equal '1 1', bind(':a :a', :a => 1) # ' ruby-mode - + assert_nothing_raised { bind("'+00:00'", :foo => "bar") } assert_kind_of Firm, Company.find(:first, :conditions => ["name = :name", { :name => "37signals" }]) @@ -589,6 +589,38 @@ class FinderTest < ActiveRecord::TestCase assert_nil Topic.find_by_title_and_author_name("The First Topic", "Mary") end + def test_find_last_by_one_attribute + assert_equal Topic.last, Topic.find_last_by_title(Topic.last.title) + assert_nil Topic.find_last_by_title("A title with no matches") + end + + def test_find_last_by_one_attribute_caches_dynamic_finder + # ensure this test can run independently of order + class << Topic; self; end.send(:remove_method, :find_last_by_title) if Topic.public_methods.any? { |m| m.to_s == 'find_last_by_title' } + assert !Topic.public_methods.any? { |m| m.to_s == 'find_last_by_title' } + t = Topic.find_last_by_title(Topic.last.title) + assert Topic.public_methods.any? { |m| m.to_s == 'find_last_by_title' } + end + + def test_find_last_by_invalid_method_syntax + assert_raises(NoMethodError) { Topic.fail_to_find_last_by_title("The First Topic") } + assert_raises(NoMethodError) { Topic.find_last_by_title?("The First Topic") } + end + + def test_find_last_by_one_attribute_with_several_options + assert_equal accounts(:signals37), Account.find_last_by_credit_limit(50, :order => 'id DESC', :conditions => ['id != ?', 3]) + end + + def test_find_last_by_one_missing_attribute + assert_raises(NoMethodError) { Topic.find_last_by_undertitle("The Last Topic!") } + end + + def test_find_last_by_two_attributes + topic = Topic.last + assert_equal topic, Topic.find_last_by_title_and_author_name(topic.title, topic.author_name) + assert_nil Topic.find_last_by_title_and_author_name(topic.title, "Anonymous") + end + def test_find_all_by_one_attribute topics = Topic.find_all_by_content("Have a nice day") assert_equal 2, topics.size @@ -782,7 +814,7 @@ class FinderTest < ActiveRecord::TestCase assert c.valid? assert !c.new_record? end - + def test_dynamic_find_or_initialize_from_one_attribute_caches_method class << Company; self; end.send(:remove_method, :find_or_initialize_by_name) if Company.public_methods.any? { |m| m.to_s == 'find_or_initialize_by_name' } assert !Company.public_methods.any? { |m| m.to_s == 'find_or_initialize_by_name' } diff --git a/activerecord/test/fixtures/customers.yml b/activerecord/test/fixtures/customers.yml index f802aac5ea..0399ff83b9 100644 --- a/activerecord/test/fixtures/customers.yml +++ b/activerecord/test/fixtures/customers.yml @@ -6,7 +6,7 @@ david: address_city: Scary Town address_country: Loony Land gps_location: 35.544623640962634x-105.9309951055148 - + zaphod: id: 2 name: Zaphod @@ -14,4 +14,13 @@ zaphod: address_street: Avenue Road address_city: Hamlet Town address_country: Nation Land + gps_location: NULL + +barney: + id: 3 + name: Barney Gumble + balance: 1 + address_street: Quiet Road + address_city: Peaceful Town + address_country: Tranquil Land gps_location: NULL
\ No newline at end of file diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index c6aa0293c2..37551c8157 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -1,7 +1,8 @@ class Author < ActiveRecord::Base - has_many :posts, :accessible => true + has_many :posts has_many :posts_with_comments, :include => :comments, :class_name => "Post" has_many :posts_with_comments_sorted_by_comment_id, :include => :comments, :class_name => "Post", :order => 'comments.id' + has_many :posts_sorted_by_id_limited, :class_name => "Post", :order => 'posts.id', :limit => 1 has_many :posts_with_categories, :include => :categories, :class_name => "Post" has_many :posts_with_comments_and_categories, :include => [ :comments, :categories ], :order => "posts.id", :class_name => "Post" has_many :posts_containing_the_letter_a, :class_name => "Post" diff --git a/activerecord/test/models/customer.rb b/activerecord/test/models/customer.rb index 030bbc6237..e258ccdb6c 100644 --- a/activerecord/test/models/customer.rb +++ b/activerecord/test/models/customer.rb @@ -1,7 +1,8 @@ class Customer < ActiveRecord::Base composed_of :address, :mapping => [ %w(address_street street), %w(address_city city), %w(address_country country) ], :allow_nil => true - composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) { |balance| balance.to_money } + composed_of :balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| balance.to_money } composed_of :gps_location, :allow_nil => true + composed_of :fullname, :mapping => %w(name to_s), :constructor => Proc.new { |name| Fullname.parse(name) }, :converter => :parse end class Address @@ -53,3 +54,20 @@ class GpsLocation self.latitude == other.latitude && self.longitude == other.longitude end end + +class Fullname + attr_reader :first, :last + + def self.parse(str) + return nil unless str + new(*str.to_s.split) + end + + def initialize(first, last = nil) + @first, @last = first, last + end + + def to_s + "#{first} #{last.upcase}" + end +end
\ No newline at end of file diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index e23818eb33..3adbc0ce1f 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -33,12 +33,6 @@ class Post < ActiveRecord::Base has_and_belongs_to_many :categories has_and_belongs_to_many :special_categories, :join_table => "categories_posts", :association_foreign_key => 'category_id' - belongs_to :creatable_author, :class_name => 'Author', :accessible => true - has_one :uncreatable_comment, :class_name => 'Comment', :accessible => false, :order => 'id desc' - has_one :creatable_comment, :class_name => 'Comment', :accessible => true, :order => 'id desc' - has_many :creatable_comments, :class_name => 'Comment', :accessible => true, :dependent => :destroy - has_and_belongs_to_many :creatable_categories, :class_name => 'Category', :accessible => true - has_many :taggings, :as => :taggable has_many :tags, :through => :taggings do def add_joins_and_select diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 604462c706..b38a5061d5 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Added Inflector#parameterize for easy slug generation ("Donald E. Knuth".parameterize => "donald-e-knuth") #713 [Matt Darby] + * Changed cache benchmarking to be reported in milliseconds [DHH] * Fix Ruby's Time marshaling bug in pre-1.9 versions of Ruby: utc instances are now correctly unmarshaled with a utc zone instead of the system local zone [#900 state:resolved] [Luca Guidi, Geoff Buesing] diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 659bde64f0..ef533633b2 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -65,6 +65,6 @@ module ActiveSupport end end end - end + end end end diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 7b905930bb..5cdcaf5ad1 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -188,7 +188,7 @@ module ActiveSupport "Callbacks must be a symbol denoting the method to call, a string to be evaluated, " + "a block to be invoked, or an object responding to the callback method." end - end + end end def should_run_callback?(*args) diff --git a/activesupport/lib/active_support/core_ext/bigdecimal/conversions.rb b/activesupport/lib/active_support/core_ext/bigdecimal/conversions.rb index 94c7c779f7..bc9d578f38 100644 --- a/activesupport/lib/active_support/core_ext/bigdecimal/conversions.rb +++ b/activesupport/lib/active_support/core_ext/bigdecimal/conversions.rb @@ -4,35 +4,31 @@ module ActiveSupport #:nodoc: module CoreExtensions #:nodoc: module BigDecimal #:nodoc: module Conversions + DEFAULT_STRING_FORMAT = 'F'.freeze + YAML_TAG = 'tag:yaml.org,2002:float'.freeze + YAML_MAPPING = { 'Infinity' => '.Inf', '-Infinity' => '-.Inf', 'NaN' => '.NaN' } + def self.included(base) #:nodoc: - base.instance_eval do + base.class_eval do alias_method :_original_to_s, :to_s alias_method :to_s, :to_formatted_s + + yaml_as YAML_TAG end end - - def to_formatted_s(format="F") + + def to_formatted_s(format = DEFAULT_STRING_FORMAT) _original_to_s(format) end - - yaml_as "tag:yaml.org,2002:float" - def to_yaml( opts = {} ) - YAML::quick_emit( nil, opts ) do |out| - # This emits the number without any scientific notation. - # I prefer it to using self.to_f.to_s, which would lose precision. - # - # Note that YAML allows that when reconstituting floats - # to native types, some precision may get lost. - # There is no full precision real YAML tag that I am aware of. - str = self.to_s - if str == "Infinity" - str = ".Inf" - elsif str == "-Infinity" - str = "-.Inf" - elsif str == "NaN" - str = ".NaN" - end - out.scalar( "tag:yaml.org,2002:float", str, :plain ) + + # This emits the number without any scientific notation. + # This is better than self.to_f.to_s since it doesn't lose precision. + # + # Note that reconstituting YAML floats to native floats may lose precision. + def to_yaml(opts = {}) + YAML.quick_emit(nil, opts) do |out| + string = to_s + out.scalar(YAML_TAG, YAML_MAPPING[string] || string, :plain) end end end diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index fd94bc051f..788f3a7e9e 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -77,9 +77,9 @@ module Enumerable # (1..5).each_with_object(1) { |value, memo| memo *= value } # => 1 # def each_with_object(memo, &block) - returning memo do |memo| + returning memo do |m| each do |element| - block.call(element, memo) + block.call(element, m) end end end unless [].respond_to?(:each_with_object) diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index 2c606b401b..50dc7c61fc 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -156,7 +156,7 @@ module ActiveSupport #:nodoc: XML_FORMATTING[type_name] ? XML_FORMATTING[type_name].call(value) : value, attributes ) - end + end end end diff --git a/activesupport/lib/active_support/core_ext/module/synchronization.rb b/activesupport/lib/active_support/core_ext/module/synchronization.rb index 6253594dfa..251606024e 100644 --- a/activesupport/lib/active_support/core_ext/module/synchronization.rb +++ b/activesupport/lib/active_support/core_ext/module/synchronization.rb @@ -18,11 +18,13 @@ class Module raise ArgumentError, "Synchronization needs a mutex. Supply an options hash with a :with key as the last argument (e.g. synchronize :hello, :with => :@mutex)." end - methods.flatten.each do |method| + methods.each do |method| aliased_method, punctuation = method.to_s.sub(/([?!=])$/, ''), $1 - if instance_methods.include?("#{aliased_method}_without_synchronization#{punctuation}") + + if method_defined?("#{aliased_method}_without_synchronization#{punctuation}") raise ArgumentError, "#{method} is already synchronized. Double synchronization is not currently supported." end + module_eval(<<-EOS, __FILE__, __LINE__) def #{aliased_method}_with_synchronization#{punctuation}(*args, &block) #{with}.synchronize do @@ -30,7 +32,8 @@ class Module end end EOS + alias_method_chain method, :synchronization end end -end
\ No newline at end of file +end diff --git a/activesupport/lib/active_support/core_ext/rexml.rb b/activesupport/lib/active_support/core_ext/rexml.rb index af8ce3af47..187f3e0f5e 100644 --- a/activesupport/lib/active_support/core_ext/rexml.rb +++ b/activesupport/lib/active_support/core_ext/rexml.rb @@ -5,7 +5,8 @@ require 'rexml/entity' # http://www.ruby-lang.org/en/news/2008/08/23/dos-vulnerability-in-rexml/ # This fix is identical to rexml-expansion-fix version 1.0.1 -unless REXML::VERSION > "3.1.7.2" +# Earlier versions of rexml defined REXML::Version, newer ones REXML::VERSION +unless (defined?(REXML::VERSION) ? REXML::VERSION : REXML::Version) > "3.1.7.2" module REXML class Entity < Child undef_method :unnormalized diff --git a/activesupport/lib/active_support/core_ext/string/inflections.rb b/activesupport/lib/active_support/core_ext/string/inflections.rb index 3bbad7dad8..de99fe5791 100644 --- a/activesupport/lib/active_support/core_ext/string/inflections.rb +++ b/activesupport/lib/active_support/core_ext/string/inflections.rb @@ -87,6 +87,25 @@ module ActiveSupport #:nodoc: Inflector.demodulize(self) end + # Replaces special characters in a string so that it may be used as part of a 'pretty' URL. + # + # ==== Examples + # + # class Person + # def to_param + # "#{id}-#{name.parameterize}" + # end + # end + # + # @person = Person.find(1) + # # => #<Person id: 1, name: "Donald E. Knuth"> + # + # <%= link_to(@person.name, person_path %> + # # => <a href="/person/1-donald-e-knuth">Donald E. Knuth</a> + def parameterize + Inflector.parameterize(self) + end + # Creates the name of a table like Rails does for models to table names. This method # uses the +pluralize+ method on the last word in the string. # diff --git a/activesupport/lib/active_support/inflector.rb b/activesupport/lib/active_support/inflector.rb index 7ae9e0c6ab..e5f0063285 100644 --- a/activesupport/lib/active_support/inflector.rb +++ b/activesupport/lib/active_support/inflector.rb @@ -240,6 +240,25 @@ module ActiveSupport def demodulize(class_name_in_module) class_name_in_module.to_s.gsub(/^.*::/, '') end + + # Replaces special characters in a string so that it may be used as part of a 'pretty' URL. + # + # ==== Examples + # + # class Person + # def to_param + # "#{id}-#{name.parameterize}" + # end + # end + # + # @person = Person.find(1) + # # => #<Person id: 1, name: "Donald E. Knuth"> + # + # <%= link_to(@person.name, person_path %> + # # => <a href="/person/1-donald-e-knuth">Donald E. Knuth</a> + def parameterize(string, sep = '-') + string.gsub(/[^a-z0-9]+/i, sep).downcase + end # Create the name of a table like Rails does for models to table names. This method # uses the +pluralize+ method on the last word in the string. diff --git a/activesupport/lib/active_support/memoizable.rb b/activesupport/lib/active_support/memoizable.rb index 6506238ac0..4786fd6e0b 100644 --- a/activesupport/lib/active_support/memoizable.rb +++ b/activesupport/lib/active_support/memoizable.rb @@ -60,7 +60,7 @@ module ActiveSupport #{memoized_ivar} ||= {} unless frozen? reload = args.pop if args.last == true || args.last == :reload - if #{memoized_ivar} + if defined?(#{memoized_ivar}) && #{memoized_ivar} if !reload && #{memoized_ivar}.has_key?(args) #{memoized_ivar}[args] elsif #{memoized_ivar} diff --git a/activesupport/test/inflector_test.rb b/activesupport/test/inflector_test.rb index 8eebe1be25..f304844e82 100644 --- a/activesupport/test/inflector_test.rb +++ b/activesupport/test/inflector_test.rb @@ -98,6 +98,12 @@ class InflectorTest < Test::Unit::TestCase end end + def test_parameterize + StringToParameterized.each do |some_string, parameterized_string| + assert_equal(parameterized_string, ActiveSupport::Inflector.parameterize(some_string)) + end + end + def test_classify ClassNameToTableName.each do |class_name, table_name| assert_equal(class_name, ActiveSupport::Inflector.classify(table_name)) diff --git a/activesupport/test/inflector_test_cases.rb b/activesupport/test/inflector_test_cases.rb index a9dd83a389..d399c90385 100644 --- a/activesupport/test/inflector_test_cases.rb +++ b/activesupport/test/inflector_test_cases.rb @@ -142,6 +142,11 @@ module InflectorTestCases "NodeChild" => "node_children" } + StringToParameterized = { + "Donald E. Knuth" => "donald-e-knuth", + "Random text with *(bad)* characters" => "random-text-with-bad-characters" + } + UnderscoreToHuman = { "employee_salary" => "Employee salary", "employee_id" => "Employee", diff --git a/railties/CHANGELOG b/railties/CHANGELOG index 3a276d5aad..51d66e4c01 100644 --- a/railties/CHANGELOG +++ b/railties/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Fixed that sqlite would report "db/development.sqlite3 already exists" whether true or not on db:create #614 [Antonio Cangiano] + * Added config.threadsafe! to toggle allow concurrency settings and disable the dependency loader [Josh Peek] * Turn cache_classes on by default [Josh Peek] diff --git a/railties/lib/commands/plugin.rb b/railties/lib/commands/plugin.rb index 980244a71b..9ff4739562 100644 --- a/railties/lib/commands/plugin.rb +++ b/railties/lib/commands/plugin.rb @@ -461,11 +461,11 @@ module Commands o.on("-r", "--root=DIR", String, "Set an explicit rails app directory.", - "Default: #{@rails_root}") { |@rails_root| self.environment = RailsEnvironment.new(@rails_root) } + "Default: #{@rails_root}") { |rails_root| @rails_root = rails_root; self.environment = RailsEnvironment.new(@rails_root) } o.on("-s", "--source=URL1,URL2", Array, - "Use the specified plugin repositories instead of the defaults.") { |@sources|} + "Use the specified plugin repositories instead of the defaults.") { |sources| @sources = sources} - o.on("-v", "--verbose", "Turn on verbose output.") { |$verbose| } + o.on("-v", "--verbose", "Turn on verbose output.") { |verbose| $verbose = verbose } o.on("-h", "--help", "Show this help message.") { puts o; exit } o.separator "" @@ -552,12 +552,12 @@ module Commands o.separator "Options:" o.separator "" o.on( "-s", "--source=URL1,URL2", Array, - "Use the specified plugin repositories.") {|@sources|} + "Use the specified plugin repositories.") {|sources| @sources = sources} o.on( "--local", - "List locally installed plugins.") {|@local| @remote = false} + "List locally installed plugins.") {|local| @local, @remote = local, false} o.on( "--remote", "List remotely available plugins. This is the default behavior", - "unless --local is provided.") {|@remote|} + "unless --local is provided.") {|remote| @remote = remote} end end @@ -598,7 +598,7 @@ module Commands o.separator "Options:" o.separator "" o.on( "-c", "--check", - "Report status of repository.") { |@sources|} + "Report status of repository.") { |sources| @sources = sources} end end @@ -689,7 +689,7 @@ module Commands o.separator "Options:" o.separator "" o.on( "-l", "--list", - "List but don't prompt or add discovered repositories.") { |@list| @prompt = !@list } + "List but don't prompt or add discovered repositories.") { |list| @list, @prompt = list, !@list } o.on( "-n", "--no-prompt", "Add all new repositories without prompting.") { |v| @prompt = !v } end @@ -901,7 +901,7 @@ class RecursiveHTTPFetcher def initialize(urls_to_fetch, level = 1, cwd = ".") @level = level @cwd = cwd - @urls_to_fetch = urls_to_fetch.to_a + @urls_to_fetch = RUBY_VERSION >= '1.9' ? urls_to_fetch.lines : urls_to_fetch.to_a @quiet = false end diff --git a/railties/lib/commands/process/spawner.rb b/railties/lib/commands/process/spawner.rb index dc0008698a..8bf47abb75 100644 --- a/railties/lib/commands/process/spawner.rb +++ b/railties/lib/commands/process/spawner.rb @@ -181,10 +181,10 @@ ARGV.options do |opts| opts.on(" Options:") - opts.on("-p", "--port=number", Integer, "Starting port number (default: #{OPTIONS[:port]})") { |OPTIONS[:port]| } + opts.on("-p", "--port=number", Integer, "Starting port number (default: #{OPTIONS[:port]})") { |v| OPTIONS[:port] = v } if spawner_class.can_bind_to_custom_address? - opts.on("-a", "--address=ip", String, "Bind to IP address (default: #{OPTIONS[:address]})") { |OPTIONS[:address]| } + opts.on("-a", "--address=ip", String, "Bind to IP address (default: #{OPTIONS[:address]})") { |v| OPTIONS[:address] = v } end opts.on("-p", "--port=number", Integer, "Starting port number (default: #{OPTIONS[:port]})") { |v| OPTIONS[:port] = v } diff --git a/railties/lib/rails_generator/generators/components/mailer/templates/unit_test.rb b/railties/lib/rails_generator/generators/components/mailer/templates/unit_test.rb index 1b7bcfef08..4de94076e9 100644 --- a/railties/lib/rails_generator/generators/components/mailer/templates/unit_test.rb +++ b/railties/lib/rails_generator/generators/components/mailer/templates/unit_test.rb @@ -1,7 +1,6 @@ require 'test_helper' class <%= class_name %>Test < ActionMailer::TestCase - tests <%= class_name %> <% for action in actions -%> test "<%= action %>" do @expected.subject = '<%= class_name %>#<%= action %>' diff --git a/railties/lib/tasks/databases.rake b/railties/lib/tasks/databases.rake index 21c81b3fb5..cc079b1d93 100644 --- a/railties/lib/tasks/databases.rake +++ b/railties/lib/tasks/databases.rake @@ -28,8 +28,24 @@ namespace :db do def create_database(config) begin - ActiveRecord::Base.establish_connection(config) - ActiveRecord::Base.connection + if config['adapter'] =~ /sqlite/ + if File.exist?(config['database']) + $stderr.puts "#{config['database']} already exists" + else + begin + # Create the SQLite database + ActiveRecord::Base.establish_connection(config) + ActiveRecord::Base.connection + rescue + $stderr.puts $!, *($!.backtrace) + $stderr.puts "Couldn't create database for #{config.inspect}" + end + end + return # Skip the else clause of begin/rescue + else + ActiveRecord::Base.establish_connection(config) + ActiveRecord::Base.connection + end rescue case config['adapter'] when 'mysql' @@ -52,10 +68,6 @@ namespace :db do $stderr.puts $!, *($!.backtrace) $stderr.puts "Couldn't create database for #{config.inspect}" end - when 'sqlite' - `sqlite "#{config['database']}"` - when 'sqlite3' - `sqlite3 "#{config['database']}"` end else $stderr.puts "#{config['database']} already exists" |