diff options
97 files changed, 1150 insertions, 268 deletions
diff --git a/.codeclimate.yml b/.codeclimate.yml index ab4fcd123d..7114a98266 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -1,3 +1,5 @@ +version: "2" + checks: argument-count: enabled: false @@ -20,13 +22,9 @@ checks: identical-code: enabled: false -engines: +plugins: rubocop: enabled: true - channel: rubocop-0-66 - -ratings: - paths: - - "**.rb" + channel: rubocop-0-67 -exclude_paths: [] +exclude_patterns: [] diff --git a/.rubocop.yml b/.rubocop.yml index 3dbd4a27a6..0cfe5d5d84 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,3 +1,5 @@ +require: rubocop-performance + AllCops: TargetRubyVersion: 2.5 # RuboCop has a bunch of cops enabled by default. This setting tells RuboCop @@ -29,6 +29,7 @@ gem "uglifier", ">= 1.3.0", require: false gem "json", ">= 2.0.0" gem "rubocop", ">= 0.47", require: false +gem "rubocop-performance", require: false group :doc do gem "sdoc", "~> 1.0" diff --git a/Gemfile.lock b/Gemfile.lock index b92603d799..784774577e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -385,7 +385,7 @@ GEM rails-html-sanitizer (1.0.4) loofah (~> 2.2, >= 2.2.2) rainbow (3.0.0) - rake (12.3.1) + rake (12.3.2) rb-fsevent (0.10.3) rb-inotify (0.10.0) ffi (~> 1.0) @@ -411,7 +411,7 @@ GEM resque (~> 1.26) rufus-scheduler (~> 3.2) retriable (3.1.2) - rubocop (0.66.0) + rubocop (0.67.2) jaro_winkler (~> 1.5.1) parallel (~> 1.10) parser (>= 2.5, != 2.5.1.1) @@ -419,6 +419,8 @@ GEM rainbow (>= 2.2.2, < 4.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 1.6) + rubocop-performance (1.1.0) + rubocop (>= 0.67.0) ruby-progressbar (1.10.0) ruby-vips (2.0.13) ffi (~> 1.9) @@ -581,6 +583,7 @@ DEPENDENCIES resque resque-scheduler rubocop (>= 0.47) + rubocop-performance sass-rails sdoc (~> 1.0) selenium-webdriver (>= 3.5.0, < 3.13.0) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 79f6320a04..4109ae7006 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,12 @@ +* Introduce `ActionDispatch::ActionableExceptions`. + + The `ActionDispatch::ActionableExceptions` middleware dispatches actions + from `ActiveSupport::ActionableError` descendants. + + Actionable errors let's you dispatch actions from Rails' error pages. + + *Vipul A M*, *Yao Jie*, *Genadi Samokovarov* + * Raise an `ArgumentError` if a resource custom param contains a colon (`:`). After this change it's not possible anymore to configure routes like this: diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 8f39b88d56..6a4ba9af4a 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -53,6 +53,7 @@ module ActionDispatch autoload :RequestId autoload :Callbacks autoload :Cookies + autoload :ActionableExceptions autoload :DebugExceptions autoload :DebugLocks autoload :DebugView diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index a968df5f19..dee2980eb1 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -119,7 +119,8 @@ module ActionDispatch class UnanchoredRegexp < AnchoredRegexp # :nodoc: def accept(node) - %r{\A#{visit node}(?:\b|\Z)} + path = visit node + path == "/" ? %r{\A/} : %r{\A#{path}(?:\b|\Z|/)} end end diff --git a/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb b/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb new file mode 100644 index 0000000000..e94cc46603 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require "erb" +require "action_dispatch/http/request" +require "active_support/actionable_error" + +module ActionDispatch + class ActionableExceptions # :nodoc: + cattr_accessor :endpoint, default: "/rails/actions" + + def initialize(app) + @app = app + end + + def call(env) + request = ActionDispatch::Request.new(env) + return @app.call(env) unless actionable_request?(request) + + ActiveSupport::ActionableError.dispatch(request.params[:error].to_s.safe_constantize, request.params[:action]) + + redirect_to request.params[:location] + end + + private + def actionable_request?(request) + request.show_exceptions? && request.post? && request.path == endpoint + end + + def redirect_to(location) + body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(location)}\">redirected</a>.</body></html>" + + [302, { + "Content-Type" => "text/html; charset=#{Response.default_charset}", + "Content-Length" => body.bytesize.to_s, + "Location" => location, + }, [body]] + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 59113e13f4..0b15c94122 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -4,6 +4,8 @@ require "action_dispatch/http/request" require "action_dispatch/middleware/exception_wrapper" require "action_dispatch/routing/inspector" +require "active_support/actionable_error" + require "action_view" require "action_view/base" diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb index 43c0a84504..a03650254e 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_view.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb @@ -52,5 +52,9 @@ module ActionDispatch super end end + + def protect_against_forgery? + false + end end end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.html.erb new file mode 100644 index 0000000000..b6c6d2f50d --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.html.erb @@ -0,0 +1,13 @@ +<% actions = ActiveSupport::ActionableError.actions(exception) %> + +<% if actions.any? %> + <div class="actions"> + <% actions.each do |action, _| %> + <%= button_to action, ActionDispatch::ActionableExceptions.endpoint, params: { + error: exception.class.name, + action: action, + location: request.path + } %> + <% end %> + </div> +<% end %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.text.erb new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.text.erb diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb index bde26f46c2..999e84e4d6 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb @@ -8,7 +8,11 @@ </header> <div id="container"> - <h2><%= h @exception.message %></h2> + <h2> + <%= h @exception.message %> + + <%= render "rescues/actions", exception: @exception, request: @request %> + </h2> <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx, error_index: 0 %> <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show, error_index: 0 %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index 39ea25bdfc..0f78e23b7f 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -117,6 +117,10 @@ background-color: #FFCCCC; } + .button_to { + display: inline-block; + } + .hidden { display: none; } diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 6460ca6f8d..32a0b8efeb 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -96,6 +96,7 @@ class ActionDispatch::IntegrationTest < ActiveSupport::TestCase RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware| middleware.use ActionDispatch::ShowExceptions, ActionDispatch::PublicExceptions.new("#{FIXTURE_LOAD_PATH}/public") middleware.use ActionDispatch::DebugExceptions + middleware.use ActionDispatch::ActionableExceptions middleware.use ActionDispatch::Callbacks middleware.use ActionDispatch::Cookies middleware.use ActionDispatch::Flash diff --git a/actionpack/test/dispatch/actionable_exceptions_test.rb b/actionpack/test/dispatch/actionable_exceptions_test.rb new file mode 100644 index 0000000000..9215a91e9c --- /dev/null +++ b/actionpack/test/dispatch/actionable_exceptions_test.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require "abstract_unit" + +class ActionableExceptionsTest < ActionDispatch::IntegrationTest + Actions = [] + + class ActionError < StandardError + include ActiveSupport::ActionableError + + action "Successful action" do + Actions << "Action!" + end + + action "Failed action" do + raise "Inaction!" + end + end + + Noop = -> env { [200, {}, [""]] } + + setup do + @app = ActionDispatch::ActionableExceptions.new(Noop) + + Actions.clear + end + + test "dispatches an actionable error" do + post ActionDispatch::ActionableExceptions.endpoint, params: { + error: ActionError.name, + action: "Successful action", + location: "/", + } + + assert_equal ["Action!"], Actions + + assert_equal 302, response.status + assert_equal "/", response.headers["Location"] + end + + test "cannot dispatch errors if not allowed" do + post ActionDispatch::ActionableExceptions.endpoint, params: { + error: ActionError.name, + action: "Successful action", + location: "/", + }, headers: { "action_dispatch.show_exceptions" => false } + + assert_empty Actions + end + + test "dispatched action can fail" do + assert_raise RuntimeError do + post ActionDispatch::ActionableExceptions.endpoint, params: { + error: ActionError.name, + action: "Failed action", + location: "/", + } + end + end + + test "cannot dispatch non-actionable errors" do + assert_raise ActiveSupport::ActionableError::NonActionable do + post ActionDispatch::ActionableExceptions.endpoint, params: { + error: RuntimeError.name, + action: "Inexistent action", + location: "/", + } + end + end + + test "cannot dispatch Inexistent errors" do + assert_raise ActiveSupport::ActionableError::NonActionable do + post ActionDispatch::ActionableExceptions.endpoint, params: { + error: "", + action: "Inexistent action", + location: "/", + } + end + end +end diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 2812b1b614..5ae8a20ae4 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -5,6 +5,18 @@ require "abstract_unit" class DebugExceptionsTest < ActionDispatch::IntegrationTest InterceptedErrorInstance = StandardError.new + class CustomActionableError < StandardError + include ActiveSupport::ActionableError + + action "Action 1" do + nil + end + + action "Action 2" do + nil + end + end + class Boomer attr_accessor :closed @@ -92,6 +104,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest method_that_raises when "/nested_exceptions" raise_nested_exceptions + when %r{/actionable_error} + raise CustomActionableError else raise "puke!" end @@ -589,4 +603,21 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end end end + + test "shows a buttons for every action in an actionable error" do + @app = DevelopmentApp + Rails.stub :root, Pathname.new(".") do + cleaner = ActiveSupport::BacktraceCleaner.new.tap do |bc| + bc.add_silencer { |line| line !~ %r{test/dispatch/debug_exceptions_test.rb} } + end + + get "/actionable_error", headers: { "action_dispatch.backtrace_cleaner" => cleaner } + + # Assert correct error + assert_response 500 + + assert_select 'input[value="Action 1"]' + assert_select 'input[value="Action 2"]' + end + end end diff --git a/actionpack/test/dispatch/mount_test.rb b/actionpack/test/dispatch/mount_test.rb index e42ea89f6f..758cee9930 100644 --- a/actionpack/test/dispatch/mount_test.rb +++ b/actionpack/test/dispatch/mount_test.rb @@ -27,6 +27,7 @@ class TestRoutingMount < ActionDispatch::IntegrationTest } mount SprocketsApp, at: "/sprockets" + mount SprocketsApp, at: "/star*" mount SprocketsApp => "/shorthand" mount SinatraLikeApp, at: "/fakeengine", as: :fake @@ -58,6 +59,14 @@ class TestRoutingMount < ActionDispatch::IntegrationTest def test_mounting_at_root_path get "/omg" assert_equal " -- /omg", response.body + + get "/~omg" + assert_equal " -- /~omg", response.body + end + + def test_mounting_at_path_with_non_word_character + get "/star*/omg" + assert_equal "/star* -- /omg", response.body end def test_mounting_sets_script_name diff --git a/actionpack/test/journey/path/pattern_test.rb b/actionpack/test/journey/path/pattern_test.rb index 2f39abcb92..77c19369b0 100644 --- a/actionpack/test/journey/path/pattern_test.rb +++ b/actionpack/test/journey/path/pattern_test.rb @@ -34,17 +34,17 @@ module ActionDispatch end { - "/:controller(/:action)" => %r{\A/(#{x})(?:/([^/.?]+))?(?:\b|\Z)}, - "/:controller/foo" => %r{\A/(#{x})/foo(?:\b|\Z)}, - "/:controller/:action" => %r{\A/(#{x})/([^/.?]+)(?:\b|\Z)}, - "/:controller" => %r{\A/(#{x})(?:\b|\Z)}, - "/:controller(/:action(/:id))" => %r{\A/(#{x})(?:/([^/.?]+)(?:/([^/.?]+))?)?(?:\b|\Z)}, - "/:controller/:action.xml" => %r{\A/(#{x})/([^/.?]+)\.xml(?:\b|\Z)}, - "/:controller.:format" => %r{\A/(#{x})\.([^/.?]+)(?:\b|\Z)}, - "/:controller(.:format)" => %r{\A/(#{x})(?:\.([^/.?]+))?(?:\b|\Z)}, - "/:controller/*foo" => %r{\A/(#{x})/(.+)(?:\b|\Z)}, - "/:controller/*foo/bar" => %r{\A/(#{x})/(.+)/bar(?:\b|\Z)}, - "/:foo|*bar" => %r{\A/(?:([^/.?]+)|(.+))(?:\b|\Z)}, + "/:controller(/:action)" => %r{\A/(#{x})(?:/([^/.?]+))?(?:\b|\Z|/)}, + "/:controller/foo" => %r{\A/(#{x})/foo(?:\b|\Z|/)}, + "/:controller/:action" => %r{\A/(#{x})/([^/.?]+)(?:\b|\Z|/)}, + "/:controller" => %r{\A/(#{x})(?:\b|\Z|/)}, + "/:controller(/:action(/:id))" => %r{\A/(#{x})(?:/([^/.?]+)(?:/([^/.?]+))?)?(?:\b|\Z|/)}, + "/:controller/:action.xml" => %r{\A/(#{x})/([^/.?]+)\.xml(?:\b|\Z|/)}, + "/:controller.:format" => %r{\A/(#{x})\.([^/.?]+)(?:\b|\Z|/)}, + "/:controller(.:format)" => %r{\A/(#{x})(?:\.([^/.?]+))?(?:\b|\Z|/)}, + "/:controller/*foo" => %r{\A/(#{x})/(.+)(?:\b|\Z|/)}, + "/:controller/*foo/bar" => %r{\A/(#{x})/(.+)/bar(?:\b|\Z|/)}, + "/:foo|*bar" => %r{\A/(?:([^/.?]+)|(.+))(?:\b|\Z|/)}, }.each do |path, expected| define_method(:"test_to_non_anchored_regexp_#{Regexp.escape(path)}") do path = Pattern.build( diff --git a/actiontext/lib/templates/installer.rb b/actiontext/lib/templates/installer.rb index a8000eb9fc..a15ada92bb 100644 --- a/actiontext/lib/templates/installer.rb +++ b/actiontext/lib/templates/installer.rb @@ -29,4 +29,17 @@ if APPLICATION_PACK_PATH.exist? append_to_file APPLICATION_PACK_PATH, "\n#{line}" end end +else + warn <<~WARNING + WARNING: Action Text can't locate your JavaScript bundle to add its package dependencies. + + Add these lines to any bundles: + + require("trix") + require("@rails/actiontext") + + Alternatively, install and setup the webpacker gem then rerun `bin/rails action_text:install` + to have these dependencies added automatically. + + WARNING end diff --git a/actionview/lib/action_view.rb b/actionview/lib/action_view.rb index 5ee14bfc78..7f85bf2a5e 100644 --- a/actionview/lib/action_view.rb +++ b/actionview/lib/action_view.rb @@ -44,6 +44,7 @@ module ActionView autoload :Rendering autoload :RoutingUrlFor autoload :Template + autoload :UnboundTemplate autoload :ViewPaths autoload_under "renderer" do diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index e291dc268a..ce53eb046d 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -118,7 +118,7 @@ module ActionView locals = locals.map(&:to_s).sort!.freeze cached(key, [name, prefix, partial], details, locals) do - find_templates(name, prefix, partial, details, locals) + _find_all(name, prefix, partial, details, key, locals) end end @@ -131,6 +131,10 @@ module ActionView private + def _find_all(name, prefix, partial, details, key, locals) + find_templates(name, prefix, partial, details, locals) + end + delegate :caching?, to: :class # This is what child classes implement. No defaults are needed @@ -169,35 +173,51 @@ module ActionView else @pattern = DEFAULT_PATTERN end + @unbound_templates = Concurrent::Map.new + super() + end + + def clear_cache + @unbound_templates.clear super() end private - def find_templates(name, prefix, partial, details, locals) + def _find_all(name, prefix, partial, details, key, locals) path = Path.build(name, prefix, partial) - query(path, details, details[:formats], locals) + query(path, details, details[:formats], locals, cache: !!key) end - def query(path, details, formats, locals) + def query(path, details, formats, locals, cache:) template_paths = find_template_paths_from_details(path, details) template_paths = reject_files_external_to_app(template_paths) template_paths.map do |template| - build_template(template, path.virtual, locals) + unbound_template = + if cache + @unbound_templates.compute_if_absent([template, path.virtual]) do + build_unbound_template(template, path.virtual) + end + else + build_unbound_template(template, path.virtual) + end + + unbound_template.bind_locals(locals) end end - def build_template(template, virtual_path, locals) + def build_unbound_template(template, virtual_path) handler, format, variant = extract_handler_and_format_and_variant(template) + source = Template::Sources::File.new(template) - filename = File.expand_path(template) - source = Template::Sources::File.new(filename) - Template.new(source, filename, handler, + UnboundTemplate.new( + source, + template, + handler, virtual_path: virtual_path, format: format, variant: variant, - locals: locals ) end @@ -363,8 +383,8 @@ module ActionView [new(""), new("/")] end - def build_template(template, virtual_path, locals) - super(template, nil, locals) + def build_unbound_template(template, _) + super(template, nil) end def reject_files_external_to_app(files) diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb index a97fb71b26..1bedf44934 100644 --- a/actionview/lib/action_view/testing/resolvers.rb +++ b/actionview/lib/action_view/testing/resolvers.rb @@ -23,7 +23,7 @@ module ActionView #:nodoc: private - def query(path, exts, _, locals) + def query(path, exts, _, locals, cache:) query = +"" EXTENSIONS.each do |ext, prefix| query << "(" << exts[ext].map { |e| e && Regexp.escape("#{prefix}#{e}") }.join("|") << "|)" @@ -47,7 +47,7 @@ module ActionView #:nodoc: end class NullResolver < PathResolver - def query(path, exts, _, locals) + def query(path, exts, _, locals, cache:) handler, format, variant = extract_handler_and_format_and_variant(path) [ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, virtual_path: path.virtual, format: format, variant: variant, locals: locals)] end diff --git a/actionview/lib/action_view/unbound_template.rb b/actionview/lib/action_view/unbound_template.rb new file mode 100644 index 0000000000..db69b6d016 --- /dev/null +++ b/actionview/lib/action_view/unbound_template.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require "concurrent/map" + +module ActionView + class UnboundTemplate + def initialize(source, identifer, handler, options) + @source = source + @identifer = identifer + @handler = handler + @options = options + + @templates = Concurrent::Map.new(initial_capacity: 2) + end + + def bind_locals(locals) + @templates[locals] ||= build_template(locals) + end + + private + + def build_template(locals) + options = @options.merge(locals: locals) + Template.new( + @source, + @identifer, + @handler, + options + ) + end + end +end diff --git a/actionview/test/template/file_system_resolver_test.rb b/actionview/test/template/file_system_resolver_test.rb new file mode 100644 index 0000000000..aa03fdcb13 --- /dev/null +++ b/actionview/test/template/file_system_resolver_test.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "template/resolver_shared_tests" + +class FileSystemResolverTest < ActiveSupport::TestCase + include ResolverSharedTests + + def resolver + ActionView::FileSystemResolver.new(tmpdir) + end +end diff --git a/actionview/test/template/optimized_file_system_resolver_test.rb b/actionview/test/template/optimized_file_system_resolver_test.rb new file mode 100644 index 0000000000..c0c64357ce --- /dev/null +++ b/actionview/test/template/optimized_file_system_resolver_test.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "template/resolver_shared_tests" + +class OptimizedFileSystemResolverTest < ActiveSupport::TestCase + include ResolverSharedTests + + def resolver + ActionView::OptimizedFileSystemResolver.new(tmpdir) + end +end diff --git a/actionview/test/template/resolver_shared_tests.rb b/actionview/test/template/resolver_shared_tests.rb new file mode 100644 index 0000000000..8b47c5bc89 --- /dev/null +++ b/actionview/test/template/resolver_shared_tests.rb @@ -0,0 +1,148 @@ +# frozen_string_literal: true + +module ResolverSharedTests + attr_reader :tmpdir + + def run(*args) + capture_exceptions do + Dir.mktmpdir(nil, __dir__) { |dir| @tmpdir = dir; super } + end + end + + def with_file(filename, source = "File at #{filename}") + path = File.join(tmpdir, filename) + FileUtils.mkdir_p(File.dirname(path)) + File.write(path, source) + end + + def context + @context ||= ActionView::LookupContext.new(resolver) + end + + def test_can_find_with_no_extensions + with_file "test/hello_world", "Hello default!" + + templates = resolver.find_all("hello_world", "test", false, locale: [:en], formats: [:html], variants: [:phone], handlers: [:erb]) + assert_equal 1, templates.size + assert_equal "Hello default!", templates[0].source + assert_equal "test/hello_world", templates[0].virtual_path + assert_nil templates[0].format + assert_nil templates[0].variant + assert_kind_of ActionView::Template::Handlers::Raw, templates[0].handler + end + + def test_can_find_with_just_handler + with_file "test/hello_world.erb", "Hello erb!" + + templates = resolver.find_all("hello_world", "test", false, locale: [:en], formats: [:html], variants: [:phone], handlers: [:erb]) + assert_equal 1, templates.size + assert_equal "Hello erb!", templates[0].source + assert_equal "test/hello_world", templates[0].virtual_path + assert_nil templates[0].format + assert_nil templates[0].variant + assert_kind_of ActionView::Template::Handlers::ERB, templates[0].handler + end + + def test_can_find_with_format_and_handler + with_file "test/hello_world.text.builder", "Hello plain text!" + + templates = resolver.find_all("hello_world", "test", false, locale: [:en], formats: [:html, :text], variants: [:phone], handlers: [:erb, :builder]) + assert_equal 1, templates.size + assert_equal "Hello plain text!", templates[0].source + assert_equal "test/hello_world", templates[0].virtual_path + assert_equal :text, templates[0].format + assert_nil templates[0].variant + assert_kind_of ActionView::Template::Handlers::Builder, templates[0].handler + end + + def test_can_find_with_variant_format_and_handler + with_file "test/hello_world.html+phone.erb", "Hello plain text!" + + templates = resolver.find_all("hello_world", "test", false, locale: [:en], formats: [:html], variants: [:phone], handlers: [:erb]) + assert_equal 1, templates.size + assert_equal "Hello plain text!", templates[0].source + assert_equal "test/hello_world", templates[0].virtual_path + assert_equal :html, templates[0].format + assert_equal "phone", templates[0].variant + assert_kind_of ActionView::Template::Handlers::ERB, templates[0].handler + end + + def test_can_find_with_any_variant_format_and_handler + with_file "test/hello_world.html+phone.erb", "Hello plain text!" + + templates = resolver.find_all("hello_world", "test", false, locale: [:en], formats: [:html], variants: :any, handlers: [:erb]) + assert_equal 1, templates.size + assert_equal "Hello plain text!", templates[0].source + assert_equal "test/hello_world", templates[0].virtual_path + assert_equal :html, templates[0].format + assert_equal "phone", templates[0].variant + assert_kind_of ActionView::Template::Handlers::ERB, templates[0].handler + end + + def test_doesnt_find_template_with_wrong_details + with_file "test/hello_world.html.erb", "Hello plain text!" + + templates = resolver.find_all("hello_world", "test", false, locale: [], formats: [:xml], variants: :any, handlers: [:builder]) + assert_equal 0, templates.size + + templates = resolver.find_all("hello_world", "test", false, locale: [], formats: [:xml], variants: :any, handlers: [:erb]) + assert_equal 0, templates.size + end + + def test_found_template_is_cached + with_file "test/hello_world.html.erb", "Hello HTML!" + + a = context.find("hello_world", "test", false, [], {}) + b = context.find("hello_world", "test", false, [], {}) + assert_same a, b + end + + def test_different_templates_when_cache_disabled + with_file "test/hello_world.html.erb", "Hello HTML!" + + a = context.find("hello_world", "test", false, [], {}) + b = context.disable_cache { context.find("hello_world", "test", false, [], {}) } + c = context.find("hello_world", "test", false, [], {}) + + # disable_cache should give us a new object + assert_not_same a, b + + # but it should not clear the cache + assert_same a, c + end + + def test_same_template_from_different_details_is_same_object + with_file "test/hello_world.html.erb", "Hello HTML!" + + a = context.find("hello_world", "test", false, [], locale: [:en]) + b = context.find("hello_world", "test", false, [], locale: [:fr]) + assert_same a, b + end + + def test_templates_with_optional_locale_shares_common_object + with_file "test/hello_world.text.erb", "Generic plain text!" + with_file "test/hello_world.fr.text.erb", "Texte en Francais!" + + en = context.find_all("hello_world", "test", false, [], locale: [:en]) + fr = context.find_all("hello_world", "test", false, [], locale: [:fr]) + + assert_equal 1, en.size + assert_equal 2, fr.size + + assert_equal "Generic plain text!", en[0].source + assert_equal "Texte en Francais!", fr[0].source + assert_equal "Generic plain text!", fr[1].source + + assert_same en[0], fr[1] + end + + def test_virtual_path_is_preserved_with_dot + with_file "test/hello_world.html.erb", "Hello html!" + + template = context.find("hello_world.html", "test", false, [], {}) + assert_equal "test/hello_world.html", template.virtual_path + + template = context.find("hello_world", "test", false, [], {}) + assert_equal "test/hello_world", template.virtual_path + end +end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 7ae6326fc3..485547f036 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,9 +1,32 @@ +* Fix dirty tracking after rollback. + + Fixes #15018, #30167, #33868. + + *Ryuta Kamizono* + +* Add `ActiveRecord::Relation#cache_version` to support recyclable cache keys via + the versioned entries in `ActiveSupport::Cache`. This also means that + `ActiveRecord::Relation#cache_key` will now return a stable key that does not + include the max timestamp or count any more. + + NOTE: This feature is turned off by default, and `cache_key` will still return + cache keys with timestamps until you set `ActiveRecord::Base.collection_cache_versioning = true`. + That's the setting for all new apps on Rails 6.0+ + + *Lachlan Sylvester* + +* Fix dirty tracking for `touch` to track saved changes. + + Fixes #33429. + + *Ryuta Kamzono* + * `change_column_comment` and `change_table_comment` are invertible only if `to` and `from` options are specified. *Yoshiyuki Kinjo* -* Don't call commit/rollback callbacks despite a record isn't saved. +* Don't call commit/rollback callbacks when a record isn't saved. Fixes #29747. diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index af7e46e649..238ea92da4 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -24,7 +24,7 @@ module ActiveRecord RESTRICTED_CLASS_METHODS = %w(private public protected allocate new name parent superclass) - class GeneratedAttributeMethodsBuilder < Module #:nodoc: + class GeneratedAttributeMethods < Module #:nodoc: include Mutex_m end @@ -35,7 +35,7 @@ module ActiveRecord end def initialize_generated_modules # :nodoc: - @generated_attribute_methods = const_set(:GeneratedAttributeMethods, GeneratedAttributeMethodsBuilder.new) + @generated_attribute_methods = const_set(:GeneratedAttributeMethods, GeneratedAttributeMethods.new) private_constant :GeneratedAttributeMethods @attribute_methods_generated = false include @generated_attribute_methods @@ -89,7 +89,7 @@ module ActiveRecord # If ThisClass < ... < SomeSuperClass < ... < Base and SomeSuperClass # defines its own attribute method, then we don't want to overwrite that. defined = method_defined_within?(method_name, superclass, Base) && - ! superclass.instance_method(method_name).owner.is_a?(GeneratedAttributeMethodsBuilder) + ! superclass.instance_method(method_name).owner.is_a?(GeneratedAttributeMethods) defined || super end end diff --git a/activerecord/lib/active_record/attribute_methods/before_type_cast.rb b/activerecord/lib/active_record/attribute_methods/before_type_cast.rb index dc239ff9ea..affcf2a4db 100644 --- a/activerecord/lib/active_record/attribute_methods/before_type_cast.rb +++ b/activerecord/lib/active_record/attribute_methods/before_type_cast.rb @@ -46,6 +46,7 @@ module ActiveRecord # task.read_attribute_before_type_cast('completed_on') # => "2012-10-21" # task.read_attribute_before_type_cast(:completed_on) # => "2012-10-21" def read_attribute_before_type_cast(attr_name) + sync_with_transaction_state @attributes[attr_name.to_s].value_before_type_cast end @@ -60,6 +61,7 @@ module ActiveRecord # task.attributes_before_type_cast # # => {"id"=>nil, "title"=>nil, "is_done"=>true, "completed_on"=>"2012-10-21", "created_at"=>nil, "updated_at"=>nil} def attributes_before_type_cast + sync_with_transaction_state @attributes.values_before_type_cast end @@ -71,6 +73,7 @@ module ActiveRecord end def attribute_came_from_user?(attribute_name) + sync_with_transaction_state @attributes[attribute_name].came_from_user? end end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 68ac8475b0..942fe48635 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -156,6 +156,16 @@ module ActiveRecord end private + def mutations_from_database + sync_with_transaction_state + super + end + + def mutations_before_last_save + sync_with_transaction_state + super + end + def write_attribute_without_type_cast(attr_name, value) name = attr_name.to_s if self.class.attribute_alias?(name) diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index 6af5346fa7..feaef72a30 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -16,39 +16,33 @@ module ActiveRecord # Returns the primary key column's value. def id - sync_with_transaction_state primary_key = self.class.primary_key _read_attribute(primary_key) if primary_key end # Sets the primary key column's value. def id=(value) - sync_with_transaction_state primary_key = self.class.primary_key _write_attribute(primary_key, value) if primary_key end # Queries the primary key column's value. def id? - sync_with_transaction_state query_attribute(self.class.primary_key) end # Returns the primary key column's value before type cast. def id_before_type_cast - sync_with_transaction_state read_attribute_before_type_cast(self.class.primary_key) end # Returns the primary key column's previous value. def id_was - sync_with_transaction_state attribute_was(self.class.primary_key) end # Returns the primary key column's value from the database. def id_in_database - sync_with_transaction_state attribute_in_database(self.class.primary_key) end diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index ffac5313ad..84b1ec2fea 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -9,14 +9,11 @@ module ActiveRecord private def define_method_attribute(name) - sync_with_transaction_state = "sync_with_transaction_state" if name == primary_key - ActiveModel::AttributeMethods::AttrNames.define_attribute_accessor_method( generated_attribute_methods, name ) do |temp_method_name, attr_name_expr| generated_attribute_methods.module_eval <<-RUBY, __FILE__, __LINE__ + 1 def #{temp_method_name} - #{sync_with_transaction_state} name = #{attr_name_expr} _read_attribute(name) { |n| missing_attribute(n, caller) } end @@ -36,13 +33,13 @@ module ActiveRecord primary_key = self.class.primary_key name = primary_key if name == "id" && primary_key - sync_with_transaction_state if name == primary_key _read_attribute(name, &block) end # This method exists to avoid the expensive primary_key check internally, without # breaking compatibility with the read_attribute API def _read_attribute(attr_name, &block) # :nodoc + sync_with_transaction_state @attributes.fetch_value(attr_name.to_s, &block) end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index d5ba2f42cb..d1cfe43bb2 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -13,15 +13,12 @@ module ActiveRecord private def define_method_attribute=(name) - sync_with_transaction_state = "sync_with_transaction_state" if name == primary_key - ActiveModel::AttributeMethods::AttrNames.define_attribute_accessor_method( generated_attribute_methods, name, writer: true, ) do |temp_method_name, attr_name_expr| generated_attribute_methods.module_eval <<-RUBY, __FILE__, __LINE__ + 1 def #{temp_method_name}(value) name = #{attr_name_expr} - #{sync_with_transaction_state} _write_attribute(name, value) end RUBY @@ -40,21 +37,21 @@ module ActiveRecord primary_key = self.class.primary_key name = primary_key if name == "id" && primary_key - sync_with_transaction_state if name == primary_key _write_attribute(name, value) end # This method exists to avoid the expensive primary_key check internally, without # breaking compatibility with the write_attribute API def _write_attribute(attr_name, value) # :nodoc: + sync_with_transaction_state @attributes.write_from_user(attr_name.to_s, value) value end private def write_attribute_without_type_cast(attr_name, value) - name = attr_name.to_s - @attributes.write_cast_value(name, value) + sync_with_transaction_state + @attributes.write_cast_value(attr_name.to_s, value) value end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 200184c2f9..bf0bb84c93 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -170,8 +170,11 @@ module ActiveRecord class Version include Comparable - def initialize(version_string) + attr_reader :full_version_string + + def initialize(version_string, full_version_string = nil) @version = version_string.split(".").map(&:to_i) + @full_version_string = full_version_string end def <=>(version_string) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 8b907759c6..282b2b1838 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -56,7 +56,9 @@ module ActiveRecord end def get_database_version #:nodoc: - Version.new(version_string) + full_version_string = get_full_version + version_string = version_string(full_version_string) + Version.new(version_string, full_version_string) end def mariadb? # :nodoc: @@ -788,8 +790,8 @@ module ActiveRecord MismatchedForeignKey.new(options) end - def version_string - full_version.match(/^(?:5\.5\.5-)?(\d+\.\d+\.\d+)/)[1] + def version_string(full_version_string) + full_version_string.match(/^(?:5\.5\.5-)?(\d+\.\d+\.\d+)/)[1] end class MysqlString < Type::String # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 0dc880c731..5b0335c22b 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -126,7 +126,11 @@ module ActiveRecord end def full_version - @full_version ||= @connection.server_info[:version] + schema_cache.database_version.full_version_string + end + + def get_full_version + @connection.server_info[:version] end end end diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb index 6782833c5a..040ebdb960 100644 --- a/activerecord/lib/active_record/connection_handling.rb +++ b/activerecord/lib/active_record/connection_handling.rb @@ -109,8 +109,8 @@ module ActiveRecord # a role. If you would like to use a different role you can pass a hash to database: # # ActiveRecord::Base.connected_to(database: { readonly_slow: :animals_slow_replica }) do - # Dog.run_a_long_query # runs a long query while connected to the +animals_slow_replica+ - # using the readonly_slow role. + # # runs a long query while connected to the +animals_slow_replica+ using the readonly_slow role. + # Dog.run_a_long_query # end # # When using the database key a new connection will be established every time. diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 6fed3e5c19..04b21b4d00 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -583,12 +583,6 @@ module ActiveRecord def initialize_internals_callback end - def thaw - if @attributes.frozen? - @attributes = @attributes.dup - end - end - def custom_inspect_method_defined? self.class.instance_method(:inspect).owner != ActiveRecord::Base.instance_method(:inspect).owner end diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb index b769541e95..810b6884f1 100644 --- a/activerecord/lib/active_record/integration.rb +++ b/activerecord/lib/active_record/integration.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "active_support/core_ext/string/filters" +require "active_support/deprecation" module ActiveRecord module Integration @@ -22,6 +23,14 @@ module ActiveRecord # # This is +true+, by default on Rails 5.2 and above. class_attribute :cache_versioning, instance_writer: false, default: false + + ## + # :singleton-method: + # Indicates whether to use a stable #cache_key method that is accompanied + # by a changing version in the #cache_version method on collections. + # + # This is +false+, by default until Rails 6.1. + class_attribute :collection_cache_versioning, instance_writer: false, default: false end # Returns a +String+, which Action Pack uses for constructing a URL to this @@ -154,8 +163,9 @@ module ActiveRecord end def collection_cache_key(collection = all, timestamp_column = :updated_at) # :nodoc: - collection.compute_cache_key(timestamp_column) + collection.send(:compute_cache_key, timestamp_column) end + deprecate :collection_cache_key end private diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index 6b84431343..6248c2f578 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -110,7 +110,7 @@ module ActiveRecord end def extract_query_source_location(locations) - backtrace_cleaner.clean(locations).first + backtrace_cleaner.clean(locations.lazy).first end end end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index ed0c6d48b8..f20edbeb93 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -4,6 +4,7 @@ require "benchmark" require "set" require "zlib" require "active_support/core_ext/module/attribute_accessors" +require "active_support/actionable_error" module ActiveRecord class MigrationError < ActiveRecordError #:nodoc: @@ -128,6 +129,12 @@ module ActiveRecord end class PendingMigrationError < MigrationError #:nodoc: + include ActiveSupport::ActionableError + + action "Run pending migrations" do + ActiveRecord::Tasks::DatabaseTasks.migrate + end + def initialize(message = nil) if !message && defined?(Rails.env) super("Migrations are pending. To resolve this issue, run:\n\n rails db:migrate RAILS_ENV=#{::Rails.env}") diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 447def8d77..64a4cb0886 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -78,7 +78,7 @@ db_namespace = namespace :db do desc "Migrate the database (options: VERSION=x, VERBOSE=false, SCOPE=blog)." task migrate: :load_config do - ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config| + ActiveRecord::Base.configurations.configs_for(env_name: ActiveRecord::Tasks::DatabaseTasks.env).each do |db_config| ActiveRecord::Base.establish_connection(db_config.config) ActiveRecord::Tasks::DatabaseTasks.migrate end @@ -154,7 +154,7 @@ db_namespace = namespace :db do desc "Display status of migrations" task status: :load_config do - ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config| + ActiveRecord::Base.configurations.configs_for(env_name: ActiveRecord::Tasks::DatabaseTasks.env).each do |db_config| ActiveRecord::Base.establish_connection(db_config.config) ActiveRecord::Tasks::DatabaseTasks.migrate_status end @@ -224,7 +224,7 @@ db_namespace = namespace :db do 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.configurations.configs_for(env_name: ActiveRecord::Tasks::DatabaseTasks.env).each do |db_config| ActiveRecord::Base.establish_connection(db_config.config) db_namespace["migrate"].invoke rescue ActiveRecord::NoDatabaseError @@ -295,7 +295,7 @@ db_namespace = namespace :db do desc "Creates a db/schema.rb file that is portable against any DB supported by Active Record" task dump: :load_config do require "active_record/schema_dumper" - ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config| + ActiveRecord::Base.configurations.configs_for(env_name: ActiveRecord::Tasks::DatabaseTasks.env).each do |db_config| filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(db_config.spec_name, :ruby) File.open(filename, "w:utf-8") do |file| ActiveRecord::Base.establish_connection(db_config.config) @@ -318,7 +318,7 @@ db_namespace = namespace :db do namespace :cache do desc "Creates a db/schema_cache.yml file." task dump: :load_config do - ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config| + ActiveRecord::Base.configurations.configs_for(env_name: ActiveRecord::Tasks::DatabaseTasks.env).each do |db_config| ActiveRecord::Base.establish_connection(db_config.config) filename = ActiveRecord::Tasks::DatabaseTasks.cache_dump_filename(db_config.spec_name) ActiveRecord::Tasks::DatabaseTasks.dump_schema_cache( @@ -330,7 +330,7 @@ db_namespace = namespace :db do desc "Clears a db/schema_cache.yml file." task clear: :load_config do - ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config| + ActiveRecord::Base.configurations.configs_for(env_name: ActiveRecord::Tasks::DatabaseTasks.env).each do |db_config| filename = ActiveRecord::Tasks::DatabaseTasks.cache_dump_filename(db_config.spec_name) rm_f filename, verbose: false end @@ -341,7 +341,7 @@ db_namespace = namespace :db do namespace :structure do desc "Dumps the database structure to db/structure.sql. Specify another file with SCHEMA=db/my_structure.sql" task dump: :load_config do - ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config| + ActiveRecord::Base.configurations.configs_for(env_name: ActiveRecord::Tasks::DatabaseTasks.env).each do |db_config| ActiveRecord::Base.establish_connection(db_config.config) filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(db_config.spec_name, :sql) ActiveRecord::Tasks::DatabaseTasks.structure_dump(db_config.config, filename) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index cd62b0b881..b3a1b69293 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -291,36 +291,58 @@ module ActiveRecord limit_value ? records.many? : size > 1 end - # Returns a cache key that can be used to identify the records fetched by - # this query. The cache key is built with a fingerprint of the sql query, - # the number of records matched by the query and a timestamp of the last - # updated record. When a new record comes to match the query, or any of - # the existing records is updated or deleted, the cache key changes. + # Returns a stable cache key that can be used to identify this query. + # The cache key is built with a fingerprint of the SQL query. # - # Product.where("name like ?", "%Cosmic Encounter%").cache_key - # # => "products/query-1850ab3d302391b85b8693e941286659-1-20150714212553907087000" + # Product.where("name like ?", "%Cosmic Encounter%").cache_key + # # => "products/query-1850ab3d302391b85b8693e941286659" # - # If the collection is loaded, the method will iterate through the records - # to generate the timestamp, otherwise it will trigger one SQL query like: + # If ActiveRecord::Base.collection_cache_versioning is turned off, as it was + # in Rails 6.0 and earlier, the cache key will also include a version. # - # SELECT COUNT(*), MAX("products"."updated_at") FROM "products" WHERE (name like '%Cosmic Encounter%') + # ActiveRecord::Base.collection_cache_versioning = false + # Product.where("name like ?", "%Cosmic Encounter%").cache_key + # # => "products/query-1850ab3d302391b85b8693e941286659-1-20150714212553907087000" # # You can also pass a custom timestamp column to fetch the timestamp of the # last updated record. # # Product.where("name like ?", "%Game%").cache_key(:last_reviewed_at) - # - # You can customize the strategy to generate the key on a per model basis - # overriding ActiveRecord::Base#collection_cache_key. def cache_key(timestamp_column = :updated_at) @cache_keys ||= {} - @cache_keys[timestamp_column] ||= @klass.collection_cache_key(self, timestamp_column) + @cache_keys[timestamp_column] ||= compute_cache_key(timestamp_column) end def compute_cache_key(timestamp_column = :updated_at) # :nodoc: query_signature = ActiveSupport::Digest.hexdigest(to_sql) key = "#{klass.model_name.cache_key}/query-#{query_signature}" + if cache_version(timestamp_column) + key + else + "#{key}-#{compute_cache_version(timestamp_column)}" + end + end + private :compute_cache_key + + # Returns a cache version that can be used together with the cache key to form + # a recyclable caching scheme. The cache version is built with the number of records + # matching the query, and the timestamp of the last updated record. When a new record + # comes to match the query, or any of the existing records is updated or deleted, + # the cache version changes. + # + # If the collection is loaded, the method will iterate through the records + # to generate the timestamp, otherwise it will trigger one SQL query like: + # + # SELECT COUNT(*), MAX("products"."updated_at") FROM "products" WHERE (name like '%Cosmic Encounter%') + def cache_version(timestamp_column = :updated_at) + if collection_cache_versioning + @cache_versions ||= {} + @cache_versions[timestamp_column] ||= compute_cache_version(timestamp_column) + end + end + + def compute_cache_version(timestamp_column) # :nodoc: if loaded? || distinct_value size = records.size if size > 0 @@ -356,11 +378,12 @@ module ActiveRecord end if timestamp - "#{key}-#{size}-#{timestamp.utc.to_s(cache_timestamp_format)}" + "#{size}-#{timestamp.utc.to_s(cache_timestamp_format)}" else - "#{key}-#{size}" + "#{size}" end end + private :compute_cache_version # Scope all queries to the current scope. # diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 7a53a9d1c7..d59331053e 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -45,7 +45,10 @@ module ActiveRecord private def generated_relation_methods - @generated_relation_methods ||= GeneratedRelationMethods.new + @generated_relation_methods ||= GeneratedRelationMethods.new.tap do |mod| + const_set(:GeneratedRelationMethods, mod) + private_constant :GeneratedRelationMethods + end end end diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 7285c15477..53e7e1e6d7 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -142,6 +142,8 @@ module ActiveRecord end def for_each + return {} unless defined?(Rails) + databases = Rails.application.config.load_database_yaml database_configs = ActiveRecord::DatabaseConfigurations.new(databases).configs_for(env_name: Rails.env) diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 634dc50376..ea288456b9 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -390,6 +390,7 @@ module ActiveRecord id: id, new_record: @new_record, destroyed: @destroyed, + attributes: @attributes, frozen?: frozen?, ) @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) + 1 @@ -422,12 +423,18 @@ module ActiveRecord transaction_level = (@_start_transaction_state[:level] || 0) - 1 if transaction_level < 1 || force_restore_state restore_state = @_start_transaction_state - thaw @new_record = restore_state[:new_record] @destroyed = restore_state[:destroyed] + @attributes = restore_state[:attributes].map do |attr| + value = @attributes.fetch_value(attr.name) + attr = attr.with_value_from_user(value) if attr.value != value + attr + end + @mutations_from_database = nil + @mutations_before_last_save = nil pk = self.class.primary_key - if pk && _read_attribute(pk) != restore_state[:id] - _write_attribute(pk, restore_state[:id]) + if pk && @attributes.fetch_value(pk) != restore_state[:id] + @attributes.write_from_user(pk, restore_state[:id]) end freeze if restore_state[:frozen?] end @@ -472,13 +479,13 @@ module ActiveRecord # the TransactionState, and rolls back or commits the Active Record object # as appropriate. def sync_with_transaction_state - if @transaction_state && @transaction_state.finalized? - if @transaction_state.fully_committed? + if (transaction_state = @transaction_state)&.finalized? + if transaction_state.fully_committed? force_clear_transaction_record_state - elsif @transaction_state.committed? + elsif transaction_state.committed? clear_transaction_record_state - elsif @transaction_state.rolledback? - force_restore_state = @transaction_state.fully_rolledback? + elsif transaction_state.rolledback? + force_restore_state = transaction_state.fully_rolledback? restore_transaction_record_state(force_restore_state) clear_transaction_record_state end diff --git a/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt b/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt index c1c03e2762..77b9ea1c86 100644 --- a/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt +++ b/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt @@ -1,7 +1,7 @@ <% module_namespacing do -%> class <%= class_name %> < <%= parent_class_name.classify %> <% attributes.select(&:reference?).each do |attribute| -%> - belongs_to :<%= attribute.name %><%= ', polymorphic: true' if attribute.polymorphic? %><%= ', required: true' if attribute.required? %> + belongs_to :<%= attribute.name %><%= ', polymorphic: true' if attribute.polymorphic? %> <% end -%> <% attributes.select(&:rich_text?).each do |attribute| -%> has_rich_text :<%= attribute.name %> diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 9fd62dcf72..5cbe5d796d 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -1081,9 +1081,9 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert_equal ["title"], model.accessed_fields end - test "generated attribute methods ancestors have correct class" do + test "generated attribute methods ancestors have correct module" do mod = Topic.send(:generated_attribute_methods) - assert_match %r(Topic::GeneratedAttributeMethods), mod.inspect + assert_equal "Topic::GeneratedAttributeMethods", mod.inspect end private diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 99f47cfe37..ddafa468ed 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -67,6 +67,16 @@ end class BasicsTest < ActiveRecord::TestCase fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, "warehouse-things", :authors, :author_addresses, :categorizations, :categories, :posts + def test_generated_association_methods_module_name + mod = Post.send(:generated_association_methods) + assert_equal "Post::GeneratedAssociationMethods", mod.inspect + end + + def test_generated_relation_methods_module_name + mod = Post.send(:generated_relation_methods) + assert_equal "Post::GeneratedRelationMethods", mod.inspect + end + def test_column_names_are_escaped conn = ActiveRecord::Base.connection classname = conn.class.name[/[^:]*$/] @@ -1205,6 +1215,8 @@ class BasicsTest < ActiveRecord::TestCase wr.close assert Marshal.load rd.read rd.close + ensure + self.class.send(:remove_const, "Post") if self.class.const_defined?("Post", false) end end diff --git a/activerecord/test/cases/collection_cache_key_test.rb b/activerecord/test/cases/collection_cache_key_test.rb index 483383257b..f01bc0d7f1 100644 --- a/activerecord/test/cases/collection_cache_key_test.rb +++ b/activerecord/test/cases/collection_cache_key_test.rb @@ -13,7 +13,9 @@ module ActiveRecord fixtures :developers, :projects, :developers_projects, :topics, :comments, :posts test "collection_cache_key on model" do - assert_match(/\Adevelopers\/query-(\h+)-(\d+)-(\d+)\z/, Developer.collection_cache_key) + assert_deprecated do + assert_match(/\Adevelopers\/query-(\h+)-(\d+)-(\d+)\z/, Developer.collection_cache_key) + end end test "cache_key for relation" do @@ -171,5 +173,39 @@ module ActiveRecord assert_match(/\Adevelopers\/query-(\h+)-(\d+)-(\d+)\z/, developers.cache_key) end + + test "cache_key should be stable when using collection_cache_versioning" do + with_collection_cache_versioning do + developers = Developer.where(salary: 100000) + + assert_match(/\Adevelopers\/query-(\h+)\z/, developers.cache_key) + + /\Adevelopers\/query-(\h+)\z/ =~ developers.cache_key + + assert_equal ActiveSupport::Digest.hexdigest(developers.to_sql), $1 + end + end + + test "cache_version for relation" do + with_collection_cache_versioning do + developers = Developer.where(salary: 100000).order(updated_at: :desc) + last_developer_timestamp = developers.first.updated_at + + assert_match(/(\d+)-(\d+)\z/, developers.cache_version) + + /(\d+)-(\d+)\z/ =~ developers.cache_version + + assert_equal developers.count.to_s, $1 + assert_equal last_developer_timestamp.to_s(ActiveRecord::Base.cache_timestamp_format), $2 + end + end + + def with_collection_cache_versioning(value = true) + @old_collection_cache_versioning = ActiveRecord::Base.collection_cache_versioning + ActiveRecord::Base.collection_cache_versioning = value + yield + ensure + ActiveRecord::Base.collection_cache_versioning = @old_collection_cache_versioning + end end end diff --git a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb index a2d289bf2f..d3184f39f5 100644 --- a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb @@ -209,7 +209,7 @@ module ActiveRecord config = { "default_env" => { "animals" => { adapter: "sqlite3", database: "db/animals.sqlite3" }, - "primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" } + "primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" } } } @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config @@ -236,7 +236,7 @@ module ActiveRecord config = { "default_env" => { "animals" => { adapter: "sqlite3", database: "db/animals.sqlite3" }, - "primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" } + "primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" } } } @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config diff --git a/activerecord/test/cases/connection_adapters/schema_cache_test.rb b/activerecord/test/cases/connection_adapters/schema_cache_test.rb index 89a9c30f9b..28e232b88f 100644 --- a/activerecord/test/cases/connection_adapters/schema_cache_test.rb +++ b/activerecord/test/cases/connection_adapters/schema_cache_test.rb @@ -95,6 +95,10 @@ module ActiveRecord assert_no_queries do assert_equal @database_version.to_s, @cache.database_version.to_s + + if current_adapter?(:Mysql2Adapter) + assert_not_nil @cache.database_version.full_version_string + end end end diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index dd4a0b0455..ffe94eee0f 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -760,7 +760,7 @@ module ActiveRecord end class DatabaseTasksMigrateTest < DatabaseTasksMigrationTestCase - def test_migrate_set_and_unset_verbose_and_version_env_vars + def test_can_migrate_from_pending_migration_error_action_dispatch verbose, version = ENV["VERBOSE"], ENV["VERSION"] ENV["VERSION"] = "2" ENV["VERBOSE"] = "false" @@ -772,7 +772,9 @@ module ActiveRecord ENV.delete("VERBOSE") # re-run up migration - assert_includes capture_migration_output, "migrating" + assert_includes(capture(:stdout) do + ActiveSupport::ActionableError.dispatch ActiveRecord::PendingMigrationError, "Run pending migrations" + end, "migrating") ensure ENV["VERBOSE"], ENV["VERSION"] = verbose, version end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 7bad3de343..6795996cca 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -18,6 +18,65 @@ class TransactionTest < ActiveRecord::TestCase @first, @second = Topic.find(1, 2).sort_by(&:id) end + def test_rollback_dirty_changes + topic = topics(:fifth) + + ActiveRecord::Base.transaction do + topic.update(title: "Ruby on Rails") + raise ActiveRecord::Rollback + end + + title_change = ["The Fifth Topic of the day", "Ruby on Rails"] + assert_equal title_change, topic.changes["title"] + end + + def test_rollback_dirty_changes_multiple_saves + topic = topics(:fifth) + + ActiveRecord::Base.transaction do + topic.update(title: "Ruby on Rails") + topic.update(title: "Another Title") + raise ActiveRecord::Rollback + end + + title_change = ["The Fifth Topic of the day", "Another Title"] + assert_equal title_change, topic.changes["title"] + end + + def test_rollback_dirty_changes_then_retry_save + topic = topics(:fifth) + + ActiveRecord::Base.transaction do + topic.update(title: "Ruby on Rails") + raise ActiveRecord::Rollback + end + + title_change = ["The Fifth Topic of the day", "Ruby on Rails"] + assert_equal title_change, topic.changes["title"] + + assert topic.save + + assert_equal title_change, topic.saved_changes["title"] + assert_equal topic.title, topic.reload.title + end + + def test_rollback_dirty_changes_then_retry_save_on_new_record + topic = Topic.new(title: "Ruby on Rails") + + ActiveRecord::Base.transaction do + topic.save + raise ActiveRecord::Rollback + end + + title_change = [nil, "Ruby on Rails"] + assert_equal title_change, topic.changes["title"] + + assert topic.save + + assert_equal title_change, topic.saved_changes["title"] + assert_equal topic.title, topic.reload.title + end + def test_persisted_in_a_model_with_custom_primary_key_after_failed_save movie = Movie.create assert_not_predicate movie, :persisted? diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 57e03b5e12..4c7b134c35 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,52 @@ +* Introduce `ActiveSupport::ActionableError`. + + Actionable errors let's you dispatch actions from Rails' error pages. This + can help you save time if you have a clear action for the resolution of + common development errors. + + The de-facto example are pending migrations. Every time pending migrations + are found, a middleware raises an error. With actionable errors, you can + run the migrations right from the error page. Other examples include Rails + plugins that need to run a rake task to setup themselves. They can now + raise actionable errors to run the setup straight from the error pages. + + Here is how to define an actionable error: + + ```ruby + class PendingMigrationError < MigrationError #:nodoc: + include ActiveSupport::ActionableError + + action "Run pending migrations" do + ActiveRecord::Tasks::DatabaseTasks.migrate + end + end + ``` + + To make an error actionable, include the `ActiveSupport::ActionableError` + module and invoke the `action` class macro to define the action. An action + needs a name and a procedure to execute. The name is shown as the name of a + button on the error pages. Once clicked, it will invoke the given + procedure. + + *Vipul A M*, *Yao Jie*, *Genadi Samokovarov* + +* Preserve `html_safe?` status on `ActiveSupport::SafeBuffer#*`. + + Before: + + ("<br />".html_safe * 2).html_safe? #=> nil + + After: + + ("<br />".html_safe * 2).html_safe? #=> true + + *Ryo Nakamura* + +* Calling test methods with `with_info_handler` method to allow minitest-hooks + plugin to work. + + *Mauri Mustonen* + * The Zeitwerk compatibility interface for `ActiveSupport::Dependencies` no longer implements `autoloaded_constants` or `autoloaded?` (undocumented, anyway). Experience shows introspection does not have many use cases, and diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb index 5589c71281..9e242ddeaa 100644 --- a/activesupport/lib/active_support.rb +++ b/activesupport/lib/active_support.rb @@ -34,6 +34,7 @@ module ActiveSupport extend ActiveSupport::Autoload autoload :Concern + autoload :ActionableError autoload :CurrentAttributes autoload :Dependencies autoload :DescendantsTracker diff --git a/activesupport/lib/active_support/actionable_error.rb b/activesupport/lib/active_support/actionable_error.rb new file mode 100644 index 0000000000..7db14cd178 --- /dev/null +++ b/activesupport/lib/active_support/actionable_error.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module ActiveSupport + # Actionable errors let's you define actions to resolve an error. + # + # To make an error actionable, include the <tt>ActiveSupport::ActionableError</tt> + # module and invoke the +action+ class macro to define the action. An action + # needs a name and a block to execute. + module ActionableError + extend Concern + + class NonActionable < StandardError; end + + included do + class_attribute :_actions, default: {} + end + + def self.actions(error) # :nodoc: + case error + when ActionableError, -> it { Class === it && it < ActionableError } + error._actions + else + {} + end + end + + def self.dispatch(error, name) # :nodoc: + actions(error).fetch(name).call + rescue KeyError + raise NonActionable, "Cannot find action \"#{name}\"" + end + + module ClassMethods + # Defines an action that can resolve the error. + # + # class PendingMigrationError < MigrationError + # include ActiveSupport::ActionableError + # + # action "Run pending migrations" do + # ActiveRecord::Tasks::DatabaseTasks.migrate + # end + # end + def action(name, &block) + _actions[name] = block + end + end + end +end diff --git a/activesupport/lib/active_support/backtrace_cleaner.rb b/activesupport/lib/active_support/backtrace_cleaner.rb index 62973eca58..02cbfbaee6 100644 --- a/activesupport/lib/active_support/backtrace_cleaner.rb +++ b/activesupport/lib/active_support/backtrace_cleaner.rb @@ -122,7 +122,11 @@ module ActiveSupport end def noise(backtrace) - backtrace - silence(backtrace) + backtrace.select do |line| + @silencers.any? do |s| + s.call(line) + end + end end end end diff --git a/activesupport/lib/active_support/cache/redis_cache_store.rb b/activesupport/lib/active_support/cache/redis_cache_store.rb index 87f9aa5346..bb092fcde9 100644 --- a/activesupport/lib/active_support/cache/redis_cache_store.rb +++ b/activesupport/lib/active_support/cache/redis_cache_store.rb @@ -152,12 +152,14 @@ module ActiveSupport # Creates a new Redis cache store. # - # Handles three options: block provided to instantiate, single URL - # provided, and multiple URLs provided. + # Handles four options: :redis block, :redis instance, single :url + # string, and multiple :url strings. # - # :redis Proc -> options[:redis].call - # :url String -> Redis.new(url: …) - # :url Array -> Redis::Distributed.new([{ url: … }, { url: … }, …]) + # Option Class Result + # :redis Proc -> options[:redis].call + # :redis Object -> options[:redis] + # :url String -> Redis.new(url: …) + # :url Array -> Redis::Distributed.new([{ url: … }, { url: … }, …]) # # No namespace is set by default. Provide one if the Redis cache # server is shared with other apps: <tt>namespace: 'myapp-cache'</tt>. diff --git a/activesupport/lib/active_support/concern.rb b/activesupport/lib/active_support/concern.rb index 5d356a0ab6..708c445031 100644 --- a/activesupport/lib/active_support/concern.rb +++ b/activesupport/lib/active_support/concern.rb @@ -110,7 +110,7 @@ module ActiveSupport base.instance_variable_set(:@_dependencies, []) end - def append_features(base) + def append_features(base) #:nodoc: if base.instance_variable_defined?(:@_dependencies) base.instance_variable_get(:@_dependencies) << self false @@ -123,6 +123,9 @@ module ActiveSupport end end + # Evaluate given block in context of base class, + # so that you can write class macros here. + # When you define more than one +included+ block, it raises an exception. def included(base = nil, &block) if base.nil? if instance_variable_defined?(:@_included_block) @@ -137,6 +140,26 @@ module ActiveSupport end end + # Define class methods from given block. + # You can define private class methods as well. + # + # module Example + # extend ActiveSupport::Concern + # + # class_methods do + # def foo; puts 'foo'; end + # + # private + # def bar; puts 'bar'; end + # end + # end + # + # class Buzz + # include Example + # end + # + # Buzz.foo # => "foo" + # Buzz.bar # => private method 'bar' called for Buzz:Class(NoMethodError) def class_methods(&class_methods_module_definition) mod = const_defined?(:ClassMethods, false) ? const_get(:ClassMethods) : diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb index 638152626b..645b1fea17 100644 --- a/activesupport/lib/active_support/core_ext/string/output_safety.rb +++ b/activesupport/lib/active_support/core_ext/string/output_safety.rb @@ -213,6 +213,12 @@ module ActiveSupport #:nodoc: dup.concat(other) end + def *(*) + new_safe_buffer = super + new_safe_buffer.instance_variable_set(:@html_safe, @html_safe) + new_safe_buffer + end + def %(args) case args when Hash diff --git a/activesupport/lib/active_support/deprecation/method_wrappers.rb b/activesupport/lib/active_support/deprecation/method_wrappers.rb index d99571790f..7c0a54a1d0 100644 --- a/activesupport/lib/active_support/deprecation/method_wrappers.rb +++ b/activesupport/lib/active_support/deprecation/method_wrappers.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "active_support/core_ext/array/extract_options" +require "active_support/core_ext/module/redefine_method" module ActiveSupport class Deprecation @@ -52,29 +53,17 @@ module ActiveSupport options = method_names.extract_options! deprecator = options.delete(:deprecator) || self method_names += options.keys - mod = Module.new + mod = nil method_names.each do |method_name| if target_module.method_defined?(method_name) || target_module.private_method_defined?(method_name) - aliased_method, punctuation = method_name.to_s.sub(/([?!=])$/, ""), $1 - with_method = "#{aliased_method}_with_deprecation#{punctuation}" - without_method = "#{aliased_method}_without_deprecation#{punctuation}" - - target_module.define_method(with_method) do |*args, &block| + method = target_module.instance_method(method_name) + target_module.redefine_method(method_name) do |*args, &block| deprecator.deprecation_warning(method_name, options[method_name]) - send(without_method, *args, &block) - end - - target_module.alias_method(without_method, method_name) - target_module.alias_method(method_name, with_method) - - case - when target_module.protected_method_defined?(without_method) - target_module.send(:protected, method_name) - when target_module.private_method_defined?(without_method) - target_module.send(:private, method_name) + method.bind(self).call(*args, &block) end else + mod ||= Module.new mod.define_method(method_name) do |*args, &block| deprecator.deprecation_warning(method_name, options[method_name]) super(*args, &block) @@ -82,7 +71,7 @@ module ActiveSupport end end - target_module.prepend(mod) unless mod.instance_methods(false).empty? + target_module.prepend(mod) if mod end end end diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb index 97b4634d7b..a30bd11a87 100644 --- a/activesupport/lib/active_support/duration.rb +++ b/activesupport/lib/active_support/duration.rb @@ -4,7 +4,6 @@ require "active_support/core_ext/array/conversions" require "active_support/core_ext/module/delegation" require "active_support/core_ext/object/acts_like" require "active_support/core_ext/string/filters" -require "active_support/deprecation" module ActiveSupport # Provides accurate date and time measurements using Date#advance and diff --git a/activesupport/lib/active_support/i18n_railtie.rb b/activesupport/lib/active_support/i18n_railtie.rb index 584930e413..8faa93a3e4 100644 --- a/activesupport/lib/active_support/i18n_railtie.rb +++ b/activesupport/lib/active_support/i18n_railtie.rb @@ -97,7 +97,8 @@ module I18n If you desire the default locale to be included in the defaults, please explicitly configure it with `config.i18n.fallbacks.defaults = [I18n.default_locale]` or `config.i18n.fallbacks = [I18n.default_locale, - {...}]` + {...}]`. If you want to opt-in to the new behavior, use + `config.i18n.fallbacks.defaults = [nil, {...}]`. MSG args.unshift I18n.default_locale end diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index c506b35b1e..8812b67f63 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -180,13 +180,13 @@ module ActiveSupport def start(name, id, payload) timestack = Thread.current[:_timestack] ||= [] - timestack.push Time.now + timestack.push Concurrent.monotonic_time end def finish(name, id, payload) timestack = Thread.current[:_timestack] started = timestack.pop - @delegate.call(name, started, Time.now, id, payload) + @delegate.call(name, started, Concurrent.monotonic_time, id, payload) end end diff --git a/activesupport/lib/active_support/notifications/instrumenter.rb b/activesupport/lib/active_support/notifications/instrumenter.rb index a03e7e483e..12546511a8 100644 --- a/activesupport/lib/active_support/notifications/instrumenter.rb +++ b/activesupport/lib/active_support/notifications/instrumenter.rb @@ -68,9 +68,8 @@ module ActiveSupport @transaction_id = transaction_id @end = ending @children = [] - @duration = nil - @cpu_time_start = nil - @cpu_time_finish = nil + @cpu_time_start = 0 + @cpu_time_finish = 0 @allocation_count_start = 0 @allocation_count_finish = 0 end @@ -125,7 +124,7 @@ module ActiveSupport # # @event.duration # => 1000.138 def duration - @duration ||= 1000.0 * (self.end - time) + 1000.0 * (self.end - time) end def <<(event) diff --git a/activesupport/lib/active_support/testing/parallelization.rb b/activesupport/lib/active_support/testing/parallelization.rb index 63440069b1..e760bf5ce3 100644 --- a/activesupport/lib/active_support/testing/parallelization.rb +++ b/activesupport/lib/active_support/testing/parallelization.rb @@ -71,7 +71,9 @@ module ActiveSupport fork do DRb.stop_service - after_fork(worker) + begin + after_fork(worker) + rescue => setup_exception; end queue = DRbObject.new_with_uri(@url) @@ -79,7 +81,11 @@ module ActiveSupport klass = job[0] method = job[1] reporter = job[2] - result = Minitest.run_one_method(klass, method) + result = klass.with_info_handler reporter do + Minitest.run_one_method(klass, method) + end + + add_setup_exception(result, setup_exception) if setup_exception begin queue.record(reporter, result) @@ -104,6 +110,11 @@ module ActiveSupport @queue_size.times { @queue << nil } @pool.each { |pid| Process.waitpid pid } end + + private + def add_setup_exception(result, setup_exception) + result.failures.prepend Minitest::UnexpectedError.new(setup_exception) + end end end end diff --git a/activesupport/test/actionable_error_test.rb b/activesupport/test/actionable_error_test.rb new file mode 100644 index 0000000000..63046b937c --- /dev/null +++ b/activesupport/test/actionable_error_test.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "active_support/actionable_error" + +class ActionableErrorTest < ActiveSupport::TestCase + NonActionableError = Class.new(StandardError) + + class DispatchableError < StandardError + include ActiveSupport::ActionableError + + class_attribute :flip1, default: false + class_attribute :flip2, default: false + + action "Flip 1" do + self.flip1 = true + end + + action "Flip 2" do + self.flip2 = true + end + end + + test "returns all action of an actionable error" do + assert_equal ["Flip 1", "Flip 2"], ActiveSupport::ActionableError.actions(DispatchableError).keys + assert_equal ["Flip 1", "Flip 2"], ActiveSupport::ActionableError.actions(DispatchableError.new).keys + end + + test "returns no actions for non-actionable errors" do + assert ActiveSupport::ActionableError.actions(Exception).empty? + assert ActiveSupport::ActionableError.actions(Exception.new).empty? + end + + test "dispatches actions from error and name" do + assert_changes "DispatchableError.flip1", from: false, to: true do + ActiveSupport::ActionableError.dispatch DispatchableError, "Flip 1" + end + end + + test "cannot dispatch missing actions" do + err = assert_raises ActiveSupport::ActionableError::NonActionable do + ActiveSupport::ActionableError.dispatch NonActionableError, "action" + end + + assert_equal 'Cannot find action "action"', err.to_s + end +end diff --git a/activesupport/test/deprecation/method_wrappers_test.rb b/activesupport/test/deprecation/method_wrappers_test.rb index 18729941bc..0aa3233aab 100644 --- a/activesupport/test/deprecation/method_wrappers_test.rb +++ b/activesupport/test/deprecation/method_wrappers_test.rb @@ -89,12 +89,4 @@ class MethodWrappersTest < ActiveSupport::TestCase warning = /old_method is deprecated and will be removed from Rails \d.\d \(use new_method instead\)/ assert_deprecated(warning) { assert_equal "abc", @klass.old_method } end - - def test_method_with_without_deprecation_is_exposed - ActiveSupport::Deprecation.deprecate_methods(@klass, old_method: :new_method) - - warning = /old_method is deprecated and will be removed from Rails \d.\d \(use new_method instead\)/ - assert_deprecated(warning) { assert_equal "abc", @klass.new.old_method_with_deprecation } - assert_equal "abc", @klass.new.old_method_without_deprecation - end end diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index bb20d26a25..0af59764b5 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -302,7 +302,7 @@ module Notifications class EventTest < TestCase def test_events_are_initialized_with_details - time = Time.now + time = Concurrent.monotonic_time event = event(:foo, time, time + 0.01, random_id, {}) assert_equal :foo, event.name @@ -310,15 +310,24 @@ module Notifications assert_in_delta 10.0, event.duration, 0.00001 end + def test_event_cpu_time_and_idle_time_when_start_and_finish_is_not_called + time = Concurrent.monotonic_time + event = event(:foo, time, time + 0.01, random_id, {}) + + assert_equal 0, event.cpu_time + assert_in_delta 10.0, event.idle_time, 0.00001 + end + + def test_events_consumes_information_given_as_payload - event = event(:foo, Time.now, Time.now + 1, random_id, payload: :bar) + event = event(:foo, Concurrent.monotonic_time, Concurrent.monotonic_time + 1, random_id, payload: :bar) assert_equal Hash[payload: :bar], event.payload end def test_event_is_parent_based_on_children - time = Time.utc(2009, 01, 01, 0, 0, 1) + time = Concurrent.monotonic_time - parent = event(:foo, Time.utc(2009), Time.utc(2009) + 100, random_id, {}) + parent = event(:foo, Concurrent.monotonic_time, Concurrent.monotonic_time + 100, random_id, {}) child = event(:foo, time, time + 10, random_id, {}) not_child = event(:foo, time, time + 100, random_id, {}) diff --git a/activesupport/test/safe_buffer_test.rb b/activesupport/test/safe_buffer_test.rb index b1a1c2d390..f475e05c9a 100644 --- a/activesupport/test/safe_buffer_test.rb +++ b/activesupport/test/safe_buffer_test.rb @@ -150,6 +150,14 @@ class SafeBufferTest < ActiveSupport::TestCase assert_equal "hello<>", clean + @buffer end + test "Should preserve html_safe? status on multiplication" do + multiplied_safe_buffer = "<br />".html_safe * 2 + assert_predicate multiplied_safe_buffer, :html_safe? + + multiplied_unsafe_buffer = @buffer.gsub("", "<>") * 2 + assert_not_predicate multiplied_unsafe_buffer, :html_safe? + end + test "Should concat as a normal string when safe" do clean = "hello".html_safe @buffer.gsub!("", "<>") diff --git a/guides/source/active_support_instrumentation.md b/guides/source/active_support_instrumentation.md index e5ed283c45..4868b00bbe 100644 --- a/guides/source/active_support_instrumentation.md +++ b/guides/source/active_support_instrumentation.md @@ -545,6 +545,14 @@ Active Storage | `:key` | Secure token | | `:service` | Name of the service | +### service_download_chunk.active_storage + +| Key | Value | +| ------------ | ------------------------------- | +| `:key` | Secure token | +| `:service` | Name of the service | +| `:range` | Byte range attempted to be read | + ### service_download.active_storage | Key | Value | @@ -582,6 +590,23 @@ Active Storage | `:service` | Name of the service | | `:url` | Generated URL | +### service_update_metadata.active_storage + +| Key | Value | +| --------------- | ------------------------------ | +| `:key` | Secure token | +| `:service` | Name of the service | +| `:content_type` | HTTP Content-Type field | +| `:disposition` | HTTP Content-Disposition field | + +INFO. The only ActiveStorage service that provides this hook so far is GCS. + +### preview.active_storage + +| Key | Value | +| ------------ | ------------------- | +| `:key` | Secure token | + Railties -------- diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index 454613e733..d853559440 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -33,13 +33,11 @@ passing the `--skip-sprockets` option. rails new appname --skip-sprockets ``` -Rails automatically adds the `sass-rails`, `coffee-rails` and `uglifier` -gems to your `Gemfile`, which are used by Sprockets for asset compression: +Rails automatically adds the `sass-rails` gem to your `Gemfile`, which is used +by Sprockets for asset compression: ```ruby gem 'sass-rails' -gem 'uglifier' -gem 'coffee-rails' ``` Using the `--skip-sprockets` option will prevent Rails from adding @@ -176,8 +174,7 @@ in `app/assets` are never served directly in production. ### Controller Specific Assets -When you generate a scaffold or a controller, Rails also generates a JavaScript -file (or CoffeeScript file if the `coffee-rails` gem is in the `Gemfile`) and a +When you generate a scaffold or a controller, Rails also generates a Cascading Style Sheet file (or SCSS file if `sass-rails` is in the `Gemfile`) for that controller. Additionally, when generating a scaffold, Rails generates the file `scaffolds.css` (or `scaffolds.scss` if `sass-rails` is in the @@ -434,9 +431,8 @@ one file rather than many, the load time of pages can be greatly reduced because the browser makes fewer requests. Compression also reduces file size, enabling the browser to download them faster. - -For example, a new Rails application includes a default -`app/assets/javascripts/application.js` file containing the following lines: +For example, with a `app/assets/javascripts/application.js` file containing the +following lines: ```js // ... @@ -476,8 +472,7 @@ which contains these lines: */ ``` -Rails creates both `app/assets/javascripts/application.js` and -`app/assets/stylesheets/application.css` regardless of whether the +Rails create `app/assets/stylesheets/application.css` regardless of whether the --skip-sprockets option is used when creating a new Rails application. This is so you can easily add asset pipelining later if you like. @@ -517,8 +512,7 @@ The file extensions used on an asset determine what preprocessing is applied. When a controller or a scaffold is generated with the default Rails gemset, a CoffeeScript file and a SCSS file are generated in place of a regular JavaScript and CSS file. The example used before was a controller called "projects", which -generated an `app/assets/javascripts/projects.coffee` and an -`app/assets/stylesheets/projects.scss` file. +generated an `app/assets/stylesheets/projects.scss` file. In development mode, or if the asset pipeline is disabled, when these files are requested they are processed by the processors provided by the `coffee-script` @@ -1083,7 +1077,7 @@ Possible options for JavaScript compression are `:closure`, `:uglifier` and `:yui`. These require the use of the `closure-compiler`, `uglifier` or `yui-compressor` gems, respectively. -The default `Gemfile` includes [uglifier](https://github.com/lautis/uglifier). +Take the `uglifier` gem, for example. This gem wraps [UglifyJS](https://github.com/mishoo/UglifyJS) (written for NodeJS) in Ruby. It compresses your code by removing white space and comments, shortening local variable names, and performing other micro-optimizations such @@ -1230,4 +1224,3 @@ it as a preprocessor for your mime type. ```ruby Sprockets.register_preprocessor 'text/css', AddComment ``` - diff --git a/guides/source/command_line.md b/guides/source/command_line.md index a83724f1bb..4ad143d105 100644 --- a/guides/source/command_line.md +++ b/guides/source/command_line.md @@ -257,7 +257,7 @@ We will set up a simple resource called "HighScore" that will keep track of our ```bash $ rails generate scaffold HighScore game:string score:integer invoke active_record - create db/migrate/20130717151933_create_high_scores.rb + create db/migrate/20190416145729_create_high_scores.rb create app/models/high_score.rb invoke test_unit create test/models/high_score_test.rb @@ -275,20 +275,19 @@ $ rails generate scaffold HighScore game:string score:integer create app/views/high_scores/_form.html.erb invoke test_unit create test/controllers/high_scores_controller_test.rb + create test/system/high_scores_test.rb invoke helper create app/helpers/high_scores_helper.rb + invoke test_unit invoke jbuilder create app/views/high_scores/index.json.jbuilder create app/views/high_scores/show.json.jbuilder - invoke test_unit - create test/system/high_scores_test.rb + create app/views/high_scores/_high_score.json.jbuilder invoke assets - invoke coffee - create app/assets/javascripts/high_scores.coffee invoke scss create app/assets/stylesheets/high_scores.scss invoke scss - identical app/assets/stylesheets/scaffolds.scss + create app/assets/stylesheets/scaffolds.scss ``` The generator checks that there exist the directories for models, controllers, helpers, layouts, functional and unit tests, stylesheets, creates the views, controller, model and database migration for HighScore (creating the `high_scores` table and fields), takes care of the route for the **resource**, and new tests for everything. @@ -481,6 +480,22 @@ lib/school.rb: * [ 17] [FIXME] ``` +#### Tags + +You can add more default tags to search for by using `config.annotations.register_tags`. It receives a list of tags. + +```ruby +config.annotations.register_tags("DEPRECATEME", "TESTME") +``` + +```bash +$ rails notes +app/controllers/admin/users_controller.rb: + * [ 20] [TODO] do A/B testing on this + * [ 42] [TESTME] this needs more functional tests + * [132] [DEPRECATEME] ensure this method is deprecated in next release +``` + #### Directories You can add more default directories to search from by using `config.annotations.register_directories`. It receives a list of directory names. diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index c33d523c0e..f86589bdf1 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -247,7 +247,7 @@ Rails follows a simple set of coding style conventions: * Two spaces, no tabs (for indentation). * No trailing whitespace. Blank lines should not have any spaces. -* Indent after private/protected. +* Indent and no blank line after private/protected. * Use Ruby >= 1.9 syntax for hashes. Prefer `{ a: :b }` over `{ :a => :b }`. * Prefer `&&`/`||` over `and`/`or`. * Prefer class << self over self.method for class methods. diff --git a/guides/source/documents.yaml b/guides/source/documents.yaml index 25e4fdb4e6..1e67b2bce7 100644 --- a/guides/source/documents.yaml +++ b/guides/source/documents.yaml @@ -155,6 +155,11 @@ name: Using Rails for API-only Applications url: api_app.html description: This guide explains how to effectively use Rails to develop a JSON API application. + - + name: Active Record and PostgreSQL + work_in_progress: true + url: active_record_postgresql.html + description: This guide covers PostgreSQL specific usage of Active Record. - name: Extending Rails diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index b79dbdbc6f..d743c1c0d9 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -142,6 +142,10 @@ module Rails active_storage.queues.analysis = :active_storage_analysis active_storage.queues.purge = :active_storage_purge end + + if respond_to?(:active_record) + active_record.collection_cache_versioning = true + end else raise "Unknown version #{target_version.to_s.inspect}" end diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb index 193cc59f3a..9800b19274 100644 --- a/railties/lib/rails/application/default_middleware_stack.rb +++ b/railties/lib/rails/application/default_middleware_stack.rb @@ -49,6 +49,7 @@ module Rails middleware.use ::Rails::Rack::Logger, config.log_tags middleware.use ::ActionDispatch::ShowExceptions, show_exceptions_app middleware.use ::ActionDispatch::DebugExceptions, app, config.debug_exception_response_format + middleware.use ::ActionDispatch::ActionableExceptions unless config.cache_classes middleware.use ::ActionDispatch::Reloader, app.reloader diff --git a/railties/lib/rails/commands/dev/dev_command.rb b/railties/lib/rails/commands/dev/dev_command.rb index a3f02f3172..9b2cb2b04a 100644 --- a/railties/lib/rails/commands/dev/dev_command.rb +++ b/railties/lib/rails/commands/dev/dev_command.rb @@ -5,8 +5,10 @@ require "rails/dev_caching" module Rails module Command class DevCommand < Base # :nodoc: - def help - say "rails dev:cache # Toggle development mode caching on/off." + no_commands do + def help + say "rails dev:cache # Toggle development mode caching on/off." + end end def cache diff --git a/railties/lib/rails/commands/notes/notes_command.rb b/railties/lib/rails/commands/notes/notes_command.rb index 64b339b3cd..94cf183855 100644 --- a/railties/lib/rails/commands/notes/notes_command.rb +++ b/railties/lib/rails/commands/notes/notes_command.rb @@ -5,7 +5,7 @@ require "rails/source_annotation_extractor" module Rails module Command class NotesCommand < Base # :nodoc: - class_option :annotations, aliases: "-a", desc: "Filter by specific annotations, e.g. Foobar TODO", type: :array, default: %w(OPTIMIZE FIXME TODO) + class_option :annotations, aliases: "-a", desc: "Filter by specific annotations, e.g. Foobar TODO", type: :array, default: Rails::SourceAnnotationExtractor::Annotation.tags def perform(*) require_application_and_environment! diff --git a/railties/lib/rails/generators/rails/app/templates/bin/setup.tt b/railties/lib/rails/generators/rails/app/templates/bin/setup.tt index 3f73bae3da..5928deb6aa 100644 --- a/railties/lib/rails/generators/rails/app/templates/bin/setup.tt +++ b/railties/lib/rails/generators/rails/app/templates/bin/setup.tt @@ -8,7 +8,8 @@ def system!(*args) end FileUtils.chdir APP_ROOT do - # This script is a starting point to setup your application. + # This script is a way to setup or update your development environment automatically. + # This script is idempotent, so that you can run it at anytime and get an expectable outcome. # Add necessary setup steps to this file. puts '== Installing dependencies ==' @@ -27,7 +28,7 @@ FileUtils.chdir APP_ROOT do # end puts "\n== Preparing database ==" - system! 'bin/rails db:setup' + system! 'bin/rails db:prepare' <% end -%> puts "\n== Removing old logs and tempfiles ==" diff --git a/railties/lib/rails/generators/rails/app/templates/bin/update.tt b/railties/lib/rails/generators/rails/app/templates/bin/update.tt deleted file mode 100644 index 03b77d0d46..0000000000 --- a/railties/lib/rails/generators/rails/app/templates/bin/update.tt +++ /dev/null @@ -1,33 +0,0 @@ -require 'fileutils' - -# path to your application root. -APP_ROOT = File.expand_path('..', __dir__) - -def system!(*args) - system(*args) || abort("\n== Command #{args} failed ==") -end - -FileUtils.chdir APP_ROOT do - # This script is a way to update your development environment automatically. - # Add necessary update steps to this file. - - puts '== Installing dependencies ==' - system! 'gem install bundler --conservative' - system('bundle check') || system!('bundle install') -<% unless options.skip_javascript? -%> - - # Install JavaScript dependencies - # system('bin/yarn') -<% end -%> -<% unless options.skip_active_record? -%> - - puts "\n== Updating database ==" - system! 'rails db:migrate' -<% end -%> - - puts "\n== Removing old logs and tempfiles ==" - system! 'rails log:clear tmp:clear' - - puts "\n== Restarting application server ==" - system! 'rails restart' -end diff --git a/railties/lib/rails/mailers_controller.rb b/railties/lib/rails/mailers_controller.rb index 5cffa52860..4a1942790b 100644 --- a/railties/lib/rails/mailers_controller.rb +++ b/railties/lib/rails/mailers_controller.rb @@ -5,8 +5,9 @@ require "rails/application_controller" class Rails::MailersController < Rails::ApplicationController # :nodoc: prepend_view_path ActionDispatch::DebugView::RESCUES_TEMPLATE_PATH + around_action :set_locale, only: :preview + before_action :find_preview, only: :preview before_action :require_local!, unless: :show_previews? - before_action :find_preview, :set_locale, only: :preview helper_method :part_query, :locale_query @@ -92,6 +93,8 @@ class Rails::MailersController < Rails::ApplicationController # :nodoc: end def set_locale - I18n.locale = params[:locale] || I18n.default_locale + I18n.with_locale(params[:locale] || I18n.default_locale) do + yield + end end end diff --git a/railties/lib/rails/source_annotation_extractor.rb b/railties/lib/rails/source_annotation_extractor.rb index d7170e6282..9ce22b96a6 100644 --- a/railties/lib/rails/source_annotation_extractor.rb +++ b/railties/lib/rails/source_annotation_extractor.rb @@ -29,6 +29,16 @@ module Rails directories.push(*dirs) end + def self.tags + @@tags ||= %w(OPTIMIZE FIXME TODO) + end + + # Registers additional tags + # Rails::SourceAnnotationExtractor::Annotation.register_tags("TESTME", "DEPRECATEME") + def self.register_tags(*additional_tags) + tags.push(*additional_tags) + end + def self.extensions @@extensions ||= {} end @@ -66,6 +76,8 @@ module Rails # Prints all annotations with tag +tag+ under the root directories +app+, # +config+, +db+, +lib+, and +test+ (recursively). # + # If +tag+ is <tt>nil</tt>, annotations with either default or registered tags are printed. + # # Specific directories can be explicitly set using the <tt>:dirs</tt> key in +options+. # # Rails::SourceAnnotationExtractor.enumerate 'TODO|FIXME', dirs: %w(app lib), tag: true @@ -75,7 +87,8 @@ module Rails # See <tt>#find_in</tt> for a list of file extensions that will be taken into account. # # This class method is the single entry point for the `rails notes` command. - def self.enumerate(tag, options = {}) + def self.enumerate(tag = nil, options = {}) + tag ||= Annotation.tags.join("|") extractor = new(tag) dirs = options.delete(:dirs) || Annotation.directories extractor.display(extractor.find(dirs), options) diff --git a/railties/test/application/bin_setup_test.rb b/railties/test/application/bin_setup_test.rb index a952d2466b..aa0da0931d 100644 --- a/railties/test/application/bin_setup_test.rb +++ b/railties/test/application/bin_setup_test.rb @@ -6,21 +6,12 @@ module ApplicationTests class BinSetupTest < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation - def setup - build_app - end - - def teardown - teardown_app - end + setup :build_app + teardown :teardown_app def test_bin_setup Dir.chdir(app_path) do - app_file "db/schema.rb", <<-RUBY - ActiveRecord::Schema.define(version: 20140423102712) do - create_table(:articles) {} - end - RUBY + rails "generate", "model", "article" list_tables = lambda { rails("runner", "p ActiveRecord::Base.connection.tables").strip } File.write("log/test.log", "zomg!") @@ -28,15 +19,20 @@ module ApplicationTests assert_equal "[]", list_tables.call assert_equal 5, File.size("log/test.log") assert_not File.exist?("tmp/restart.txt") + `bin/setup 2>&1` assert_equal 0, File.size("log/test.log") - assert_equal '["articles", "schema_migrations", "ar_internal_metadata"]', list_tables.call + assert_equal '["schema_migrations", "ar_internal_metadata", "articles"]', list_tables.call assert File.exist?("tmp/restart.txt") end end def test_bin_setup_output Dir.chdir(app_path) do + # SQLite3 seems to auto-create the database on first checkout. + rails "db:system:change", "--to=postgresql" + rails "db:drop" + app_file "db/schema.rb", "" output = `bin/setup 2>&1` @@ -53,8 +49,8 @@ module ApplicationTests The Gemfile's dependencies are satisfied == Preparing database == - Created database 'db/development.sqlite3' - Created database 'db/test.sqlite3' + Created database 'app_development' + Created database 'app_test' == Removing old logs and tempfiles == diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index a2e3e781c0..62d9b1c813 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1721,7 +1721,7 @@ module ApplicationTests end EOS - app_file "config/initializers/autoload.rb", "D" + app_file "config/initializers/autoload.rb", "D.class" app "development" @@ -1749,7 +1749,7 @@ module ApplicationTests end EOS - app_file "config/initializers/autoload.rb", "D" + app_file "config/initializers/autoload.rb", "D.class" app "production" diff --git a/railties/test/application/mailer_previews_test.rb b/railties/test/application/mailer_previews_test.rb index fb84276b8a..fa9ed868c4 100644 --- a/railties/test/application/mailer_previews_test.rb +++ b/railties/test/application/mailer_previews_test.rb @@ -515,6 +515,13 @@ module ApplicationTests assert_match '<option selected value="locale=ja">ja', last_response.body end + test "preview does not leak I18n global setting changes" do + I18n.with_locale(:en) do + get "/rails/mailers/notifier/foo.txt?locale=ja" + assert_equal :en, I18n.locale + end + end + test "mailer previews create correct links when loaded on a subdirectory" do mailer "notifier", <<-RUBY class Notifier < ActionMailer::Base diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 4242daf39a..54b2e95d75 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -38,6 +38,7 @@ module ApplicationTests "Rails::Rack::Logger", "ActionDispatch::ShowExceptions", "ActionDispatch::DebugExceptions", + "ActionDispatch::ActionableExceptions", "ActionDispatch::Reloader", "ActionDispatch::Callbacks", "ActiveRecord::Migration::CheckPending", @@ -70,6 +71,7 @@ module ApplicationTests "Rails::Rack::Logger", "ActionDispatch::ShowExceptions", "ActionDispatch::DebugExceptions", + "ActionDispatch::ActionableExceptions", "ActionDispatch::Reloader", "ActionDispatch::Callbacks", "Rack::Head", diff --git a/railties/test/backtrace_cleaner_test.rb b/railties/test/backtrace_cleaner_test.rb index ec512b6b64..6de23acebe 100644 --- a/railties/test/backtrace_cleaner_test.rb +++ b/railties/test/backtrace_cleaner_test.rb @@ -17,6 +17,16 @@ class BacktraceCleanerTest < ActiveSupport::TestCase assert_equal 1, result.length end + test "can filter for noise" do + backtrace = [ "(irb):1", + "/Path/to/rails/railties/lib/rails/commands/console.rb:77:in `start'", + "bin/rails:4:in `<main>'" ] + result = @cleaner.clean(backtrace, :noise) + assert_equal "/Path/to/rails/railties/lib/rails/commands/console.rb:77:in `start'", result[0] + assert_equal "bin/rails:4:in `<main>'", result[1] + assert_equal 2, result.length + end + test "should omit ActionView template methods names" do method_name = ActionView::Template.new(nil, "app/views/application/index.html.erb", nil, locals: []).send :method_name backtrace = [ "app/views/application/index.html.erb:4:in `block in #{method_name}'"] diff --git a/railties/test/commands/notes_test.rb b/railties/test/commands/notes_test.rb index 147019e299..9182541413 100644 --- a/railties/test/commands/notes_test.rb +++ b/railties/test/commands/notes_test.rb @@ -121,6 +121,47 @@ class Rails::Command::NotesTest < ActiveSupport::TestCase OUTPUT end + test "displays results from additional tags added to the default tags from a config file" do + app_file "app/models/profile.rb", "# TESTME: some method to test" + app_file "app/controllers/hello_controller.rb", "# DEPRECATEME: this action is no longer needed" + app_file "db/some_seeds.rb", "# TODO: default tags such as TODO are still present" + + add_to_config 'config.annotations.register_tags "TESTME", "DEPRECATEME"' + + assert_equal <<~OUTPUT, run_notes_command + app/controllers/hello_controller.rb: + * [1] [DEPRECATEME] this action is no longer needed + + app/models/profile.rb: + * [1] [TESTME] some method to test + + db/some_seeds.rb: + * [1] [TODO] default tags such as TODO are still present + + OUTPUT + end + + test "does not display results from tags that are neither default nor registered" do + app_file "app/models/profile.rb", "# TESTME: some method to test" + app_file "app/controllers/hello_controller.rb", "# DEPRECATEME: this action is no longer needed" + app_file "db/some_seeds.rb", "# TODO: default tags such as TODO are still present" + app_file "db/some_other_seeds.rb", "# BAD: this note should not be listed" + + add_to_config 'config.annotations.register_tags "TESTME", "DEPRECATEME"' + + assert_equal <<~OUTPUT, run_notes_command + app/controllers/hello_controller.rb: + * [1] [DEPRECATEME] this action is no longer needed + + app/models/profile.rb: + * [1] [TESTME] some method to test + + db/some_seeds.rb: + * [1] [TODO] default tags such as TODO are still present + + OUTPUT + end + private def run_notes_command(args = []) rails "notes", args diff --git a/railties/test/generators/api_app_generator_test.rb b/railties/test/generators/api_app_generator_test.rb index 4b9878187b..503564beec 100644 --- a/railties/test/generators/api_app_generator_test.rb +++ b/railties/test/generators/api_app_generator_test.rb @@ -120,7 +120,6 @@ class ApiAppGeneratorTest < Rails::Generators::TestCase bin/rails bin/rake bin/setup - bin/update config/application.rb config/boot.rb config/cable.yml diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 8771b53071..5b439fdcba 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -41,7 +41,6 @@ DEFAULT_APP_FILES = %w( bin/rails bin/rake bin/setup - bin/update bin/yarn config/application.rb config/boot.rb @@ -321,10 +320,6 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_file "#{app_root}/bin/setup" do |content| assert_no_match(/system\('bin\/yarn'\)/, content) end - - assert_file "#{app_root}/bin/update" do |content| - assert_no_match(/system\('bin\/yarn'\)/, content) - end end end diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb index c97cd17ec6..f860e328f2 100644 --- a/railties/test/generators/model_generator_test.rb +++ b/railties/test/generators/model_generator_test.rb @@ -414,23 +414,12 @@ class ModelGeneratorTest < Rails::Generators::TestCase end end - def test_required_belongs_to_adds_required_association - run_generator ["account", "supplier:references{required}"] - - expected_file = <<~FILE - class Account < ApplicationRecord - belongs_to :supplier, required: true - end - FILE - assert_file "app/models/account.rb", expected_file - end - - def test_required_polymorphic_belongs_to_generages_correct_model + def test_required_polymorphic_belongs_to_generates_correct_model run_generator ["account", "supplier:references{required,polymorphic}"] expected_file = <<~FILE class Account < ApplicationRecord - belongs_to :supplier, polymorphic: true, required: true + belongs_to :supplier, polymorphic: true end FILE assert_file "app/models/account.rb", expected_file @@ -441,7 +430,7 @@ class ModelGeneratorTest < Rails::Generators::TestCase expected_file = <<~FILE class Account < ApplicationRecord - belongs_to :supplier, polymorphic: true, required: true + belongs_to :supplier, polymorphic: true end FILE assert_file "app/models/account.rb", expected_file diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index 26ce487c5f..3e8ce1c018 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -191,10 +191,7 @@ module SharedGeneratorTests assert_no_match(/fixtures :all/, helper_content) end assert_file "#{application_path}/bin/setup" do |setup_content| - assert_no_match(/db:setup/, setup_content) - end - assert_file "#{application_path}/bin/update" do |update_content| - assert_no_match(/db:migrate/, update_content) + assert_no_match(/db:prepare/, setup_content) end assert_file ".gitignore" do |content| assert_no_match(/sqlite/i, content) diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 3fcfaa9623..fab704944b 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -18,7 +18,9 @@ require "active_support/testing/method_call_assertions" require "active_support/test_case" require "minitest/retry" -Minitest::Retry.use!(verbose: false, retry_count: 1) +if ENV["BUILDKITE"] + Minitest::Retry.use!(verbose: false, retry_count: 1) +end RAILS_FRAMEWORK_ROOT = File.expand_path("../../..", __dir__) @@ -5692,11 +5692,6 @@ trim-right@^1.0.1: resolved "https://registry.yarnpkg.com/trim-right/-/trim-right-1.0.1.tgz#cb2e1203067e0c8de1f614094b9fe45704ea6003" integrity sha1-yy4SAwZ+DI3h9hQJS5/kVwTqYAM= -trix@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/trix/-/trix-1.0.0.tgz#e9cc98cf6030c908f8d54e317b5b072f927b0c6b" - integrity sha512-feli9QVXe6gzZOCUfpPGpNDURW9jMciIRVQ5gkDudOctcA1oMtI5K/qEbsL2rFCoGl1rSoeRt+HPhIFGyQscKg== - tslib@^1.9.0: version "1.9.3" resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.9.3.tgz#d7e4dd79245d85428c4d7e4822a79917954ca286" |