diff options
59 files changed, 704 insertions, 321 deletions
diff --git a/.travis.yml b/.travis.yml index af19cd41de..5b8cfa9dbf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -93,9 +93,14 @@ matrix: - "GEM=ar:mysql2 MYSQL=mariadb" addons: mariadb: 10.0 - - rvm: 2.4.1 + - rvm: 2.3.4 env: - "GEM=ar:sqlite3_mem" + - rvm: 2.3.4 + env: + - "GEM=ar:postgresql POSTGRES=9.2" + addons: + postgresql: "9.2" - rvm: jruby-9.1.8.0 jdk: oraclejdk8 env: @@ -104,12 +109,6 @@ matrix: jdk: oraclejdk8 env: - "GEM=am,amo,aj" - # Test with old (< 9.4.2) postgresql - - rvm: 2.4.1 - env: - - "GEM=ar:postgresql" - addons: - postgresql: "9.4" allow_failures: - rvm: ruby-head - rvm: jruby-9.1.8.0 diff --git a/actionmailer/test/parameterized_test.rb b/actionmailer/test/parameterized_test.rb index 914ed12312..e988fffcb9 100644 --- a/actionmailer/test/parameterized_test.rb +++ b/actionmailer/test/parameterized_test.rb @@ -14,7 +14,6 @@ class ParameterizedTest < ActiveSupport::TestCase @previous_deliver_later_queue_name = ActionMailer::Base.deliver_later_queue_name ActionMailer::Base.deliver_later_queue_name = :test_queue - ActionMailer::Base.delivery_method = :test @mail = ParamsMailer.with(inviter: "david@basecamp.com", invitee: "jason@basecamp.com").invitation end diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index c5b679207a..51cec82801 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1 +1,13 @@ +* Add `action_controller_api` and `action_controller_base` load hooks to be called in `ActiveSupport.on_load` + + `ActionController::Base` and `ActionController::API` have differing implementations. This means that + the one umbrella hook `action_controller` is not able to address certain situations where a method + may not exist in a certain implementation. + + This is fixed by adding two new hooks so you can target `ActionController::Base` vs `ActionController::API` + + Fixes #27013. + + *Julian Nadeau* + Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/actionpack/CHANGELOG.md) for previous changes. diff --git a/actionpack/lib/action_controller/api.rb b/actionpack/lib/action_controller/api.rb index 0d1af0d0bd..94698df730 100644 --- a/actionpack/lib/action_controller/api.rb +++ b/actionpack/lib/action_controller/api.rb @@ -141,6 +141,7 @@ module ActionController include mod end + ActiveSupport.run_load_hooks(:action_controller_api, self) ActiveSupport.run_load_hooks(:action_controller, self) end end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index b420e00c78..8c2b111f89 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -267,6 +267,7 @@ module ActionController end end + ActiveSupport.run_load_hooks(:action_controller_base, self) ActiveSupport.run_load_hooks(:action_controller, self) end end diff --git a/actionview/app/assets/javascripts/rails-ujs/utils/ajax.coffee b/actionview/app/assets/javascripts/rails-ujs/utils/ajax.coffee index 9af515beda..26df7b9a3f 100644 --- a/actionview/app/assets/javascripts/rails-ujs/utils/ajax.coffee +++ b/actionview/app/assets/javascripts/rails-ujs/utils/ajax.coffee @@ -29,6 +29,7 @@ Rails.ajax = (options) -> fire(document, 'ajaxStop') # to be compatible with jQuery.ajax prepareOptions = (options) -> + options.url = options.url or location.href options.type = options.type.toUpperCase() # append data to url if it's a GET request if options.type is 'GET' and options.data @@ -63,10 +64,10 @@ processResponse = (response, type) -> if typeof response is 'string' and typeof type is 'string' if type.match(/\bjson\b/) try response = JSON.parse(response) - else if type.match(/\bjavascript\b/) + else if type.match(/\b(?:java|ecma)script\b/) script = document.createElement('script') - script.innerHTML = response - document.body.appendChild(script) + script.text = response + document.head.appendChild(script).parentNode.removeChild(script) else if type.match(/\b(xml|html|svg)\b/) parser = new DOMParser() type = type.replace(/;.+/, '') # remove something like ';charset=utf-8' diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index ba189e23fe..5ddf1ceb66 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -62,7 +62,7 @@ module ActionView node end else - unless name.include?('#') # Dynamic template partial names can never be tracked + unless name.include?("#") # Dynamic template partial names can never be tracked logger.error " Couldn't find template for digesting: #{name}" end diff --git a/actionview/lib/action_view/helpers/form_helper.rb b/actionview/lib/action_view/helpers/form_helper.rb index 26a625e4fe..96f8aede76 100644 --- a/actionview/lib/action_view/helpers/form_helper.rb +++ b/actionview/lib/action_view/helpers/form_helper.rb @@ -201,9 +201,9 @@ module ActionView # <%= f.submit %> # <% end %> # - # This also works for the methods in FormOptionHelper and DateHelper that + # This also works for the methods in FormOptionsHelper and DateHelper that # are designed to work with an object as base, like - # FormOptionHelper#collection_select and DateHelper#datetime_select. + # FormOptionsHelper#collection_select and DateHelper#datetime_select. # # === #form_for with a model object # @@ -416,13 +416,13 @@ module ActionView # # To set an authenticity token you need to pass an <tt>:authenticity_token</tt> parameter # - # <%= form_for @invoice, url: external_url, authenticity_token: 'external_token' do |f| + # <%= form_for @invoice, url: external_url, authenticity_token: 'external_token' do |f| %> # ... # <% end %> # # If you don't want to an authenticity token field be rendered at all just pass <tt>false</tt>: # - # <%= form_for @invoice, url: external_url, authenticity_token: false do |f| + # <%= form_for @invoice, url: external_url, authenticity_token: false do |f| %> # ... # <% end %> def form_for(record, options = {}, &block) @@ -632,9 +632,9 @@ module ActionView # <%= form.submit %> # <% end %> # - # Same goes for the methods in FormOptionHelper and DateHelper designed + # Same goes for the methods in FormOptionsHelper and DateHelper designed # to work with an object as a base, like - # FormOptionHelper#collection_select and DateHelper#datetime_select. + # FormOptionsHelper#collection_select and DateHelper#datetime_select. # # === Setting the method # @@ -791,9 +791,9 @@ module ActionView # _class_ of the model object, e.g. if <tt>@person.permission</tt>, is # of class +Permission+, the field will still be named <tt>permission[admin]</tt>. # - # Note: This also works for the methods in FormOptionHelper and + # Note: This also works for the methods in FormOptionsHelper and # DateHelper that are designed to work with an object as base, like - # FormOptionHelper#collection_select and DateHelper#datetime_select. + # FormOptionsHelper#collection_select and DateHelper#datetime_select. # # === Nested Attributes Examples # @@ -1033,9 +1033,9 @@ module ActionView # <%= check_box_tag "comment[all_caps]", "1", @comment.commenter.hulk_mode? %> # <% end %> # - # Same goes for the methods in FormOptionHelper and DateHelper designed + # Same goes for the methods in FormOptionsHelper and DateHelper designed # to work with an object as a base, like - # FormOptionHelper#collection_select and DateHelper#datetime_select. + # FormOptionsHelper#collection_select and DateHelper#datetime_select. def fields(scope = nil, model: nil, **options, &block) options[:allow_method_names_outside_object] = true options[:skip_default_ids] = true @@ -1724,9 +1724,9 @@ module ActionView # _class_ of the model object, e.g. if <tt>@person.permission</tt>, is # of class +Permission+, the field will still be named <tt>permission[admin]</tt>. # - # Note: This also works for the methods in FormOptionHelper and + # Note: This also works for the methods in FormOptionsHelper and # DateHelper that are designed to work with an object as base, like - # FormOptionHelper#collection_select and DateHelper#datetime_select. + # FormOptionsHelper#collection_select and DateHelper#datetime_select. # # === Nested Attributes Examples # diff --git a/actionview/test/ujs/public/test/call-remote.js b/actionview/test/ujs/public/test/call-remote.js index dbeb8ad832..5932195363 100644 --- a/actionview/test/ujs/public/test/call-remote.js +++ b/actionview/test/ujs/public/test/call-remote.js @@ -100,6 +100,34 @@ asyncTest('JS code should be executed', 1, function() { submit() }) +asyncTest('ecmascript code should be executed', 1, function() { + buildForm({ method: 'post', 'data-type': 'script' }) + + $('form').append('<input type="text" name="content_type" value="application/ecmascript">') + $('form').append('<input type="text" name="content" value="ok(true, \'remote code should be run\')">') + + submit() +}) + +asyncTest('execution of JS code does not modify current DOM', 1, function() { + var docLength, newDocLength + function getDocLength() { + return document.documentElement.outerHTML.length + } + + buildForm({ method: 'post', 'data-type': 'script' }) + + $('form').append('<input type="text" name="content_type" value="text/javascript">') + $('form').append('<input type="text" name="content" value="\'remote code should be run\'">') + + docLength = getDocLength() + + submit(function() { + newDocLength = getDocLength() + ok(docLength === newDocLength, 'executed JS should not present in the document') + }) +}) + asyncTest('XML document should be parsed', 1, function() { buildForm({ method: 'post', 'data-type': 'html' }) diff --git a/actionview/test/ujs/public/test/data-remote.js b/actionview/test/ujs/public/test/data-remote.js index b756add24e..161a92ac11 100644 --- a/actionview/test/ujs/public/test/data-remote.js +++ b/actionview/test/ujs/public/test/data-remote.js @@ -1,3 +1,17 @@ +(function() { + +function buildSelect(attrs) { + attrs = $.extend({ + 'name': 'user_data', 'data-remote': 'true', 'data-url': '/echo', 'data-params': 'data1=value1' + }, attrs) + + $('#qunit-fixture').append( + $('<select />', attrs) + .append($('<option />', {value: 'optionValue1', text: 'option1'})) + .append($('<option />', {value: 'optionValue2', text: 'option2'})) + ) +} + module('data-remote', { setup: function() { $('#qunit-fixture') @@ -135,17 +149,7 @@ asyncTest('clicking on a button with data-remote attribute', 5, function() { }) asyncTest('changing a select option with data-remote attribute', 5, function() { - $('form') - .append( - $('<select />', { - 'name': 'user_data', - 'data-remote': 'true', - 'data-params': 'data1=value1', - 'data-url': '/echo' - }) - .append($('<option />', {value: 'optionValue1', text: 'option1'})) - .append($('<option />', {value: 'optionValue2', text: 'option2'})) - ) + buildSelect() $('select[data-remote]') .bindNative('ajax:success', function(e, data, status, xhr) { @@ -350,17 +354,7 @@ asyncTest('submitting a form with falsy "data-remote" attribute', 0, function() }) asyncTest('changing a select option with falsy "data-remote" attribute', 0, function() { - $('form') - .append( - $('<select />', { - 'name': 'user_data', - 'data-remote': 'false', - 'data-params': 'data1=value1', - 'data-url': '/echo' - }) - .append($('<option />', {value: 'optionValue1', text: 'option1'})) - .append($('<option />', {value: 'optionValue2', text: 'option2'})) - ) + buildSelect({'data-remote': 'false'}) $('select[data-remote=false]:first') .bindNative('ajax:beforeSend', function() { @@ -413,3 +407,32 @@ asyncTest('form buttons should only be serialized when clicked', 4, function() { }) .find('[name=submit2]').triggerNative('click') }) + +asyncTest('changing a select option without "data-url" attribute still fires ajax request to current location', 1, function() { + var currentLocation, ajaxLocation + + buildSelect({'data-url': ''}); + + $('select[data-remote]') + .bindNative('ajax:beforeSend', function(e, xhr, settings) { + // Get current location (the same way jQuery does) + try { + currentLocation = location.href + } catch(err) { + currentLocation = document.createElement( 'a' ) + currentLocation.href = '' + currentLocation = currentLocation.href + } + + ajaxLocation = settings.url.replace(settings.data, '').replace(/&$/, '').replace(/\?$/, '') + equal(ajaxLocation, currentLocation, 'URL should be current page by default') + + return false + }) + .val('optionValue2') + .triggerNative('change') + + setTimeout(function() { start() }, 20) +}) + +})() diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 166c6ac21f..b5c0b43b61 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -472,5 +472,9 @@ module ActiveModel def missing_attribute(attr_name, stack) raise ActiveModel::MissingAttributeError, "missing attribute: #{attr_name}", stack end + + def _read_attribute(attr) + __send__(attr) + end end end diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index 6e0af99ad7..dc81f74779 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -179,13 +179,13 @@ module ActiveModel # Handles <tt>*_changed?</tt> for +method_missing+. def attribute_changed?(attr, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) # :nodoc: !!changes_include?(attr) && - (to == OPTION_NOT_GIVEN || to == __send__(attr)) && + (to == OPTION_NOT_GIVEN || to == _read_attribute(attr)) && (from == OPTION_NOT_GIVEN || from == changed_attributes[attr]) end # Handles <tt>*_was</tt> for +method_missing+. def attribute_was(attr) # :nodoc: - attribute_changed?(attr) ? changed_attributes[attr] : __send__(attr) + attribute_changed?(attr) ? changed_attributes[attr] : _read_attribute(attr) end # Handles <tt>*_previously_changed?</tt> for +method_missing+. @@ -226,7 +226,7 @@ module ActiveModel # Handles <tt>*_change</tt> for +method_missing+. def attribute_change(attr) - [changed_attributes[attr], __send__(attr)] if attribute_changed?(attr) + [changed_attributes[attr], _read_attribute(attr)] if attribute_changed?(attr) end # Handles <tt>*_previous_change</tt> for +method_missing+. @@ -239,7 +239,7 @@ module ActiveModel return if attribute_changed?(attr) begin - value = __send__(attr) + value = _read_attribute(attr) value = value.duplicable? ? value.clone : value rescue TypeError, NoMethodError end diff --git a/activemodel/lib/active_model/type/string.rb b/activemodel/lib/active_model/type/string.rb index c7e0208a5a..850cab962b 100644 --- a/activemodel/lib/active_model/type/string.rb +++ b/activemodel/lib/active_model/type/string.rb @@ -12,7 +12,12 @@ module ActiveModel private def cast_value(value) - ::String.new(super) + case value + when ::String then ::String.new(value) + when true then "t".freeze + when false then "f".freeze + else value.to_s + end end end end diff --git a/activemodel/lib/active_model/validations/format.rb b/activemodel/lib/active_model/validations/format.rb index b4b8d9f33c..98b3f6a8e5 100644 --- a/activemodel/lib/active_model/validations/format.rb +++ b/activemodel/lib/active_model/validations/format.rb @@ -1,4 +1,3 @@ - module ActiveModel module Validations class FormatValidator < EachValidator # :nodoc: diff --git a/activemodel/lib/active_model/validations/presence.rb b/activemodel/lib/active_model/validations/presence.rb index 6e8a434bbe..ce4106a5e1 100644 --- a/activemodel/lib/active_model/validations/presence.rb +++ b/activemodel/lib/active_model/validations/presence.rb @@ -1,4 +1,3 @@ - module ActiveModel module Validations class PresenceValidator < EachValidator # :nodoc: diff --git a/activemodel/test/cases/type/string_test.rb b/activemodel/test/cases/type/string_test.rb index 222083817e..47d412e27e 100644 --- a/activemodel/test/cases/type/string_test.rb +++ b/activemodel/test/cases/type/string_test.rb @@ -12,16 +12,25 @@ module ActiveModel end test "cast strings are mutable" do - s = "foo" type = Type::String.new + + s = "foo" assert_equal false, type.cast(s).frozen? + assert_equal false, s.frozen? + + f = "foo".freeze + assert_equal false, type.cast(f).frozen? + assert_equal true, f.frozen? end test "values are duped coming out" do - s = "foo" type = Type::String.new + + s = "foo" assert_not_same s, type.cast(s) + assert_equal s, type.cast(s) assert_not_same s, type.deserialize(s) + assert_equal s, type.deserialize(s) end end end diff --git a/activemodel/test/validators/email_validator.rb b/activemodel/test/validators/email_validator.rb index 9b74d83b37..f733c45cf0 100644 --- a/activemodel/test/validators/email_validator.rb +++ b/activemodel/test/validators/email_validator.rb @@ -1,4 +1,3 @@ - class EmailValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) record.errors[attribute] << (options[:message] || "is not an email") unless diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 9e3b686bbb..ffe588b3c5 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix inconsistency with changed attributes when overriding AR attribute reader. + + *bogdanvlviv* + * When calling the dynamic fixture accessor method with no arguments it now returns all fixtures of this type. Previously this method always returned an empty array. diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 6efa448d49..e52c2004f3 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1649,7 +1649,7 @@ module ActiveRecord # you don't want to have association presence validated, use <tt>optional: true</tt>. # [:default] # Provide a callable (i.e. proc or lambda) to specify that the association should - # be initialized with a particular record before validation. + # be initialized with a particular record before validation. # # Option examples: # belongs_to :firm, foreign_key: "client_of" diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 6bb072dd73..19b7821494 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -149,57 +149,67 @@ module ActiveRecord end def begin_transaction(options = {}) - run_commit_callbacks = !current_transaction.joinable? - transaction = - if @stack.empty? - RealTransaction.new(@connection, options, run_commit_callbacks: run_commit_callbacks) - else - SavepointTransaction.new(@connection, "active_record_#{@stack.size}", options, - run_commit_callbacks: run_commit_callbacks) - end + @connection.lock.synchronize do + run_commit_callbacks = !current_transaction.joinable? + transaction = + if @stack.empty? + RealTransaction.new(@connection, options, run_commit_callbacks: run_commit_callbacks) + else + SavepointTransaction.new(@connection, "active_record_#{@stack.size}", options, + run_commit_callbacks: run_commit_callbacks) + end - @stack.push(transaction) - transaction + @stack.push(transaction) + transaction + end end def commit_transaction - transaction = @stack.last + @connection.lock.synchronize do + transaction = @stack.last - begin - transaction.before_commit_records - ensure - @stack.pop - end + begin + transaction.before_commit_records + ensure + @stack.pop + end - transaction.commit - transaction.commit_records + transaction.commit + transaction.commit_records + end end def rollback_transaction(transaction = nil) - transaction ||= @stack.pop - transaction.rollback - transaction.rollback_records + @connection.lock.synchronize do + transaction ||= @stack.pop + transaction.rollback + transaction.rollback_records + end end def within_new_transaction(options = {}) - transaction = begin_transaction options - yield - rescue Exception => error - if transaction - rollback_transaction - after_failure_actions(transaction, error) - end - raise - ensure - unless error - if Thread.current.status == "aborting" - rollback_transaction if transaction - else - begin - commit_transaction - rescue Exception - rollback_transaction(transaction) unless transaction.state.completed? - raise + @connection.lock.synchronize do + begin + transaction = begin_transaction options + yield + rescue Exception => error + if transaction + rollback_transaction + after_failure_actions(transaction, error) + end + raise + ensure + unless error + if Thread.current.status == "aborting" + rollback_transaction if transaction + else + begin + commit_transaction + rescue Exception + rollback_transaction(transaction) unless transaction.state.completed? + raise + end + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 96083e6519..85d6fbe8b3 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -74,7 +74,7 @@ module ActiveRecord SIMPLE_INT = /\A\d+\z/ attr_accessor :visitor, :pool - attr_reader :schema_cache, :owner, :logger, :prepared_statements + attr_reader :schema_cache, :owner, :logger, :prepared_statements, :lock alias :in_use? :owner def self.type_cast_config_to_integer(config) 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 1d439acb07..02a6da2f71 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -344,13 +344,17 @@ module ActiveRecord def primary_keys(table_name) # :nodoc: select_values(<<-SQL.strip_heredoc, "SCHEMA") - SELECT a.attname FROM pg_index i - CROSS JOIN generate_subscripts(i.indkey, 1) k - JOIN pg_attribute a - ON a.attrelid = i.indrelid - AND a.attnum = i.indkey[k] - WHERE i.indrelid = #{quote(quote_table_name(table_name))}::regclass - AND i.indisprimary + SELECT a.attname + FROM ( + SELECT indrelid, indkey, generate_subscripts(indkey, 1) idx + FROM pg_index + WHERE indrelid = #{quote(quote_table_name(table_name))}::regclass + AND indisprimary + ) i + JOIN pg_attribute a + ON a.attrelid = i.indrelid + AND a.attnum = i.indkey[i.idx] + ORDER BY i.idx SQL end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 0ad114165e..7ae9bd9a5f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -239,7 +239,9 @@ module ActiveRecord # Is this connection alive and ready for queries? def active? - @connection.query "SELECT 1" + @lock.synchronize do + @connection.query "SELECT 1" + end true rescue PG::Error false @@ -247,26 +249,32 @@ module ActiveRecord # Close then reopen the connection. def reconnect! - super - @connection.reset - configure_connection + @lock.synchronize do + super + @connection.reset + configure_connection + end end def reset! - clear_cache! - reset_transaction - unless @connection.transaction_status == ::PG::PQTRANS_IDLE - @connection.query "ROLLBACK" + @lock.synchronize do + clear_cache! + reset_transaction + unless @connection.transaction_status == ::PG::PQTRANS_IDLE + @connection.query "ROLLBACK" + end + @connection.query "DISCARD ALL" + configure_connection end - @connection.query "DISCARD ALL" - configure_connection end # Disconnects from the database if already connected. Otherwise, this # method does nothing. def disconnect! - super - @connection.close rescue nil + @lock.synchronize do + super + @connection.close rescue nil + end end def native_database_types #:nodoc: diff --git a/activerecord/lib/active_record/dynamic_matchers.rb b/activerecord/lib/active_record/dynamic_matchers.rb index 08d42f3dd4..d9912cfb22 100644 --- a/activerecord/lib/active_record/dynamic_matchers.rb +++ b/activerecord/lib/active_record/dynamic_matchers.rb @@ -1,4 +1,3 @@ - module ActiveRecord module DynamicMatchers #:nodoc: def respond_to_missing?(name, include_private = false) diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 14c2ddd85b..a1459c87c6 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -383,7 +383,7 @@ module ActiveRecord end def construct_relation_for_exists(relation, conditions) - relation = relation.except(:select, :distinct)._select!(ONE_AS_ONE).limit!(1) + relation = relation.except(:select, :distinct, :order)._select!(ONE_AS_ONE).limit!(1) case conditions when Array, Hash diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index fca914aedd..183fe91c05 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -1,13 +1,14 @@ +require "active_record/relation/predicate_builder/array_handler" +require "active_record/relation/predicate_builder/base_handler" +require "active_record/relation/predicate_builder/basic_object_handler" +require "active_record/relation/predicate_builder/range_handler" +require "active_record/relation/predicate_builder/relation_handler" + +require "active_record/relation/predicate_builder/association_query_value" +require "active_record/relation/predicate_builder/polymorphic_array_value" + module ActiveRecord class PredicateBuilder # :nodoc: - require "active_record/relation/predicate_builder/array_handler" - require "active_record/relation/predicate_builder/association_query_handler" - require "active_record/relation/predicate_builder/base_handler" - require "active_record/relation/predicate_builder/basic_object_handler" - require "active_record/relation/predicate_builder/polymorphic_array_handler" - require "active_record/relation/predicate_builder/range_handler" - require "active_record/relation/predicate_builder/relation_handler" - delegate :resolve_column_aliases, to: :table def initialize(table) @@ -20,8 +21,6 @@ module ActiveRecord register_handler(RangeHandler::RangeWithBinds, RangeHandler.new) register_handler(Relation, RelationHandler.new) register_handler(Array, ArrayHandler.new(self)) - register_handler(AssociationQueryValue, AssociationQueryHandler.new(self)) - register_handler(PolymorphicArrayValue, PolymorphicArrayHandler.new(self)) end def build_from_hash(attributes) @@ -87,7 +86,6 @@ module ActiveRecord binds = [] attributes.each do |column_name, value| - binds.concat(value.bound_attributes) if value.is_a?(Relation) case when value.is_a?(Hash) && !table.has_column?(column_name) attrs, bvs = associated_predicate_builder(column_name).create_binds_for_hash(value) @@ -99,7 +97,21 @@ module ActiveRecord # # For polymorphic relationships, find the foreign key and type: # PriceEstimate.where(estimate_of: treasure) - result[column_name] = AssociationQueryHandler.value_for(table, column_name, value) + associated_table = table.associated_table(column_name) + if associated_table.polymorphic_association? + case value.is_a?(Array) ? value.first : value + when Base, Relation + value = [value] unless value.is_a?(Array) + klass = PolymorphicArrayValue + end + end + + klass ||= AssociationQueryValue + result[column_name] = klass.new(associated_table, value).queries.map do |query| + attrs, bvs = create_binds_for_hash(query) + binds.concat(bvs) + attrs + end when value.is_a?(Range) && !table.type(column_name).respond_to?(:subtype) first = value.begin last = value.end @@ -117,6 +129,8 @@ module ActiveRecord if can_be_bound?(column_name, value) result[column_name] = Arel::Nodes::BindParam.new binds << build_bind_param(column_name, value) + elsif value.is_a?(Relation) + binds.concat(value.bound_attributes) end end end diff --git a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb index 88b6c37d43..1068e700e2 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb @@ -6,11 +6,11 @@ module ActiveRecord end def call(attribute, value) + return attribute.in([]) if value.empty? + return queries_predicates(value) if value.all? { |v| v.is_a?(Hash) } + values = value.map { |x| x.is_a?(Base) ? x.id : x } nils, values = values.partition(&:nil?) - - return attribute.in([]) if values.empty? && nils.empty? - ranges, values = values.partition { |v| v.is_a?(Range) } values_predicate = @@ -26,7 +26,7 @@ module ActiveRecord array_predicates = ranges.map { |range| predicate_builder.build(attribute, range) } array_predicates.unshift(values_predicate) - array_predicates.inject { |composite, predicate| composite.or(predicate) } + array_predicates.inject(&:or) end # TODO Change this to private once we've dropped Ruby 2.2 support. @@ -40,6 +40,17 @@ module ActiveRecord other end end + + private + def queries_predicates(queries) + if queries.size > 1 + queries.map do |query| + Arel::Nodes::And.new(predicate_builder.build_from_hash(query)) + end.inject(&:or) + else + predicate_builder.build_from_hash(queries.first) + end + end end end end diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb deleted file mode 100644 index 29860ec677..0000000000 --- a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb +++ /dev/null @@ -1,88 +0,0 @@ -module ActiveRecord - class PredicateBuilder - class AssociationQueryHandler # :nodoc: - def self.value_for(table, column, value) - associated_table = table.associated_table(column) - klass = if associated_table.polymorphic_association? && ::Array === value && value.first.is_a?(Base) - PolymorphicArrayValue - else - AssociationQueryValue - end - - klass.new(associated_table, value) - end - - def initialize(predicate_builder) - @predicate_builder = predicate_builder - end - - def call(attribute, value) - queries = {} - - table = value.associated_table - if value.base_class - queries[table.association_foreign_type.to_s] = value.base_class.name - end - - queries[table.association_foreign_key.to_s] = value.ids - predicate_builder.build_from_hash(queries) - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :predicate_builder - end - - class AssociationQueryValue # :nodoc: - attr_reader :associated_table, :value - - def initialize(associated_table, value) - @associated_table = associated_table - @value = value - end - - def ids - case value - when Relation - value.select(primary_key) - when Array - value.map { |v| convert_to_id(v) } - else - convert_to_id(value) - end - end - - def base_class - if associated_table.polymorphic_association? - @base_class ||= polymorphic_base_class_from_value - end - end - - private - - def primary_key - associated_table.association_primary_key(base_class) - end - - def polymorphic_base_class_from_value - case value - when Relation - value.klass.base_class - when Base - value.class.base_class - end - end - - def convert_to_id(value) - case value - when Base - value._read_attribute(primary_key) - else - value - end - end - end - end -end diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb new file mode 100644 index 0000000000..2fe0f81cab --- /dev/null +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb @@ -0,0 +1,41 @@ +module ActiveRecord + class PredicateBuilder + class AssociationQueryValue # :nodoc: + attr_reader :associated_table, :value + + def initialize(associated_table, value) + @associated_table = associated_table + @value = value + end + + def queries + [associated_table.association_foreign_key.to_s => ids] + end + + private + def ids + case value + when Relation + value.select_values.empty? ? value.select(primary_key) : value + when Array + value.map { |v| convert_to_id(v) } + else + convert_to_id(value) + end + end + + def primary_key + associated_table.association_primary_key + end + + def convert_to_id(value) + case value + when Base + value._read_attribute(primary_key) + else + value + end + end + end + end +end diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb deleted file mode 100644 index 335124c952..0000000000 --- a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb +++ /dev/null @@ -1,59 +0,0 @@ -module ActiveRecord - class PredicateBuilder - class PolymorphicArrayHandler # :nodoc: - def initialize(predicate_builder) - @predicate_builder = predicate_builder - end - - def call(attribute, value) - table = value.associated_table - queries = value.type_to_ids_mapping.map do |type, ids| - { table.association_foreign_type.to_s => type, table.association_foreign_key.to_s => ids } - end - - predicates = queries.map { |query| predicate_builder.build_from_hash(query) } - - if predicates.size > 1 - type_and_ids_predicates = predicates.map { |type_predicate, id_predicate| Arel::Nodes::Grouping.new(type_predicate.and(id_predicate)) } - type_and_ids_predicates.inject(&:or) - else - predicates.first - end - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :predicate_builder - end - - class PolymorphicArrayValue # :nodoc: - attr_reader :associated_table, :values - - def initialize(associated_table, values) - @associated_table = associated_table - @values = values - end - - def type_to_ids_mapping - default_hash = Hash.new { |hsh, key| hsh[key] = [] } - values.each_with_object(default_hash) { |value, hash| hash[base_class(value).name] << convert_to_id(value) } - end - - private - - def primary_key(value) - associated_table.association_primary_key(base_class(value)) - end - - def base_class(value) - value.class.base_class - end - - def convert_to_id(value) - value._read_attribute(primary_key(value)) - end - end - end -end diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_value.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_value.rb new file mode 100644 index 0000000000..9bb2f8c8dc --- /dev/null +++ b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_value.rb @@ -0,0 +1,49 @@ +module ActiveRecord + class PredicateBuilder + class PolymorphicArrayValue # :nodoc: + attr_reader :associated_table, :values + + def initialize(associated_table, values) + @associated_table = associated_table + @values = values + end + + def queries + type_to_ids_mapping.map do |type, ids| + { + associated_table.association_foreign_type.to_s => type, + associated_table.association_foreign_key.to_s => ids.size > 1 ? ids : ids.first + } + end + end + + private + def type_to_ids_mapping + default_hash = Hash.new { |hsh, key| hsh[key] = [] } + values.each_with_object(default_hash) { |value, hash| hash[base_class(value).name] << convert_to_id(value) } + end + + def primary_key(value) + associated_table.association_primary_key(base_class(value)) + end + + def base_class(value) + case value + when Base + value.class.base_class + when Relation + value.klass.base_class + end + end + + def convert_to_id(value) + case value + when Base + value._read_attribute(primary_key(value)) + when Relation + value.select(primary_key(value)) + end + end + end + end +end diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 657bd43b86..94d63765c9 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -47,17 +47,18 @@ module ActiveRecord @options = options end - # turns 20170404131909 into "2017_04_04131909" + # turns 20170404131909 into "2017_04_04_131909" def formatted_version - return "" unless @version stringified = @version.to_s return stringified unless stringified.length == 14 stringified.insert(4, "_").insert(7, "_").insert(10, "_") end - def header(stream) - define_params = @version ? "version: #{formatted_version}" : "" + def define_params + @version ? "version: #{formatted_version}" : "" + end + def header(stream) stream.puts <<HEADER # This file is auto-generated from the current state of the database. Instead # of editing this file, please use the migrations feature of Active Record to diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index 6aa6a79705..52e4a38cae 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -13,6 +13,10 @@ module PostgresqlUUIDHelper def uuid_function connection.supports_pgcrypto_uuid? ? "gen_random_uuid()" : "uuid_generate_v4()" end + + def uuid_default + connection.supports_pgcrypto_uuid? ? {} : { default: uuid_function } + end end class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase @@ -178,7 +182,7 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase t.uuid "other_uuid_2", default: "my_uuid_generator()" end - connection.create_table("pg_uuids_3", id: :uuid) do |t| + connection.create_table("pg_uuids_3", id: :uuid, **uuid_default) do |t| t.string "name" end end @@ -320,10 +324,10 @@ class PostgresqlUUIDTestInverseOf < ActiveRecord::PostgreSQLTestCase setup do connection.transaction do - connection.create_table("pg_uuid_posts", id: :uuid) do |t| + connection.create_table("pg_uuid_posts", id: :uuid, **uuid_default) do |t| t.string "title" end - connection.create_table("pg_uuid_comments", id: :uuid) do |t| + connection.create_table("pg_uuid_comments", id: :uuid, **uuid_default) do |t| t.references :uuid_post, type: :uuid t.string "content" end diff --git a/activerecord/test/cases/coders/yaml_column_test.rb b/activerecord/test/cases/coders/yaml_column_test.rb index 59ef389326..a26a72712d 100644 --- a/activerecord/test/cases/coders/yaml_column_test.rb +++ b/activerecord/test/cases/coders/yaml_column_test.rb @@ -1,4 +1,3 @@ - require "cases/helper" module ActiveRecord diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index f9eccfbda1..721861975a 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -671,6 +671,47 @@ class DirtyTest < ActiveRecord::TestCase assert binary.changed? end + test "changes is correct for subclass" do + foo = Class.new(Pirate) do + def catchphrase + super.upcase + end + end + + pirate = foo.create!(catchphrase: "arrrr") + + new_catchphrase = "arrrr matey!" + + pirate.catchphrase = new_catchphrase + assert pirate.catchphrase_changed? + + expected_changes = { + "catchphrase" => ["arrrr", new_catchphrase] + } + + assert_equal new_catchphrase.upcase, pirate.catchphrase + assert_equal expected_changes, pirate.changes + end + + test "changes is correct if override attribute reader" do + pirate = Pirate.create!(catchphrase: "arrrr") + def pirate.catchphrase + super.upcase + end + + new_catchphrase = "arrrr matey!" + + pirate.catchphrase = new_catchphrase + assert pirate.catchphrase_changed? + + expected_changes = { + "catchphrase" => ["arrrr", new_catchphrase] + } + + assert_equal new_catchphrase.upcase, pirate.catchphrase + assert_equal expected_changes, pirate.changes + end + test "attribute_changed? doesn't compute in-place changes for unrelated attributes" do test_type_class = Class.new(ActiveRecord::Type::Value) do define_method(:changed_in_place?) do |*| diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 4fa664697c..a7b6333010 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -208,6 +208,11 @@ class FinderTest < ActiveRecord::TestCase assert_equal true, Topic.order(:id).distinct.exists? end + # Ensure +exists?+ runs without an error by excluding order value. + def test_exists_with_order + assert_equal true, Topic.order("invalid sql here").exists? + end + def test_exists_with_joins assert_equal true, Topic.joins(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists? end diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb index d7aa091623..0678bb714f 100644 --- a/activerecord/test/cases/integration_test.rb +++ b/activerecord/test/cases/integration_test.rb @@ -1,4 +1,3 @@ - require "cases/helper" require "models/company" require "models/developer" diff --git a/activerecord/test/cases/migration/rename_table_test.rb b/activerecord/test/cases/migration/rename_table_test.rb index 19588d28a2..7bcabd0cc6 100644 --- a/activerecord/test/cases/migration/rename_table_test.rb +++ b/activerecord/test/cases/migration/rename_table_test.rb @@ -80,7 +80,7 @@ module ActiveRecord end def test_renaming_table_doesnt_attempt_to_rename_non_existent_sequences - connection.create_table :cats, id: :uuid + connection.create_table :cats, id: :uuid, default: "uuid_generate_v4()" assert_nothing_raised { rename_table :cats, :felines } assert connection.table_exists? :felines ensure diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index c7b2ac90fb..cbc466d6b8 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -289,6 +289,11 @@ module ActiveRecord assert_equal essays(:david_modest_proposal), essay end + def test_where_on_association_with_select_relation + essay = Essay.where(author: Author.where(name: "David").select(:name)).take + assert_equal essays(:david_modest_proposal), essay + end + def test_where_with_strong_parameters protected_params = Class.new do attr_reader :permitted diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 856469c710..fcf68b0f2a 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1902,7 +1902,7 @@ class RelationTest < ActiveRecord::TestCase end test "relations don't load all records in #inspect" do - assert_sql(/LIMIT/) do + assert_sql(/LIMIT|ROWNUM <=|FETCH FIRST/) do Post.all.inspect end end diff --git a/activerecord/test/cases/view_test.rb b/activerecord/test/cases/view_test.rb index 07288568e8..1d21a2454f 100644 --- a/activerecord/test/cases/view_test.rb +++ b/activerecord/test/cases/view_test.rb @@ -154,7 +154,9 @@ if ActiveRecord::Base.connection.supports_views? end # sqlite dose not support CREATE, INSERT, and DELETE for VIEW - if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter, :SQLServerAdapter) + if current_adapter?(:Mysql2Adapter, :SQLServerAdapter) || + current_adapter?(:PostgreSQLAdapter) && ActiveRecord::Base.connection.postgresql_version >= 90300 + class UpdateableViewTest < ActiveRecord::TestCase self.use_transactional_tests = false fixtures :books diff --git a/activerecord/test/models/essay.rb b/activerecord/test/models/essay.rb index 13267fbc21..1f9772870e 100644 --- a/activerecord/test/models/essay.rb +++ b/activerecord/test/models/essay.rb @@ -1,4 +1,5 @@ class Essay < ActiveRecord::Base + belongs_to :author belongs_to :writer, primary_key: :name, polymorphic: true belongs_to :category, primary_key: :name has_one :owner, primary_key: :name diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb index 860c63b27c..e56e8fa36a 100644 --- a/activerecord/test/schema/postgresql_specific_schema.rb +++ b/activerecord/test/schema/postgresql_specific_schema.rb @@ -3,11 +3,13 @@ ActiveRecord::Schema.define do enable_extension!("uuid-ossp", ActiveRecord::Base.connection) enable_extension!("pgcrypto", ActiveRecord::Base.connection) if ActiveRecord::Base.connection.supports_pgcrypto_uuid? - create_table :uuid_parents, id: :uuid, force: true do |t| + uuid_default = connection.supports_pgcrypto_uuid? ? {} : { default: "uuid_generate_v4()" } + + create_table :uuid_parents, id: :uuid, force: true, **uuid_default do |t| t.string :name end - create_table :uuid_children, id: :uuid, force: true do |t| + create_table :uuid_children, id: :uuid, force: true, **uuid_default do |t| t.string :name t.uuid :uuid_parent_id end @@ -102,7 +104,7 @@ _SQL end create_table :uuid_items, force: true, id: false do |t| - t.uuid :uuid, primary_key: true + t.uuid :uuid, primary_key: true, **uuid_default t.string :title end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 6f99155199..6146dd989f 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1 +1,17 @@ +* Add support for `:offset` and `:zone` to `ActiveSupport::TimeWithZone#change` + + *Andrew White* + +* Add support for `:offset` to `Time#change` + + Fixes #28723. + + *Andrew White* + +* Add `fetch_values` for `HashWithIndifferentAccess` + + The method was originally added to `Hash` in Ruby 2.3.0. + + *Josh Pencheon* + Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/activesupport/CHANGELOG.md) for previous changes. diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index cdf27f49ad..85ab739095 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -267,7 +267,10 @@ class Module module_eval <<-RUBY, __FILE__, __LINE__ + 1 def respond_to_missing?(name, include_private = false) - #{target}.respond_to?(name, include_private) + # It may look like an oversight, but we deliberately do not pass + # +include_private+, because they do not get delegated. + + #{target}.respond_to?(name) || super end def method_missing(method, *args, &block) diff --git a/activesupport/lib/active_support/core_ext/time/calculations.rb b/activesupport/lib/active_support/core_ext/time/calculations.rb index 7b7aeef25a..d3f23f4663 100644 --- a/activesupport/lib/active_support/core_ext/time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/time/calculations.rb @@ -107,21 +107,22 @@ class Time # to the +options+ parameter. The time options (<tt>:hour</tt>, <tt>:min</tt>, # <tt>:sec</tt>, <tt>:usec</tt>, <tt>:nsec</tt>) reset cascadingly, so if only # the hour is passed, then minute, sec, usec and nsec is set to 0. If the hour - # and minute is passed, then sec, usec and nsec is set to 0. The +options+ - # parameter takes a hash with any of these keys: <tt>:year</tt>, <tt>:month</tt>, - # <tt>:day</tt>, <tt>:hour</tt>, <tt>:min</tt>, <tt>:sec</tt>, <tt>:usec</tt> - # <tt>:nsec</tt>. Pass either <tt>:usec</tt> or <tt>:nsec</tt>, not both. + # and minute is passed, then sec, usec and nsec is set to 0. The +options+ parameter + # takes a hash with any of these keys: <tt>:year</tt>, <tt>:month</tt>, <tt>:day</tt>, + # <tt>:hour</tt>, <tt>:min</tt>, <tt>:sec</tt>, <tt>:usec</tt>, <tt>:nsec</tt>, + # <tt>:offset</tt>. Pass either <tt>:usec</tt> or <tt>:nsec</tt>, not both. # # Time.new(2012, 8, 29, 22, 35, 0).change(day: 1) # => Time.new(2012, 8, 1, 22, 35, 0) # Time.new(2012, 8, 29, 22, 35, 0).change(year: 1981, day: 1) # => Time.new(1981, 8, 1, 22, 35, 0) # Time.new(2012, 8, 29, 22, 35, 0).change(year: 1981, hour: 0) # => Time.new(1981, 8, 29, 0, 0, 0) def change(options) - new_year = options.fetch(:year, year) - new_month = options.fetch(:month, month) - new_day = options.fetch(:day, day) - new_hour = options.fetch(:hour, hour) - new_min = options.fetch(:min, options[:hour] ? 0 : min) - new_sec = options.fetch(:sec, (options[:hour] || options[:min]) ? 0 : sec) + new_year = options.fetch(:year, year) + new_month = options.fetch(:month, month) + new_day = options.fetch(:day, day) + new_hour = options.fetch(:hour, hour) + new_min = options.fetch(:min, options[:hour] ? 0 : min) + new_sec = options.fetch(:sec, (options[:hour] || options[:min]) ? 0 : sec) + new_offset = options.fetch(:offset, nil) if new_nsec = options[:nsec] raise ArgumentError, "Can't change both :nsec and :usec at the same time: #{options.inspect}" if options[:usec] @@ -130,13 +131,18 @@ class Time new_usec = options.fetch(:usec, (options[:hour] || options[:min] || options[:sec]) ? 0 : Rational(nsec, 1000)) end - if utc? - ::Time.utc(new_year, new_month, new_day, new_hour, new_min, new_sec, new_usec) + raise ArgumentError, "argument out of range" if new_usec >= 1000000 + + new_sec += Rational(new_usec, 1000000) + + if new_offset + ::Time.new(new_year, new_month, new_day, new_hour, new_min, new_sec, new_offset) + elsif utc? + ::Time.utc(new_year, new_month, new_day, new_hour, new_min, new_sec) elsif zone - ::Time.local(new_year, new_month, new_day, new_hour, new_min, new_sec, new_usec) + ::Time.local(new_year, new_month, new_day, new_hour, new_min, new_sec) else - raise ArgumentError, "argument out of range" if new_usec >= 1000000 - ::Time.new(new_year, new_month, new_day, new_hour, new_min, new_sec + (new_usec.to_r / 1000000), utc_offset) + ::Time.new(new_year, new_month, new_day, new_hour, new_min, new_sec, utc_offset) end end diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 1e1d6d3aac..3b185dd86b 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -195,6 +195,19 @@ module ActiveSupport indices.collect { |key| self[convert_key(key)] } end + # Returns an array of the values at the specified indices, but also + # raises an exception when one of the keys can't be found. + # + # hash = ActiveSupport::HashWithIndifferentAccess.new + # hash[:a] = 'x' + # hash[:b] = 'y' + # hash.fetch_values('a', 'b') # => ["x", "y"] + # hash.fetch_values('a', 'c') { |key| 'z' } # => ["x", "z"] + # hash.fetch_values('a', 'c') # => KeyError: key not found: "c" + def fetch_values(*indices, &block) + indices.collect { |key| fetch(key, &block) } + end if Hash.method_defined?(:fetch_values) + # Returns a shallow copy of the hash. # # hash = ActiveSupport::HashWithIndifferentAccess.new({ a: { b: 'b' } }) diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb index b0dd6b7e8c..ecb9fb6785 100644 --- a/activesupport/lib/active_support/time_with_zone.rb +++ b/activesupport/lib/active_support/time_with_zone.rb @@ -330,6 +330,42 @@ module ActiveSupport since(-other) end + # Returns a new +ActiveSupport::TimeWithZone+ where one or more of the elements have + # been changed according to the +options+ parameter. The time options (<tt>:hour</tt>, + # <tt>:min</tt>, <tt>:sec</tt>, <tt>:usec</tt>, <tt>:nsec</tt>) reset cascadingly, + # so if only the hour is passed, then minute, sec, usec and nsec is set to 0. If the + # hour and minute is passed, then sec, usec and nsec is set to 0. The +options+ + # parameter takes a hash with any of these keys: <tt>:year</tt>, <tt>:month</tt>, + # <tt>:day</tt>, <tt>:hour</tt>, <tt>:min</tt>, <tt>:sec</tt>, <tt>:usec</tt>, + # <tt>:nsec</tt>, <tt>:offset</tt>, <tt>:zone</tt>. Pass either <tt>:usec</tt> + # or <tt>:nsec</tt>, not both. Similarly, pass either <tt>:zone</tt> or + # <tt>:offset</tt>, not both. + # + # t = Time.zone.now # => Fri, 14 Apr 2017 11:45:15 EST -05:00 + # t.change(year: 2020) # => Tue, 14 Apr 2020 11:45:15 EST -05:00 + # t.change(hour: 12) # => Fri, 14 Apr 2017 12:00:00 EST -05:00 + # t.change(min: 30) # => Fri, 14 Apr 2017 11:30:00 EST -05:00 + # t.change(offset: "-10:00") # => Fri, 14 Apr 2017 11:45:15 HST -10:00 + # t.change(zone: "Hawaii") # => Fri, 14 Apr 2017 11:45:15 HST -10:00 + def change(options) + if options[:zone] && options[:offset] + raise ArgumentError, "Can't change both :offset and :zone at the same time: #{options.inspect}" + end + + new_time = time.change(options) + + if options[:zone] + new_zone = ::Time.find_zone(options[:zone]) + elsif options[:offset] + new_zone = ::Time.find_zone(new_time.utc_offset) + end + + new_zone ||= time_zone + periods = new_zone.periods_for_local(new_time) + + self.class.new(nil, new_zone, new_time, periods.include?(period) ? period : nil) + end + # Uses Date to provide precise Time calculations for years, months, and days # according to the proleptic Gregorian calendar. The result is returned as a # new TimeWithZone object. diff --git a/activesupport/test/core_ext/date_time_ext_test.rb b/activesupport/test/core_ext/date_time_ext_test.rb index 36f0ee22b8..be7c14e9b4 100644 --- a/activesupport/test/core_ext/date_time_ext_test.rb +++ b/activesupport/test/core_ext/date_time_ext_test.rb @@ -166,6 +166,9 @@ class DateTimeExtCalculationsTest < ActiveSupport::TestCase assert_equal DateTime.civil(2005, 2, 22, 16, 45), DateTime.civil(2005, 2, 22, 15, 15, 10).change(hour: 16, min: 45) assert_equal DateTime.civil(2005, 2, 22, 15, 45), DateTime.civil(2005, 2, 22, 15, 15, 10).change(min: 45) + # datetime with non-zero offset + assert_equal DateTime.civil(2005, 2, 22, 15, 15, 10, Rational(-5, 24)), DateTime.civil(2005, 2, 22, 15, 15, 10, 0).change(offset: Rational(-5, 24)) + # datetime with fractions of a second assert_equal DateTime.civil(2005, 2, 1, 15, 15, 10.7), DateTime.civil(2005, 2, 22, 15, 15, 10.7).change(day: 1) assert_equal DateTime.civil(2005, 1, 2, 11, 22, Rational(33000008, 1000000)), DateTime.civil(2005, 1, 2, 11, 22, 33).change(usec: 8) diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index a17438bf4d..085fd6592d 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -70,7 +70,23 @@ Product = Struct.new(:name) do end end +module ExtraMissing + def method_missing(sym, *args) + if sym == :extra_missing + 42 + else + super + end + end + + def respond_to_missing?(sym, priv = false) + sym == :extra_missing || super + end +end + DecoratedTester = Struct.new(:client) do + include ExtraMissing + delegate_missing_to :client end @@ -356,6 +372,22 @@ class ModuleTest < ActiveSupport::TestCase assert_match(/undefined method `my_fake_method' for/, e.message) end + def test_delegate_to_missing_affects_respond_to + assert DecoratedTester.new(@david).respond_to?(:name) + assert_not DecoratedTester.new(@david).respond_to?(:private_name) + assert_not DecoratedTester.new(@david).respond_to?(:my_fake_method) + + assert DecoratedTester.new(@david).respond_to?(:name, true) + assert_not DecoratedTester.new(@david).respond_to?(:private_name, true) + assert_not DecoratedTester.new(@david).respond_to?(:my_fake_method, true) + end + + def test_delegate_to_missing_respects_superclass_missing + assert_equal 42, DecoratedTester.new(@david).extra_missing + + assert_respond_to DecoratedTester.new(@david), :extra_missing + end + def test_delegate_with_case event = Event.new(Tester.new) assert_equal 1, event.foo diff --git a/activesupport/test/core_ext/time_ext_test.rb b/activesupport/test/core_ext/time_ext_test.rb index bd644c8457..625a5bffb8 100644 --- a/activesupport/test/core_ext/time_ext_test.rb +++ b/activesupport/test/core_ext/time_ext_test.rb @@ -433,6 +433,13 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase assert_raise(ArgumentError) { Time.new(2005, 2, 22, 15, 15, 45, "-08:00").change(nsec: 1000000000) } end + def test_change_offset + assert_equal Time.new(2006, 2, 22, 15, 15, 10, "-08:00"), Time.new(2006, 2, 22, 15, 15, 10, "+01:00").change(offset: "-08:00") + assert_equal Time.new(2006, 2, 22, 15, 15, 10, -28800), Time.new(2006, 2, 22, 15, 15, 10, 3600).change(offset: -28800) + assert_raise(ArgumentError) { Time.new(2005, 2, 22, 15, 15, 45, "+01:00").change(usec: 1000000, offset: "-08:00") } + assert_raise(ArgumentError) { Time.new(2005, 2, 22, 15, 15, 45, "+01:00").change(nsec: 1000000000, offset: -28800) } + end + def test_advance assert_equal Time.local(2006, 2, 28, 15, 15, 10), Time.local(2005, 2, 28, 15, 15, 10).advance(years: 1) assert_equal Time.local(2005, 6, 28, 15, 15, 10), Time.local(2005, 2, 28, 15, 15, 10).advance(months: 4) diff --git a/activesupport/test/core_ext/time_with_zone_test.rb b/activesupport/test/core_ext/time_with_zone_test.rb index c3afe68378..70ae793cda 100644 --- a/activesupport/test/core_ext/time_with_zone_test.rb +++ b/activesupport/test/core_ext/time_with_zone_test.rb @@ -625,6 +625,12 @@ class TimeWithZoneTest < ActiveSupport::TestCase assert_equal "Fri, 31 Dec 1999 06:00:00 EST -05:00", @twz.change(hour: 6).inspect assert_equal "Fri, 31 Dec 1999 19:15:00 EST -05:00", @twz.change(min: 15).inspect assert_equal "Fri, 31 Dec 1999 19:00:30 EST -05:00", @twz.change(sec: 30).inspect + assert_equal "Fri, 31 Dec 1999 19:00:00 HST -10:00", @twz.change(offset: "-10:00").inspect + assert_equal "Fri, 31 Dec 1999 19:00:00 HST -10:00", @twz.change(offset: -36000).inspect + assert_equal "Fri, 31 Dec 1999 19:00:00 HST -10:00", @twz.change(zone: "Hawaii").inspect + assert_equal "Fri, 31 Dec 1999 19:00:00 HST -10:00", @twz.change(zone: -10).inspect + assert_equal "Fri, 31 Dec 1999 19:00:00 HST -10:00", @twz.change(zone: -36000).inspect + assert_equal "Fri, 31 Dec 1999 19:00:00 HST -10:00", @twz.change(zone: "Pacific/Honolulu").inspect end def test_change_at_dst_boundary diff --git a/activesupport/test/hash_with_indifferent_access_test.rb b/activesupport/test/hash_with_indifferent_access_test.rb index b1177c6c15..d68add46cd 100644 --- a/activesupport/test/hash_with_indifferent_access_test.rb +++ b/activesupport/test/hash_with_indifferent_access_test.rb @@ -166,6 +166,18 @@ class HashWithIndifferentAccessTest < ActiveSupport::TestCase assert_equal [1, 2], @mixed.values_at(:a, :b) end + def test_indifferent_fetch_values + skip unless Hash.method_defined?(:fetch_values) + + @mixed = @mixed.with_indifferent_access + + assert_equal [1, 2], @mixed.fetch_values("a", "b") + assert_equal [1, 2], @mixed.fetch_values(:a, :b) + assert_equal [1, 2], @mixed.fetch_values(:a, "b") + assert_equal [1, "c"], @mixed.fetch_values(:a, :c) { |key| key } + assert_raise(KeyError) { @mixed.fetch_values(:a, :c) } + end + def test_indifferent_reading hash = HashWithIndifferentAccess.new hash["a"] = 1 diff --git a/guides/source/active_record_callbacks.md b/guides/source/active_record_callbacks.md index de0ef7c4e5..b1705855d0 100644 --- a/guides/source/active_record_callbacks.md +++ b/guides/source/active_record_callbacks.md @@ -117,6 +117,10 @@ Here is a list with all the available Active Record callbacks, listed in the sam WARNING. `after_save` runs both on create and update, but always _after_ the more specific callbacks `after_create` and `after_update`, no matter the order in which the macro calls were executed. +NOTE: `before_destroy` callbacks should be placed before `dependent: :destroy` +associations (or use the `prepend: true` option), to ensure they execute before +the records are deleted by `dependent: :destroy`. + ### `after_initialize` and `after_find` The `after_initialize` callback will be called whenever an Active Record object is instantiated, either by directly using `new` or when a record is loaded from the database. It can be useful to avoid the need to directly override your Active Record `initialize` method. diff --git a/guides/source/engines.md b/guides/source/engines.md index 180a786237..2276f348a1 100644 --- a/guides/source/engines.md +++ b/guides/source/engines.md @@ -14,6 +14,7 @@ After reading this guide, you will know: * How to build features for the engine. * How to hook the engine into an application. * How to override engine functionality in the application. +* Avoid loading Rails frameworks with Load and Configuration Hooks -------------------------------------------------------------------------------- @@ -1410,3 +1411,114 @@ module MyEngine end end ``` + +Active Support On Load Hooks +---------------------------- + +Active Support is the Ruby on Rails component responsible for providing Ruby language extensions, utilities, and other transversal utilities. + +Rails code can often be referenced on load of an application. Rails is responsible for the load order of these frameworks, so when you load frameworks, such as `ActiveRecord::Base`, prematurely you are violating an implicit contract your application has with Rails. Moreover, by loading code such as `ActiveRecord::Base` on boot of your application you are loading entire frameworks which may slow down your boot time and could cause conflicts with load order and boot of your application. + +On Load hooks are the API that allow you to hook into this initialization process without violating the load contract with Rails. This will also mitigate boot performance degradation and avoid conflicts. + +## What are `on_load` hooks? + +Since Ruby is a dynamic language, some code will cause different Rails frameworks to load. Take this snippet for instance: + +```ruby +ActiveRecord::Base.include(MyActiveRecordHelper) +``` + +This snippet means that when this file is loaded, it will encounter `ActiveRecord::Base`. This encounter causes Ruby to look for the definition of that constant and will require it. This causes the entire Active Record framework to be loaded on boot. + +`ActiveSupport.on_load` is a mechanism that can be used to defer the loading of code until it is actually needed. The snippet above can be changed to: + +```ruby +ActiveSupport.on_load(:active_record) { include MyActiveRecordHelper } +``` + +This new snippet will only include `MyActiveRecordHelper` when `ActiveRecord::Base` is loaded. + +## How does it work? + +In the Rails framework these hooks are called when a specific library is loaded. For example, when `ActionController::Base` is loaded, the `:action_controller_base` hook is called. This means that all `ActiveSupport.on_load` calls with `:action_controller_base` hooks will be called in the context of `ActionController::Base` (that means `self` will be an `ActionController::Base`). + +## Modifying code to use `on_load` hooks + +Modifying code is generally straightforward. If you have a line of code that refers to a Rails framework such as `ActiveRecord::Base` you can wrap that code in an `on_load` hook. + +### Example 1 + +```ruby +ActiveRecord::Base.include(MyActiveRecordHelper) +``` + +becomes + +```ruby +ActiveSupport.on_load(:active_record) { include MyActiveRecordHelper } # self refers to ActiveRecord::Base here, so we can simply #include +``` + +### Example 2 + +```ruby +ActionController::Base.prepend(MyActionControllerHelper) +``` + +becomes + +```ruby +ActiveSupport.on_load(:action_controller_base) { prepend MyActionControllerHelper } # self refers to ActionController::Base here, so we can simply #prepend +``` + +### Example 3 + +```ruby +ActiveRecord::Base.include_root_in_json = true +``` + +becomes + +```ruby +ActiveSupport.on_load(:active_record) { self.include_root_in_json = true } # self refers to ActiveRecord::Base here +``` + +## Available Hooks + +These are the hooks you can use in your own code. + +To hook into the initialization process of one of the following classes use the available hook. + +| Class | Available Hooks | +| --------------------------------- | ------------------------------------ | +| `ActionCable` | `action_cable` | +| `ActionController::API` | `action_controller_api` | +| `ActionController::API` | `action_controller` | +| `ActionController::Base` | `action_controller_base` | +| `ActionController::Base` | `action_controller` | +| `ActionController::TestCase` | `action_controller_test_case` | +| `ActionDispatch::IntegrationTest` | `action_dispatch_integration_test` | +| `ActionMailer::Base` | `action_mailer` | +| `ActionMailer::TestCase` | `action_mailer_test_case` | +| `ActionView::Base` | `action_view` | +| `ActionView::TestCase` | `action_view_test_case` | +| `ActiveJob::Base` | `active_job` | +| `ActiveJob::TestCase` | `active_job_test_case` | +| `ActiveRecord::Base` | `active_record` | +| `ActiveSupport::TestCase` | `active_support_test_case` | +| `i18n` | `i18n` | + +## Configuration hooks + +These are the available configuration hooks. They do not hook into any particular framework, instead they run in context of the entire application. + +| Hook | Use Case | +| ---------------------- | ------------------------------------------------------------------------------------- | +| `before_configuration` | First configurable block to run. Called before any initializers are run. | +| `before_initialize` | Second configurable block to run. Called before frameworks initialize. | +| `before_eager_load` | Third configurable block to run. Does not run if `config.cache_classes` set to false. | +| `after_initialize` | Last configurable block to run. Called after frameworks initialize. | + +### Example + +`config.before_configuration { puts 'I am called before any initializers' }` diff --git a/guides/source/rails_on_rack.md b/guides/source/rails_on_rack.md index 340933c7ee..f25b185fb5 100644 --- a/guides/source/rails_on_rack.md +++ b/guides/source/rails_on_rack.md @@ -20,9 +20,9 @@ Introduction to Rack Rack provides a minimal, modular and adaptable interface for developing web applications in Ruby. By wrapping HTTP requests and responses in the simplest way possible, it unifies and distills the API for web servers, web frameworks, and software in between (the so-called middleware) into a single method call. -* [Rack API Documentation](http://rack.github.io/) - -Explaining Rack is not really in the scope of this guide. In case you are not familiar with Rack's basics, you should check out the [Resources](#resources) section below. +Explaining how Rack works is not really in the scope of this guide. In case you +are not familiar with Rack's basics, you should check out the [Resources](#resources) +section below. Rails on Rack ------------- @@ -74,7 +74,7 @@ And start the server: $ rackup config.ru ``` -To find out more about different `rackup` options: +To find out more about different `rackup` options, you can run: ```bash $ rackup --help @@ -89,7 +89,8 @@ Action Dispatcher Middleware Stack Many of Action Dispatcher's internal components are implemented as Rack middlewares. `Rails::Application` uses `ActionDispatch::MiddlewareStack` to combine various internal and external middlewares to form a complete Rails Rack application. -NOTE: `ActionDispatch::MiddlewareStack` is Rails equivalent of `Rack::Builder`, but built for better flexibility and more features to meet Rails' requirements. +NOTE: `ActionDispatch::MiddlewareStack` is Rails' equivalent of `Rack::Builder`, +but is built for better flexibility and more features to meet Rails' requirements. ### Inspecting Middleware Stack diff --git a/guides/source/routing.md b/guides/source/routing.md index 545f53a8e0..f7dbbc510e 100644 --- a/guides/source/routing.md +++ b/guides/source/routing.md @@ -142,10 +142,10 @@ Sometimes, you have a resource that clients always look up without referencing a get 'profile', to: 'users#show' ``` -Passing a `String` to `get` will expect a `controller#action` format, while passing a `Symbol` will map directly to an action but you must also specify the `controller:` to use: +Passing a `String` to `to:` will expect a `controller#action` format. When using a `Symbol`, the `to:` option should be replaced with `action:`. When using a `String` without a `#`, the `to:` option should be replaced with `controller:`: ```ruby -get 'profile', to: :show, controller: 'users' +get 'profile', action: :show, controller: 'users' ``` This resourceful route: diff --git a/railties/lib/rails/code_statistics.rb b/railties/lib/rails/code_statistics.rb index 9c4bd16aad..70dce268f1 100644 --- a/railties/lib/rails/code_statistics.rb +++ b/railties/lib/rails/code_statistics.rb @@ -7,7 +7,8 @@ class CodeStatistics #:nodoc: "Model tests", "Mailer tests", "Job tests", - "Integration tests"] + "Integration tests", + "System tests"] HEADERS = { lines: " Lines", code_lines: " LOC", classes: "Classes", methods: "Methods" } diff --git a/railties/lib/rails/generators/named_base.rb b/railties/lib/rails/generators/named_base.rb index 46001f306a..aef66adb64 100644 --- a/railties/lib/rails/generators/named_base.rb +++ b/railties/lib/rails/generators/named_base.rb @@ -150,7 +150,7 @@ module Rails end def field_id(attribute_name) - [singular_table_name, attribute_name].join('_') + [singular_table_name, attribute_name].join("_") end def singular_table_name # :doc: |