diff options
129 files changed, 1858 insertions, 816 deletions
diff --git a/.travis.yml b/.travis.yml index 8bbc7c8f7e..d686f6de03 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,6 @@ language: ruby sudo: false script: 'ci/travis.rb' before_install: - - "rvm current | grep 'jruby' && export AR_JDBC=true || echo" - gem install bundler before_script: - bundle update @@ -24,12 +23,12 @@ rvm: - 2.2 - ruby-head - rbx-2 - - jruby + - jruby-head matrix: allow_failures: - env: "GEM=ar:mysql" - rvm: rbx-2 - - rvm: jruby + - rvm: jruby-head - env: "GEM=aj" - env: "GEM=aj:integration" fast_finish: true diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 6cd0b2d15d..d545c59afc 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,15 @@ +* Add `ActionController::Renderer` to render arbitrary templates + outside controller actions. + + Its functionality is accessible through class methods `render` and + `renderer` of `ActionController::Base`. + + *Ravil Bayramgalin* + +* Support `:assigns` option when rendering with controllers/mailers. + + *Ravil Bayramgalin* + * Default headers, removed in controller actions, are no longer reapplied on the test response. diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index e977bbce99..7667e469d3 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -11,6 +11,7 @@ module ActionController autoload :Caching autoload :Metal autoload :Middleware + autoload :Renderer autoload_under "metal" do autoload :Compatibility diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 993f8e150d..ae111e4951 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -190,11 +190,15 @@ module ActionController end def dispatch(name, request) #:nodoc: + set_request!(request) + process(name) + to_a + end + + def set_request!(request) #:nodoc: @_request = request @_env = request.env @_env['action_controller.instance'] = self - process(name) - to_a end def to_a #:nodoc: diff --git a/actionpack/lib/action_controller/metal/rack_delegation.rb b/actionpack/lib/action_controller/metal/rack_delegation.rb index 545d4a7e6e..ae9d89cc8c 100644 --- a/actionpack/lib/action_controller/metal/rack_delegation.rb +++ b/actionpack/lib/action_controller/metal/rack_delegation.rb @@ -8,9 +8,15 @@ module ActionController delegate :headers, :status=, :location=, :content_type=, :status, :location, :content_type, :response_code, :to => "@_response" - def dispatch(action, request) + module ClassMethods + def build_with_env(env = {}) #:nodoc: + new.tap { |c| c.set_request! ActionDispatch::Request.new(env) } + end + end + + def set_request!(request) #:nodoc: + super set_response!(request) - super(action, request) end def response_body=(body) diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 7bbff0450a..2d15c39d88 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -4,6 +4,17 @@ module ActionController RENDER_FORMATS_IN_PRIORITY = [:body, :text, :plain, :html] + module ClassMethods + # Documentation at ActionController::Renderer#render + delegate :render, to: :renderer + + # Returns a renderer class (inherited from ActionController::Renderer) + # for the controller. + def renderer + @renderer ||= Renderer.for(self) + end + end + # Before processing, set the request formats in current controller formats. def process_action(*) #:nodoc: self.formats = request.formats.map(&:ref).compact diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb new file mode 100644 index 0000000000..e8b29c5b5e --- /dev/null +++ b/actionpack/lib/action_controller/renderer.rb @@ -0,0 +1,100 @@ +require 'active_support/core_ext/hash/keys' + +module ActionController + # ActionController::Renderer allows to render arbitrary templates + # without requirement of being in controller actions. + # + # You get a concrete renderer class by invoking ActionController::Base#renderer. + # For example, + # + # ApplicationController.renderer + # + # It allows you to call method #render directly. + # + # ApplicationController.renderer.render template: '...' + # + # You can use a shortcut on controller to replace previous example with: + # + # ApplicationController.render template: '...' + # + # #render method allows you to use any options as when rendering in controller. + # For example, + # + # FooController.render :action, locals: { ... }, assigns: { ... } + # + # The template will be rendered in a Rack environment which is accessible through + # ActionController::Renderer#env. You can set it up in two ways: + # + # * by changing renderer defaults, like + # + # ApplicationController.renderer.defaults # => hash with default Rack environment + # + # * by initializing an instance of renderer by passing it a custom environment. + # + # ApplicationController.renderer.new(method: 'post', https: true) + # + class Renderer + class_attribute :controller, :defaults + # Rack environment to render templates in. + attr_reader :env + + class << self + delegate :render, to: :new + + # Create a new renderer class for a specific controller class. + def for(controller) + Class.new self do + self.controller = controller + self.defaults = { + http_host: 'example.org', + https: false, + method: 'get', + script_name: '', + 'rack.input' => '' + } + end + end + end + + # Accepts a custom Rack environment to render templates in. + # It will be merged with ActionController::Renderer.defaults + def initialize(env = {}) + @env = normalize_keys(defaults).merge normalize_keys(env) + @env['action_dispatch.routes'] = controller._routes + end + + # Render templates with any options from ActionController::Base#render_to_string. + def render(*args) + raise 'missing controller' unless controller? + + instance = controller.build_with_env(env) + instance.render_to_string(*args) + end + + private + def normalize_keys(env) + http_header_format(env).tap do |new_env| + handle_method_key! new_env + handle_https_key! new_env + end + end + + def http_header_format(env) + env.transform_keys do |key| + key.is_a?(Symbol) ? key.to_s.upcase : key + end + end + + def handle_method_key!(env) + if method = env.delete('METHOD') + env['REQUEST_METHOD'] = method.upcase + end + end + + def handle_https_key!(env) + if env.has_key? 'HTTPS' + env['HTTPS'] = env['HTTPS'] ? 'on' : 'off' + end + end + end +end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 8b04dfaa45..f2c9e7b1a0 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1739,9 +1739,10 @@ module ActionDispatch member_name = parent_resource.member_name end - name = @scope.action_name(name_prefix, prefix, collection_name, member_name) + action_name = @scope.action_name(name_prefix, prefix, collection_name, member_name) + candidate = action_name.select(&:present?).join('_') - if candidate = name.compact.join("_").presence + unless candidate.empty? # If a name was not explicitly given, we check if it is valid # and return nil in case it isn't. Otherwise, we pass the invalid name # forward so the underlying router engine treats it and raises an exception. diff --git a/actionpack/test/controller/new_base/bare_metal_test.rb b/actionpack/test/controller/new_base/bare_metal_test.rb index 246ba099af..710c428dcc 100644 --- a/actionpack/test/controller/new_base/bare_metal_test.rb +++ b/actionpack/test/controller/new_base/bare_metal_test.rb @@ -31,6 +31,15 @@ module BareMetalTest controller.index assert_equal ["Hello world"], controller.response_body end + + test "connect a request to controller instance without dispatch" do + env = {} + controller = BareController.new + controller.set_request! ActionDispatch::Request.new(env) + assert controller.request + assert controller.response + assert env['action_controller.instance'] + end end class HeadController < ActionController::Metal diff --git a/actionpack/test/controller/renderer_test.rb b/actionpack/test/controller/renderer_test.rb new file mode 100644 index 0000000000..6d5508323b --- /dev/null +++ b/actionpack/test/controller/renderer_test.rb @@ -0,0 +1,103 @@ +require 'abstract_unit' + +class RendererTest < ActiveSupport::TestCase + test 'creating with a controller' do + controller = CommentsController + renderer = ActionController::Renderer.for controller + + assert_equal controller, renderer.controller + end + + test 'creating from a controller' do + controller = AccountsController + renderer = controller.renderer + + assert_equal controller, renderer.controller + end + + test 'rendering with a class renderer' do + renderer = ApplicationController.renderer + content = renderer.render template: 'ruby_template' + + assert_equal 'Hello from Ruby code', content + end + + test 'rendering with an instance renderer' do + renderer = ApplicationController.renderer.new + content = renderer.render file: 'test/hello_world' + + assert_equal 'Hello world!', content + end + + test 'rendering with a controller class' do + assert_equal 'Hello world!', ApplicationController.render('test/hello_world') + end + + test 'rendering with locals' do + renderer = ApplicationController.renderer + content = renderer.render template: 'test/render_file_with_locals', + locals: { secret: 'bar' } + + assert_equal "The secret is bar\n", content + end + + test 'rendering with assigns' do + renderer = ApplicationController.renderer + content = renderer.render template: 'test/render_file_with_ivar', + assigns: { secret: 'foo' } + + assert_equal "The secret is foo\n", content + end + + test 'rendering with custom env' do + renderer = ApplicationController.renderer.new method: 'post' + content = renderer.render inline: '<%= request.post? %>' + + assert_equal 'true', content + end + + test 'rendering with defaults' do + renderer = ApplicationController.renderer + renderer.defaults[:https] = true + content = renderer.render inline: '<%= request.ssl? %>' + + assert_equal 'true', content + end + + test 'same defaults from the same controller' do + defaults = ->(controller) { controller.renderer.defaults } + + assert defaults[AccountsController].equal? defaults[AccountsController] + assert_not defaults[AccountsController].equal? defaults[CommentsController] + end + + test 'rendering with different formats' do + html = 'Hello world!' + xml = "<p>Hello world!</p>\n" + + assert_equal html, render['respond_to/using_defaults'] + assert_equal xml, render['respond_to/using_defaults.xml.builder'] + assert_equal xml, render['respond_to/using_defaults', formats: :xml] + end + + test 'rendering with helpers' do + assert_equal "<p>1\n<br />2</p>", render[inline: '<%= simple_format "1\n2" %>'] + end + + test 'rendering from inherited renderer' do + inherited = Class.new ApplicationController.renderer do + defaults[:script_name] = 'script' + def render(options) + super options.merge(locals: { param: :value }) + end + end + + template = '<%= url_for controller: :foo, action: :bar, param: param %>' + assert_equal 'script/foo/bar?param=value', inherited.render(inline: template) + end + + private + def render + @render ||= ApplicationController.renderer.method(:render) + end +end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 450681c356..3373705326 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3439,6 +3439,44 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal '/bar/comments/1', comment_path('1') end + def test_resource_where_as_is_empty + draw do + resource :post, as: '' + + scope 'post', as: 'post' do + resource :comment, as: '' + end + end + + assert_equal '/post/new', new_path + assert_equal '/post/comment/new', new_post_path + end + + def test_resources_where_as_is_empty + draw do + resources :posts, as: '' + + scope 'posts', as: 'posts' do + resources :comments, as: '' + end + end + + assert_equal '/posts/new', new_path + assert_equal '/posts/comments/new', new_posts_path + end + + def test_scope_where_as_is_empty + draw do + scope 'post', as: '' do + resource :user + resources :comments + end + end + + assert_equal '/post/user/new', new_user_path + assert_equal '/post/comments/new', new_comment_path + end + private def draw(&block) diff --git a/actionview/lib/action_view/helpers/date_helper.rb b/actionview/lib/action_view/helpers/date_helper.rb index 4b4f0ae577..46ce7cf0be 100644 --- a/actionview/lib/action_view/helpers/date_helper.rb +++ b/actionview/lib/action_view/helpers/date_helper.rb @@ -177,6 +177,8 @@ module ActionView # and +:name+ (string). A format string would be something like "%{name} (%<number>02d)" for example. # See <tt>Kernel.sprintf</tt> for documentation on format sequences. # * <tt>:date_separator</tt> - Specifies a string to separate the date fields. Default is "" (i.e. nothing). + # * <tt>:time_separator</tt> - Specifies a string to separate the time fields. Default is "" (i.e. nothing). + # * <tt>:datetime_separator</tt>- Specifies a string to separate the date and time fields. Default is "" (i.e. nothing). # * <tt>:start_year</tt> - Set the start year for the year select. Default is <tt>Date.today.year - 5</tt> if # you are creating new record. While editing existing record, <tt>:start_year</tt> defaults to # the current selected year minus 5. diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index 93c04fbec6..8bb243b8b7 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -777,10 +777,10 @@ module ActionView # # => <input id="quantity" name="quantity" min="1" max="9" type="number" /> # # number_field_tag 'quantity', nil, min: 1, max: 10 - # # => <input id="quantity" name="quantity" min="1" max="9" type="number" /> + # # => <input id="quantity" name="quantity" min="1" max="10" type="number" /> # # number_field_tag 'quantity', nil, min: 1, max: 10, step: 2 - # # => <input id="quantity" name="quantity" min="1" max="9" step="2" type="number" /> + # # => <input id="quantity" name="quantity" min="1" max="10" step="2" type="number" /> # # number_field_tag 'quantity', '1', class: 'special_input', disabled: true # # => <input disabled="disabled" class="special_input" id="quantity" name="quantity" type="number" value="1" /> diff --git a/actionview/lib/action_view/helpers/sanitize_helper.rb b/actionview/lib/action_view/helpers/sanitize_helper.rb index e72e85ee5f..463a4e9f60 100644 --- a/actionview/lib/action_view/helpers/sanitize_helper.rb +++ b/actionview/lib/action_view/helpers/sanitize_helper.rb @@ -8,76 +8,77 @@ module ActionView # These helper methods extend Action View making them callable within your template files. module SanitizeHelper extend ActiveSupport::Concern - # This +sanitize+ helper will HTML encode all tags and strip all attributes that - # aren't specifically allowed. + # Sanitizes HTML input, stripping all tags and attributes that aren't whitelisted. # - # It also strips href/src tags with invalid protocols, like javascript: especially. - # It does its best to counter any tricks that hackers may use, like throwing in - # unicode/ascii/hex values to get past the javascript: filters. Check out - # the extensive test suite. + # It also strips href/src attributes with unsafe protocols like + # <tt>javascript:</tt>, while also protecting against attempts to use Unicode, + # ASCII, and hex character references to work around these protocol filters. # - # <%= sanitize @article.body %> + # The default sanitizer is Rails::Html::WhiteListSanitizer. See {Rails HTML + # Sanitizers}[https://github.com/rails/rails-html-sanitizer] for more information. # - # You can add or remove tags/attributes if you want to customize it a bit. - # See ActionView::Base for full docs on the available options. You can add - # tags/attributes for single uses of +sanitize+ by passing either the - # <tt>:attributes</tt> or <tt>:tags</tt> options: + # Custom sanitization rules can also be provided. # - # Normal Use - # - # <%= sanitize @article.body %> + # Please note that sanitizing user-provided text does not guarantee that the + # resulting markup is valid or even well-formed. For example, the output may still + # contain unescaped characters like <tt><</tt>, <tt>></tt>, or <tt>&</tt>. # - # Custom Use - Custom Scrubber - # (supply a Loofah::Scrubber that does the sanitization) + # ==== Options # - # scrubber can either wrap a block: - # scrubber = Loofah::Scrubber.new do |node| - # node.text = "dawn of cats" - # end + # * <tt>:tags</tt> - An array of allowed tags. + # * <tt>:attributes</tt> - An array of allowed attributes. + # * <tt>:scrubber</tt> - A {Rails::Html scrubber}[https://github.com/rails/rails-html-sanitizer] + # or {Loofah::Scrubber}[https://github.com/flavorjones/loofah] object that + # defines custom sanitization rules. A custom scrubber takes precedence over + # custom tags and attributes. # - # or be a subclass of Loofah::Scrubber which responds to scrub: - # class KittyApocalypse < Loofah::Scrubber - # def scrub(node) - # node.text = "dawn of cats" - # end - # end - # scrubber = KittyApocalypse.new + # ==== Examples # - # <%= sanitize @article.body, scrubber: scrubber %> + # Normal use: # - # A custom scrubber takes precedence over custom tags and attributes - # Learn more about scrubbers here: https://github.com/flavorjones/loofah + # <%= sanitize @comment.body %> # - # Custom Use - tags and attributes - # (only the mentioned tags and attributes are allowed, nothing else) + # Providing custom whitelisted tags and attributes: # - # <%= sanitize @article.body, tags: %w(table tr td), attributes: %w(id class style) %> + # <%= sanitize @comment.body, tags: %w(strong em a), attributes: %w(href) %> # - # Add table tags to the default allowed tags + # Providing a custom Rails::Html scrubber: # - # class Application < Rails::Application - # config.action_view.sanitized_allowed_tags = ['table', 'tr', 'td'] - # end + # class CommentScrubber < Rails::Html::PermitScrubber + # def allowed_node?(node) + # !%w(form script comment blockquote).include?(node.name) + # end # - # Remove tags to the default allowed tags + # def skip_node?(node) + # node.text? + # end # - # class Application < Rails::Application - # config.after_initialize do - # ActionView::Base.sanitized_allowed_tags.delete 'div' + # def scrub_attribute?(name) + # name == 'style' # end # end # - # Change allowed default attributes + # <%= sanitize @comment.body, scrubber: CommentScrubber.new %> + # + # See {Rails HTML Sanitizer}[https://github.com/rails/rails-html-sanitizer] for + # documentation about Rails::Html scrubbers. # - # class Application < Rails::Application - # config.action_view.sanitized_allowed_attributes = ['id', 'class', 'style'] + # Providing a custom Loofah::Scrubber: + # + # scrubber = Loofah::Scrubber.new do |node| + # node.remove if node.name == 'script' # end # - # Please note that sanitizing user-provided text does not guarantee that the - # resulting markup is valid (conforming to a document type) or even well-formed. - # The output may still contain e.g. unescaped '<', '>', '&' characters and - # confuse browsers. + # <%= sanitize @comment.body, scrubber: scrubber %> + # + # See {Loofah's documentation}[https://github.com/flavorjones/loofah] for more + # information about defining custom Loofah::Scrubber objects. # + # To set the default allowed tags or attributes across your application: + # + # # In config/application.rb + # config.action_view.sanitized_allowed_tags = ['strong', 'em', 'a'] + # config.action_view.sanitized_allowed_attributes = ['href', 'title'] def sanitize(html, options = {}) self.class.white_list_sanitizer.sanitize(html, options).try(:html_safe) end @@ -87,9 +88,7 @@ module ActionView self.class.white_list_sanitizer.sanitize_css(style) end - # Strips all HTML tags from the +html+, including comments. This uses - # Nokogiri for tokenization (via Loofah) and so its HTML parsing ability - # is limited by that of Nokogiri. + # Strips all HTML tags from +html+, including comments. # # strip_tags("Strip <i>these</i> tags!") # # => Strip these tags! @@ -103,7 +102,7 @@ module ActionView self.class.full_sanitizer.sanitize(html) end - # Strips all link tags from +text+ leaving just the link text. + # Strips all link tags from +html+ leaving just the link text. # # strip_links('<a href="http://www.rubyonrails.org">Ruby on Rails</a>') # # => Ruby on Rails @@ -166,30 +165,6 @@ module ActionView def white_list_sanitizer @white_list_sanitizer ||= sanitizer_vendor.white_list_sanitizer.new end - - ## - # :method: sanitized_allowed_tags= - # - # :call-seq: sanitized_allowed_tags=(tags) - # - # Replaces the allowed tags for the +sanitize+ helper. - # - # class Application < Rails::Application - # config.action_view.sanitized_allowed_tags = ['table', 'tr', 'td'] - # end - # - - ## - # :method: sanitized_allowed_attributes= - # - # :call-seq: sanitized_allowed_attributes=(attributes) - # - # Replaces the allowed HTML attributes for the +sanitize+ helper. - # - # class Application < Rails::Application - # config.action_view.sanitized_allowed_attributes = ['onclick', 'longdesc'] - # end - # end end end diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index abd3b77c67..1e8e7415d1 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -92,12 +92,15 @@ module ActionView # Find and render a template based on the options given. # :api: private def _render_template(options) #:nodoc: - variant = options[:variant] + variant = options.delete(:variant) + assigns = options.delete(:assigns) + context = view_context + context.assign assigns if assigns lookup_context.rendered_format = nil if options[:formats] lookup_context.variants = variant if variant - view_renderer.render(view_context, options) + view_renderer.render(context, options) end # Assign the rendered format to lookup context. diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb index 563caee8a2..94f519e330 100644 --- a/actionview/test/actionpack/controller/render_test.rb +++ b/actionview/test/actionpack/controller/render_test.rb @@ -453,6 +453,10 @@ class TestController < ApplicationController render :text => "foo" end + def render_with_assigns_option + render inline: '<%= @hello %>', assigns: { hello: "world" } + end + def yield_content_for render :action => "content_for", :layout => "yield" end @@ -1102,6 +1106,11 @@ class RenderTest < ActionController::TestCase assert_equal "world", assigns["hello"] end + def test_render_text_with_assigns_option + get :render_with_assigns_option + assert_equal 'world', response.body + end + # :ported: def test_template_with_locals get :render_with_explicit_template_with_locals diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index fb4b972282..77386e5e41 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,7 +1,56 @@ +* Assigning an unknown attribute key to an `ActiveModel` instance during initialization + will now raise `ActiveModel::AttributeAssignment::UnknownAttributeError` instead of + `NoMethodError` + + ```ruby + User.new(foo: 'some value') + # => ActiveModel::AttributeAssignment::UnknownAttributeError: unknown attribute 'foo' for User. + ``` + + *Eugene Gilburg* + +* Extracted `ActiveRecord::AttributeAssignment` to `ActiveModel::AttributeAssignment` + allowing to use it for any object as an includable module + + ``` ruby + class Cat + include ActiveModel::AttributeAssignment + attr_accessor :name, :status + end + + cat = Cat.new + cat.assign_attributes(name: "Gorby", status: "yawning") + cat.name # => 'Gorby' + cat.status => 'yawning' + cat.assign_attributes(status: "sleeping") + cat.name # => 'Gorby' + cat.status => 'sleeping' + ``` + + *Bogdan Gusiev* + +* Add `ActiveModel::Errors#details` + + To be able to return type of used validator, one can now call `details` + on Errors instance: + + ```ruby + class User < ActiveRecord::Base + validates :name, presence: true + end + ``` + + ```ruby + user = User.new; user.valid?; user.errors.details + => {name: [{error: :blank}]} + ``` + + *Wojciech Wnętrzak* + * Change validates_acceptance_of to accept true by default. The default for validates_acceptance_of is now "1" and true. - In the past, only "1" was the default and you were required to add + In the past, only "1" was the default and you were required to add accept: true. * Remove deprecated `ActiveModel::Dirty#reset_#{attribute}` and diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb index 46d60db756..58fe08cc11 100644 --- a/activemodel/lib/active_model.rb +++ b/activemodel/lib/active_model.rb @@ -28,6 +28,7 @@ require 'active_model/version' module ActiveModel extend ActiveSupport::Autoload + autoload :AttributeAssignment autoload :AttributeMethods autoload :BlockValidator, 'active_model/validator' autoload :Callbacks diff --git a/activemodel/lib/active_model/attribute_assignment.rb b/activemodel/lib/active_model/attribute_assignment.rb new file mode 100644 index 0000000000..356421476c --- /dev/null +++ b/activemodel/lib/active_model/attribute_assignment.rb @@ -0,0 +1,63 @@ +require 'active_support/core_ext/hash/keys' + +module ActiveModel + module AttributeAssignment + include ActiveModel::ForbiddenAttributesProtection + + # Allows you to set all the attributes by passing in a hash of attributes with + # keys matching the attribute names. + # + # If the passed hash responds to <tt>permitted?</tt> method and the return value + # of this method is +false+ an <tt>ActiveModel::ForbiddenAttributesError</tt> + # exception is raised. + # + # class Cat + # include ActiveModel::AttributeAssignment + # attr_accessor :name, :status + # end + # + # cat = Cat.new + # cat.assign_attributes(name: "Gorby", status: "yawning") + # cat.name # => 'Gorby' + # cat.status => 'yawning' + # cat.assign_attributes(status: "sleeping") + # cat.name # => 'Gorby' + # cat.status => 'sleeping' + def assign_attributes(new_attributes) + if !new_attributes.respond_to?(:stringify_keys) + raise ArgumentError, "When assigning attributes, you must pass a hash as an argument." + end + return if new_attributes.blank? + + attributes = new_attributes.stringify_keys + _assign_attributes(sanitize_for_mass_assignment(attributes)) + end + + private + + def _assign_attributes(attributes) + attributes.each do |k, v| + _assign_attribute(k, v) + end + end + + def _assign_attribute(k, v) + if respond_to?("#{k}=") + public_send("#{k}=", v) + else + raise UnknownAttributeError.new(self, k) + end + end + + # Raised when unknown attributes are supplied via mass assignment. + class UnknownAttributeError < NoMethodError + attr_reader :record, :attribute + + def initialize(record, attribute) + @record = record + @attribute = attribute + super("unknown attribute '#{attribute}' for #{@record.class}.") + end + end + end +end diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 477edbd120..a809c72ccd 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -2,6 +2,7 @@ require 'active_support/core_ext/array/conversions' require 'active_support/core_ext/string/inflections' +require 'active_support/core_ext/object/deep_dup' module ActiveModel # == Active \Model \Errors @@ -23,7 +24,7 @@ module ActiveModel # attr_reader :errors # # def validate! - # errors.add(:name, "cannot be nil") if name.nil? + # errors.add(:name, :blank, message: "cannot be nil") if name.nil? # end # # # The following methods are needed to be minimally implemented @@ -58,8 +59,9 @@ module ActiveModel include Enumerable CALLBACKS_OPTIONS = [:if, :unless, :on, :allow_nil, :allow_blank, :strict] + MESSAGE_OPTIONS = [:message] - attr_reader :messages + attr_reader :messages, :details # Pass in the instance of the object that is using the errors object. # @@ -71,10 +73,12 @@ module ActiveModel def initialize(base) @base = base @messages = {} + @details = Hash.new { |details, attribute| details[attribute] = [] } end def initialize_dup(other) # :nodoc: @messages = other.messages.dup + @details = other.details.deep_dup super end @@ -85,6 +89,7 @@ module ActiveModel # person.errors.full_messages # => [] def clear messages.clear + details.clear end # Returns +true+ if the error messages include an error for the given key @@ -126,6 +131,7 @@ module ActiveModel # person.errors.get(:name) # => nil def delete(key) messages.delete(key) + details.delete(key) end # When passed a symbol or a name of a method, returns an array of errors @@ -149,12 +155,12 @@ module ActiveModel # Yields the attribute and the error for that attribute. If the attribute # has more than one error message, yields once for each error message. # - # person.errors.add(:name, "can't be blank") + # person.errors.add(:name, :blank, message: "can't be blank") # person.errors.each do |attribute, error| # # Will yield :name and "can't be blank" # end # - # person.errors.add(:name, "must be specified") + # person.errors.add(:name, :not_specified, message: "must be specified") # person.errors.each do |attribute, error| # # Will yield :name and "can't be blank" # # then yield :name and "must be specified" @@ -167,9 +173,9 @@ module ActiveModel # Returns the number of error messages. # - # person.errors.add(:name, "can't be blank") + # person.errors.add(:name, :blank, message: "can't be blank") # person.errors.size # => 1 - # person.errors.add(:name, "must be specified") + # person.errors.add(:name, :not_specified, message: "must be specified") # person.errors.size # => 2 def size values.flatten.size @@ -193,8 +199,8 @@ module ActiveModel # Returns an array of error messages, with the attribute name included. # - # person.errors.add(:name, "can't be blank") - # person.errors.add(:name, "must be specified") + # person.errors.add(:name, :blank, message: "can't be blank") + # person.errors.add(:name, :not_specified, message: "must be specified") # person.errors.to_a # => ["name can't be blank", "name must be specified"] def to_a full_messages @@ -202,9 +208,9 @@ module ActiveModel # Returns the number of error messages. # - # person.errors.add(:name, "can't be blank") + # person.errors.add(:name, :blank, message: "can't be blank") # person.errors.count # => 1 - # person.errors.add(:name, "must be specified") + # person.errors.add(:name, :not_specified, message: "must be specified") # person.errors.count # => 2 def count to_a.size @@ -223,8 +229,8 @@ module ActiveModel # Returns an xml formatted representation of the Errors hash. # - # person.errors.add(:name, "can't be blank") - # person.errors.add(:name, "must be specified") + # person.errors.add(:name, :blank, message: "can't be blank") + # person.errors.add(:name, :not_specified, message: "must be specified") # person.errors.to_xml # # => # # <?xml version=\"1.0\" encoding=\"UTF-8\"?> @@ -261,17 +267,20 @@ module ActiveModel end end - # Adds +message+ to the error messages on +attribute+. More than one error - # can be added to the same +attribute+. If no +message+ is supplied, - # <tt>:invalid</tt> is assumed. + # Adds +message+ to the error messages and used validator type to +details+ on +attribute+. + # More than one error can be added to the same +attribute+. + # If no +message+ is supplied, <tt>:invalid</tt> is assumed. # # person.errors.add(:name) # # => ["is invalid"] - # person.errors.add(:name, 'must be implemented') + # person.errors.add(:name, :not_implemented, message: "must be implemented") # # => ["is invalid", "must be implemented"] # # person.errors.messages - # # => {:name=>["must be implemented", "is invalid"]} + # # => {:name=>["is invalid", "must be implemented"]} + # + # person.errors.details + # # => {:name=>[{error: :not_implemented}, {error: :invalid}]} # # If +message+ is a symbol, it will be translated using the appropriate # scope (see +generate_message+). @@ -293,16 +302,22 @@ module ActiveModel # +attribute+ should be set to <tt>:base</tt> if the error is not # directly associated with a single attribute. # - # person.errors.add(:base, "either name or email must be present") + # person.errors.add(:base, :name_or_email_blank, + # message: "either name or email must be present") # person.errors.messages # # => {:base=>["either name or email must be present"]} + # person.errors.details + # # => {:base=>[{error: :name_or_email_blank}]} def add(attribute, message = :invalid, options = {}) + message = message.call if message.respond_to?(:call) + detail = normalize_detail(attribute, message, options) message = normalize_message(attribute, message, options) if exception = options[:strict] exception = ActiveModel::StrictValidationFailed if exception == true raise exception, full_message(attribute, message) end + details[attribute.to_sym] << detail self[attribute] << message end @@ -339,6 +354,7 @@ module ActiveModel # person.errors.add :name, :blank # person.errors.added? :name, :blank # => true def added?(attribute, message = :invalid, options = {}) + message = message.call if message.respond_to?(:call) message = normalize_message(attribute, message, options) self[attribute].include? message end @@ -447,12 +463,14 @@ module ActiveModel case message when Symbol generate_message(attribute, message, options.except(*CALLBACKS_OPTIONS)) - when Proc - message.call else message end end + + def normalize_detail(attribute, message, options) + { error: message }.merge(options.except(*CALLBACKS_OPTIONS + MESSAGE_OPTIONS)) + end end # Raised when a validation cannot be corrected by end users and are considered diff --git a/activemodel/lib/active_model/model.rb b/activemodel/lib/active_model/model.rb index d51d6ddcc9..dac8d549a7 100644 --- a/activemodel/lib/active_model/model.rb +++ b/activemodel/lib/active_model/model.rb @@ -57,6 +57,7 @@ module ActiveModel # (see below). module Model extend ActiveSupport::Concern + include ActiveModel::AttributeAssignment include ActiveModel::Validations include ActiveModel::Conversion @@ -75,10 +76,8 @@ module ActiveModel # person = Person.new(name: 'bob', age: '18') # person.name # => "bob" # person.age # => "18" - def initialize(params={}) - params.each do |attr, value| - self.public_send("#{attr}=", value) - end if params + def initialize(attributes={}) + assign_attributes(attributes) if attributes super() end diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index ada1f9a4f3..2bc3eeaa19 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/hash/except' require 'active_support/core_ext/module/introspection' +require 'active_support/core_ext/module/remove_method' module ActiveModel class Name diff --git a/activemodel/test/cases/attribute_assignment_test.rb b/activemodel/test/cases/attribute_assignment_test.rb new file mode 100644 index 0000000000..402caf21f7 --- /dev/null +++ b/activemodel/test/cases/attribute_assignment_test.rb @@ -0,0 +1,107 @@ +require "cases/helper" +require "active_support/hash_with_indifferent_access" + +class AttributeAssignmentTest < ActiveModel::TestCase + class Model + include ActiveModel::AttributeAssignment + + attr_accessor :name, :description + + def initialize(attributes = {}) + assign_attributes(attributes) + end + + def broken_attribute=(value) + raise ErrorFromAttributeWriter + end + + protected + + attr_writer :metadata + end + + class ErrorFromAttributeWriter < StandardError + end + + class ProtectedParams < ActiveSupport::HashWithIndifferentAccess + def permit! + @permitted = true + end + + def permitted? + @permitted ||= false + end + + def dup + super.tap do |duplicate| + duplicate.instance_variable_set :@permitted, permitted? + end + end + end + + test "simple assignment" do + model = Model.new + + model.assign_attributes(name: "hello", description: "world") + assert_equal "hello", model.name + assert_equal "world", model.description + end + + test "assign non-existing attribute" do + model = Model.new + error = assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) do + model.assign_attributes(hz: 1) + end + + assert_equal model, error.record + assert_equal "hz", error.attribute + end + + test "assign private attribute" do + model = Model.new + assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) do + model.assign_attributes(metadata: { a: 1 }) + end + end + + test "does not swallow errors raised in an attribute writer" do + assert_raises(ErrorFromAttributeWriter) do + Model.new(broken_attribute: 1) + end + end + + test "an ArgumentError is raised if a non-hash-like obejct is passed" do + assert_raises(ArgumentError) do + Model.new(1) + end + end + + test "forbidden attributes cannot be used for mass assignment" do + params = ProtectedParams.new(name: "Guille", description: "m") + + assert_raises(ActiveModel::ForbiddenAttributesError) do + Model.new(params) + end + end + + test "permitted attributes can be used for mass assignment" do + params = ProtectedParams.new(name: "Guille", description: "desc") + params.permit! + model = Model.new(params) + + assert_equal "Guille", model.name + assert_equal "desc", model.description + end + + test "regular hash should still be used for mass assignment" do + model = Model.new(name: "Guille", description: "m") + + assert_equal "Guille", model.name + assert_equal "m", model.description + end + + test "assigning no attributes should not raise, even if the hash is un-permitted" do + model = Model.new + assert_nil model.assign_attributes(ProtectedParams.new({})) + end +end diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb index efedd9055f..17dbb817d0 100644 --- a/activemodel/test/cases/errors_test.rb +++ b/activemodel/test/cases/errors_test.rb @@ -323,4 +323,46 @@ class ErrorsTest < ActiveModel::TestCase person.errors.expects(:generate_message).with(:name, :blank, { message: 'custom' }) person.errors.add_on_blank :name, message: 'custom' end + + test "details returns added error detail" do + person = Person.new + person.errors.add(:name, :invalid) + assert_equal({ name: [{ error: :invalid }] }, person.errors.details) + end + + test "details returns added error detail with custom option" do + person = Person.new + person.errors.add(:name, :greater_than, count: 5) + assert_equal({ name: [{ error: :greater_than, count: 5 }] }, person.errors.details) + end + + test "details do not include message option" do + person = Person.new + person.errors.add(:name, :invalid, message: "is bad") + assert_equal({ name: [{ error: :invalid }] }, person.errors.details) + end + + test "dup duplicates details" do + errors = ActiveModel::Errors.new(Person.new) + errors.add(:name, :invalid) + errors_dup = errors.dup + errors_dup.add(:name, :taken) + assert_not_equal errors_dup.details, errors.details + end + + test "delete removes details on given attribute" do + errors = ActiveModel::Errors.new(Person.new) + errors.add(:name, :invalid) + errors.delete(:name) + assert_empty errors.details[:name] + end + + test "clear removes details" do + person = Person.new + person.errors.add(:name, :invalid) + + assert_equal 1, person.errors.details.count + person.errors.clear + assert person.errors.details.empty? + end end diff --git a/activemodel/test/cases/model_test.rb b/activemodel/test/cases/model_test.rb index ee0fa26546..9a8d873ec9 100644 --- a/activemodel/test/cases/model_test.rb +++ b/activemodel/test/cases/model_test.rb @@ -70,6 +70,8 @@ class ModelTest < ActiveModel::TestCase end def test_mixin_initializer_when_args_dont_exist - assert_raises(NoMethodError) { SimpleModel.new(hello: 'world') } + assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) do + SimpleModel.new(hello: 'world') + end end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index bfd3073293..22b1c8e95b 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,4 +1,58 @@ -* `time` columns can now affected by `time_zone_aware_attributes`. If you have +* Integer types will no longer raise a `RangeError` when assigning an + attribute, but will instead raise when going to the database. + + Fixes several vague issues which were never reported directly. See the + commit message from the commit which added this line for some examples. + + *Sean Griffin* + +* Values which would error while being sent to the database (such as an + ASCII-8BIT string with invalid UTF-8 bytes on Sqlite3), no longer error on + assignment. They will still error when sent to the database, but you are + given the ability to re-assign it to a valid value. + + Fixes #18580. + + *Sean Griffin* + +* Don't remove join dependencies in `Relation#exists?` + + Fixes #18632. + + *Sean Griffin* + +* Invalid values assigned to a JSON column are assumed to be `nil`. + + Fixes #18629. + + *Sean Griffin* + +* Add `ActiveRecord::Base#accessed_fields`, which can be used to quickly + discover which fields were read from a model when you are looking to only + select the data you need from the database. + + *Sean Griffin* + +* Introduce the `:if_exists` option for `drop_table`. + + Example: + + drop_table(:posts, if_exists: true) + + That would execute: + + DROP TABLE IF EXISTS posts + + If the table doesn't exist, `if_exists: false` (the default) raises an + exception whereas `if_exists: true` does nothing. + + *Cody Cutrer*, *Stefan Kanev*, *Ryuta Kamizono* + +* Don't run SQL if attribute value is not changed for update_attribute method. + + *Prathamesh Sonpatki* + +* `time` columns can now get affected by `time_zone_aware_attributes`. If you have set `config.time_zone` to a value other than `'UTC'`, they will be treated as in that time zone by default in Rails 5.1. If this is not the desired behavior, you can set @@ -12,6 +66,15 @@ *Sean Griffin* +* Tests now run after_commit callbacks. You no longer have to declare + `uses_transaction ‘test name’` to test the results of an after_commit. + + after_commit callbacks run after committing a transaction whose parent + is not `joinable?`: un-nested transactions, transactions within test cases, + and transactions in `console --sandbox`. + + *arthurnn*, *Ravil Bayramgalin*, *Matthew Draper* + * `nil` as a value for a binary column in a query no longer logs as "<NULL binary data>", and instead logs as just "nil". diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index d06b7b3508..2416167834 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -2,42 +2,30 @@ module ActiveRecord module Associations class AssociationScope #:nodoc: def self.scope(association, connection) - INSTANCE.scope association, connection - end - - class BindSubstitution - def initialize(block) - @block = block - end - - def bind_value(scope, column, value, connection) - substitute = connection.substitute_at(column) - scope.bind_values += [[column, @block.call(value)]] - substitute - end + INSTANCE.scope(association, connection) end def self.create(&block) - block = block ? block : lambda { |val| val } - new BindSubstitution.new(block) + block ||= lambda { |val| val } + new(block) end - def initialize(bind_substitution) - @bind_substitution = bind_substitution + def initialize(value_transformation) + @value_transformation = value_transformation end INSTANCE = create def scope(association, connection) - klass = association.klass - reflection = association.reflection - scope = klass.unscoped - owner = association.owner + klass = association.klass + reflection = association.reflection + scope = klass.unscoped + owner = association.owner alias_tracker = AliasTracker.create connection, association.klass.table_name, klass.type_caster chain_head, chain_tail = get_chain(reflection, association, alias_tracker) scope.extending! Array(reflection.options[:extend]) - add_constraints(scope, owner, klass, reflection, connection, chain_head, chain_tail) + add_constraints(scope, owner, klass, reflection, chain_head, chain_tail) end def join_type @@ -61,43 +49,36 @@ module ActiveRecord binds end + protected + + attr_reader :value_transformation + private def join(table, constraint) table.create_join(table, table.create_on(constraint), join_type) end - def column_for(table_name, column_name, connection) - columns = connection.schema_cache.columns_hash(table_name) - columns[column_name] - end - - def bind_value(scope, column, value, connection) - @bind_substitution.bind_value scope, column, value, connection - end - - def bind(scope, table_name, column_name, value, connection) - column = column_for table_name, column_name, connection - bind_value scope, column, value, connection - end - - def last_chain_scope(scope, table, reflection, owner, connection, association_klass) + def last_chain_scope(scope, table, reflection, owner, association_klass) join_keys = reflection.join_keys(association_klass) key = join_keys.key foreign_key = join_keys.foreign_key - bind_val = bind scope, table.table_name, key.to_s, owner[foreign_key], connection - scope = scope.where(table[key].eq(bind_val)) + value = transform_value(owner[foreign_key]) + scope = scope.where(table.name => { key => value }) if reflection.type - value = owner.class.base_class.name - bind_val = bind scope, table.table_name, reflection.type, value, connection - scope = scope.where(table[reflection.type].eq(bind_val)) - else - scope + polymorphic_type = transform_value(owner.class.base_class.name) + scope = scope.where(table.name => { reflection.type => polymorphic_type }) end + + scope + end + + def transform_value(value) + value_transformation.call(value) end - def next_chain_scope(scope, table, reflection, connection, association_klass, foreign_table, next_reflection) + def next_chain_scope(scope, table, reflection, association_klass, foreign_table, next_reflection) join_keys = reflection.join_keys(association_klass) key = join_keys.key foreign_key = join_keys.foreign_key @@ -105,9 +86,8 @@ module ActiveRecord constraint = table[key].eq(foreign_table[foreign_key]) if reflection.type - value = next_reflection.klass.base_class.name - bind_val = bind scope, table.table_name, reflection.type, value, connection - scope = scope.where(table[reflection.type].eq(bind_val)) + value = transform_value(next_reflection.klass.base_class.name) + scope = scope.where(table.name => { reflection.type => value }) end scope = scope.joins(join(foreign_table, constraint)) @@ -138,10 +118,10 @@ module ActiveRecord [runtime_reflection, previous_reflection] end - def add_constraints(scope, owner, association_klass, refl, connection, chain_head, chain_tail) + def add_constraints(scope, owner, association_klass, refl, chain_head, chain_tail) owner_reflection = chain_tail table = owner_reflection.alias_name - scope = last_chain_scope(scope, table, owner_reflection, owner, connection, association_klass) + scope = last_chain_scope(scope, table, owner_reflection, owner, association_klass) reflection = chain_head loop do @@ -151,7 +131,7 @@ module ActiveRecord unless reflection == chain_tail next_reflection = reflection.next foreign_table = next_reflection.alias_name - scope = next_chain_scope(scope, table, reflection, connection, association_klass, foreign_table, next_reflection) + scope = next_chain_scope(scope, table, reflection, association_klass, foreign_table, next_reflection) end # Exclude the scope of the association itself, because that @@ -160,15 +140,14 @@ module ActiveRecord item = eval_scope(reflection.klass, scope_chain_item, owner) if scope_chain_item == refl.scope - scope.merge! item.except(:where, :includes, :bind) + scope.merge! item.except(:where, :includes) end reflection.all_includes do scope.includes! item.includes_values end - scope.where_values += item.where_values - scope.bind_values += item.bind_values + scope.where_clause += item.where_clause scope.order_values |= item.order_values end diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index c63b42e2a0..265a65c4c1 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -68,6 +68,9 @@ module ActiveRecord def increment_counter(counter_cache_name) if foreign_key_present? klass.increment_counter(counter_cache_name, target_id) + if target && !stale_target? + target.increment(counter_cache_name) + end end end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index f2c96e9a2a..4b7591e15c 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -151,6 +151,7 @@ module ActiveRecord # be chained. Since << flattens its argument list and inserts each record, # +push+ and +concat+ behave identically. def concat(*records) + records = records.flatten if owner.new_record? load_target concat_records(records) @@ -549,7 +550,7 @@ module ActiveRecord def concat_records(records, should_raise = false) result = true - records.flatten.each do |record| + records.each do |record| raise_on_type_mismatch!(record) add_to_target(record) do |rec| result &&= insert_record(rec, true, should_raise) unless owner.new_record? diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 2a782c06d0..b574185561 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -101,7 +101,7 @@ module ActiveRecord end def update_counter_in_memory(difference, reflection = reflection()) - if has_cached_counter?(reflection) + if counter_must_be_updated_by_has_many?(reflection) counter = cached_counter_attribute_name(reflection) owner[counter] += difference owner.send(:clear_attribute_changes, counter) # eww @@ -118,18 +118,28 @@ module ActiveRecord # it will be decremented twice. # # Hence this method. - def inverse_updates_counter_cache?(reflection = reflection()) + def inverse_which_updates_counter_cache(reflection = reflection()) counter_name = cached_counter_attribute_name(reflection) - inverse_updates_counter_named?(counter_name, reflection) + inverse_which_updates_counter_named(counter_name, reflection) end + alias inverse_updates_counter_cache? inverse_which_updates_counter_cache - def inverse_updates_counter_named?(counter_name, reflection = reflection()) - reflection.klass._reflections.values.any? { |inverse_reflection| + def inverse_which_updates_counter_named(counter_name, reflection) + reflection.klass._reflections.values.find { |inverse_reflection| inverse_reflection.belongs_to? && inverse_reflection.counter_cache_column == counter_name } end + def inverse_updates_counter_in_memory?(reflection) + inverse = inverse_which_updates_counter_cache(reflection) + inverse && inverse == reflection.inverse_of + end + + def counter_must_be_updated_by_has_many?(reflection) + !inverse_updates_counter_in_memory?(reflection) && has_cached_counter?(reflection) + end + def delete_count(method, scope) if method == :delete_all scope.delete_all diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index c1ef86a95b..3a184cee37 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -25,7 +25,7 @@ module ActiveRecord def join_constraints(foreign_table, foreign_klass, node, join_type, tables, scope_chain, chain) joins = [] - bind_values = [] + binds = [] tables = tables.reverse scope_chain_index = 0 @@ -66,7 +66,7 @@ module ActiveRecord end if rel && !rel.arel.constraints.empty? - bind_values.concat rel.bind_values + binds += rel.bound_attributes constraint = constraint.and rel.arel.constraints end @@ -75,7 +75,7 @@ module ActiveRecord column = klass.columns_hash[reflection.type.to_s] substitute = klass.connection.substitute_at(column) - bind_values.push [column, value] + binds << Attribute.with_cast_value(column.name, value, klass.type_for_attribute(column.name)) constraint = constraint.and table[reflection.type].eq substitute end @@ -85,7 +85,7 @@ module ActiveRecord foreign_table, foreign_klass = table, klass end - JoinInformation.new joins, bind_values + JoinInformation.new joins, binds end # Builds equality condition. diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 4358f3b581..97f4bd3811 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -89,7 +89,7 @@ module ActiveRecord # { author: :avatar } # [ :books, { author: :avatar } ] - NULL_RELATION = Struct.new(:values, :bind_values).new({}, []) + NULL_RELATION = Struct.new(:values, :where_clause, :joins_values).new({}, Relation::WhereClause.empty, []) def preload(records, associations, preload_scope = nil) records = Array.wrap(records).compact.uniq diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index afcaa5d55a..9e4a2b925c 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -131,18 +131,19 @@ module ActiveRecord def build_scope scope = klass.unscoped - values = reflection_scope.values - reflection_binds = reflection_scope.bind_values + values = reflection_scope.values preload_values = preload_scope.values - preload_binds = preload_scope.bind_values - scope.where_values = Array(values[:where]) + Array(preload_values[:where]) + scope.where_clause = reflection_scope.where_clause + preload_scope.where_clause scope.references_values = Array(values[:references]) + Array(preload_values[:references]) - scope.bind_values = (reflection_binds + preload_binds) - scope._select! preload_values[:select] || values[:select] || table[Arel.star] + scope._select! preload_values[:select] || values[:select] || table[Arel.star] scope.includes! preload_values[:includes] || values[:includes] - scope.joins! preload_values[:joins] || values[:joins] + if preload_scope.joins_values.any? + scope.joins!(preload_scope.joins_values) + else + scope.joins!(reflection_scope.joins_values) + end scope.order! preload_values[:order] || values[:order] if preload_values[:readonly] || values[:readonly] diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 12bf3ef138..56aa23b173 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -78,10 +78,9 @@ module ActiveRecord if options[:source_type] scope.where! reflection.foreign_type => options[:source_type] else - unless reflection_scope.where_values.empty? + unless reflection_scope.where_clause.empty? scope.includes_values = Array(reflection_scope.values[:includes] || options[:source]) - scope.where_values = reflection_scope.values[:where] - scope.bind_values = reflection_scope.bind_values + scope.where_clause = reflection_scope.where_clause end scope.references! reflection_scope.values[:references] diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index 09828dbd9b..3ce9cffdbc 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -18,7 +18,7 @@ module ActiveRecord reflection_scope = reflection.scope if reflection_scope && reflection_scope.arity.zero? - relation.merge!(reflection_scope) + relation = relation.merge(reflection_scope) end scope.merge!( diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb index 51e4fae62e..eadc17035e 100644 --- a/activerecord/lib/active_record/attribute.rb +++ b/activerecord/lib/active_record/attribute.rb @@ -51,7 +51,7 @@ module ActiveRecord end def changed_in_place_from?(old_value) - type.changed_in_place?(old_value, value) + has_been_read? && type.changed_in_place?(old_value, value) end def with_value_from_user(value) @@ -78,6 +78,10 @@ module ActiveRecord false end + def has_been_read? + defined?(@value) + end + def ==(other) self.class == other.class && name == other.name && @@ -152,6 +156,6 @@ module ActiveRecord false end end - private_constant :FromDatabase, :FromUser, :Null, :Uninitialized + private_constant :FromDatabase, :FromUser, :Null, :Uninitialized, :WithCastValue end end diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index bf64830417..fdc90df5b6 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -3,63 +3,32 @@ require 'active_model/forbidden_attributes_protection' module ActiveRecord module AttributeAssignment extend ActiveSupport::Concern - include ActiveModel::ForbiddenAttributesProtection - - # Allows you to set all the attributes by passing in a hash of attributes with - # keys matching the attribute names (which again matches the column names). - # - # If the passed hash responds to <tt>permitted?</tt> method and the return value - # of this method is +false+ an <tt>ActiveModel::ForbiddenAttributesError</tt> - # exception is raised. - # - # cat = Cat.new(name: "Gorby", status: "yawning") - # cat.attributes # => { "name" => "Gorby", "status" => "yawning", "created_at" => nil, "updated_at" => nil} - # cat.assign_attributes(status: "sleeping") - # cat.attributes # => { "name" => "Gorby", "status" => "sleeping", "created_at" => nil, "updated_at" => nil } - # - # New attributes will be persisted in the database when the object is saved. - # - # Aliased to <tt>attributes=</tt>. - def assign_attributes(new_attributes) - if !new_attributes.respond_to?(:stringify_keys) - raise ArgumentError, "When assigning attributes, you must pass a hash as an argument." - end - return if new_attributes.blank? + include ActiveModel::AttributeAssignment + + # Alias for `assign_attributes`. See +ActiveModel::AttributeAssignment+. + def attributes=(attributes) + assign_attributes(attributes) + end - attributes = new_attributes.stringify_keys - multi_parameter_attributes = [] - nested_parameter_attributes = [] + private - attributes = sanitize_for_mass_assignment(attributes) + def _assign_attributes(attributes) # :nodoc: + multi_parameter_attributes = {} + nested_parameter_attributes = {} attributes.each do |k, v| if k.include?("(") - multi_parameter_attributes << [ k, v ] + multi_parameter_attributes[k] = attributes.delete(k) elsif v.is_a?(Hash) - nested_parameter_attributes << [ k, v ] - else - _assign_attribute(k, v) + nested_parameter_attributes[k] = attributes.delete(k) end end + super(attributes) assign_nested_parameter_attributes(nested_parameter_attributes) unless nested_parameter_attributes.empty? assign_multiparameter_attributes(multi_parameter_attributes) unless multi_parameter_attributes.empty? end - alias attributes= assign_attributes - - private - - def _assign_attribute(k, v) - public_send("#{k}=", v) - rescue NoMethodError - if respond_to?("#{k}=") - raise - else - raise UnknownAttributeError.new(self, k) - end - end - # Assign any deferred nested attributes after the base attributes have been set. def assign_nested_parameter_attributes(pairs) pairs.each { |k, v| _assign_attribute(k, v) } diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 8f165fb1dc..6de71896ee 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -369,6 +369,39 @@ module ActiveRecord write_attribute(attr_name, value) end + # Returns the name of all database fields which have been read from this + # model. This can be useful in development mode to determine which fields + # need to be selected. For performance critical pages, selecting only the + # required fields can be an easy performance win (assuming you aren't using + # all of the fields on the model). + # + # For example: + # + # class PostsController < ActionController::Base + # after_action :print_accessed_fields, only: :index + # + # def index + # @posts = Post.all + # end + # + # private + # + # def print_accessed_fields + # p @posts.first.accessed_fields + # end + # end + # + # Which allows you to quickly change your code to: + # + # class PostsController < ActionController::Base + # def index + # @posts = Post.select(:id, :title, :author_id, :updated_at) + # end + # end + def accessed_fields + @attributes.accessed + end + protected def clone_attribute_value(reader_method, attribute_name) # :nodoc: diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index bce9c5e1e3..06d87ee01e 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -165,7 +165,7 @@ module ActiveRecord end def store_original_raw_attribute(attr_name) - original_raw_attributes[attr_name] = @attributes[attr_name].value_for_database + original_raw_attributes[attr_name] = @attributes[attr_name].value_for_database rescue nil end def store_original_raw_attributes diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb index 66fcaf6945..fdce68ce45 100644 --- a/activerecord/lib/active_record/attribute_set.rb +++ b/activerecord/lib/active_record/attribute_set.rb @@ -64,6 +64,10 @@ module ActiveRecord end end + def accessed + attributes.select { |_, attr| attr.has_been_read? }.keys + end + protected attr_reader :attributes diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index b263a89d79..faf5d632ec 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -14,7 +14,7 @@ module ActiveRecord end module ClassMethods # :nodoc: - # Defines or overrides a attribute on this model. This allows customization of + # Defines or overrides an attribute on this model. This allows customization of # Active Record's type casting behavior, as well as adding support for user defined # types. # 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 0f44c332ae..f905669a24 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -380,7 +380,7 @@ module ActiveRecord # it can be helpful to provide these in a migration's +change+ method so it can be reverted. # In that case, +options+ and the block will be used by create_table. def drop_table(table_name, options = {}) - execute "DROP TABLE #{quote_table_name(table_name)}" + execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}" end # Adds a new column to the named table. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 7535e9147a..7f738e89c9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -111,7 +111,6 @@ module ActiveRecord def rollback connection.rollback_to_savepoint(savepoint_name) super - rollback_records end def commit @@ -136,13 +135,11 @@ module ActiveRecord def rollback connection.rollback_db_transaction super - rollback_records end def commit connection.commit_db_transaction super - commit_records end end @@ -159,18 +156,28 @@ module ActiveRecord else SavepointTransaction.new(@connection, "active_record_#{@stack.size}", options) end + @stack.push(transaction) transaction end def commit_transaction - transaction = @stack.pop - transaction.commit - transaction.records.each { |r| current_transaction.add_record(r) } + inner_transaction = @stack.pop + inner_transaction.commit + + if current_transaction.joinable? + inner_transaction.records.each do |r| + current_transaction.add_record(r) + end + else + inner_transaction.commit_records + end end - def rollback_transaction - @stack.pop.rollback + def rollback_transaction(transaction = nil) + transaction ||= @stack.pop + transaction.rollback + transaction.rollback_records end def within_new_transaction(options = {}) @@ -187,7 +194,7 @@ module ActiveRecord begin commit_transaction rescue Exception - transaction.rollback unless transaction.state.completed? + rollback_transaction(transaction) unless transaction.state.completed? raise 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 e9a3c26c32..b61b717a61 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -502,7 +502,7 @@ module ActiveRecord end def drop_table(table_name, options = {}) - execute "DROP#{' TEMPORARY' if options[:temporary]} TABLE #{quote_table_name(table_name)}#{' CASCADE' if options[:force] == :cascade}" + execute "DROP#{' TEMPORARY' if options[:temporary]} TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}#{' CASCADE' if options[:force] == :cascade}" end def rename_index(table_name, old_name, new_name) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb index e12ddd9901..7dadc09a44 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb @@ -11,7 +11,7 @@ module ActiveRecord def type_cast_from_database(value) if value.is_a?(::String) - ::ActiveSupport::JSON.decode(value) + ::ActiveSupport::JSON.decode(value) rescue nil else super 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 a90adcf4aa..afd7a17c03 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -112,7 +112,7 @@ module ActiveRecord end def drop_table(table_name, options = {}) - execute "DROP TABLE #{quote_table_name(table_name)}#{' CASCADE' if options[:force] == :cascade}" + execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}#{' CASCADE' if options[:force] == :cascade}" end # Returns true if schema exists. diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 03dfd29a0a..52dce6291a 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -53,7 +53,7 @@ module ActiveRecord class SQLite3String < Type::String # :nodoc: def type_cast_for_database(value) if value.is_a?(::String) && value.encoding == Encoding::ASCII_8BIT - value.encode(Encoding::UTF_8) + value.encode(Encoding::UTF_8, undef: :replace) else super end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index a7aff9f724..4705f129f2 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -456,6 +456,7 @@ module ActiveRecord # Takes a PP and prettily prints this record to it, allowing you to get a nice result from `pp record` # when pp is required. def pretty_print(pp) + return super if custom_inspect_method_defined? pp.object_address_group(self) do if defined?(@attributes) && @attributes column_names = self.class.column_names.select { |name| has_attribute?(name) || new_record? } @@ -560,5 +561,9 @@ module ActiveRecord @attributes = @attributes.dup end end + + def custom_inspect_method_defined? + self.class.instance_method(:inspect).owner != ActiveRecord::Base.instance_method(:inspect).owner + end end end diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 7d8e0a2063..82596b63df 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -156,7 +156,7 @@ module ActiveRecord def each_counter_cached_associations _reflections.each do |name, reflection| - yield association(name) if reflection.belongs_to? && reflection.counter_cache_column + yield association(name.to_sym) if reflection.belongs_to? && reflection.counter_cache_column end end diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index fc28ab585f..d710d96a9a 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -178,18 +178,10 @@ module ActiveRecord class DangerousAttributeError < ActiveRecordError end - # Raised when unknown attributes are supplied via mass assignment. - class UnknownAttributeError < NoMethodError - - attr_reader :record, :attribute - - def initialize(record, attribute) - @record = record - @attribute = attribute.to_s - super("unknown attribute '#{attribute}' for #{@record.class}.") - end - - end + UnknownAttributeError = ActiveSupport::Deprecation::DeprecatedConstantProxy.new( # :nodoc: + 'ActiveRecord::UnknownAttributeError', + 'ActiveModel::AttributeAssignment::UnknownAttributeError' + ) # Raised when an error occurred while doing a mass assignment to an attribute through the # +attributes=+ method. The exception has an +attribute+ property that is the name of the diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 641512d323..af0b667262 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -111,17 +111,6 @@ module ActiveRecord # class Mouse < ActiveRecord::Base # self.table_name = "mice" # end - # - # Alternatively, you can override the table_name method to define your - # own computation. (Possibly using <tt>super</tt> to manipulate the default - # table name.) Example: - # - # class Post < ActiveRecord::Base - # def self.table_name - # "special_" + super - # end - # end - # Post.table_name # => "special_posts" def table_name reset_table_name unless defined?(@table_name) @table_name @@ -132,9 +121,6 @@ module ActiveRecord # class Project < ActiveRecord::Base # self.table_name = "project" # end - # - # You can also just define your own <tt>self.table_name</tt> method; see - # the documentation for ActiveRecord::Base#table_name. def table_name=(value) value = value && value.to_s diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 6306a25745..22112fe8ff 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -245,8 +245,8 @@ module ActiveRecord def update_attribute(name, value) name = name.to_s verify_readonly_attribute(name) - send("#{name}=", value) - save(validate: false) + public_send("#{name}=", value) + save(validate: false) if changed? end # Updates the attributes of the model from the passed-in hash and saves the @@ -352,7 +352,7 @@ module ActiveRecord # method toggles directly the underlying value without calling any setter. # Returns +self+. def toggle(attribute) - self[attribute] = !send("#{attribute}?") + self[attribute] = !public_send("#{attribute}?") self end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index dd78814c6a..07e27fe59e 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -1,18 +1,19 @@ # -*- coding: utf-8 -*- -require 'arel/collectors/bind' +require "arel/collectors/bind" module ActiveRecord # = Active Record Relation class Relation MULTI_VALUE_METHODS = [:includes, :eager_load, :preload, :select, :group, - :order, :joins, :where, :having, :bind, :references, + :order, :joins, :references, :extending, :unscope] - SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :from, :reordering, + SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :reordering, :reverse_order, :distinct, :create_with, :uniq] + CLAUSE_METHODS = [:where, :having, :from] INVALID_METHODS_FOR_DELETE_ALL = [:limit, :distinct, :offset, :group, :having] - VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS + VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS + CLAUSE_METHODS include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches, Explain, Delegation @@ -33,7 +34,6 @@ module ActiveRecord # This method is a hot spot, so for now, use Hash[] to dup the hash. # https://bugs.ruby-lang.org/issues/7166 @values = Hash[@values] - @values[:bind] = @values[:bind].dup if @values.key? :bind reset end @@ -342,8 +342,7 @@ module ActiveRecord stmt.wheres = arel.constraints end - bvs = arel.bind_values + bind_values - @klass.connection.update stmt, 'SQL', bvs + @klass.connection.update stmt, 'SQL', bind_values end # Updates an object (or multiple objects) and saves it to the database, if validations pass. @@ -467,8 +466,10 @@ module ActiveRecord invalid_methods = INVALID_METHODS_FOR_DELETE_ALL.select { |method| if MULTI_VALUE_METHODS.include?(method) send("#{method}_values").any? - else + elsif SINGLE_VALUE_METHODS.include?(method) send("#{method}_value") + elsif CLAUSE_METHODS.include?(method) + send("#{method}_clause").any? end } if invalid_methods.any? @@ -557,11 +558,10 @@ module ActiveRecord find_with_associations { |rel| relation = rel } end - arel = relation.arel - binds = arel.bind_values + relation.bind_values + binds = relation.bind_values binds = connection.prepare_binds_for_database(binds) binds.map! { |_, value| connection.quote(value) } - collect = visitor.accept(arel.ast, Arel::Collectors::Bind.new) + collect = visitor.accept(relation.arel.ast, Arel::Collectors::Bind.new) collect.substitute_binds(binds).join end end @@ -571,22 +571,7 @@ module ActiveRecord # User.where(name: 'Oscar').where_values_hash # # => {name: "Oscar"} def where_values_hash(relation_table_name = table_name) - equalities = where_values.grep(Arel::Nodes::Equality).find_all { |node| - node.left.relation.name == relation_table_name - } - - binds = Hash[bind_values.find_all(&:first).map { |column, v| [column.name, v] }] - - Hash[equalities.map { |where| - name = where.left.name - [name, binds.fetch(name.to_s) { - case where.right - when Array then where.right.map(&:val) - when Arel::Nodes::Casted, Arel::Nodes::Quoted - where.right.val - end - }] - }] + where_clause.to_h(relation_table_name) end def scope_for_create @@ -649,7 +634,7 @@ module ActiveRecord private def exec_queries - @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, arel.bind_values + bind_values) + @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bind_values) preload = preload_values preload += includes_values unless eager_loading? diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 1d4cb1a83b..724bba5f87 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -166,7 +166,7 @@ module ActiveRecord relation.select_values = column_names.map { |cn| columns_hash.key?(cn) ? arel_table[cn] : cn } - result = klass.connection.select_all(relation.arel, nil, relation.arel.bind_values + bind_values) + result = klass.connection.select_all(relation.arel, nil, bind_values) result.cast_values(klass.column_types) end end @@ -227,14 +227,11 @@ module ActiveRecord column_alias = column_name - bind_values = nil - if operation == "count" && (relation.limit_value || relation.offset_value) # Shortcut when limit is zero. return 0 if relation.limit_value == 0 query_builder = build_count_subquery(relation, column_name, distinct) - bind_values = query_builder.bind_values + relation.bind_values else column = aggregate_column(column_name) @@ -245,7 +242,6 @@ module ActiveRecord relation.select_values = [select_value] query_builder = relation.arel - bind_values = query_builder.bind_values + relation.bind_values end result = @klass.connection.select_all(query_builder, nil, bind_values) @@ -290,7 +286,7 @@ module ActiveRecord operation, distinct).as(aggregate_alias) ] - select_values += select_values unless having_values.empty? + select_values += select_values unless having_clause.empty? select_values.concat group_fields.zip(group_aliases).map { |field,aliaz| if field.respond_to?(:as) @@ -304,7 +300,7 @@ module ActiveRecord relation.group_values = group relation.select_values = select_values - calculated_data = @klass.connection.select_all(relation, nil, relation.arel.bind_values + bind_values) + calculated_data = @klass.connection.select_all(relation, nil, relation.bind_values) if association key_ids = calculated_data.collect { |row| row[group_aliases.first] } @@ -378,11 +374,9 @@ module ActiveRecord aliased_column = aggregate_column(column_name == :all ? 1 : column_name).as(column_alias) relation.select_values = [aliased_column] - arel = relation.arel - subquery = arel.as(subquery_alias) + subquery = relation.arel.as(subquery_alias) sm = Arel::SelectManager.new relation.engine - sm.bind_values = arel.bind_values select_value = operation_over_aggregate_column(column_alias, 'count', distinct) sm.project(select_value).from(subquery) end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 088bc203b7..77bf1217e6 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -307,11 +307,11 @@ module ActiveRecord relation = relation.where(conditions) else unless conditions == :none - relation = where(primary_key => conditions) + relation = relation.where(primary_key => conditions) end end - connection.select_value(relation, "#{name} Exists", relation.arel.bind_values + relation.bind_values) ? true : false + connection.select_value(relation, "#{name} Exists", relation.bind_values) ? true : false end # This method is called whenever no records are found with either a single @@ -365,7 +365,7 @@ module ActiveRecord [] else arel = relation.arel - rows = connection.select_all(arel, 'SQL', arel.bind_values + relation.bind_values) + rows = connection.select_all(arel, 'SQL', relation.bind_values) join_dependency.instantiate(rows, aliases) end end @@ -410,7 +410,7 @@ module ActiveRecord relation = relation.except(:select).select(values).distinct! arel = relation.arel - id_rows = @klass.connection.select_all(arel, 'SQL', arel.bind_values + relation.bind_values) + id_rows = @klass.connection.select_all(arel, 'SQL', relation.bind_values) id_rows.map {|row| row[primary_key]} end diff --git a/activerecord/lib/active_record/relation/from_clause.rb b/activerecord/lib/active_record/relation/from_clause.rb new file mode 100644 index 0000000000..a93952fa30 --- /dev/null +++ b/activerecord/lib/active_record/relation/from_clause.rb @@ -0,0 +1,32 @@ +module ActiveRecord + class Relation + class FromClause + attr_reader :value, :name + + def initialize(value, name) + @value = value + @name = name + end + + def binds + if value.is_a?(Relation) + value.bound_attributes + else + [] + end + end + + def merge(other) + self + end + + def empty? + value.nil? + end + + def self.empty + new(nil, nil) + end + end + end +end diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index afb0b208c3..65b607ff1c 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -49,9 +49,9 @@ module ActiveRecord @other = other end - NORMAL_VALUES = Relation::SINGLE_VALUE_METHODS + - Relation::MULTI_VALUE_METHODS - - [:joins, :where, :order, :bind, :reverse_order, :lock, :create_with, :reordering, :from] # :nodoc: + NORMAL_VALUES = Relation::VALUE_METHODS - + Relation::CLAUSE_METHODS - + [:joins, :order, :reverse_order, :lock, :create_with, :reordering] # :nodoc: def normal_values NORMAL_VALUES @@ -75,6 +75,7 @@ module ActiveRecord merge_multi_values merge_single_values + merge_clauses merge_joins relation @@ -107,20 +108,6 @@ module ActiveRecord end def merge_multi_values - lhs_wheres = relation.where_values - rhs_wheres = other.where_values - - lhs_binds = relation.bind_values - rhs_binds = other.bind_values - - removed, kept = partition_overwrites(lhs_wheres, rhs_wheres) - - where_values = kept + rhs_wheres - bind_values = filter_binds(lhs_binds, removed) + rhs_binds - - relation.where_values = where_values - relation.bind_values = bind_values - if other.reordering_value # override any order specified in the original relation relation.reorder! other.order_values @@ -133,36 +120,18 @@ module ActiveRecord end def merge_single_values - relation.from_value = other.from_value unless relation.from_value - relation.lock_value = other.lock_value unless relation.lock_value + relation.lock_value ||= other.lock_value unless other.create_with_value.blank? relation.create_with_value = (relation.create_with_value || {}).merge(other.create_with_value) end end - def filter_binds(lhs_binds, removed_wheres) - return lhs_binds if removed_wheres.empty? - - set = Set.new removed_wheres.map { |x| x.left.name.to_s } - lhs_binds.dup.delete_if { |col,_| set.include? col.name } - end - - # Remove equalities from the existing relation with a LHS which is - # present in the relation being merged in. - # returns [things_to_remove, things_to_keep] - def partition_overwrites(lhs_wheres, rhs_wheres) - if lhs_wheres.empty? || rhs_wheres.empty? - return [[], lhs_wheres] - end - - nodes = rhs_wheres.find_all do |w| - w.respond_to?(:operator) && w.operator == :== - end - seen = Set.new(nodes) { |node| node.left } - - lhs_wheres.partition do |w| - w.respond_to?(:operator) && w.operator == :== && seen.include?(w.left) + def merge_clauses + CLAUSE_METHODS.each do |name| + clause = relation.send("#{name}_clause") + other_clause = other.send("#{name}_clause") + relation.send("#{name}_clause=", clause.merge(other_clause)) end end end diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 567efce8ae..7b605db88c 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -28,6 +28,11 @@ module ActiveRecord expand_from_hash(attributes) end + def create_binds(attributes) + attributes = convert_dot_notation_to_hash(attributes.stringify_keys) + create_binds_for_hash(attributes) + end + def expand(column, value) # Find the foreign key when using queries such as: # Post.where(author: author) @@ -80,16 +85,43 @@ module ActiveRecord attributes.flat_map do |key, value| if value.is_a?(Hash) - builder = self.class.new(table.associated_table(key)) - builder.expand_from_hash(value) + associated_predicate_builder(key).expand_from_hash(value) else expand(key, value) end end end + + def create_binds_for_hash(attributes) + result = attributes.dup + binds = [] + + attributes.each do |column_name, value| + case value + when Hash + attrs, bvs = associated_predicate_builder(column_name).create_binds_for_hash(value) + result[column_name] = attrs + binds += bvs + when Relation + binds += value.bound_attributes + else + if can_be_bound?(column_name, value) + result[column_name] = Arel::Nodes::BindParam.new + binds << Attribute.with_cast_value(column_name.to_s, value, table.type(column_name)) + end + end + end + + [result, binds] + end + private + def associated_predicate_builder(association_name) + self.class.new(table.associated_table(association_name)) + end + def convert_dot_notation_to_hash(attributes) dot_notation = attributes.keys.select { |s| s.include?(".") } @@ -107,5 +139,11 @@ module ActiveRecord def handler_for(object) @handlers.detect { |klass, _| klass === object }.last end + + def can_be_bound?(column_name, value) + !value.nil? && + handler_for(value).is_a?(BasicObjectHandler) && + !table.associated_with?(column_name) + end end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index d6e6cb4d05..bc5f126a23 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1,6 +1,8 @@ -require 'active_support/core_ext/array/wrap' -require 'active_support/core_ext/string/filters' +require "active_record/relation/from_clause" +require "active_record/relation/where_clause" +require "active_record/relation/where_clause_factory" require 'active_model/forbidden_attributes_protection' +require 'active_support/core_ext/string/filters' module ActiveRecord module QueryMethods @@ -39,23 +41,10 @@ module ActiveRecord # User.where.not(name: "Jon", role: "admin") # # SELECT * FROM users WHERE name != 'Jon' AND role != 'admin' def not(opts, *rest) - where_value = @scope.send(:build_where, opts, rest).map do |rel| - case rel - when NilClass - raise ArgumentError, 'Invalid argument for .where.not(), got nil.' - when Arel::Nodes::In - Arel::Nodes::NotIn.new(rel.left, rel.right) - when Arel::Nodes::Equality - Arel::Nodes::NotEqual.new(rel.left, rel.right) - when String - Arel::Nodes::Not.new(Arel::Nodes::SqlLiteral.new(rel)) - else - Arel::Nodes::Not.new(rel) - end - end + where_clause = @scope.send(:where_clause_factory).build(opts, rest) @scope.references!(PredicateBuilder.references(opts)) if Hash === opts - @scope.where_values += where_value + @scope.where_clause += where_clause.invert @scope end end @@ -90,6 +79,33 @@ module ActiveRecord CODE end + Relation::CLAUSE_METHODS.each do |name| + class_eval <<-CODE, __FILE__, __LINE__ + 1 + def #{name}_clause # def where_clause + @values[:#{name}] || new_#{name}_clause # @values[:where] || new_where_clause + end # end + # + def #{name}_clause=(value) # def where_clause=(value) + assert_mutability! # assert_mutability! + @values[:#{name}] = value # @values[:where] = value + end # end + CODE + end + + def bound_attributes + from_clause.binds + arel.bind_values + where_clause.binds + having_clause.binds + end + + def bind_values + # convert to old style + bound_attributes.map do |attribute| + if attribute.name + column = ConnectionAdapters::Column.new(attribute.name, nil, attribute.type) + end + [column, attribute.value_before_type_cast] + end + end + def create_with_value # :nodoc: @values[:create_with] || {} end @@ -392,9 +408,8 @@ module ActiveRecord raise ArgumentError, "Hash arguments in .unscope(*args) must have :where as the key." end - Array(target_value).each do |val| - where_unscoping(val) - end + target_values = Array(target_value).map(&:to_s) + self.where_clause = where_clause.except(*target_values) end else raise ArgumentError, "Unrecognized scoping: #{args.inspect}. Use .unscope(where: :attribute_name) or .unscope(:order), for example." @@ -425,15 +440,6 @@ module ActiveRecord self end - def bind(value) # :nodoc: - spawn.bind!(value) - end - - def bind!(value) # :nodoc: - self.bind_values += [value] - self - end - # Returns a new relation, which is the result of filtering the current relation # according to the conditions in the arguments. # @@ -569,7 +575,7 @@ module ActiveRecord references!(PredicateBuilder.references(opts)) end - self.where_values += build_where(opts, rest) + self.where_clause += where_clause_factory.build(opts, rest) self end @@ -596,7 +602,7 @@ module ActiveRecord def having!(opts, *rest) # :nodoc: references!(PredicateBuilder.references(opts)) if Hash === opts - self.having_values += build_where(opts, rest) + self.having_clause += having_clause_factory.build(opts, rest) self end @@ -744,10 +750,7 @@ module ActiveRecord end def from!(value, subquery_name = nil) # :nodoc: - self.from_value = [value, subquery_name] - if value.is_a? Relation - self.bind_values = value.arel.bind_values + value.bind_values + bind_values - end + self.from_clause = Relation::FromClause.new(value, subquery_name) self end @@ -858,13 +861,10 @@ module ActiveRecord build_joins(arel, joins_values.flatten) unless joins_values.empty? - collapse_wheres(arel, (where_values - [''])) #TODO: Add uniq with real value comparison / ignore uniqs that have binds - - arel.having(*having_values.uniq.reject(&:blank?)) unless having_values.empty? - + arel.where(where_clause.ast) unless where_clause.empty? + arel.having(having_clause.ast) unless having_clause.empty? arel.take(connection.sanitize_limit(limit_value)) if limit_value arel.skip(offset_value.to_i) if offset_value - arel.group(*group_values.uniq.reject(&:blank?)) unless group_values.empty? build_order(arel) @@ -872,7 +872,7 @@ module ActiveRecord build_select(arel, select_values.uniq) arel.distinct(distinct_value) - arel.from(build_from) if from_value + arel.from(build_from) unless from_clause.empty? arel.lock(lock_value) if lock_value arel @@ -883,114 +883,24 @@ module ActiveRecord raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}." end - single_val_method = Relation::SINGLE_VALUE_METHODS.include?(scope) - unscope_code = "#{scope}_value#{'s' unless single_val_method}=" + clause_method = Relation::CLAUSE_METHODS.include?(scope) + multi_val_method = Relation::MULTI_VALUE_METHODS.include?(scope) + if clause_method + unscope_code = "#{scope}_clause=" + else + unscope_code = "#{scope}_value#{'s' if multi_val_method}=" + end case scope when :order result = [] - when :where - self.bind_values = [] else - result = [] unless single_val_method + result = [] if multi_val_method end self.send(unscope_code, result) end - def where_unscoping(target_value) - target_value = target_value.to_s - - where_values.reject! do |rel| - case rel - when Arel::Nodes::Between, Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality, Arel::Nodes::NotEqual, Arel::Nodes::LessThanOrEqual, Arel::Nodes::GreaterThanOrEqual - subrelation = (rel.left.kind_of?(Arel::Attributes::Attribute) ? rel.left : rel.right) - subrelation.name.to_s == target_value - end - end - - bind_values.reject! { |col,_| col.name == target_value } - end - - def custom_join_ast(table, joins) - joins = joins.reject(&:blank?) - - return [] if joins.empty? - - joins.map! do |join| - case join - when Array - join = Arel.sql(join.join(' ')) if array_of_strings?(join) - when String - join = Arel.sql(join) - end - table.create_string_join(join) - end - end - - def collapse_wheres(arel, wheres) - predicates = wheres.map do |where| - next where if ::Arel::Nodes::Equality === where - where = Arel.sql(where) if String === where - Arel::Nodes::Grouping.new(where) - end - - arel.where(Arel::Nodes::And.new(predicates)) if predicates.present? - end - - def build_where(opts, other = []) - case opts - when String, Array - [@klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))] - when Hash - opts = predicate_builder.resolve_column_aliases(opts) - - tmp_opts, bind_values = create_binds(opts) - self.bind_values += bind_values - - attributes = @klass.send(:expand_hash_conditions_for_aggregates, tmp_opts) - add_relations_to_bind_values(attributes) - - predicate_builder.build_from_hash(attributes) - else - [opts] - end - end - - def create_binds(opts) - bindable, non_binds = opts.partition do |column, value| - case value - when String, Integer, ActiveRecord::StatementCache::Substitute - @klass.columns_hash.include? column.to_s - else - false - end - end - - association_binds, non_binds = non_binds.partition do |column, value| - value.is_a?(Hash) && association_for_table(column) - end - - new_opts = {} - binds = [] - - bindable.each do |(column,value)| - binds.push [@klass.columns_hash[column.to_s], value] - new_opts[column] = connection.substitute_at(column) - end - - association_binds.each do |(column, value)| - association_relation = association_for_table(column).klass.send(:relation) - association_new_opts, association_bind = association_relation.send(:create_binds, value) - new_opts[column] = association_new_opts - binds += association_bind - end - - non_binds.each { |column,value| new_opts[column] = value } - - [new_opts, binds] - end - def association_for_table(table_name) table_name = table_name.to_s @klass._reflect_on_association(table_name) || @@ -998,7 +908,8 @@ module ActiveRecord end def build_from - opts, name = from_value + opts = from_clause.value + name = from_clause.name case opts when Relation name ||= 'subquery' @@ -1023,13 +934,14 @@ module ActiveRecord raise 'unknown class: %s' % join.class.name end end + buckets.default = [] - association_joins = buckets[:association_join] || [] - stashed_association_joins = buckets[:stashed_join] || [] - join_nodes = (buckets[:join_node] || []).uniq - string_joins = (buckets[:string_join] || []).map(&:strip).uniq + association_joins = buckets[:association_join] + stashed_association_joins = buckets[:stashed_join] + join_nodes = buckets[:join_node].uniq + string_joins = buckets[:string_join].map(&:strip).uniq - join_list = join_nodes + custom_join_ast(manager, string_joins) + join_list = join_nodes + convert_join_strings_to_ast(manager, string_joins) join_dependency = ActiveRecord::Associations::JoinDependency.new( @klass, @@ -1049,6 +961,13 @@ module ActiveRecord manager end + def convert_join_strings_to_ast(table, joins) + joins + .flatten + .reject(&:blank?) + .map { |join| table.create_string_join(Arel.sql(join)) } + end + def build_select(arel, selects) if !selects.empty? expanded_select = selects.map do |field| @@ -1083,10 +1002,6 @@ module ActiveRecord end end - def array_of_strings?(o) - o.is_a?(Array) && o.all? { |obj| obj.is_a?(String) } - end - def build_order(arel) orders = order_values.uniq orders.reject!(&:blank?) @@ -1154,16 +1069,18 @@ module ActiveRecord end end - def add_relations_to_bind_values(attributes) - if attributes.is_a?(Hash) - attributes.each_value do |value| - if value.is_a?(ActiveRecord::Relation) - self.bind_values += value.arel.bind_values + value.bind_values - else - add_relations_to_bind_values(value) - end - end - end + def new_where_clause + Relation::WhereClause.empty + end + alias new_having_clause new_where_clause + + def where_clause_factory + @where_clause_factory ||= Relation::WhereClauseFactory.new(klass, predicate_builder) + end + alias having_clause_factory where_clause_factory + + def new_from_clause + Relation::FromClause.empty end end end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 01bddea6c9..dd3610d7aa 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -58,9 +58,6 @@ module ActiveRecord # Post.order('id asc').only(:where) # discards the order condition # Post.order('id asc').only(:where, :order) # uses the specified order def only(*onlies) - if onlies.any? { |o| o == :where } - onlies << :bind - end relation_with values.slice(*onlies) end diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb new file mode 100644 index 0000000000..ae5667dfd6 --- /dev/null +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -0,0 +1,160 @@ +module ActiveRecord + class Relation + class WhereClause # :nodoc: + attr_reader :binds + + delegate :any?, :empty?, to: :predicates + + def initialize(predicates, binds) + @predicates = predicates + @binds = binds + end + + def +(other) + WhereClause.new( + predicates + other.predicates, + binds + other.binds, + ) + end + + def merge(other) + WhereClause.new( + predicates_unreferenced_by(other) + other.predicates, + non_conflicting_binds(other) + other.binds, + ) + end + + def except(*columns) + WhereClause.new( + predicates_except(columns), + binds_except(columns), + ) + end + + def to_h(table_name = nil) + equalities = predicates.grep(Arel::Nodes::Equality) + if table_name + equalities = equalities.select do |node| + node.left.relation.name == table_name + end + end + + binds = self.binds.map { |attr| [attr.name, attr.value] }.to_h + + equalities.map { |node| + name = node.left.name + [name, binds.fetch(name.to_s) { + case node.right + when Array then node.right.map(&:val) + when Arel::Nodes::Casted, Arel::Nodes::Quoted + node.right.val + end + }] + }.to_h + end + + def ast + Arel::Nodes::And.new(predicates_with_wrapped_sql_literals) + end + + def ==(other) + other.is_a?(WhereClause) && + predicates == other.predicates && + binds == other.binds + end + + def invert + WhereClause.new(inverted_predicates, binds) + end + + def self.empty + new([], []) + end + + protected + + attr_reader :predicates + + def referenced_columns + @referenced_columns ||= begin + equality_nodes = predicates.select { |n| equality_node?(n) } + Set.new(equality_nodes, &:left) + end + end + + private + + def predicates_unreferenced_by(other) + predicates.reject do |n| + equality_node?(n) && other.referenced_columns.include?(n.left) + end + end + + def equality_node?(node) + node.respond_to?(:operator) && node.operator == :== + end + + def non_conflicting_binds(other) + conflicts = referenced_columns & other.referenced_columns + conflicts.map! { |node| node.name.to_s } + binds.reject { |attr| conflicts.include?(attr.name) } + end + + def inverted_predicates + predicates.map { |node| invert_predicate(node) } + end + + def invert_predicate(node) + case node + when NilClass + raise ArgumentError, 'Invalid argument for .where.not(), got nil.' + when Arel::Nodes::In + Arel::Nodes::NotIn.new(node.left, node.right) + when Arel::Nodes::Equality + Arel::Nodes::NotEqual.new(node.left, node.right) + when String + Arel::Nodes::Not.new(Arel::Nodes::SqlLiteral.new(node)) + else + Arel::Nodes::Not.new(node) + end + end + + def predicates_except(columns) + predicates.reject do |node| + case node + when Arel::Nodes::Between, Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality, Arel::Nodes::NotEqual, Arel::Nodes::LessThanOrEqual, Arel::Nodes::GreaterThanOrEqual + subrelation = (node.left.kind_of?(Arel::Attributes::Attribute) ? node.left : node.right) + columns.include?(subrelation.name.to_s) + end + end + end + + def binds_except(columns) + binds.reject do |attr| + columns.include?(attr.name) + end + end + + def predicates_with_wrapped_sql_literals + non_empty_predicates.map do |node| + if Arel::Nodes::Equality === node + node + else + wrap_sql_literal(node) + end + end + end + + def non_empty_predicates + predicates - [''] + end + + def wrap_sql_literal(node) + if ::String === node + node = Arel.sql(node) + end + Arel::Nodes::Grouping.new(node) + end + end + end +end diff --git a/activerecord/lib/active_record/relation/where_clause_factory.rb b/activerecord/lib/active_record/relation/where_clause_factory.rb new file mode 100644 index 0000000000..0430922be3 --- /dev/null +++ b/activerecord/lib/active_record/relation/where_clause_factory.rb @@ -0,0 +1,34 @@ +module ActiveRecord + class Relation + class WhereClauseFactory + def initialize(klass, predicate_builder) + @klass = klass + @predicate_builder = predicate_builder + end + + def build(opts, other) + binds = [] + + case opts + when String, Array + parts = [klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))] + when Hash + attributes = predicate_builder.resolve_column_aliases(opts) + attributes = klass.send(:expand_hash_conditions_for_aggregates, attributes) + + attributes, binds = predicate_builder.create_binds(attributes) + + parts = predicate_builder.build_from_hash(attributes) + else + parts = [opts] + end + + WhereClause.new(parts, binds) + end + + protected + + attr_reader :klass, :predicate_builder + end + end +end diff --git a/activerecord/lib/active_record/table_metadata.rb b/activerecord/lib/active_record/table_metadata.rb index 11e33e8dfe..6c8792ee80 100644 --- a/activerecord/lib/active_record/table_metadata.rb +++ b/activerecord/lib/active_record/table_metadata.rb @@ -22,6 +22,14 @@ module ActiveRecord arel_table[column_name] end + def type(column_name) + if klass + klass.type_for_attribute(column_name.to_s) + else + Type::Value.new + end + end + def associated_with?(association_name) klass && klass._reflect_on_association(association_name) end @@ -34,7 +42,7 @@ module ActiveRecord association_klass = association.klass arel_table = association_klass.arel_table else - type_caster = TypeCaster::Connection.new(klass.connection, table_name) + type_caster = TypeCaster::Connection.new(klass, table_name) association_klass = nil arel_table = Arel::Table.new(table_name, type_caster: type_caster) end diff --git a/activerecord/lib/active_record/type/integer.rb b/activerecord/lib/active_record/type/integer.rb index 394fe008f9..90ca9f88da 100644 --- a/activerecord/lib/active_record/type/integer.rb +++ b/activerecord/lib/active_record/type/integer.rb @@ -16,13 +16,19 @@ module ActiveRecord :integer end - alias type_cast_for_database type_cast - def type_cast_from_database(value) return if value.nil? value.to_i end + def type_cast_for_database(value) + result = type_cast(value) + if result + ensure_in_range(result) + end + result + end + protected attr_reader :range @@ -34,9 +40,7 @@ module ActiveRecord when true then 1 when false then 0 else - result = value.to_i rescue nil - ensure_in_range(result) if result - result + value.to_i rescue nil end end diff --git a/activerecord/lib/active_record/type_caster/connection.rb b/activerecord/lib/active_record/type_caster/connection.rb index 9e4a130b40..1d204edb76 100644 --- a/activerecord/lib/active_record/type_caster/connection.rb +++ b/activerecord/lib/active_record/type_caster/connection.rb @@ -1,8 +1,8 @@ module ActiveRecord module TypeCaster class Connection - def initialize(connection, table_name) - @connection = connection + def initialize(klass, table_name) + @klass = klass @table_name = table_name end @@ -14,7 +14,8 @@ module ActiveRecord protected - attr_reader :connection, :table_name + attr_reader :table_name + delegate :connection, to: :@klass private diff --git a/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb b/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb index 9c49599d34..06b0cb5515 100644 --- a/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb +++ b/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb @@ -16,7 +16,7 @@ module ActiveRecord def test_initializes_schema_migrations_for_encoding_utf8mb4 smtn = ActiveRecord::Migrator.schema_migrations_table_name - connection.drop_table(smtn) if connection.table_exists?(smtn) + connection.drop_table smtn, if_exists: true config = connection.instance_variable_get(:@config) original_encoding = config[:encoding] diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb index 5f6cda1986..f5ae872483 100644 --- a/activerecord/test/cases/adapters/postgresql/json_test.rb +++ b/activerecord/test/cases/adapters/postgresql/json_test.rb @@ -179,6 +179,14 @@ module PostgresqlJSONSharedTestCases assert_equal({ 'one' => 'two', 'three' => 'four' }, json.payload) assert_not json.changed? end + + def test_assigning_invalid_json + json = JsonDataType.new + + json.payload = 'foo' + + assert_nil json.payload + end end class PostgresqlJSONTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb b/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb index c88259d274..4506e874bc 100644 --- a/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb +++ b/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb @@ -18,8 +18,8 @@ class PostgresqlTypeLookupTest < ActiveRecord::TestCase bigint_array = @connection.type_map.lookup(1016, -1, "bigint[]") big_array = [123456789123456789] - assert_raises(RangeError) { int_array.type_cast_from_user(big_array) } - assert_equal big_array, bigint_array.type_cast_from_user(big_array) + assert_raises(RangeError) { int_array.type_cast_for_database(big_array) } + assert_equal "{123456789123456789}", bigint_array.type_cast_for_database(big_array) end test "range types correctly respect registration of subtypes" do diff --git a/activerecord/test/cases/associations/association_scope_test.rb b/activerecord/test/cases/associations/association_scope_test.rb index 3e0032ec73..472e270f8c 100644 --- a/activerecord/test/cases/associations/association_scope_test.rb +++ b/activerecord/test/cases/associations/association_scope_test.rb @@ -8,12 +8,7 @@ module ActiveRecord test 'does not duplicate conditions' do scope = AssociationScope.scope(Author.new.association(:welcome_posts), Author.connection) - wheres = scope.where_values.map(&:right) - binds = scope.bind_values.map(&:last) - wheres = scope.where_values.map(&:right).reject { |node| - Arel::Nodes::BindParam === node - } - assert_equal wheres.uniq, wheres + binds = scope.where_clause.binds.map(&:value) assert_equal binds.uniq, binds end end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 17394cb6f7..a425b3ed88 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -122,14 +122,14 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase Firm.create("name" => "Apple") Client.create("name" => "Citibank", :firm_name => "Apple") citibank_result = Client.all.merge!(:where => {:name => "Citibank"}, :includes => :firm_with_primary_key).first - assert citibank_result.association_cache.key?(:firm_with_primary_key) + assert citibank_result.association(:firm_with_primary_key).loaded? end def test_eager_loading_with_primary_key_as_symbol Firm.create("name" => "Apple") Client.create("name" => "Citibank", :firm_name => "Apple") citibank_result = Client.all.merge!(:where => {:name => "Citibank"}, :includes => :firm_with_primary_key_symbols).first - assert citibank_result.association_cache.key?(:firm_with_primary_key_symbols) + assert citibank_result.association(:firm_with_primary_key_symbols).loaded? end def test_creating_the_belonging_object diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 21a45042fa..19f4384e83 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -316,16 +316,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase # would be convenient), because this would cause that scope to be applied to any callbacks etc. def test_build_and_create_should_not_happen_within_scope car = cars(:honda) - scoped_count = car.foo_bulbs.where_values.count + scope = car.foo_bulbs.where_values_hash bulb = car.foo_bulbs.build - assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count + assert_not_equal scope, bulb.scope_after_initialize.where_values_hash bulb = car.foo_bulbs.create - assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count + assert_not_equal scope, bulb.scope_after_initialize.where_values_hash bulb = car.foo_bulbs.create! - assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count + assert_not_equal scope, bulb.scope_after_initialize.where_values_hash end def test_no_sql_should_be_fired_if_association_already_loaded @@ -1307,7 +1307,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_nothing_raised { topic.destroy } end - uses_transaction :test_dependence_with_transaction_support_on_failure def test_dependence_with_transaction_support_on_failure firm = companies(:first_firm) clients = firm.clients diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 9b6757e256..3b6a4038cd 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -237,16 +237,16 @@ class HasOneAssociationsTest < ActiveRecord::TestCase def test_build_and_create_should_not_happen_within_scope pirate = pirates(:blackbeard) - scoped_count = pirate.association(:foo_bulb).scope.where_values.count + scope = pirate.association(:foo_bulb).scope.where_values_hash bulb = pirate.build_foo_bulb - assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count + assert_not_equal scope, bulb.scope_after_initialize.where_values_hash bulb = pirate.create_foo_bulb - assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count + assert_not_equal scope, bulb.scope_after_initialize.where_values_hash bulb = pirate.create_foo_bulb! - assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count + assert_not_equal scope, bulb.scope_after_initialize.where_values_hash end def test_create_association @@ -276,7 +276,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase def test_create_with_inexistent_foreign_key_failing firm = Firm.create(name: 'GlobalMegaCorp') - assert_raises(ActiveRecord::UnknownAttributeError) do + assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) do firm.create_account_with_inexistent_foreign_key end end diff --git a/activerecord/test/cases/associations/required_test.rb b/activerecord/test/cases/associations/required_test.rb index 321fb6c8dd..a6123ac432 100644 --- a/activerecord/test/cases/associations/required_test.rb +++ b/activerecord/test/cases/associations/required_test.rb @@ -18,8 +18,8 @@ class RequiredAssociationsTest < ActiveRecord::TestCase end teardown do - @connection.drop_table 'parents' if @connection.table_exists? 'parents' - @connection.drop_table 'children' if @connection.table_exists? 'children' + @connection.drop_table 'parents', if_exists: true + @connection.drop_table 'children', if_exists: true end test "belongs_to associations are not required by default" do diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 72963fd56c..de358114ab 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -141,7 +141,7 @@ class AssociationsTest < ActiveRecord::TestCase def test_association_with_references firm = companies(:first_firm) - assert_equal ['foo'], firm.association_with_references.references_values + assert_includes firm.association_with_references.references_values, 'foo' end end @@ -238,7 +238,7 @@ class AssociationProxyTest < ActiveRecord::TestCase end def test_scoped_allows_conditions - assert developers(:david).projects.merge!(where: 'foo').where_values.include?('foo') + assert developers(:david).projects.merge(where: 'foo').to_sql.include?('foo') end test "getting a scope from an association" do diff --git a/activerecord/test/cases/attribute_decorators_test.rb b/activerecord/test/cases/attribute_decorators_test.rb index 53bd58e22e..9ad02ffae8 100644 --- a/activerecord/test/cases/attribute_decorators_test.rb +++ b/activerecord/test/cases/attribute_decorators_test.rb @@ -28,7 +28,7 @@ module ActiveRecord teardown do return unless @connection - @connection.drop_table 'attribute_decorators_model' if @connection.table_exists? 'attribute_decorators_model' + @connection.drop_table 'attribute_decorators_model', if_exists: true Model.attribute_type_decorations.clear Model.reset_column_information end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 4036294121..243c90e945 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -758,12 +758,12 @@ class AttributeMethodsTest < ActiveRecord::TestCase def test_bulk_update_respects_access_control privatize("title=(value)") - assert_raise(ActiveRecord::UnknownAttributeError) { @target.new(:title => "Rants about pants") } - assert_raise(ActiveRecord::UnknownAttributeError) { @target.new.attributes = { :title => "Ants in pants" } } + assert_raise(ActiveModel::AttributeAssignment::UnknownAttributeError) { @target.new(:title => "Rants about pants") } + assert_raise(ActiveModel::AttributeAssignment::UnknownAttributeError) { @target.new.attributes = { :title => "Ants in pants" } } end def test_bulk_update_raise_unknown_attribute_error - error = assert_raises(ActiveRecord::UnknownAttributeError) { + error = assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) { Topic.new(hello: "world") } assert_instance_of Topic, error.record @@ -937,6 +937,16 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert model.id_came_from_user? end + def test_accessed_fields + model = @target.first + + assert_equal [], model.accessed_fields + + model.title + + assert_equal ["title"], model.accessed_fields + end + private def new_topic_like_ar_class(&block) diff --git a/activerecord/test/cases/attribute_set_test.rb b/activerecord/test/cases/attribute_set_test.rb index ba53612d30..8025c7c4d2 100644 --- a/activerecord/test/cases/attribute_set_test.rb +++ b/activerecord/test/cases/attribute_set_test.rb @@ -186,5 +186,16 @@ module ActiveRecord attributes.freeze assert_equal({ foo: "1" }, attributes.to_hash) end + + test "#accessed_attributes returns only attributes which have been read" do + builder = AttributeSet::Builder.new(foo: Type::Value.new, bar: Type::Value.new) + attributes = builder.build_from_database(foo: "1", bar: "2") + + assert_equal [], attributes.accessed + + attributes.fetch_value(:foo) + + assert_equal [:foo], attributes.accessed + end end end diff --git a/activerecord/test/cases/attribute_test.rb b/activerecord/test/cases/attribute_test.rb index 39a976fcc8..eac73e11e8 100644 --- a/activerecord/test/cases/attribute_test.rb +++ b/activerecord/test/cases/attribute_test.rb @@ -169,5 +169,24 @@ module ActiveRecord second = Attribute.from_user(:foo, 1, Type::Integer.new) assert_not_equal first, second end + + test "an attribute has not been read by default" do + attribute = Attribute.from_database(:foo, 1, Type::Value.new) + assert_not attribute.has_been_read? + end + + test "an attribute has been read when its value is calculated" do + attribute = Attribute.from_database(:foo, 1, Type::Value.new) + attribute.value + assert attribute.has_been_read? + end + + test "an attribute can not be mutated if it has not been read, + and skips expensive calculations" do + type_which_raises_from_all_methods = Object.new + attribute = Attribute.from_database(:foo, "bar", type_which_raises_from_all_methods) + + assert_not attribute.changed_in_place_from?("bar") + end end end diff --git a/activerecord/test/cases/attributes_test.rb b/activerecord/test/cases/attributes_test.rb index dbe1eb48db..4ddf6e7ba0 100644 --- a/activerecord/test/cases/attributes_test.rb +++ b/activerecord/test/cases/attributes_test.rb @@ -58,7 +58,7 @@ module ActiveRecord data = OverloadedType.new(non_existent_decimal: 1) assert_equal BigDecimal.new(1), data.non_existent_decimal - assert_raise ActiveRecord::UnknownAttributeError do + assert_raise ActiveModel::AttributeAssignment::UnknownAttributeError do UnoverloadedType.new(non_existent_decimal: 1) end end diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index 66663b3e0e..b6ab576d76 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -22,8 +22,8 @@ module ActiveRecord def setup super @connection = ActiveRecord::Base.connection - @subscriber = LogListener.new - @pk = Topic.columns_hash[Topic.primary_key] + @subscriber = LogListener.new + @pk = ConnectionAdapters::Column.new(Topic.primary_key, nil, Topic.type_for_attribute(Topic.primary_key)) @subscription = ActiveSupport::Notifications.subscribe('sql.active_record', @subscriber) end diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb index 670d94dc06..3ae4a6eade 100644 --- a/activerecord/test/cases/callbacks_test.rb +++ b/activerecord/test/cases/callbacks_test.rb @@ -288,7 +288,12 @@ class CallbacksTest < ActiveRecord::TestCase [ :after_save, :string ], [ :after_save, :proc ], [ :after_save, :object ], - [ :after_save, :block ] + [ :after_save, :block ], + [ :after_commit, :block ], + [ :after_commit, :object ], + [ :after_commit, :proc ], + [ :after_commit, :string ], + [ :after_commit, :method ] ], david.history end @@ -357,7 +362,12 @@ class CallbacksTest < ActiveRecord::TestCase [ :after_save, :string ], [ :after_save, :proc ], [ :after_save, :object ], - [ :after_save, :block ] + [ :after_save, :block ], + [ :after_commit, :block ], + [ :after_commit, :object ], + [ :after_commit, :proc ], + [ :after_commit, :string ], + [ :after_commit, :method ] ], david.history end @@ -408,7 +418,12 @@ class CallbacksTest < ActiveRecord::TestCase [ :after_destroy, :string ], [ :after_destroy, :proc ], [ :after_destroy, :object ], - [ :after_destroy, :block ] + [ :after_destroy, :block ], + [ :after_commit, :block ], + [ :after_commit, :object ], + [ :after_commit, :proc ], + [ :after_commit, :string ], + [ :after_commit, :method ] ], david.history end diff --git a/activerecord/test/cases/core_test.rb b/activerecord/test/cases/core_test.rb index 715d92af99..3cb98832c5 100644 --- a/activerecord/test/cases/core_test.rb +++ b/activerecord/test/cases/core_test.rb @@ -98,4 +98,15 @@ class CoreTest < ActiveRecord::TestCase assert actual.start_with?(expected.split('XXXXXX').first) assert actual.end_with?(expected.split('XXXXXX').last) end + + def test_pretty_print_overridden_by_inspect + subtopic = Class.new(Topic) do + def inspect + "inspecting topic" + end + end + actual = '' + PP.pp(subtopic.new, StringIO.new(actual)) + assert_equal "inspecting topic\n", actual + end end diff --git a/activerecord/test/cases/counter_cache_test.rb b/activerecord/test/cases/counter_cache_test.rb index 07a182070b..1f5055b2a2 100644 --- a/activerecord/test/cases/counter_cache_test.rb +++ b/activerecord/test/cases/counter_cache_test.rb @@ -180,4 +180,22 @@ class CounterCacheTest < ActiveRecord::TestCase SpecialTopic.reset_counters(special.id, :lightweight_special_replies) end end + + test "counters are updated both in memory and in the database on create" do + car = Car.new(engines_count: 0) + car.engines = [Engine.new, Engine.new] + car.save! + + assert_equal 2, car.engines_count + assert_equal 2, car.reload.engines_count + end + + test "counter caches are updated in memory when the default value is nil" do + car = Car.new(engines_count: nil) + car.engines = [Engine.new, Engine.new] + car.save! + + assert_equal 2, car.engines_count + assert_equal 2, car.reload.engines_count + end end diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index e9bc583bf4..b9db0d0123 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -40,7 +40,7 @@ class DefaultNumbersTest < ActiveRecord::TestCase end teardown do - @connection.drop_table "default_numbers" if @connection.table_exists? 'default_numbers' + @connection.drop_table :default_numbers, if_exists: true end def test_default_positive_integer diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index fe6323ab02..6b18bfe84f 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -294,12 +294,12 @@ class InheritanceTest < ActiveRecord::TestCase def test_eager_load_belongs_to_something_inherited account = Account.all.merge!(:includes => :firm).find(1) - assert account.association_cache.key?(:firm), "nil proves eager load failed" + assert account.association(:firm).loaded?, "association was not eager loaded" end def test_alt_eager_loading cabbage = RedCabbage.all.merge!(:includes => :seller).find(4) - assert cabbage.association_cache.key?(:seller), "nil proves eager load failed" + assert cabbage.association(:seller).loaded?, "association was not eager loaded" end def test_eager_load_belongs_to_primary_key_quoting diff --git a/activerecord/test/cases/migration/change_schema_test.rb b/activerecord/test/cases/migration/change_schema_test.rb index b3129a8984..a4bff0cbb9 100644 --- a/activerecord/test/cases/migration/change_schema_test.rb +++ b/activerecord/test/cases/migration/change_schema_test.rb @@ -403,6 +403,17 @@ module ActiveRecord end end + def test_drop_table_if_exists + connection.create_table(:testings) + assert connection.table_exists?(:testings) + connection.drop_table(:testings, if_exists: true) + assert_not connection.table_exists?(:testings) + end + + def test_drop_table_if_exists_nothing_raised + assert_nothing_raised { connection.drop_table(:nonexistent, if_exists: true) } + end + private def testing_table_with_only_foo_attribute connection.create_table :testings, :id => false do |t| @@ -426,7 +437,7 @@ module ActiveRecord teardown do [:wagons, :trains].each do |table| - @connection.drop_table(table) if @connection.table_exists?(table) + @connection.drop_table table, if_exists: true end end diff --git a/activerecord/test/cases/migration/column_positioning_test.rb b/activerecord/test/cases/migration/column_positioning_test.rb index 62186e13a5..4637970ce0 100644 --- a/activerecord/test/cases/migration/column_positioning_test.rb +++ b/activerecord/test/cases/migration/column_positioning_test.rb @@ -3,7 +3,7 @@ require 'cases/helper' module ActiveRecord class Migration class ColumnPositioningTest < ActiveRecord::TestCase - attr_reader :connection, :table_name + attr_reader :connection alias :conn :connection def setup diff --git a/activerecord/test/cases/migration/foreign_key_test.rb b/activerecord/test/cases/migration/foreign_key_test.rb index f8b1bf8c9d..78d9dd90a3 100644 --- a/activerecord/test/cases/migration/foreign_key_test.rb +++ b/activerecord/test/cases/migration/foreign_key_test.rb @@ -29,8 +29,8 @@ module ActiveRecord teardown do if defined?(@connection) - @connection.drop_table "astronauts" if @connection.table_exists? 'astronauts' - @connection.drop_table "rockets" if @connection.table_exists? 'rockets' + @connection.drop_table "astronauts", if_exists: true + @connection.drop_table "rockets", if_exists: true end end diff --git a/activerecord/test/cases/migration/references_foreign_key_test.rb b/activerecord/test/cases/migration/references_foreign_key_test.rb index bba8897a0d..99de7db70c 100644 --- a/activerecord/test/cases/migration/references_foreign_key_test.rb +++ b/activerecord/test/cases/migration/references_foreign_key_test.rb @@ -10,8 +10,8 @@ module ActiveRecord end teardown do - @connection.execute("drop table if exists testings") - @connection.execute("drop table if exists testing_parents") + @connection.drop_table("testings") if @connection.table_exists? "testings" + @connection.drop_table("testing_parents") if @connection.table_exists? "testing_parents" end test "foreign keys can be created with the table" do diff --git a/activerecord/test/cases/migration/rename_table_test.rb b/activerecord/test/cases/migration/rename_table_test.rb index a018bac43d..3eef308428 100644 --- a/activerecord/test/cases/migration/rename_table_test.rb +++ b/activerecord/test/cases/migration/rename_table_test.rb @@ -86,8 +86,8 @@ module ActiveRecord assert connection.table_exists? :felines ensure disable_extension!('uuid-ossp', connection) - connection.drop_table :cats if connection.table_exists? :cats - connection.drop_table :felines if connection.table_exists? :felines + connection.drop_table :cats, if_exists: true + connection.drop_table :felines, if_exists: true end end end diff --git a/activerecord/test/cases/migration/table_and_index_test.rb b/activerecord/test/cases/migration/table_and_index_test.rb index 8fd770abd1..24cba84a09 100644 --- a/activerecord/test/cases/migration/table_and_index_test.rb +++ b/activerecord/test/cases/migration/table_and_index_test.rb @@ -6,11 +6,11 @@ module ActiveRecord def test_add_schema_info_respects_prefix_and_suffix conn = ActiveRecord::Base.connection - conn.drop_table(ActiveRecord::Migrator.schema_migrations_table_name) if conn.table_exists?(ActiveRecord::Migrator.schema_migrations_table_name) + conn.drop_table(ActiveRecord::Migrator.schema_migrations_table_name, if_exists: true) # Use shorter prefix and suffix as in Oracle database identifier cannot be larger than 30 characters ActiveRecord::Base.table_name_prefix = 'p_' ActiveRecord::Base.table_name_suffix = '_s' - conn.drop_table(ActiveRecord::Migrator.schema_migrations_table_name) if conn.table_exists?(ActiveRecord::Migrator.schema_migrations_table_name) + conn.drop_table(ActiveRecord::Migrator.schema_migrations_table_name, if_exists: true) conn.initialize_schema_migrations_table diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 7c2d3e81d6..a5d7cc2dff 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -90,7 +90,7 @@ class MigrationTest < ActiveRecord::TestCase end def test_migration_detection_without_schema_migration_table - ActiveRecord::Base.connection.drop_table('schema_migrations') if ActiveRecord::Base.connection.table_exists?('schema_migrations') + ActiveRecord::Base.connection.drop_table 'schema_migrations', if_exists: true migrations_path = MIGRATIONS_ROOT + "/valid" old_path = ActiveRecord::Migrator.migrations_paths diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 5c7e8a65d2..8bc42b8890 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -672,7 +672,7 @@ module NestedAttributesOnACollectionAssociationTests end def test_should_not_assign_destroy_key_to_a_record - assert_nothing_raised ActiveRecord::UnknownAttributeError do + assert_nothing_raised ActiveModel::AttributeAssignment::UnknownAttributeError do @pirate.send(association_setter, { 'foo' => { '_destroy' => '0' }}) end end diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index d6816041bc..2803ad2de0 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -356,6 +356,16 @@ class PersistenceTest < ActiveRecord::TestCase assert_equal("David", topic_reloaded.author_name) end + def test_update_attribute_does_not_run_sql_if_attribute_is_not_changed + klass = Class.new(Topic) do + def self.name; 'Topic'; end + end + topic = klass.create(title: 'Another New Topic') + assert_queries(0) do + topic.update_attribute(:title, 'Another New Topic') + end + end + def test_delete topic = Topic.find(1) assert_equal topic, topic.delete, 'topic.delete did not return self' diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index 751eccc015..4b668f84dd 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -210,7 +210,7 @@ class PrimaryKeyAnyTypeTest < ActiveRecord::TestCase end teardown do - @connection.execute("DROP TABLE IF EXISTS barcodes") + @connection.drop_table(:barcodes) if @connection.table_exists? :barcodes end def test_any_type_primary_key diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index eb76ef6328..fe0854b7b4 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -91,17 +91,6 @@ class RelationMergingTest < ActiveRecord::TestCase assert merged.to_sql.include?("bbq") end - def test_merging_keeps_lhs_bind_parameters - column = Post.columns_hash['id'] - binds = [[column, 20]] - - right = Post.where(id: 20) - left = Post.where(id: 10) - - merged = left.merge(right) - assert_equal binds, merged.bind_values - end - def test_merging_reorders_bind_params post = Post.first right = Post.where(id: 1) diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb index 2443f10269..45ead08bd5 100644 --- a/activerecord/test/cases/relation/mutation_test.rb +++ b/activerecord/test/cases/relation/mutation_test.rb @@ -81,7 +81,7 @@ module ActiveRecord assert_equal [], relation.extending_values end - (Relation::SINGLE_VALUE_METHODS - [:from, :lock, :reordering, :reverse_order, :create_with]).each do |method| + (Relation::SINGLE_VALUE_METHODS - [:lock, :reordering, :reverse_order, :create_with]).each do |method| test "##{method}!" do assert relation.public_send("#{method}!", :foo).equal?(relation) assert_equal :foo, relation.public_send("#{method}_value") @@ -90,7 +90,7 @@ module ActiveRecord test '#from!' do assert relation.from!('foo').equal?(relation) - assert_equal ['foo', nil], relation.from_value + assert_equal 'foo', relation.from_clause.value end test '#lock!' do @@ -136,12 +136,12 @@ module ActiveRecord end test 'test_merge!' do - assert relation.merge!(where: :foo).equal?(relation) - assert_equal [:foo], relation.where_values + assert relation.merge!(select: :foo).equal?(relation) + assert_equal [:foo], relation.select_values end test 'merge with a proc' do - assert_equal [:foo], relation.merge(-> { where(:foo) }).where_values + assert_equal [:foo], relation.merge(-> { select(:foo) }).select_values end test 'none!' do diff --git a/activerecord/test/cases/relation/where_chain_test.rb b/activerecord/test/cases/relation/where_chain_test.rb index 619055f1e7..27bbd80f79 100644 --- a/activerecord/test/cases/relation/where_chain_test.rb +++ b/activerecord/test/cases/relation/where_chain_test.rb @@ -11,22 +11,11 @@ module ActiveRecord @name = 'title' end - def test_not_eq + def test_not_inverts_where_clause relation = Post.where.not(title: 'hello') + expected_where_clause = Post.where(title: 'hello').where_clause.invert - assert_equal 1, relation.where_values.length - - value = relation.where_values.first - bind = relation.bind_values.first - - assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual - assert_equal 'hello', bind.last - end - - def test_not_null - expected = Post.arel_table[@name].not_eq(nil) - relation = Post.where.not(title: nil) - assert_equal([expected], relation.where_values) + assert_equal expected_where_clause, relation.where_clause end def test_not_with_nil @@ -35,146 +24,81 @@ module ActiveRecord end end - def test_not_in - expected = Post.arel_table[@name].not_in(%w[hello goodbye]) - relation = Post.where.not(title: %w[hello goodbye]) - assert_equal([expected], relation.where_values) - end - def test_association_not_eq - expected = Comment.arel_table[@name].not_eq('hello') + expected = Arel::Nodes::Grouping.new(Comment.arel_table[@name].not_eq(Arel::Nodes::BindParam.new)) relation = Post.joins(:comments).where.not(comments: {title: 'hello'}) - assert_equal(expected.to_sql, relation.where_values.first.to_sql) + assert_equal(expected.to_sql, relation.where_clause.ast.to_sql) end def test_not_eq_with_preceding_where relation = Post.where(title: 'hello').where.not(title: 'world') + expected_where_clause = + Post.where(title: 'hello').where_clause + + Post.where(title: 'world').where_clause.invert - value = relation.where_values.first - bind = relation.bind_values.first - assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::Equality - assert_equal 'hello', bind.last - - value = relation.where_values.last - bind = relation.bind_values.last - assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual - assert_equal 'world', bind.last + assert_equal expected_where_clause, relation.where_clause end def test_not_eq_with_succeeding_where relation = Post.where.not(title: 'hello').where(title: 'world') + expected_where_clause = + Post.where(title: 'hello').where_clause.invert + + Post.where(title: 'world').where_clause - value = relation.where_values.first - bind = relation.bind_values.first - assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual - assert_equal 'hello', bind.last - - value = relation.where_values.last - bind = relation.bind_values.last - assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::Equality - assert_equal 'world', bind.last - end - - def test_not_eq_with_string_parameter - expected = Arel::Nodes::Not.new("title = 'hello'") - relation = Post.where.not("title = 'hello'") - assert_equal([expected], relation.where_values) - end - - def test_not_eq_with_array_parameter - expected = Arel::Nodes::Not.new("title = 'hello'") - relation = Post.where.not(['title = ?', 'hello']) - assert_equal([expected], relation.where_values) + assert_equal expected_where_clause, relation.where_clause end def test_chaining_multiple relation = Post.where.not(author_id: [1, 2]).where.not(title: 'ruby on rails') + expected_where_clause = + Post.where(author_id: [1, 2]).where_clause.invert + + Post.where(title: 'ruby on rails').where_clause.invert - expected = Post.arel_table['author_id'].not_in([1, 2]) - assert_equal(expected, relation.where_values[0]) - - value = relation.where_values[1] - bind = relation.bind_values.first - - assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual - assert_equal 'ruby on rails', bind.last + assert_equal expected_where_clause, relation.where_clause end def test_rewhere_with_one_condition relation = Post.where(title: 'hello').where(title: 'world').rewhere(title: 'alone') + expected = Post.where(title: 'alone') - assert_equal 1, relation.where_values.size - value = relation.where_values.first - bind = relation.bind_values.first - assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::Equality - assert_equal 'alone', bind.last + assert_equal expected.where_clause, relation.where_clause end def test_rewhere_with_multiple_overwriting_conditions relation = Post.where(title: 'hello').where(body: 'world').rewhere(title: 'alone', body: 'again') + expected = Post.where(title: 'alone', body: 'again') - assert_equal 2, relation.where_values.size - - value = relation.where_values.first - bind = relation.bind_values.first - assert_bound_ast value, Post.arel_table['title'], Arel::Nodes::Equality - assert_equal 'alone', bind.last - - value = relation.where_values[1] - bind = relation.bind_values[1] - assert_bound_ast value, Post.arel_table['body'], Arel::Nodes::Equality - assert_equal 'again', bind.last - end - - def assert_bound_ast value, table, type - assert_equal table, value.left - assert_kind_of type, value - assert_kind_of Arel::Nodes::BindParam, value.right + assert_equal expected.where_clause, relation.where_clause end def test_rewhere_with_one_overwriting_condition_and_one_unrelated relation = Post.where(title: 'hello').where(body: 'world').rewhere(title: 'alone') + expected = Post.where(body: 'world', title: 'alone') - assert_equal 2, relation.where_values.size - - value = relation.where_values.first - bind = relation.bind_values.first - - assert_bound_ast value, Post.arel_table['body'], Arel::Nodes::Equality - assert_equal 'world', bind.last - - value = relation.where_values.second - bind = relation.bind_values.second - - assert_bound_ast value, Post.arel_table['title'], Arel::Nodes::Equality - assert_equal 'alone', bind.last + assert_equal expected.where_clause, relation.where_clause end def test_rewhere_with_range relation = Post.where(comments_count: 1..3).rewhere(comments_count: 3..5) - assert_equal 1, relation.where_values.size assert_equal Post.where(comments_count: 3..5), relation end def test_rewhere_with_infinite_upper_bound_range relation = Post.where(comments_count: 1..Float::INFINITY).rewhere(comments_count: 3..5) - assert_equal 1, relation.where_values.size assert_equal Post.where(comments_count: 3..5), relation end def test_rewhere_with_infinite_lower_bound_range relation = Post.where(comments_count: -Float::INFINITY..1).rewhere(comments_count: 3..5) - assert_equal 1, relation.where_values.size assert_equal Post.where(comments_count: 3..5), relation end def test_rewhere_with_infinite_range relation = Post.where(comments_count: -Float::INFINITY..Float::INFINITY).rewhere(comments_count: 3..5) - assert_equal 1, relation.where_values.size assert_equal Post.where(comments_count: 3..5), relation end end diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb new file mode 100644 index 0000000000..db18980e0b --- /dev/null +++ b/activerecord/test/cases/relation/where_clause_test.rb @@ -0,0 +1,162 @@ +require "cases/helper" + +class ActiveRecord::Relation + class WhereClauseTest < ActiveRecord::TestCase + test "+ combines two where clauses" do + first_clause = WhereClause.new([table["id"].eq(bind_param)], [["id", 1]]) + second_clause = WhereClause.new([table["name"].eq(bind_param)], [["name", "Sean"]]) + combined = WhereClause.new( + [table["id"].eq(bind_param), table["name"].eq(bind_param)], + [["id", 1], ["name", "Sean"]], + ) + + assert_equal combined, first_clause + second_clause + end + + test "+ is associative, but not commutative" do + a = WhereClause.new(["a"], ["bind a"]) + b = WhereClause.new(["b"], ["bind b"]) + c = WhereClause.new(["c"], ["bind c"]) + + assert_equal a + (b + c), (a + b) + c + assert_not_equal a + b, b + a + end + + test "an empty where clause is the identity value for +" do + clause = WhereClause.new([table["id"].eq(bind_param)], [["id", 1]]) + + assert_equal clause, clause + WhereClause.empty + end + + test "merge combines two where clauses" do + a = WhereClause.new([table["id"].eq(1)], []) + b = WhereClause.new([table["name"].eq("Sean")], []) + expected = WhereClause.new([table["id"].eq(1), table["name"].eq("Sean")], []) + + assert_equal expected, a.merge(b) + end + + test "merge keeps the right side, when two equality clauses reference the same column" do + a = WhereClause.new([table["id"].eq(1), table["name"].eq("Sean")], []) + b = WhereClause.new([table["name"].eq("Jim")], []) + expected = WhereClause.new([table["id"].eq(1), table["name"].eq("Jim")], []) + + assert_equal expected, a.merge(b) + end + + test "merge removes bind parameters matching overlapping equality clauses" do + a = WhereClause.new( + [table["id"].eq(bind_param), table["name"].eq(bind_param)], + [attribute("id", 1), attribute("name", "Sean")], + ) + b = WhereClause.new( + [table["name"].eq(bind_param)], + [attribute("name", "Jim")] + ) + expected = WhereClause.new( + [table["id"].eq(bind_param), table["name"].eq(bind_param)], + [attribute("id", 1), attribute("name", "Jim")], + ) + + assert_equal expected, a.merge(b) + end + + test "merge allows for columns with the same name from different tables" do + skip "This is not possible as of 4.2, and the binds do not yet contain sufficient information for this to happen" + # We might be able to change the implementation to remove conflicts by index, rather than column name + end + + test "a clause knows if it is empty" do + assert WhereClause.empty.empty? + assert_not WhereClause.new(["anything"], []).empty? + end + + test "invert cannot handle nil" do + where_clause = WhereClause.new([nil], []) + + assert_raises ArgumentError do + where_clause.invert + end + end + + test "invert replaces each part of the predicate with its inverse" do + random_object = Object.new + original = WhereClause.new([ + table["id"].in([1, 2, 3]), + table["id"].eq(1), + "sql literal", + random_object + ], []) + expected = WhereClause.new([ + table["id"].not_in([1, 2, 3]), + table["id"].not_eq(1), + Arel::Nodes::Not.new(Arel::Nodes::SqlLiteral.new("sql literal")), + Arel::Nodes::Not.new(random_object) + ], []) + + assert_equal expected, original.invert + end + + test "accept removes binary predicates referencing a given column" do + where_clause = WhereClause.new([ + table["id"].in([1, 2, 3]), + table["name"].eq(bind_param), + table["age"].gteq(bind_param), + ], [ + attribute("name", "Sean"), + attribute("age", 30), + ]) + expected = WhereClause.new([table["age"].gteq(bind_param)], [attribute("age", 30)]) + + assert_equal expected, where_clause.except("id", "name") + end + + test "ast groups its predicates with AND" do + predicates = [ + table["id"].in([1, 2, 3]), + table["name"].eq(bind_param), + ] + where_clause = WhereClause.new(predicates, []) + expected = Arel::Nodes::And.new(predicates) + + assert_equal expected, where_clause.ast + end + + test "ast wraps any SQL literals in parenthesis" do + random_object = Object.new + where_clause = WhereClause.new([ + table["id"].in([1, 2, 3]), + "foo = bar", + random_object, + ], []) + expected = Arel::Nodes::And.new([ + table["id"].in([1, 2, 3]), + Arel::Nodes::Grouping.new(Arel.sql("foo = bar")), + Arel::Nodes::Grouping.new(random_object), + ]) + + assert_equal expected, where_clause.ast + end + + test "ast removes any empty strings" do + where_clause = WhereClause.new([table["id"].in([1, 2, 3])], []) + where_clause_with_empty = WhereClause.new([table["id"].in([1, 2, 3]), ''], []) + + assert_equal where_clause.ast, where_clause_with_empty.ast + end + + private + + def table + Arel::Table.new("table") + end + + def bind_param + Arel::Nodes::BindParam.new + end + + def attribute(name, value) + ActiveRecord::Attribute.with_cast_value(name, value, ActiveRecord::Type::Value.new) + end + end +end diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index b0573579da..537937decd 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -28,6 +28,14 @@ module ActiveRecord } end + def test_where_copies_bind_params_in_the_right_order + author = authors(:david) + posts = author.posts.where.not(id: 1) + joined = Post.where(id: posts, title: posts.first.title) + + assert_equal joined, [posts.first] + end + def test_where_copies_arel_bind_params chef = Chef.create! CakeDesigner.create!(chef: chef) diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index f7cb471984..194faa9473 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -156,12 +156,12 @@ module ActiveRecord relation = Relation.new(FakeKlass, :b, nil) relation = relation.merge where: :lol, readonly: true - assert_equal [:lol], relation.where_values + assert_equal Relation::WhereClause.new([:lol], []), relation.where_clause assert_equal true, relation.readonly_value end test 'merging an empty hash into a relation' do - assert_equal [], Relation.new(FakeKlass, :b, nil).merge({}).where_values + assert_equal Relation::WhereClause.empty, Relation.new(FakeKlass, :b, nil).merge({}).where_clause end test 'merging a hash with unknown keys raises' do @@ -173,18 +173,12 @@ module ActiveRecord values = relation.values values[:where] = nil - assert_not_nil relation.where_values + assert_not_nil relation.where_clause end test 'relations can be created with a values hash' do - relation = Relation.new(FakeKlass, :b, nil, where: [:foo]) - assert_equal [:foo], relation.where_values - end - - test 'merging a single where value' do - relation = Relation.new(FakeKlass, :b, nil) - relation.merge!(where: :foo) - assert_equal [:foo], relation.where_values + relation = Relation.new(FakeKlass, :b, nil, select: [:foo]) + assert_equal [:foo], relation.select_values end test 'merging a hash interpolates conditions' do @@ -197,7 +191,7 @@ module ActiveRecord relation = Relation.new(klass, :b, nil) relation.merge!(where: ['foo = ?', 'bar']) - assert_equal ['foo = bar'], relation.where_values + assert_equal Relation::WhereClause.new(['foo = bar'], []), relation.where_clause end def test_merging_readonly_false diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 9631ea79be..1e39a9bd28 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -39,15 +39,6 @@ class RelationTest < ActiveRecord::TestCase assert_equal van, Minivan.where(:minivan_id => [van]).to_a.first end - def test_bind_values - relation = Post.all - assert_equal [], relation.bind_values - - relation2 = relation.bind 'foo' - assert_equal %w{ foo }, relation2.bind_values - assert_equal [], relation.bind_values - end - def test_two_scopes_with_includes_should_not_drop_any_include # heat habtm cache car = Car.incl_engines.incl_tyres.first @@ -857,6 +848,12 @@ class RelationTest < ActiveRecord::TestCase assert ! fake.exists?(authors(:david).id) end + def test_exists_uses_existing_scope + post = authors(:david).posts.first + authors = Author.includes(:posts).where(name: "David", posts: { id: post.id }) + assert authors.exists?(authors(:david).id) + end + def test_last authors = Author.all assert_equal authors(:bob), authors.last @@ -1462,10 +1459,27 @@ class RelationTest < ActiveRecord::TestCase def test_doesnt_add_having_values_if_options_are_blank scope = Post.having('') - assert_equal [], scope.having_values + assert scope.having_clause.empty? scope = Post.having([]) - assert_equal [], scope.having_values + assert scope.having_clause.empty? + end + + def test_having_with_binds_for_both_where_and_having + post = Post.first + having_then_where = Post.having(id: post.id).where(title: post.title).group(:id) + where_then_having = Post.where(title: post.title).having(id: post.id).group(:id) + + assert_equal [post], having_then_where + assert_equal [post], where_then_having + end + + def test_multiple_where_and_having_clauses + post = Post.first + having_then_where = Post.having(id: post.id).where(title: post.title) + .having(id: post.id).where(title: post.title).group(:id) + + assert_equal [post], having_then_where end def test_references_triggers_eager_loading @@ -1744,14 +1758,13 @@ class RelationTest < ActiveRecord::TestCase end def test_merging_keeps_lhs_bind_parameters - column = Post.columns_hash['id'] - binds = [[column, 20]] + binds = [ActiveRecord::Attribute.with_cast_value("id", 20, Post.type_for_attribute("id"))] right = Post.where(id: 20) left = Post.where(id: 10) merged = left.merge(right) - assert_equal binds, merged.bind_values + assert_equal binds, merged.bound_attributes end def test_merging_reorders_bind_params diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index 0738df1b54..4137b20c4a 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -284,8 +284,8 @@ class DefaultScopingTest < ActiveRecord::TestCase def test_unscope_merging merged = Developer.where(name: "Jamis").merge(Developer.unscope(:where)) - assert merged.where_values.empty? - assert !merged.where(name: "Jon").where_values.empty? + assert merged.where_clause.empty? + assert !merged.where(name: "Jon").where_clause.empty? end def test_order_in_default_scope_should_not_prevail @@ -426,19 +426,19 @@ class DefaultScopingTest < ActiveRecord::TestCase test "additional conditions are ANDed with the default scope" do scope = DeveloperCalledJamis.where(name: "David") - assert_equal 2, scope.where_values.length + assert_equal 2, scope.where_clause.ast.children.length assert_equal [], scope.to_a end test "additional conditions in a scope are ANDed with the default scope" do scope = DeveloperCalledJamis.david - assert_equal 2, scope.where_values.length + assert_equal 2, scope.where_clause.ast.children.length assert_equal [], scope.to_a end test "a scope can remove the condition from the default scope" do scope = DeveloperCalledJamis.david2 - assert_equal 1, scope.where_values.length - assert_equal Developer.where(name: "David").map(&:id), scope.map(&:id) + assert_equal 1, scope.where_clause.ast.children.length + assert_equal Developer.where(name: "David"), scope end end diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb index 41f3449828..8cd94ebcc2 100644 --- a/activerecord/test/cases/scoping/named_scoping_test.rb +++ b/activerecord/test/cases/scoping/named_scoping_test.rb @@ -380,8 +380,8 @@ class NamedScopingTest < ActiveRecord::TestCase end def test_should_not_duplicates_where_values - where_values = Topic.where("1=1").scope_with_lambda.where_values - assert_equal ["1=1"], where_values + relation = Topic.where("1=1") + assert_equal relation.where_clause, relation.scope_with_lambda.where_clause end def test_chaining_with_duplicate_joins diff --git a/activerecord/test/cases/scoping/relation_scoping_test.rb b/activerecord/test/cases/scoping/relation_scoping_test.rb index d7bcbf6203..02b32abebf 100644 --- a/activerecord/test/cases/scoping/relation_scoping_test.rb +++ b/activerecord/test/cases/scoping/relation_scoping_test.rb @@ -184,7 +184,7 @@ class RelationScopingTest < ActiveRecord::TestCase rescue end - assert !Developer.all.where_values.include?("name = 'Jamis'") + assert_not Developer.all.to_sql.include?("name = 'Jamis'"), "scope was not restored" end def test_default_scope_filters_on_joins diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index 185fc22e98..f185cda263 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -4,7 +4,6 @@ require 'models/pet' require 'models/topic' class TransactionCallbacksTest < ActiveRecord::TestCase - self.use_transactional_fixtures = false fixtures :topics, :owners, :pets class ReplyWithCallbacks < ActiveRecord::Base @@ -200,21 +199,21 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end def test_call_after_rollback_when_commit_fails - @first.class.connection.singleton_class.send(:alias_method, :real_method_commit_db_transaction, :commit_db_transaction) - begin - @first.class.connection.singleton_class.class_eval do - def commit_db_transaction; raise "boom!"; end - end + @first.after_commit_block { |r| r.history << :after_commit } + @first.after_rollback_block { |r| r.history << :after_rollback } - @first.after_commit_block{|r| r.history << :after_commit} - @first.after_rollback_block{|r| r.history << :after_rollback} + assert_raises RuntimeError do + @first.transaction do + tx = @first.class.connection.transaction_manager.current_transaction + def tx.commit + raise + end - assert !@first.save rescue nil - assert_equal [:after_rollback], @first.history - ensure - @first.class.connection.singleton_class.send(:remove_method, :commit_db_transaction) - @first.class.connection.singleton_class.send(:alias_method, :commit_db_transaction, :real_method_commit_db_transaction) + @first.save + end end + + assert_equal [:after_rollback], @first.history end def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint diff --git a/activerecord/test/cases/type/integer_test.rb b/activerecord/test/cases/type/integer_test.rb index ff956b7680..0c60f0690c 100644 --- a/activerecord/test/cases/type/integer_test.rb +++ b/activerecord/test/cases/type/integer_test.rb @@ -60,55 +60,68 @@ module ActiveRecord test "values below int min value are out of range" do assert_raises(::RangeError) do - Integer.new.type_cast_from_user("-2147483649") + Integer.new.type_cast_for_database(-2147483649) end end test "values above int max value are out of range" do assert_raises(::RangeError) do - Integer.new.type_cast_from_user("2147483648") + Integer.new.type_cast_for_database(2147483648) end end test "very small numbers are out of range" do assert_raises(::RangeError) do - Integer.new.type_cast_from_user("-9999999999999999999999999999999") + Integer.new.type_cast_for_database(-9999999999999999999999999999999) end end test "very large numbers are out of range" do assert_raises(::RangeError) do - Integer.new.type_cast_from_user("9999999999999999999999999999999") + Integer.new.type_cast_for_database(9999999999999999999999999999999) end end test "normal numbers are in range" do type = Integer.new - assert_equal(0, type.type_cast_from_user("0")) - assert_equal(-1, type.type_cast_from_user("-1")) - assert_equal(1, type.type_cast_from_user("1")) + assert_equal(0, type.type_cast_for_database(0)) + assert_equal(-1, type.type_cast_for_database(-1)) + assert_equal(1, type.type_cast_for_database(1)) end test "int max value is in range" do - assert_equal(2147483647, Integer.new.type_cast_from_user("2147483647")) + assert_equal(2147483647, Integer.new.type_cast_for_database(2147483647)) end test "int min value is in range" do - assert_equal(-2147483648, Integer.new.type_cast_from_user("-2147483648")) + assert_equal(-2147483648, Integer.new.type_cast_for_database(-2147483648)) end test "columns with a larger limit have larger ranges" do type = Integer.new(limit: 8) - assert_equal(9223372036854775807, type.type_cast_from_user("9223372036854775807")) - assert_equal(-9223372036854775808, type.type_cast_from_user("-9223372036854775808")) + assert_equal(9223372036854775807, type.type_cast_for_database(9223372036854775807)) + assert_equal(-9223372036854775808, type.type_cast_for_database(-9223372036854775808)) assert_raises(::RangeError) do - type.type_cast_from_user("-9999999999999999999999999999999") + type.type_cast_for_database(-9999999999999999999999999999999) end assert_raises(::RangeError) do - type.type_cast_from_user("9999999999999999999999999999999") + type.type_cast_for_database(9999999999999999999999999999999) end end + + test "values which are out of range can be re-assigned" do + klass = Class.new(ActiveRecord::Base) do + self.table_name = 'posts' + attribute :foo, Type::Integer.new + end + model = klass.new + + model.foo = 2147483648 + model.foo = 1 + + assert_equal 1, model.foo + end end end end diff --git a/activerecord/test/cases/type/unsigned_integer_test.rb b/activerecord/test/cases/type/unsigned_integer_test.rb index b6f673109e..4b8e2fb518 100644 --- a/activerecord/test/cases/type/unsigned_integer_test.rb +++ b/activerecord/test/cases/type/unsigned_integer_test.rb @@ -4,12 +4,12 @@ module ActiveRecord module Type class UnsignedIntegerTest < ActiveRecord::TestCase test "unsigned int max value is in range" do - assert_equal(4294967295, UnsignedInteger.new.type_cast_from_user("4294967295")) + assert_equal(4294967295, UnsignedInteger.new.type_cast_for_database(4294967295)) end test "minus value is out of range" do assert_raises(::RangeError) do - UnsignedInteger.new.type_cast_from_user("-1") + UnsignedInteger.new.type_cast_for_database(-1) end end end diff --git a/activerecord/test/cases/types_test.rb b/activerecord/test/cases/types_test.rb index 73e92addfe..d35d34ff2d 100644 --- a/activerecord/test/cases/types_test.rb +++ b/activerecord/test/cases/types_test.rb @@ -117,6 +117,23 @@ module ActiveRecord assert_equal Encoding::ASCII_8BIT, type_cast.encoding end end + + def test_attributes_which_are_invalid_for_database_can_still_be_reassigned + type_which_cannot_go_to_the_database = Type::Value.new + def type_which_cannot_go_to_the_database.type_cast_for_database(*) + raise + end + klass = Class.new(ActiveRecord::Base) do + self.table_name = 'posts' + attribute :foo, type_which_cannot_go_to_the_database + end + model = klass.new + + model.foo = "foo" + model.foo = "bar" + + assert_equal "bar", model.foo + end end end end diff --git a/activerecord/test/models/car.rb b/activerecord/test/models/car.rb index db0f93f63b..81263b79d1 100644 --- a/activerecord/test/models/car.rb +++ b/activerecord/test/models/car.rb @@ -8,7 +8,7 @@ class Car < ActiveRecord::Base has_one :bulb has_many :tyres - has_many :engines, :dependent => :destroy + has_many :engines, :dependent => :destroy, inverse_of: :my_car has_many :wheels, :as => :wheelable, :dependent => :destroy scope :incl_tyres, -> { includes(:tyres) } diff --git a/activesupport/lib/active_support/core_ext/numeric/time.rb b/activesupport/lib/active_support/core_ext/numeric/time.rb index ef32817f55..98716383f4 100644 --- a/activesupport/lib/active_support/core_ext/numeric/time.rb +++ b/activesupport/lib/active_support/core_ext/numeric/time.rb @@ -1,6 +1,8 @@ require 'active_support/duration' require 'active_support/core_ext/time/calculations' require 'active_support/core_ext/time/acts_like' +require 'active_support/core_ext/date/calculations' +require 'active_support/core_ext/date/acts_like' class Numeric # Enables the use of time calculations and declarations, like 45.minutes + 2.hours + 4.years. diff --git a/guides/bug_report_templates/active_record_gem.rb b/guides/bug_report_templates/active_record_gem.rb index 66bbb15afb..b295d9d21f 100644 --- a/guides/bug_report_templates/active_record_gem.rb +++ b/guides/bug_report_templates/active_record_gem.rb @@ -12,10 +12,10 @@ ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:' ActiveRecord::Base.logger = Logger.new(STDOUT) ActiveRecord::Schema.define do - create_table :posts do |t| + create_table :posts, force: true do |t| end - create_table :comments do |t| + create_table :comments, force: true do |t| t.integer :post_id end end diff --git a/guides/bug_report_templates/active_record_master.rb b/guides/bug_report_templates/active_record_master.rb index d95354e12d..9557f0b7c5 100644 --- a/guides/bug_report_templates/active_record_master.rb +++ b/guides/bug_report_templates/active_record_master.rb @@ -21,10 +21,10 @@ ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:' ActiveRecord::Base.logger = Logger.new(STDOUT) ActiveRecord::Schema.define do - create_table :posts do |t| + create_table :posts, force: true do |t| end - create_table :comments do |t| + create_table :comments, force: true do |t| t.integer :post_id end end diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md index c9af70934a..67cc6a4db3 100644 --- a/guides/source/active_record_validations.md +++ b/guides/source/active_record_validations.md @@ -227,8 +227,26 @@ end ``` We'll cover validation errors in greater depth in the [Working with Validation -Errors](#working-with-validation-errors) section. For now, let's turn to the -built-in validation helpers that Rails provides by default. +Errors](#working-with-validation-errors) section. + +### `errors.details` + +To check what validator type was used on invalid attribute, you can use +`errors.details[:attribute]`. It returns array of hashes where under `:error` + key you will find symbol of used validator. + +```ruby +class Person < ActiveRecord::Base + validates :name, presence: true +end + +>> person = Person.new +>> person.valid? +>> person.errors.details[:name] #=> [{error: :blank}] +``` + +Using `details` with custom validators are covered in the [Working with +Validation Errors](#working-with-validation-errors) section. Validation Helpers ------------------ @@ -452,7 +470,7 @@ point number. To specify that only integral numbers are allowed set If you set `:only_integer` to `true`, then it will use the ```ruby -/\A[+-]?\d+\Z/ +/\A[+-]?\d+\z/ ``` regular expression to validate the attribute's value. Otherwise, it will try to @@ -1074,6 +1092,42 @@ Another way to do this is using `[]=` setter # => ["Name cannot contain the characters !@#%*()_-+="] ``` +### `errors.details` + +You can add validator type to details hash when using `errors.add` method. + +```ruby + class Person < ActiveRecord::Base + def a_method_used_for_validation_purposes + errors.add(:name, :invalid_characters) + end + end + + person = Person.create(name: "!@#") + + person.errors.details[:name] + # => [{error: :invalid_characters}] +``` + +To improve error details to contain not allowed characters set, you can +pass additional options to `errors.add` method. + +```ruby + class Person < ActiveRecord::Base + def a_method_used_for_validation_purposes + errors.add(:name, :invalid_characters, not_allowed: "!@#%*()_-+=") + end + end + + person = Person.create(name: "!@#") + + person.errors.details[:name] + # => [{error: :invalid_characters, not_allowed: "!@#%*()_-+="}] +``` + +All built in Rails validators populate details hash with corresponding +validator types. + ### `errors[:base]` You can add error messages that are related to the object's state as a whole, instead of being related to a specific attribute. You can use this method when you want to say that the object is invalid, no matter the values of its attributes. Since `errors[:base]` is an array, you can simply add a string to it and it will be used as an error message. diff --git a/guides/source/autoloading_and_reloading_constants.md b/guides/source/autoloading_and_reloading_constants.md index c4fac1cff5..51c46d707c 100644 --- a/guides/source/autoloading_and_reloading_constants.md +++ b/guides/source/autoloading_and_reloading_constants.md @@ -114,6 +114,16 @@ certain nesting does not necessarily correlate with the namespaces at the spot. Even more, they are totally independent, take for instance ```ruby +module X + module Y + end +end + +module A + module B + end +end + module X::Y module A::B # (3) @@ -141,9 +151,10 @@ executed, and popped after it. * A singleton class opened with `class << object` gets pushed, and popped later. -* When any of the `*_eval` family of methods is called using a string argument, +* When `instance_eval` is called using a string argument, the singleton class of the receiver is pushed to the nesting of the eval'ed -code. +code. When `class_eval` or `module_eval` is called using a string argument, +the receiver is pushed to the nesting of the eval'ed code. * The nesting at the top-level of code interpreted by `Kernel#load` is empty unless the `load` call receives a true value as second argument, in which case diff --git a/guides/source/documents.yaml b/guides/source/documents.yaml index 67032a31f5..7ae3640937 100644 --- a/guides/source/documents.yaml +++ b/guides/source/documents.yaml @@ -33,7 +33,7 @@ url: active_record_querying.html description: This guide covers the database query interface provided by Active Record. - - name: Active Model basics + name: Active Model Basics url: active_model_basics.html description: This guide covers the use of model classes without Active Record. work_in_progress: true diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index 5945d48e98..31f2d2ed2f 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -193,6 +193,9 @@ following in the `blog` directory: $ bin/rails server ``` +TIP: If you are using Windows, you have to pass the scripts under the `bin` +folder directly to the Ruby interpreter e.g. `ruby bin\rails server`. + TIP: Compiling CoffeeScript and JavaScript asset compression requires you have a JavaScript runtime available on your system, in the absence of a runtime you will see an `execjs` error during asset compilation. diff --git a/guides/source/i18n.md b/guides/source/i18n.md index fd2051e675..fbee267975 100644 --- a/guides/source/i18n.md +++ b/guides/source/i18n.md @@ -809,7 +809,7 @@ So, for example, instead of the default error message `"cannot be blank"` you co | validation | with option | message | interpolation | | ------------ | ------------------------- | ------------------------- | ------------- | -| confirmation | - | :confirmation | - | +| confirmation | - | :confirmation | attribute | | acceptance | - | :accepted | - | | presence | - | :blank | - | | absence | - | :present | - | @@ -829,6 +829,7 @@ So, for example, instead of the default error message `"cannot be blank"` you co | numericality | :equal_to | :equal_to | count | | numericality | :less_than | :less_than | count | | numericality | :less_than_or_equal_to | :less_than_or_equal_to | count | +| numericality | :other_than | :other_than | count | | numericality | :only_integer | :not_an_integer | - | | numericality | :odd | :odd | - | | numericality | :even | :even | - | diff --git a/guides/source/layouts_and_rendering.md b/guides/source/layouts_and_rendering.md index eb3c188d38..69d3f6e86c 100644 --- a/guides/source/layouts_and_rendering.md +++ b/guides/source/layouts_and_rendering.md @@ -316,12 +316,13 @@ NOTE: Unless overridden, your response returned from this render option will be #### Options for `render` -Calls to the `render` method generally accept four options: +Calls to the `render` method generally accept five options: * `:content_type` * `:layout` * `:location` * `:status` +* `:formats` ##### The `:content_type` Option @@ -430,6 +431,15 @@ Rails understands both numeric status codes and the corresponding symbols shown NOTE: If you try to render content along with a non-content status code (100-199, 204, 205 or 304), it will be dropped from the response. +##### The `:formats` Option + +Rails uses the format specified in request (or `:html` by default). You can change this adding the `:formats` option with a symbol or an array: + +```ruby +render formats: :xml +render formats: [:json, :xml] +``` + #### Finding Layouts To find the current layout, Rails first looks for a file in `app/views/layouts` with the same base name as the controller. For example, rendering actions from the `PhotosController` class will use `app/views/layouts/photos.html.erb` (or `app/views/layouts/photos.builder`). If there is no such controller-specific layout, Rails will use `app/views/layouts/application.html.erb` or `app/views/layouts/application.builder`. If there is no `.erb` layout, Rails will use a `.builder` layout if one exists. Rails also provides several ways to more precisely assign specific layouts to individual controllers and actions. diff --git a/guides/source/testing.md b/guides/source/testing.md index 94cfcf12b7..fa55c09c64 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -1201,7 +1201,7 @@ within a model: ```ruby require 'test_helper' -class ProductTest < ActiveSupport::TestCase +class ProductTest < ActiveJob::TestCase test 'billing job scheduling' do assert_enqueued_with(job: BillingJob) do product.charge(account) diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 2821c8d757..dc3ec4274b 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -6,7 +6,7 @@ require 'rails/source_annotation_extractor' module Rails class Application class Configuration < ::Rails::Engine::Configuration - attr_accessor :allow_concurrency, :asset_host, :assets, :autoflush_log, + attr_accessor :allow_concurrency, :asset_host, :autoflush_log, :cache_classes, :cache_store, :consider_all_requests_local, :console, :eager_load, :exceptions_app, :file_watcher, :filter_parameters, :force_ssl, :helpers_paths, :logger, :log_formatter, :log_tags, @@ -49,21 +49,6 @@ module Rails @secret_token = nil @secret_key_base = nil @x = Custom.new - - @assets = ActiveSupport::OrderedOptions.new - @assets.enabled = true - @assets.paths = [] - @assets.precompile = [ Proc.new { |path, fn| fn =~ /app\/assets/ && !%w(.js .css).include?(File.extname(path)) }, - /(?:\/|\\|\A)application\.(css|js)$/ ] - @assets.prefix = "/assets" - @assets.version = '1.0' - @assets.debug = false - @assets.compile = true - @assets.digest = false - @assets.cache_store = [ :file_store, "#{root}/tmp/cache/assets/#{Rails.env}/" ] - @assets.js_compressor = nil - @assets.css_compressor = nil - @assets.logger = nil end def encoding=(value) diff --git a/railties/lib/rails/commands/dbconsole.rb b/railties/lib/rails/commands/dbconsole.rb index 63bf8b11ba..5175e31f14 100644 --- a/railties/lib/rails/commands/dbconsole.rb +++ b/railties/lib/rails/commands/dbconsole.rb @@ -171,7 +171,9 @@ module Rails commands = Array(commands) dirs_on_path = ENV['PATH'].to_s.split(File::PATH_SEPARATOR) - commands += commands.map{|cmd| "#{cmd}.exe"} if RbConfig::CONFIG['host_os'] =~ /mswin|mingw/ + unless (ext = RbConfig::CONFIG['EXEEXT']).empty? + commands = commands.map{|cmd| "#{cmd}#{ext}"} + end full_path_command = nil found = commands.detect do |cmd| diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index a338f31f15..1290b16872 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -599,12 +599,6 @@ module Rails end end - initializer :append_assets_path, group: :all do |app| - app.config.assets.paths.unshift(*paths["vendor/assets"].existent_directories) - app.config.assets.paths.unshift(*paths["lib/assets"].existent_directories) - app.config.assets.paths.unshift(*paths["app/assets"].existent_directories) - end - initializer :prepend_helpers_path do |app| if !isolated? || (app == self) app.config.helpers_paths.unshift(*paths["app/helpers"].existent) diff --git a/railties/lib/rails/generators/migration.rb b/railties/lib/rails/generators/migration.rb index cd388e590a..51e6d68bf0 100644 --- a/railties/lib/rails/generators/migration.rb +++ b/railties/lib/rails/generators/migration.rb @@ -3,8 +3,8 @@ require 'rails/generators/actions/create_migration' module Rails module Generators - # Holds common methods for migrations. It assumes that migrations has the - # [0-9]*_name format and can be used by another frameworks (like Sequel) + # Holds common methods for migrations. It assumes that migrations have the + # [0-9]*_name format and can be used by other frameworks (like Sequel) # just by implementing the next migration version method. module Migration extend ActiveSupport::Concern diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css b/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css index f9cd5b3483..0cdd2788d0 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css +++ b/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css @@ -6,9 +6,8 @@ * or any plugin's vendor/assets/stylesheets directory can be referenced here using a relative path. * * You're free to add application-wide styles to this file and they'll appear at the bottom of the - * compiled file so the styles you add here take precedence over styles defined in any styles - * defined in the other CSS/SCSS files in this directory. It is generally better to create a new - * file per style scope. + * compiled file so the styles you add here take precedence over styles defined in any other CSS/SCSS + * files in this directory. It is generally better to create a new file per style scope. * *= require_tree . *= require_self diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/application_controller_renderer.rb b/railties/lib/rails/generators/rails/app/templates/config/initializers/application_controller_renderer.rb new file mode 100644 index 0000000000..ea930f54da --- /dev/null +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/application_controller_renderer.rb @@ -0,0 +1,6 @@ +## Change renderer defaults here. +# +# ApplicationController.renderer.defaults.merge!( +# http_host: 'example.org', +# https: false +# ) diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/wrap_parameters.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/wrap_parameters.rb.tt index f2110c2c70..94f612c3dd 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/wrap_parameters.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/wrap_parameters.rb.tt @@ -11,6 +11,6 @@ end # To enable root element in JSON for ActiveRecord objects. # ActiveSupport.on_load(:active_record) do -# self.include_root_in_json = true +# self.include_root_in_json = true # end <%- end -%> diff --git a/railties/lib/rails/generators/rails/plugin/templates/rails/stylesheets.css b/railties/lib/rails/generators/rails/plugin/templates/rails/stylesheets.css index f9cd5b3483..0cdd2788d0 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/rails/stylesheets.css +++ b/railties/lib/rails/generators/rails/plugin/templates/rails/stylesheets.css @@ -6,9 +6,8 @@ * or any plugin's vendor/assets/stylesheets directory can be referenced here using a relative path. * * You're free to add application-wide styles to this file and they'll appear at the bottom of the - * compiled file so the styles you add here take precedence over styles defined in any styles - * defined in the other CSS/SCSS files in this directory. It is generally better to create a new - * file per style scope. + * compiled file so the styles you add here take precedence over styles defined in any other CSS/SCSS + * files in this directory. It is generally better to create a new file per style scope. * *= require_tree . *= require_self diff --git a/railties/lib/rails/tasks/tmp.rake b/railties/lib/rails/tasks/tmp.rake index b33ae9862b..9162ef234a 100644 --- a/railties/lib/rails/tasks/tmp.rake +++ b/railties/lib/rails/tasks/tmp.rake @@ -11,7 +11,7 @@ namespace :tmp do tmp_dirs.each { |d| directory d } - desc "Creates tmp directories for sessions, cache, sockets, and pids" + desc "Creates tmp directories for cache, sockets, and pids" task create: tmp_dirs namespace :cache do |