diff options
21 files changed, 149 insertions, 41 deletions
diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index d66b2214ce..17fcc2fa02 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -3,12 +3,27 @@ module ActionController include BasicImplicitRender + # Renders the template corresponding to the controller action, if it exists. + # The action name, format, and variant are all taken into account. + # For example, the "new" action with an HTML format and variant "phone" + # would try to render the <tt>new.html+phone.erb</tt> template. + # + # If no template is found <tt>ActionController::BasicImplicitRender</tt>'s implementation is called, unless + # a block is passed. In that case, it will override the super implementation. + # + # default_render do + # head 404 # No template was found + # end def default_render(*args) if template_exists?(action_name.to_s, _prefixes, variants: request.variant) render(*args) else - logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger - super + if block_given? + yield(*args) + else + logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger + super + end end end diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 7aef8a50ce..8591bdb4d9 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -793,3 +793,24 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "phone", @response.body end end + +class RespondToWithBlockOnDefaultRenderController < ActionController::Base + def show + default_render do + render text: 'default_render yielded' + end + end +end + +class RespondToWithBlockOnDefaultRenderControllerTest < ActionController::TestCase + def setup + super + @request.host = "www.example.com" + end + + def test_default_render_uses_block_when_no_template_exists + get :show + assert_equal "default_render yielded", @response.body + assert_equal "text/html", @response.content_type + end +end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 6223a52a76..aca28ae8d1 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -280,7 +280,7 @@ class CookiesTest < ActionController::TestCase def test_setting_the_same_value_to_cookie request.cookies[:user_name] = 'david' get :authenticate - assert response.cookies.empty? + assert_predicate response.cookies, :empty? end def test_setting_the_same_value_to_permanent_cookie @@ -360,7 +360,7 @@ class CookiesTest < ActionController::TestCase def test_delete_unexisting_cookie request.cookies.clear get :delete_cookie - assert @response.cookies.empty? + assert_predicate @response.cookies, :empty? end def test_deleted_cookie_predicate @@ -378,7 +378,7 @@ class CookiesTest < ActionController::TestCase def test_cookies_persist_throughout_request response = get :authenticate - assert response.headers["Set-Cookie"] =~ /user_name=david/ + assert_match(/user_name=david/, response.headers["Set-Cookie"]) end def test_set_permanent_cookie diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index fe47fb47fd..069d181674 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,16 @@ +* Asset helpers raise `ArgumentError` when `nil` is passed as a source. + + *Anton Kolomiychuk* + +* Always attach the template digest to the cache key for collection caching + even when `virtual_path` is not available from the view context. + Which could happen if the rendering was done directly in the controller + and not in a template. + + Fixes #20535 + + *Roque Pinel* + * Improve detection of partial templates eligible for collection caching, now allowing multi-line comments at the beginning of the template file. diff --git a/actionview/lib/action_view/helpers/asset_url_helper.rb b/actionview/lib/action_view/helpers/asset_url_helper.rb index ef4a6c98c0..b19dc25025 100644 --- a/actionview/lib/action_view/helpers/asset_url_helper.rb +++ b/actionview/lib/action_view/helpers/asset_url_helper.rb @@ -121,6 +121,8 @@ module ActionView # asset_path "application", type: :stylesheet # => /assets/application.css # asset_path "http://www.example.com/js/xmlhr.js" # => http://www.example.com/js/xmlhr.js def asset_path(source, options = {}) + raise ArgumentError, "nil is not a valid asset source" if source.nil? + source = source.to_s return "" unless source.present? return source if source =~ URI_REGEXP diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb index 72e2aa1807..8945575860 100644 --- a/actionview/lib/action_view/helpers/cache_helper.rb +++ b/actionview/lib/action_view/helpers/cache_helper.rb @@ -137,7 +137,7 @@ module ActionView # The automatic cache multi read can be turned off like so: # # <%= render @notifications, cache: false %> - def cache(name = {}, options = nil, &block) + def cache(name = {}, options = {}, &block) if controller.respond_to?(:perform_caching) && controller.perform_caching safe_concat(fragment_for(cache_fragment_name(name, options), options, &block)) else @@ -153,7 +153,7 @@ module ActionView # <b>All the topics on this project</b> # <%= render project.topics %> # <% end %> - def cache_if(condition, name = {}, options = nil, &block) + def cache_if(condition, name = {}, options = {}, &block) if condition cache(name, options, &block) else @@ -169,22 +169,23 @@ module ActionView # <b>All the topics on this project</b> # <%= render project.topics %> # <% end %> - def cache_unless(condition, name = {}, options = nil, &block) + def cache_unless(condition, name = {}, options = {}, &block) cache_if !condition, name, options, &block end # This helper returns the name of a cache key for a given fragment cache - # call. By supplying skip_digest: true to cache, the digestion of cache + # call. By supplying +skip_digest:+ true to cache, the digestion of cache # fragments can be manually bypassed. This is useful when cache fragments # cannot be manually expired unless you know the exact key which is the # case when using memcached. - def cache_fragment_name(name = {}, options = nil) - skip_digest = options && options[:skip_digest] - + # + # The digest will be generated using +virtual_path:+ if it is provided. + # + def cache_fragment_name(name = {}, skip_digest: nil, virtual_path: nil) if skip_digest name else - fragment_name_with_digest(name) + fragment_name_with_digest(name, virtual_path) end end @@ -198,10 +199,11 @@ module ActionView private - def fragment_name_with_digest(name) #:nodoc: - if @virtual_path + def fragment_name_with_digest(name, virtual_path) #:nodoc: + virtual_path ||= @virtual_path + if virtual_path names = Array(name.is_a?(Hash) ? controller.url_for(name).split("://").last : name) - digest = Digestor.digest name: @virtual_path, finder: lookup_context, dependencies: view_cache_dependencies + digest = Digestor.digest name: virtual_path, finder: lookup_context, dependencies: view_cache_dependencies [ *names, digest ] else diff --git a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb index c8268e226e..1147963882 100644 --- a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb +++ b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb @@ -51,7 +51,7 @@ module ActionView end def expanded_cache_key(key) - key = @view.fragment_cache_key(@view.cache_fragment_name(key)) + key = @view.fragment_cache_key(@view.cache_fragment_name(key, virtual_path: @template.virtual_path)) key.frozen? ? key.dup : key # #read_multi & #write may require mutability, Dalli 2.6.0. end diff --git a/actionview/test/template/asset_tag_helper_test.rb b/actionview/test/template/asset_tag_helper_test.rb index 6e6ce20924..01fc66bed6 100644 --- a/actionview/test/template/asset_tag_helper_test.rb +++ b/actionview/test/template/asset_tag_helper_test.rb @@ -310,6 +310,11 @@ class AssetTagHelperTest < ActionView::TestCase AssetPathToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end + def test_asset_path_tag_raises_an_error_for_nil_source + e = assert_raise(ArgumentError) { asset_path(nil) } + assert_equal("nil is not a valid asset source", e.message) + end + def test_asset_path_tag_to_not_create_duplicate_slashes @controller.config.asset_host = "host/" assert_dom_equal('http://host/foo', asset_path("foo")) diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index 27bbb9b6c1..9c2c9507b7 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -7,7 +7,10 @@ end module RenderTestCases def setup_view(paths) @assigns = { :secret => 'in the sauce' } - @view = ActionView::Base.new(paths, @assigns) + @view = Class.new(ActionView::Base) do + def view_cache_dependencies; end + end.new(paths, @assigns) + @controller_view = TestController.new.view_context # Reload and register danish language for testing @@ -616,7 +619,7 @@ class CachedCollectionViewRenderTest < CachedViewRenderTest test "with custom key" do customer = Customer.new("david") - key = ActionController::Base.new.fragment_cache_key([customer, 'key']) + key = cache_key([customer, 'key'], "test/_customer") ActionView::PartialRenderer.collection_cache.write(key, 'Hello') @@ -626,7 +629,7 @@ class CachedCollectionViewRenderTest < CachedViewRenderTest test "automatic caching with inferred cache name" do customer = CachedCustomer.new("david") - key = ActionController::Base.new.fragment_cache_key(customer) + key = cache_key(customer, "test/_cached_customer") ActionView::PartialRenderer.collection_cache.write(key, 'Cached') @@ -636,11 +639,17 @@ class CachedCollectionViewRenderTest < CachedViewRenderTest test "automatic caching with as name" do customer = CachedCustomer.new("david") - key = ActionController::Base.new.fragment_cache_key(customer) + key = cache_key(customer, "test/_cached_customer_as") ActionView::PartialRenderer.collection_cache.write(key, 'Cached') assert_equal "Cached", @view.render(partial: "test/cached_customer_as", collection: [customer], as: :buyer) end + + private + def cache_key(names, virtual_path) + digest = ActionView::Digestor.digest name: virtual_path, finder: @view.lookup_context, dependencies: [] + @view.fragment_cache_key([ *Array(names), digest ]) + end end diff --git a/activemodel/lib/active_model/validations/acceptance.rb b/activemodel/lib/active_model/validations/acceptance.rb index ee160fb483..1bcfedb35d 100644 --- a/activemodel/lib/active_model/validations/acceptance.rb +++ b/activemodel/lib/active_model/validations/acceptance.rb @@ -42,9 +42,10 @@ module ActiveModel # Configuration options: # * <tt>:message</tt> - A custom error message (default is: "must be # accepted"). - # * <tt>:accept</tt> - Specifies value that is considered accepted. - # The default value is a string "1", which makes it easy to relate to - # an HTML checkbox. This should be set to +true+ if you are validating + # * <tt>:accept</tt> - Specifies a value that is considered accepted. + # Also accepts an array of possible values. The default value is + # an array ["1", true], which makes it easy to relate to an HTML + # checkbox. This should be set to, or include, +true+ if you are validating # a database column, since the attribute is typecast from "1" to +true+ # before validation. # diff --git a/activemodel/lib/active_model/validations/exclusion.rb b/activemodel/lib/active_model/validations/exclusion.rb index f342d27275..6f4276cc2a 100644 --- a/activemodel/lib/active_model/validations/exclusion.rb +++ b/activemodel/lib/active_model/validations/exclusion.rb @@ -29,7 +29,9 @@ module ActiveModel # Configuration options: # * <tt>:in</tt> - An enumerable object of items that the value shouldn't # be part of. This can be supplied as a proc, lambda or symbol which returns an - # enumerable. If the enumerable is a range the test is performed with + # enumerable. If the enumerable is a numerical, time or datetime range the test + # is performed with <tt>Range#cover?</tt>, otherwise with <tt>include?</tt>. When + # using a proc or lambda the instance under validation is passed as an argument. # * <tt>:within</tt> - A synonym(or alias) for <tt>:in</tt> # <tt>Range#cover?</tt>, otherwise with <tt>include?</tt>. # * <tt>:message</tt> - Specifies a custom error message (default is: "is diff --git a/activemodel/lib/active_model/validations/inclusion.rb b/activemodel/lib/active_model/validations/inclusion.rb index c84025f083..03e0ef56d8 100644 --- a/activemodel/lib/active_model/validations/inclusion.rb +++ b/activemodel/lib/active_model/validations/inclusion.rb @@ -28,9 +28,9 @@ module ActiveModel # Configuration options: # * <tt>:in</tt> - An enumerable object of available items. This can be # supplied as a proc, lambda or symbol which returns an enumerable. If the - # enumerable is a numerical range the test is performed with <tt>Range#cover?</tt>, - # otherwise with <tt>include?</tt>. When using a proc or lambda the instance - # under validation is passed as an argument. + # enumerable is a numerical, time or datetime range the test is performed + # with <tt>Range#cover?</tt>, otherwise with <tt>include?</tt>. When using + # a proc or lambda the instance under validation is passed as an argument. # * <tt>:within</tt> - A synonym(or alias) for <tt>:in</tt> # * <tt>:message</tt> - Specifies a custom error message (default is: "is # not included in the list"). diff --git a/activemodel/test/cases/validations/acceptance_validation_test.rb b/activemodel/test/cases/validations/acceptance_validation_test.rb index 9c2114d83d..d3995ad5af 100644 --- a/activemodel/test/cases/validations/acceptance_validation_test.rb +++ b/activemodel/test/cases/validations/acceptance_validation_test.rb @@ -50,6 +50,20 @@ class AcceptanceValidationTest < ActiveModel::TestCase assert t.valid? end + def test_terms_of_service_agreement_with_multiple_accept_values + Topic.validates_acceptance_of(:terms_of_service, accept: [1, "I concur."]) + + t = Topic.new("title" => "We should be confirmed", "terms_of_service" => "") + assert t.invalid? + assert_equal ["must be accepted"], t.errors[:terms_of_service] + + t.terms_of_service = 1 + assert t.valid? + + t.terms_of_service = "I concur." + assert t.valid? + end + def test_validates_acceptance_of_for_ruby_class Person.validates_acceptance_of :karma diff --git a/activemodel/test/cases/validations/exclusion_validation_test.rb b/activemodel/test/cases/validations/exclusion_validation_test.rb index b269c3691a..005bc15df5 100644 --- a/activemodel/test/cases/validations/exclusion_validation_test.rb +++ b/activemodel/test/cases/validations/exclusion_validation_test.rb @@ -1,4 +1,5 @@ require 'cases/helper' +require 'active_support/core_ext/numeric/time' require 'models/topic' require 'models/person' @@ -64,6 +65,22 @@ class ExclusionValidationTest < ActiveModel::TestCase assert t.valid? end + def test_validates_exclusion_of_with_range + Topic.validates_exclusion_of :content, in: ("a".."g") + + assert Topic.new(content: 'g').invalid? + assert Topic.new(content: 'h').valid? + end + + def test_validates_exclusion_of_with_time_range + Topic.validates_exclusion_of :created_at, in: 6.days.ago..2.days.ago + + assert Topic.new(created_at: 5.days.ago).invalid? + assert Topic.new(created_at: 3.days.ago).invalid? + assert Topic.new(created_at: 7.days.ago).valid? + assert Topic.new(created_at: 1.day.ago).valid? + end + def test_validates_inclusion_of_with_symbol Person.validates_exclusion_of :karma, in: :reserved_karmas diff --git a/activerecord/lib/active_record/table_metadata.rb b/activerecord/lib/active_record/table_metadata.rb index 3dd6321a97..41f1c55c3c 100644 --- a/activerecord/lib/active_record/table_metadata.rb +++ b/activerecord/lib/active_record/table_metadata.rb @@ -41,7 +41,7 @@ module ActiveRecord association = klass._reflect_on_association(table_name) if association && !association.polymorphic? association_klass = association.klass - arel_table = association_klass.arel_table + arel_table = association_klass.arel_table.alias(table_name) else type_caster = TypeCaster::Connection.new(klass, table_name) association_klass = nil diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index d506722f75..09fbdc0d32 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -738,7 +738,7 @@ You can choose not to yield and build the response yourself, in which case the a While the most common way to use filters is by creating private methods and using *_action to add them, there are two other ways to do the same thing. -The first is to use a block directly with the *\_action methods. The block receives the controller as an argument, and the `require_login` filter from above could be rewritten to use a block: +The first is to use a block directly with the *\_action methods. The block receives the controller as an argument. The `require_login` filter from above could be rewritten to use a block: ```ruby class ApplicationController < ActionController::Base diff --git a/guides/source/association_basics.md b/guides/source/association_basics.md index 5c96a53d09..c0fa3cfd04 100644 --- a/guides/source/association_basics.md +++ b/guides/source/association_basics.md @@ -1519,20 +1519,30 @@ conditions exists in the collection. It uses the same syntax and options as ##### `collection.build(attributes = {}, ...)` -The `collection.build` method returns one or more new objects of the associated type. These objects will be instantiated from the passed attributes, and the link through their foreign key will be created, but the associated objects will _not_ yet be saved. +The `collection.build` method returns a single or array of new objects of the associated type. The object(s) will be instantiated from the passed attributes, and the link through their foreign key will be created, but the associated objects will _not_ yet be saved. ```ruby @order = @customer.orders.build(order_date: Time.now, order_number: "A12345") + +@orders = @customer.orders.build([ + { order_date: Time.now, order_number: "A12346" }, + { order_date: Time.now, order_number: "A12347" } +]) ``` ##### `collection.create(attributes = {})` -The `collection.create` method returns a new object of the associated type. This object will be instantiated from the passed attributes, the link through its foreign key will be created, and, once it passes all of the validations specified on the associated model, the associated object _will_ be saved. +The `collection.create` method returns a single or array of new objects of the associated type. The object(s) will be instantiated from the passed attributes, the link through its foreign key will be created, and, once it passes all of the validations specified on the associated model, the associated object _will_ be saved. ```ruby @order = @customer.orders.create(order_date: Time.now, order_number: "A12345") + +@orders = @customer.orders.create([ + { order_date: Time.now, order_number: "A12346" }, + { order_date: Time.now, order_number: "A12347" } +]) ``` ##### `collection.create!(attributes = {})` diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index 3279c99c42..c19813c8a5 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -28,7 +28,7 @@ NOTE: Bugs in the most recent released version of Ruby on Rails are likely to ge If you've found a problem in Ruby on Rails which is not a security risk, do a search on GitHub under [Issues](https://github.com/rails/rails/issues) in case it has already been reported. If you are unable to find any open GitHub issues addressing the problem you found, your next step will be to [open a new one](https://github.com/rails/rails/issues/new). (See the next section for reporting security issues.) -Your issue report should contain a title and a clear description of the issue at the bare minimum. You should include as much relevant information as possible and should at least post a code sample that demonstrates the issue. It would be even better if you could include a unit test that shows how the expected behavior is not occurring. Your goal should be to make it easy for yourself - and others - to replicate the bug and figure out a fix. +Your issue report should contain a title and a clear description of the issue at the bare minimum. You should include as much relevant information as possible and should at least post a code sample that demonstrates the issue. It would be even better if you could include a unit test that shows how the expected behavior is not occurring. Your goal should be to make it easy for yourself - and others - to reproduce the bug and figure out a fix. Then, don't get your hopes up! Unless you have a "Code Red, Mission Critical, the World is Coming to an End" kind of bug, you're creating this issue report in the hope that others with the same problem will be able to collaborate with you on solving it. Do not expect that the issue report will automatically see any activity or that others will jump to fix it. Creating an issue like this is mostly to help yourself start on the path of fixing the problem and for others to confirm it with an "I'm having this problem too" comment. diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index a091f84fb0..dfdc11de7a 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -1546,8 +1546,6 @@ class CreateComments < ActiveRecord::Migration create_table :comments do |t| t.string :commenter t.text :body - - # this line adds an integer column called `article_id`. t.references :article, index: true, foreign_key: true t.timestamps null: false @@ -1556,9 +1554,9 @@ class CreateComments < ActiveRecord::Migration end ``` -The `t.references` line sets up a foreign key column for the association between -the two models. An index for this association is also created on this column. -Go ahead and run the migration: +The `t.references` line creates an integer column called `article_id`, an index +for it, and a foreign key constraint that points to the `articles` table. Go +ahead and run the migration: ```bash $ bin/rake db:migrate diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 75fb3343c9..573b1f8f69 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,4 +1,4 @@ -* Fix NoMethodError that occurs when generating scaffold inside full mode engine. +* Fix `NoMethodError` when generating a scaffold inside a full engine. *Yuji Yaginuma* diff --git a/railties/lib/rails/commands/server.rb b/railties/lib/rails/commands/server.rb index c1bd4072ac..6289d4cc46 100644 --- a/railties/lib/rails/commands/server.rb +++ b/railties/lib/rails/commands/server.rb @@ -96,8 +96,7 @@ module Rails DoNotReverseLookup: true, environment: (ENV['RAILS_ENV'] || ENV['RACK_ENV'] || "development").dup, daemonize: false, - pid: File.expand_path("tmp/pids/server.pid"), - config: File.expand_path("config.ru") + pid: File.expand_path("tmp/pids/server.pid") }) end |