diff options
106 files changed, 788 insertions, 407 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index 62cab7261b..7617942267 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -63,6 +63,7 @@ PATH activesupport (= 6.0.0.beta3) activestorage (6.0.0.beta3) actionpack (= 6.0.0.beta3) + activejob (= 6.0.0.beta3) activerecord (= 6.0.0.beta3) marcel (~> 0.3.1) activesupport (6.0.0.beta3) diff --git a/actioncable/app/assets/javascripts/action_cable.js b/actioncable/app/assets/javascripts/action_cable.js index 4efab2ed46..8349361405 100644 --- a/actioncable/app/assets/javascripts/action_cable.js +++ b/actioncable/app/assets/javascripts/action_cable.js @@ -28,6 +28,22 @@ throw new TypeError("Cannot call a class as a function"); } }; + var createClass = function() { + function defineProperties(target, props) { + for (var i = 0; i < props.length; i++) { + var descriptor = props[i]; + descriptor.enumerable = descriptor.enumerable || false; + descriptor.configurable = true; + if ("value" in descriptor) descriptor.writable = true; + Object.defineProperty(target, descriptor.key, descriptor); + } + } + return function(Constructor, protoProps, staticProps) { + if (protoProps) defineProperties(Constructor.prototype, protoProps); + if (staticProps) defineProperties(Constructor, staticProps); + return Constructor; + }; + }(); var now = function now() { return new Date().getTime(); }; @@ -432,7 +448,7 @@ var Consumer = function() { function Consumer(url) { classCallCheck(this, Consumer); - this.url = url; + this._url = url; this.subscriptions = new Subscriptions(this); this.connection = new Connection(this); } @@ -452,11 +468,31 @@ return this.connection.open(); } }; + createClass(Consumer, [ { + key: "url", + get: function get$$1() { + return createWebSocketURL(this._url); + } + } ]); return Consumer; }(); + function createWebSocketURL(url) { + if (typeof url === "function") { + url = url(); + } + if (url && !/^wss?:/i.test(url)) { + var a = document.createElement("a"); + a.href = url; + a.href = a.href; + a.protocol = a.protocol.replace("http", "ws"); + return a.href; + } else { + return url; + } + } function createConsumer() { var url = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : getConfig("url") || INTERNAL.default_mount_path; - return new Consumer(createWebSocketURL(url)); + return new Consumer(url); } function getConfig(name) { var element = document.head.querySelector("meta[name='action-cable-" + name + "']"); @@ -464,18 +500,6 @@ return element.getAttribute("content"); } } - function createWebSocketURL(url) { - var webSocketURL = typeof url === "function" ? url() : url; - if (webSocketURL && !/^wss?:/i.test(webSocketURL)) { - var a = document.createElement("a"); - a.href = webSocketURL; - a.href = a.href; - a.protocol = a.protocol.replace("http", "ws"); - return a.href; - } else { - return webSocketURL; - } - } exports.Connection = Connection; exports.ConnectionMonitor = ConnectionMonitor; exports.Consumer = Consumer; @@ -483,10 +507,10 @@ exports.Subscription = Subscription; exports.Subscriptions = Subscriptions; exports.adapters = adapters; + exports.createWebSocketURL = createWebSocketURL; exports.logger = logger; exports.createConsumer = createConsumer; exports.getConfig = getConfig; - exports.createWebSocketURL = createWebSocketURL; Object.defineProperty(exports, "__esModule", { value: true }); diff --git a/actioncable/app/javascript/action_cable/consumer.js b/actioncable/app/javascript/action_cable/consumer.js index e8440f39f5..e2e0dea8b5 100644 --- a/actioncable/app/javascript/action_cable/consumer.js +++ b/actioncable/app/javascript/action_cable/consumer.js @@ -29,11 +29,15 @@ import Subscriptions from "./subscriptions" export default class Consumer { constructor(url) { - this.url = url + this._url = url this.subscriptions = new Subscriptions(this) this.connection = new Connection(this) } + get url() { + return createWebSocketURL(this._url) + } + send(data) { return this.connection.send(data) } @@ -52,3 +56,20 @@ export default class Consumer { } } } + +export function createWebSocketURL(url) { + if (typeof url === "function") { + url = url() + } + + if (url && !/^wss?:/i.test(url)) { + const a = document.createElement("a") + a.href = url + // Fix populating Location properties in IE. Otherwise, protocol will be blank. + a.href = a.href + a.protocol = a.protocol.replace("http", "ws") + return a.href + } else { + return url + } +} diff --git a/actioncable/app/javascript/action_cable/index.js b/actioncable/app/javascript/action_cable/index.js index e679745fd7..848b5631d6 100644 --- a/actioncable/app/javascript/action_cable/index.js +++ b/actioncable/app/javascript/action_cable/index.js @@ -1,6 +1,6 @@ import Connection from "./connection" import ConnectionMonitor from "./connection_monitor" -import Consumer from "./consumer" +import Consumer, { createWebSocketURL } from "./consumer" import INTERNAL from "./internal" import Subscription from "./subscription" import Subscriptions from "./subscriptions" @@ -15,11 +15,12 @@ export { Subscription, Subscriptions, adapters, + createWebSocketURL, logger, } export function createConsumer(url = getConfig("url") || INTERNAL.default_mount_path) { - return new Consumer(createWebSocketURL(url)) + return new Consumer(url) } export function getConfig(name) { @@ -28,18 +29,3 @@ export function getConfig(name) { return element.getAttribute("content") } } - -export function createWebSocketURL(url) { - const webSocketURL = typeof url === "function" ? url() : url - - if (webSocketURL && !/^wss?:/i.test(webSocketURL)) { - const a = document.createElement("a") - a.href = webSocketURL - // Fix populating Location properties in IE. Otherwise, protocol will be blank. - a.href = a.href - a.protocol = a.protocol.replace("http", "ws") - return a.href - } else { - return webSocketURL - } -} diff --git a/actioncable/lib/action_cable/connection/test_case.rb b/actioncable/lib/action_cable/connection/test_case.rb index 8d25a55c8a..f1673fea08 100644 --- a/actioncable/lib/action_cable/connection/test_case.rb +++ b/actioncable/lib/action_cable/connection/test_case.rb @@ -176,7 +176,7 @@ module ActionCable # # Accepts request path as the first argument and the following request options: # - # - params – url parameters (Hash) + # - params – URL parameters (Hash) # - headers – request headers (Hash) # - session – session data (Hash) # - env – additional Rack env configuration (Hash) diff --git a/actioncable/test/javascript/src/unit/action_cable_test.js b/actioncable/test/javascript/src/unit/action_cable_test.js index c9d34abc6d..c46f9878d2 100644 --- a/actioncable/test/javascript/src/unit/action_cable_test.js +++ b/actioncable/test/javascript/src/unit/action_cable_test.js @@ -42,12 +42,16 @@ module("ActionCable", () => { assert.equal(consumer.url, testURL) }) - test("uses function to generate URL", assert => { + test("dynamically computes URL from function", assert => { + let dynamicURL = testURL const generateURL = () => { - return testURL + return dynamicURL } const consumer = ActionCable.createConsumer(generateURL) assert.equal(consumer.url, testURL) + + dynamicURL = `${testURL}foo` + assert.equal(consumer.url, `${testURL}foo`) }) }) }) diff --git a/actionmailbox/CHANGELOG.md b/actionmailbox/CHANGELOG.md index f59c052d63..f5fb573090 100644 --- a/actionmailbox/CHANGELOG.md +++ b/actionmailbox/CHANGELOG.md @@ -11,6 +11,7 @@ *Pratik Naik* + ## Rails 6.0.0.beta1 (January 18, 2019) ## * Added to Rails. diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 9931a0de81..79f6320a04 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -43,7 +43,7 @@ *Rafael Mendonça França* -* Introduce ActionDispatch::HostAuthorization +* Introduce `ActionDispatch::HostAuthorization`. This is a new middleware that guards against DNS rebinding attacks by explicitly permitting the hosts a request can be made to. @@ -73,7 +73,7 @@ * Raise an error on root route naming conflicts. - Raises an ArgumentError when multiple root routes are defined in the + Raises an `ArgumentError` when multiple root routes are defined in the same context instead of assigning nil names to subsequent roots. *Gannon McGibbon* diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 972953d4f3..d0a7eadf45 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -333,7 +333,7 @@ module ActionDispatch end end - # strategy for building urls to send to the client + # strategy for building URLs to send to the client PATH = ->(options) { ActionDispatch::Http::URL.path_for(options) } UNKNOWN = ->(options) { ActionDispatch::Http::URL.url_for(options) } diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index ecb8c37e6b..51286155b9 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -282,7 +282,7 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase assert_no_match( /#{request.protocol}#{request.host}\/\/www.rubyonrails.org/, ex.message, - "protocol relative url was incorrectly normalized" + "protocol relative URL was incorrectly normalized" ) end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index b5503a9c64..4dddd98f9f 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -808,17 +808,17 @@ class UrlOptionsIntegrationTest < ActionDispatch::IntegrationTest end end - test "session uses default url options from routes" do + test "session uses default URL options from routes" do assert_equal "http://foo.com/foo", foos_url end - test "current host overrides default url options from routes" do + test "current host overrides default URL options from routes" do get "/foo" assert_response :success assert_equal "http://www.example.com/foo", foos_url end - test "controller can override default url options from request" do + test "controller can override default URL options from request" do get "/bar" assert_response :success assert_equal "http://bar.com/foo", foos_url diff --git a/actionpack/test/controller/new_base/render_file_test.rb b/actionpack/test/controller/new_base/render_file_test.rb index de8af029e0..82325c5bb2 100644 --- a/actionpack/test/controller/new_base/render_file_test.rb +++ b/actionpack/test/controller/new_base/render_file_test.rb @@ -40,32 +40,44 @@ module RenderFile testing RenderFile::BasicController test "rendering simple template" do - get :index + assert_deprecated do + get :index + end assert_response "Hello world!" end test "rendering template with ivar" do - get :with_instance_variables + assert_deprecated do + get :with_instance_variables + end assert_response "The secret is in the sauce\n" end test "rendering a relative path" do - get :relative_path + assert_deprecated do + get :relative_path + end assert_response "The secret is in the sauce\n" end test "rendering a relative path with dot" do - get :relative_path_with_dot + assert_deprecated do + get :relative_path_with_dot + end assert_response "The secret is in the sauce\n" end test "rendering a Pathname" do - get :pathname + assert_deprecated do + get :pathname + end assert_response "The secret is in the sauce\n" end test "rendering file with locals" do - get :with_locals + assert_deprecated do + get :with_locals + end assert_response "The secret is in the sauce\n" end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 4750093c5c..6d198ca42f 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -325,7 +325,7 @@ class ExpiresInRenderTest < ActionController::TestCase def test_dynamic_render_with_file # This is extremely bad, but should be possible to do. assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__)) - response = get :dynamic_render_with_file, params: { id: '../\\../test/abstract_unit.rb' } + response = assert_deprecated { get :dynamic_render_with_file, params: { id: '../\\../test/abstract_unit.rb' } } assert_equal File.read(File.expand_path("../../test/abstract_unit.rb", __dir__)), response.body end @@ -351,7 +351,7 @@ class ExpiresInRenderTest < ActionController::TestCase def test_permitted_dynamic_render_file_hash assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__)) - response = get :dynamic_render_permit, params: { id: { file: '../\\../test/abstract_unit.rb' } } + response = assert_deprecated { get :dynamic_render_permit, params: { id: { file: '../\\../test/abstract_unit.rb' } } } assert_equal File.read(File.expand_path("../../test/abstract_unit.rb", __dir__)), response.body end diff --git a/actionpack/test/controller/renderer_test.rb b/actionpack/test/controller/renderer_test.rb index ae8330e029..ea79f4de85 100644 --- a/actionpack/test/controller/renderer_test.rb +++ b/actionpack/test/controller/renderer_test.rb @@ -40,7 +40,7 @@ class RendererTest < ActiveSupport::TestCase test "rendering with an instance renderer" do renderer = ApplicationController.renderer.new - content = renderer.render file: "test/hello_world" + content = assert_deprecated { renderer.render file: "test/hello_world" } assert_equal "Hello world!", content end @@ -115,14 +115,14 @@ class RendererTest < ActiveSupport::TestCase assert_equal "true", content end - test "return valid asset url with defaults" do + test "return valid asset URL with defaults" do renderer = ApplicationController.renderer content = renderer.render inline: "<%= asset_url 'asset.jpg' %>" assert_equal "http://example.org/asset.jpg", content end - test "return valid asset url when https is true" do + test "return valid asset URL when https is true" do renderer = ApplicationController.renderer.new https: true content = renderer.render inline: "<%= asset_url 'asset.jpg' %>" diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index d1cd190747..998a495d0d 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -952,7 +952,7 @@ XML get :create assert_response :created - # Redirect url doesn't care that it wasn't a :redirect response. + # Redirect URL doesn't care that it wasn't a :redirect response. assert_equal "/resource", @response.redirect_url assert_equal @response.redirect_url, redirect_to_url diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 2c67bb779f..d129fa717d 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -317,7 +317,7 @@ class CookiesTest < ActionController::TestCase end def rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_off - cookies[:favorite] = "Wmg4amgvcVVvWGcwK3c4WjJEbTdRQUgrWXhBdDliUTR0cVNidXpmVTMrc2RjcitwUzVsWWEwZGtuVGtFUjJwNi0tcVhVMTFMOTQ1d0hIVE1FK0pJc05SQT09--8b2a55c375049a50f7a959b9d42b31ef0b2bb594" + cookies[:favorite] = "rTG4zs5UufEFAr+ppKwh+MDMymKyAUMOSaWyYa3uUVmD8sMQqyiyQBxgYeAncDHVZIlo4y+kDVSzp66u1/7BNYpnmFe8ES/YT2m8ckNA23jBDmnRZ9CTNfMIRXjFtfxO9YxEOzzhn0ZiA0/zFtr5wkluXtxplOz959Q7MgLOyvTze2h9p8A=--QHOS3rAEGq/HCxXs--xQNra8dk24Idc2qBtpMLpg==" head :ok end @@ -341,7 +341,7 @@ class CookiesTest < ActionController::TestCase SECRET_KEY_BASE = "b3c631c314c0bbca50c1b2843150fe33" SIGNED_COOKIE_SALT = "signed cookie" ENCRYPTED_COOKIE_SALT = "encrypted cookie" - ENCRYPTED_SIGNED_COOKIE_SALT = "sigend encrypted cookie" + ENCRYPTED_SIGNED_COOKIE_SALT = "signed encrypted cookie" AUTHENTICATED_ENCRYPTED_COOKIE_SALT = "authenticated encrypted cookie" def setup diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 8b1b3c0277..2812b1b614 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -341,7 +341,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match %r{NameError}, body end - test "named urls missing keys raise 500 level error" do + test "named URLs missing keys raise 500 level error" do @app = DevelopmentApp get "/missing_keys", headers: { "action_dispatch.show_exceptions" => true } diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index 7a7a201b11..63c147cb1b 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -151,17 +151,17 @@ module TestGenerationPrefix include BlogEngine.routes.mounted_helpers # Inside Engine - test "[ENGINE] generating engine's url use SCRIPT_NAME from request" do + test "[ENGINE] generating engine's URL use SCRIPT_NAME from request" do get "/pure-awesomeness/blog/posts/1" assert_equal "/pure-awesomeness/blog/posts/1", response.body end - test "[ENGINE] generating application's url never uses SCRIPT_NAME from request" do + test "[ENGINE] generating application's URL never uses SCRIPT_NAME from request" do get "/pure-awesomeness/blog/url_to_application" assert_equal "/generate", response.body end - test "[ENGINE] generating engine's url with polymorphic path" do + test "[ENGINE] generating engine's URL with polymorphic path" do get "/pure-awesomeness/blog/polymorphic_path_for_engine" assert_equal "/pure-awesomeness/blog/posts/1", response.body end @@ -243,7 +243,7 @@ module TestGenerationPrefix assert_equal "/something/awesome/blog/posts/1", response.body end - test "[APP] generating engine's url with polymorphic path" do + test "[APP] generating engine's URL with polymorphic path" do get "/polymorphic_path_for_engine" assert_equal "/awesome/blog/posts/1", response.body end @@ -253,7 +253,7 @@ module TestGenerationPrefix assert_equal "/posts/1", response.body end - test "[APP] generating engine's url with url_for(@post)" do + test "[APP] generating engine's URL with url_for(@post)" do get "/polymorphic_with_url_for" assert_equal "http://www.example.com/awesome/blog/posts/1", response.body end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 2a4d59affe..eb49396145 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -411,7 +411,7 @@ class RequestPath < BaseRequestTest assert_equal "/foo?bar", path end - test "original_url returns url built using ORIGINAL_FULLPATH" do + test "original_url returns URL built using ORIGINAL_FULLPATH" do request = stub_request("ORIGINAL_FULLPATH" => "/foo?bar", "HTTP_HOST" => "example.org", "rack.url_scheme" => "http") diff --git a/actionpack/test/dispatch/routing/route_set_test.rb b/actionpack/test/dispatch/routing/route_set_test.rb index e61d47b160..e6a2c35798 100644 --- a/actionpack/test/dispatch/routing/route_set_test.rb +++ b/actionpack/test/dispatch/routing/route_set_test.rb @@ -29,7 +29,7 @@ module ActionDispatch assert_not empty? end - test "url helpers are added when route is added" do + test "URL helpers are added when route is added" do draw do get "foo", to: SimpleApp.new("foo#index") end @@ -48,7 +48,7 @@ module ActionDispatch assert_equal "/bar", url_helpers.bar_path end - test "url helpers are updated when route is updated" do + test "URL helpers are updated when route is updated" do draw do get "bar", to: SimpleApp.new("bar#index"), as: :bar end @@ -62,7 +62,7 @@ module ActionDispatch assert_equal "/baz", url_helpers.bar_path end - test "url helpers are removed when route is removed" do + test "URL helpers are removed when route is removed" do draw do get "foo", to: SimpleApp.new("foo#index") get "bar", to: SimpleApp.new("bar#index") diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 7b763ec2bd..4645ba476f 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4392,7 +4392,7 @@ class TestNamedRouteUrlHelpers < ActionDispatch::IntegrationTest include Routes.url_helpers - test "url helpers do not ignore nil parameters when using non-optimized routes" do + test "URL helpers do not ignore nil parameters when using non-optimized routes" do Routes.stub :optimize_routes_generation?, false do get "/categories/1" assert_response :success @@ -4764,7 +4764,7 @@ class TestUrlGenerationErrors < ActionDispatch::IntegrationTest include Routes.url_helpers - test "url helpers raise a 'missing keys' error for a nil param with optimized helpers" do + test "URL helpers raise a 'missing keys' error for a nil param with optimized helpers" do url, missing = { action: "show", controller: "products", id: nil }, [:id] message = "No route matches #{url.inspect}, missing required keys: #{missing.inspect}" @@ -4772,7 +4772,7 @@ class TestUrlGenerationErrors < ActionDispatch::IntegrationTest assert_equal message, error.message end - test "url helpers raise a 'constraint failure' error for a nil param with non-optimized helpers" do + test "URL helpers raise a 'constraint failure' error for a nil param with non-optimized helpers" do url, missing = { action: "show", controller: "products", id: nil }, [:id] message = "No route matches #{url.inspect}, possible unmatched constraints: #{missing.inspect}" @@ -4780,15 +4780,15 @@ class TestUrlGenerationErrors < ActionDispatch::IntegrationTest assert_equal message, error.message end - test "url helpers raise message with mixed parameters when generation fails" do + test "URL helpers raise message with mixed parameters when generation fails" do url, missing = { action: "show", controller: "products", id: nil, "id" => "url-tested" }, [:id] message = "No route matches #{url.inspect}, possible unmatched constraints: #{missing.inspect}" - # Optimized url helper + # Optimized URL helper error = assert_raises(ActionController::UrlGenerationError) { product_path(nil, "id" => "url-tested") } assert_equal message, error.message - # Non-optimized url helper + # Non-optimized URL helper error = assert_raises(ActionController::UrlGenerationError, message) { product_path(id: nil, "id" => "url-tested") } assert_equal message, error.message end @@ -5006,7 +5006,7 @@ class FlashRedirectTest < ActionDispatch::IntegrationTest ) Rotations = ActiveSupport::Messages::RotationConfiguration.new SIGNED_COOKIE_SALT = "signed cookie" - ENCRYPTED_SIGNED_COOKIE_SALT = "sigend encrypted cookie" + ENCRYPTED_SIGNED_COOKIE_SALT = "signed encrypted cookie" class KeyGeneratorMiddleware def initialize(app) diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 43688fc8a7..abb97804e1 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,12 @@ +* Only clear ActionView cache in development on file changes + + To speed up development mode, view caches are only cleared when files in + the view paths have changed. Applications which have implemented custom + `ActionView::Resolver` subclasses may need to add their own cache clearing. + + *John Hawthorn* + + ## Rails 6.0.0.beta3 (March 11, 2019) ## * Only accept formats from registered mime types @@ -14,18 +23,19 @@ ## Rails 6.0.0.beta2 (February 25, 2019) ## -* ActionView::Template.finalize_compiled_template_methods is deprecated with +* `ActionView::Template.finalize_compiled_template_methods` is deprecated with no replacement. *tenderlove* -* config.action_view.finalize_compiled_template_methods is deprecated with +* `config.action_view.finalize_compiled_template_methods` is deprecated with no replacement. *tenderlove* * Ensure unique DOM IDs for collection inputs with float values. - Fixes #34974 + + Fixes #34974. *Mark Edmondson* diff --git a/actionview/lib/action_view.rb b/actionview/lib/action_view.rb index 8cb4648a67..f398ba0ff0 100644 --- a/actionview/lib/action_view.rb +++ b/actionview/lib/action_view.rb @@ -81,6 +81,7 @@ module ActionView end end + autoload :CacheExpiry autoload :TestCase def self.eager_load! diff --git a/actionview/lib/action_view/cache_expiry.rb b/actionview/lib/action_view/cache_expiry.rb new file mode 100644 index 0000000000..820afc093d --- /dev/null +++ b/actionview/lib/action_view/cache_expiry.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module ActionView + class CacheExpiry + class Executor + def initialize(watcher:) + @cache_expiry = CacheExpiry.new(watcher: watcher) + end + + def before(target) + @cache_expiry.clear_cache_if_necessary + end + end + + def initialize(watcher:) + @watched_dirs = nil + @watcher_class = watcher + @watcher = nil + end + + def clear_cache_if_necessary + watched_dirs = dirs_to_watch + if watched_dirs != @watched_dirs + @watched_dirs = watched_dirs + @watcher = @watcher_class.new([], watched_dirs) do + clear_cache + end + @watcher.execute + else + @watcher.execute_if_updated + end + end + + def clear_cache + ActionView::LookupContext::DetailsKey.clear + end + + private + + def dirs_to_watch + fs_paths = all_view_paths.grep(FileSystemResolver) + fs_paths.map(&:path).sort.uniq + end + + def all_view_paths + ActionView::ViewPaths.all_view_paths.flat_map(&:paths) + end + end +end diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 9fa8d7eab1..97a6da3634 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -6,12 +6,6 @@ module ActionView class Digestor @@digest_mutex = Mutex.new - module PerExecutionDigestCacheExpiry - def self.before(target) - ActionView::LookupContext::DetailsKey.clear - end - end - class << self # Supported options: # diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index c0996049f0..5d4ff36425 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -24,7 +24,7 @@ module ActionView mattr_accessor :default_enforce_utf8, default: true - # Starts a form tag that points the action to a url configured with <tt>url_for_options</tt> just like + # Starts a form tag that points the action to a URL configured with <tt>url_for_options</tt> just like # ActionController::Base#url_for. The method for the form defaults to POST. # # ==== Options diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index d63ada3890..df83dff681 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -553,7 +553,7 @@ module ActionView url_string = URI.parser.unescape(url_for(options)).force_encoding(Encoding::BINARY) # We ignore any extra parameters in the request_uri if the - # submitted url doesn't have any either. This lets the function + # submitted URL doesn't have any either. This lets the function # work with things like ?order=asc # the behaviour can be disabled with check_parameters: true request_uri = url_string.index("?") || check_parameters ? request.fullpath : request.path diff --git a/actionview/lib/action_view/railtie.rb b/actionview/lib/action_view/railtie.rb index a25e1d3d02..8bd526cdf9 100644 --- a/actionview/lib/action_view/railtie.rb +++ b/actionview/lib/action_view/railtie.rb @@ -81,7 +81,7 @@ module ActionView initializer "action_view.per_request_digest_cache" do |app| ActiveSupport.on_load(:action_view) do unless ActionView::Resolver.caching? - app.executor.to_run ActionView::Digestor::PerExecutionDigestCacheExpiry + app.executor.to_run ActionView::CacheExpiry::Executor.new(watcher: app.config.file_watcher) end end end diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index 9548fe12c4..1faef9ca81 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -26,7 +26,12 @@ module ActionView elsif options.key?(:html) Template::HTML.new(options[:html], formats.first) elsif options.key?(:file) - @lookup_context.with_fallbacks.find_file(options[:file], nil, false, keys, @details) + if File.exist?(options[:file]) + Template::RawFile.new(options[:file]) + else + ActiveSupport::Deprecation.warn "render file: should be given the absolute path to a file" + @lookup_context.with_fallbacks.find_file(options[:file], nil, false, keys, @details) + end elsif options.key?(:inline) handler = Template.handler_for_extension(options[:type] || "erb") format = if handler.respond_to?(:default_format) @@ -84,6 +89,7 @@ module ActionView when String begin if layout.start_with?("/") + ActiveSupport::Deprecation.warn "Rendering layouts from an absolute path is deprecated." @lookup_context.with_fallbacks.find_template(layout, nil, false, [], details) else @lookup_context.find_template(layout, nil, false, [], details) diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 7bc5e86edb..94f8a194a0 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -113,6 +113,7 @@ module ActionView eager_autoload do autoload :Error + autoload :RawFile autoload :Handlers autoload :HTML autoload :Inline @@ -139,7 +140,7 @@ module ActionView @virtual_path = virtual_path @variable = if @virtual_path - base = @virtual_path[-1] == "/" ? "" : File.basename(@virtual_path) + base = @virtual_path[-1] == "/" ? "" : ::File.basename(@virtual_path) base =~ /\A_?(.*?)(?:\.\w+)*\z/ $1.to_sym end diff --git a/actionview/lib/action_view/template/raw_file.rb b/actionview/lib/action_view/template/raw_file.rb new file mode 100644 index 0000000000..623351305f --- /dev/null +++ b/actionview/lib/action_view/template/raw_file.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module ActionView #:nodoc: + # = Action View RawFile Template + class Template #:nodoc: + class RawFile #:nodoc: + attr_accessor :type, :format + + def initialize(filename) + @filename = filename.to_s + extname = ::File.extname(filename).delete(".") + @type = Template::Types[extname] || Template::Types[:text] + @format = @type.symbol + end + + def identifier + @filename + end + + def render(*args) + ::File.read(@filename) + end + + def formats; Array(format); end + deprecate :formats + end + end +end diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 3b4c8a94bc..095e6cc3a1 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -280,6 +280,8 @@ module ActionView # A resolver that loads files from the filesystem. class FileSystemResolver < PathResolver + attr_reader :path + def initialize(path, pattern = nil) raise ArgumentError, "path already is a Resolver class" if path.is_a?(Resolver) super(pattern) diff --git a/actionview/test/actionpack/abstract/render_test.rb b/actionview/test/actionpack/abstract/render_test.rb index d863548a5c..e4e8ac93b2 100644 --- a/actionview/test/actionpack/abstract/render_test.rb +++ b/actionview/test/actionpack/abstract/render_test.rb @@ -26,7 +26,7 @@ module AbstractController end def file - render file: "some/file" + ActiveSupport::Deprecation.silence { render file: "some/file" } end def inline diff --git a/actionview/test/actionpack/controller/layout_test.rb b/actionview/test/actionpack/controller/layout_test.rb index 838b564c5d..f946e4360d 100644 --- a/actionview/test/actionpack/controller/layout_test.rb +++ b/actionview/test/actionpack/controller/layout_test.rb @@ -233,7 +233,9 @@ class LayoutSetInResponseTest < ActionController::TestCase def test_absolute_pathed_layout @controller = AbsolutePathLayoutController.new - get :hello + assert_deprecated do + get :hello + end assert_equal "layout_test.erb hello.erb", @response.body.strip end end diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb index 52c3c54d96..c8ce7366d1 100644 --- a/actionview/test/actionpack/controller/render_test.rb +++ b/actionview/test/actionpack/controller/render_test.rb @@ -872,48 +872,64 @@ class RenderTest < ActionController::TestCase # :ported: def test_render_file_with_instance_variables - get :render_file_with_instance_variables + assert_deprecated do + get :render_file_with_instance_variables + end assert_equal "The secret is in the sauce\n", @response.body end def test_render_file - get :hello_world_file + assert_deprecated do + get :hello_world_file + end assert_equal "Hello world!", @response.body end # :ported: def test_render_file_not_using_full_path - get :render_file_not_using_full_path + assert_deprecated do + get :render_file_not_using_full_path + end assert_equal "The secret is in the sauce\n", @response.body end # :ported: def test_render_file_not_using_full_path_with_dot_in_path - get :render_file_not_using_full_path_with_dot_in_path + assert_deprecated do + get :render_file_not_using_full_path_with_dot_in_path + end assert_equal "The secret is in the sauce\n", @response.body end # :ported: def test_render_file_using_pathname - get :render_file_using_pathname + assert_deprecated do + get :render_file_using_pathname + end assert_equal "The secret is in the sauce\n", @response.body end # :ported: def test_render_file_with_locals - get :render_file_with_locals + assert_deprecated do + get :render_file_with_locals + end assert_equal "The secret is in the sauce\n", @response.body end # :ported: def test_render_file_as_string_with_locals - get :render_file_as_string_with_locals + assert_deprecated do + get :render_file_as_string_with_locals + end assert_equal "The secret is in the sauce\n", @response.body end # :assessed: def test_render_file_from_template - get :render_file_from_template + assert_deprecated do + get :render_file_from_template + end assert_equal "The secret is in the sauce\n", @response.body end @@ -1133,11 +1149,19 @@ class RenderTest < ActionController::TestCase end def test_bad_render_to_string_still_throws_exception - assert_raise(ActionView::MissingTemplate) { get :render_to_string_with_exception } + assert_deprecated do + assert_raise(ActionView::MissingTemplate) do + get :render_to_string_with_exception + end + end end def test_render_to_string_that_throws_caught_exception_doesnt_break_assigns - assert_nothing_raised { get :render_to_string_with_caught_exception } + assert_deprecated do + assert_nothing_raised do + get :render_to_string_with_caught_exception + end + end assert_equal "i'm before the render", @controller.instance_variable_get(:@before) assert_equal "i'm after the render", @controller.instance_variable_get(:@after) end diff --git a/actionview/test/template/javascript_helper_test.rb b/actionview/test/template/javascript_helper_test.rb index 4c28aeaee1..f974e5ae0c 100644 --- a/actionview/test/template/javascript_helper_test.rb +++ b/actionview/test/template/javascript_helper_test.rb @@ -54,7 +54,7 @@ class JavaScriptHelperTest < ActionView::TestCase assert_equal "foo", output_buffer, "javascript_tag without a block should not concat to output_buffer" end - # Setting the :extname option will control what extension (if any) is appended to the url for assets + # Setting the :extname option will control what extension (if any) is appended to the URL for assets def test_javascript_include_tag assert_dom_equal "<script src='/foo.js'></script>", javascript_include_tag("/foo") assert_dom_equal "<script src='/foo'></script>", javascript_include_tag("/foo", extname: false) diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index 85735139c1..8b160a7336 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -51,9 +51,20 @@ class AVLogSubscriberTest < ActiveSupport::TestCase def @view.combined_fragment_cache_key(*); "ahoy `controller` dependency"; end end + def test_render_template_template + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + @view.render(template: "test/hello_world") + wait + + assert_equal 2, @logger.logged(:info).size + assert_match(/Rendering test\/hello_world\.erb/, @logger.logged(:info).first) + assert_match(/Rendered test\/hello_world\.erb/, @logger.logged(:info).last) + end + end + def test_render_file_template Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do - @view.render(file: "test/hello_world") + @view.render(file: "#{FIXTURE_LOAD_PATH}/test/hello_world.erb") wait assert_equal 2, @logger.logged(:info).size diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index baff7cd83f..15c41051de 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -53,15 +53,20 @@ module RenderTestCases assert_match(/You invoked render but did not give any of (.+) option\./, e.message) end + def test_render_template + assert_equal "Hello world!", @view.render(template: "test/hello_world") + end + + def test_render_file - assert_equal "Hello world!", @view.render(file: "test/hello_world") + assert_equal "Hello world!", assert_deprecated { @view.render(file: "test/hello_world") } end # Test if :formats, :locale etc. options are passed correctly to the resolvers. def test_render_file_with_format - assert_match "<h1>No Comment</h1>", @view.render(file: "comments/empty", formats: [:html]) - assert_match "<error>No Comment</error>", @view.render(file: "comments/empty", formats: [:xml]) - assert_match "<error>No Comment</error>", @view.render(file: "comments/empty", formats: :xml) + assert_match "<h1>No Comment</h1>", assert_deprecated { @view.render(file: "comments/empty", formats: [:html]) } + assert_match "<error>No Comment</error>", assert_deprecated { @view.render(file: "comments/empty", formats: [:xml]) } + assert_match "<error>No Comment</error>", assert_deprecated { @view.render(file: "comments/empty", formats: :xml) } end def test_render_template_with_format @@ -94,8 +99,8 @@ module RenderTestCases end def test_render_file_with_locale - assert_equal "<h1>Kein Kommentar</h1>", @view.render(file: "comments/empty", locale: [:de]) - assert_equal "<h1>Kein Kommentar</h1>", @view.render(file: "comments/empty", locale: :de) + assert_equal "<h1>Kein Kommentar</h1>", assert_deprecated { @view.render(file: "comments/empty", locale: [:de]) } + assert_equal "<h1>Kein Kommentar</h1>", assert_deprecated { @view.render(file: "comments/empty", locale: :de) } end def test_render_template_with_locale @@ -107,8 +112,8 @@ module RenderTestCases end def test_render_file_with_handlers - assert_equal "<h1>No Comment</h1>\n", @view.render(file: "comments/empty", handlers: [:builder]) - assert_equal "<h1>No Comment</h1>\n", @view.render(file: "comments/empty", handlers: :builder) + assert_equal "<h1>No Comment</h1>\n", assert_deprecated { @view.render(file: "comments/empty", handlers: [:builder]) } + assert_equal "<h1>No Comment</h1>\n", assert_deprecated { @view.render(file: "comments/empty", handlers: :builder) } end def test_render_template_with_handlers @@ -156,22 +161,27 @@ module RenderTestCases assert_equal "Elastica", @view.render(template: "/shared") end - def test_render_file_with_full_path + def test_render_file_with_full_path_no_extension template_path = File.expand_path("../fixtures/test/hello_world", __dir__) + assert_equal "Hello world!", assert_deprecated { @view.render(file: template_path) } + end + + def test_render_file_with_full_path + template_path = File.expand_path("../fixtures/test/hello_world.erb", __dir__) assert_equal "Hello world!", @view.render(file: template_path) end def test_render_file_with_instance_variables - assert_equal "The secret is in the sauce\n", @view.render(file: "test/render_file_with_ivar") + assert_equal "The secret is in the sauce\n", assert_deprecated { @view.render(file: "test/render_file_with_ivar") } end def test_render_file_with_locals locals = { secret: "in the sauce" } - assert_equal "The secret is in the sauce\n", @view.render(file: "test/render_file_with_locals", locals: locals) + assert_equal "The secret is in the sauce\n", assert_deprecated { @view.render(file: "test/render_file_with_locals", locals: locals) } end def test_render_file_not_using_full_path_with_dot_in_path - assert_equal "The secret is in the sauce\n", @view.render(file: "test/dot.directory/render_file_with_ivar") + assert_equal "The secret is in the sauce\n", assert_deprecated { @view.render(file: "test/dot.directory/render_file_with_ivar") } end def test_render_partial_from_default @@ -292,7 +302,7 @@ module RenderTestCases end def test_render_file_with_errors - e = assert_raises(ActionView::Template::Error) { @view.render(file: File.expand_path("test/_raise", FIXTURE_LOAD_PATH)) } + e = assert_raises(ActionView::Template::Error) { assert_deprecated { @view.render(file: File.expand_path("test/_raise", FIXTURE_LOAD_PATH)) } } assert_match %r!method.*doesnt_exist!, e.message assert_equal "", e.sub_template_message assert_equal "1", e.line_number diff --git a/actionview/test/template/streaming_render_test.rb b/actionview/test/template/streaming_render_test.rb index a5b59a700e..a5e673e71e 100644 --- a/actionview/test/template/streaming_render_test.rb +++ b/actionview/test/template/streaming_render_test.rb @@ -47,12 +47,12 @@ class FiberedTest < SetupFiberedBase end def test_render_file - assert_equal "Hello world!", buffered_render(file: "test/hello_world") + assert_equal "Hello world!", assert_deprecated { buffered_render(file: "test/hello_world") } end def test_render_file_with_locals locals = { secret: "in the sauce" } - assert_equal "The secret is in the sauce\n", buffered_render(file: "test/render_file_with_locals", locals: locals) + assert_equal "The secret is in the sauce\n", assert_deprecated { buffered_render(file: "test/render_file_with_locals", locals: locals) } end def test_render_partial diff --git a/actionview/test/ujs/public/test/call-remote.js b/actionview/test/ujs/public/test/call-remote.js index 0f92007007..fb033491f9 100644 --- a/actionview/test/ujs/public/test/call-remote.js +++ b/actionview/test/ujs/public/test/call-remote.js @@ -53,7 +53,7 @@ asyncTest('form default method is GET', 1, function() { }) }) -asyncTest('form url is picked up from "action"', 1, function() { +asyncTest('form URL is picked up from "action"', 1, function() { buildForm({ method: 'post' }) submit(function(e, data, status, xhr) { @@ -61,7 +61,7 @@ asyncTest('form url is picked up from "action"', 1, function() { }) }) -asyncTest('form url is read from "action" not "href"', 1, function() { +asyncTest('form URL is read from "action" not "href"', 1, function() { buildForm({ method: 'post', href: '/echo2' }) submit(function(e, data, status, xhr) { @@ -69,7 +69,7 @@ asyncTest('form url is read from "action" not "href"', 1, function() { }) }) -asyncTest('form url is read from submit button "formaction" if submit is triggered by that button', 1, function() { +asyncTest('form URL is read from submit button "formaction" if submit is triggered by that button', 1, function() { var submitButton = $('<input type="submit" formaction="/echo">') buildForm({ method: 'post', href: '/echo2' }) diff --git a/actionview/test/ujs/public/test/data-remote.js b/actionview/test/ujs/public/test/data-remote.js index 55d39b0a52..9e41067549 100644 --- a/actionview/test/ujs/public/test/data-remote.js +++ b/actionview/test/ujs/public/test/data-remote.js @@ -121,7 +121,7 @@ asyncTest('clicking on a link with both query string in href and data-params', 4 App.assertGetRequest(data) equal(data.params.data1, 'value1', 'ajax arguments should have key data1 with right value') equal(data.params.data2, 'value2', 'ajax arguments should have key data2 with right value') - equal(data.params.data3, 'value3', 'query string in url should be passed to server with right value') + equal(data.params.data3, 'value3', 'query string in URL should be passed to server with right value') }) .bindNative('ajax:complete', function() { start() }) .triggerNative('click') @@ -135,7 +135,7 @@ asyncTest('clicking on a link with both query string in href and data-params wit App.assertPostRequest(data) equal(data.params.data1, 'value1', 'ajax arguments should have key data1 with right value') equal(data.params.data2, 'value2', 'ajax arguments should have key data2 with right value') - equal(data.params.data3, 'value3', 'query string in url should be passed to server with right value') + equal(data.params.data3, 'value3', 'query string in URL should be passed to server with right value') }) .bindNative('ajax:complete', function() { start() }) .triggerNative('click') diff --git a/actionview/test/ujs/public/test/settings.js b/actionview/test/ujs/public/test/settings.js index 682d044403..ff2b057012 100644 --- a/actionview/test/ujs/public/test/settings.js +++ b/actionview/test/ujs/public/test/settings.js @@ -18,7 +18,7 @@ App.assertPostRequest = function(requestEnv) { } App.assertRequestPath = function(requestEnv, path) { - equal(requestEnv['PATH_INFO'], path, 'request should be sent to right url') + equal(requestEnv['PATH_INFO'], path, 'request should be sent to right URL') } App.getVal = function(el) { diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index e09ba4b055..138c8a8ef4 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -34,7 +34,7 @@ *Edouard Chin* -* Restore HashWithIndifferentAccess support to ActiveJob::Arguments.deserialize. +* Restore `HashWithIndifferentAccess` support to `ActiveJob::Arguments.deserialize`. *Gannon McGibbon* diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 74a0913b6f..ad87abfa3a 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -41,12 +41,12 @@ Before: Day.new({"day(1i)"=>"1", "day(2i)"=>"1", "day(3i)"=>"1"}) - => #<Day id: nil, day: "0001-01-03", created_at: nil, updated_at: nil> + # => #<Day id: nil, day: "0001-01-03", created_at: nil, updated_at: nil> After: Day.new({"day(1i)"=>"1", "day(2i)"=>"1", "day(3i)"=>"1"}) - => #<Day id: nil, day: "0001-01-01", created_at: nil, updated_at: nil> + # => #<Day id: nil, day: "0001-01-01", created_at: nil, updated_at: nil> Fixes #28521. diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 690d487a09..d73d9ade41 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,4 +1,11 @@ -* Add `ActiveRecord::Relation#extract_associated` for extracting associated records from a relation. +* Assign all attributes before calling `build` to ensure the child record is visible in + `before_add` and `after_add` callbacks for `has_many :through` associations. + + Fixes #33249. + + *Ryan H. Kerr* + +* Add `ActiveRecord::Relation#extract_associated` for extracting associated records from a relation. ``` account.memberships.extract_associated(:user) @@ -341,7 +348,7 @@ *Gannon McGibbon* -* Cached columns_hash fields should be excluded from ResultSet#column_types +* Cached `columns_hash` fields should be excluded from `ResultSet#column_types`. PR #34528 addresses the inconsistent behaviour when attribute is defined for an ignored column. The following test was passing for SQLite and MySQL, but failed for PostgreSQL: @@ -372,12 +379,12 @@ * Make the implicit order column configurable. - When calling ordered finder methods such as +first+ or +last+ without an + When calling ordered finder methods such as `first` or `last` without an explicit order clause, ActiveRecord sorts records by primary key. This can result in unpredictable and surprising behaviour when the primary key is not an auto-incrementing integer, for example when it's a UUID. This change makes it possible to override the column used for implicit ordering such - that +first+ and +last+ will return more predictable results. + that `first` and `last` will return more predictable results. Example: @@ -447,7 +454,7 @@ *Sean Griffin* -* Add support for hash and url configs in database hash of `ActiveRecord::Base.connected_to`. +* Add support for hash and URL configs in database hash of `ActiveRecord::Base.connected_to`. ```` User.connected_to(database: { writing: "postgres://foo" }) do @@ -523,10 +530,10 @@ * Enum raises on invalid definition values - When defining a Hash enum it can be easy to use [] instead of {}. This + When defining a Hash enum it can be easy to use `[]` instead of `{}`. This commit checks that only valid definition values are provided, those can be a Hash, an array of Symbols or an array of Strings. Otherwise it - raises an ArgumentError. + raises an `ArgumentError`. Fixes #33961 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 84a9797aa5..0d384950fe 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -57,21 +57,14 @@ module ActiveRecord @through_records[record.object_id] ||= begin ensure_mutable - through_record = through_association.build(*options_for_through_record) - through_record.send("#{source_reflection.name}=", record) + attributes = through_scope_attributes + attributes[source_reflection.name] = record + attributes[source_reflection.foreign_type] = options[:source_type] if options[:source_type] - if options[:source_type] - through_record.send("#{source_reflection.foreign_type}=", options[:source_type]) - end - - through_record + through_association.build(attributes) end end - def options_for_through_record - [through_scope_attributes] - end - def through_scope_attributes scope.where_values_hash(through_association.reflection.name.to_s). except!(through_association.reflection.foreign_key, diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index 7431a1c759..44b5cfc738 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -7,7 +7,7 @@ require "active_record/database_configurations/url_config" module ActiveRecord # ActiveRecord::DatabaseConfigurations returns an array of DatabaseConfig # objects (either a HashConfig or UrlConfig) that are constructed from the - # application's database configuration hash or url string. + # application's database configuration hash or URL string. class DatabaseConfigurations attr_reader :configurations delegate :any?, to: :configurations diff --git a/activerecord/lib/active_record/insert_all.rb b/activerecord/lib/active_record/insert_all.rb index d30aee7c00..4b02d40aa0 100644 --- a/activerecord/lib/active_record/insert_all.rb +++ b/activerecord/lib/active_record/insert_all.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module ActiveRecord - class InsertAll + class InsertAll # :nodoc: attr_reader :model, :connection, :inserts, :keys attr_reader :on_duplicate, :returning, :unique_by @@ -73,15 +73,11 @@ module ActiveRecord raise ArgumentError, "#{connection.class} does not support :returning" end - unless %i{ raise skip update }.member?(on_duplicate) - raise NotImplementedError, "#{on_duplicate.inspect} is an unknown value for :on_duplicate. Valid values are :raise, :skip, and :update" - end - - if on_duplicate == :skip && !connection.supports_insert_on_duplicate_skip? + if skip_duplicates? && !connection.supports_insert_on_duplicate_skip? raise ArgumentError, "#{connection.class} does not support skipping duplicates" end - if on_duplicate == :update && !connection.supports_insert_on_duplicate_update? + if update_duplicates? && !connection.supports_insert_on_duplicate_update? raise ArgumentError, "#{connection.class} does not support upsert" end @@ -137,16 +133,16 @@ module ActiveRecord end def returning - quote_columns(insert_all.returning).join(",") if insert_all.returning + format_columns(insert_all.returning) if insert_all.returning end def conflict_target if index = insert_all.unique_by - sql = +"(#{quote_columns(index.columns).join(',')})" + sql = +"(#{format_columns(index.columns)})" sql << " WHERE #{index.where}" if index.where sql elsif update_duplicates? - "(#{quote_columns(insert_all.primary_keys).join(',')})" + "(#{format_columns(insert_all.primary_keys)})" end end @@ -158,7 +154,7 @@ module ActiveRecord attr_reader :connection, :insert_all def columns_list - quote_columns(insert_all.keys).join(",") + format_columns(insert_all.keys) end def extract_types_from_columns_on(table_name, keys:) @@ -170,6 +166,10 @@ module ActiveRecord keys.map { |key| [ key, connection.lookup_cast_type_from_column(columns[key]) ] }.to_h end + def format_columns(columns) + quote_columns(columns).join(",") + end + def quote_columns(columns) columns.map(&connection.method(:quote_column_name)) end diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index f021a8f6c4..447def8d77 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -222,6 +222,16 @@ db_namespace = namespace :db do desc "Creates the database, loads the schema, and initializes with the seed data (use db:reset to also drop the database first)" task setup: ["db:schema:load_if_ruby", "db:structure:load_if_sql", :seed] + desc "Runs setup if database does not exist, or runs migrations if it does" + task prepare: :load_config do + ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config| + ActiveRecord::Base.establish_connection(db_config.config) + db_namespace["migrate"].invoke + rescue ActiveRecord::NoDatabaseError + db_namespace["setup"].invoke + end + end + desc "Loads the seed data from db/seeds.rb" task seed: :load_config do db_namespace["abort_if_pending_migrations"].invoke diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 36c2422d84..dd821431e1 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -389,8 +389,6 @@ module ActiveRecord stmt.set Arel.sql(klass.sanitize_sql_for_assignment(updates, table.name)) end - stmt.comment(*arel.comment_node.values) if arel.comment_node - @klass.connection.update stmt, "#{@klass} Update All" end @@ -506,7 +504,6 @@ module ActiveRecord stmt.offset(arel.offset) stmt.order(*arel.orders) stmt.wheres = arel.constraints - stmt.comment(*arel.comment_node.values) if arel.comment_node affected = @klass.connection.delete(stmt, "#{@klass} Destroy") diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index c37855172b..b0535cfff5 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -100,7 +100,7 @@ module ActiveRecord # # === conditions # - # If you want to add conditions to your included models you'll have + # If you want to add string conditions to your included models, you'll have # to explicitly reference them. For example: # # User.includes(:posts).where('posts.name = ?', 'example') @@ -111,6 +111,12 @@ module ActiveRecord # # Note that #includes works with association names while #references needs # the actual table name. + # + # If you pass the conditions via hash, you don't need to call #references + # explicitly, as #where references the tables for you. For example, this + # will work correctly: + # + # User.includes(:posts).where(posts: { name: 'example' }) def includes(*args) check_if_method_has_arguments!(:includes, args) spawn.includes!(*args) diff --git a/activerecord/lib/active_record/store.rb b/activerecord/lib/active_record/store.rb index 3537e2d008..6fecb06897 100644 --- a/activerecord/lib/active_record/store.rb +++ b/activerecord/lib/active_record/store.rb @@ -11,6 +11,12 @@ module ActiveRecord # of the model. This is very helpful for easily exposing store keys to a form or elsewhere that's # already built around just accessing attributes on the model. # + # Every accessor comes with dirty tracking methods (+key_changed?+, +key_was+ and +key_change+) and + # methods to access the changes made during the last save (+saved_change_to_key?+, +saved_change_to_key+ and + # +key_before_last_save+). + # + # NOTE: There is no +key_will_change!+ method for accessors, use +store_will_change!+ instead. + # # Make sure that you declare the database column used for the serialized store as a text, so there's # plenty of room. # @@ -49,6 +55,12 @@ module ActiveRecord # u.settings[:country] # => 'Denmark' # u.settings['country'] # => 'Denmark' # + # # Dirty tracking + # u.color = 'green' + # u.color_changed? # => true + # u.color_was # => 'black' + # u.color_change # => ['black', 'red'] + # # # Add additional accessors to an existing store through store_accessor # class SuperUser < User # store_accessor :settings, :privileges, :servants @@ -127,6 +139,42 @@ module ActiveRecord define_method(accessor_key) do read_store_attribute(store_attribute, key) end + + define_method("#{accessor_key}_changed?") do + return false unless attribute_changed?(store_attribute) + prev_store, new_store = changes[store_attribute] + prev_store&.dig(key) != new_store&.dig(key) + end + + define_method("#{accessor_key}_change") do + return unless attribute_changed?(store_attribute) + prev_store, new_store = changes[store_attribute] + [prev_store&.dig(key), new_store&.dig(key)] + end + + define_method("#{accessor_key}_was") do + return unless attribute_changed?(store_attribute) + prev_store, _new_store = changes[store_attribute] + prev_store&.dig(key) + end + + define_method("saved_change_to_#{accessor_key}?") do + return false unless saved_change_to_attribute?(store_attribute) + prev_store, new_store = saved_change_to_attribute(store_attribute) + prev_store&.dig(key) != new_store&.dig(key) + end + + define_method("saved_change_to_#{accessor_key}") do + return unless saved_change_to_attribute?(store_attribute) + prev_store, new_store = saved_change_to_attribute(store_attribute) + [prev_store&.dig(key), new_store&.dig(key)] + end + + define_method("#{accessor_key}_before_last_save") do + return unless saved_change_to_attribute?(store_attribute) + prev_store, _new_store = saved_change_to_attribute(store_attribute) + prev_store&.dig(key) + end end end diff --git a/activerecord/lib/arel/nodes/delete_statement.rb b/activerecord/lib/arel/nodes/delete_statement.rb index 56249b2bad..a419975335 100644 --- a/activerecord/lib/arel/nodes/delete_statement.rb +++ b/activerecord/lib/arel/nodes/delete_statement.rb @@ -3,7 +3,7 @@ module Arel # :nodoc: all module Nodes class DeleteStatement < Arel::Nodes::Node - attr_accessor :left, :right, :orders, :limit, :offset, :key, :comment + attr_accessor :left, :right, :orders, :limit, :offset, :key alias :relation :left alias :relation= :left= @@ -18,18 +18,16 @@ module Arel # :nodoc: all @limit = nil @offset = nil @key = nil - @comment = nil end def initialize_copy(other) super @left = @left.clone if @left @right = @right.clone if @right - @comment = @comment.clone if @comment end def hash - [self.class, @left, @right, @orders, @limit, @offset, @key, @comment].hash + [self.class, @left, @right, @orders, @limit, @offset, @key].hash end def eql?(other) @@ -39,8 +37,7 @@ module Arel # :nodoc: all self.orders == other.orders && self.limit == other.limit && self.offset == other.offset && - self.key == other.key && - self.comment == other.comment + self.key == other.key end alias :== :eql? end diff --git a/activerecord/lib/arel/nodes/insert_statement.rb b/activerecord/lib/arel/nodes/insert_statement.rb index 8430dd23da..d28fd1f6c8 100644 --- a/activerecord/lib/arel/nodes/insert_statement.rb +++ b/activerecord/lib/arel/nodes/insert_statement.rb @@ -3,7 +3,7 @@ module Arel # :nodoc: all module Nodes class InsertStatement < Arel::Nodes::Node - attr_accessor :relation, :columns, :values, :select, :comment + attr_accessor :relation, :columns, :values, :select def initialize super() @@ -11,7 +11,6 @@ module Arel # :nodoc: all @columns = [] @values = nil @select = nil - @comment = nil end def initialize_copy(other) @@ -19,11 +18,10 @@ module Arel # :nodoc: all @columns = @columns.clone @values = @values.clone if @values @select = @select.clone if @select - @comment = @comment.clone if @comment end def hash - [@relation, @columns, @values, @select, @comment].hash + [@relation, @columns, @values, @select].hash end def eql?(other) @@ -31,8 +29,7 @@ module Arel # :nodoc: all self.relation == other.relation && self.columns == other.columns && self.select == other.select && - self.values == other.values && - self.comment == other.comment + self.values == other.values end alias :== :eql? end diff --git a/activerecord/lib/arel/nodes/select_core.rb b/activerecord/lib/arel/nodes/select_core.rb index b6154b7ff4..11b4f39ece 100644 --- a/activerecord/lib/arel/nodes/select_core.rb +++ b/activerecord/lib/arel/nodes/select_core.rb @@ -40,7 +40,6 @@ module Arel # :nodoc: all @groups = @groups.clone @havings = @havings.clone @windows = @windows.clone - @comment = @comment.clone if @comment end def hash diff --git a/activerecord/lib/arel/nodes/update_statement.rb b/activerecord/lib/arel/nodes/update_statement.rb index 015bcd7613..cfaa19e392 100644 --- a/activerecord/lib/arel/nodes/update_statement.rb +++ b/activerecord/lib/arel/nodes/update_statement.rb @@ -3,7 +3,7 @@ module Arel # :nodoc: all module Nodes class UpdateStatement < Arel::Nodes::Node - attr_accessor :relation, :wheres, :values, :orders, :limit, :offset, :key, :comment + attr_accessor :relation, :wheres, :values, :orders, :limit, :offset, :key def initialize @relation = nil @@ -13,18 +13,16 @@ module Arel # :nodoc: all @limit = nil @offset = nil @key = nil - @comment = nil end def initialize_copy(other) super @wheres = @wheres.clone @values = @values.clone - @comment = @comment.clone if @comment end def hash - [@relation, @wheres, @values, @orders, @limit, @offset, @key, @comment].hash + [@relation, @wheres, @values, @orders, @limit, @offset, @key].hash end def eql?(other) @@ -35,8 +33,7 @@ module Arel # :nodoc: all self.orders == other.orders && self.limit == other.limit && self.offset == other.offset && - self.key == other.key && - self.comment == other.comment + self.key == other.key end alias :== :eql? end diff --git a/activerecord/lib/arel/select_manager.rb b/activerecord/lib/arel/select_manager.rb index 4e9f527235..ddc9e394dd 100644 --- a/activerecord/lib/arel/select_manager.rb +++ b/activerecord/lib/arel/select_manager.rb @@ -249,10 +249,6 @@ module Arel # :nodoc: all self end - def comment_node - @ctx.comment - end - private def collapse(exprs) exprs = exprs.compact diff --git a/activerecord/lib/arel/tree_manager.rb b/activerecord/lib/arel/tree_manager.rb index 326c4f995c..0476399618 100644 --- a/activerecord/lib/arel/tree_manager.rb +++ b/activerecord/lib/arel/tree_manager.rb @@ -36,11 +36,6 @@ module Arel # :nodoc: all @ast.wheres << expr self end - - def comment(*values) - @ast.comment = Nodes::Comment.new(values) - self - end end attr_reader :ast diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb index 4192d9efdc..277d553e6c 100644 --- a/activerecord/lib/arel/visitors/to_sql.rb +++ b/activerecord/lib/arel/visitors/to_sql.rb @@ -35,7 +35,6 @@ module Arel # :nodoc: all collect_nodes_for o.wheres, collector, " WHERE ", " AND " collect_nodes_for o.orders, collector, " ORDER BY " maybe_visit o.limit, collector - maybe_visit o.comment, collector end def visit_Arel_Nodes_UpdateStatement(o, collector) @@ -48,7 +47,6 @@ module Arel # :nodoc: all collect_nodes_for o.wheres, collector, " WHERE ", " AND " collect_nodes_for o.orders, collector, " ORDER BY " maybe_visit o.limit, collector - maybe_visit o.comment, collector end def visit_Arel_Nodes_InsertStatement(o, collector) @@ -64,9 +62,9 @@ module Arel # :nodoc: all maybe_visit o.values, collector elsif o.select maybe_visit o.select, collector + else + collector end - - maybe_visit o.comment, collector end def visit_Arel_Nodes_Exists(o, collector) diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb index cd45975f70..671d8211a7 100644 --- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb +++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb @@ -153,6 +153,22 @@ class PostgresqlHstoreTest < ActiveRecord::PostgreSQLTestCase assert_equal "GMT", y.timezone end + def test_changes_with_store_accessors + x = Hstore.new(language: "de") + assert x.language_changed? + assert_nil x.language_was + assert_equal [nil, "de"], x.language_change + x.save! + + assert_not x.language_changed? + x.reload + + x.settings = nil + assert x.language_changed? + assert_equal "de", x.language_was + assert_equal ["de", nil], x.language_change + end + def test_changes_in_place hstore = Hstore.create!(settings: { "one" => "two" }) hstore.settings["three"] = "four" diff --git a/activerecord/test/cases/arel/delete_manager_test.rb b/activerecord/test/cases/arel/delete_manager_test.rb index 63cd1bffe3..0bad02f4d2 100644 --- a/activerecord/test/cases/arel/delete_manager_test.rb +++ b/activerecord/test/cases/arel/delete_manager_test.rb @@ -49,23 +49,5 @@ module Arel dm.where(table[:id].eq(10)).must_equal dm end end - - describe "comment" do - it "chains" do - manager = Arel::DeleteManager.new - manager.comment("deleting").must_equal manager - end - - it "appends a comment to the generated query" do - table = Table.new(:users) - dm = Arel::DeleteManager.new - dm.from table - dm.comment("deletion") - assert_match(%r{DELETE FROM "users" /\* deletion \*/}, dm.to_sql) - - dm.comment("deletion", "with", "comment") - assert_match(%r{DELETE FROM "users" /\* deletion \*/ /\* with \*/ /\* comment \*/}, dm.to_sql) - end - end end end diff --git a/activerecord/test/cases/arel/nodes/delete_statement_test.rb b/activerecord/test/cases/arel/nodes/delete_statement_test.rb index 8ba268653d..3f078063a4 100644 --- a/activerecord/test/cases/arel/nodes/delete_statement_test.rb +++ b/activerecord/test/cases/arel/nodes/delete_statement_test.rb @@ -18,10 +18,8 @@ describe Arel::Nodes::DeleteStatement do it "is equal with equal ivars" do statement1 = Arel::Nodes::DeleteStatement.new statement1.wheres = %w[a b c] - statement1.comment = Arel::Nodes::Comment.new(["comment"]) statement2 = Arel::Nodes::DeleteStatement.new statement2.wheres = %w[a b c] - statement2.comment = Arel::Nodes::Comment.new(["comment"]) array = [statement1, statement2] assert_equal 1, array.uniq.size end @@ -29,14 +27,8 @@ describe Arel::Nodes::DeleteStatement do it "is not equal with different ivars" do statement1 = Arel::Nodes::DeleteStatement.new statement1.wheres = %w[a b c] - statement1.comment = Arel::Nodes::Comment.new(["comment"]) statement2 = Arel::Nodes::DeleteStatement.new statement2.wheres = %w[1 2 3] - statement2.comment = Arel::Nodes::Comment.new(["comment"]) - array = [statement1, statement2] - assert_equal 2, array.uniq.size - statement2.wheres = %w[a b c] - statement2.comment = Arel::Nodes::Comment.new(["other"]) array = [statement1, statement2] assert_equal 2, array.uniq.size end diff --git a/activerecord/test/cases/arel/nodes/insert_statement_test.rb b/activerecord/test/cases/arel/nodes/insert_statement_test.rb index 036576b231..252a0d0d0b 100644 --- a/activerecord/test/cases/arel/nodes/insert_statement_test.rb +++ b/activerecord/test/cases/arel/nodes/insert_statement_test.rb @@ -23,11 +23,9 @@ describe Arel::Nodes::InsertStatement do statement1 = Arel::Nodes::InsertStatement.new statement1.columns = %w[a b c] statement1.values = %w[x y z] - statement1.comment = Arel::Nodes::Comment.new(["comment"]) statement2 = Arel::Nodes::InsertStatement.new statement2.columns = %w[a b c] statement2.values = %w[x y z] - statement2.comment = Arel::Nodes::Comment.new(["comment"]) array = [statement1, statement2] assert_equal 1, array.uniq.size end @@ -36,15 +34,9 @@ describe Arel::Nodes::InsertStatement do statement1 = Arel::Nodes::InsertStatement.new statement1.columns = %w[a b c] statement1.values = %w[x y z] - statement1.comment = Arel::Nodes::Comment.new(["comment"]) statement2 = Arel::Nodes::InsertStatement.new statement2.columns = %w[a b c] statement2.values = %w[1 2 3] - statement2.comment = Arel::Nodes::Comment.new(["comment"]) - array = [statement1, statement2] - assert_equal 2, array.uniq.size - statement2.values = %w[x y z] - statement2.comment = Arel::Nodes::Comment.new("other") array = [statement1, statement2] assert_equal 2, array.uniq.size end diff --git a/activerecord/test/cases/arel/nodes/update_statement_test.rb b/activerecord/test/cases/arel/nodes/update_statement_test.rb index f133ddf7eb..a83ce32f68 100644 --- a/activerecord/test/cases/arel/nodes/update_statement_test.rb +++ b/activerecord/test/cases/arel/nodes/update_statement_test.rb @@ -27,7 +27,6 @@ describe Arel::Nodes::UpdateStatement do statement1.orders = %w[x y z] statement1.limit = 42 statement1.key = "zomg" - statement1.comment = Arel::Nodes::Comment.new(["comment"]) statement2 = Arel::Nodes::UpdateStatement.new statement2.relation = "zomg" statement2.wheres = 2 @@ -35,7 +34,6 @@ describe Arel::Nodes::UpdateStatement do statement2.orders = %w[x y z] statement2.limit = 42 statement2.key = "zomg" - statement2.comment = Arel::Nodes::Comment.new(["comment"]) array = [statement1, statement2] assert_equal 1, array.uniq.size end @@ -48,7 +46,6 @@ describe Arel::Nodes::UpdateStatement do statement1.orders = %w[x y z] statement1.limit = 42 statement1.key = "zomg" - statement1.comment = Arel::Nodes::Comment.new(["comment"]) statement2 = Arel::Nodes::UpdateStatement.new statement2.relation = "zomg" statement2.wheres = 2 @@ -56,11 +53,6 @@ describe Arel::Nodes::UpdateStatement do statement2.orders = %w[x y z] statement2.limit = 42 statement2.key = "wth" - statement2.comment = Arel::Nodes::Comment.new(["comment"]) - array = [statement1, statement2] - assert_equal 2, array.uniq.size - statement2.key = "zomg" - statement2.comment = Arel::Nodes::Comment.new(["other"]) array = [statement1, statement2] assert_equal 2, array.uniq.size end diff --git a/activerecord/test/cases/arel/update_manager_test.rb b/activerecord/test/cases/arel/update_manager_test.rb index e13cb6aa52..cc1b9ac5b3 100644 --- a/activerecord/test/cases/arel/update_manager_test.rb +++ b/activerecord/test/cases/arel/update_manager_test.rb @@ -122,29 +122,5 @@ module Arel @um.key.must_equal @table[:foo] end end - - describe "comment" do - it "chains" do - manager = Arel::UpdateManager.new - manager.comment("updating").must_equal manager - end - - it "appends a comment to the generated query" do - table = Table.new :users - - manager = Arel::UpdateManager.new - manager.table table - - manager.comment("updating") - manager.to_sql.must_be_like %{ - UPDATE "users" /* updating */ - } - - manager.comment("updating", "with", "comment") - manager.to_sql.must_be_like %{ - UPDATE "users" /* updating */ /* with */ /* comment */ - } - end - end end end diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 67e013c6e0..fa540c9dab 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -1474,6 +1474,24 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal [subscription2], post.subscriptions.to_a end + def test_child_is_visible_to_join_model_in_add_association_callbacks + [:before_add, :after_add].each do |callback_name| + sentient_treasure = Class.new(Treasure) do + def self.name; "SentientTreasure"; end + + has_many :pet_treasures, foreign_key: :treasure_id, callback_name => :check_pet! + has_many :pets, through: :pet_treasures + + def check_pet!(added) + raise "No pet!" if added.pet.nil? + end + end + + treasure = sentient_treasure.new + assert_nothing_raised { treasure.pets << pets(:mochi) } + end + end + private def make_model(name) Class.new(ActiveRecord::Base) { define_singleton_method(:name) { name } } diff --git a/activerecord/test/cases/insert_all_test.rb b/activerecord/test/cases/insert_all_test.rb index 0818d7c1ab..fc25701c80 100644 --- a/activerecord/test/cases/insert_all_test.rb +++ b/activerecord/test/cases/insert_all_test.rb @@ -11,6 +11,20 @@ class InsertAllTest < ActiveRecord::TestCase fixtures :books def test_insert + skip unless supports_insert_on_duplicate_skip? + + id = 1_000_000 + + assert_difference "Book.count", +1 do + Book.insert(id: id, name: "Rework", author_id: 1) + end + + Book.upsert(id: id, name: "Remote", author_id: 1) + + assert_equal "Remote", Book.find(id).name + end + + def test_insert! assert_difference "Book.count", +1 do Book.insert! name: "Rework", author_id: 1 end diff --git a/activerecord/test/cases/relation/delete_all_test.rb b/activerecord/test/cases/relation/delete_all_test.rb index 9b76936b7e..d1c13fa1b5 100644 --- a/activerecord/test/cases/relation/delete_all_test.rb +++ b/activerecord/test/cases/relation/delete_all_test.rb @@ -99,23 +99,4 @@ class DeleteAllTest < ActiveRecord::TestCase assert_raise(ActiveRecord::RecordNotFound) { posts(:thinking) } assert posts(:welcome) end - - def test_delete_all_with_annotation_includes_a_query_comment - davids = Author.where(name: "David").annotate("deleting all") - - assert_sql(%r{/\* deleting all \*/}) do - assert_difference("Author.count", -1) { davids.delete_all } - end - end - - def test_delete_all_without_annotation_does_not_include_an_empty_comment - davids = Author.where(name: "David") - - log = capture_sql do - assert_difference("Author.count", -1) { davids.delete_all } - end - - assert_not_predicate log, :empty? - assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty? - end end diff --git a/activerecord/test/cases/relation/update_all_test.rb b/activerecord/test/cases/relation/update_all_test.rb index 526f926841..0500574f28 100644 --- a/activerecord/test/cases/relation/update_all_test.rb +++ b/activerecord/test/cases/relation/update_all_test.rb @@ -241,33 +241,6 @@ class UpdateAllTest < ActiveRecord::TestCase end end - def test_update_all_with_annotation_includes_a_query_comment - tag = Tag.first - - assert_sql(%r{/\* updating all \*/}) do - Post.tagged_with(tag.id).annotate("updating all").update_all(title: "rofl") - end - - posts = Post.tagged_with(tag.id).all.to_a - assert_operator posts.length, :>, 0 - posts.each { |post| assert_equal "rofl", post.title } - end - - def test_update_all_without_annotation_does_not_include_an_empty_comment - tag = Tag.first - - log = capture_sql do - Post.tagged_with(tag.id).update_all(title: "rofl") - end - - assert_not_predicate log, :empty? - assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty? - - posts = Post.tagged_with(tag.id).all.to_a - assert_operator posts.length, :>, 0 - posts.each { |post| assert_equal "rofl", post.title } - end - # Oracle UPDATE does not support ORDER BY unless current_adapter?(:OracleAdapter) def test_update_all_ignores_order_without_limit_from_association diff --git a/activerecord/test/cases/store_test.rb b/activerecord/test/cases/store_test.rb index 4457cfbd37..91c0e959f4 100644 --- a/activerecord/test/cases/store_test.rb +++ b/activerecord/test/cases/store_test.rb @@ -79,6 +79,74 @@ class StoreTest < ActiveRecord::TestCase assert_not_predicate @john, :settings_changed? end + test "updating the store will mark accessor as changed" do + @john.color = "red" + assert @john.color_changed? + end + + test "new record and no accessors changes" do + user = Admin::User.new + assert_not user.color_changed? + assert_nil user.color_was + assert_nil user.color_change + + user.color = "red" + assert user.color_changed? + assert_nil user.color_was + assert_equal "red", user.color_change[1] + end + + test "updating the store won't mark accessor as changed if the whole store was updated" do + @john.settings = { color: @john.color, some: "thing" } + assert @john.settings_changed? + assert_not @john.color_changed? + end + + test "updating the store populates the accessor changed array correctly" do + @john.color = "red" + assert_equal "black", @john.color_was + assert_equal "black", @john.color_change[0] + assert_equal "red", @john.color_change[1] + end + + test "updating the store won't mark accessor as changed if the value isn't changed" do + @john.color = @john.color + assert_not @john.color_changed? + end + + test "nullifying the store mark accessor as changed" do + color = @john.color + @john.settings = nil + assert @john.color_changed? + assert_equal color, @john.color_was + assert_equal [color, nil], @john.color_change + end + + test "dirty methods for suffixed accessors" do + @john.configs[:two_factor_auth] = true + assert @john.two_factor_auth_configs_changed? + assert_nil @john.two_factor_auth_configs_was + assert_equal [nil, true], @john.two_factor_auth_configs_change + end + + test "dirty methods for prefixed accessors" do + @john.spouse[:name] = "Lena" + assert @john.partner_name_changed? + assert_equal "Dallas", @john.partner_name_was + assert_equal ["Dallas", "Lena"], @john.partner_name_change + end + + test "saved changes tracking for accessors" do + @john.spouse[:name] = "Lena" + assert @john.partner_name_changed? + + @john.save! + assert_not @john.partner_name_change + assert @john.saved_change_to_partner_name? + assert_equal ["Dallas", "Lena"], @john.saved_change_to_partner_name + assert_equal "Dallas", @john.partner_name_before_last_save + end + test "object initialization with not nullable column" do assert_equal true, @john.remember_login end diff --git a/activestorage/activestorage.gemspec b/activestorage/activestorage.gemspec index 34029ac8ad..0ae2dcdd3e 100644 --- a/activestorage/activestorage.gemspec +++ b/activestorage/activestorage.gemspec @@ -28,7 +28,8 @@ Gem::Specification.new do |s| # NOTE: Please read our dependency guidelines before updating versions: # https://edgeguides.rubyonrails.org/security.html#dependency-management-and-cves - s.add_dependency "actionpack", version + s.add_dependency "actionpack", version + s.add_dependency "activejob", version s.add_dependency "activerecord", version s.add_dependency "marcel", "~> 0.3.1" diff --git a/activestorage/app/controllers/active_storage/disk_controller.rb b/activestorage/app/controllers/active_storage/disk_controller.rb index df8d73cc91..df3116afd7 100644 --- a/activestorage/app/controllers/active_storage/disk_controller.rb +++ b/activestorage/app/controllers/active_storage/disk_controller.rb @@ -3,7 +3,7 @@ # Serves files stored with the disk service in the same way that the cloud services do. # This means using expiring, signed URLs that are meant for immediate access, not permanent linking. # Always go through the BlobsController, or your own authenticated controller, rather than directly -# to the service url. +# to the service URL. class ActiveStorage::DiskController < ActiveStorage::BaseController skip_forgery_protection diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 384e6ebfa6..fc75a8f816 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true require "rails" +require "action_controller/railtie" +require "active_job/railtie" +require "active_record/railtie" + require "active_storage" require "active_storage/previewer/poppler_pdf_previewer" diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index b7b37bacf5..9fd75a1b4a 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -114,7 +114,7 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase end end - test "urls expiring in 5 minutes" do + test "URLs expiring in 5 minutes" do blob = create_blob freeze_time do @@ -123,7 +123,7 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase end end - test "urls force content_type to binary and attachment as content disposition for content types served as binary" do + test "URLs force content_type to binary and attachment as content disposition for content types served as binary" do blob = create_blob(content_type: "text/html") freeze_time do @@ -132,7 +132,7 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase end end - test "urls force attachment as content disposition when the content type is not allowed inline" do + test "URLs force attachment as content disposition when the content type is not allowed inline" do blob = create_blob(content_type: "application/zip") freeze_time do @@ -141,7 +141,7 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase end end - test "urls allow for custom filename" do + test "URLs allow for custom filename" do blob = create_blob(filename: "original.txt") new_filename = ActiveStorage::Filename.new("new.txt") @@ -153,7 +153,7 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase end end - test "urls allow for custom options" do + test "URLs allow for custom options" do blob = create_blob(filename: "original.txt") arguments = [ diff --git a/activestorage/test/service/disk_service_test.rb b/activestorage/test/service/disk_service_test.rb index a0218bff1c..f3c4dd26bd 100644 --- a/activestorage/test/service/disk_service_test.rb +++ b/activestorage/test/service/disk_service_test.rb @@ -7,7 +7,7 @@ class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase include ActiveStorage::Service::SharedServiceTests - test "url generation" do + test "URL generation" do assert_match(/^https:\/\/example.com\/rails\/active_storage\/disk\/.*\/avatar\.png\?content_type=image%2Fpng&disposition=inline/, @service.url(@key, expires_in: 5.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png")) end diff --git a/activestorage/test/template/image_tag_test.rb b/activestorage/test/template/image_tag_test.rb index f0b166c225..258cf702ad 100644 --- a/activestorage/test/template/image_tag_test.rb +++ b/activestorage/test/template/image_tag_test.rb @@ -37,7 +37,7 @@ class ActiveStorage::ImageTagTest < ActionView::TestCase assert_raises(ArgumentError) { image_tag(@user.avatar) } end - test "error when object can't be resolved into url" do + test "error when object can't be resolved into URL" do unresolvable_object = ActionView::Helpers::AssetTagHelper assert_raises(ArgumentError) { image_tag(unresolvable_object) } end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index f30249cb3f..db9c7e96f3 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,16 +1,25 @@ +* In `:zeitwerk` mode, eager load directories in engines and applications only + if present in their respective `config.eager_load_paths`. + + A common use case for this is adding `lib` to `config.autoload_paths`, but + not to `config.eager_load_paths`. In that configuration, for example, files + in the `lib` directory should not be eager loaded. + + *Xavier Noria* + * Fix bug in Range comparisons when comparing to an excluded-end Range Before: - (1..10).cover?(1...11) => false + (1..10).cover?(1...11) # => false After: - (1..10).cover?(1...11) => true + (1..10).cover?(1...11) # => true With the same change for `Range#include?` and `Range#===`. - *Owen Stephens* + *Owen Stephens* * Use weak references in descendants tracker to allow anonymous subclasses to be garbage collected. @@ -27,11 +36,11 @@ * Fix `Time#advance` to work with dates before 1001-03-07 Before: - + Time.utc(1001, 3, 6).advance(years: -1) # => 1000-03-05 00:00:00 UTC - + After - + Time.utc(1001, 3, 6).advance(years: -1) # => 1000-03-06 00:00:00 UTC Note that this doesn't affect `DateTime#advance` as that doesn't use a proleptic calendar. @@ -46,27 +55,27 @@ I18n.backend.store_translations(:de, i18n: { transliterate: { rule: { "ü" => "ue" } } }) - ActiveSupport::Inflector.transliterate("ü", locale: :de) => "ue" - "Fünf autos".parameterize(locale: :de) => "fuenf-autos" - ActiveSupport::Inflector.parameterize("Fünf autos", locale: :de) => "fuenf-autos" + ActiveSupport::Inflector.transliterate("ü", locale: :de) # => "ue" + "Fünf autos".parameterize(locale: :de) # => "fuenf-autos" + ActiveSupport::Inflector.parameterize("Fünf autos", locale: :de) # => "fuenf-autos" *Kaan Ozkan*, *Sharang Dashputre* -* Allow Array#excluding and Enumerable#excluding to deal with a passed array gracefully. +* Allow `Array#excluding` and `Enumerable#excluding` to deal with a passed array gracefully. - [ 1, 2, 3, 4, 5 ].excluding([4, 5]) => [ 1, 2, 3 ] + [ 1, 2, 3, 4, 5 ].excluding([4, 5]) # => [ 1, 2, 3 ] *DHH* -* Renamed Array#without and Enumerable#without to Array#excluding and Enumerable#excluding, to create parity with - Array#including and Enumerable#including. Retained the old names as aliases. +* Renamed `Array#without` and `Enumerable#without` to `Array#excluding` and `Enumerable#excluding`, to create parity with + `Array#including` and `Enumerable#including`. Retained the old names as aliases. *DHH* -* Added Array#including and Enumerable#including to conveniently enlarge a collection with more members using a method rather than an operator: +* Added `Array#including` and `Enumerable#including` to conveniently enlarge a collection with more members using a method rather than an operator: - [ 1, 2, 3 ].including(4, 5) => [ 1, 2, 3, 4, 5 ] - post.authors.including(Current.person) => All the authors plus the current person! + [ 1, 2, 3 ].including(4, 5) # => [ 1, 2, 3, 4, 5 ] + post.authors.including(Current.person) # => All the authors plus the current person! *DHH* diff --git a/activesupport/lib/active_support/core_ext/array/access.rb b/activesupport/lib/active_support/core_ext/array/access.rb index 10e4c6b09d..ea01e5891c 100644 --- a/activesupport/lib/active_support/core_ext/array/access.rb +++ b/activesupport/lib/active_support/core_ext/array/access.rb @@ -31,16 +31,16 @@ class Array # Returns a new array that includes the passed elements. # - # [ 1, 2, 3 ].including(4, 5) => [ 1, 2, 3, 4, 5 ] - # [ [ 0, 1 ] ].including([ [ 1, 0 ] ]) => [ [ 0, 1 ], [ 1, 0 ] ] + # [ 1, 2, 3 ].including(4, 5) # => [ 1, 2, 3, 4, 5 ] + # [ [ 0, 1 ] ].including([ [ 1, 0 ] ]) # => [ [ 0, 1 ], [ 1, 0 ] ] def including(*elements) self + elements.flatten(1) end # Returns a copy of the Array excluding the specified elements. # - # ["David", "Rafael", "Aaron", "Todd"].excluding("Aaron", "Todd") => ["David", "Rafael"] - # [ [ 0, 1 ], [ 1, 0 ] ].excluding([ [ 1, 0 ] ]) => [ [ 0, 1 ] ] + # ["David", "Rafael", "Aaron", "Todd"].excluding("Aaron", "Todd") # => ["David", "Rafael"] + # [ [ 0, 1 ], [ 1, 0 ] ].excluding([ [ 1, 0 ] ]) # => [ [ 0, 1 ] ] # # Note: This is an optimization of <tt>Enumerable#excluding</tt> that uses <tt>Array#-</tt> # instead of <tt>Array#reject</tt> for performance reasons. diff --git a/activesupport/lib/active_support/core_ext/hash/except.rb b/activesupport/lib/active_support/core_ext/hash/except.rb index 6258610c98..5013812460 100644 --- a/activesupport/lib/active_support/core_ext/hash/except.rb +++ b/activesupport/lib/active_support/core_ext/hash/except.rb @@ -10,7 +10,7 @@ class Hash # This is useful for limiting a set of parameters to everything but a few known toggles: # @person.update(params[:person].except(:admin)) def except(*keys) - dup.except!(*keys) + slice(*self.keys - keys) end # Removes the given keys from hash and returns it. diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index d5d00b5e6e..82f07c085e 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -70,6 +70,11 @@ module ActiveSupport #:nodoc: # only once. All directories in this set must also be present in +autoload_paths+. mattr_accessor :autoload_once_paths, default: [] + # This is a private set that collects all eager load paths during bootstrap. + # Useful for Zeitwerk integration. Its public interface is the config.* path + # accessors of each engine. + mattr_accessor :_eager_load_paths, default: Set.new + # An array of qualified constant names that have been loaded. Adding a name # to this array will cause it to be unloaded the next time Dependencies are # cleared. diff --git a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb index c6fdade006..a43d03cf09 100644 --- a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb +++ b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "set" require "active_support/core_ext/string/inflections" module ActiveSupport @@ -52,7 +53,7 @@ module ActiveSupport class << self def take_over setup_autoloaders - freeze_autoload_paths + freeze_paths decorate_dependencies end @@ -64,11 +65,11 @@ module ActiveSupport # prevent misconfigurations. next unless File.directory?(autoload_path) - if autoload_once?(autoload_path) - Rails.autoloaders.once.push_dir(autoload_path) - else - Rails.autoloaders.main.push_dir(autoload_path) - end + autoloader = \ + autoload_once?(autoload_path) ? Rails.autoloaders.once : Rails.autoloaders.main + + autoloader.push_dir(autoload_path) + autoloader.do_not_eager_load(autoload_path) unless eager_load?(autoload_path) end Rails.autoloaders.each(&:setup) @@ -78,9 +79,14 @@ module ActiveSupport Dependencies.autoload_once_paths.include?(autoload_path) end - def freeze_autoload_paths + def eager_load?(autoload_path) + Dependencies._eager_load_paths.member?(autoload_path) + end + + def freeze_paths Dependencies.autoload_paths.freeze Dependencies.autoload_once_paths.freeze + Dependencies._eager_load_paths.freeze end def decorate_dependencies diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 3a2b2652c4..42ae7e9b7b 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -225,8 +225,8 @@ module ActiveSupport # hash[:a] = 'x' # hash[:b] = 'y' # hash.values_at('a', 'b') # => ["x", "y"] - def values_at(*indices) - indices.collect { |key| self[convert_key(key)] } + def values_at(*keys) + super(*keys.map { |key| convert_key(key) }) end # Returns an array of the values at the specified indices, but also @@ -239,7 +239,7 @@ module ActiveSupport # 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) } + super(*indices.map { |key| convert_key(key) }, &block) end # Returns a shallow copy of the hash. @@ -293,6 +293,9 @@ module ActiveSupport super(convert_key(key)) end + def except(*keys) + slice(*self.keys - keys.map { |key| convert_key(key) }) + end alias_method :without, :except def stringify_keys!; self end diff --git a/guides/rails_guides/markdown/renderer.rb b/guides/rails_guides/markdown/renderer.rb index f186ac526f..7f14c28bbc 100644 --- a/guides/rails_guides/markdown/renderer.rb +++ b/guides/rails_guides/markdown/renderer.rb @@ -16,7 +16,7 @@ HTML end def link(url, title, content) - if url.start_with?("http://api.rubyonrails.org") + if %r{https?://api\.rubyonrails\.org}.match?(url) %(<a href="#{api_link(url)}">#{content}</a>) elsif title %(<a href="#{url}" title="#{title}">#{content}</a>) @@ -115,7 +115,7 @@ HTML end def api_link(url) - if %r{http://api\.rubyonrails\.org/v\d+\.}.match?(url) + if %r{https?://api\.rubyonrails\.org/v\d+\.}.match?(url) url elsif edge url.sub("api", "edgeapi") diff --git a/guides/source/3_1_release_notes.md b/guides/source/3_1_release_notes.md index 9549b0b911..09faeea614 100644 --- a/guides/source/3_1_release_notes.md +++ b/guides/source/3_1_release_notes.md @@ -240,7 +240,7 @@ Action Pack * Added `config.action_controller.include_all_helpers`. By default `helper :all` is done in `ActionController::Base`, which includes all the helpers by default. Setting `include_all_helpers` to `false` will result in including only application_helper and the helper corresponding to controller (like foo_helper for foo_controller). -* `url_for` and named url helpers now accept `:subdomain` and `:domain` as options. +* `url_for` and named URL helpers now accept `:subdomain` and `:domain` as options. * Added `Base.http_basic_authenticate_with` to do simple http basic authentication with a single class method call. @@ -293,7 +293,7 @@ Action Pack You can restrict it to some actions by using `:only` or `:except`. Please read the docs at [`ActionController::Streaming`](https://api.rubyonrails.org/v3.1.0/classes/ActionController/Streaming.html) for more information. -* The redirect route method now also accepts a hash of options which will only change the parts of the url in question, or an object which responds to call, allowing for redirects to be reused. +* The redirect route method now also accepts a hash of options which will only change the parts of the URL in question, or an object which responds to call, allowing for redirects to be reused. ### Action Dispatch diff --git a/guides/source/action_cable_overview.md b/guides/source/action_cable_overview.md index c531b6eee2..f1e2a0081f 100644 --- a/guides/source/action_cable_overview.md +++ b/guides/source/action_cable_overview.md @@ -190,6 +190,23 @@ This will ready a consumer that'll connect against `/cable` on your server by de The connection won't be established until you've also specified at least one subscription you're interested in having. +The consumer can optionally take an argument that specifies the URL to connect to. This +can be a string, or a function that returns a string that will be called when the +WebSocket is opened. + +```js +// Specify a different URL to connect to +createConsumer('https://ws.example.com/cable') + +// Use a function to dynamically generate the URL +createConsumer(getWebSocketURL) + +function getWebSocketURL { + const token = localStorage.get('auth-token') + return `https://ws.example.com/cable?token=${token}` +} +``` + #### Subscriber A consumer becomes a subscriber by creating a subscription to a given channel: diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md index df74b4ebd0..f600cf29ce 100644 --- a/guides/source/action_mailer_basics.md +++ b/guides/source/action_mailer_basics.md @@ -573,7 +573,7 @@ web addresses. Thus, you should always use the "_url" variant of named route helpers. If you did not configure the `:host` option globally make sure to pass it to the -url helper. +URL helper. ```erb <%= user_url(@user, host: 'example.com') %> diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index 50d4a4c57d..270e4a3bf9 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -465,7 +465,6 @@ number of digits after the decimal point. * `default` Allows to set a default value on the column. Note that if you are using a dynamic value (such as a date), the default will only be calculated the first time (i.e. on the date the migration is applied). -* `index` Adds an index for the column. * `comment` Adds a comment for the column. Some adapters may support additional options; see the adapter specific API docs diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index cc6e08aaec..e40f16e62d 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -65,6 +65,7 @@ The methods are: * `distinct` * `eager_load` * `extending` +* `extract_associated` * `from` * `group` * `having` diff --git a/guides/source/active_support_instrumentation.md b/guides/source/active_support_instrumentation.md index d7dbc5cea8..e5ed283c45 100644 --- a/guides/source/active_support_instrumentation.md +++ b/guides/source/active_support_instrumentation.md @@ -580,7 +580,7 @@ Active Storage | ------------ | ------------------- | | `:key` | Secure token | | `:service` | Name of the service | -| `:url` | Generated url | +| `:url` | Generated URL | Railties -------- diff --git a/guides/source/command_line.md b/guides/source/command_line.md index ddb30a3aec..a83724f1bb 100644 --- a/guides/source/command_line.md +++ b/guides/source/command_line.md @@ -344,7 +344,7 @@ irb(main):001:0> Inside the `rails console` you have access to the `app` and `helper` instances. -With the `app` method you can access url and path helpers, as well as do requests. +With the `app` method you can access URL and path helpers, as well as do requests. ```bash >> app.root_path diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 86e2dd284e..e1c9fad232 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1162,7 +1162,7 @@ Imagine you have a server which mirrors the production environment but is only u That environment is no different than the default ones, start a server with `rails server -e staging`, a console with `rails console -e staging`, `Rails.env.staging?` works, etc. -### Deploy to a subdirectory (relative url root) +### Deploy to a subdirectory (relative URL root) By default Rails expects that your application is running at the root (eg. `/`). This section explains how to run your application inside a directory. diff --git a/guides/source/form_helpers.md b/guides/source/form_helpers.md index b6674ae7dd..6418005921 100644 --- a/guides/source/form_helpers.md +++ b/guides/source/form_helpers.md @@ -248,7 +248,7 @@ There are a few things to note here: * `@article` is the actual object being edited. * There is a single hash of options. HTML options (except `id` and `class`) are passed in the `:html` hash. Also you can provide a `:namespace` option for your form to ensure uniqueness of id attributes on form elements. The scope attribute will be prefixed with underscore on the generated HTML id. * The `form_with` method yields a **form builder** object (the `f` variable). -* If you wish to direct your form request to a particular url, you would use `form_with url: my_nifty_url_path` instead. To see more in depth options on what `form_with` accepts be sure to [check out the API documentation](https://api.rubyonrails.org/classes/ActionView/Helpers/FormHelper.html#method-i-form_with). +* If you wish to direct your form request to a particular URL, you would use `form_with url: my_nifty_url_path` instead. To see more in depth options on what `form_with` accepts be sure to [check out the API documentation](https://api.rubyonrails.org/classes/ActionView/Helpers/FormHelper.html#method-i-form_with). * Methods to create form controls are called **on** the form builder object `f`. The resulting HTML is: diff --git a/guides/source/rails_on_rack.md b/guides/source/rails_on_rack.md index 69b5f254bf..d60e53b052 100644 --- a/guides/source/rails_on_rack.md +++ b/guides/source/rails_on_rack.md @@ -13,7 +13,7 @@ After reading this guide, you will know: -------------------------------------------------------------------------------- -WARNING: This guide assumes a working knowledge of Rack protocol and Rack concepts such as middlewares, url maps, and `Rack::Builder`. +WARNING: This guide assumes a working knowledge of Rack protocol and Rack concepts such as middlewares, URL maps, and `Rack::Builder`. Introduction to Rack -------------------- diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index df0d6d4fa0..c5136b7ab0 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,14 @@ +* The `connection` option of `rails dbconsole` command is deprecated in + favor of `database` option. + + *Yuji Yaginuma* + +* Replace `chromedriver-helper` gem with `webdrivers` in default Gemfile. + `chromedriver-helper` is deprecated as of March 31, 2019 and won't + receive any further updates. + + *Guillermo Iguaran* + * Applications running in `:zeitwerk` mode that use `bootsnap` need to upgrade `bootsnap` to at least 1.4.2. @@ -6,7 +17,7 @@ * Add `config.disable_sandbox` option to Rails console. This setting will disable `rails console --sandbox` mode, preventing - developer from accidentally starting a sandbox console, + developer from accidentally starting a sandbox console, which when left inactive, can cause the database server to run out of memory. *Prem Sichanugrist* @@ -15,6 +26,7 @@ *Yuji Yaginuma* + ## Rails 6.0.0.beta3 (March 11, 2019) ## * Generate random development secrets @@ -29,7 +41,6 @@ *Eileen M. Uchitelle*, *Aaron Patterson*, *John Hawthorn* - ## Rails 6.0.0.beta2 (February 25, 2019) ## * Fix non-symbol access to nested hashes returned from `Rails::Application.config_for` diff --git a/railties/lib/rails/commands/dbconsole/dbconsole_command.rb b/railties/lib/rails/commands/dbconsole/dbconsole_command.rb index 0fac7d34a0..72f3235ce3 100644 --- a/railties/lib/rails/commands/dbconsole/dbconsole_command.rb +++ b/railties/lib/rails/commands/dbconsole/dbconsole_command.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/deprecation" +require "active_support/core_ext/string/filters" require "rails/command/environment_argument" module Rails @@ -89,15 +91,15 @@ module Rails def config @config ||= begin - # We need to check whether the user passed the connection the + # We need to check whether the user passed the database the # first time around to show a consistent error message to people # relying on 2-level database configuration. - if @options["connection"] && configurations[connection].blank? - raise ActiveRecord::AdapterNotSpecified, "'#{connection}' connection is not configured. Available configuration: #{configurations.inspect}" - elsif configurations[environment].blank? && configurations[connection].blank? + if @options["database"] && configurations[database].blank? + raise ActiveRecord::AdapterNotSpecified, "'#{database}' database is not configured. Available configuration: #{configurations.inspect}" + elsif configurations[environment].blank? && configurations[database].blank? raise ActiveRecord::AdapterNotSpecified, "'#{environment}' database is not configured. Available configuration: #{configurations.inspect}" else - configurations[connection] || configurations[environment].presence + configurations[database] || configurations[environment].presence end end end @@ -106,8 +108,8 @@ module Rails Rails.respond_to?(:env) ? Rails.env : Rails::Command.environment end - def connection - @options.fetch(:connection, "primary") + def database + @options.fetch(:database, "primary") end private @@ -156,12 +158,22 @@ module Rails class_option :connection, aliases: "-c", type: :string, desc: "Specifies the connection to use." + class_option :database, aliases: "--db", type: :string, + desc: "Specifies the database to use." + def perform extract_environment_option_from_argument # RAILS_ENV needs to be set before config/application is required. ENV["RAILS_ENV"] = options[:environment] + if options["connection"] + ActiveSupport::Deprecation.warn(<<-MSG.squish) + `connection` option is deprecated and will be removed in Rails 6.1. Please use `database` option instead. + MSG + options["database"] = options["connection"] + end + require_application_and_environment! Rails::DBConsole.start(options) end diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 07bd56c978..778bbebe75 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -230,7 +230,7 @@ module Rails # # If +MyEngine+ is isolated, The routes above will point to # <tt>MyEngine::ArticlesController</tt>. You also don't need to use longer - # url helpers like +my_engine_articles_path+. Instead, you should simply use + # URL helpers like +my_engine_articles_path+. Instead, you should simply use # +articles_path+, like you would do with your main application. # # To make this behavior consistent with other parts of the framework, @@ -238,7 +238,7 @@ module Rails # normal Rails app, when you use a namespaced model such as # <tt>Namespace::Article</tt>, <tt>ActiveModel::Naming</tt> will generate # names with the prefix "namespace". In an isolated engine, the prefix will - # be omitted in url helpers and form fields, for convenience. + # be omitted in URL helpers and form fields, for convenience. # # polymorphic_url(MyEngine::Article.new) # # => "articles_path" # not "my_engine_articles_path" @@ -286,11 +286,11 @@ module Rails # Note that the <tt>:as</tt> option given to mount takes the <tt>engine_name</tt> as default, so most of the time # you can simply omit it. # - # Finally, if you want to generate a url to an engine's route using + # Finally, if you want to generate a URL to an engine's route using # <tt>polymorphic_url</tt>, you also need to pass the engine helper. Let's # say that you want to create a form pointing to one of the engine's routes. # All you need to do is pass the helper as the first element in array with - # attributes for url: + # attributes for URL: # # form_for([my_engine, @user]) # @@ -469,13 +469,16 @@ module Rails self end - # Eager load the application by loading all ruby - # files inside eager_load paths. def eager_load! - if Rails.autoloaders.zeitwerk_enabled? - eager_load_with_zeitwerk! - else - eager_load_with_dependencies! + # Already done by Zeitwerk::Loader.eager_load_all in the finisher. + return if Rails.autoloaders.zeitwerk_enabled? + + config.eager_load_paths.each do |load_path| + # Starts after load_path plus a slash, ends before ".rb". + relname_range = (load_path.to_s.length + 1)...-3 + Dir.glob("#{load_path}/**/*.rb").sort.each do |file| + require_dependency file[relname_range] + end end end @@ -567,12 +570,15 @@ module Rails ActiveSupport::Dependencies.autoload_paths.unshift(*_all_autoload_paths) ActiveSupport::Dependencies.autoload_once_paths.unshift(*_all_autoload_once_paths) - # Freeze so future modifications will fail rather than do nothing mysteriously config.autoload_paths.freeze - config.eager_load_paths.freeze config.autoload_once_paths.freeze end + initializer :set_eager_load_paths, before: :bootstrap_hook do + ActiveSupport::Dependencies._eager_load_paths.merge(config.eager_load_paths) + config.eager_load_paths.freeze + end + initializer :add_routing_paths do |app| routing_paths = paths["config/routes.rb"].existent @@ -651,22 +657,6 @@ module Rails private - def eager_load_with_zeitwerk! - (config.eager_load_paths - Zeitwerk::Loader.all_dirs).each do |path| - Dir.glob("#{path}/**/*.rb").sort.each { |file| require file } - end - end - - def eager_load_with_dependencies! - config.eager_load_paths.each do |load_path| - # Starts after load_path plus a slash, ends before ".rb". - relname_range = (load_path.to_s.length + 1)...-3 - Dir.glob("#{load_path}/**/*.rb").sort.each do |file| - require_dependency file[relname_range] - end - end - end - def load_config_initializer(initializer) # :doc: ActiveSupport::Notifications.instrument("load_config_initializer.railties", initializer: initializer) do load(initializer) diff --git a/railties/lib/rails/generators/rails/app/templates/ruby-version.tt b/railties/lib/rails/generators/rails/app/templates/ruby-version.tt index bac1339923..096cfd36a8 100644 --- a/railties/lib/rails/generators/rails/app/templates/ruby-version.tt +++ b/railties/lib/rails/generators/rails/app/templates/ruby-version.tt @@ -1 +1 @@ -<%= ENV["RBENV_VERSION"] || ENV["rvm_ruby_string"] || "#{RUBY_ENGINE}-#{RUBY_ENGINE_VERSION}" -%> +<%= ENV["RBENV_VERSION"] || ENV["rvm_ruby_string"] || "#{RUBY_ENGINE}-#{RUBY_ENGINE_VERSION}" %> diff --git a/railties/test/application/asset_debugging_test.rb b/railties/test/application/asset_debugging_test.rb index 3e0f31860b..7623e8e352 100644 --- a/railties/test/application/asset_debugging_test.rb +++ b/railties/test/application/asset_debugging_test.rb @@ -95,7 +95,7 @@ module ApplicationTests end end - test "public url methods are not over-written by the asset pipeline" do + test "public URL methods are not over-written by the asset pipeline" do contents = "doesnotexist" cases = { asset_url: %r{http://example.org/#{contents}}, diff --git a/railties/test/application/assets_test.rb b/railties/test/application/assets_test.rb index 46ee0d670e..a80581211b 100644 --- a/railties/test/application/assets_test.rb +++ b/railties/test/application/assets_test.rb @@ -450,7 +450,7 @@ module ApplicationTests assert_equal 0, files.length, "Expected application.js asset to be removed, but still exists" end - test "asset urls should use the request's protocol by default" do + test "asset URLs should use the request's protocol by default" do app_with_assets_in_view add_to_config "config.asset_host = 'example.com'" add_to_env_config "development", "config.assets.digest = false" @@ -466,7 +466,7 @@ module ApplicationTests assert_match('src="https://example.com/assets/application.self.js', last_response.body) end - test "asset urls should be protocol-relative if no request is in scope" do + test "asset URLs should be protocol-relative if no request is in scope" do app_file "app/assets/images/rails.png", "notreallyapng" app_file "app/assets/javascripts/image_loader.js.erb", "var src='<%= image_path('rails.png') %>';" add_to_config "config.assets.precompile = %w{rails.png image_loader.js}" diff --git a/railties/test/application/initializers/frameworks_test.rb b/railties/test/application/initializers/frameworks_test.rb index 3cd4b8fe33..a35247fc43 100644 --- a/railties/test/application/initializers/frameworks_test.rb +++ b/railties/test/application/initializers/frameworks_test.rb @@ -39,7 +39,7 @@ module ApplicationTests assert_equal expanded_path, ActionMailer::Base.view_paths[0].to_s end - test "allows me to configure default url options for ActionMailer" do + test "allows me to configure default URL options for ActionMailer" do app_file "config/environments/development.rb", <<-RUBY Rails.application.configure do config.action_mailer.default_url_options = { :host => "test.rails" } @@ -61,7 +61,7 @@ module ApplicationTests assert_equal "https", ActionMailer::Base.default_url_options[:protocol] end - test "includes url helpers as action methods" do + test "includes URL helpers as action methods" do app_file "config/routes.rb", <<-RUBY Rails.application.routes.draw do get "/foo", :to => lambda { |env| [200, {}, []] }, :as => :foo diff --git a/railties/test/application/middleware/exceptions_test.rb b/railties/test/application/middleware/exceptions_test.rb index 2d659ade8d..17df78ed4e 100644 --- a/railties/test/application/middleware/exceptions_test.rb +++ b/railties/test/application/middleware/exceptions_test.rb @@ -60,7 +60,7 @@ module ApplicationTests assert_equal "YOU FAILED", last_response.body end - test "url generation error when action_dispatch.show_exceptions is set raises an exception" do + test "URL generation error when action_dispatch.show_exceptions is set raises an exception" do controller :foo, <<-RUBY class FooController < ActionController::Base def index diff --git a/railties/test/application/rake/dbs_test.rb b/railties/test/application/rake/dbs_test.rb index a1e237fa7b..258066a7e6 100644 --- a/railties/test/application/rake/dbs_test.rb +++ b/railties/test/application/rake/dbs_test.rb @@ -40,12 +40,12 @@ module ApplicationTests end end - test "db:create and db:drop without database url" do + test "db:create and db:drop without database URL" do require "#{app_path}/config/environment" db_create_and_drop ActiveRecord::Base.configurations[Rails.env]["database"] end - test "db:create and db:drop with database url" do + test "db:create and db:drop with database URL" do require "#{app_path}/config/environment" set_database_url db_create_and_drop database_url_db_name @@ -553,6 +553,22 @@ module ApplicationTests end end end + + test "db:prepare setup the database" do + Dir.chdir(app_path) do + rails "generate", "model", "book", "title:string" + output = rails("db:prepare") + assert_match(/CreateBooks: migrated/, output) + + output = rails("db:drop") + assert_match(/Dropped database/, output) + + rails "generate", "model", "recipe", "title:string" + output = rails("db:prepare") + assert_match(/CreateBooks: migrated/, output) + assert_match(/CreateRecipes: migrated/, output) + end + end end end end diff --git a/railties/test/application/rake/multi_dbs_test.rb b/railties/test/application/rake/multi_dbs_test.rb index d676e7486e..147b8f94e1 100644 --- a/railties/test/application/rake/multi_dbs_test.rb +++ b/railties/test/application/rake/multi_dbs_test.rb @@ -137,6 +137,21 @@ module ApplicationTests end end + def db_prepare + Dir.chdir(app_path) do + generate_models_for_animals + output = rails("db:prepare") + + ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config| + if db_config.spec_name == "primary" + assert_match(/CreateBooks: migrated/, output) + else + assert_match(/CreateDogs: migrated/, output) + end + end + end + end + def write_models_for_animals # make a directory for the animals migration FileUtils.mkdir_p("#{app_path}/db/animals_migrate") @@ -226,6 +241,11 @@ module ApplicationTests require "#{app_path}/config/environment" db_migrate_and_schema_cache_dump_and_schema_cache_clear end + + test "db:prepare works on all databases" do + require "#{app_path}/config/environment" + db_prepare + end end end end diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb index c82b37d07d..5d2e34433a 100644 --- a/railties/test/application/zeitwerk_integration_test.rb +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -124,12 +124,26 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase app_file "app/models/user.rb", "class User; end; $zeitwerk_integration_test_user = true" app_file "app/models/post.rb", "class Post; end; $zeitwerk_integration_test_post = true" + boot("production") assert $zeitwerk_integration_test_user assert $zeitwerk_integration_test_post end + test "eager loading loads code in engines" do + $test_blog_engine_eager_loaded = false + + engine("blog") do |bukkit| + bukkit.write("lib/blog.rb", "class BlogEngine < Rails::Engine; end") + bukkit.write("app/models/post.rb", "Post = $test_blog_engine_eager_loaded = true") + end + + boot("production") + + assert $test_blog_engine_eager_loaded + end + test "eager loading loads anything managed by Zeitwerk" do $zeitwerk_integration_test_user = false app_file "app/models/user.rb", "class User; end; $zeitwerk_integration_test_user = true" @@ -149,6 +163,34 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase assert $zeitwerk_integration_test_extras end + test "autoload directories not present in eager load paths are not eager loaded" do + $zeitwerk_integration_test_user = false + app_file "app/models/user.rb", "class User; end; $zeitwerk_integration_test_user = true" + + $zeitwerk_integration_test_lib = false + app_dir "lib" + app_file "lib/webhook_hacks.rb", "WebhookHacks = 1; $zeitwerk_integration_test_lib = true" + + $zeitwerk_integration_test_extras = false + app_dir "extras" + app_file "extras/websocket_hacks.rb", "WebsocketHacks = 1; $zeitwerk_integration_test_extras = true" + + add_to_config "config.autoload_paths << '#{app_path}/lib'" + add_to_config "config.autoload_once_paths << '#{app_path}/extras'" + + boot("production") + + assert $zeitwerk_integration_test_user + assert !$zeitwerk_integration_test_lib + assert !$zeitwerk_integration_test_extras + + assert WebhookHacks + assert WebsocketHacks + + assert $zeitwerk_integration_test_lib + assert $zeitwerk_integration_test_extras + end + test "autoload_paths are set as root dirs of main, and in the same order" do boot diff --git a/railties/test/commands/dbconsole_test.rb b/railties/test/commands/dbconsole_test.rb index 65f6916acb..76a7cd055f 100644 --- a/railties/test/commands/dbconsole_test.rb +++ b/railties/test/commands/dbconsole_test.rb @@ -216,22 +216,22 @@ class Rails::DBConsoleTest < ActiveSupport::TestCase end end - def test_specifying_a_custom_connection_and_environment + def test_specifying_a_custom_database_and_environment stub_available_environments(["development"]) do - dbconsole = parse_arguments(["-c", "custom", "-e", "development"]) + dbconsole = parse_arguments(["--db", "custom", "-e", "development"]) assert_equal "development", dbconsole[:environment] - assert_equal "custom", dbconsole.connection + assert_equal "custom", dbconsole.database end end - def test_specifying_a_missing_connection + def test_specifying_a_missing_database app_db_config({}) do e = assert_raises(ActiveRecord::AdapterNotSpecified) do - Rails::Command.invoke(:dbconsole, ["-c", "i_do_not_exist"]) + Rails::Command.invoke(:dbconsole, ["--db", "i_do_not_exist"]) end - assert_includes e.message, "'i_do_not_exist' connection is not configured." + assert_includes e.message, "'i_do_not_exist' database is not configured." end end @@ -245,6 +245,18 @@ class Rails::DBConsoleTest < ActiveSupport::TestCase end end + def test_connection_options_is_deprecate + command = Rails::Command::DbconsoleCommand.new([], ["-c", "custom"]) + Rails::DBConsole.stub(:start, nil) do + assert_deprecated("`connection` option is deprecated") do + command.perform + end + end + + assert_equal "custom", command.options["connection"] + assert_equal "custom", command.options["database"] + end + def test_print_help_short stdout = capture(:stdout) do Rails::Command.invoke(:dbconsole, ["-h"]) diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index d5a3599f67..d30cd9e718 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -968,6 +968,8 @@ class AppGeneratorTest < Rails::Generators::TestCase else assert_match(/#{RUBY_ENGINE}-#{RUBY_ENGINE_VERSION}/, content) end + + assert content.end_with?("\n"), "expected .ruby-version to end with newline" end end |