aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/CHANGELOG.md19
-rw-r--r--actionpack/lib/action_controller/metal/data_streaming.rb1
-rw-r--r--actionpack/lib/action_dispatch/http/response.rb23
-rw-r--r--actionpack/lib/action_dispatch/routing/mapper.rb2
-rw-r--r--actionpack/test/controller/force_ssl_test.rb2
-rw-r--r--actionpack/test/controller/send_file_test.rb14
-rw-r--r--actionpack/test/dispatch/routing_test.rb42
-rw-r--r--activerecord/CHANGELOG.md5
-rw-r--r--activerecord/lib/active_record/autosave_association.rb4
-rw-r--r--activerecord/test/cases/associations/belongs_to_associations_test.rb6
-rw-r--r--activesupport/lib/active_support/testing/assertions.rb2
-rw-r--r--activesupport/lib/active_support/time_with_zone.rb2
-rw-r--r--activesupport/test/test_case_test.rb9
-rw-r--r--guides/source/contributing_to_ruby_on_rails.md2
-rw-r--r--guides/source/testing.md19
15 files changed, 124 insertions, 28 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index a66a1e8af3..999ac82d42 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,10 +1,27 @@
+* Fix nested multiple roots
+
+ The PR #20940 enabled the use of multiple roots with different constraints
+ at the top level but unfortunately didn't work when those roots were inside
+ a namespace and also broke the use of root inside a namespace after a top
+ level root was defined because the check for the existence of the named route
+ used the global :root name and not the namespaced name.
+
+ This is fixed by using the name_for_action method to expand the :root name to
+ the full namespaced name. We can pass nil for the second argument as we're not
+ dealing with resource definitions so don't need to handle the cases for edit
+ and new routes.
+
+ Fixes #26148.
+
+ *Ryo Hashimoto*, *Andrew White*
+
* Include the content of the flash in the auto-generated etag. This solves the following problem:
1. POST /messages
2. redirect_to messages_url, notice: 'Message was created'
3. GET /messages/1
4. GET /messages
-
+
Step 4 would before still include the flash message, even though it's no longer relevant,
because the etag cache was recorded with the flash in place and didn't change when it was gone.
diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb
index b598dd4d77..6a1acb64f7 100644
--- a/actionpack/lib/action_controller/metal/data_streaming.rb
+++ b/actionpack/lib/action_controller/metal/data_streaming.rb
@@ -70,6 +70,7 @@ module ActionController #:nodoc:
send_file_headers! options
self.status = options[:status] || 200
+ self.content_type = options[:type] if options.key?(:type)
self.content_type = options[:content_type] if options.key?(:content_type)
response.send_file path
end
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index 9788431943..e8173e2a99 100644
--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -224,8 +224,10 @@ module ActionDispatch # :nodoc:
# Sets the HTTP content type.
def content_type=(content_type)
- header_info = parse_content_type
- set_content_type content_type.to_s, header_info.charset || self.class.default_charset
+ return unless content_type
+ new_header_info = parse_content_type(content_type.to_s)
+ prev_header_info = parsed_content_type_header
+ set_content_type new_header_info.mime_type, new_header_info.charset || prev_header_info.charset || self.class.default_charset
end
# Sets the HTTP response's content MIME type. For example, in the controller
@@ -238,7 +240,7 @@ module ActionDispatch # :nodoc:
# information.
def content_type
- parse_content_type.mime_type
+ parsed_content_type_header.mime_type
end
def sending_file=(v)
@@ -253,7 +255,7 @@ module ActionDispatch # :nodoc:
# response.charset = 'utf-16' # => 'utf-16'
# response.charset = nil # => 'utf-8'
def charset=(charset)
- header_info = parse_content_type
+ header_info = parsed_content_type_header
if false == charset
set_header CONTENT_TYPE, header_info.mime_type
else
@@ -265,7 +267,7 @@ module ActionDispatch # :nodoc:
# The charset of the response. HTML wants to know the encoding of the
# content you're giving them, so we need to send that along.
def charset
- header_info = parse_content_type
+ header_info = parsed_content_type_header
header_info.charset || self.class.default_charset
end
@@ -403,8 +405,7 @@ module ActionDispatch # :nodoc:
ContentTypeHeader = Struct.new :mime_type, :charset
NullContentTypeHeader = ContentTypeHeader.new nil, nil
- def parse_content_type
- content_type = get_header CONTENT_TYPE
+ def parse_content_type(content_type)
if content_type
type, charset = content_type.split(/;\s*charset=/)
type = nil if type.empty?
@@ -414,6 +415,12 @@ module ActionDispatch # :nodoc:
end
end
+ # Small internal convenience method to get the parsed version of the current
+ # content type header.
+ def parsed_content_type_header
+ parse_content_type(get_header(CONTENT_TYPE))
+ end
+
def set_content_type(content_type, charset)
type = (content_type || "").dup
type << "; charset=#{charset}" if charset
@@ -450,7 +457,7 @@ module ActionDispatch # :nodoc:
def assign_default_content_type_and_charset!
return if content_type
- ct = parse_content_type
+ ct = parsed_content_type_header
set_content_type(ct.mime_type || Mime[:html].to_s,
ct.charset || self.class.default_charset)
end
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index b3acac42fc..67b9463a6a 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -1927,7 +1927,7 @@ to this:
end
def match_root_route(options)
- name = has_named_route?(:root) ? nil : :root
+ name = has_named_route?(name_for_action(:root, nil)) ? nil : :root
args = ["/", { as: name, via: :get }.merge!(options)]
match(*args)
diff --git a/actionpack/test/controller/force_ssl_test.rb b/actionpack/test/controller/force_ssl_test.rb
index 8d5e3ce2fe..2b3859aa57 100644
--- a/actionpack/test/controller/force_ssl_test.rb
+++ b/actionpack/test/controller/force_ssl_test.rb
@@ -229,14 +229,12 @@ class ForceSSLFlashTest < ActionController::TestCase
assert_response 302
assert_equal "http://test.host/force_ssl_flash/cheeseburger", redirect_to_url
- # FIXME: AC::TestCase#build_request_uri doesn't build a new uri if PATH_INFO exists
@request.env.delete("PATH_INFO")
get :cheeseburger
assert_response 301
assert_equal "https://test.host/force_ssl_flash/cheeseburger", redirect_to_url
- # FIXME: AC::TestCase#build_request_uri doesn't build a new uri if PATH_INFO exists
@request.env.delete("PATH_INFO")
get :use_flash
diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb
index 25420ead3b..8dc565ac8d 100644
--- a/actionpack/test/controller/send_file_test.rb
+++ b/actionpack/test/controller/send_file_test.rb
@@ -233,4 +233,18 @@ class SendFileTest < ActionController::TestCase
response = process("file")
assert_equal 200, response.status
end
+
+ def test_send_file_charset_with_type_options_key
+ @controller = SendFileWithActionControllerLive.new
+ @controller.options = { type: "text/calendar; charset=utf-8" }
+ response = process("file")
+ assert_equal "text/calendar; charset=utf-8", response.headers["Content-Type"]
+ end
+
+ def test_send_file_charset_with_content_type_options_key
+ @controller = SendFileWithActionControllerLive.new
+ @controller.options = { content_type: "text/calendar" }
+ response = process("file")
+ assert_equal "text/calendar; charset=utf-8", response.headers["Content-Type"]
+ end
end
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index ee5c30ef0e..0938460632 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -3669,6 +3669,48 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
end
end
+ def test_multiple_roots
+ draw do
+ namespace :foo do
+ root "pages#index", constraints: { host: 'www.example.com' }
+ root "admin/pages#index", constraints: { host: 'admin.example.com' }
+ end
+
+ root "pages#index", constraints: { host: 'www.example.com' }
+ root "admin/pages#index", constraints: { host: 'admin.example.com' }
+ end
+
+ get "http://www.example.com/foo"
+ assert_equal "foo/pages#index", @response.body
+
+ get "http://admin.example.com/foo"
+ assert_equal "foo/admin/pages#index", @response.body
+
+ get "http://www.example.com/"
+ assert_equal "pages#index", @response.body
+
+ get "http://admin.example.com/"
+ assert_equal "admin/pages#index", @response.body
+ end
+
+ def test_namespaced_roots
+ draw do
+ namespace :foo do
+ root "test#index"
+ end
+
+ root "test#index"
+
+ namespace :bar do
+ root "test#index"
+ end
+ end
+
+ assert_equal "/foo", foo_root_path
+ assert_equal "/", root_path
+ assert_equal "/bar", bar_root_path
+ end
+
private
def draw(&block)
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 4aa1853bde..7838fe7167 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+* Remove unnecessarily association load when a `belongs_to` association has already been
+ loaded then the foreign key is changed directly and the record saved.
+
+ *James Coleman*
+
* Remove standardized column types/arguments spaces in schema dump.
*Tim Petricola*
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 6e6620aad5..db84876b0a 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -457,7 +457,9 @@ module ActiveRecord
# In addition, it will destroy the association if it was marked for destruction.
def save_belongs_to_association(reflection)
association = association_instance_get(reflection.name)
- record = association && association.load_target
+ return unless association && association.loaded? && !association.stale_target?
+
+ record = association.load_target
if record && !record.destroyed?
autosave = reflection.options[:autosave]
diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index 3f42cb9b9d..2418346d1b 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -626,6 +626,12 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
assert_queries(0) { tagging.super_tag }
end
+ def test_dont_find_target_when_saving_foreign_key_after_stale_association_loaded
+ client = Client.create!(name: "Test client", firm_with_basic_id: Firm.find(1))
+ client.firm_id = Firm.create!(name: "Test firm").id
+ assert_queries(1) { client.save! }
+ end
+
def test_field_name_same_as_foreign_key
computer = Computer.find(1)
assert_not_nil computer.developer, ":foreign key == attribute didn't lock up" # '
diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb
index 6a6941c72a..8c9ea2c0e8 100644
--- a/activesupport/lib/active_support/testing/assertions.rb
+++ b/activesupport/lib/active_support/testing/assertions.rb
@@ -147,7 +147,7 @@ module ActiveSupport
retval = yield
unless from == UNTRACKED
- error = "#{expression.inspect} isn't #{from}"
+ error = "#{expression.inspect} isn't #{from.inspect}"
error = "#{message}.\n#{error}" if message
assert from === before, error
end
diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb
index 9d1147620c..74c4a39342 100644
--- a/activesupport/lib/active_support/time_with_zone.rb
+++ b/activesupport/lib/active_support/time_with_zone.rb
@@ -80,7 +80,7 @@ module ActiveSupport
# Returns a <tt>Time</tt> instance of the simultaneous time in the system timezone.
def localtime(utc_offset = nil)
- utc.getlocal(utc_offset)
+ @localtime ||= utc.getlocal(utc_offset)
end
alias_method :getlocal, :localtime
diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb
index d3d2ed8d7a..dfbb8aa41d 100644
--- a/activesupport/test/test_case_test.rb
+++ b/activesupport/test/test_case_test.rb
@@ -138,6 +138,15 @@ class AssertDifferenceTest < ActiveSupport::TestCase
end
end
+ def test_assert_changes_with_from_option_with_nil
+ error = assert_raises Minitest::Assertion do
+ assert_changes "@object.num", from: nil do
+ @object.increment
+ end
+ end
+ assert_equal "\"@object.num\" isn't nil", error.message
+ end
+
def test_assert_changes_with_to_option
assert_changes "@object.num", to: 1 do
@object.increment
diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md
index ba8d085f79..332d8fc852 100644
--- a/guides/source/contributing_to_ruby_on_rails.md
+++ b/guides/source/contributing_to_ruby_on_rails.md
@@ -677,7 +677,7 @@ $ git format-patch master --stdout > ~/my_changes.patch
Switch over to the target branch and apply your changes:
```bash
-$ git checkout -b my_backport_branch 3-2-stable
+$ git checkout -b my_backport_branch 4-2-stable
$ git apply ~/my_changes.patch
```
diff --git a/guides/source/testing.md b/guides/source/testing.md
index 4de32d9d77..8f9246dea2 100644
--- a/guides/source/testing.md
+++ b/guides/source/testing.md
@@ -239,8 +239,8 @@ Run options: --seed 1808
Error:
ArticleTest#test_should_report_error:
-NameError: undefined local variable or method `some_undefined_variable' for #<ArticleTest:0x007fee3aa71798>
- test/models/article_test.rb:11:in `block in <class:ArticleTest>'
+NameError: undefined local variable or method 'some_undefined_variable' for #<ArticleTest:0x007fee3aa71798>
+ test/models/article_test.rb:11:in 'block in <class:ArticleTest>'
bin/rails test test/models/article_test.rb:9
@@ -369,7 +369,7 @@ documentation](http://docs.seattlerb.org/minitest).
We can run all of our tests at once by using the `bin/rails test` command.
-Or we can run a single test by passing the `bin/rails test` command the filename containing the test cases.
+Or we can run a single test file by passing the `bin/rails test` command the filename containing the test cases.
```bash
$ bin/rails test test/models/article_test.rb
@@ -763,16 +763,11 @@ The `get` method kicks off the web request and populates the results into the `@
* The action of the controller you are requesting.
This can be in the form of a string or a route (i.e. `articles_url`).
-
* `params`: option with a hash of request parameters to pass into the action
(e.g. query string parameters or article variables).
-
* `headers`: for setting the headers that will be passed with the request.
-
* `env`: for customizing the request environment as needed.
-
* `xhr`: whether the request is Ajax request or not. Can be set to true for marking the request as Ajax.
-
* `as`: for encoding the request with different content type. Supports `:json` by default.
All of these keyword arguments are optional.
@@ -865,8 +860,8 @@ class ArticlesControllerTest < ActionDispatch::IntegrationTest
test "should get index" do
get articles_url
- assert_equal "index", @controller.action_name
- assert_equal "application/x-www-form-urlencoded", @request.media_type
+ assert_equal "index", @controller.action_name
+ assert_equal "application/x-www-form-urlencoded", @request.media_type
assert_match "Articles", @response.body
end
end
@@ -1056,7 +1051,7 @@ To avoid code duplication, you can add your own test helpers.
Sign in helper can be a good example:
```ruby
-#test/test_helper.rb
+# test/test_helper.rb
module SignInHelper
def sign_in_as(user)
@@ -1362,7 +1357,7 @@ Here is an example using the [`travel_to`](http://api.rubyonrails.org/classes/Ac
user = User.create(name: 'Gaurish', activation_date: Date.new(2004, 10, 24))
assert_not user.applicable_for_gifting?
travel_to Date.new(2004, 11, 24) do
- assert_equal Date.new(2004, 10, 24), user.activation_date # inside the travel_to block `Date.current` is mocked
+ assert_equal Date.new(2004, 10, 24), user.activation_date # inside the `travel_to` block `Date.current` is mocked
assert user.applicable_for_gifting?
end
assert_equal Date.new(2004, 10, 24), user.activation_date # The change was visible only inside the `travel_to` block.