diff options
57 files changed, 262 insertions, 132 deletions
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 798241aaf8..a70bf1544a 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -84,7 +84,8 @@ module ActionMailer # name as the method in your mailer model. For example, in the mailer defined above, the template at # <tt>app/views/notifier/welcome.text.erb</tt> would be used to generate the email. # - # Variables defined in the model are accessible as instance variables in the view. + # Variables defined in the methods of your mailer model are accessible as instance variables in their + # corresponding view. # # Emails by default are sent in plain text, so a sample view for our model example might look like this: # @@ -702,8 +703,8 @@ module ActionMailer # The main method that creates the message and renders the email templates. There are # two ways to call this method, with a block, or without a block. # - # Both methods accept a headers hash. This hash allows you to specify the most used headers - # in an email message, these are: + # It accepts a headers hash. This hash allows you to specify + # the most used headers in an email message, these are: # # * +:subject+ - The subject of the message, if this is omitted, Action Mailer will # ask the Rails I18n class for a translated +:subject+ in the scope of diff --git a/actionmailer/lib/action_mailer/delivery_job.rb b/actionmailer/lib/action_mailer/delivery_job.rb index 622f202695..95231411fb 100644 --- a/actionmailer/lib/action_mailer/delivery_job.rb +++ b/actionmailer/lib/action_mailer/delivery_job.rb @@ -1,6 +1,8 @@ require 'active_job' module ActionMailer + # The <tt>ActionMailer::DeliveryJob</tt> class is used when you + # want to send emails outside of the request-response cycle. class DeliveryJob < ActiveJob::Base #:nodoc: queue_as :mailers diff --git a/actionpack/lib/abstract_controller/helpers.rb b/actionpack/lib/abstract_controller/helpers.rb index 95c67d482b..df7382f02d 100644 --- a/actionpack/lib/abstract_controller/helpers.rb +++ b/actionpack/lib/abstract_controller/helpers.rb @@ -150,7 +150,17 @@ module AbstractController rescue LoadError => e raise AbstractController::Helpers::MissingHelperError.new(e, file_name) end - file_name.camelize.constantize + + mod_name = file_name.camelize + begin + mod_name.constantize + rescue LoadError + # dependencies.rb gives a similar error message but its wording is + # not as clear because it mentions autoloading. To the user all it + # matters is that a helper module couldn't be loaded, autoloading + # is an internal mechanism that should not leak. + raise NameError, "Couldn't find #{mod_name}, expected it to be defined in helpers/#{file_name}.rb" + end when Module arg else diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index a4f376816f..fd20682f8f 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -1,5 +1,6 @@ require 'rack/session/abstract/id' require 'action_controller/metal/exceptions' +require 'active_support/security_utils' module ActionController #:nodoc: class InvalidAuthenticityToken < ActionControllerError #:nodoc: @@ -305,8 +306,7 @@ module ActionController #:nodoc: end def compare_with_real_token(token, session) - # Borrow a constant-time comparison from Rack - Rack::Utils.secure_compare(token, real_csrf_token(session)) + ActiveSupport::SecurityUtils.secure_compare(token, real_csrf_token(session)) end def real_csrf_token(session) diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index 59b353b1b7..992c1a9efe 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -40,7 +40,7 @@ module ActionDispatch end message = "No route matches #{Hash[constraints.sort].inspect}" - message << " missing required keys: #{missing_keys.sort.inspect}" if name + message << " missing required keys: #{missing_keys.sort.inspect}" unless missing_keys.empty? raise ActionController::UrlGenerationError, message end diff --git a/actionpack/test/controller/helper_test.rb b/actionpack/test/controller/helper_test.rb index 20f99f19ee..936b8c2450 100644 --- a/actionpack/test/controller/helper_test.rb +++ b/actionpack/test/controller/helper_test.rb @@ -60,6 +60,12 @@ class HelpersPathsController < ActionController::Base end end +class HelpersTypoController < ActionController::Base + path = File.expand_path('../../fixtures/helpers_typo', __FILE__) + $:.unshift(path) + self.helpers_path = path +end + module LocalAbcHelper def a() end def b() end @@ -82,6 +88,22 @@ class HelperPathsTest < ActiveSupport::TestCase end end +class HelpersTypoControllerTest < ActiveSupport::TestCase + def setup + @autoload_paths = ActiveSupport::Dependencies.autoload_paths + ActiveSupport::Dependencies.autoload_paths = Array(HelpersTypoController.helpers_path) + end + + def test_helper_typo_error_message + e = assert_raise(NameError) { HelpersTypoController.helper 'admin/users' } + assert_equal "Couldn't find Admin::UsersHelper, expected it to be defined in helpers/admin/users_helper.rb", e.message + end + + def teardown + ActiveSupport::Dependencies.autoload_paths = @autoload_paths + end +end + class HelperTest < ActiveSupport::TestCase class TestController < ActionController::Base attr_accessor :delegate_attr diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index d141210ad9..d0a624784a 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4420,7 +4420,7 @@ class TestUrlGenerationErrors < ActionDispatch::IntegrationTest include Routes.url_helpers - test "url helpers raise a helpful error message whem generation fails" do + test "url helpers raise a helpful error message when generation fails" do url, missing = { action: 'show', controller: 'products', id: nil }, [:id] message = "No route matches #{url.inspect} missing required keys: #{missing.inspect}" diff --git a/actionpack/test/fixtures/helpers_typo/admin/users_helper.rb b/actionpack/test/fixtures/helpers_typo/admin/users_helper.rb new file mode 100644 index 0000000000..7d2326e04d --- /dev/null +++ b/actionpack/test/fixtures/helpers_typo/admin/users_helper.rb @@ -0,0 +1,5 @@ +module Admin + module UsersHelpeR + end +end + diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 8e90d4100f..fbac86e8ca 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -201,6 +201,16 @@ module ActionDispatch assert_match(/missing required keys: \[:id\]/, error.message) end + def test_does_not_include_missing_keys_message + route_name = "gorby_thunderhorse" + + error = assert_raises(ActionController::UrlGenerationError) do + @formatter.generate(route_name, { }, { }) + end + + assert_no_match(/missing required keys: \[\]/, error.message) + end + def test_X_Cascade add_routes @router, [ "/messages(.:format)" ] resp = @router.serve(rails_env({ 'REQUEST_METHOD' => 'GET', 'PATH_INFO' => '/lol' })) diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 8ac74dc938..e388e6ecd3 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,9 @@ +* Update `select_tag` to work correctly with `:include_blank` option passing a string. + + Fixes #16483. + + *Frank Groeneveld* + * Changed the meaning of `render "foo/bar"`. Previously, calling `render "foo/bar"` in a controller action is equivalent diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index 69893b8abd..c0218fd55d 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -133,12 +133,18 @@ module ActionView option_tags ||= "" html_name = (options[:multiple] == true && !name.to_s.ends_with?("[]")) ? "#{name}[]" : name - if options.delete(:include_blank) - option_tags = content_tag(:option, '', :value => '').safe_concat(option_tags) + if options.include?(:include_blank) + include_blank = options.delete(:include_blank) + + if include_blank == true + include_blank = '' + end + + option_tags = content_tag(:option, include_blank, value: '').safe_concat(option_tags) end if prompt = options.delete(:prompt) - option_tags = content_tag(:option, prompt, :value => '').safe_concat(option_tags) + option_tags = content_tag(:option, prompt, value: '').safe_concat(option_tags) end content_tag :select, option_tags, { "name" => html_name, "id" => sanitize_to_id(name) }.update(options.stringify_keys) diff --git a/actionview/lib/action_view/helpers/sanitize_helper.rb b/actionview/lib/action_view/helpers/sanitize_helper.rb index 4f2db0a0c4..7cb55cc214 100644 --- a/actionview/lib/action_view/helpers/sanitize_helper.rb +++ b/actionview/lib/action_view/helpers/sanitize_helper.rb @@ -57,7 +57,7 @@ module ActionView # Add table tags to the default allowed tags # # class Application < Rails::Application - # config.action_view.sanitized_allowed_tags = 'table', 'tr', 'td' + # config.action_view.sanitized_allowed_tags = ['table', 'tr', 'td'] # end # # Remove tags to the default allowed tags @@ -176,7 +176,7 @@ module ActionView # Replaces the allowed tags for the +sanitize+ helper. # # class Application < Rails::Application - # config.action_view.sanitized_allowed_tags = 'table', 'tr', 'td' + # config.action_view.sanitized_allowed_tags = ['table', 'tr', 'td'] # end # diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index f3a48ecfa0..cd21d7ab47 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -18,7 +18,7 @@ module ActionView # Determine the template to be rendered using the given options. def determine_template(options) - keys = options.fetch(:locals, {}).keys + keys = options.has_key?(:locals) ? options[:locals].keys : [] if options.key?(:body) Template::Text.new(options[:body]) diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 379f31bdaf..6b61378a1f 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -313,11 +313,15 @@ module ActionView def locals_code #:nodoc: # Double assign to suppress the dreaded 'assigned but unused variable' warning - @locals.map { |key| "#{key} = #{key} = local_assigns[:#{key}];" }.join + @locals.each_with_object('') { |key, code| code << "#{key} = #{key} = local_assigns[:#{key}];" } end def method_name #:nodoc: - @method_name ||= "_#{identifier_method_name}__#{@identifier.hash}_#{__id__}".tr('-', "_") + @method_name ||= begin + m = "_#{identifier_method_name}__#{@identifier.hash}_#{__id__}" + m.tr!('-', '_') + m + end end def identifier_method_name #:nodoc: diff --git a/actionview/test/template/form_tag_helper_test.rb b/actionview/test/template/form_tag_helper_test.rb index 2d89332841..f8fd642809 100644 --- a/actionview/test/template/form_tag_helper_test.rb +++ b/actionview/test/template/form_tag_helper_test.rb @@ -232,6 +232,12 @@ class FormTagHelperTest < ActionView::TestCase assert_dom_equal expected, actual end + def test_select_tag_with_include_blank_string + actual = select_tag "places", "<option>Home</option><option>Work</option><option>Pub</option>".html_safe, include_blank: 'Choose' + expected = %(<select id="places" name="places"><option value="">Choose</option><option>Home</option><option>Work</option><option>Pub</option></select>) + assert_dom_equal expected, actual + end + def test_select_tag_with_prompt actual = select_tag "places", "<option>Home</option><option>Work</option><option>Pub</option>".html_safe, :prompt => "string" expected = %(<select id="places" name="places"><option value="">string</option><option>Home</option><option>Work</option><option>Pub</option></select>) diff --git a/activejob/lib/active_job/base.rb b/activejob/lib/active_job/base.rb index 913e221d89..0c4a29090e 100644 --- a/activejob/lib/active_job/base.rb +++ b/activejob/lib/active_job/base.rb @@ -34,17 +34,17 @@ module ActiveJob #:nodoc: # Records that are passed in are serialized/deserialized using Global # Id. More information can be found in Arguments. # - # To queue a job to be processed asynchronously immediately: + # To enqueue a job to be performed as soon the queueing system is free: # # ProcessPhotoJob.perform_later(photo) # - # To queue a job to be processed at some point in the future: + # To enqueue a job to be processed at some point in the future: # # ProcessPhotoJob.set(wait_until: Date.tomorrow.noon).perform_later(photo) # # More information can be found in ActiveJob::Core::ClassMethods#set # - # A job can also be processed synchronously: + # A job can also be processed immediately without sending to the queue: # # ProcessPhotoJob.perform_now(photo) # diff --git a/activejob/lib/active_job/enqueuing.rb b/activejob/lib/active_job/enqueuing.rb index 74bcc1fa5d..cca0a2a8d6 100644 --- a/activejob/lib/active_job/enqueuing.rb +++ b/activejob/lib/active_job/enqueuing.rb @@ -10,7 +10,7 @@ module ActiveJob # GlobalID::Identification instances. Arbitrary Ruby objects # are not supported. # - # Returns an instance of the job class queued with args available in + # Returns an instance of the job class queued with arguments available in # Job#arguments. def perform_later(*args) job_or_instantiate(*args).enqueue diff --git a/activejob/lib/active_job/queue_adapters/queue_classic_adapter.rb b/activejob/lib/active_job/queue_adapters/queue_classic_adapter.rb index 1a46f20420..34c11a68b2 100644 --- a/activejob/lib/active_job/queue_adapters/queue_classic_adapter.rb +++ b/activejob/lib/active_job/queue_adapters/queue_classic_adapter.rb @@ -26,7 +26,7 @@ module ActiveJob queue = build_queue(job.queue_name) unless queue.respond_to?(:enqueue_at) raise NotImplementedError, 'To be able to schedule jobs with queue_classic ' \ - 'the QC::Queue needs to respond to `enqueue_at(timestamp, method, *args)`. ' + 'the QC::Queue needs to respond to `enqueue_at(timestamp, method, *args)`. ' \ 'You can implement this yourself or you can use the queue_classic-later gem.' end queue.enqueue_at(timestamp, "#{JobWrapper.name}.perform", job.serialize) diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index af62fae9b9..ade7a94c9d 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -71,7 +71,7 @@ module ActiveJob # # Note: This assertion is simply a shortcut for: # - # assert_enqueued_jobs 0 + # assert_enqueued_jobs 0, &block def assert_no_enqueued_jobs(&block) assert_enqueued_jobs 0, &block end @@ -130,7 +130,7 @@ module ActiveJob # # Note: This assertion is simply a shortcut for: # - # assert_performed_jobs 0 + # assert_performed_jobs 0, &block def assert_no_performed_jobs(&block) assert_performed_jobs 0, &block end diff --git a/activejob/test/cases/logging_test.rb b/activejob/test/cases/logging_test.rb index 9c56ee08b6..3d4e561117 100644 --- a/activejob/test/cases/logging_test.rb +++ b/activejob/test/cases/logging_test.rb @@ -33,7 +33,7 @@ class AdapterTest < ActiveSupport::TestCase def teardown super ActiveJob::Logging::LogSubscriber.log_subscribers.pop - ActiveJob::Base.logger = @old_logger + set_logger @old_logger end def set_logger(logger) diff --git a/activejob/test/cases/rescue_test.rb b/activejob/test/cases/rescue_test.rb index 1b6c2e9fac..58c9ca8992 100644 --- a/activejob/test/cases/rescue_test.rb +++ b/activejob/test/cases/rescue_test.rb @@ -2,8 +2,6 @@ require 'helper' require 'jobs/rescue_job' require 'models/person' -require 'active_support/core_ext/object/inclusion' - class RescueTest < ActiveSupport::TestCase setup do JobBuffer.clear diff --git a/activejob/test/helper.rb b/activejob/test/helper.rb index ce22833b11..ec0c8a8ede 100644 --- a/activejob/test/helper.rb +++ b/activejob/test/helper.rb @@ -26,5 +26,4 @@ end require 'active_support/testing/autorun' -ActiveJob::Base.logger.level = Logger::DEBUG ActiveSupport::TestCase.test_order = :random diff --git a/activejob/test/integration/queuing_test.rb b/activejob/test/integration/queuing_test.rb index 219be11509..38874b51a8 100644 --- a/activejob/test/integration/queuing_test.rb +++ b/activejob/test/integration/queuing_test.rb @@ -3,13 +3,13 @@ require 'jobs/logging_job' require 'active_support/core_ext/numeric/time' class QueuingTest < ActiveSupport::TestCase - test 'should run jobs enqueued on a listenting queue' do + test 'should run jobs enqueued on a listening queue' do TestJob.perform_later @id wait_for_jobs_to_finish_for(5.seconds) assert job_executed end - test 'should not run jobs queued on a non-listenting queue' do + test 'should not run jobs queued on a non-listening queue' do skip if adapter_is?(:inline) || adapter_is?(:sucker_punch) old_queue = TestJob.queue_name diff --git a/activejob/test/jobs/callback_job.rb b/activejob/test/jobs/callback_job.rb index 056dd073e8..891ed9464e 100644 --- a/activejob/test/jobs/callback_job.rb +++ b/activejob/test/jobs/callback_job.rb @@ -1,12 +1,21 @@ class CallbackJob < ActiveJob::Base before_perform ->(job) { job.history << "CallbackJob ran before_perform" } - after_perform ->(job) { job.history << "CallbackJob ran after_perform" } + after_perform ->(job) { job.history << "CallbackJob ran after_perform" } before_enqueue ->(job) { job.history << "CallbackJob ran before_enqueue" } - after_enqueue ->(job) { job.history << "CallbackJob ran after_enqueue" } + after_enqueue ->(job) { job.history << "CallbackJob ran after_enqueue" } - around_perform :around_perform - around_enqueue :around_enqueue + around_perform do |job, block| + job.history << "CallbackJob ran around_perform_start" + block.call + job.history << "CallbackJob ran around_perform_stop" + end + + around_enqueue do |job, block| + job.history << "CallbackJob ran around_enqueue_start" + block.call + job.history << "CallbackJob ran around_enqueue_stop" + end def perform(person = "david") @@ -17,16 +26,4 @@ class CallbackJob < ActiveJob::Base @history ||= [] end - # FIXME: Not sure why these can't be declared inline like before/after - def around_perform - history << "CallbackJob ran around_perform_start" - yield - history << "CallbackJob ran around_perform_stop" - end - - def around_enqueue - history << "CallbackJob ran around_enqueue_start" - yield - history << "CallbackJob ran around_enqueue_stop" - end end diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index 1116f6b8ee..c1e344b215 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -392,7 +392,7 @@ module ActiveModel protected def run_validations! #:nodoc: - run_validate_callbacks + _run_validate_callbacks errors.empty? end end diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb index 1a5192b0ff..25ccabd66b 100644 --- a/activemodel/lib/active_model/validations/callbacks.rb +++ b/activemodel/lib/active_model/validations/callbacks.rb @@ -110,7 +110,7 @@ module ActiveModel # Overwrite run validations to include callbacks. def run_validations! #:nodoc: - run_validation_callbacks { super } + _run_validation_callbacks { super } end end end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 455a540bdb..57affce4f7 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -159,7 +159,7 @@ module ActiveRecord count = scope.destroy_all.length else scope.to_a.each do |record| - record.run_destroy_callbacks + record._run_destroy_callbacks end arel = scope.arel diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index 1aa760157a..523d492a48 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -289,25 +289,25 @@ module ActiveRecord end def destroy #:nodoc: - run_destroy_callbacks { super } + _run_destroy_callbacks { super } end def touch(*) #:nodoc: - run_touch_callbacks { super } + _run_touch_callbacks { super } end private def create_or_update #:nodoc: - run_save_callbacks { super } + _run_save_callbacks { super } end def _create_record #:nodoc: - run_create_callbacks { super } + _run_create_callbacks { super } end def _update_record(*) #:nodoc: - run_update_callbacks { super } + _run_update_callbacks { super } end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index da43e5bb10..eba51baba1 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -360,7 +360,7 @@ module ActiveRecord synchronize do owner = conn.owner - conn.run_checkin_callbacks do + conn._run_checkin_callbacks do conn.expire end @@ -449,7 +449,7 @@ module ActiveRecord end def checkout_and_verify(c) - c.run_checkout_callbacks do + c._run_checkout_callbacks do c.verify! end c diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index ff8b3e9890..1a3ed28d66 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -337,8 +337,9 @@ module ActiveRecord # Returns an ActiveRecord::Result instance. def select(sql, name = nil, binds = []) + exec_query(sql, name, binds) end - undef_method :select + # Returns the last auto-generated ID from the affected table. def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 38bdddefba..5b83131f0e 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -232,11 +232,6 @@ module ActiveRecord alias exec_without_stmt exec_query - # Returns an ActiveRecord::Result instance. - def select(sql, name = nil, binds = []) - exec_query(sql, name) - end - def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) super id_value || @connection.last_id diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index da3aecf69a..cff6798eb3 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -465,7 +465,7 @@ module ActiveRecord def select(sql, name = nil, binds = []) @connection.query_with_result = true - rows = exec_query(sql, name, binds) + rows = super @connection.more_results && @connection.next_result # invoking stored procedures with CLIENT_MULTI_RESULTS requires this to tidy up else connection will be dropped rows end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb index e396ff4a1e..35e699eeda 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb @@ -4,7 +4,7 @@ module ActiveRecord module OID # :nodoc: # This class uses the data from PostgreSQL pg_type table to build # the OID -> Type mapping. - # - OID is and integer representing the type. + # - OID is an integer representing the type. # - Type is an OID::Type object. # This class has side effects on the +store+ passed during initialization. class TypeMapInitializer # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index fe7648291d..3294238fa0 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -698,12 +698,6 @@ module ActiveRecord exec_query("SELECT currval('#{sequence_name}')", 'SQL') end - # Executes a SELECT query and returns the results, performing any data type - # conversions that are required to be performed here instead of in PostgreSQLColumn. - def select(sql, name = nil, binds = []) - exec_query(sql, name, binds) - end - # Returns the list of a table's column names, data types, and default values. # # The underlying query is roughly: diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index ebb311df57..4756896ac5 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -514,10 +514,6 @@ module ActiveRecord register_class_with_limit m, %r(char)i, SQLite3String end - def select(sql, name = nil, binds = []) #:nodoc: - exec_query(sql, name, binds) - end - def table_structure(table_name) structure = exec_query("PRAGMA table_info(#{quote_table_name(table_name)})", 'SCHEMA').to_hash raise(ActiveRecord::StatementInvalid, "Could not find table '#{table_name}'") if structure.empty? diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 5571a2d297..f9b6375459 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -272,7 +272,7 @@ module ActiveRecord init_attributes(attributes, options) if attributes yield self if block_given? - run_initialize_callbacks + _run_initialize_callbacks end # Initialize an empty model object from +coder+. +coder+ must contain @@ -294,8 +294,8 @@ module ActiveRecord self.class.define_attribute_methods - run_find_callbacks - run_initialize_callbacks + _run_find_callbacks + _run_initialize_callbacks self end @@ -331,7 +331,7 @@ module ActiveRecord @attributes = @attributes.dup @attributes.reset(self.class.primary_key) - run_initialize_callbacks + _run_initialize_callbacks @aggregation_cache = {} @association_cache = {} diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index bb06d0304b..f92e1de03b 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -309,7 +309,7 @@ module ActiveRecord # Ensure that it is not called if the object was never persisted (failed create), # but call it after the commit of a destroyed object. def committed!(should_run_callbacks = true) #:nodoc: - run_commit_callbacks if should_run_callbacks && destroyed? || persisted? + _run_commit_callbacks if should_run_callbacks && destroyed? || persisted? ensure force_clear_transaction_record_state end @@ -317,7 +317,7 @@ module ActiveRecord # Call the +after_rollback+ callbacks. The +force_restore_state+ argument indicates if the record # state should be rolled back to the beginning or just to the last savepoint. def rolledback!(force_restore_state = false, should_run_callbacks = true) #:nodoc: - run_rollback_callbacks if should_run_callbacks + _run_rollback_callbacks if should_run_callbacks ensure restore_transaction_record_state(force_restore_state) clear_transaction_record_state diff --git a/activerecord/lib/active_record/type/boolean.rb b/activerecord/lib/active_record/type/boolean.rb index 1311be3944..612478e67d 100644 --- a/activerecord/lib/active_record/type/boolean.rb +++ b/activerecord/lib/active_record/type/boolean.rb @@ -15,7 +15,7 @@ module ActiveRecord else if !ConnectionAdapters::Column::FALSE_VALUES.include?(value) ActiveSupport::Deprecation.warn(<<-EOM) - You attempted to assign a value which is not explicitly true or false to a boolean column. Currently this value casts to false. This will change to match Ruby's sematics, and will cast to true in Rails 5.0. If you would like to maintain the current behavior, you should explicitly handle the values you would like cast to false. + You attempted to assign a value which is not explicitly true or false to a boolean column. Currently this value casts to false. This will change to match Ruby's semantics, and will cast to true in Rails 5.0. If you would like to maintain the current behavior, you should explicitly handle the values you would like cast to false. EOM end false diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index a935d33686..f9df972929 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -63,7 +63,7 @@ *Guo Xiang Tan* -* Added instance_eval version to Object#try, so you can do this: +* Added instance_eval version to Object#try and Object#try!, so you can do this: person.try { name.first } @@ -71,7 +71,7 @@ person.try { |person| person.name.first } - *DHH* + *DHH*, *Ari Pollak* * Fix the `ActiveSupport::Duration#instance_of?` method to return the right value with the class itself since it was previously delegated to the diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 45231bc101..24c702b602 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -78,7 +78,7 @@ module ActiveSupport # save # end def run_callbacks(kind, &block) - send "run_#{kind}_callbacks", &block + send "_run_#{kind}_callbacks", &block end private @@ -730,7 +730,7 @@ module ActiveSupport set_callbacks name, CallbackChain.new(name, options) module_eval <<-RUBY, __FILE__, __LINE__ + 1 - def run_#{name}_callbacks(&block) + def _run_#{name}_callbacks(&block) _run_callbacks(_#{name}_callbacks, &block) end RUBY diff --git a/activesupport/lib/active_support/core_ext/array/access.rb b/activesupport/lib/active_support/core_ext/array/access.rb index caa499dfa2..45b89d2705 100644 --- a/activesupport/lib/active_support/core_ext/array/access.rb +++ b/activesupport/lib/active_support/core_ext/array/access.rb @@ -20,7 +20,11 @@ class Array # %w( a b c d ).to(-2) # => ["a", "b", "c"] # %w( a b c ).to(-10) # => [] def to(position) - self[0..position] + if position >= 0 + first position + 1 + else + self[0..position] + end end # Equal to <tt>self[1]</tt>. diff --git a/activesupport/lib/active_support/core_ext/object/try.rb b/activesupport/lib/active_support/core_ext/object/try.rb index 31919474ed..56da398978 100644 --- a/activesupport/lib/active_support/core_ext/object/try.rb +++ b/activesupport/lib/active_support/core_ext/object/try.rb @@ -9,7 +9,23 @@ class Object # # instead of # - # @person ? @person.name : nil + # @person.name if @person + # + # +try+ calls can be chained: + # + # @person.try(:spouse).try(:name) + # + # instead of + # + # @person.spouse.name if @person && @person.spouse + # + # +try+ will also return +nil+ if the receiver does not respond to the method: + # + # @person.try(:non_existing_method) #=> nil + # + # instead of + # + # @person.non_existing_method if @person.respond_to?(:non_existing_method) #=> nil # # +try+ returns +nil+ when called on +nil+ regardless of whether it responds # to the method: @@ -24,7 +40,7 @@ class Object # # The number of arguments in the signature must match. If the object responds # to the method the call is attempted and +ArgumentError+ is still raised - # otherwise. + # in case of argument mismatch. # # If +try+ is called without arguments it yields the receiver to a given # block unless it is +nil+: @@ -38,12 +54,19 @@ class Object # # @person.try { upcase.truncate(50) } # - # Please also note that +try+ is defined on +Object+, therefore it won't work + # Please also note that +try+ is defined on +Object+. Therefore, it won't work # with instances of classes that do not have +Object+ among their ancestors, # like direct subclasses of +BasicObject+. For example, using +try+ with # +SimpleDelegator+ will delegate +try+ to the target instead of calling it on - # delegator itself. + # the delegator itself. def try(*a, &b) + try!(*a, &b) if a.empty? || respond_to?(a.first) + end + + # Same as #try, but will raise a NoMethodError exception if the receiver is not +nil+ and + # does not implement the tried method. + + def try!(*a, &b) if a.empty? && block_given? if b.arity.zero? instance_eval(&b) @@ -51,16 +74,6 @@ class Object yield self end else - public_send(*a, &b) if respond_to?(a.first) - end - end - - # Same as #try, but will raise a NoMethodError exception if the receiving is not nil and - # does not implement the tried method. - def try!(*a, &b) - if a.empty? && block_given? - yield self - else public_send(*a, &b) end end @@ -68,12 +81,12 @@ end class NilClass # Calling +try+ on +nil+ always returns +nil+. - # It becomes specially helpful when navigating through associations that may return +nil+. + # It becomes especially helpful when navigating through associations that may return +nil+. # # nil.try(:name) # => nil # # Without +try+ - # @person && !@person.children.blank? && @person.children.first.name + # @person && @person.children.any? && @person.children.first.name # # With +try+ # @person.try(:children).try(:first).try(:name) diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb index c761325108..042283e4fc 100644 --- a/activesupport/lib/active_support/core_ext/string/output_safety.rb +++ b/activesupport/lib/active_support/core_ext/string/output_safety.rb @@ -150,7 +150,7 @@ module ActiveSupport #:nodoc: else if html_safe? new_safe_buffer = super - new_safe_buffer.instance_eval { @html_safe = true } + new_safe_buffer.instance_variable_set :@html_safe, true new_safe_buffer else to_str[*args] diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 93a11d4586..a89c769e34 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -30,6 +30,10 @@ module ActiveSupport #:nodoc: mattr_accessor :loaded self.loaded = Set.new + # Stack of files being loaded. + mattr_accessor :loading + self.loading = [] + # Should we load files or require them? mattr_accessor :mechanism self.mechanism = ENV['NO_RELOAD'] ? :require : :load @@ -317,6 +321,7 @@ module ActiveSupport #:nodoc: def clear log_call loaded.clear + loading.clear remove_unloadable_constants! end @@ -329,6 +334,7 @@ module ActiveSupport #:nodoc: # Record that we've seen this file *before* loading it to avoid an # infinite loop with mutual dependencies. loaded << expanded + loading << expanded begin if load? @@ -351,6 +357,8 @@ module ActiveSupport #:nodoc: rescue Exception loaded.delete expanded raise + ensure + loading.pop end # Record history *after* loading so first load gets warnings. @@ -475,7 +483,7 @@ module ActiveSupport #:nodoc: expanded = File.expand_path(file_path) expanded.sub!(/\.rb\z/, '') - if loaded.include?(expanded) + if loading.include?(expanded) raise "Circular dependency detected while autoloading constant #{qualified_name}" else require_or_load(expanded, qualified_name) diff --git a/activesupport/lib/active_support/inflector/inflections.rb b/activesupport/lib/active_support/inflector/inflections.rb index 97401ccec7..486838bd15 100644 --- a/activesupport/lib/active_support/inflector/inflections.rb +++ b/activesupport/lib/active_support/inflector/inflections.rb @@ -154,7 +154,7 @@ module ActiveSupport end end - # Add uncountable words that shouldn't be attempted inflected. + # Specifies words that are uncountable and should not be inflected. # # uncountable 'money' # uncountable 'money', 'information' diff --git a/activesupport/lib/active_support/message_verifier.rb b/activesupport/lib/active_support/message_verifier.rb index 6cb2884fb7..4e0796f4f8 100644 --- a/activesupport/lib/active_support/message_verifier.rb +++ b/activesupport/lib/active_support/message_verifier.rb @@ -1,5 +1,6 @@ require 'base64' require 'active_support/core_ext/object/blank' +require 'active_support/security_utils' module ActiveSupport # +MessageVerifier+ makes it easy to generate and verify messages which are @@ -37,7 +38,7 @@ module ActiveSupport raise InvalidSignature if signed_message.blank? data, digest = signed_message.split("--") - if data.present? && digest.present? && secure_compare(digest, generate_digest(data)) + if data.present? && digest.present? && ActiveSupport::SecurityUtils.secure_compare(digest, generate_digest(data)) begin @serializer.load(::Base64.strict_decode64(data)) rescue ArgumentError => argument_error @@ -55,17 +56,6 @@ module ActiveSupport end private - # constant-time comparison algorithm to prevent timing attacks - def secure_compare(a, b) - return false unless a.bytesize == b.bytesize - - l = a.unpack "C#{a.bytesize}" - - res = 0 - b.each_byte { |byte| res |= byte ^ l.shift } - res == 0 - end - def generate_digest(data) require 'openssl' unless defined?(OpenSSL) OpenSSL::HMAC.hexdigest(OpenSSL::Digest.const_get(@digest).new, @secret, data) diff --git a/activesupport/lib/active_support/security_utils.rb b/activesupport/lib/active_support/security_utils.rb new file mode 100644 index 0000000000..64c4801179 --- /dev/null +++ b/activesupport/lib/active_support/security_utils.rb @@ -0,0 +1,20 @@ +module ActiveSupport + module SecurityUtils + # Constant time string comparison. + # + # The values compared should be of fixed length, such as strings + # that have already been processed by HMAC. This should not be used + # on variable length plaintext strings because it could leak length info + # via timing attacks. + def secure_compare(a, b) + return false unless a.bytesize == b.bytesize + + l = a.unpack "C#{a.bytesize}" + + res = 0 + b.each_byte { |byte| res |= byte ^ l.shift } + res == 0 + end + module_function :secure_compare + end +end diff --git a/activesupport/test/autoloading_fixtures/typo.rb b/activesupport/test/autoloading_fixtures/typo.rb new file mode 100644 index 0000000000..8e047f5fd4 --- /dev/null +++ b/activesupport/test/autoloading_fixtures/typo.rb @@ -0,0 +1,2 @@ +TypO = 1 + diff --git a/activesupport/test/core_ext/object/try_test.rb b/activesupport/test/core_ext/object/try_test.rb index 225c20fa36..efc6beaf02 100644 --- a/activesupport/test/core_ext/object/try_test.rb +++ b/activesupport/test/core_ext/object/try_test.rb @@ -30,10 +30,6 @@ class ObjectTryTest < ActiveSupport::TestCase assert_raise(NoMethodError) { @string.try!(method, 'llo', 'y') } end - def test_try_only_block_bang - assert_equal @string.reverse, @string.try! { |s| s.reverse } - end - def test_valid_method assert_equal 5, @string.try(:size) end @@ -59,6 +55,10 @@ class ObjectTryTest < ActiveSupport::TestCase assert_equal @string.reverse, @string.try { |s| s.reverse } end + def test_try_only_block_bang + assert_equal @string.reverse, @string.try! { |s| s.reverse } + end + def test_try_only_block_nil ran = false nil.try { ran = true } @@ -69,6 +69,10 @@ class ObjectTryTest < ActiveSupport::TestCase assert_equal @string.reverse, @string.try { reverse } end + def test_try_with_instance_eval_block_bang + assert_equal @string.reverse, @string.try! { reverse } + end + def test_try_with_private_method_bang klass = Class.new do private diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 899bb75eae..f2f60167c4 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -157,6 +157,31 @@ class DependenciesTest < ActiveSupport::TestCase end end + def test_ensures_the_expected_constant_is_defined + with_autoloading_fixtures do + e = assert_raise(LoadError) { Typo } + assert_match %r{Unable to autoload constant Typo, expected .*activesupport/test/autoloading_fixtures/typo.rb to define it}, e.message + end + end + + def test_require_dependency_does_not_assume_any_particular_constant_is_defined + with_autoloading_fixtures do + require_dependency 'typo' + assert_equal 1, TypO + end + end + + # Regression, see https://github.com/rails/rails/issues/16468. + def test_require_dependency_interaction_with_autoloading + with_autoloading_fixtures do + require_dependency 'typo' + assert_equal 1, TypO + + e = assert_raise(LoadError) { Typo } + assert_match %r{Unable to autoload constant Typo, expected .*activesupport/test/autoloading_fixtures/typo.rb to define it}, e.message + end + end + def test_module_loading with_autoloading_fixtures do assert_kind_of Module, A diff --git a/activesupport/test/dependencies_test_helpers.rb b/activesupport/test/dependencies_test_helpers.rb index 9268512a97..e4d5197112 100644 --- a/activesupport/test/dependencies_test_helpers.rb +++ b/activesupport/test/dependencies_test_helpers.rb @@ -13,6 +13,7 @@ module DependenciesTestHelpers ActiveSupport::Dependencies.autoload_paths = prior_autoload_paths ActiveSupport::Dependencies.mechanism = old_mechanism ActiveSupport::Dependencies.explicitly_unloadable_constants = [] + ActiveSupport::Dependencies.clear end def with_autoloading_fixtures(&block) diff --git a/activesupport/test/security_utils_test.rb b/activesupport/test/security_utils_test.rb new file mode 100644 index 0000000000..08d2e3baa6 --- /dev/null +++ b/activesupport/test/security_utils_test.rb @@ -0,0 +1,9 @@ +require 'abstract_unit' +require 'active_support/security_utils' + +class SecurityUtilsTest < ActiveSupport::TestCase + def test_secure_compare_should_perform_string_comparison + assert ActiveSupport::SecurityUtils.secure_compare('a', 'a') + assert !ActiveSupport::SecurityUtils.secure_compare('a', 'b') + end +end diff --git a/guides/source/4_2_release_notes.md b/guides/source/4_2_release_notes.md index a598c7c319..d903752cff 100644 --- a/guides/source/4_2_release_notes.md +++ b/guides/source/4_2_release_notes.md @@ -750,6 +750,10 @@ Please refer to the [Changelog][active-support] for detailed changes. ### Notable changes +* `Object#try` and `Object#try!` can now be used without an explicit receiver. + ([Commit](https://github.com/rails/rails/commit/5e51bdda59c9ba8e5faf86294e3e431bd45f1830), + [Pull Request](https://github.com/rails/rails/pull/17361)) + * Introduced new configuration option `active_support.test_order` for specifying the order test cases are executed. This option currently defaults to `:sorted` but will be changed to `:random` in Rails 5.0. diff --git a/guides/source/active_record_basics.md b/guides/source/active_record_basics.md index ecf3483d7e..bd074d0055 100644 --- a/guides/source/active_record_basics.md +++ b/guides/source/active_record_basics.md @@ -31,7 +31,7 @@ Object Relational Mapping system. in his book _Patterns of Enterprise Application Architecture_. In Active Record, objects carry both persistent data and behavior which operates on that data. Active Record takes the opinion that ensuring -data access logic is part of the object will educate users of that +data access logic as part of the object will educate users of that object on how to write to and read from the database. ### Object Relational Mapping diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index 229c6ee458..c8a31fe7b8 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -466,7 +466,7 @@ add_foreign_key :articles, :authors ``` This adds a new foreign key to the `author_id` column of the `articles` -table. The key references the `id` column of the `articles` table. If the +table. The key references the `id` column of the `authors` table. If the column names can not be derived from the table names, you can use the `:column` and `:primary_key` options. diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index de42f13145..f6f96b79c6 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -1833,16 +1833,14 @@ attribute names: ```ruby def full_messages - full_messages = [] - - each do |attribute, messages| - ... - attr_name = attribute.to_s.gsub('.', '_').humanize - attr_name = @base.class.human_attribute_name(attribute, default: attr_name) - ... - end + map { |attribute, message| full_message(attribute, message) } +end - full_messages +def full_message + ... + attr_name = attribute.to_s.tr('.', '_').humanize + attr_name = @base.class.human_attribute_name(attribute, default: attr_name) + ... end ``` diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index 302c4ca9c0..4eb360cc7a 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -193,7 +193,7 @@ Now get busy and add/edit code. You're on your branch now, so you can write what * Update the (surrounding) documentation, examples elsewhere, and the guides: whatever is affected by your contribution. -TIP: Changes that are cosmetic in nature and do not add anything substantial to the stability, functionality, or testability of Rails will generally not be accepted. +TIP: Changes that are cosmetic in nature and do not add anything substantial to the stability, functionality, or testability of Rails will generally not be accepted (read more about [our rationales behind this decision](https://github.com/rails/rails/pull/13771#issuecomment-32746700)). #### Follow the Coding Conventions |