diff options
88 files changed, 685 insertions, 218 deletions
diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index c95b9a4097..96d701dba5 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -88,7 +88,7 @@ module AbstractController # Returns the full controller name, underscored, without the ending Controller. # # class MyApp::MyPostsController < AbstractController::Base - # end + # # end # # MyApp::MyPostsController.controller_path # => "my_app/my_posts" 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/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index c98e937423..09867e2407 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -11,9 +11,9 @@ module ActionController # # params = ActionController::Parameters.new(a: {}) # params.fetch(:b) - # # => ActionController::ParameterMissing: param not found: b + # # => ActionController::ParameterMissing: param is missing or the value is empty: b # params.require(:a) - # # => ActionController::ParameterMissing: param not found: a + # # => ActionController::ParameterMissing: param is missing or the value is empty: a class ParameterMissing < KeyError attr_reader :param # :nodoc: @@ -23,11 +23,13 @@ module ActionController end end - # Raised when a supplied parameter is not expected. + # Raised when a supplied parameter is not expected and + # ActionController::Parameters.action_on_unpermitted_parameters + # is set to <tt>:raise</tt>. # # params = ActionController::Parameters.new(a: "123", b: "456") # params.permit(:c) - # # => ActionController::UnpermittedParameters: found unexpected keys: a, b + # # => ActionController::UnpermittedParameters: found unpermitted parameters: a, b class UnpermittedParameters < IndexError attr_reader :params # :nodoc: @@ -236,10 +238,10 @@ module ActionController # # => {"name"=>"Francesco"} # # ActionController::Parameters.new(person: nil).require(:person) - # # => ActionController::ParameterMissing: param not found: person + # # => ActionController::ParameterMissing: param is missing or the value is empty: person # # ActionController::Parameters.new(person: {}).require(:person) - # # => ActionController::ParameterMissing: param not found: person + # # => ActionController::ParameterMissing: param is missing or the value is empty: person def require(key) value = self[key] if value.present? || value == false @@ -356,7 +358,7 @@ module ActionController # # params = ActionController::Parameters.new(person: { name: 'Francesco' }) # params.fetch(:person) # => {"name"=>"Francesco"} - # params.fetch(:none) # => ActionController::ParameterMissing: param not found: none + # params.fetch(:none) # => ActionController::ParameterMissing: param is missing or the value is empty: none # params.fetch(:none, 'Francesco') # => "Francesco" # params.fetch(:none) { 'Francesco' } # => "Francesco" def fetch(key, *args) diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index cc4bd6105d..b84aad8eb6 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -121,7 +121,8 @@ module ActionDispatch end def match_head_routes(routes, req) - head_routes = match_routes(routes, req) + verb_specific_routes = routes.reject { |route| route.verb == // } + head_routes = match_routes(verb_specific_routes, req) if head_routes.empty? begin 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/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 77f86b7a62..26b8636c2f 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3567,12 +3567,11 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest mount lambda { |env| [200, {}, [env['REQUEST_METHOD']]] }, at: '/' end - # TODO: HEAD request should match `get /home` rather than the + # HEAD request should match `get /home` rather than the # lower-precedence Rack app mounted at `/`. head '/home' assert_response :ok - #assert_equal 'test#index', @response.body - assert_equal 'HEAD', @response.body + assert_equal 'test#index', @response.body # But the Rack app can still respond to its own HEAD requests. head '/foobar' diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 7b6008d5ed..069d181674 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,21 @@ +* 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. + + *Dov Murik* + * Raise an ArgumentError when a false value for `include_blank` is passed to a required select field (to comply with the HTML5 spec). diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index b29eb48425..df8059d04e 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -12,7 +12,7 @@ module ActionView # Supported options: # # * <tt>name</tt> - Template name - # * <tt>finder</tt> - An instance of ActionView::LookupContext + # * <tt>finder</tt> - An instance of <tt>ActionView::LookupContext</tt> # * <tt>dependencies</tt> - An array of dependent views # * <tt>partial</tt> - Specifies whether the template is a partial def digest(options) diff --git a/actionview/lib/action_view/helpers/asset_url_helper.rb b/actionview/lib/action_view/helpers/asset_url_helper.rb index f36e9a108f..b19dc25025 100644 --- a/actionview/lib/action_view/helpers/asset_url_helper.rb +++ b/actionview/lib/action_view/helpers/asset_url_helper.rb @@ -121,7 +121,7 @@ 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, "Cannot pass nil as asset source" if source.nil? + raise ArgumentError, "nil is not a valid asset source" if source.nil? source = source.to_s return "" unless source.present? 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/helpers/date_helper.rb b/actionview/lib/action_view/helpers/date_helper.rb index 98ef87e427..9c8edc69a9 100644 --- a/actionview/lib/action_view/helpers/date_helper.rb +++ b/actionview/lib/action_view/helpers/date_helper.rb @@ -69,8 +69,8 @@ module ActionView # distance_of_time_in_words(to_time, from_time, include_seconds: true) # => about 6 years # distance_of_time_in_words(Time.now, Time.now) # => less than a minute # - # With the <tt>scope</tt> you can define a custom scope for Rails lookup - # the translation. + # With the <tt>scope</tt> option, you can define a custom scope for Rails + # to lookup the translation. # # For example you can define the following in your locale (e.g. en.yml). # @@ -78,8 +78,8 @@ module ActionView # distance_in_words: # short: # about_x_hours: - # one: 1 hr - # other: '%{count} hr' + # one: 'an hour' + # other: '%{count} hours' # # See https://github.com/svenfuchs/rails-i18n/blob/master/rails/locale/en.yml # for more examples. @@ -87,8 +87,8 @@ module ActionView # Which will then result in the following: # # from_time = Time.now - # distance_of_time_in_words(from_time, from_time + 50.minutes, scope: 'datetime.distance_in_words.short') # => 1 hr - # distance_of_time_in_words(from_time, from_time + 3.hours, scope: 'datetime.distance_in_words.short') # => 3 hr + # distance_of_time_in_words(from_time, from_time + 50.minutes, scope: 'datetime.distance_in_words.short') # => "an hour" + # distance_of_time_in_words(from_time, from_time + 3.hours, scope: 'datetime.distance_in_words.short') # => "3 hours" def distance_of_time_in_words(from_time, to_time = 0, options = {}) options = { scope: :'datetime.distance_in_words' @@ -485,7 +485,7 @@ module ActionView # The <tt>datetime</tt> can be either a +Time+ or +DateTime+ object or an integer. # Override the field name using the <tt>:field_name</tt> option, 'second' by default. # - # my_time = Time.now + 16.minutes + # my_time = Time.now + 16.seconds # # # Generates a select field for seconds that defaults to the seconds for the time in my_time. # select_second(my_time) diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 4452dcfed5..817b30dd2f 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -6,7 +6,7 @@ require 'action_view/template/resolver' module ActionView # = Action View Lookup Context # - # LookupContext is the object responsible to hold all information required to lookup + # <tt>LookupContext</tt> is the object responsible to hold all information required to lookup # templates, i.e. view paths and details. The LookupContext is also responsible to # generate a key, given to view paths, used in the resolver cache lookup. Since # this key is generated just once during the request, it speeds up all cache accesses. diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index b751bca31e..780fdabbd1 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -348,8 +348,6 @@ module ActionView content end - private - # Sets up instance variables needed for rendering a partial. This method # finds the options and details and extracts them. The method also contains # logic that handles the type of object passed in as the partial. 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/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index 1e8e7415d1..91b7e845d6 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -59,7 +59,7 @@ module ActionView @_view_context_class ||= self.class.view_context_class end - # An instance of a view class. The default view class is ActionView::Base + # An instance of a view class. The default view class is ActionView::Base. # # The view class must have the following methods: # View.new[lookup_context, assigns, controller] diff --git a/actionview/lib/action_view/template/handlers/erb.rb b/actionview/lib/action_view/template/handlers/erb.rb index 88a8570706..da96347e4d 100644 --- a/actionview/lib/action_view/template/handlers/erb.rb +++ b/actionview/lib/action_view/template/handlers/erb.rb @@ -138,7 +138,7 @@ module ActionView # # <% cache notification.event do %> # => nil def resource_cache_call_pattern - /\A(?:<%#.*%>\n?)?<% cache\(?\s*(\w+\.?)/ + /\A(?:<%#.*%>)*\s*<%\s*cache\(?\s*(\w+)[\s\)]/m end private diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 955118a554..2b00b0303d 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -181,9 +181,9 @@ module ActionView def query(path, details, formats) query = build_query(path, details) - template_paths = find_template_paths query + template_paths = find_template_paths(query) - template_paths.map { |template| + template_paths.map do |template| handler, format, variant = extract_handler_and_format_and_variant(template, formats) contents = File.binread(template) @@ -193,29 +193,29 @@ module ActionView :variant => variant, :updated_at => mtime(template) ) - } + end end def find_template_paths(query) - Dir[query].reject { |filename| + Dir[query].reject do |filename| File.directory?(filename) || # deals with case-insensitive file systems. !File.fnmatch(query, filename, File::FNM_EXTGLOB) - } + end end # Helper for building query glob string based on resolver's pattern. def build_query(path, details) query = @pattern.dup - prefix = path.prefix.empty? ? "" : "#{escape_entry(path.prefix)}\\1" - query.gsub!(/\:prefix(\/)?/, prefix) + prefix = path.prefix.empty? ? '' : "#{escape_entry(path.prefix)}\\1" + query.gsub!(/:prefix(\/)?/, prefix) partial = escape_entry(path.partial? ? "_#{path.name}" : path.name) - query.gsub!(/\:action/, partial) + query.gsub!(/:action/, partial) details.each do |ext, variants| - query.gsub!(/\:#{ext}/, "{#{variants.compact.uniq.join(',')}}") + query.gsub!(/:#{ext}/, "{#{variants.compact.uniq.join(',')}}") end File.expand_path(query, @path) @@ -234,7 +234,7 @@ module ActionView # from the path, or the handler, we should return the array of formats given # to the resolver. def extract_handler_and_format_and_variant(path, default_formats) - pieces = File.basename(path).split(".") + pieces = File.basename(path).split('.') pieces.shift extension = pieces.pop diff --git a/actionview/lib/action_view/view_paths.rb b/actionview/lib/action_view/view_paths.rb index 492f67f45d..ebca598337 100644 --- a/actionview/lib/action_view/view_paths.rb +++ b/actionview/lib/action_view/view_paths.rb @@ -36,8 +36,8 @@ module ActionView self.class._prefixes end - # LookupContext is the object responsible to hold all information required to lookup - # templates, i.e. view paths and details. Check ActionView::LookupContext for more + # <tt>LookupContext</tt> is the object responsible to hold all information required to lookup + # templates, i.e. view paths and details. Check <tt>ActionView::LookupContext</tt> for more # information. def lookup_context @_lookup_context ||= diff --git a/actionview/test/template/asset_tag_helper_test.rb b/actionview/test/template/asset_tag_helper_test.rb index d8a2a8bad0..01fc66bed6 100644 --- a/actionview/test/template/asset_tag_helper_test.rb +++ b/actionview/test/template/asset_tag_helper_test.rb @@ -311,8 +311,8 @@ class AssetTagHelperTest < ActionView::TestCase end def test_asset_path_tag_raises_an_error_for_nil_source - exception = assert_raise(ArgumentError) { asset_path(nil) } - assert_equal("Cannot pass nil as asset source", exception.message) + 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 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/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index aae6a9aa09..d17034e88f 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -190,6 +190,36 @@ class TestERBTemplate < ActiveSupport::TestCase assert_match(/\xFC/, e.message) end + def test_not_eligible_for_collection_caching_without_cache_call + [ + "<%= 'Hello' %>", + "<% cache_customer = 42 %>", + "<% cache customer.name do %><% end %>" + ].each do |body| + template = new_template(body, virtual_path: "test/foo/_customer") + assert_not template.eligible_for_collection_caching?, "Template #{body.inspect} should not be eligible for collection caching" + end + end + + def test_eligible_for_collection_caching_with_cache_call + [ + "<% cache customer do %><% end %>", + "<% cache(customer) do %><% end %>", + "<% cache( customer) do %><% end %>", + "<% cache( customer ) do %><% end %>", + "<%cache customer do %><% end %>", + "<% cache customer do %><% end %>", + " <% cache customer do %>\n<% end %>\n", + "<%# comment %><% cache customer do %><% end %>", + "<%# comment %>\n<% cache customer do %><% end %>", + "<%# comment\n line 2\n line 3 %>\n<% cache customer do %><% end %>", + "<%# comment 1 %>\n<%# comment 2 %>\n<% cache customer do %><% end %>" + ].each do |body| + template = new_template(body, virtual_path: "test/foo/_customer") + assert template.eligible_for_collection_caching?, "Template #{body.inspect} should be eligible for collection caching" + end + end + def with_external_encoding(encoding) old = Encoding.default_external Encoding::Converter.new old, encoding if old != encoding diff --git a/activejob/lib/active_job/queue_name.rb b/activejob/lib/active_job/queue_name.rb index 9ae0345120..65786a49ff 100644 --- a/activejob/lib/active_job/queue_name.rb +++ b/activejob/lib/active_job/queue_name.rb @@ -39,7 +39,7 @@ module ActiveJob self.queue_name_delimiter = '_' # set default delimiter to '_' end - # Returns the name of the queue the job will be run on + # Returns the name of the queue the job will be run on. def queue_name if @queue_name.is_a?(Proc) @queue_name = self.class.queue_name_from_part(instance_exec(&@queue_name)) 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/helper_methods.rb b/activemodel/lib/active_model/validations/helper_methods.rb new file mode 100644 index 0000000000..2176115334 --- /dev/null +++ b/activemodel/lib/active_model/validations/helper_methods.rb @@ -0,0 +1,13 @@ +module ActiveModel + module Validations + module HelperMethods # :nodoc: + private + def _merge_attributes(attr_names) + options = attr_names.extract_options!.symbolize_keys + attr_names.flatten! + options[:attributes] = attr_names + options + end + end + end +end 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/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb index c22a58f9e1..fb33342d30 100644 --- a/activemodel/lib/active_model/validations/length.rb +++ b/activemodel/lib/active_model/validations/length.rb @@ -1,8 +1,6 @@ require "active_support/core_ext/string/strip" module ActiveModel - - # == Active \Model Length Validator module Validations class LengthValidator < EachValidator # :nodoc: MESSAGES = { is: :wrong_length, minimum: :too_short, maximum: :too_long }.freeze @@ -100,7 +98,7 @@ module ActiveModel module HelperMethods - # Validates that the specified attribute matches the length restrictions + # Validates that the specified attributes match the length restrictions # supplied. Only one option can be used at a time: # # class Person < ActiveRecord::Base diff --git a/activemodel/lib/active_model/validations/with.rb b/activemodel/lib/active_model/validations/with.rb index a2531327bf..6de01b3392 100644 --- a/activemodel/lib/active_model/validations/with.rb +++ b/activemodel/lib/active_model/validations/with.rb @@ -1,15 +1,5 @@ module ActiveModel module Validations - module HelperMethods - private - def _merge_attributes(attr_names) - options = attr_names.extract_options!.symbolize_keys - attr_names.flatten! - options[:attributes] = attr_names - options - end - end - class WithValidator < EachValidator # :nodoc: def validate_each(record, attr, val) method_name = options[:with] 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/CHANGELOG.md b/activerecord/CHANGELOG.md index 99c78ad934..794bb779fa 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,43 @@ +* Add alternate syntax to make `change_column_default` reversible. + + User can pass in `:from` and `:to` to make `change_column_default` command + become reversible. + + Example: + + change_column_default :posts, :status, from: nil, to: "draft" + change_column_default :users, authorized, from: true, to: false + + *Prem Sichanugrist* + +* Prevent error when using `force_reload: true` on an unassigned polymorphic + belongs_to association. + + Fixes #20426. + + *James Dabbs* + +* Correctly raise `ActiveRecord::AssociationTypeMismatch` when assigning + a wrong type to a namespaced association. + + Fixes #20545. + + *Diego Carrion* + +* `validates_absence_of` respects `marked_for_destruction?`. + + Fixes #20449. + + *Yves Senn* + +* Include the `Enumerable` module in `ActiveRecord::Relation` + + *Sean Griffin & bogdan* + +* Use `Enumerable#sum` in `ActiveRecord::Relation` if a block is given. + + *Sean Griffin* + * Let `WITH` queries (Common Table Expressions) be explainable. *Vladimir Kochnev* diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 930f678ae8..7c729676a7 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -211,9 +211,12 @@ module ActiveRecord # the kind of the class of the associated objects. Meant to be used as # a sanity check when you are about to assign an associated record. def raise_on_type_mismatch!(record) - unless record.is_a?(reflection.klass) || record.is_a?(reflection.class_name.constantize) - message = "#{reflection.class_name}(##{reflection.klass.object_id}) expected, got #{record.class}(##{record.class.object_id})" - raise ActiveRecord::AssociationTypeMismatch, message + unless record.is_a?(reflection.klass) + fresh_class = reflection.class_name.safe_constantize + unless fresh_class && record.is_a?(fresh_class) + message = "#{reflection.class_name}(##{reflection.klass.object_id}) expected, got #{record.class}(##{record.class.object_id})" + raise ActiveRecord::AssociationTypeMismatch, message + end end end diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 97f4bd3811..d60dd911e1 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -10,13 +10,13 @@ module ActiveRecord # end # # class Book < ActiveRecord::Base - # # columns: title, sales + # # columns: title, sales, author_id # end # # When you load an author with all associated books Active Record will make # multiple queries like this: # - # Author.includes(:books).where(:name => ['bell hooks', 'Homer').to_a + # Author.includes(:books).where(name: ['bell hooks', 'Homer']).to_a # # => SELECT `authors`.* FROM `authors` WHERE `name` IN ('bell hooks', 'Homer') # => SELECT `books`.* FROM `books` WHERE `author_id` IN (2, 5) diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index 58d0f7d65d..bec9505bd2 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -3,7 +3,7 @@ module ActiveRecord class SingularAssociation < Association #:nodoc: # Implements the reader method, e.g. foo.bar for Foo.has_one :bar def reader(force_reload = false) - if force_reload + if force_reload && klass klass.uncached { reload } elsif !loaded? || stale_target? reload diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 158b773e11..d17e272ed1 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -592,10 +592,11 @@ module ActiveRecord # # t.change_default(:qualification, 'new') # t.change_default(:authorized, 1) + # t.change_default(:status, from: nil, to: "draft") # # See SchemaStatements#change_column_default - def change_default(column_name, default) - @base.change_column_default(name, column_name, default) + def change_default(column_name, default_or_changes) + @base.change_column_default(name, column_name, default_or_changes) end # Removes the column(s) from the table definition. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 49ffd7ccf0..e3115abe66 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -460,7 +460,12 @@ module ActiveRecord # # change_column_default(:users, :email, nil) # - def change_column_default(table_name, column_name, default) + # Passing a hash containing +:from+ and +:to+ will make this change + # reversible in migration: + # + # change_column_default(:posts, :state, from: nil, to: "draft") + # + def change_column_default(table_name, column_name, default_or_changes) raise NotImplementedError, "change_column_default is not implemented" end @@ -832,7 +837,10 @@ module ActiveRecord end def foreign_key_column_for(table_name) # :nodoc: - "#{table_name.to_s.singularize}_id" + prefix = Base.table_name_prefix + suffix = Base.table_name_suffix + name = table_name.to_s =~ /#{prefix}(.+)#{suffix}/ ? $1 : table_name.to_s + "#{name.singularize}_id" end def dump_schema_information #:nodoc: @@ -1068,6 +1076,14 @@ module ActiveRecord raise ArgumentError, "Index name '#{new_name}' on table '#{table_name}' is too long; the limit is #{allowed_index_name_length} characters" end end + + def extract_new_default_value(default_or_changes) + if default_or_changes.is_a?(Hash) && default_or_changes.has_key?(:from) && default_or_changes.has_key?(:to) + default_or_changes[:to] + else + default_or_changes + end + end end end end 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 00e3d2965b..2027492f29 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -629,7 +629,8 @@ module ActiveRecord end end - def change_column_default(table_name, column_name, default) #:nodoc: + def change_column_default(table_name, column_name, default_or_changes) #:nodoc: + default = extract_new_default_value(default_or_changes) column = column_for(table_name, column_name) change_column table_name, column_name, column.sql_type, :default => default end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 595c635fc0..d114cad16b 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -410,11 +410,12 @@ module ActiveRecord end # Changes the default value of a table column. - def change_column_default(table_name, column_name, default) # :nodoc: + def change_column_default(table_name, column_name, default_or_changes) # :nodoc: clear_cache! column = column_for(table_name, column_name) return unless column + default = extract_new_default_value(default_or_changes) alter_column_query = "ALTER TABLE #{quote_table_name(table_name)} ALTER COLUMN #{quote_column_name(column_name)} %s" if default.nil? # <tt>DEFAULT NULL</tt> results in the same behavior as <tt>DROP DEFAULT</tt>. However, PostgreSQL will diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 87129c42cf..7c809b088c 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -422,7 +422,9 @@ module ActiveRecord end end - def change_column_default(table_name, column_name, default) #:nodoc: + def change_column_default(table_name, column_name, default_or_changes) #:nodoc: + default = extract_new_default_value(default_or_changes) + alter_table(table_name) do |definition| definition[column_name].default = default end diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index af816a278e..4d597a0ab1 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -47,18 +47,21 @@ module ActiveRecord binds = " " + payload[:binds].map { |attr| render_bind(attr) }.inspect end - if odd? - name = color(name, CYAN, true) - sql = color(sql, nil, true) - else - name = color(name, MAGENTA, true) - end + name = color(name, nil, true) + sql = color(sql, sql_color(sql), true) debug " #{name} #{sql}#{binds}" end - def odd? - @odd = !@odd + def sql_color(sql) + case sql + when /\s*\Ainsert/i then GREEN + when /\s*\Aselect/i then BLUE + when /\s*\Aupdate/i then YELLOW + when /\s*\Adelete/i then RED + when /transaction\s*\Z/i then CYAN + else MAGENTA + end end def logger diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index d41b7e88d5..4cfda302ea 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -640,7 +640,8 @@ module ActiveRecord unless @connection.respond_to? :revert unless arguments.empty? || [:execute, :enable_extension, :disable_extension].include?(method) arguments[0] = proper_table_name(arguments.first, table_name_options) - if [:rename_table, :add_foreign_key].include?(method) + if [:rename_table, :add_foreign_key].include?(method) || + (method == :remove_foreign_key && !arguments.second.is_a?(Hash)) arguments[1] = proper_table_name(arguments.second, table_name_options) end end diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index b592c004aa..dcc2362397 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -72,7 +72,7 @@ module ActiveRecord [:create_table, :create_join_table, :rename_table, :add_column, :remove_column, :rename_index, :rename_column, :add_index, :remove_index, :add_timestamps, :remove_timestamps, - :change_column_default, :add_reference, :remove_reference, :transaction, + :add_reference, :remove_reference, :transaction, :drop_join_table, :drop_table, :execute_block, :enable_extension, :change_column, :execute, :remove_columns, :change_column_null, :add_foreign_key, :remove_foreign_key @@ -166,6 +166,16 @@ module ActiveRecord alias :invert_add_belongs_to :invert_add_reference alias :invert_remove_belongs_to :invert_remove_reference + def invert_change_column_default(args) + table, column, options = *args + + unless options && options.is_a?(Hash) && options.has_key?(:from) && options.has_key?(:to) + raise ActiveRecord::IrreversibleMigration, "change_column_default is only reversible if given a :from and :to option." + end + + [:change_column_default, [table, column, from: options[:to], to: options[:from]]] + end + def invert_change_column_null(args) args[2] = !args[2] [:change_column_null, args] diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 7d37313058..e4df122af6 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -15,6 +15,7 @@ module ActiveRecord VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS + CLAUSE_METHODS + include Enumerable include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches, Explain, Delegation attr_reader :table, :klass, :loaded, :predicate_builder @@ -275,38 +276,26 @@ module ActiveRecord # Returns true if there are no records. def none? - if block_given? - to_a.none? { |*block_args| yield(*block_args) } - else - empty? - end + return super if block_given? + empty? end # Returns true if there are any records. def any? - if block_given? - to_a.any? { |*block_args| yield(*block_args) } - else - !empty? - end + return super if block_given? + !empty? end # Returns true if there is exactly one record. def one? - if block_given? - to_a.one? { |*block_args| yield(*block_args) } - else - limit_value ? to_a.one? : size == 1 - end + return super if block_given? + limit_value ? to_a.one? : size == 1 end # Returns true if there is more than one record. def many? - if block_given? - to_a.many? { |*block_args| yield(*block_args) } - else - limit_value ? to_a.many? : size > 1 - end + return super if block_given? + limit_value ? to_a.many? : size > 1 end # Scope all queries to the current scope. diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 7a28a98721..df72ba7e9c 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -71,6 +71,7 @@ module ActiveRecord # # Person.sum(:age) # => 4562 def sum(*args) + return super if block_given? calculate(:sum, *args) end @@ -138,7 +139,7 @@ module ActiveRecord # # SELECT people.id, people.name FROM people # # => [[1, 'David'], [2, 'Jeremy'], [3, 'Jose']] # - # Person.pluck('DISTINCT role') + # Person.uniq.pluck(:role) # # SELECT DISTINCT role FROM people # # => ['admin', 'member', 'guest'] # diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 6020aa238f..9fef55adea 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -62,11 +62,8 @@ module ActiveRecord # Person.where(name: 'Spartacus', rating: 4).pluck(:field1, :field2) # # returns an Array of the required fields. def find(*args) - if block_given? - to_a.find(*args) { |*block_args| yield(*block_args) } - else - find_with_ids(*args) - end + return super if block_given? + find_with_ids(*args) end # Finds the first record matching the specified conditions. There diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index f85dc35e89..706c99c245 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -242,12 +242,9 @@ module ActiveRecord # Model.select(:field).first.other_field # # => ActiveModel::MissingAttributeError: missing attribute: other_field def select(*fields) - if block_given? - to_a.select { |*block_args| yield(*block_args) } - else - raise ArgumentError, 'Call this with at least one field' if fields.empty? - spawn._select!(*fields) - end + return super if block_given? + raise ArgumentError, 'Call this with at least one field' if fields.empty? + spawn._select!(*fields) end def _select!(*fields) # :nodoc: 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/activerecord/lib/active_record/type/decimal.rb b/activerecord/lib/active_record/type/decimal.rb index 867b5f75c7..f200a92d10 100644 --- a/activerecord/lib/active_record/type/decimal.rb +++ b/activerecord/lib/active_record/type/decimal.rb @@ -8,7 +8,7 @@ module ActiveRecord end def type_cast_for_schema(value) - value.to_s + value.to_s.inspect end private diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index e227212827..34d96b19fe 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -76,4 +76,5 @@ end require "active_record/validations/associated" require "active_record/validations/uniqueness" require "active_record/validations/presence" +require "active_record/validations/absence" require "active_record/validations/length" diff --git a/activerecord/lib/active_record/validations/absence.rb b/activerecord/lib/active_record/validations/absence.rb new file mode 100644 index 0000000000..2e19e6dc5c --- /dev/null +++ b/activerecord/lib/active_record/validations/absence.rb @@ -0,0 +1,24 @@ +module ActiveRecord + module Validations + class AbsenceValidator < ActiveModel::Validations::AbsenceValidator # :nodoc: + def validate_each(record, attribute, association_or_value) + return unless should_validate?(record) + if record.class._reflect_on_association(attribute) + association_or_value = Array.wrap(association_or_value).reject(&:marked_for_destruction?) + end + super + end + end + + module ClassMethods + # Validates that the specified attributes are not present (as defined by + # Object#present?). If the attribute is an association, the associated object + # is considered absent if it was marked for destruction. + # + # See ActiveModel::Validations::HelperMethods.validates_absence_of for more information. + def validates_absence_of(*attr_names) + validates_with AbsenceValidator, _merge_attributes(attr_names) + end + end + end +end diff --git a/activerecord/lib/active_record/validations/length.rb b/activerecord/lib/active_record/validations/length.rb index 5991fbad8e..69e048eef1 100644 --- a/activerecord/lib/active_record/validations/length.rb +++ b/activerecord/lib/active_record/validations/length.rb @@ -22,7 +22,10 @@ module ActiveRecord end module ClassMethods - # See <tt>ActiveModel::Validation::LengthValidator</tt> for more information. + # Validates that the specified attributes match the length restrictions supplied. + # If the attribute is an association, records that are marked for destruction are not counted. + # + # See ActiveModel::Validations::HelperMethods.validates_length_of for more information. def validates_length_of(*attr_names) validates_with LengthValidator, _merge_attributes(attr_names) end diff --git a/activerecord/lib/active_record/validations/presence.rb b/activerecord/lib/active_record/validations/presence.rb index a9b791397b..23a3985d35 100644 --- a/activerecord/lib/active_record/validations/presence.rb +++ b/activerecord/lib/active_record/validations/presence.rb @@ -1,18 +1,12 @@ module ActiveRecord module Validations class PresenceValidator < ActiveModel::Validations::PresenceValidator # :nodoc: - def validate(record) + def validate_each(record, attribute, association_or_value) return unless should_validate?(record) - super - attributes.each do |attribute| - next unless record.class._reflect_on_association(attribute) - associated_records = Array.wrap(record.send(attribute)) - - # Superclass validates presence. Ensure present records aren't about to be destroyed. - if associated_records.present? && associated_records.all?(&:marked_for_destruction?) - record.errors.add(attribute, :blank, options) - end + if record.class._reflect_on_association(attribute) + association_or_value = Array.wrap(association_or_value).reject(&:marked_for_destruction?) end + super end end diff --git a/activerecord/test/cases/adapters/postgresql/money_test.rb b/activerecord/test/cases/adapters/postgresql/money_test.rb index e3670c203d..c031178479 100644 --- a/activerecord/test/cases/adapters/postgresql/money_test.rb +++ b/activerecord/test/cases/adapters/postgresql/money_test.rb @@ -56,7 +56,7 @@ class PostgresqlMoneyTest < ActiveRecord::PostgreSQLTestCase def test_schema_dumping output = dump_table_schema("postgresql_moneys") assert_match %r{t\.money\s+"wealth",\s+scale: 2$}, output - assert_match %r{t\.money\s+"depth",\s+scale: 2,\s+default: 150\.55$}, output + assert_match %r{t\.money\s+"depth",\s+scale: 2,\s+default: "150\.55"$}, output end def test_create_and_update_money diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index 9aba2b5976..35d5581aa7 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -480,6 +480,7 @@ class DefaultsUsingMultipleSchemasAndDomainTest < ActiveRecord::PostgreSQLTestCa @connection.create_table "defaults" do |t| t.text "text_col", default: "some value" t.string "string_col", default: "some value" + t.decimal "decimal_col", default: "3.14159265358979323846" end Default.reset_column_information end @@ -498,6 +499,10 @@ class DefaultsUsingMultipleSchemasAndDomainTest < ActiveRecord::PostgreSQLTestCa assert_equal "some value", Default.new.string_col, "Default of string column was not correctly parsed" end + def test_decimal_defaults_in_new_schema_when_overriding_domain + assert_equal BigDecimal.new("3.14159265358979323846"), Default.new.decimal_col, "Default of decimal column was not correctly parsed" + end + def test_bpchar_defaults_in_new_schema_when_overriding_domain @connection.execute "ALTER TABLE defaults ADD bpchar_col bpchar DEFAULT 'some value'" Default.reset_column_information diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 039cc46b0b..ebe08c618f 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -19,6 +19,8 @@ require 'models/invoice' require 'models/line_item' require 'models/column' require 'models/record' +require 'models/admin' +require 'models/admin/user' class BelongsToAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :developers, :projects, :topics, @@ -151,6 +153,30 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_raise(ActiveRecord::AssociationTypeMismatch) { Account.find(1).firm = Project.find(1) } end + def test_raises_type_mismatch_with_namespaced_class + assert_nil defined?(Region), "This test requires that there is no top-level Region class" + + ActiveRecord::Base.connection.instance_eval do + create_table(:admin_regions) { |t| t.string :name } + add_column :admin_users, :region_id, :integer + end + Admin.const_set "RegionalUser", Class.new(Admin::User) { belongs_to(:region) } + Admin.const_set "Region", Class.new(ActiveRecord::Base) + + e = assert_raise(ActiveRecord::AssociationTypeMismatch) { + Admin::RegionalUser.new(region: 'wrong value') + } + assert_match(/^Region\([^)]+\) expected, got String\([^)]+\)$/, e.message) + ensure + Admin.send :remove_const, "Region" if Admin.const_defined?("Region") + Admin.send :remove_const, "RegionalUser" if Admin.const_defined?("RegionalUser") + + ActiveRecord::Base.connection.instance_eval do + remove_column :admin_users, :region_id if column_exists?(:admin_users, :region_id) + drop_table :admin_regions, if_exists: true + end + end + def test_natural_assignment apple = Firm.create("name" => "Apple") citibank = Account.create("credit_limit" => 10) @@ -292,9 +318,11 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase def test_polymorphic_association_class sponsor = Sponsor.new assert_nil sponsor.association(:sponsorable).send(:klass) + assert_nil sponsor.sponsorable(force_reload: true) sponsor.sponsorable_type = '' # the column doesn't have to be declared NOT NULL assert_nil sponsor.association(:sponsorable).send(:klass) + assert_nil sponsor.sponsorable(force_reload: true) sponsor.sponsorable = Member.new :name => "Bert" assert_equal Member, sponsor.association(:sponsorable).send(:klass) diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index cb4681109c..aa10817527 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -671,4 +671,14 @@ class CalculationsTest < ActiveRecord::TestCase developer.ratings.includes(comment: :post).where(posts: { id: 1 }).count end end + + def test_sum_uses_enumerable_version_when_block_is_given + block_called = false + relation = Client.all.load + + assert_no_queries do + assert_equal 0, relation.sum { block_called = true; 0 } + end + assert block_called + end end diff --git a/activerecord/test/cases/migration/columns_test.rb b/activerecord/test/cases/migration/columns_test.rb index 5fc7702dfa..ab3f584350 100644 --- a/activerecord/test/cases/migration/columns_test.rb +++ b/activerecord/test/cases/migration/columns_test.rb @@ -267,6 +267,13 @@ module ActiveRecord assert_nil TestModel.new.first_name end + def test_change_column_default_with_from_and_to + add_column "test_models", "first_name", :string + connection.change_column_default "test_models", "first_name", from: nil, to: "Tester" + + assert_equal "Tester", TestModel.new.first_name + end + def test_remove_column_no_second_parameter_raises_exception assert_raise(ArgumentError) { connection.remove_column("funny") } end diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index 90b7c6b38a..99f1dc65b0 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -169,6 +169,16 @@ module ActiveRecord end end + def test_invert_change_column_default_with_from_and_to + change = @recorder.inverse_of :change_column_default, [:table, :column, from: "old_value", to: "new_value"] + assert_equal [:change_column_default, [:table, :column, from: "new_value", to: "old_value"]], change + end + + def test_invert_change_column_default_with_from_and_to_with_boolean + change = @recorder.inverse_of :change_column_default, [:table, :column, from: true, to: false] + assert_equal [:change_column_default, [:table, :column, from: false, to: true]], change + end + def test_invert_change_column_null add = @recorder.inverse_of :change_column_null, [:table, :column, true] assert_equal [:change_column_null, [:table, :column, false]], add diff --git a/activerecord/test/cases/migration/foreign_key_test.rb b/activerecord/test/cases/migration/foreign_key_test.rb index 7f4790bf3e..72f2fa95f1 100644 --- a/activerecord/test/cases/migration/foreign_key_test.rb +++ b/activerecord/test/cases/migration/foreign_key_test.rb @@ -243,6 +243,37 @@ module ActiveRecord silence_stream($stdout) { migration.migrate(:down) } end + class CreateSchoolsAndClassesMigration < ActiveRecord::Migration + def change + create_table(:schools) + + create_table(:classes) do |t| + t.column :school_id, :integer + end + add_foreign_key :classes, :schools + end + end + + def test_add_foreign_key_with_prefix + ActiveRecord::Base.table_name_prefix = 'p_' + migration = CreateSchoolsAndClassesMigration.new + silence_stream($stdout) { migration.migrate(:up) } + assert_equal 1, @connection.foreign_keys("p_classes").size + ensure + silence_stream($stdout) { migration.migrate(:down) } + ActiveRecord::Base.table_name_prefix = nil + end + + def test_add_foreign_key_with_suffix + ActiveRecord::Base.table_name_suffix = '_s' + migration = CreateSchoolsAndClassesMigration.new + silence_stream($stdout) { migration.migrate(:up) } + assert_equal 1, @connection.foreign_keys("classes_s").size + ensure + silence_stream($stdout) { migration.migrate(:down) } + ActiveRecord::Base.table_name_suffix = nil + end + end end end diff --git a/activerecord/test/cases/reload_models_test.rb b/activerecord/test/cases/reload_models_test.rb index 0d16a3526f..431fbf1297 100644 --- a/activerecord/test/cases/reload_models_test.rb +++ b/activerecord/test/cases/reload_models_test.rb @@ -3,7 +3,7 @@ require 'models/owner' require 'models/pet' class ReloadModelsTest < ActiveRecord::TestCase - fixtures :pets + fixtures :pets, :owners def test_has_one_with_reload pet = Pet.find_by_name('parrot') diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 51170c533b..feb1c29656 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -239,7 +239,7 @@ class SchemaDumperTest < ActiveRecord::TestCase def test_schema_dump_includes_decimal_options output = dump_all_table_schema([/^[^n]/]) - assert_match %r{precision: 3,[[:space:]]+scale: 2,[[:space:]]+default: 2\.78}, output + assert_match %r{precision: 3,[[:space:]]+scale: 2,[[:space:]]+default: "2\.78"}, output end if current_adapter?(:PostgreSQLAdapter) @@ -255,7 +255,7 @@ class SchemaDumperTest < ActiveRecord::TestCase def test_schema_dump_allows_array_of_decimal_defaults output = standard_dump - assert_match %r{t\.decimal\s+"decimal_array_default",\s+default: \[1.23, 3.45\],\s+array: true}, output + assert_match %r{t\.decimal\s+"decimal_array_default",\s+default: \["1.23", "3.45"\],\s+array: true}, output end if ActiveRecord::Base.connection.supports_extensions? diff --git a/activerecord/test/cases/touch_later_test.rb b/activerecord/test/cases/touch_later_test.rb index 11804ff90b..49ada22529 100644 --- a/activerecord/test/cases/touch_later_test.rb +++ b/activerecord/test/cases/touch_later_test.rb @@ -2,8 +2,11 @@ require 'cases/helper' require 'models/invoice' require 'models/line_item' require 'models/topic' +require 'models/node' +require 'models/tree' class TouchLaterTest < ActiveRecord::TestCase + fixtures :nodes, :trees def test_touch_laster_raise_if_non_persisted invoice = Invoice.new @@ -90,4 +93,22 @@ class TouchLaterTest < ActiveRecord::TestCase invoice.touch_later end end + + def test_touching_three_deep + skip "Pending from #19324" + + previous_tree_updated_at = trees(:root).updated_at + previous_grandparent_updated_at = nodes(:grandparent).updated_at + previous_parent_updated_at = nodes(:parent_a).updated_at + previous_child_updated_at = nodes(:child_one_of_a).updated_at + + travel 5.seconds + + Node.create! parent: nodes(:child_one_of_a), tree: trees(:root) + + assert_not_equal nodes(:child_one_of_a).reload.updated_at, previous_child_updated_at + assert_not_equal nodes(:parent_a).reload.updated_at, previous_parent_updated_at + assert_not_equal nodes(:grandparent).reload.updated_at, previous_grandparent_updated_at + assert_not_equal trees(:root).reload.updated_at, previous_tree_updated_at + end end diff --git a/activerecord/test/cases/validations/absence_validation_test.rb b/activerecord/test/cases/validations/absence_validation_test.rb new file mode 100644 index 0000000000..dd43ee358c --- /dev/null +++ b/activerecord/test/cases/validations/absence_validation_test.rb @@ -0,0 +1,75 @@ +require "cases/helper" +require 'models/face' +require 'models/interest' +require 'models/man' +require 'models/topic' + +class AbsenceValidationTest < ActiveRecord::TestCase + def test_non_association + boy_klass = Class.new(Man) do + def self.name; "Boy" end + validates_absence_of :name + end + + assert boy_klass.new.valid? + assert_not boy_klass.new(name: "Alex").valid? + end + + def test_has_one_marked_for_destruction + boy_klass = Class.new(Man) do + def self.name; "Boy" end + validates_absence_of :face + end + + boy = boy_klass.new(face: Face.new) + assert_not boy.valid?, "should not be valid if has_one association is present" + assert_equal 1, boy.errors[:face].size, "should only add one error" + + boy.face.mark_for_destruction + assert boy.valid?, "should be valid if association is marked for destruction" + end + + def test_has_many_marked_for_destruction + boy_klass = Class.new(Man) do + def self.name; "Boy" end + validates_absence_of :interests + end + boy = boy_klass.new + boy.interests << [i1 = Interest.new, i2 = Interest.new] + assert_not boy.valid?, "should not be valid if has_many association is present" + + i1.mark_for_destruction + assert_not boy.valid?, "should not be valid if has_many association is present" + + i2.mark_for_destruction + assert boy.valid? + end + + def test_does_not_call_to_a_on_associations + boy_klass = Class.new(Man) do + def self.name; "Boy" end + validates_absence_of :face + end + + face_with_to_a = Face.new + def face_with_to_a.to_a; ['(/)', '(\)']; end + + assert_nothing_raised { boy_klass.new(face: face_with_to_a).valid? } + end + + def test_does_not_validate_if_parent_record_is_validate_false + repair_validations(Interest) do + Interest.validates_absence_of(:topic) + interest = Interest.new(topic: Topic.new(title: "Math")) + interest.save!(validate: false) + assert interest.persisted? + + man = Man.new(interest_ids: [interest.id]) + man.save! + + assert_equal man.interests.size, 1 + assert interest.valid? + assert man.valid? + end + end +end diff --git a/activerecord/test/fixtures/nodes.yml b/activerecord/test/fixtures/nodes.yml new file mode 100644 index 0000000000..b8bb8216ee --- /dev/null +++ b/activerecord/test/fixtures/nodes.yml @@ -0,0 +1,29 @@ +grandparent: + id: 1 + tree_id: 1 + name: Grand Parent + +parent_a: + id: 2 + tree_id: 1 + parent_id: 1 + name: Parent A + +parent_b: + id: 3 + tree_id: 1 + parent_id: 1 + name: Parent B + +child_one_of_a: + id: 4 + tree_id: 1 + parent_id: 2 + name: Child one + +child_two_of_b: + id: 5 + tree_id: 1 + parent_id: 2 + name: Child two + diff --git a/activerecord/test/fixtures/trees.yml b/activerecord/test/fixtures/trees.yml new file mode 100644 index 0000000000..9e030b7632 --- /dev/null +++ b/activerecord/test/fixtures/trees.yml @@ -0,0 +1,3 @@ +root: + id: 1 + name: The Root diff --git a/activerecord/test/models/node.rb b/activerecord/test/models/node.rb new file mode 100644 index 0000000000..07dd2dbccb --- /dev/null +++ b/activerecord/test/models/node.rb @@ -0,0 +1,5 @@ +class Node < ActiveRecord::Base + belongs_to :tree, touch: true + belongs_to :parent, class_name: 'Node', touch: true, optional: true + has_many :children, class_name: 'Node', foreign_key: :parent_id, dependent: :destroy +end diff --git a/activerecord/test/models/tree.rb b/activerecord/test/models/tree.rb new file mode 100644 index 0000000000..dc29cccc9c --- /dev/null +++ b/activerecord/test/models/tree.rb @@ -0,0 +1,3 @@ +class Tree < ActiveRecord::Base + has_many :nodes, dependent: :destroy +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index dffccc9326..6872b49ad9 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -867,6 +867,17 @@ ActiveRecord::Schema.define do t.string 'from' end + create_table :nodes, force: true do |t| + t.integer :tree_id + t.integer :parent_id + t.string :name + t.datetime :updated_at + end + create_table :trees, force: true do |t| + t.string :name + t.datetime :updated_at + end + create_table :hotels, force: true do |t| end create_table :departments, force: true do |t| diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 8e22d4ed2f..69aea6213f 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -1026,10 +1026,6 @@ class NullStoreTest < ActiveSupport::TestCase end assert_nil @cache.read("name") end - - def test_setting_nil_cache_store - assert ActiveSupport::Cache.lookup_store.class.name, ActiveSupport::Cache::NullStore.name - end end class CacheStoreLoggerTest < ActiveSupport::TestCase diff --git a/guides/assets/images/getting_started/rails_welcome.png b/guides/assets/images/getting_started/rails_welcome.png Binary files differindex 3e07c948a0..4d0cb417b7 100644 --- a/guides/assets/images/getting_started/rails_welcome.png +++ b/guides/assets/images/getting_started/rails_welcome.png 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/action_view_overview.md b/guides/source/action_view_overview.md index 09fac41491..3c1c3c7873 100644 --- a/guides/source/action_view_overview.md +++ b/guides/source/action_view_overview.md @@ -725,7 +725,7 @@ Returns a select tag with options for each of the seconds 0 through 59 with the ```ruby # Generates a select field for seconds that defaults to the seconds for the time provided -select_second(Time.now + 16.minutes) +select_second(Time.now + 16.seconds) ``` #### select_time @@ -1429,7 +1429,7 @@ This sanitize helper will HTML encode all tags and strip all attributes that are sanitize @article.body ``` -If either the `:attributes` or `:tags` options are passed, only the mentioned tags and attributes are allowed and nothing else. +If either the `:attributes` or `:tags` options are passed, only the mentioned attributes and tags are allowed and nothing else. ```ruby sanitize @article.body, tags: %w(table tr td), attributes: %w(id class style) diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index ad069a112e..0b84001ca5 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -423,21 +423,23 @@ change_column :products, :part_number, :text ``` This changes the column `part_number` on products table to be a `:text` field. +Note that `change_column` command is irreversible. Besides `change_column`, the `change_column_null` and `change_column_default` -methods are used specifically to change a not null constraint and default values of a -column. +methods are used specifically to change a not null constraint and default +values of a column. ```ruby change_column_null :products, :name, false -change_column_default :products, :approved, false +change_column_default :products, :approved, from: true, to: false ``` This sets `:name` field on products to a `NOT NULL` column and the default -value of the `:approved` field to false. +value of the `:approved` field from true to false. -TIP: Unlike `change_column` (and `change_column_default`), `change_column_null` -is reversible. +Note: You could also write the above `change_column_default` migration as +`change_column_default :products, :approved, false`, but unlike the previous +example, this would make your migration irreversible. ### Column Modifiers diff --git a/guides/source/active_support_instrumentation.md b/guides/source/active_support_instrumentation.md index 373dbbb9aa..e49abc41f4 100644 --- a/guides/source/active_support_instrumentation.md +++ b/guides/source/active_support_instrumentation.md @@ -321,17 +321,6 @@ Action Mailer } ``` -Active Resource --------------- - -### request.active_resource - -| Key | Value | -| -------------- | -------------------- | -| `:method` | HTTP method | -| `:request_uri` | Complete URI | -| `:result` | HTTP response object | - Active Support -------------- diff --git a/guides/source/association_basics.md b/guides/source/association_basics.md index 05dd0d2a04..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 = {})` @@ -2344,13 +2354,13 @@ associations, public methods, etc. Creating a car will save it in the `vehicles` table with "Car" as the `type` field: ```ruby -Car.create color: 'Red', price: 10000 +Car.create(color: 'Red', price: 10000) ``` will generate the following SQL: ```sql -INSERT INTO "vehicles" ("type", "color", "price") VALUES ("Car", "Red", 10000) +INSERT INTO "vehicles" ("type", "color", "price") VALUES ('Car', 'Red', 10000) ``` Querying car records will just search for vehicles that are cars: 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/generators.md b/guides/source/generators.md index 14f451cbc9..32bbdc554a 100644 --- a/guides/source/generators.md +++ b/guides/source/generators.md @@ -503,6 +503,14 @@ Adds a specified source to `Gemfile`: add_source "http://gems.github.com" ``` +This method also takes a block: + +```ruby +add_source "http://gems.github.com" do + gem "rspec-rails" +end +``` + ### `inject_into_file` Injects a block of code into a defined position in your file. diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index 79d4393f32..dfdc11de7a 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -298,6 +298,7 @@ Rails.application.routes.draw do # The priority is based upon order of creation: # first created -> highest priority. + # See how all your routes lay out with "rake routes". # # You can have the root of your site routed with "root" # root 'welcome#index' @@ -1545,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 @@ -1555,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/guides/source/i18n.md b/guides/source/i18n.md index 31682464ee..272a0e3623 100644 --- a/guides/source/i18n.md +++ b/guides/source/i18n.md @@ -210,7 +210,7 @@ This approach has almost the same set of advantages as setting the locale from t Getting the locale from `params` and setting it accordingly is not hard; including it in every URL and thus **passing it through the requests** is. To include an explicit option in every URL, e.g. `link_to(books_url(locale: I18n.locale))`, would be tedious and probably impossible, of course. -Rails contains infrastructure for "centralizing dynamic decisions about the URLs" in its [`ApplicationController#default_url_options`](http://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Base.html#method-i-default_url_options), which is useful precisely in this scenario: it enables us to set "defaults" for [`url_for`](http://api.rubyonrails.org/classes/ActionDispatch/Routing/UrlFor.html#method-i-url_for) and helper methods dependent on it (by implementing/overriding this method). +Rails contains infrastructure for "centralizing dynamic decisions about the URLs" in its [`ApplicationController#default_url_options`](http://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Base.html#method-i-default_url_options), which is useful precisely in this scenario: it enables us to set "defaults" for [`url_for`](http://api.rubyonrails.org/classes/ActionDispatch/Routing/UrlFor.html#method-i-url_for) and helper methods dependent on it (by implementing/overriding `default_url_options`). We can include something like this in our `ApplicationController` then: diff --git a/guides/source/rails_on_rack.md b/guides/source/rails_on_rack.md index 117017af90..1e2fe94010 100644 --- a/guides/source/rails_on_rack.md +++ b/guides/source/rails_on_rack.md @@ -58,22 +58,6 @@ class Server < ::Rack::Server end ``` -Here's how it loads the middlewares: - -```ruby -def middleware - middlewares = [] - middlewares << [::Rack::ContentLength] - Hash.new(middlewares) -end -``` - -The following table explains the usage of the loaded middlewares: - -| Middleware | Purpose | -| ----------------------- | --------------------------------------------------------------------------------- | -| `Rack::ContentLength` | Counts the number of bytes in the response and set the HTTP Content-Length header | - ### `rackup` To use `rackup` instead of Rails' `rails server`, you can put the following inside `config.ru` of your Rails application's root directory: @@ -81,8 +65,6 @@ To use `rackup` instead of Rails' `rails server`, you can put the following insi ```ruby # Rails.root/config.ru require ::File.expand_path('../config/environment', __FILE__) - -use Rack::ContentLength run Rails.application ``` @@ -327,8 +309,6 @@ Resources * [Official Rack Website](http://rack.github.io) * [Introducing Rack](http://chneukirchen.org/blog/archive/2007/02/introducing-rack.html) -* [Ruby on Rack #1 - Hello Rack!](http://m.onkey.org/ruby-on-rack-1-hello-rack) -* [Ruby on Rack #2 - The Builder](http://m.onkey.org/ruby-on-rack-2-the-builder) ### Understanding Middlewares diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 7367f0a813..7c0d4e3b31 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,7 @@ +* Adding support for passing a block to the `add_source` action of a custom generator + + *Mike Dalton*, *Hirofumi Wakasugi* + * `assert_file` understands paths with special characters (eg. `v0.1.4~alpha+nightly`). 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 diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index 70a20801a0..560a553789 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -63,12 +63,26 @@ module Rails # Add the given source to +Gemfile+ # + # If block is given, gem entries in block are wrapped into the source group. + # # add_source "http://gems.github.com/" - def add_source(source, options={}) + # + # add_source "http://gems.github.com/" do + # gem "rspec-rails" + # end + def add_source(source, options={}, &block) log :source, source in_root do - prepend_file "Gemfile", "source #{quote(source)}\n", verbose: false + if block + append_file "Gemfile", "source #{quote(source)} do", force: true + @in_group = true + instance_eval(&block) + @in_group = false + append_file "Gemfile", "\nend\n", force: true + else + prepend_file "Gemfile", "source #{quote(source)}\n", verbose: false + end end end diff --git a/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb b/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb index 0852ffce9a..95adcc06ff 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb +++ b/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb @@ -20,6 +20,6 @@ Dir["#{File.dirname(__FILE__)}/support/**/*.rb"].each { |f| require f } # Load fixtures from the engine if ActiveSupport::TestCase.respond_to?(:fixture_path=) ActiveSupport::TestCase.fixture_path = File.expand_path("../fixtures", __FILE__) - ActiveSupport::TestCase.file_fixture_path = ActiveSupport::TestCase.fixture_path + "files" + ActiveSupport::TestCase.file_fixture_path = ActiveSupport::TestCase.fixture_path + "/files" ActiveSupport::TestCase.fixtures :all end diff --git a/railties/lib/rails/test_unit/reporter.rb b/railties/lib/rails/test_unit/reporter.rb index faf551f381..09b8675cf8 100644 --- a/railties/lib/rails/test_unit/reporter.rb +++ b/railties/lib/rails/test_unit/reporter.rb @@ -7,7 +7,7 @@ module Rails self.executable = "bin/rails test" def report - return if results.empty? + return if filtered_results.empty? io.puts io.puts "Failed tests:" io.puts @@ -15,14 +15,20 @@ module Rails end def aggregated_results # :nodoc: - filtered_results = results.dup - filtered_results.reject!(&:skipped?) unless options[:verbose] filtered_results.map do |result| location, line = result.method(result.name).source_location "#{self.executable} #{relative_path_for(location)}:#{line}" end.join "\n" end + def filtered_results + if options[:verbose] + results + else + results.reject(&:skipped?) + end + end + def relative_path_for(file) file.sub(/^#{Rails.root}\/?/, '') end diff --git a/railties/test/generators/actions_test.rb b/railties/test/generators/actions_test.rb index 4a4317c4f4..2857dae07e 100644 --- a/railties/test/generators/actions_test.rb +++ b/railties/test/generators/actions_test.rb @@ -47,6 +47,14 @@ class ActionsTest < Rails::Generators::TestCase assert_file 'Gemfile', /source 'http:\/\/gems\.github\.com'/ end + def test_add_source_with_block_adds_source_to_gemfile_with_gem + run_generator + action :add_source, 'http://gems.github.com' do + gem 'rspec-rails' + end + assert_file 'Gemfile', /source 'http:\/\/gems\.github\.com' do\n gem 'rspec-rails'\nend/ + end + def test_gem_should_put_gem_dependency_in_gemfile run_generator action :gem, 'will-paginate' diff --git a/railties/test/test_unit/reporter_test.rb b/railties/test/test_unit/reporter_test.rb index d619a3e515..3066ba82d6 100644 --- a/railties/test/test_unit/reporter_test.rb +++ b/railties/test/test_unit/reporter_test.rb @@ -32,6 +32,7 @@ class TestUnitReporterTest < ActiveSupport::TestCase @reporter.record(passing_test) @reporter.record(skipped_test) @reporter.report + assert_no_match 'Failed tests:', @output.string assert_rerun_snippet_count 0 end @@ -47,7 +48,6 @@ class TestUnitReporterTest < ActiveSupport::TestCase original_executable = Rails::TestUnitReporter.executable begin Rails::TestUnitReporter.executable = "bin/test" - verbose = Rails::TestUnitReporter.new @output, verbose: true @reporter.record(failed_test) @reporter.report |
