diff options
| -rw-r--r-- | .travis.yml | 2 | ||||
| -rw-r--r-- | Gemfile.lock | 2 | ||||
| -rw-r--r-- | actionmailer/lib/action_mailer/base.rb | 12 | ||||
| -rw-r--r-- | actionpack/CHANGELOG.md | 12 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/redirecting.rb | 33 | ||||
| -rw-r--r-- | actionpack/test/controller/redirect_test.rb | 38 | ||||
| -rw-r--r-- | activerecord/lib/active_record/base.rb | 2 | ||||
| -rw-r--r-- | activerecord/lib/rails/generators/active_record/model/model_generator.rb | 18 | ||||
| -rw-r--r-- | activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb | 16 | ||||
| -rw-r--r-- | activerecord/test/cases/base_test.rb | 16 | ||||
| -rw-r--r-- | activerecord/test/cases/migration_test.rb | 1 | ||||
| -rw-r--r-- | activesupport/lib/active_support/logger.rb | 8 | ||||
| -rw-r--r-- | activesupport/lib/active_support/logger_silence.rb | 5 | ||||
| -rw-r--r-- | activesupport/test/broadcast_logger_test.rb | 50 | ||||
| -rw-r--r-- | guides/source/action_controller_overview.md | 2 | ||||
| -rw-r--r-- | guides/source/active_record_basics.md | 2 | ||||
| -rw-r--r-- | guides/source/layouts_and_rendering.md | 7 |
17 files changed, 161 insertions, 65 deletions
diff --git a/.travis.yml b/.travis.yml index 11693319da..efa25c8de6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -25,7 +25,7 @@ env: - "GEM=aj:integration" - "GEM=guides" rvm: - - 2.2.3 + - 2.2.4 - ruby-head matrix: allow_failures: diff --git a/Gemfile.lock b/Gemfile.lock index dd137dae94..ab4183f916 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -395,4 +395,4 @@ DEPENDENCIES w3c_validators BUNDLED WITH - 1.11.0 + 1.11.2 diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index cbbf480da8..bb3cb1be45 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -658,18 +658,18 @@ module ActionMailer # # mail.attachments['filename.jpg'] = File.read('/path/to/filename.jpg') # - # If you do this, then Mail will take the file name and work out the mime type - # set the Content-Type, Content-Disposition, Content-Transfer-Encoding and - # base64 encode the contents of the attachment all for you. + # If you do this, then Mail will take the file name and work out the mime type. + # It will also set the Content-Type, Content-Disposition, Content-Transfer-Encoding + # and encode the contents of the attachment in Base64. # # You can also specify overrides if you want by passing a hash instead of a string: # # mail.attachments['filename.jpg'] = {mime_type: 'application/x-gzip', # content: File.read('/path/to/filename.jpg')} # - # If you want to use a different encoding than Base64, you can pass an encoding in, - # but then it is up to you to pass in the content pre-encoded, and don't expect - # Mail to know how to decode this data: + # If you want to use encoding other than Base64 then you will need to pass encoding + # type along with the pre-encoded content as Mail doesn't know how to decode the + # data: # # file_content = SpecialEncode(File.read('/path/to/filename.jpg')) # mail.attachments['filename.jpg'] = {mime_type: 'application/x-gzip', diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 608512d846..8b2943af74 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,15 @@ +* Deprecate `redirect_to :back` in favor of `redirect_back`, which accepts a + required `fallback_location` argument, thus eliminating the possibility of a + `RedirectBackError`. + + *Derek Prior* + +* Add `redirect_back` method to `ActionController::Redirecting` to provide a + way to safely redirect to the `HTTP_REFERER` if it is present, falling back + to a provided redirect otherwise. + + *Derek Prior* + * `ActionController::TestCase` will be moved to it's own gem in Rails 5.1 With the speed improvements made to `ActionDispatch::IntegrationTest` we no diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 0febc905f1..aeecb48f85 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -20,8 +20,6 @@ module ActionController # * <tt>String</tt> starting with <tt>protocol://</tt> (like <tt>http://</tt>) or a protocol relative reference (like <tt>//</tt>) - Is passed straight through as the target for redirection. # * <tt>String</tt> not containing a protocol - The current protocol and host is prepended to the string. # * <tt>Proc</tt> - A block that will be executed in the controller's context. Should return any option accepted by +redirect_to+. - # * <tt>:back</tt> - Back to the page that issued the request. Useful for forms that are triggered from multiple places. - # Short-hand for <tt>redirect_to(request.env["HTTP_REFERER"])</tt> # # === Examples: # @@ -30,7 +28,6 @@ module ActionController # redirect_to "http://www.rubyonrails.org" # redirect_to "/images/screenshot.jpg" # redirect_to articles_url - # redirect_to :back # redirect_to proc { edit_post_url(@post) } # # The redirection happens as a "302 Found" header unless otherwise specified using the <tt>:status</tt> option: @@ -61,10 +58,6 @@ module ActionController # redirect_to post_url(@post), status: 301, flash: { updated_post_id: @post.id } # redirect_to({ action: 'atom' }, alert: "Something serious happened") # - # When using <tt>redirect_to :back</tt>, if there is no referrer, - # <tt>ActionController::RedirectBackError</tt> will be raised. You - # may specify some fallback behavior for this case by rescuing - # <tt>ActionController::RedirectBackError</tt>. def redirect_to(options = {}, response_status = {}) #:doc: raise ActionControllerError.new("Cannot redirect to nil!") unless options raise ActionControllerError.new("Cannot redirect to a parameter hash!") if options.is_a?(ActionController::Parameters) @@ -75,6 +68,26 @@ module ActionController self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(location)}\">redirected</a>.</body></html>" end + # Redirects the browser to the page that issued the request if possible, + # otherwise redirects to provided default fallback location. + # + # redirect_back fallback_location: { action: "show", id: 5 } + # redirect_back fallback_location: post + # redirect_back fallback_location: "http://www.rubyonrails.org" + # redirect_back fallback_location: "/images/screenshot.jpg" + # redirect_back fallback_location: articles_url + # redirect_back fallback_location: proc { edit_post_url(@post) } + # + # All options that can be passed to <tt>redirect_to</tt> are accepted as + # options and the behavior is indetical. + def redirect_back(fallback_location:, **args) + if referer = request.headers["Referer"] + redirect_to referer, **args + else + redirect_to fallback_location, **args + end + end + def _compute_redirect_to_location(request, options) #:nodoc: case options # The scheme name consist of a letter followed by any combination of @@ -87,6 +100,12 @@ module ActionController when String request.protocol + request.host_with_port + options when :back + ActiveSupport::Deprecation.warn(<<-MESSAGE.squish) + `redirect_to :back` is deprecated and will be removed from Rails 5.1. + Please use `redirect_back(fallback_location: fallback_location)` where + `fallback_location` represents the location to use if the request has + no HTTP referer information. + MESSAGE request.headers["Referer"] or raise RedirectBackError when Proc _compute_redirect_to_location request, options.call diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 631ff7d02a..21dfd9cd03 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -42,6 +42,10 @@ class RedirectController < ActionController::Base redirect_to :back, :status => 307 end + def redirect_back_with_status + redirect_back(fallback_location: "/things/stuff", status: 307) + end + def host_redirect redirect_to :action => "other_host", :only_path => false, :host => 'other.test.host' end @@ -187,7 +191,11 @@ class RedirectTest < ActionController::TestCase def test_redirect_to_back_with_status @request.env["HTTP_REFERER"] = "http://www.example.com/coming/from" - get :redirect_to_back_with_status + + assert_deprecated do + get :redirect_to_back_with_status + end + assert_response 307 assert_equal "http://www.example.com/coming/from", redirect_to_url end @@ -236,7 +244,11 @@ class RedirectTest < ActionController::TestCase def test_redirect_to_back @request.env["HTTP_REFERER"] = "http://www.example.com/coming/from" - get :redirect_to_back + + assert_deprecated do + get :redirect_to_back + end + assert_response :redirect assert_equal "http://www.example.com/coming/from", redirect_to_url end @@ -244,10 +256,32 @@ class RedirectTest < ActionController::TestCase def test_redirect_to_back_with_no_referer assert_raise(ActionController::RedirectBackError) { @request.env["HTTP_REFERER"] = nil + + assert_deprecated do + get :redirect_to_back + end + get :redirect_to_back } end + def test_redirect_back + referer = "http://www.example.com/coming/from" + @request.env["HTTP_REFERER"] = referer + + get :redirect_back_with_status + + assert_response 307 + assert_equal referer, redirect_to_url + end + + def test_redirect_back_with_no_referer + get :redirect_back_with_status + + assert_response 307 + assert_equal "http://test.host/things/stuff", redirect_to_url + end + def test_redirect_to_record with_routing do |set| set.draw do diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 9782e58299..4a31a1aa84 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -261,7 +261,7 @@ module ActiveRecord #:nodoc: # The +errors+ property of this exception contains an array of # AttributeAssignmentError # objects that should be inspected to determine which attributes triggered the errors. - # * RecordInvalid - raised by {ActiveRecord::Base#save}[rdoc-ref:Persistence#save] and + # * RecordInvalid - raised by {ActiveRecord::Base#save!}[rdoc-ref:Persistence#save!] and # {ActiveRecord::Base.create!}[rdoc-ref:Persistence::ClassMethods#create!] # when the record is invalid. # * RecordNotFound - No record responded to the {ActiveRecord::Base.find}[rdoc-ref:FinderMethods#find] method. diff --git a/activerecord/lib/rails/generators/active_record/model/model_generator.rb b/activerecord/lib/rails/generators/active_record/model/model_generator.rb index e4c9539362..15aecf28ca 100644 --- a/activerecord/lib/rails/generators/active_record/model/model_generator.rb +++ b/activerecord/lib/rails/generators/active_record/model/model_generator.rb @@ -29,25 +29,25 @@ module ActiveRecord template 'module.rb', File.join('app/models', "#{class_path.join('/')}.rb") if behavior == :invoke end - def attributes_with_index - attributes.select { |a| !a.reference? && a.has_index? } - end - - def accessible_attributes - attributes.reject(&:reference?) - end - hook_for :test_framework protected + def attributes_with_index + attributes.select { |a| !a.reference? && a.has_index? } + end + # Used by the migration template to determine the parent name of the model def parent_class_name options[:parent] || determine_default_parent_class end def determine_default_parent_class - if File.exist?('app/models/application_record.rb') + application_record = nil + + in_root { application_record = File.exist?('app/models/application_record.rb') } + + if application_record "ApplicationRecord" else "ActiveRecord::Base" diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb index 887dcfc96c..9b675b804b 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb @@ -6,13 +6,17 @@ module ActiveRecord class SQLite3CreateFolder < ActiveRecord::SQLite3TestCase def test_sqlite_creates_directory Dir.mktmpdir do |dir| - dir = Pathname.new(dir) - @conn = Base.sqlite3_connection :database => dir.join("db/foo.sqlite3"), - :adapter => 'sqlite3', - :timeout => 100 + begin + dir = Pathname.new(dir) + @conn = Base.sqlite3_connection :database => dir.join("db/foo.sqlite3"), + :adapter => 'sqlite3', + :timeout => 100 - assert Dir.exist? dir.join('db') - assert File.exist? dir.join('db/foo.sqlite3') + assert Dir.exist? dir.join('db') + assert File.exist? dir.join('db/foo.sqlite3') + ensure + @conn.disconnect! if @conn + end end end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index dc555caaff..d4d08049c8 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -211,7 +211,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_preserving_time_objects_with_local_time_conversion_to_default_timezone_utc - with_env_tz 'America/New_York' do + with_env_tz eastern_time_zone do with_timezone_config default: :utc do time = Time.local(2000) topic = Topic.create('written_on' => time) @@ -224,7 +224,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_preserving_time_objects_with_time_with_zone_conversion_to_default_timezone_utc - with_env_tz 'America/New_York' do + with_env_tz eastern_time_zone do with_timezone_config default: :utc do Time.use_zone 'Central Time (US & Canada)' do time = Time.zone.local(2000) @@ -239,7 +239,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_preserving_time_objects_with_utc_time_conversion_to_default_timezone_local - with_env_tz 'America/New_York' do + with_env_tz eastern_time_zone do with_timezone_config default: :local do time = Time.utc(2000) topic = Topic.create('written_on' => time) @@ -252,7 +252,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_preserving_time_objects_with_time_with_zone_conversion_to_default_timezone_local - with_env_tz 'America/New_York' do + with_env_tz eastern_time_zone do with_timezone_config default: :local do Time.use_zone 'Central Time (US & Canada)' do time = Time.zone.local(2000) @@ -266,6 +266,14 @@ class BasicsTest < ActiveRecord::TestCase end end + def eastern_time_zone + if Gem.win_platform? + "EST5EDT" + else + "America/New_York" + end + end + def test_custom_mutator topic = Topic.find(1) # This mutator is protected in the class definition diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 19be357b6e..b6813891c6 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -549,7 +549,6 @@ class MigrationTest < ActiveRecord::TestCase expected_id = Zlib.crc32(current_database) * salt assert lock_id == expected_id, "expected lock id generated by the migrator to be #{expected_id}, but it was #{lock_id} instead" - assert lock_id.is_a?(Fixnum), "expected lock id to be a Fixnum, but it wasn't" assert lock_id.bit_length <= 63, "lock id must be a signed integer of max 63 bits magnitude" end diff --git a/activesupport/lib/active_support/logger.rb b/activesupport/lib/active_support/logger.rb index 33fccdcf95..82117a64d2 100644 --- a/activesupport/lib/active_support/logger.rb +++ b/activesupport/lib/active_support/logger.rb @@ -1,4 +1,3 @@ -require 'active_support/core_ext/module/attribute_accessors' require 'active_support/logger_silence' require 'logger' @@ -6,16 +5,18 @@ module ActiveSupport class Logger < ::Logger include LoggerSilence + attr_accessor :broadcast_messages + # Broadcasts logs to multiple loggers. def self.broadcast(logger) # :nodoc: Module.new do define_method(:add) do |*args, &block| - logger.add(*args, &block) + logger.add(*args, &block) if broadcast_messages super(*args, &block) end define_method(:<<) do |x| - logger << x + logger << x if broadcast_messages super(x) end @@ -44,6 +45,7 @@ module ActiveSupport def initialize(*args) super @formatter = SimpleFormatter.new + @broadcast_messages = true end # Simple formatter which only displays the message. diff --git a/activesupport/lib/active_support/logger_silence.rb b/activesupport/lib/active_support/logger_silence.rb index a8efdef944..7d92256f24 100644 --- a/activesupport/lib/active_support/logger_silence.rb +++ b/activesupport/lib/active_support/logger_silence.rb @@ -1,8 +1,9 @@ require 'active_support/concern' +require 'active_support/core_ext/module/attribute_accessors' module LoggerSilence extend ActiveSupport::Concern - + included do cattr_accessor :silencer self.silencer = true @@ -21,4 +22,4 @@ module LoggerSilence yield self end end -end
\ No newline at end of file +end diff --git a/activesupport/test/broadcast_logger_test.rb b/activesupport/test/broadcast_logger_test.rb index 6d4e3b74f7..e7d56c80c3 100644 --- a/activesupport/test/broadcast_logger_test.rb +++ b/activesupport/test/broadcast_logger_test.rb @@ -2,56 +2,69 @@ require 'abstract_unit' module ActiveSupport class BroadcastLoggerTest < TestCase - attr_reader :logger, :log1, :log2 + attr_reader :logger, :receiving_logger def setup - @log1 = FakeLogger.new - @log2 = FakeLogger.new - @log1.extend Logger.broadcast @log2 - @logger = @log1 + @logger = FakeLogger.new + @receiving_logger = FakeLogger.new + @logger.extend Logger.broadcast @receiving_logger end def test_debug logger.debug "foo" - assert_equal 'foo', log1.adds.first[2] - assert_equal 'foo', log2.adds.first[2] + assert_equal 'foo', logger.adds.first[2] + assert_equal 'foo', receiving_logger.adds.first[2] + end + + def test_debug_without_message_broadcasts + logger.broadcast_messages = false + logger.debug "foo" + assert_equal 'foo', logger.adds.first[2] + assert_equal [], receiving_logger.adds end def test_close logger.close - assert log1.closed, 'should be closed' - assert log2.closed, 'should be closed' + assert logger.closed, 'should be closed' + assert receiving_logger.closed, 'should be closed' end def test_chevrons logger << "foo" - assert_equal %w{ foo }, log1.chevrons - assert_equal %w{ foo }, log2.chevrons + assert_equal %w{ foo }, logger.chevrons + assert_equal %w{ foo }, receiving_logger.chevrons + end + + def test_chevrons_without_message_broadcasts + logger.broadcast_messages = false + logger << "foo" + assert_equal %w{ foo }, logger.chevrons + assert_equal [], receiving_logger.chevrons end def test_level assert_nil logger.level logger.level = 10 - assert_equal 10, log1.level - assert_equal 10, log2.level + assert_equal 10, logger.level + assert_equal 10, receiving_logger.level end def test_progname assert_nil logger.progname logger.progname = 10 - assert_equal 10, log1.progname - assert_equal 10, log2.progname + assert_equal 10, logger.progname + assert_equal 10, receiving_logger.progname end def test_formatter assert_nil logger.formatter logger.formatter = 10 - assert_equal 10, log1.formatter - assert_equal 10, log2.formatter + assert_equal 10, logger.formatter + assert_equal 10, receiving_logger.formatter end class FakeLogger attr_reader :adds, :closed, :chevrons - attr_accessor :level, :progname, :formatter + attr_accessor :level, :progname, :formatter, :broadcast_messages def initialize @adds = [] @@ -60,6 +73,7 @@ module ActiveSupport @level = nil @progname = nil @formatter = nil + @broadcast_messages = true end def debug msg, &block diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index 7e43ba375a..6c622a3643 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -1150,7 +1150,7 @@ class ApplicationController < ActionController::Base def user_not_authorized flash[:error] = "You don't have access to this section." - redirect_to :back + redirect_back(fallback_location: root_path) end end diff --git a/guides/source/active_record_basics.md b/guides/source/active_record_basics.md index 8bd135ddd5..56f69f8f82 100644 --- a/guides/source/active_record_basics.md +++ b/guides/source/active_record_basics.md @@ -168,7 +168,7 @@ What if you need to follow a different naming convention or need to use your Rails application with a legacy database? No problem, you can easily override the default conventions. -`ApplicationRecord` inherits from `ActionController::Base`, which defines a +`ApplicationRecord` inherits from `ActiveRecord::Base`, which defines a number of helpful methods. You can use the `ActiveRecord::Base.table_name=` method to specify the table name that should be used: diff --git a/guides/source/layouts_and_rendering.md b/guides/source/layouts_and_rendering.md index 71cc030f6a..4bb364c0f8 100644 --- a/guides/source/layouts_and_rendering.md +++ b/guides/source/layouts_and_rendering.md @@ -622,10 +622,13 @@ Another way to handle returning responses to an HTTP request is with `redirect_t redirect_to photos_url ``` -You can use `redirect_to` with any arguments that you could use with `link_to` or `url_for`. There's also a special redirect that sends the user back to the page they just came from: +You can use `redirect_back` to return the user to the page they just came from. +This location is pulled from the `HTTP_REFERER` header which is not guaranteed +to be set by the browser, so you must provide the `fallback_location` +to use in this case. ```ruby -redirect_to :back +redirect_back(fallback_location: root_path) ``` #### Getting a Different Redirect Status Code |