diff options
76 files changed, 520 insertions, 252 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 5f0f1b78d1..b4d3da3603 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,4 +1,10 @@ -* Add Mime::Type.register "text/vcard", :vcf to the default list of mime types +* Fix generating a path for engine inside a resources block. + + Fixes #8533. + + *Piotr Sarnacki* + +* Add `Mime::Type.register "text/vcard", :vcf` to the default list of mime types. *DHH* @@ -22,7 +28,7 @@ The request variant is a specialization of the request format, like `:tablet`, `:phone`, or `:desktop`. - You can set the variant in a before_action: + You can set the variant in a `before_action`: request.variant = :tablet if request.user_agent =~ /iPad/ @@ -41,6 +47,14 @@ app/views/projects/show.html+tablet.erb app/views/projects/show.html+phone.erb + You can also simplify the variants definition using the inline syntax: + + respond_to do |format| + format.js { render "trash" } + format.html.phone { redirect_to progress_path } + format.html.none { render "trash" } + end + *Łukasz Strzałkowski* * Fix header `Content-Type: #<Mime::NullType:...>` in localized template. @@ -81,9 +95,7 @@ * Add `session#fetch` method - fetch behaves similarly to [Hash#fetch](http://www.ruby-doc.org/core-1.9.3/Hash.html#method-i-fetch), - with the exception that the returned value is always saved into the session. - + fetch behaves like [Hash#fetch](http://www.ruby-doc.org/core-1.9.3/Hash.html#method-i-fetch). It returns a value from the hash for the given key. If the key can’t be found, there are several options: diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index d3aa8f90c5..b0e164bc57 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -67,7 +67,7 @@ module ActionController private - # A hook invoked everytime a before callback is halted. + # A hook invoked every time a before callback is halted. def halted_callback_hook(filter) ActiveSupport::Notifications.instrument("halted_callback.action_controller", :filter => filter) end diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 0dd788645b..b7d1334dd3 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -48,7 +48,7 @@ module ActionController # the server will receive a +Last-Event-ID+ header with value equal to +id+. # # After setting an option in the constructor of the SSE object, all future - # SSEs sent accross the stream will use those options unless overridden. + # SSEs sent across the stream will use those options unless overridden. # # Example Usage: # diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index b47abb8b8c..60fababd83 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -209,6 +209,15 @@ module ActionController #:nodoc: # app/views/projects/show.html+tablet.erb # app/views/projects/show.html+phone.erb # + # When you're not sharing any code within the format, you can simplify defining variants + # using the inline syntax: + # + # respond_to do |format| + # format.js { render "trash" } + # format.html.phone { redirect_to progress_path } + # format.html.none { render "trash" } + # end + # # Be sure to check the documentation of +respond_with+ and # <tt>ActionController::MimeResponds.respond_to</tt> for more examples. def respond_to(*mimes, &block) @@ -430,7 +439,8 @@ module ActionController #:nodoc: def initialize(mimes) @responses = {} - mimes.each { |mime| send(mime) } + + mimes.each { |mime| @responses["Mime::#{mime.upcase}".constantize] = nil } end def any(*args, &block) @@ -444,12 +454,18 @@ module ActionController #:nodoc: def custom(mime_type, &block) mime_type = Mime::Type.lookup(mime_type.to_s) unless mime_type.is_a?(Mime::Type) - @responses[mime_type] ||= block + @responses[mime_type] ||= if block_given? + block + else + VariantCollector.new + end end def response(variant) response = @responses.fetch(format, @responses[Mime::ALL]) - if response.nil? || response.arity == 0 + if response.is_a?(VariantCollector) + response.variant(variant) + elsif response.nil? || response.arity == 0 response else lambda { response.call VariantFilter.new(variant) } @@ -460,6 +476,22 @@ module ActionController #:nodoc: @format = request.negotiate_mime(@responses.keys) end + #Used for inline syntax + class VariantCollector #:nodoc: + def initialize + @variants = {} + end + + def method_missing(name, *args, &block) + @variants[name] = block if block_given? + end + + def variant(name) + @variants[name.nil? ? :none : name] + end + end + + #Used for nested block syntax class VariantFilter #:nodoc: def initialize(variant) @variant = variant diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index b4ba169e8f..e24b56fa91 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -270,7 +270,7 @@ module ActionController #:nodoc: resource.respond_to?(:errors) && !resource.errors.empty? end - # Check whether the neceessary Renderer is available + # Check whether the necessary Renderer is available def has_renderer? Renderers::RENDERERS.include?(format) end diff --git a/actionpack/lib/action_dispatch/journey/parser.rb b/actionpack/lib/action_dispatch/journey/parser.rb index bb4cbb00e2..430812fafe 100644 --- a/actionpack/lib/action_dispatch/journey/parser.rb +++ b/actionpack/lib/action_dispatch/journey/parser.rb @@ -1,7 +1,7 @@ # # DO NOT MODIFY!!!! # This file is automatically generated by Racc 1.4.9 -# from Racc grammer file "". +# from Racc grammar file "". # require 'racc/parser.rb' diff --git a/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb index 95461fa693..323873ba4b 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb @@ -89,8 +89,8 @@ } // takes an array of elements with a data-regexp attribute and - // passes their their parent <tr> into the callback function - // if the regexp matchs a given path + // passes their parent <tr> into the callback function + // if the regexp matches a given path function eachElemsForPath(elems, path, func) { each(elems, function(e){ var reg = e.getAttribute("data-regexp"); diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index 6d911a75f1..973627f106 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -7,6 +7,9 @@ module ActionDispatch ENV_SESSION_KEY = Rack::Session::Abstract::ENV_SESSION_KEY # :nodoc: ENV_SESSION_OPTIONS_KEY = Rack::Session::Abstract::ENV_SESSION_OPTIONS_KEY # :nodoc: + # Singleton object used to determine if an optional param wasn't specified + Unspecified = Object.new + def self.create(store, env, default_options) session_was = find env session = Request::Session.new(store, env) @@ -127,15 +130,12 @@ module ActionDispatch @delegate.delete key.to_s end - def fetch(key, default=nil) - if self.key?(key) - self[key] - elsif default - self[key] = default - elsif block_given? - self[key] = yield(key) + def fetch(key, default=Unspecified, &block) + load_for_read! + if default == Unspecified + @delegate.fetch(key.to_s, &block) else - raise KeyError + @delegate.fetch(key.to_s, default, &block) end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 846a6345cb..bfba8d143d 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -502,11 +502,12 @@ module ActionDispatch raise "A rack application must be specified" unless path options[:as] ||= app_name(app) + target_as = name_for_action(options[:as], path) options[:via] ||= :all match(path, options.merge(:to => app, :anchor => false, :format => false)) - define_generate_prefix(app, options[:as]) + define_generate_prefix(app, target_as) self end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index b8abdabca5..04faabef37 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -361,7 +361,7 @@ module ActionDispatch include UrlFor end - # Contains all the mounted helpers accross different + # Contains all the mounted helpers across different # engines and the `main_app` helper for the application. # You can include this in your classes if you want to # access routes for other engines. diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index c258bbec06..d84eb5790d 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -175,6 +175,22 @@ class RespondToController < ActionController::Base end end + def variant_inline_syntax + respond_to do |format| + format.js { render text: "js" } + format.html.none { render text: "none" } + format.html.phone { render text: "phone" } + end + end + + def variant_inline_syntax_without_block + respond_to do |format| + format.js + format.html.none + format.html.phone + end + end + protected def set_layout case action_name @@ -554,10 +570,31 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "tablet", @response.body end - def test_no_variant_in_variant_setup get :variant_plus_none_for_format assert_equal "text/html", @response.content_type assert_equal "none", @response.body end + + def test_variant_inline_syntax + get :variant_inline_syntax, format: :js + assert_equal "text/javascript", @response.content_type + assert_equal "js", @response.body + + get :variant_inline_syntax + assert_equal "text/html", @response.content_type + assert_equal "none", @response.body + + @request.variant = :phone + get :variant_inline_syntax + assert_equal "text/html", @response.content_type + assert_equal "phone", @response.body + end + + def test_variant_inline_syntax_without_block + @request.variant = :phone + get :variant_inline_syntax_without_block + assert_equal "text/html", @response.content_type + assert_equal "phone", @response.body + end end diff --git a/actionpack/test/dispatch/mount_test.rb b/actionpack/test/dispatch/mount_test.rb index 30e95a0b75..683a4f01e2 100644 --- a/actionpack/test/dispatch/mount_test.rb +++ b/actionpack/test/dispatch/mount_test.rb @@ -5,7 +5,7 @@ class TestRoutingMount < ActionDispatch::IntegrationTest class FakeEngine def self.routes - Object.new + @routes ||= ActionDispatch::Routing::RouteSet.new end def self.call(env) @@ -27,12 +27,23 @@ class TestRoutingMount < ActionDispatch::IntegrationTest scope "/its_a" do mount SprocketsApp, :at => "/sprocket" end + + resources :users do + mount FakeEngine, :at => "/fakeengine", :as => :fake_mounted_at_resource + end end def app Router end + def test_app_name_is_properly_generated_when_engine_is_mounted_in_resources + assert Router.mounted_helpers.method_defined?(:user_fake_mounted_at_resource), + "A mounted helper should be defined with a parent's prefix" + assert Router.named_routes.routes[:user_fake_mounted_at_resource], + "A named route should be defined with a parent's prefix" + end + def test_trailing_slash_is_not_removed_from_path_info get "/sprockets/omg/" assert_equal "/sprockets -- /omg/", response.body diff --git a/actionpack/test/dispatch/request/session_test.rb b/actionpack/test/dispatch/request/session_test.rb index a244d1364c..df55fcc8bc 100644 --- a/actionpack/test/dispatch/request/session_test.rb +++ b/actionpack/test/dispatch/request/session_test.rb @@ -68,13 +68,12 @@ module ActionDispatch assert_equal '1', session.fetch(:one) assert_equal '2', session.fetch(:two, '2') - assert_equal '2', session.fetch(:two) + assert_nil session.fetch(:two, nil) assert_equal 'three', session.fetch(:three) {|el| el.to_s } - assert_equal 'three', session.fetch(:three) assert_raise KeyError do - session.fetch(:four) + session.fetch(:three) end end diff --git a/actionpack/test/fixtures/respond_to/variant_inline_syntax_without_block.html+phone.erb b/actionpack/test/fixtures/respond_to/variant_inline_syntax_without_block.html+phone.erb new file mode 100644 index 0000000000..cd222a4a49 --- /dev/null +++ b/actionpack/test/fixtures/respond_to/variant_inline_syntax_without_block.html+phone.erb @@ -0,0 +1 @@ +phone
\ No newline at end of file diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index 142c27ace0..7ceb56625f 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -465,7 +465,7 @@ module ActionView # # <strong>Ask me!</strong> # # </button> # - # button_tag "Checkout", data: { disable_with => "Please wait..." } + # button_tag "Checkout", data: { :disable_with => "Please wait..." } # # => <button data-disable-with="Please wait..." name="button" type="submit">Checkout</button> # def button_tag(content_or_options = nil, options = nil, &block) diff --git a/actionview/lib/action_view/renderer/streaming_template_renderer.rb b/actionview/lib/action_view/renderer/streaming_template_renderer.rb index 9cf6eb0c65..3ab2cd36fc 100644 --- a/actionview/lib/action_view/renderer/streaming_template_renderer.rb +++ b/actionview/lib/action_view/renderer/streaming_template_renderer.rb @@ -58,7 +58,7 @@ module ActionView def delayed_render(buffer, template, layout, view, locals) # Wrap the given buffer in the StreamingBuffer and pass it to the - # underlying template handler. Now, everytime something is concatenated + # underlying template handler. Now, every time something is concatenated # to the buffer, it is not appended to an array, but streamed straight # to the client. output = ActionView::StreamingBuffer.new(buffer) diff --git a/actionview/test/template/form_collections_helper_test.rb b/actionview/test/template/form_collections_helper_test.rb index 68c83f2059..7a62b9d907 100644 --- a/actionview/test/template/form_collections_helper_test.rb +++ b/actionview/test/template/form_collections_helper_test.rb @@ -60,7 +60,7 @@ class FormCollectionsHelperTest < ActionView::TestCase assert_no_select 'input[type=radio][value=other][disabled=disabled]' end - test 'collection radio accepts single disable item' do + test 'collection radio accepts single disabled item' do collection = [[1, true], [0, false]] with_collection_radio_buttons :user, :active, collection, :last, :first, :disabled => true @@ -300,7 +300,7 @@ class FormCollectionsHelperTest < ActionView::TestCase assert_no_select 'input[type=checkbox][value=2][disabled=disabled]' end - test 'collection check boxes accepts single disable item' do + test 'collection check boxes accepts single disabled item' do collection = (1..3).map{|i| [i, "Category #{i}"] } with_collection_check_boxes :user, :category_ids, collection, :first, :last, :disabled => 1 diff --git a/activemodel/lib/active_model/conversion.rb b/activemodel/lib/active_model/conversion.rb index 21e4eb3c86..1856c852a2 100644 --- a/activemodel/lib/active_model/conversion.rb +++ b/activemodel/lib/active_model/conversion.rb @@ -41,7 +41,7 @@ module ActiveModel end # Returns an Enumerable of all key attributes if any is set, regardless if - # the object is persisted or not. If there no key attributes, returns +nil+. + # the object is persisted or not. Returns +nil+ if there are no key attributes. # # class Person < ActiveRecord::Base # end diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index c5f1b3f11a..8ccb25c613 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -149,12 +149,12 @@ module ActiveModel end # Handle <tt>*_changed?</tt> for +method_missing+. - def attribute_changed?(attr) + def attribute_changed?(attr) # :nodoc: changed_attributes.include?(attr) end # Handle <tt>*_was</tt> for +method_missing+. - def attribute_was(attr) + def attribute_was(attr) # :nodoc: attribute_changed?(attr) ? changed_attributes[attr] : __send__(attr) end diff --git a/activemodel/lib/active_model/validator.rb b/activemodel/lib/active_model/validator.rb index 690856aee1..aad5ff480b 100644 --- a/activemodel/lib/active_model/validator.rb +++ b/activemodel/lib/active_model/validator.rb @@ -61,7 +61,7 @@ module ActiveModel # end # # Note that the validator is initialized only once for the whole application - # lifecycle, and not on each validation run. + # life cycle, and not on each validation run. # # The easiest way to add custom validators for validating individual attributes # is with the convenient <tt>ActiveModel::EachValidator</tt>. diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb index 41d0b2263e..0314803af6 100644 --- a/activemodel/test/cases/secure_password_test.rb +++ b/activemodel/test/cases/secure_password_test.rb @@ -2,7 +2,6 @@ require 'cases/helper' require 'models/user' require 'models/oauthed_user' require 'models/visitor' -require 'models/administrator' class SecurePasswordTest < ActiveModel::TestCase setup do diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4222308f8b..26f5aa2c73 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,19 @@ +* Use the right column to type cast grouped calculations with custom expressions. + + Fixes #13230. + + Example: + + # Before + Account.group(:firm_name).sum('0.01 * credit_limit') + # => { '37signals' => '0.5' } + + # After + Account.group(:firm_name).sum('0.01 * credit_limit') + # => { '37signals' => 0.5 } + + *Paul Nikitochkin* + * Polymorphic `belongs_to` associations with the `touch: true` option set update the timestamps of the old and new owner correctly when moved between owners of different types. diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index ce7d41cf54..67ea489b22 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -104,11 +104,12 @@ module ActiveRecord # Set the inverse association, if possible def set_inverse_instance(record) - if record && invertible_for?(record) + if invertible_for?(record) inverse = record.association(inverse_reflection_for(record).name) inverse.target = owner inverse.inversed = true end + record end # Returns the class of the target. belongs_to polymorphic overrides this to look at the diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index e1fa5225b5..7dc817fc66 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -8,13 +8,16 @@ module ActiveRecord end def replace(record) - raise_on_type_mismatch!(record) if record - - update_counters(record) - replace_keys(record) - set_inverse_instance(record) - - @updated = true if record + if record + raise_on_type_mismatch!(record) + update_counters(record) + replace_keys(record) + set_inverse_instance(record) + @updated = true + else + update_counters(record) + remove_keys + end self.target = record end @@ -58,11 +61,11 @@ module ActiveRecord end def replace_keys(record) - if record - owner[reflection.foreign_key] = record[reflection.association_primary_key(record.class)] - else - owner[reflection.foreign_key] = nil - end + owner[reflection.foreign_key] = record[reflection.association_primary_key(record.class)] + end + + def remove_keys + owner[reflection.foreign_key] = nil end def foreign_key_present? diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index eae5eed3a1..81d4abfa68 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -11,7 +11,7 @@ module ActiveRecord def replace_keys(record) super - owner[reflection.foreign_type] = record && record.class.base_class.name + owner[reflection.foreign_type] = record.class.base_class.name end def different_target?(record) diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 37ba1c73b1..3911d1b520 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/module/attribute_accessors' + # This is the parent Association class which defines the variables # used by all associations. # @@ -13,65 +15,67 @@ module ActiveRecord::Associations::Builder class Association #:nodoc: class << self attr_accessor :extensions + # TODO: This class accessor is needed to make activerecord-deprecated_finders work. + # We can move it to a constant in 5.0. + attr_accessor :valid_options end self.extensions = [] - VALID_OPTIONS = [:class_name, :class, :foreign_key, :validate] + self.valid_options = [:class_name, :class, :foreign_key, :validate] + + attr_reader :name, :scope, :options def self.build(model, name, scope, options, &block) - extension = define_extensions model, name, &block - reflection = create_reflection model, name, scope, options, extension + builder = create_builder model, name, scope, options, &block + reflection = builder.build(model) define_accessors model, reflection define_callbacks model, reflection + builder.define_extensions model reflection end - def self.create_reflection(model, name, scope, options, extension = nil) + def self.create_builder(model, name, scope, options, &block) raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol) + new(model, name, scope, options, &block) + end + + def initialize(model, name, scope, options) + # TODO: Move this to create_builder as soon we drop support to activerecord-deprecated_finders. if scope.is_a?(Hash) options = scope scope = nil end - validate_options(options) + # TODO: Remove this model argument as soon we drop support to activerecord-deprecated_finders. + @name = name + @scope = scope + @options = options - scope = build_scope(scope, extension) - - ActiveRecord::Reflection.create(macro, name, scope, options, model) - end - - def self.build_scope(scope, extension) - new_scope = scope + validate_options if scope && scope.arity == 0 - new_scope = proc { instance_exec(&scope) } - end - - if extension - new_scope = wrap_scope new_scope, extension + @scope = proc { instance_exec(&scope) } end - - new_scope end - def self.wrap_scope(scope, extension) - scope + def build(model) + ActiveRecord::Reflection.create(macro, name, scope, options, model) end - def self.macro + def macro raise NotImplementedError end - def self.valid_options(options) - VALID_OPTIONS + Association.extensions.flat_map(&:valid_options) + def valid_options + Association.valid_options + Association.extensions.flat_map(&:valid_options) end - def self.validate_options(options) - options.assert_valid_keys(valid_options(options)) + def validate_options + options.assert_valid_keys(valid_options) end - def self.define_extensions(model, name) + def define_extensions(model) end def self.define_callbacks(model, reflection) @@ -114,6 +118,8 @@ module ActiveRecord::Associations::Builder raise NotImplementedError end + private + def self.add_before_destroy_callbacks(model, reflection) unless valid_dependent_options.include? reflection.options[:dependent] raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{reflection.options[:dependent]}" diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index e8e36e7cd0..62cc1e3a8d 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -1,10 +1,10 @@ module ActiveRecord::Associations::Builder class BelongsTo < SingularAssociation #:nodoc: - def self.macro + def macro :belongs_to end - def self.valid_options(options) + def valid_options super + [:foreign_type, :polymorphic, :touch, :counter_cache] end @@ -23,6 +23,8 @@ module ActiveRecord::Associations::Builder add_counter_cache_methods mixin end + private + def self.add_counter_cache_methods(mixin) return if mixin.method_defined? :belongs_to_counter_cache_after_create diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index 2ff67f904d..bc15a49996 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -7,11 +7,22 @@ module ActiveRecord::Associations::Builder CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove] - def self.valid_options(options) + def valid_options super + [:table_name, :before_add, :after_add, :before_remove, :after_remove, :extend] end + attr_reader :block_extension + + def initialize(model, name, scope, options) + super + @mod = nil + if block_given? + @mod = Module.new(&Proc.new) + @scope = wrap_scope @scope, @mod + end + end + def self.define_callbacks(model, reflection) super name = reflection.name @@ -21,11 +32,10 @@ module ActiveRecord::Associations::Builder } end - def self.define_extensions(model, name) - if block_given? + def define_extensions(model) + if @mod extension_module_name = "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension" - extension = Module.new(&Proc.new) - model.parent.const_set(extension_module_name, extension) + model.parent.const_set(extension_module_name, @mod) end end @@ -68,7 +78,9 @@ module ActiveRecord::Associations::Builder CODE end - def self.wrap_scope(scope, mod) + private + + def wrap_scope(scope, mod) if scope proc { |owner| instance_exec(owner, &scope).extending(mod) } else diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 1c9c04b044..e472277374 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -20,7 +20,7 @@ module ActiveRecord::Associations::Builder def self.build(lhs_class, name, options) if options[:join_table] - KnownTable.new options[:join_table] + KnownTable.new options[:join_table].to_s else class_name = options.fetch(:class_name) { name.to_s.camelize.singularize @@ -84,11 +84,11 @@ module ActiveRecord::Associations::Builder middle_name = [lhs_model.name.downcase.pluralize, association_name].join('_').gsub(/::/, '_').to_sym middle_options = middle_options join_model - - HasMany.create_reflection(lhs_model, - middle_name, - nil, - middle_options) + hm_builder = HasMany.create_builder(lhs_model, + middle_name, + nil, + middle_options) + hm_builder.build lhs_model end private diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index 227184cd19..7909b93622 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -1,10 +1,10 @@ module ActiveRecord::Associations::Builder class HasMany < CollectionAssociation #:nodoc: - def self.macro + def macro :has_many end - def self.valid_options(options) + def valid_options super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :counter_cache] end diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index 064a3c8b51..f359efd496 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -1,10 +1,10 @@ module ActiveRecord::Associations::Builder class HasOne < SingularAssociation #:nodoc: - def self.macro + def macro :has_one end - def self.valid_options(options) + def valid_options valid = super + [:order, :as] valid += [:through, :source, :source_type] if options[:through] valid @@ -14,6 +14,8 @@ module ActiveRecord::Associations::Builder [:destroy, :delete, :nullify, :restrict_with_error, :restrict_with_exception] end + private + def self.add_before_destroy_callbacks(model, reflection) super unless reflection.options[:through] end diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index a6e8300642..e655c389a6 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -2,7 +2,7 @@ module ActiveRecord::Associations::Builder class SingularAssociation < Association #:nodoc: - def self.valid_options(options) + def valid_options super + [:remote, :dependent, :primary_key, :inverse_of] end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 62f23f54f9..e7bcc59354 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -193,7 +193,11 @@ module ActiveRecord # Count all records using SQL. Construct options and pass them with # scope to the target class's +count+. - def count(column_name = nil) + def count(column_name = nil, count_options = {}) + # TODO: Remove count_options argument as soon we remove support to + # activerecord-deprecated_finders. + column_name, count_options = nil, column_name if column_name.is_a?(Hash) + relation = scope if association_scope.distinct_value # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL. diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 0b37ecf5b7..0dabe256e3 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -669,8 +669,10 @@ module ActiveRecord # # #<Pet id: 2, name: "Spook", person_id: 1>, # # #<Pet id: 3, name: "Choo-Choo", person_id: 1> # # ] - def count(column_name = nil) - @association.count(column_name) + def count(column_name = nil, options = {}) + # TODO: Remove options argument as soon we remove support to + # activerecord-deprecated_finders. + @association.count(column_name, options) end # Returns the size of the collection. If the collection hasn't been loaded, diff --git a/activerecord/lib/active_record/associations/preloader/singular_association.rb b/activerecord/lib/active_record/associations/preloader/singular_association.rb index 2b5cfda8ce..f60647a81e 100644 --- a/activerecord/lib/active_record/associations/preloader/singular_association.rb +++ b/activerecord/lib/active_record/associations/preloader/singular_association.rb @@ -11,7 +11,7 @@ module ActiveRecord association = owner.association(reflection.name) association.target = record - association.set_inverse_instance(record) + association.set_inverse_instance(record) if record end end diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index 02dc464536..e4500af5b2 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -39,7 +39,9 @@ module ActiveRecord end def find_target - scope.first.tap { |record| set_inverse_instance(record) } + if record = scope.first + set_inverse_instance record + end end def replace(record) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index e3270e734f..1b9f865666 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -148,7 +148,7 @@ module ActiveRecord QUOTED_TRUE, QUOTED_FALSE = '1', '0' NATIVE_DATABASE_TYPES = { - :primary_key => "int(11) DEFAULT NULL auto_increment PRIMARY KEY", + :primary_key => "int(11) auto_increment PRIMARY KEY", :string => { :name => "varchar", :limit => 255 }, :text => { :name => "text" }, :integer => { :name => "int", :limit => 4 }, diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 2cf1015f2c..a02eda5603 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -586,7 +586,11 @@ module ActiveRecord def translate_exception(exception, message) case exception.message - when /column(s)? .* (is|are) not unique/ + # SQLite 3.8.2 returns a newly formatted error message: + # UNIQUE constraint failed: *table_name*.*column_name* + # Older versions of SQLite return: + # column *column_name* is not unique + when /column(s)? .* (is|are) not unique/, /UNIQUE constraint failed: .*/ RecordNotUnique.new(message, exception) else super diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 7e3bef9431..dcbdf75627 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -82,10 +82,10 @@ module ActiveRecord # Increment a numeric field by one, via a direct SQL update. # - # This method is used primarily for maintaining counter_cache columns used to - # store aggregate values. For example, a DiscussionBoard may cache posts_count - # and comments_count to avoid running an SQL query to calculate the number of - # posts and comments there are each time it is displayed. + # This method is used primarily for maintaining counter_cache columns that are + # used to store aggregate values. For example, a DiscussionBoard may cache + # posts_count and comments_count to avoid running an SQL query to calculate the + # number of posts and comments there are, each time it is displayed. # # ==== Parameters # diff --git a/activerecord/lib/active_record/dynamic_matchers.rb b/activerecord/lib/active_record/dynamic_matchers.rb index e650ebcf64..5caab09038 100644 --- a/activerecord/lib/active_record/dynamic_matchers.rb +++ b/activerecord/lib/active_record/dynamic_matchers.rb @@ -84,13 +84,18 @@ module ActiveRecord "#{finder}(#{attributes_hash})" end + # The parameters in the signature may have reserved Ruby words, in order + # to prevent errors, we start each param name with `_`. + # # Extended in activerecord-deprecated_finders def signature - attribute_names.join(', ') + attribute_names.map { |name| "_#{name}" }.join(', ') end + # Given that the parameters starts with `_`, the finder needs to use the + # same parameter name. def attributes_hash - "{" + attribute_names.map { |name| ":#{name} => #{name}" }.join(',') + "}" + "{" + attribute_names.map { |name| ":#{name} => _#{name}" }.join(',') + "}" end def finder diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 7e38719811..2602f79db9 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -188,7 +188,7 @@ module ActiveRecord end end - # Raised when a primary key is needed, but there is not one specified in the schema or model. + # Raised when a primary key is needed, but not specified in the schema or model. class UnknownPrimaryKey < ActiveRecordError attr_reader :model diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 080b20134d..5b255c3fe5 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -50,8 +50,10 @@ module ActiveRecord 0 end - def calculate(_operation, _column_name) - if _operation == :count + def calculate(operation, _column_name, _options = {}) + # TODO: Remove _options argument as soon we remove support to + # activerecord-deprecated_finders. + if operation == :count 0 else nil diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 2d267183ce..45ffb99868 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -19,16 +19,21 @@ module ActiveRecord # # Person.group(:city).count # # => { 'Rome' => 5, 'Paris' => 3 } - def count(column_name = nil) - calculate(:count, column_name) + def count(column_name = nil, options = {}) + # TODO: Remove options argument as soon we remove support to + # activerecord-deprecated_finders. + column_name, options = nil, column_name if column_name.is_a?(Hash) + calculate(:count, column_name, options) end # Calculates the average value on a given column. Returns +nil+ if there's # no row. See +calculate+ for examples with options. # # Person.average(:age) # => 35.8 - def average(column_name) - calculate(:average, column_name) + def average(column_name, options = {}) + # TODO: Remove options argument as soon we remove support to + # activerecord-deprecated_finders. + calculate(:average, column_name, options) end # Calculates the minimum value on a given column. The value is returned @@ -36,8 +41,10 @@ module ActiveRecord # +calculate+ for examples with options. # # Person.minimum(:age) # => 7 - def minimum(column_name) - calculate(:minimum, column_name) + def minimum(column_name, options = {}) + # TODO: Remove options argument as soon we remove support to + # activerecord-deprecated_finders. + calculate(:minimum, column_name, options) end # Calculates the maximum value on a given column. The value is returned @@ -45,8 +52,10 @@ module ActiveRecord # +calculate+ for examples with options. # # Person.maximum(:age) # => 93 - def maximum(column_name) - calculate(:maximum, column_name) + def maximum(column_name, options = {}) + # TODO: Remove options argument as soon we remove support to + # activerecord-deprecated_finders. + calculate(:maximum, column_name, options) end # Calculates the sum of values on a given column. The value is returned @@ -89,15 +98,17 @@ module ActiveRecord # Person.group(:last_name).having("min(age) > 17").minimum(:age) # # Person.sum("2 * age") - def calculate(operation, column_name) + def calculate(operation, column_name, options = {}) + # TODO: Remove options argument as soon we remove support to + # activerecord-deprecated_finders. if column_name.is_a?(Symbol) && attribute_alias?(column_name) column_name = attribute_alias(column_name) end if has_include?(column_name) - construct_relation_for_association_calculations.calculate(operation, column_name) + construct_relation_for_association_calculations.calculate(operation, column_name, options) else - perform_calculation(operation, column_name) + perform_calculation(operation, column_name, options) end end @@ -180,7 +191,9 @@ module ActiveRecord eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?)) end - def perform_calculation(operation, column_name) + def perform_calculation(operation, column_name, options = {}) + # TODO: Remove options argument as soon we remove support to + # activerecord-deprecated_finders. operation = operation.to_s.downcase # If #count is used with #distinct / #uniq it is considered distinct. (eg. relation.distinct.count) @@ -311,7 +324,9 @@ module ActiveRecord } key = key.first if key.size == 1 key = key_records[key] if associated - [key, type_cast_calculated_value(row[aggregate_alias], column_for(column_name), operation)] + + column_type = calculated_data.column_types.fetch(aggregate_alias) { column_for(column_name) } + [key, type_cast_calculated_value(row[aggregate_alias], column_type, operation)] end] end diff --git a/activerecord/test/cases/adapters/mysql/connection_test.rb b/activerecord/test/cases/adapters/mysql/connection_test.rb index a1b41b6991..5cd5d8ac5f 100644 --- a/activerecord/test/cases/adapters/mysql/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql/connection_test.rb @@ -71,7 +71,7 @@ class MysqlConnectionTest < ActiveRecord::TestCase def test_exec_no_binds @connection.exec_query('drop table if exists ex') @connection.exec_query(<<-eosql) - CREATE TABLE `ex` (`id` int(11) DEFAULT NULL auto_increment PRIMARY KEY, + CREATE TABLE `ex` (`id` int(11) auto_increment PRIMARY KEY, `data` varchar(255)) eosql result = @connection.exec_query('SELECT id, data FROM ex') @@ -93,7 +93,7 @@ class MysqlConnectionTest < ActiveRecord::TestCase def test_exec_with_binds @connection.exec_query('drop table if exists ex') @connection.exec_query(<<-eosql) - CREATE TABLE `ex` (`id` int(11) DEFAULT NULL auto_increment PRIMARY KEY, + CREATE TABLE `ex` (`id` int(11) auto_increment PRIMARY KEY, `data` varchar(255)) eosql @connection.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")') @@ -109,7 +109,7 @@ class MysqlConnectionTest < ActiveRecord::TestCase def test_exec_typecasts_bind_vals @connection.exec_query('drop table if exists ex') @connection.exec_query(<<-eosql) - CREATE TABLE `ex` (`id` int(11) DEFAULT NULL auto_increment PRIMARY KEY, + CREATE TABLE `ex` (`id` int(11) auto_increment PRIMARY KEY, `data` varchar(255)) eosql @connection.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")') diff --git a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb index 9ad0744aee..a2c933d96a 100644 --- a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb @@ -10,7 +10,7 @@ module ActiveRecord @conn.exec_query('drop table if exists ex') @conn.exec_query(<<-eosql) CREATE TABLE `ex` ( - `id` int(11) DEFAULT NULL auto_increment PRIMARY KEY, + `id` int(11) auto_increment PRIMARY KEY, `number` integer, `data` varchar(255)) eosql @@ -75,7 +75,7 @@ module ActiveRecord @conn.exec_query('drop table if exists ex_with_non_standard_pk') @conn.exec_query(<<-eosql) CREATE TABLE `ex_with_non_standard_pk` ( - `code` INT(11) DEFAULT NULL auto_increment, + `code` INT(11) auto_increment, PRIMARY KEY (`code`)) eosql pk, seq = @conn.pk_and_sequence_for('ex_with_non_standard_pk') @@ -87,7 +87,7 @@ module ActiveRecord @conn.exec_query('drop table if exists ex_with_custom_index_type_pk') @conn.exec_query(<<-eosql) CREATE TABLE `ex_with_custom_index_type_pk` ( - `id` INT(11) DEFAULT NULL auto_increment, + `id` INT(11) auto_increment, PRIMARY KEY USING BTREE (`id`)) eosql pk, seq = @conn.pk_and_sequence_for('ex_with_custom_index_type_pk') diff --git a/activerecord/test/cases/adapters/mysql2/explain_test.rb b/activerecord/test/cases/adapters/mysql2/explain_test.rb index 68ed361aeb..1cd356e868 100644 --- a/activerecord/test/cases/adapters/mysql2/explain_test.rb +++ b/activerecord/test/cases/adapters/mysql2/explain_test.rb @@ -10,15 +10,15 @@ module ActiveRecord def test_explain_for_one_query explain = Developer.where(:id => 1).explain assert_match %(EXPLAIN for: SELECT `developers`.* FROM `developers` WHERE `developers`.`id` = 1), explain - assert_match %(developers | const), explain + assert_match %r(developers |.* const), explain end def test_explain_with_eager_loading explain = Developer.where(:id => 1).includes(:audit_logs).explain assert_match %(EXPLAIN for: SELECT `developers`.* FROM `developers` WHERE `developers`.`id` = 1), explain - assert_match %(developers | const), explain + assert_match %r(developers |.* const), explain assert_match %(EXPLAIN for: SELECT `audit_logs`.* FROM `audit_logs` WHERE `audit_logs`.`developer_id` IN (1)), explain - assert_match %(audit_logs | ALL), explain + assert_match %r(audit_logs |.* ALL), explain end end end diff --git a/activerecord/test/cases/associations/extension_test.rb b/activerecord/test/cases/associations/extension_test.rb index f8f2832ab1..4c1fdfdd9a 100644 --- a/activerecord/test/cases/associations/extension_test.rb +++ b/activerecord/test/cases/associations/extension_test.rb @@ -75,6 +75,7 @@ class AssociationsExtensionsTest < ActiveRecord::TestCase private def extend!(model) - ActiveRecord::Associations::Builder::HasMany.define_extensions(model, :association_name) { } + builder = ActiveRecord::Associations::Builder::HasMany.new(model, :association_name, nil, {}) { } + builder.define_extensions(model) end end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index be928ec8ee..8aee7ff40e 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -570,6 +570,13 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert !developer.special_projects.include?(other_project) end + def test_symbol_join_table + developer = Developer.first + sp = developer.sym_special_projects.create("name" => "omg") + developer.reload + assert_includes developer.sym_special_projects, sp + end + def test_update_attributes_after_push_without_duplicate_join_table_rows developer = Developer.new("name" => "Kano") project = SpecialProject.create("name" => "Special Project") diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 2c41656b3d..2f6913167d 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -211,6 +211,10 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal 19.83, NumericData.sum(:bank_balance) end + def test_should_return_type_casted_values_with_group_and_expression + assert_equal 0.5, Account.group(:firm_name).sum('0.01 * credit_limit')['37signals'] + end + def test_should_group_by_summed_field_with_conditions c = Account.where('firm_id > 1').group(:firm_id).sum(:credit_limit) assert_nil c[1] diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 5e8f1c851f..7a00f7fa8f 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -12,6 +12,7 @@ require 'models/developer' require 'models/customer' require 'models/toy' require 'models/matey' +require 'models/dog' class FinderTest < ActiveRecord::TestCase fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers, :categories, :categorizations @@ -641,6 +642,13 @@ class FinderTest < ActiveRecord::TestCase assert_raise(ActiveRecord::RecordNotFound) { Topic.find_by_title!("The First Topic!") } end + def test_find_by_on_attribute_that_is_a_reserved_word + dog_alias = 'Dog' + dog = Dog.create(alias: dog_alias) + + assert_equal dog, Dog.find_by_alias(dog_alias) + end + def test_find_by_one_attribute_that_is_an_alias assert_equal topics(:first), Topic.find_by_heading("The First Topic") assert_nil Topic.find_by_heading("The First Topic!") diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 34e8f1be0f..40be378797 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -149,23 +149,6 @@ end load_schema -class << Time - unless method_defined? :now_before_time_travel - alias_method :now_before_time_travel, :now - end - - def now - (@now ||= nil) || now_before_time_travel - end - - def travel_to(time, &block) - @now = time - block.call - ensure - @now = nil - end -end - class SQLSubscriber attr_reader :logged attr_reader :payloads @@ -183,7 +166,6 @@ class SQLSubscriber def finish(name, id, payload); end end - module InTimeZone private diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 7c3988859f..519045095d 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -743,7 +743,7 @@ class CopyMigrationsTest < ActiveRecord::TestCase @migrations_path = MIGRATIONS_ROOT + "/valid_with_timestamps" @existing_migrations = Dir[@migrations_path + "/*.rb"] - Time.travel_to(Time.utc(2010, 7, 26, 10, 10, 10)) do + travel_to(Time.utc(2010, 7, 26, 10, 10, 10)) do copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"}) assert File.exist?(@migrations_path + "/20100726101010_people_have_hobbies.bukkits.rb") assert File.exist?(@migrations_path + "/20100726101011_people_have_descriptions.bukkits.rb") @@ -768,7 +768,7 @@ class CopyMigrationsTest < ActiveRecord::TestCase sources[:bukkits] = MIGRATIONS_ROOT + "/to_copy_with_timestamps" sources[:omg] = MIGRATIONS_ROOT + "/to_copy_with_timestamps2" - Time.travel_to(Time.utc(2010, 7, 26, 10, 10, 10)) do + travel_to(Time.utc(2010, 7, 26, 10, 10, 10)) do copied = ActiveRecord::Migration.copy(@migrations_path, sources) assert File.exist?(@migrations_path + "/20100726101010_people_have_hobbies.bukkits.rb") assert File.exist?(@migrations_path + "/20100726101011_people_have_descriptions.bukkits.rb") @@ -788,7 +788,7 @@ class CopyMigrationsTest < ActiveRecord::TestCase @migrations_path = MIGRATIONS_ROOT + "/valid_with_timestamps" @existing_migrations = Dir[@migrations_path + "/*.rb"] - Time.travel_to(Time.utc(2010, 2, 20, 10, 10, 10)) do + travel_to(Time.utc(2010, 2, 20, 10, 10, 10)) do ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"}) assert File.exist?(@migrations_path + "/20100301010102_people_have_hobbies.bukkits.rb") assert File.exist?(@migrations_path + "/20100301010103_people_have_descriptions.bukkits.rb") @@ -863,7 +863,7 @@ class CopyMigrationsTest < ActiveRecord::TestCase @migrations_path = MIGRATIONS_ROOT + "/non_existing" @existing_migrations = [] - Time.travel_to(Time.utc(2010, 7, 26, 10, 10, 10)) do + travel_to(Time.utc(2010, 7, 26, 10, 10, 10)) do copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"}) assert File.exist?(@migrations_path + "/20100726101010_people_have_hobbies.bukkits.rb") assert File.exist?(@migrations_path + "/20100726101011_people_have_descriptions.bukkits.rb") @@ -878,7 +878,7 @@ class CopyMigrationsTest < ActiveRecord::TestCase @migrations_path = MIGRATIONS_ROOT + "/empty" @existing_migrations = [] - Time.travel_to(Time.utc(2010, 7, 26, 10, 10, 10)) do + travel_to(Time.utc(2010, 7, 26, 10, 10, 10)) do copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"}) assert File.exist?(@migrations_path + "/20100726101010_people_have_hobbies.bukkits.rb") assert File.exist?(@migrations_path + "/20100726101011_people_have_descriptions.bukkits.rb") diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index baa3acf3fb..09724d9884 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -295,7 +295,7 @@ class RelationTest < ActiveRecord::TestCase def test_null_relation_calculations_methods assert_no_queries do assert_equal 0, Developer.none.count - assert_equal 0, Developer.none.calculate(:count, nil) + assert_equal 0, Developer.none.calculate(:count, nil, {}) assert_equal nil, Developer.none.calculate(:average, 'salary') end end diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index a26de55758..2e2d8a0d37 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -36,6 +36,10 @@ class Developer < ActiveRecord::Base end has_and_belongs_to_many :special_projects, :join_table => 'developers_projects', :association_foreign_key => 'project_id' + has_and_belongs_to_many :sym_special_projects, + :join_table => :developers_projects, + :association_foreign_key => 'project_id', + :class_name => 'SpecialProject' has_many :audit_logs has_many :contracts diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 92e2e4d409..ac546fc296 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -245,6 +245,7 @@ ActiveRecord::Schema.define do t.integer :trainer_id t.integer :breeder_id t.integer :dog_lover_id + t.string :alias end create_table :edges, force: true, id: false do |t| diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index d23b598144..994e44db00 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,4 +1,4 @@ -* Fix file descriptor being leaked on each call to `Kernel.silence_stream` +* Fix file descriptor being leaked on each call to `Kernel.silence_stream`. *Mario Visic* diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 5cd6065077..8ed60aebac 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -22,15 +22,15 @@ module ActiveSupport extend Strategy::LocalCache end - # Deletes all items from the cache. In this case it deletes all the entries in the specified - # file store directory except for .gitkeep. Be careful which directory is specified in your + # Deletes all items from the cache. In this case it deletes all the entries in the specified + # file store directory except for .gitkeep. Be careful which directory is specified in your # config file when using +FileStore+ because everything in that directory will be deleted. def clear(options = nil) root_dirs = Dir.entries(cache_path).reject {|f| (EXCLUDED_DIRS + [".gitkeep"]).include?(f)} FileUtils.rm_r(root_dirs.collect{|f| File.join(cache_path, f)}) end - # Premptively iterates through all stored keys and removes the ones which have expired. + # Preemptively iterates through all stored keys and removes the ones which have expired. def cleanup(options = nil) options = merged_options(options) search_dir(cache_path) do |fname| @@ -43,33 +43,13 @@ module ActiveSupport # Increments an already existing integer value that is stored in the cache. # If the key is not found nothing is done. def increment(name, amount = 1, options = nil) - file_name = key_file_path(namespaced_key(name, options)) - lock_file(file_name) do - options = merged_options(options) - if num = read(name, options) - num = num.to_i + amount - write(name, num, options) - num - else - nil - end - end + modify_value(name, amount, options) end # Decrements an already existing integer value that is stored in the cache. # If the key is not found nothing is done. def decrement(name, amount = 1, options = nil) - file_name = key_file_path(namespaced_key(name, options)) - lock_file(file_name) do - options = merged_options(options) - if num = read(name, options) - num = num.to_i - amount - write(name, num, options) - num - else - nil - end - end + modify_value(name, -amount, options) end def delete_matched(matcher, options = nil) @@ -184,6 +164,22 @@ module ActiveSupport end end end + + # Modifies the amount of an already existing integer value that is stored in the cache. + # If the key is not found nothing is done. + def modify_value(name, amount, options) + file_name = key_file_path(namespaced_key(name, options)) + + lock_file(file_name) do + options = merged_options(options) + + if num = read(name, options) + num = num.to_i + amount + write(name, num, options) + num + end + end + end end end end diff --git a/activesupport/lib/active_support/cache/memory_store.rb b/activesupport/lib/active_support/cache/memory_store.rb index 34ac91334a..8a0523d0e2 100644 --- a/activesupport/lib/active_support/cache/memory_store.rb +++ b/activesupport/lib/active_support/cache/memory_store.rb @@ -36,7 +36,7 @@ module ActiveSupport end end - # Premptively iterates through all stored keys and removes the ones which have expired. + # Preemptively iterates through all stored keys and removes the ones which have expired. def cleanup(options = nil) options = merged_options(options) instrument(:cleanup, :size => @data.size) do diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index c3aac31323..3052e5011c 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -7,14 +7,14 @@ require 'active_support/core_ext/kernel/singleton_class' require 'thread' module ActiveSupport - # Callbacks are code hooks that are run at key points in an object's lifecycle. + # Callbacks are code hooks that are run at key points in an object's life cycle. # The typical use case is to have a base class define a set of callbacks # relevant to the other functionality it supplies, so that subclasses can # install callbacks that enhance or modify the base functionality without # needing to override or redefine methods of the base class. # # Mixing in this module allows you to define the events in the object's - # lifecycle that will support callbacks (via +ClassMethods.define_callbacks+), + # life cycle that will support callbacks (via +ClassMethods.define_callbacks+), # set the instance methods, procs, or callback objects to be called (via # +ClassMethods.set_callback+), and run the installed callbacks at the # appropriate times (via +run_callbacks+). @@ -89,7 +89,7 @@ module ActiveSupport private - # A hook invoked everytime a before callback is halted. + # A hook invoked every time a before callback is halted. # This can be overridden in AS::Callback implementors in order # to provide better debugging/logging. def halted_callback_hook(filter) @@ -648,7 +648,7 @@ module ActiveSupport self.set_callbacks name, callbacks.dup.clear end - # Define sets of events in the object lifecycle that support callbacks. + # Define sets of events in the object life cycle that support callbacks. # # define_callbacks :validate # define_callbacks :initialize, :save, :destroy diff --git a/activesupport/lib/active_support/core_ext/date/conversions.rb b/activesupport/lib/active_support/core_ext/date/conversions.rb index 6bc8f12176..09504a4d9c 100644 --- a/activesupport/lib/active_support/core_ext/date/conversions.rb +++ b/activesupport/lib/active_support/core_ext/date/conversions.rb @@ -37,12 +37,12 @@ class Date # date.to_formatted_s(:rfc822) # => "10 Nov 2007" # date.to_formatted_s(:iso8601) # => "2007-11-10" # - # == Adding your own time formats to to_formatted_s + # == Adding your own date formats to to_formatted_s # You can add your own formats to the Date::DATE_FORMATS hash. # Use the format name as the hash key and either a strftime string # or Proc instance that takes a date argument as the value. # - # # config/initializers/time_formats.rb + # # config/initializers/date_formats.rb # Date::DATE_FORMATS[:month_and_year] = '%B %Y' # Date::DATE_FORMATS[:short_ordinal] = ->(date) { date.strftime("%B #{date.day.ordinalize}") } def to_formatted_s(format = :default) diff --git a/activesupport/lib/active_support/inflector/methods.rb b/activesupport/lib/active_support/inflector/methods.rb index 0f7ae98a8a..cdee4c2ca5 100644 --- a/activesupport/lib/active_support/inflector/methods.rb +++ b/activesupport/lib/active_support/inflector/methods.rb @@ -117,7 +117,8 @@ module ActiveSupport result.gsub!(/([a-z\d]*)/i) { |match| "#{inflections.acronyms[match] || match.downcase}" } - options.fetch(:capitalize, true) ? result.gsub(/^\w/) { $&.upcase } : result + result.gsub!(/^\w/) { $&.upcase } if options.fetch(:capitalize, true) + result end # Capitalizes all the words and replaces some characters in the string to diff --git a/activesupport/lib/active_support/json/encoding.rb b/activesupport/lib/active_support/json/encoding.rb index 060dcb6995..2859075e10 100644 --- a/activesupport/lib/active_support/json/encoding.rb +++ b/activesupport/lib/active_support/json/encoding.rb @@ -63,33 +63,28 @@ module ActiveSupport private_constant :ESCAPED_CHARS, :ESCAPE_REGEX_WITH_HTML_ENTITIES, :ESCAPE_REGEX_WITHOUT_HTML_ENTITIES, :EscapedString - # Recursively turn the given object into a "jsonified" Ruby data structure - # that the JSON gem understands - i.e. we want only Hash, Array, String, - # Numeric, true, false and nil in the final tree. Calls #as_json on it if - # it's not from one of these base types. - # - # This allows developers to implement #as_json withouth having to worry - # about what base types of objects they are allowed to return and having - # to remember calling #as_json recursively. - # - # By default, the options hash is not passed to the children data structures - # to avoid undesiarable result. Develoers must opt-in by implementing - # custom #as_json methods (e.g. Hash#as_json and Array#as_json). + # Convert an object into a "JSON-ready" representation composed of + # primitives like Hash, Array, String, Numeric, and true/false/nil. + # Recursively calls #as_json to the object to recursively build a + # fully JSON-ready object. + # + # This allows developers to implement #as_json without having to + # worry about what base types of objects they are allowed to return + # or having to remember to call #as_json recursively. + # + # Note: the +options+ hash passed to +object.to_json+ is only passed + # to +object.as_json+, not any of this method's recursive +#as_json+ + # calls. def jsonify(value) - if value.is_a?(Hash) - Hash[value.map { |k, v| [jsonify(k), jsonify(v)] }] - elsif value.is_a?(Array) - value.map { |v| jsonify(v) } - elsif value.is_a?(String) + case value + when String EscapedString.new(value) - elsif value.is_a?(Numeric) + when Numeric, NilClass, TrueClass, FalseClass value - elsif value == true - true - elsif value == false - false - elsif value == nil - nil + when Hash + Hash[value.map { |k, v| [jsonify(k), jsonify(v)] }] + when Array + value.map { |v| jsonify(v) } else jsonify value.as_json end diff --git a/activesupport/lib/active_support/message_verifier.rb b/activesupport/lib/active_support/message_verifier.rb index a35d5980fe..8e6e1dcfeb 100644 --- a/activesupport/lib/active_support/message_verifier.rb +++ b/activesupport/lib/active_support/message_verifier.rb @@ -39,8 +39,9 @@ module ActiveSupport if data.present? && digest.present? && secure_compare(digest, generate_digest(data)) begin @serializer.load(::Base64.strict_decode64(data)) - rescue ArgumentError - raise InvalidSignature + rescue ArgumentError => argument_error + raise InvalidSignature if argument_error.message =~ %r{invalid base64} + raise end else raise InvalidSignature diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb index c6658dba96..b169e3af01 100644 --- a/activesupport/lib/active_support/number_helper.rb +++ b/activesupport/lib/active_support/number_helper.rb @@ -1,4 +1,3 @@ - module ActiveSupport module NumberHelper extend ActiveSupport::Autoload @@ -343,6 +342,5 @@ module ActiveSupport def number_to_human(number, options = {}) NumberToHumanConverter.convert(number, options) end - end end diff --git a/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb b/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb index d1335f6910..78d2c9ae6e 100644 --- a/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb +++ b/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb @@ -1,6 +1,6 @@ module ActiveSupport module NumberHelper - class NumberToHumanSizeConverter < NumberConverter + class NumberToHumanSizeConverter < NumberConverter #:nodoc: STORAGE_UNITS = [:byte, :kb, :mb, :gb, :tb] self.namespace = :human diff --git a/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb b/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb index 4c33c30772..af2ee56d91 100644 --- a/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb +++ b/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb @@ -1,6 +1,6 @@ module ActiveSupport module NumberHelper - class NumberToPhoneConverter < NumberConverter + class NumberToPhoneConverter < NumberConverter #:nodoc: def convert str = country_code(opts[:country_code]) str << convert_to_phone_number(number.to_s.strip) diff --git a/activesupport/lib/active_support/testing/deprecation.rb b/activesupport/lib/active_support/testing/deprecation.rb index a8342904dc..6c94c611b6 100644 --- a/activesupport/lib/active_support/testing/deprecation.rb +++ b/activesupport/lib/active_support/testing/deprecation.rb @@ -19,18 +19,17 @@ module ActiveSupport result end - private - def collect_deprecations - old_behavior = ActiveSupport::Deprecation.behavior - deprecations = [] - ActiveSupport::Deprecation.behavior = Proc.new do |message, callstack| - deprecations << message - end - result = yield - [result, deprecations] - ensure - ActiveSupport::Deprecation.behavior = old_behavior + def collect_deprecations + old_behavior = ActiveSupport::Deprecation.behavior + deprecations = [] + ActiveSupport::Deprecation.behavior = Proc.new do |message, callstack| + deprecations << message end + result = yield + [result, deprecations] + ensure + ActiveSupport::Deprecation.behavior = old_behavior + end end end end diff --git a/activesupport/lib/active_support/testing/isolation.rb b/activesupport/lib/active_support/testing/isolation.rb index 18c48a7e0c..75ead48376 100644 --- a/activesupport/lib/active_support/testing/isolation.rb +++ b/activesupport/lib/active_support/testing/isolation.rb @@ -1,5 +1,4 @@ require 'rbconfig' -require 'minitest/parallel' module ActiveSupport module Testing @@ -7,11 +6,9 @@ module ActiveSupport require 'thread' def self.included(klass) #:nodoc: - klass.extend(Module.new { - def test_methods - ParallelEach.new super - end - }) + klass.class_eval do + parallelize_me! + end end def self.forking_env? diff --git a/activesupport/test/abstract_unit.rb b/activesupport/test/abstract_unit.rb index 65de48a7f6..4600855998 100644 --- a/activesupport/test/abstract_unit.rb +++ b/activesupport/test/abstract_unit.rb @@ -15,7 +15,6 @@ silence_warnings do end require 'active_support/testing/autorun' -require 'empty_bool' ENV['NO_RELOAD'] = '1' require 'active_support' diff --git a/activesupport/test/core_ext/blank_test.rb b/activesupport/test/core_ext/blank_test.rb index a68c074777..3e3176f993 100644 --- a/activesupport/test/core_ext/blank_test.rb +++ b/activesupport/test/core_ext/blank_test.rb @@ -4,6 +4,14 @@ require 'abstract_unit' require 'active_support/core_ext/object/blank' class BlankTest < ActiveSupport::TestCase + class EmptyTrue + def empty?() true; end + end + + class EmptyFalse + def empty?() false; end + end + BLANK = [ EmptyTrue.new, nil, false, '', ' ', " \n\t \r ", ' ', [], {} ] NOT = [ EmptyFalse.new, Object.new, true, 0, 1, 'a', [nil], { nil => 0 } ] diff --git a/activesupport/test/empty_bool.rb b/activesupport/test/empty_bool.rb deleted file mode 100644 index 005b3523ef..0000000000 --- a/activesupport/test/empty_bool.rb +++ /dev/null @@ -1,7 +0,0 @@ -class EmptyTrue - def empty?() true; end -end - -class EmptyFalse - def empty?() false; end -end diff --git a/activesupport/test/message_verifier_test.rb b/activesupport/test/message_verifier_test.rb index f208814468..a5748d28ba 100644 --- a/activesupport/test/message_verifier_test.rb +++ b/activesupport/test/message_verifier_test.rb @@ -55,6 +55,20 @@ class MessageVerifierTest < ActiveSupport::TestCase ActiveSupport.use_standard_json_time_format = prev end + def test_raise_error_when_argument_class_is_not_loaded + # To generate the valid message below: + # + # AutoloadClass = Struct.new(:foo) + # valid_message = @verifier.generate(foo: AutoloadClass.new('foo')) + # + valid_message = "BAh7BjoIZm9vbzonTWVzc2FnZVZlcmlmaWVyVGVzdDo6QXV0b2xvYWRDbGFzcwY6CUBmb29JIghmb28GOgZFVA==--f3ef39a5241c365083770566dc7a9eb5d6ace914" + exception = assert_raise(ArgumentError, NameError) do + @verifier.verify(valid_message) + end + assert_includes ["uninitialized constant MessageVerifierTest::AutoloadClass", + "undefined class/module MessageVerifierTest::AutoloadClass"], exception.message + end + def assert_not_verified(message) assert_raise(ActiveSupport::MessageVerifier::InvalidSignature) do @verifier.verify(message) diff --git a/guides/source/4_1_release_notes.md b/guides/source/4_1_release_notes.md index 5c50ed83ea..3126f4e0e1 100644 --- a/guides/source/4_1_release_notes.md +++ b/guides/source/4_1_release_notes.md @@ -62,6 +62,16 @@ app/views/projects/show.html+tablet.erb app/views/projects/show.html+phone.erb ``` +You can also simplify the variants definition using the inline syntax: + +```ruby +respond_to do |format| + format.js { render "trash" } + format.html.phone { redirect_to progress_path } + format.html.none { render "trash" } +end +``` + ### Spring New Rails 4.1 applications will ship with "springified" binstubs. This means diff --git a/guides/source/active_record_callbacks.md b/guides/source/active_record_callbacks.md index 863da3be72..667433285f 100644 --- a/guides/source/active_record_callbacks.md +++ b/guides/source/active_record_callbacks.md @@ -55,7 +55,7 @@ class User < ActiveRecord::Base end ``` -Callbacks can also be registered to only fire on certain lifecycle events: +Callbacks can also be registered to only fire on certain life cycle events: ```ruby class User < ActiveRecord::Base @@ -141,6 +141,55 @@ You have initialized an object! => #<User id: 1> ``` +### `after_touch` + +The `after_touch` callback will be called whenever an Active Record object is touched. + +```ruby +class User < ActiveRecord::Base + after_touch do |user| + puts "You have touched an object" + end +end + +>> u = User.create(name: 'Kuldeep') +=> #<User id: 1, name: "Kuldeep", created_at: "2013-11-25 12:17:49", updated_at: "2013-11-25 12:17:49"> + +>> u.touch +You have touched an object +=> true +``` + +It can be used along with `belongs_to`: + +```ruby +class Employee < ActiveRecord::Base + belongs_to :company, touch: true + after_touch do + puts 'An Employee was touched' + end +end + +class Company < ActiveRecord::Base + has_many :employees + after_touch :log_when_employees_or_company_touched + + private + def log_when_employees_or_company_touched + puts 'Employee/Company was touched' + end +end + +>> @employee = Employee.last +=> #<Employee id: 1, company_id: 1, created_at: "2013-11-25 17:04:22", updated_at: "2013-11-25 17:05:05"> + +# triggers @employee.company.touch +>> @employee.touch +Employee/Company was touched +An Employee was touched +=> true +``` + Running Callbacks ----------------- diff --git a/guides/source/credits.html.erb b/guides/source/credits.html.erb index 5beae9c29b..7c6858fa2c 100644 --- a/guides/source/credits.html.erb +++ b/guides/source/credits.html.erb @@ -76,5 +76,5 @@ Oscar Del Ben is a software engineer at <a href="http://www.wildfireapp.com/">Wi <% end %> <%= author('Akshay Surve', 'startupjockey', 'akshaysurve.jpg') do %> - Akshay Surve is the Founder at <a href="http://www.deltax.com">DeltaX</a>, hackathon specialist, a midnight code junkie and ocassionally writes prose. You can connect with him on <a href="https://twitter.com/akshaysurve">Twitter</a>, <a href="http://www.linkedin.com/in/akshaysurve">Linkedin</a>, <a href="http://www.akshaysurve.com/">Personal Blog</a> or <a href="http://www.quora.com/Akshay-Surve">Quora</a>. + Akshay Surve is the Founder at <a href="http://www.deltax.com">DeltaX</a>, hackathon specialist, a midnight code junkie and occasionally writes prose. You can connect with him on <a href="https://twitter.com/akshaysurve">Twitter</a>, <a href="http://www.linkedin.com/in/akshaysurve">Linkedin</a>, <a href="http://www.akshaysurve.com/">Personal Blog</a> or <a href="http://www.quora.com/Akshay-Surve">Quora</a>. <% end %> diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index de06ab291f..9be48acb12 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -93,6 +93,39 @@ If you application depends on one of these features, you can get them back by adding the [`activesupport-json_encoder`](https://github.com/rails/activesupport-json_encoder) gem to your Gemfile. +### Usage of `return` within inline callback blocks + +Previously, Rails allowed you to `return` from an inline callback block: + +```ruby +class ReadOnlyModel < ActiveRecord::Base + before_save { return false } +end +``` + +This behaviour was never intentionally supported. Due to a change in the internals +of `ActiveSupport::Callbacks`, this is no longer allowed in Rails 4.1. Using a +`return` statement in an inline callback block will cause a `LocalJumpError` to +be raised when the callback is executed. If you need to use `return` statements +in your callbacks, it is recommended that you explicitly define them as methods +and pass the method name as a symbol instead: + +```ruby +class ReadOnlyModel < ActiveRecord::Base + before_save :before_save_callback + + private + def before_save_callback + return false + end +end +``` + +This change applies to most places in Rails where callbacks are used, including +Active Record and Active Model callbacks, as well as "filters" in Action +Controller (e.g. `before_action`). See [this pull request](https://github.com/rails/rails/pull/13271) +for more details. + ### Methods defined in Active Record fixtures Rails 4.1 evaluates each fixture's ERB in a separate context, so helper methods |