diff options
154 files changed, 1929 insertions, 905 deletions
diff --git a/.travis.yml b/.travis.yml index 1a3890b7d9..e357a0ca3e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -28,7 +28,6 @@ bundler_args: --without test --jobs 3 --retry 3 before_install: - "rm ${BUNDLE_GEMFILE}.lock" - "travis_retry gem update --system" - - "rvm @global do gem uninstall bundler --all --ignore-dependencies --executables" - "travis_retry gem install bundler -v 1.15.4" - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/kr/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" - "pushd /tmp/beanstalkd-1.10 && make && (./beanstalkd &); popd" @@ -80,8 +80,7 @@ group :cable do gem "hiredis", require: false gem "redis", "~> 4.0", require: false - # For Redis 4.0 support. Unreleased 9cb81bf. - gem "redis-namespace", github: "resque/redis-namespace" + gem "redis-namespace" gem "websocket-client-simple", github: "matthewd/websocket-client-simple", branch: "close-race", require: false diff --git a/Gemfile.lock b/Gemfile.lock index cc3b0508c5..d0bfe589be 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -53,13 +53,6 @@ GIT tilt (>= 1.1, < 3) GIT - remote: https://github.com/resque/redis-namespace.git - revision: 1403f511f6ae1ec9a8f330298a4cacf73fb10afc - specs: - redis-namespace (1.5.3) - redis (>= 3.0.4) - -GIT remote: https://github.com/robin850/sdoc.git revision: 0e340352f3ab2f196c8a8743f83c2ee286e4f71c branch: upgrade @@ -409,6 +402,8 @@ GEM rdoc (5.1.0) redcarpet (3.2.3) redis (4.0.1) + redis-namespace (1.6.0) + redis (>= 3.0.4) representable (3.0.4) declarative (< 0.1.0) declarative-option (< 0.2.0) @@ -563,7 +558,7 @@ DEPENDENCIES rb-inotify! redcarpet (~> 3.2.3) redis (~> 4.0) - redis-namespace! + redis-namespace resque resque-scheduler! rubocop (>= 0.47) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 32239d202c..e01f88e902 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Make `assert_recognizes` to traverse mounted engines + + *Yuichiro Kaneko* + * Remove deprecated `ActionController::ParamsParser::ParseError`. *Rafael Mendonça França* diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index d6cd5fd9e0..bd133f24a1 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -248,6 +248,7 @@ module ActionController #:nodoc: "If you know what you're doing, go ahead and disable forgery " \ "protection on this action to permit cross-origin JavaScript embedding." private_constant :CROSS_ORIGIN_JAVASCRIPT_WARNING + # :startdoc: # If `verify_authenticity_token` was run (indicating that we have # forgery protection enabled for this request) then also verify that diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 41a47f2c82..ec86b8bc47 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -9,7 +9,7 @@ module ActionDispatch # sub-hashes of the params hash to filter. Filtering only certain sub-keys # from a hash is possible by using the dot notation: 'credit_card.number'. # If a block is given, each key and value of the params hash and all - # sub-hashes is passed to it, the value or key can be replaced using + # sub-hashes is passed to it, where the value or the key can be replaced using # String#replace or similar method. # # env["action_dispatch.parameter_filter"] = [:password] @@ -48,7 +48,7 @@ module ActionDispatch @filtered_env ||= env_filter.filter(@env) end - # Reconstructed a path with all sensitive GET parameters replaced. + # Reconstructs a path with all sensitive GET parameters replaced. def filtered_path @filtered_path ||= query_string.empty? ? path : "#{path}?#{filtered_query_string}" end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 60aa1d4e8a..d631281e4b 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -204,7 +204,7 @@ module ActionDispatch # # If the env contains +rack.early_hints+ then the server accepts HTTP2 push for Link headers. # - # The +send_early_hints+ method accepts an hash of links as follows: + # The +send_early_hints+ method accepts a hash of links as follows: # # send_early_hints("Link" => "</style.css>; rel=preload; as=style\n</script.js>; rel=preload") # diff --git a/actionpack/lib/action_dispatch/routing/endpoint.rb b/actionpack/lib/action_dispatch/routing/endpoint.rb index e911b6537b..24dced1efd 100644 --- a/actionpack/lib/action_dispatch/routing/endpoint.rb +++ b/actionpack/lib/action_dispatch/routing/endpoint.rb @@ -3,10 +3,12 @@ module ActionDispatch module Routing class Endpoint # :nodoc: - def dispatcher?; false; end - def redirect?; false; end - def matches?(req); true; end - def app; self; end + def dispatcher?; false; end + def redirect?; false; end + def engine?; rack_app.respond_to?(:routes); end + def matches?(req); true; end + def app; self; end + def rack_app; app; end end end end diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index b2868b7427..a2205569b4 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -15,7 +15,7 @@ module ActionDispatch end def rack_app - app.app + app.rack_app end def path @@ -47,7 +47,7 @@ module ActionDispatch end def engine? - rack_app.respond_to?(:routes) + app.engine? end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index dea8387c3d..ded42adee9 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -475,6 +475,16 @@ module ActionDispatch # # resources :users, param: :name # + # The +users+ resource here will have the following routes generated for it: + # + # GET /users(.:format) + # POST /users(.:format) + # GET /users/new(.:format) + # GET /users/:name/edit(.:format) + # GET /users/:name(.:format) + # PATCH/PUT /users/:name(.:format) + # DELETE /users/:name(.:format) + # # You can override <tt>ActiveRecord::Base#to_param</tt> of a related # model to construct a URL: # @@ -484,8 +494,8 @@ module ActionDispatch # end # end # - # user = User.find_by(name: 'Phusion') - # user_path(user) # => "/users/Phusion" + # user = User.find_by(name: 'Phusion') + # user_path(user) # => "/users/Phusion" # # [:path] # The path prefix for the routes. @@ -1265,7 +1275,7 @@ module ActionDispatch # POST /profile # # === Options - # Takes same options as +resources+. + # Takes same options as resources[rdoc-ref:#resources] def resource(*resources, &block) options = resources.extract_options!.dup @@ -1330,7 +1340,7 @@ module ActionDispatch # DELETE /photos/:photo_id/comments/:id # # === Options - # Takes same options as <tt>Base#match</tt> as well as: + # Takes same options as match[rdoc-ref:Base#match] as well as: # # [:path_names] # Allows you to change the segment component of the +edit+ and +new+ actions. diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 71cb458112..987e709f6f 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -842,6 +842,10 @@ module ActionDispatch end req = make_request(env) + recognize_path_with_request(req, path, extras) + end + + def recognize_path_with_request(req, path, extras) @router.recognize(req) do |route, params| params.merge!(extras) params.each do |key, value| @@ -860,6 +864,9 @@ module ActionDispatch end return req.path_parameters + elsif app.matches?(req) && app.engine? + path_parameters = app.rack_app.routes.recognize_path_with_request(req, path, extras) + return path_parameters end end diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb index 770fbde74e..2687772b4b 100644 --- a/actionpack/lib/action_dispatch/system_testing/driver.rb +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -59,7 +59,7 @@ module ActionDispatch def register_webkit(app) Capybara::Webkit::Driver.new(app, Capybara::Webkit::Configuration.to_hash.merge(@options)).tap do |driver| - driver.resize_window(*@screen_size) + driver.resize_window_to(driver.current_window_handle, *@screen_size) end end diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index f0f106c8ba..be455642de 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -98,7 +98,7 @@ class ACLogSubscriberTest < ActionController::TestCase @old_logger = ActionController::Base.logger - @cache_path = File.join Dir.tmpdir, Dir::Tmpname.make_tmpname("tmp", "cache") + @cache_path = Dir.mktmpdir(%w[tmp cache]) @controller.cache_store = :file_store, @cache_path ActionController::LogSubscriber.attach_to :action_controller end diff --git a/actionpack/test/dispatch/routing_assertions_test.rb b/actionpack/test/dispatch/routing_assertions_test.rb index e492a56653..a5198f2f13 100644 --- a/actionpack/test/dispatch/routing_assertions_test.rb +++ b/actionpack/test/dispatch/routing_assertions_test.rb @@ -1,14 +1,40 @@ # frozen_string_literal: true require "abstract_unit" +require "rails/engine" require "controller/fake_controllers" class SecureArticlesController < ArticlesController; end class BlockArticlesController < ArticlesController; end class QueryArticlesController < ArticlesController; end +class SecureBooksController < BooksController; end +class BlockBooksController < BooksController; end +class QueryBooksController < BooksController; end + class RoutingAssertionsTest < ActionController::TestCase def setup + engine = Class.new(Rails::Engine) do + def self.name + "blog_engine" + end + end + engine.routes.draw do + resources :books + + scope "secure", constraints: { protocol: "https://" } do + resources :books, controller: "secure_books" + end + + scope "block", constraints: lambda { |r| r.ssl? } do + resources :books, controller: "block_books" + end + + scope "query", constraints: lambda { |r| r.params[:use_query] == "true" } do + resources :books, controller: "query_books" + end + end + @routes = ActionDispatch::Routing::RouteSet.new @routes.draw do resources :articles @@ -24,6 +50,8 @@ class RoutingAssertionsTest < ActionController::TestCase scope "query", constraints: lambda { |r| r.params[:use_query] == "true" } do resources :articles, controller: "query_articles" end + + mount engine => "/shelf" end end @@ -83,6 +111,49 @@ class RoutingAssertionsTest < ActionController::TestCase assert_match err.message, "This is a really bad msg" end + def test_assert_recognizes_with_engine + assert_recognizes({ controller: "books", action: "index" }, "/shelf/books") + assert_recognizes({ controller: "books", action: "show", id: "1" }, "/shelf/books/1") + end + + def test_assert_recognizes_with_engine_and_extras + assert_recognizes({ controller: "books", action: "index", page: "1" }, "/shelf/books", page: "1") + end + + def test_assert_recognizes_with_engine_and_method + assert_recognizes({ controller: "books", action: "create" }, { path: "/shelf/books", method: :post }) + assert_recognizes({ controller: "books", action: "update", id: "1" }, { path: "/shelf/books/1", method: :put }) + end + + def test_assert_recognizes_with_engine_and_hash_constraint + assert_raise(Assertion) do + assert_recognizes({ controller: "secure_books", action: "index" }, "http://test.host/shelf/secure/books") + end + assert_recognizes({ controller: "secure_books", action: "index", protocol: "https://" }, "https://test.host/shelf/secure/books") + end + + def test_assert_recognizes_with_engine_and_block_constraint + assert_raise(Assertion) do + assert_recognizes({ controller: "block_books", action: "index" }, "http://test.host/shelf/block/books") + end + assert_recognizes({ controller: "block_books", action: "index" }, "https://test.host/shelf/block/books") + end + + def test_assert_recognizes_with_engine_and_query_constraint + assert_raise(Assertion) do + assert_recognizes({ controller: "query_books", action: "index", use_query: "false" }, "/shelf/query/books", use_query: "false") + end + assert_recognizes({ controller: "query_books", action: "index", use_query: "true" }, "/shelf/query/books", use_query: "true") + end + + def test_assert_recognizes_raises_message_with_engine + err = assert_raise(Assertion) do + assert_recognizes({ controller: "secure_books", action: "index" }, "http://test.host/shelf/secure/books", {}, "This is a really bad msg") + end + + assert_match err.message, "This is a really bad msg" + end + def test_assert_routing assert_routing("/articles", controller: "articles", action: "index") end diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index e3227103d6..c700cb72ec 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,9 @@ +* Fix issues with `field_error_proc` wrapping `optgroup` and select divider `option`. + + Fixes #31088 + + *Matthias Neumayr* + * Remove deprecated Erubis ERB handler. *Rafael Mendonça França* diff --git a/actionview/Rakefile b/actionview/Rakefile index 9e5ef35334..8650f541b0 100644 --- a/actionview/Rakefile +++ b/actionview/Rakefile @@ -32,7 +32,7 @@ namespace :test do task :ujs do begin Dir.mkdir("log") - pid = spawn("bundle exec rackup test/ujs/config.ru -p 4567 -s puma > log/test.log 2>&1") + pid = spawn("bundle exec rackup test/ujs/config.ru -p 4567 -s puma > log/test.log 2>&1", pgroup: true) start_time = Time.now @@ -48,7 +48,7 @@ namespace :test do system("npm run lint && bundle exec ruby ../ci/qunit-selenium-runner.rb http://localhost:4567/") status = $?.exitstatus ensure - Process.kill("KILL", pid) if pid + Process.kill("KILL", -pid) if pid FileUtils.rm_rf("log") end diff --git a/actionview/lib/action_view/helpers/active_model_helper.rb b/actionview/lib/action_view/helpers/active_model_helper.rb index f1ef715710..e41a95d2ce 100644 --- a/actionview/lib/action_view/helpers/active_model_helper.rb +++ b/actionview/lib/action_view/helpers/active_model_helper.rb @@ -17,8 +17,8 @@ module ActionView end end - def content_tag(*) - error_wrapping(super) + def content_tag(type, options, *) + select_markup_helper?(type) ? super : error_wrapping(super) end def tag(type, options, *) @@ -43,6 +43,10 @@ module ActionView object.respond_to?(:errors) && object.errors.respond_to?(:[]) && error_message.present? end + def select_markup_helper?(type) + ["optgroup", "option"].include?(type) + end + def tag_generate_errors?(options) options["type"] != "hidden" end diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index 80d53c2c6e..e86e18dd78 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -115,7 +115,7 @@ module ActionView # # <option>Write</option></select> # # select_tag "people", options_from_collection_for_select(@people, "id", "name"), include_blank: true - # # => <select id="people" name="people"><option value=""></option><option value="1">David</option></select> + # # => <select id="people" name="people"><option value="" label=" "></option><option value="1">David</option></select> # # select_tag "people", options_from_collection_for_select(@people, "id", "name"), include_blank: "All" # # => <select id="people" name="people"><option value="">All</option><option value="1">David</option></select> diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index efda549c0d..9900e0cd03 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -589,10 +589,14 @@ module ActionView end def add_method_to_attributes!(html_options, method) - if method && method.to_s.downcase != "get".freeze && html_options["rel".freeze] !~ /nofollow/ - html_options["rel".freeze] = "#{html_options["rel".freeze]} nofollow".lstrip + if method && method.to_s.downcase != "get" && html_options["rel"] !~ /nofollow/ + if html_options["rel"].blank? + html_options["rel"] = "nofollow" + else + html_options["rel"] = "#{html_options["rel"]} nofollow" + end end - html_options["data-method".freeze] = method + html_options["data-method"] = method end def token_tag(token = nil, form_options: {}) diff --git a/actionview/test/template/active_model_helper_test.rb b/actionview/test/template/active_model_helper_test.rb index a929d8dc3d..36afed6dd6 100644 --- a/actionview/test/template/active_model_helper_test.rb +++ b/actionview/test/template/active_model_helper_test.rb @@ -6,7 +6,7 @@ class ActiveModelHelperTest < ActionView::TestCase tests ActionView::Helpers::ActiveModelHelper silence_warnings do - Post = Struct.new(:author_name, :body, :updated_at) do + Post = Struct.new(:author_name, :body, :category, :published, :updated_at) do include ActiveModel::Conversion include ActiveModel::Validations @@ -22,10 +22,14 @@ class ActiveModelHelperTest < ActionView::TestCase @post = Post.new @post.errors[:author_name] << "can't be empty" @post.errors[:body] << "foo" + @post.errors[:category] << "must exist" + @post.errors[:published] << "must be accepted" @post.errors[:updated_at] << "bar" @post.author_name = "" @post.body = "Back to the hill and over it again!" + @post.category = "rails" + @post.published = false @post.updated_at = Date.new(2004, 6, 15) end @@ -56,6 +60,25 @@ class ActiveModelHelperTest < ActionView::TestCase assert_dom_equal(expected_dom, select("post", "author_name", [:a, :b], prompt: "Choose one...")) end + def test_select_grouped_options_with_errors + grouped_options = [ + ["A", [["A1"], ["A2"]]], + ["B", [["B1"], ["B2"]]], + ] + + assert_dom_equal( + %(<div class="field_with_errors"><select name="post[category]" id="post_category"><optgroup label="A"><option value="A1">A1</option>\n<option value="A2">A2</option></optgroup><optgroup label="B"><option value="B1">B1</option>\n<option value="B2">B2</option></optgroup></select></div>), + select("post", "category", grouped_options) + ) + end + + def test_collection_select_with_errors + assert_dom_equal( + %(<div class="field_with_errors"><select name="post[author_name]" id="post_author_name"><option value="a">a</option>\n<option value="b">b</option></select></div>), + collection_select("post", "author_name", [:a, :b], :to_s, :to_s) + ) + end + def test_date_select_with_errors assert_dom_equal( %(<div class="field_with_errors"><select id="post_updated_at_1i" name="post[updated_at(1i)]">\n<option selected="selected" value="2004">2004</option>\n<option value="2005">2005</option>\n</select>\n<input id="post_updated_at_2i" name="post[updated_at(2i)]" type="hidden" value="6" />\n<input id="post_updated_at_3i" name="post[updated_at(3i)]" type="hidden" value="1" />\n</div>), @@ -77,6 +100,55 @@ class ActiveModelHelperTest < ActionView::TestCase ) end + def test_label_with_errors + assert_dom_equal( + %(<div class="field_with_errors"><label for="post_body">Body</label></div>), + label("post", "body") + ) + end + + def test_check_box_with_errors + assert_dom_equal( + %(<input name="post[published]" type="hidden" value="0" /><div class="field_with_errors"><input type="checkbox" value="1" name="post[published]" id="post_published" /></div>), + check_box("post", "published") + ) + end + + def test_check_boxes_with_errors + assert_dom_equal( + %(<input name="post[published]" type="hidden" value="0" /><div class="field_with_errors"><input type="checkbox" value="1" name="post[published]" id="post_published" /></div><input name="post[published]" type="hidden" value="0" /><div class="field_with_errors"><input type="checkbox" value="1" name="post[published]" id="post_published" /></div>), + check_box("post", "published") + check_box("post", "published") + ) + end + + def test_radio_button_with_errors + assert_dom_equal( + %(<div class="field_with_errors"><input type="radio" value="rails" checked="checked" name="post[category]" id="post_category_rails" /></div>), + radio_button("post", "category", "rails") + ) + end + + def test_radio_buttons_with_errors + assert_dom_equal( + %(<div class="field_with_errors"><input type="radio" value="rails" checked="checked" name="post[category]" id="post_category_rails" /></div><div class="field_with_errors"><input type="radio" value="java" name="post[category]" id="post_category_java" /></div>), + radio_button("post", "category", "rails") + radio_button("post", "category", "java") + ) + end + + def test_collection_check_boxes_with_errors + assert_dom_equal( + %(<input type="hidden" name="post[category][]" value="" /><div class="field_with_errors"><input type="checkbox" value="ruby" name="post[category][]" id="post_category_ruby" /></div><label for="post_category_ruby">ruby</label><div class="field_with_errors"><input type="checkbox" value="java" name="post[category][]" id="post_category_java" /></div><label for="post_category_java">java</label>), + collection_check_boxes("post", "category", [:ruby, :java], :to_s, :to_s) + ) + end + + def test_collection_radio_buttons_with_errors + assert_dom_equal( + %(<input type="hidden" name="post[category]" value="" /><div class="field_with_errors"><input type="radio" value="ruby" name="post[category]" id="post_category_ruby" /></div><label for="post_category_ruby">ruby</label><div class="field_with_errors"><input type="radio" value="java" name="post[category]" id="post_category_java" /></div><label for="post_category_java">java</label>), + collection_radio_buttons("post", "category", [:ruby, :java], :to_s, :to_s) + ) + end + def test_hidden_field_does_not_render_errors assert_dom_equal( %(<input id="post_author_name" name="post[author_name]" type="hidden" value="" />), diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb index a66db2f3dc..f0eed1e290 100644 --- a/actionview/test/template/form_options_helper_test.rb +++ b/actionview/test/template/form_options_helper_test.rb @@ -1251,6 +1251,25 @@ class FormOptionsHelperTest < ActionView::TestCase html end + def test_time_zone_select_with_priority_zones_and_errors + @firm = Firm.new("D") + @firm.extend ActiveModel::Validations + @firm.errors[:time_zone] << "invalid" + zones = [ ActiveSupport::TimeZone.new("A"), ActiveSupport::TimeZone.new("D") ] + html = time_zone_select("firm", "time_zone", zones) + assert_dom_equal "<div class=\"field_with_errors\">" \ + "<select id=\"firm_time_zone\" name=\"firm[time_zone]\">" \ + "<option value=\"A\">A</option>\n" \ + "<option value=\"D\" selected=\"selected\">D</option>" \ + "<option value=\"\" disabled=\"disabled\">-------------</option>\n" \ + "<option value=\"B\">B</option>\n" \ + "<option value=\"C\">C</option>\n" \ + "<option value=\"E\">E</option>" \ + "</select>" \ + "</div>", + html + end + def test_time_zone_select_with_default_time_zone_and_nil_value @firm = Firm.new() @firm.time_zone = nil diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index a4d89ba0d1..7f4fd25573 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -30,7 +30,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase ActiveSupport::LogSubscriber.log_subscribers.clear # We need to undef `root`, RenderTestCases don't want this to be defined - Rails.instance_eval { undef :root } if @defined_root + Rails.instance_eval { undef :root } if defined?(@defined_root) end def set_logger(logger) diff --git a/activejob/test/support/integration/dummy_app_template.rb b/activejob/test/support/integration/dummy_app_template.rb index ac382bd1b7..7ea78c3350 100644 --- a/activejob/test/support/integration/dummy_app_template.rb +++ b/activejob/test/support/integration/dummy_app_template.rb @@ -4,8 +4,6 @@ if ENV["AJ_ADAPTER"] == "delayed_job" generate "delayed_job:active_record", "--quiet" end -rails_command("db:migrate") - initializer "activejob.rb", <<-CODE require "#{File.expand_path("jobs_manager.rb", __dir__)}" JobsManager.current_manager.setup diff --git a/activejob/test/support/integration/helper.rb b/activejob/test/support/integration/helper.rb index a058da141f..a02d874e2e 100644 --- a/activejob/test/support/integration/helper.rb +++ b/activejob/test/support/integration/helper.rb @@ -18,6 +18,7 @@ Rails::Generators::AppGenerator.start args require "#{dummy_app_path}/config/environment.rb" ActiveRecord::Migrator.migrations_paths = [ Rails.root.join("db/migrate").to_s ] +ActiveRecord::Tasks::DatabaseTasks.migrate require "rails/test_help" Rails.backtrace_cleaner.remove_silencers! diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 82e3a7f4dd..794744c646 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,7 @@ +* Execute `ConfirmationValidator` validation when `_confirmation`'s value is `false`. + + *bogdanvlviv* + * Allow passing a Proc or Symbol to length validator options. *Matt Rohrer* diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb index a97d7989be..faecd6b96f 100644 --- a/activemodel/lib/active_model.rb +++ b/activemodel/lib/active_model.rb @@ -30,6 +30,8 @@ require "active_model/version" module ActiveModel extend ActiveSupport::Autoload + autoload :Attribute + autoload :Attributes autoload :AttributeAssignment autoload :AttributeMethods autoload :BlockValidator, "active_model/validator" diff --git a/activerecord/lib/active_record/attribute.rb b/activemodel/lib/active_model/attribute.rb index fc474edc15..43130c37c5 100644 --- a/activerecord/lib/active_record/attribute.rb +++ b/activemodel/lib/active_model/attribute.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module ActiveRecord +module ActiveModel class Attribute # :nodoc: class << self def from_database(name, value, type) @@ -130,8 +130,6 @@ module ActiveRecord coder["value"] = value if defined?(@value) end - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :original_attribute @@ -237,6 +235,7 @@ module ActiveRecord self.class.new(name, type) end end + private_constant :FromDatabase, :FromUser, :Null, :Uninitialized, :WithCastValue end end diff --git a/activerecord/lib/active_record/attribute/user_provided_default.rb b/activemodel/lib/active_model/attribute/user_provided_default.rb index f746960fac..f274b687d4 100644 --- a/activerecord/lib/active_record/attribute/user_provided_default.rb +++ b/activemodel/lib/active_model/attribute/user_provided_default.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require "active_record/attribute" +require "active_model/attribute" -module ActiveRecord +module ActiveModel class Attribute # :nodoc: class UserProvidedDefault < FromUser # :nodoc: def initialize(name, value, type, database_default) @@ -22,8 +22,6 @@ module ActiveRecord self.class.new(name, user_provided_value, type, original_attribute) end - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :user_provided_value diff --git a/activerecord/lib/active_record/attribute_mutation_tracker.rb b/activemodel/lib/active_model/attribute_mutation_tracker.rb index 94bf641a5d..9072460124 100644 --- a/activerecord/lib/active_record/attribute_mutation_tracker.rb +++ b/activemodel/lib/active_model/attribute_mutation_tracker.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module ActiveRecord +module ActiveModel class AttributeMutationTracker # :nodoc: OPTION_NOT_GIVEN = Object.new @@ -107,5 +107,8 @@ module ActiveRecord def original_value(*) end + + def force_change(*) + end end end diff --git a/activerecord/lib/active_record/attribute_set.rb b/activemodel/lib/active_model/attribute_set.rb index 9785666e77..a892accbc6 100644 --- a/activerecord/lib/active_record/attribute_set.rb +++ b/activemodel/lib/active_model/attribute_set.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true -require "active_record/attribute_set/builder" -require "active_record/attribute_set/yaml_encoder" +require "active_model/attribute_set/builder" +require "active_model/attribute_set/yaml_encoder" -module ActiveRecord +module ActiveModel class AttributeSet # :nodoc: delegate :each_value, :fetch, to: :attributes diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activemodel/lib/active_model/attribute_set/builder.rb index 349cc7e403..f94f47370f 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activemodel/lib/active_model/attribute_set/builder.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require "active_record/attribute" +require "active_model/attribute" -module ActiveRecord +module ActiveModel class AttributeSet # :nodoc: class Builder # :nodoc: attr_reader :types, :always_initialized, :default @@ -92,8 +92,6 @@ module ActiveRecord @materialized = true end - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :types, :values, :additional_types, :delegate_hash, :default diff --git a/activerecord/lib/active_record/attribute_set/yaml_encoder.rb b/activemodel/lib/active_model/attribute_set/yaml_encoder.rb index 9254ce16ab..4ea945b956 100644 --- a/activerecord/lib/active_record/attribute_set/yaml_encoder.rb +++ b/activemodel/lib/active_model/attribute_set/yaml_encoder.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true -module ActiveRecord +module ActiveModel class AttributeSet # Attempts to do more intelligent YAML dumping of an - # ActiveRecord::AttributeSet to reduce the size of the resulting string + # ActiveModel::AttributeSet to reduce the size of the resulting string class YAMLEncoder # :nodoc: def initialize(default_types) @default_types = default_types @@ -33,8 +33,6 @@ module ActiveRecord end end - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :default_types diff --git a/activemodel/lib/active_model/attributes.rb b/activemodel/lib/active_model/attributes.rb index 3e34d3b83a..13cad87875 100644 --- a/activemodel/lib/active_model/attributes.rb +++ b/activemodel/lib/active_model/attributes.rb @@ -1,24 +1,30 @@ # frozen_string_literal: true +require "active_support/core_ext/object/deep_dup" require "active_model/type" +require "active_model/attribute_set" +require "active_model/attribute/user_provided_default" module ActiveModel module Attributes #:nodoc: extend ActiveSupport::Concern include ActiveModel::AttributeMethods - include ActiveModel::Dirty included do attribute_method_suffix "=" class_attribute :attribute_types, :_default_attributes, instance_accessor: false - self.attribute_types = {} - self._default_attributes = {} + self.attribute_types = Hash.new(Type.default_value) + self._default_attributes = AttributeSet.new({}) end module ClassMethods - def attribute(name, cast_type = Type::Value.new, **options) - self.attribute_types = attribute_types.merge(name.to_s => cast_type) - self._default_attributes = _default_attributes.merge(name.to_s => options[:default]) + def attribute(name, type = Type::Value.new, **options) + name = name.to_s + if type.is_a?(Symbol) + type = ActiveModel::Type.lookup(type, **options.except(:default)) + end + self.attribute_types = attribute_types.merge(name => type) + define_default_attribute(name, options.fetch(:default, NO_DEFAULT_PROVIDED), type) define_attribute_methods(name) end @@ -37,11 +43,29 @@ module ActiveModel undef_method :__temp__#{safe_name}= STR end + + NO_DEFAULT_PROVIDED = Object.new # :nodoc: + private_constant :NO_DEFAULT_PROVIDED + + def define_default_attribute(name, value, type) + self._default_attributes = _default_attributes.deep_dup + if value == NO_DEFAULT_PROVIDED + default_attribute = _default_attributes[name].with_type(type) + else + default_attribute = Attribute::UserProvidedDefault.new( + name, + value, + type, + _default_attributes.fetch(name.to_s) { nil }, + ) + end + _default_attributes[name] = default_attribute + end end def initialize(*) + @attributes = self.class._default_attributes.deep_dup super - clear_changes_information end private @@ -53,21 +77,17 @@ module ActiveModel attr_name.to_s end - cast_type = self.class.attribute_types[name] - - deserialized_value = ActiveModel::Type.lookup(cast_type).cast(value) - attribute_will_change!(name) unless deserialized_value == attribute(name) - instance_variable_set("@#{name}", deserialized_value) - deserialized_value + @attributes.write_from_user(attr_name.to_s, value) + value end - def attribute(name) - if instance_variable_defined?("@#{name}") - instance_variable_get("@#{name}") + def attribute(attr_name) + name = if self.class.attribute_alias?(attr_name) + self.class.attribute_alias(attr_name).to_s else - default = self.class._default_attributes[name] - default.respond_to?(:call) ? default.call : default + attr_name.to_s end + @attributes.fetch_value(name) end # Handle *= for method_missing. diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index 943db0ab52..ddd93e34a6 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -2,6 +2,7 @@ require "active_support/hash_with_indifferent_access" require "active_support/core_ext/object/duplicable" +require "active_model/attribute_mutation_tracker" module ActiveModel # == Active \Model \Dirty @@ -130,6 +131,24 @@ module ActiveModel attribute_method_affix prefix: "restore_", suffix: "!" end + def initialize_dup(other) # :nodoc: + super + if self.class.respond_to?(:_default_attributes) + @attributes = self.class._default_attributes.map do |attr| + attr.with_value_from_user(@attributes.fetch_value(attr.name)) + end + end + @mutations_from_database = nil + end + + def changes_applied # :nodoc: + @previously_changed = changes + @mutations_before_last_save = mutations_from_database + @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new + forget_attribute_assignments + @mutations_from_database = nil + end + # Returns +true+ if any of the attributes have unsaved changes, +false+ otherwise. # # person.changed? # => false @@ -148,6 +167,60 @@ module ActiveModel changed_attributes.keys end + # Handles <tt>*_changed?</tt> for +method_missing+. + def attribute_changed?(attr, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) # :nodoc: + !!changes_include?(attr) && + (to == OPTION_NOT_GIVEN || to == _read_attribute(attr)) && + (from == OPTION_NOT_GIVEN || from == changed_attributes[attr]) + end + + # Handles <tt>*_was</tt> for +method_missing+. + def attribute_was(attr) # :nodoc: + attribute_changed?(attr) ? changed_attributes[attr] : _read_attribute(attr) + end + + # Handles <tt>*_previously_changed?</tt> for +method_missing+. + def attribute_previously_changed?(attr) #:nodoc: + previous_changes_include?(attr) + end + + # Restore all previous data of the provided attributes. + def restore_attributes(attributes = changed) + attributes.each { |attr| restore_attribute! attr } + end + + # Clears all dirty data: current changes and previous changes. + def clear_changes_information + @previously_changed = ActiveSupport::HashWithIndifferentAccess.new + @mutations_before_last_save = nil + @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new + forget_attribute_assignments + @mutations_from_database = nil + end + + def clear_attribute_changes(attr_names) + attributes_changed_by_setter.except!(*attr_names) + attr_names.each do |attr_name| + clear_attribute_change(attr_name) + end + end + + # Returns a hash of the attributes with unsaved changes indicating their original + # values like <tt>attr => original value</tt>. + # + # person.name # => "bob" + # person.name = 'robert' + # person.changed_attributes # => {"name" => "bob"} + def changed_attributes + # This should only be set by methods which will call changed_attributes + # multiple times when it is known that the computed value cannot change. + if defined?(@cached_changed_attributes) + @cached_changed_attributes + else + attributes_changed_by_setter.reverse_merge(mutations_from_database.changed_values).freeze + end + end + # Returns a hash of changed attributes indicating their original # and new values like <tt>attr => [original value, new value]</tt>. # @@ -155,7 +228,9 @@ module ActiveModel # person.name = 'bob' # person.changes # => { "name" => ["bill", "bob"] } def changes - ActiveSupport::HashWithIndifferentAccess[changed.map { |attr| [attr, attribute_change(attr)] }] + cache_changed_attributes do + ActiveSupport::HashWithIndifferentAccess[changed.map { |attr| [attr, attribute_change(attr)] }] + end end # Returns a hash of attributes that were changed before the model was saved. @@ -166,45 +241,51 @@ module ActiveModel # person.previous_changes # => {"name" => ["bob", "robert"]} def previous_changes @previously_changed ||= ActiveSupport::HashWithIndifferentAccess.new + @previously_changed.merge(mutations_before_last_save.changes) end - # Returns a hash of the attributes with unsaved changes indicating their original - # values like <tt>attr => original value</tt>. - # - # person.name # => "bob" - # person.name = 'robert' - # person.changed_attributes # => {"name" => "bob"} - def changed_attributes - @changed_attributes ||= ActiveSupport::HashWithIndifferentAccess.new + def attribute_changed_in_place?(attr_name) # :nodoc: + mutations_from_database.changed_in_place?(attr_name) end - # Handles <tt>*_changed?</tt> for +method_missing+. - def attribute_changed?(attr, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) # :nodoc: - !!changes_include?(attr) && - (to == OPTION_NOT_GIVEN || to == _read_attribute(attr)) && - (from == OPTION_NOT_GIVEN || from == changed_attributes[attr]) - end + private + def clear_attribute_change(attr_name) + mutations_from_database.forget_change(attr_name) + end - # Handles <tt>*_was</tt> for +method_missing+. - def attribute_was(attr) # :nodoc: - attribute_changed?(attr) ? changed_attributes[attr] : _read_attribute(attr) - end + def mutations_from_database + unless defined?(@mutations_from_database) + @mutations_from_database = nil + end + @mutations_from_database ||= if @attributes + ActiveModel::AttributeMutationTracker.new(@attributes) + else + NullMutationTracker.instance + end + end - # Handles <tt>*_previously_changed?</tt> for +method_missing+. - def attribute_previously_changed?(attr) #:nodoc: - previous_changes_include?(attr) - end + def forget_attribute_assignments + @attributes = @attributes.map(&:forgetting_assignment) if @attributes + end - # Restore all previous data of the provided attributes. - def restore_attributes(attributes = changed) - attributes.each { |attr| restore_attribute! attr } - end + def mutations_before_last_save + @mutations_before_last_save ||= ActiveModel::NullMutationTracker.instance + end - private + def cache_changed_attributes + @cached_changed_attributes = changed_attributes + yield + ensure + clear_changed_attributes_cache + end + + def clear_changed_attributes_cache + remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes) + end # Returns +true+ if attr_name is changed, +false+ otherwise. def changes_include?(attr_name) - attributes_changed_by_setter.include?(attr_name) + attributes_changed_by_setter.include?(attr_name) || mutations_from_database.changed?(attr_name) end alias attribute_changed_by_setter? changes_include? @@ -214,18 +295,6 @@ module ActiveModel previous_changes.include?(attr_name) end - # Removes current changes and makes them accessible through +previous_changes+. - def changes_applied # :doc: - @previously_changed = changes - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - end - - # Clears all dirty data: current changes and previous changes. - def clear_changes_information # :doc: - @previously_changed = ActiveSupport::HashWithIndifferentAccess.new - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - end - # Handles <tt>*_change</tt> for +method_missing+. def attribute_change(attr) [changed_attributes[attr], _read_attribute(attr)] if attribute_changed?(attr) @@ -238,15 +307,16 @@ module ActiveModel # Handles <tt>*_will_change!</tt> for +method_missing+. def attribute_will_change!(attr) - return if attribute_changed?(attr) + unless attribute_changed?(attr) + begin + value = _read_attribute(attr) + value = value.duplicable? ? value.clone : value + rescue TypeError, NoMethodError + end - begin - value = _read_attribute(attr) - value = value.duplicable? ? value.clone : value - rescue TypeError, NoMethodError + set_attribute_was(attr, value) end - - set_attribute_was(attr, value) + mutations_from_database.force_change(attr) end # Handles <tt>restore_*!</tt> for +method_missing+. @@ -257,18 +327,13 @@ module ActiveModel end end - # This is necessary because `changed_attributes` might be overridden in - # other implementations (e.g. in `ActiveRecord`) - alias_method :attributes_changed_by_setter, :changed_attributes # :nodoc: + def attributes_changed_by_setter + @attributes_changed_by_setter ||= ActiveSupport::HashWithIndifferentAccess.new + end # Force an attribute to have a particular "before" value def set_attribute_was(attr, old_value) attributes_changed_by_setter[attr] = old_value end - - # Remove changes information for the provided attributes. - def clear_attribute_changes(attributes) # :doc: - attributes_changed_by_setter.except!(*attributes) - end end end diff --git a/activemodel/lib/active_model/type.rb b/activemodel/lib/active_model/type.rb index b0ed67f28e..39324999c9 100644 --- a/activemodel/lib/active_model/type.rb +++ b/activemodel/lib/active_model/type.rb @@ -32,6 +32,10 @@ module ActiveModel def lookup(*args, **kwargs) # :nodoc: registry.lookup(*args, **kwargs) end + + def default_value # :nodoc: + @default_value ||= Value.new + end end register(:big_integer, Type::BigInteger) diff --git a/activemodel/lib/active_model/validations/confirmation.rb b/activemodel/lib/active_model/validations/confirmation.rb index 0abec56b68..1b5d5b09ab 100644 --- a/activemodel/lib/active_model/validations/confirmation.rb +++ b/activemodel/lib/active_model/validations/confirmation.rb @@ -9,7 +9,7 @@ module ActiveModel end def validate_each(record, attribute, value) - if (confirmed = record.send("#{attribute}_confirmation")) + unless (confirmed = record.send("#{attribute}_confirmation")).nil? unless confirmation_value_equal?(record, attribute, value, confirmed) human_attribute_name = record.class.human_attribute_name(attribute) record.errors.add(:"#{attribute}_confirmation", :confirmation, options.except(:case_sensitive).merge!(attribute: human_attribute_name)) diff --git a/activerecord/test/cases/attribute_set_test.rb b/activemodel/test/cases/attribute_set_test.rb index 8be77ed88f..d50e6cfa7a 100644 --- a/activerecord/test/cases/attribute_set_test.rb +++ b/activemodel/test/cases/attribute_set_test.rb @@ -2,8 +2,8 @@ require "cases/helper" -module ActiveRecord - class AttributeSetTest < ActiveRecord::TestCase +module ActiveModel + class AttributeSetTest < ActiveModel::TestCase test "building a new set from raw attributes" do builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Float.new) attributes = builder.build_from_database(foo: "1.1", bar: "2.2") diff --git a/activerecord/test/cases/attribute_test.rb b/activemodel/test/cases/attribute_test.rb index 1731e7926e..14d86cef97 100644 --- a/activerecord/test/cases/attribute_test.rb +++ b/activemodel/test/cases/attribute_test.rb @@ -2,8 +2,8 @@ require "cases/helper" -module ActiveRecord - class AttributeTest < ActiveRecord::TestCase +module ActiveModel + class AttributeTest < ActiveModel::TestCase setup do @type = Minitest::Mock.new end diff --git a/activemodel/test/cases/attributes_dirty_test.rb b/activemodel/test/cases/attributes_dirty_test.rb index 26b0e85db3..83a86371e0 100644 --- a/activemodel/test/cases/attributes_dirty_test.rb +++ b/activemodel/test/cases/attributes_dirty_test.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true require "cases/helper" -require "active_model/attributes" class AttributesDirtyTest < ActiveModel::TestCase class DirtyModel include ActiveModel::Model include ActiveModel::Attributes + include ActiveModel::Dirty attribute :name, :string attribute :color, :string attribute :size, :integer @@ -69,12 +69,10 @@ class AttributesDirtyTest < ActiveModel::TestCase end test "attribute mutation" do - @model.instance_variable_set("@name", "Yam".dup) + @model.name = "Yam" + @model.save assert !@model.name_changed? @model.name.replace("Hadad") - assert !@model.name_changed? - @model.name_will_change! - @model.name.replace("Baal") assert @model.name_changed? end @@ -190,4 +188,18 @@ class AttributesDirtyTest < ActiveModel::TestCase assert_equal "Dmitry", @model.name assert_equal "White", @model.color end + + test "changing the attribute reports a change only when the cast value changes" do + @model.size = "2.3" + @model.save + @model.size = "2.1" + + assert_equal false, @model.changed? + + @model.size = "5.1" + + assert_equal true, @model.changed? + assert_equal true, @model.size_changed? + assert_equal({ "size" => [2, 5] }, @model.changes) + end end diff --git a/activemodel/test/cases/attributes_test.rb b/activemodel/test/cases/attributes_test.rb index 064cba40e3..914aee1ac0 100644 --- a/activemodel/test/cases/attributes_test.rb +++ b/activemodel/test/cases/attributes_test.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "cases/helper" -require "active_model/attributes" module ActiveModel class AttributesTest < ActiveModel::TestCase @@ -13,7 +12,7 @@ module ActiveModel attribute :string_field, :string attribute :decimal_field, :decimal attribute :string_with_default, :string, default: "default string" - attribute :date_field, :string, default: -> { Date.new(2016, 1, 1) } + attribute :date_field, :date, default: -> { Date.new(2016, 1, 1) } attribute :boolean_field, :boolean end @@ -48,31 +47,6 @@ module ActiveModel assert_equal true, data.boolean_field end - test "dirty" do - data = ModelForAttributesTest.new( - integer_field: "2.3", - string_field: "Rails FTW", - decimal_field: "12.3", - boolean_field: "0" - ) - - assert_equal false, data.changed? - - data.integer_field = "2.1" - - assert_equal false, data.changed? - - data.string_with_default = "default string" - - assert_equal false, data.changed? - - data.integer_field = "5.1" - - assert_equal true, data.changed? - assert_equal true, data.integer_field_changed? - assert_equal({ "integer_field" => [2, 5] }, data.changes) - end - test "nonexistent attribute" do assert_raise ActiveModel::UnknownAttributeError do ModelForAttributesTest.new(nonexistent: "nonexistent") diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb index 2cd9e185e6..dfe041ff50 100644 --- a/activemodel/test/cases/dirty_test.rb +++ b/activemodel/test/cases/dirty_test.rb @@ -219,4 +219,8 @@ class DirtyTest < ActiveModel::TestCase assert_equal "Dmitry", @model.name assert_equal "White", @model.color end + + test "model can be dup-ed without Attributes" do + assert @model.dup + end end diff --git a/activemodel/test/cases/validations/conditional_validation_test.rb b/activemodel/test/cases/validations/conditional_validation_test.rb index aa027c4128..caea8b65ef 100644 --- a/activemodel/test/cases/validations/conditional_validation_test.rb +++ b/activemodel/test/cases/validations/conditional_validation_test.rb @@ -18,6 +18,22 @@ class ConditionalValidationTest < ActiveModel::TestCase assert_equal ["hoo 5"], t.errors["title"] end + def test_if_validation_using_array_of_true_methods + Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", if: [:condition_is_true, :condition_is_true]) + t = Topic.new("title" => "uhohuhoh", "content" => "whatever") + assert t.invalid? + assert t.errors[:title].any? + assert_equal ["hoo 5"], t.errors["title"] + end + + def test_unless_validation_using_array_of_false_methods + Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", unless: [:condition_is_false, :condition_is_false]) + t = Topic.new("title" => "uhohuhoh", "content" => "whatever") + assert t.invalid? + assert t.errors[:title].any? + assert_equal ["hoo 5"], t.errors["title"] + end + def test_unless_validation_using_method_true # When the method returns true Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", unless: :condition_is_true) @@ -26,9 +42,23 @@ class ConditionalValidationTest < ActiveModel::TestCase assert_empty t.errors[:title] end + def test_if_validation_using_array_of_true_and_false_methods + Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", if: [:condition_is_true, :condition_is_false]) + t = Topic.new("title" => "uhohuhoh", "content" => "whatever") + assert t.valid? + assert_empty t.errors[:title] + end + + def test_unless_validation_using_array_of_true_and_felse_methods + Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", unless: [:condition_is_true, :condition_is_false]) + t = Topic.new("title" => "uhohuhoh", "content" => "whatever") + assert t.valid? + assert_empty t.errors[:title] + end + def test_if_validation_using_method_false # When the method returns false - Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", if: :condition_is_true_but_its_not) + Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", if: :condition_is_false) t = Topic.new("title" => "uhohuhoh", "content" => "whatever") assert t.valid? assert_empty t.errors[:title] @@ -36,7 +66,7 @@ class ConditionalValidationTest < ActiveModel::TestCase def test_unless_validation_using_method_false # When the method returns false - Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", unless: :condition_is_true_but_its_not) + Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", unless: :condition_is_false) t = Topic.new("title" => "uhohuhoh", "content" => "whatever") assert t.invalid? assert t.errors[:title].any? @@ -80,4 +110,19 @@ class ConditionalValidationTest < ActiveModel::TestCase assert t.errors[:title].any? assert_equal ["hoo 5"], t.errors["title"] end + + def test_validation_using_conbining_if_true_and_unless_true_conditions + Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", if: :condition_is_true, unless: :condition_is_true) + t = Topic.new("title" => "uhohuhoh", "content" => "whatever") + assert t.valid? + assert_empty t.errors[:title] + end + + def test_validation_using_conbining_if_true_and_unless_false_conditions + Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", if: :condition_is_true, unless: :condition_is_false) + t = Topic.new("title" => "uhohuhoh", "content" => "whatever") + assert t.invalid? + assert t.errors[:title].any? + assert_equal ["hoo 5"], t.errors["title"] + end end diff --git a/activemodel/test/cases/validations/confirmation_validation_test.rb b/activemodel/test/cases/validations/confirmation_validation_test.rb index e84415a868..8b2c65289b 100644 --- a/activemodel/test/cases/validations/confirmation_validation_test.rb +++ b/activemodel/test/cases/validations/confirmation_validation_test.rb @@ -37,6 +37,19 @@ class ConfirmationValidationTest < ActiveModel::TestCase assert t.valid? end + def test_validates_confirmation_of_with_boolean_attribute + Topic.validates_confirmation_of(:approved) + + t = Topic.new(approved: true, approved_confirmation: nil) + assert t.valid? + + t.approved_confirmation = false + assert t.invalid? + + t.approved_confirmation = true + assert t.valid? + end + def test_validates_confirmation_of_for_ruby_class Person.validates_confirmation_of :karma diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index 001815e28f..fbbaf8a30c 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -59,7 +59,7 @@ class NumericalityValidationTest < ActiveModel::TestCase end def test_validates_numericality_of_with_integer_only_and_symbol_as_value - Topic.validates_numericality_of :approved, only_integer: :condition_is_true_but_its_not + Topic.validates_numericality_of :approved, only_integer: :condition_is_false invalid!(NIL + BLANK + JUNK) valid!(FLOATS + INTEGERS + BIGDECIMAL + INFINITY) diff --git a/activemodel/test/cases/validations/validates_test.rb b/activemodel/test/cases/validations/validates_test.rb index 77cb8ebdc1..7f32f5dc74 100644 --- a/activemodel/test/cases/validations/validates_test.rb +++ b/activemodel/test/cases/validations/validates_test.rb @@ -62,17 +62,23 @@ class ValidatesTest < ActiveModel::TestCase end def test_validates_with_if_as_local_conditions - Person.validates :karma, presence: true, email: { unless: :condition_is_true } + Person.validates :karma, presence: true, email: { if: :condition_is_false } person = Person.new person.valid? assert_equal ["can't be blank"], person.errors[:karma] end def test_validates_with_if_as_shared_conditions - Person.validates :karma, presence: true, email: true, if: :condition_is_true + Person.validates :karma, presence: true, email: true, if: :condition_is_false + person = Person.new + assert person.valid? + end + + def test_validates_with_unless_as_local_conditions + Person.validates :karma, presence: true, email: { unless: :condition_is_true } person = Person.new person.valid? - assert_equal ["can't be blank", "is not an email"], person.errors[:karma].sort + assert_equal ["can't be blank"], person.errors[:karma] end def test_validates_with_unless_shared_conditions diff --git a/activemodel/test/models/person.rb b/activemodel/test/models/person.rb index b61fdf76b1..8dd8ceadad 100644 --- a/activemodel/test/models/person.rb +++ b/activemodel/test/models/person.rb @@ -9,6 +9,10 @@ class Person def condition_is_true true end + + def condition_is_false + false + end end class Person::Gender diff --git a/activemodel/test/models/topic.rb b/activemodel/test/models/topic.rb index 2f4e92c3b2..b0af00ee45 100644 --- a/activemodel/test/models/topic.rb +++ b/activemodel/test/models/topic.rb @@ -23,7 +23,7 @@ class Topic true end - def condition_is_true_but_its_not + def condition_is_false false end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4aa7ecfc7e..e6a41ec281 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* Fix `bin/rails db:migrate` with specified `VERSION`. + `bin/rails db:migrate` with empty VERSION behaves as without `VERSION`. + Check a format of `VERSION`: Allow a migration version number + or name of a migration file. Raise error if format of `VERSION` is invalid. + Raise error if target migration doesn't exist. + + *bogdanvlviv* + * Fixed a bug where column orders for an index weren't written to db/schema.rb when using the sqlite adapter. diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index bf6dfd46e1..0e036f05f5 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -27,14 +27,14 @@ require "active_support" require "active_support/rails" require "active_model" require "arel" +require "yaml" require "active_record/version" -require "active_record/attribute_set" +require "active_model/attribute_set" module ActiveRecord extend ActiveSupport::Autoload - autoload :Attribute autoload :Base autoload :Callbacks autoload :Core @@ -181,3 +181,7 @@ end ActiveSupport.on_load(:i18n) do I18n.load_path << File.expand_path("active_record/locale/en.yml", __dir__) end + +YAML.load_tags["!ruby/object:ActiveRecord::AttributeSet"] = "ActiveModel::AttributeSet" +YAML.load_tags["!ruby/object:ActiveRecord::Attribute::FromDatabase"] = "ActiveModel::Attribute::FromDatabase" +YAML.load_tags["!ruby/object:ActiveRecord::LazyAttributeHash"] = "ActiveModel::LazyAttributeHash" diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index bc7e288500..661605d3e5 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -140,26 +140,6 @@ module ActiveRecord class HasOneThroughCantAssociateThroughHasOneOrManyReflection < ThroughCantAssociateThroughHasOneOrManyReflection #:nodoc: end - class HasManyThroughCantAssociateNewRecords < ActiveRecordError #:nodoc: - def initialize(owner = nil, reflection = nil) - if owner && reflection - super("Cannot associate new records through '#{owner.class.name}##{reflection.name}' on '#{reflection.source_reflection.class_name rescue nil}##{reflection.source_reflection.name rescue nil}'. Both records must have an id in order to create the has_many :through record associating them.") - else - super("Cannot associate new records.") - end - end - end - - class HasManyThroughCantDissociateNewRecords < ActiveRecordError #:nodoc: - def initialize(owner = nil, reflection = nil) - if owner && reflection - super("Cannot dissociate new records through '#{owner.class.name}##{reflection.name}' on '#{reflection.source_reflection.class_name rescue nil}##{reflection.source_reflection.name rescue nil}'. Both records must have an id in order to delete the has_many :through record associating them.") - else - super("Cannot dissociate new records.") - end - end - end - class ThroughNestedAssociationsAreReadonly < ActiveRecordError #:nodoc: def initialize(owner = nil, reflection = nil) if owner && reflection @@ -189,16 +169,6 @@ module ActiveRecord end end - class ReadOnlyAssociation < ActiveRecordError #:nodoc: - def initialize(reflection = nil) - if reflection - super("Cannot add to a has_many :through association. Try adding to #{reflection.through_reflection.name.inspect}.") - else - super("Read-only reflection error.") - end - end - end - # This error is raised when trying to destroy a parent instance in N:1 or 1:1 associations # (has_many, has_one) when there is at least 1 child associated instance. # ex: if @project.tasks.size > 0, DeleteRestrictionError will be raised when trying to destroy @project diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index e1754d4a19..5a93a89d0a 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -166,8 +166,6 @@ module ActiveRecord end class AlreadyLoaded # :nodoc: - attr_reader :owners, :reflection - def initialize(klass, owners, reflection, preload_scope) @owners = owners @reflection = reflection @@ -178,6 +176,9 @@ module ActiveRecord def preloaded_records owners.flat_map { |owner| owner.association(reflection.name).target } end + + protected + attr_reader :owners, :reflection end # Returns a class containing the logic needed to load preload the data diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 607d376a08..19c337dc39 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -4,7 +4,6 @@ module ActiveRecord module Associations class Preloader class Association #:nodoc: - attr_reader :owners, :reflection, :preload_scope, :model, :klass attr_reader :preloaded_records def initialize(klass, owners, reflection, preload_scope) @@ -17,11 +16,20 @@ module ActiveRecord end def run(preloader) - associated_records_by_owner(preloader).each do |owner, records| - associate_records_to_owner(owner, records) + records = load_records do |record| + owner = owners_by_key[convert_key(record[association_key_name])] + association = owner.association(reflection.name) + association.set_inverse_instance(record) + end + + owners.each do |owner| + associate_records_to_owner(owner, records[convert_key(owner[owner_key_name])] || []) end end + protected + attr_reader :owners, :reflection, :preload_scope, :model, :klass + private # The name of the key on the associated records def association_key_name @@ -33,18 +41,6 @@ module ActiveRecord reflection.join_foreign_key end - def associated_records_by_owner(preloader) - records = load_records do |record| - owner = owners_by_key[convert_key(record[association_key_name])] - association = owner.association(reflection.name) - association.set_inverse_instance(record) - end - - owners.each_with_object({}) do |owner, result| - result[owner] = records[convert_key(owner[owner_key_name])] || [] - end - end - def associate_records_to_owner(owner, records) raise NotImplementedError end diff --git a/activerecord/lib/active_record/associations/preloader/has_many_through.rb b/activerecord/lib/active_record/associations/preloader/has_many_through.rb index 0639fdca44..3e17d07a33 100644 --- a/activerecord/lib/active_record/associations/preloader/has_many_through.rb +++ b/activerecord/lib/active_record/associations/preloader/has_many_through.rb @@ -5,16 +5,6 @@ module ActiveRecord class Preloader class HasManyThrough < CollectionAssociation #:nodoc: include ThroughAssociation - - def associated_records_by_owner(preloader) - records_by_owner = super - - if reflection_scope.distinct_value - records_by_owner.each_value(&:uniq!) - end - - records_by_owner - end end end end diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index fa32cc5553..762275fbad 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -4,77 +4,53 @@ module ActiveRecord module Associations class Preloader module ThroughAssociation #:nodoc: - def through_reflection - reflection.through_reflection - end - - def source_reflection - reflection.source_reflection - end - - def associated_records_by_owner(preloader) - through_scope = through_scope() - - preloader.preload(owners, - through_reflection.name, - through_scope) - - through_records = owners.map do |owner| - center = owner.association(through_reflection.name).target - [owner, Array(center)] - end - - reset_association(owners, through_reflection.name, through_scope) - - middle_records = through_records.flat_map(&:last) - - reflection_scope = reflection_scope() if reflection.scope - - preloaders = preloader.preload(middle_records, - source_reflection.name, - reflection_scope) - + def run(preloader) + already_loaded = owners.first.association(through_reflection.name).loaded? + through_scope = through_scope() + reflection_scope = target_reflection_scope + through_preloaders = preloader.preload(owners, through_reflection.name, through_scope) + middle_records = through_preloaders.flat_map(&:preloaded_records) + preloaders = preloader.preload(middle_records, source_reflection.name, reflection_scope) @preloaded_records = preloaders.flat_map(&:preloaded_records) - middle_to_pl = preloaders.each_with_object({}) do |pl, h| - pl.owners.each { |middle| - h[middle] = pl - } - end - - through_records.each_with_object({}) do |(lhs, center), records_by_owner| - pl_to_middle = center.group_by { |record| middle_to_pl[record] } - - records_by_owner[lhs] = pl_to_middle.flat_map do |pl, middles| - rhs_records = middles.flat_map { |r| - r.association(source_reflection.name).target - }.compact - - # Respect the order on `reflection_scope` if it exists, else use the natural order. - if reflection_scope && !reflection_scope.order_values.empty? - @id_map ||= id_to_index_map @preloaded_records - rhs_records.sort_by { |rhs| @id_map[rhs] } - else - rhs_records + owners.each do |owner| + through_records = Array(owner.association(through_reflection.name).target) + if already_loaded + if source_type = reflection.options[:source_type] + through_records = through_records.select do |record| + record[reflection.foreign_type] == source_type + end end + else + owner.association(through_reflection.name).reset if through_scope + end + result = through_records.flat_map do |record| + association = record.association(source_reflection.name) + target = association.target + association.reset if preload_scope + target end + result.compact! + if reflection_scope + result.sort_by! { |rhs| preload_index[rhs] } if reflection_scope.order_values.any? + result.uniq! if reflection_scope.distinct_value + end + associate_records_to_owner(owner, result) end end private + def through_reflection + reflection.through_reflection + end - def id_to_index_map(ids) - id_map = {} - ids.each_with_index { |id, index| id_map[id] = index } - id_map + def source_reflection + reflection.source_reflection end - def reset_association(owners, association_name, should_reset) - # Don't cache the association - we would only be caching a subset - if should_reset - owners.each { |owner| - owner.association(association_name).reset - } + def preload_index + @preload_index ||= @preloaded_records.each_with_object({}).with_index do |(id, result), index| + result[id] = index end end @@ -115,6 +91,16 @@ module ActiveRecord scope unless scope.empty_scope? end + + def target_reflection_scope + if preload_scope + reflection_scope.merge(preload_scope) + elsif reflection.scope + reflection_scope + else + nil + end + end end end end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 79110d04f4..3de6fe566d 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/core_ext/module/attribute_accessors" -require "active_record/attribute_mutation_tracker" module ActiveRecord module AttributeMethods @@ -33,65 +32,13 @@ module ActiveRecord # <tt>reload</tt> the record and clears changed attributes. def reload(*) super.tap do + @previously_changed = ActiveSupport::HashWithIndifferentAccess.new @mutations_before_last_save = nil + @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new @mutations_from_database = nil - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new end end - def initialize_dup(other) # :nodoc: - super - @attributes = self.class._default_attributes.map do |attr| - attr.with_value_from_user(@attributes.fetch_value(attr.name)) - end - @mutations_from_database = nil - end - - def changes_applied # :nodoc: - @mutations_before_last_save = mutations_from_database - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - forget_attribute_assignments - @mutations_from_database = nil - end - - def clear_changes_information # :nodoc: - @mutations_before_last_save = nil - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - forget_attribute_assignments - @mutations_from_database = nil - end - - def clear_attribute_changes(attr_names) # :nodoc: - super - attr_names.each do |attr_name| - clear_attribute_change(attr_name) - end - end - - def changed_attributes # :nodoc: - # This should only be set by methods which will call changed_attributes - # multiple times when it is known that the computed value cannot change. - if defined?(@cached_changed_attributes) - @cached_changed_attributes - else - super.reverse_merge(mutations_from_database.changed_values).freeze - end - end - - def changes # :nodoc: - cache_changed_attributes do - super - end - end - - def previous_changes # :nodoc: - mutations_before_last_save.changes - end - - def attribute_changed_in_place?(attr_name) # :nodoc: - mutations_from_database.changed_in_place?(attr_name) - end - # Did this attribute change when we last saved? This method can be invoked # as +saved_change_to_name?+ instead of <tt>saved_change_to_attribute?("name")</tt>. # Behaves similarly to +attribute_changed?+. This method is useful in @@ -182,26 +129,6 @@ module ActiveRecord result end - def mutations_from_database - unless defined?(@mutations_from_database) - @mutations_from_database = nil - end - @mutations_from_database ||= AttributeMutationTracker.new(@attributes) - end - - def changes_include?(attr_name) - super || mutations_from_database.changed?(attr_name) - end - - def clear_attribute_change(attr_name) - mutations_from_database.forget_change(attr_name) - end - - def attribute_will_change!(attr_name) - super - mutations_from_database.force_change(attr_name) - end - def _update_record(*) partial_writes? ? super(keys_for_partial_write) : super end @@ -213,25 +140,6 @@ module ActiveRecord def keys_for_partial_write changed_attribute_names_to_save & self.class.column_names end - - def forget_attribute_assignments - @attributes = @attributes.map(&:forgetting_assignment) - end - - def mutations_before_last_save - @mutations_before_last_save ||= NullMutationTracker.instance - end - - def cache_changed_attributes - @cached_changed_attributes = changed_attributes - yield - ensure - clear_changed_attributes_cache - end - - def clear_changed_attributes_cache - remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes) - end end end end diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index 9224d58928..0b7c9398a8 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "active_record/attribute/user_provided_default" +require "active_model/attribute/user_provided_default" module ActiveRecord # See ActiveRecord::Attributes::ClassMethods for documentation @@ -250,14 +250,14 @@ module ActiveRecord if value == NO_DEFAULT_PROVIDED default_attribute = _default_attributes[name].with_type(type) elsif from_user - default_attribute = Attribute::UserProvidedDefault.new( + default_attribute = ActiveModel::Attribute::UserProvidedDefault.new( name, value, type, _default_attributes.fetch(name.to_s) { nil }, ) else - default_attribute = Attribute.from_database(name, value, type) + default_attribute = ActiveModel::Attribute.from_database(name, value, type) end _default_attributes[name] = default_attribute end diff --git a/activerecord/lib/active_record/collection_cache_key.rb b/activerecord/lib/active_record/collection_cache_key.rb index f3e6516414..88b398ad45 100644 --- a/activerecord/lib/active_record/collection_cache_key.rb +++ b/activerecord/lib/active_record/collection_cache_key.rb @@ -12,6 +12,9 @@ module ActiveRecord timestamp = collection.max_by(×tamp_column)._read_attribute(timestamp_column) end else + if collection.eager_loading? + collection = collection.send(:apply_join_dependency) + end column_type = type_for_attribute(timestamp_column.to_s) column = connection.column_name_from_arel_node(collection.arel_attribute(timestamp_column)) select_values = "COUNT(*) AS #{connection.quote_column_name("size")}, MAX(%s) AS timestamp" diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 147e16e9fa..d9ac8db6a8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -240,7 +240,7 @@ module ActiveRecord rollback_transaction if transaction else begin - commit_transaction + commit_transaction if transaction rescue Exception rollback_transaction(transaction) unless transaction.state.completed? raise diff --git a/activerecord/lib/active_record/legacy_yaml_adapter.rb b/activerecord/lib/active_record/legacy_yaml_adapter.rb index 23644aab8f..ffa095dd94 100644 --- a/activerecord/lib/active_record/legacy_yaml_adapter.rb +++ b/activerecord/lib/active_record/legacy_yaml_adapter.rb @@ -8,7 +8,7 @@ module ActiveRecord case coder["active_record_yaml_version"] when 1, 2 then coder else - if coder["attributes"].is_a?(AttributeSet) + if coder["attributes"].is_a?(ActiveModel::AttributeSet) Rails420.convert(klass, coder) else Rails41.convert(klass, coder) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 1485687053..d12a979a7f 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -1224,7 +1224,7 @@ module ActiveRecord # Return true if a valid version is not provided. def invalid_target? - !target && @target_version && @target_version > 0 + @target_version && @target_version != 0 && !target end def execute_migration_in_transaction(migration, direction) diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index bed9400f51..12ee4a4137 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -323,7 +323,7 @@ module ActiveRecord end def attributes_builder # :nodoc: - @attributes_builder ||= AttributeSet::Builder.new(attribute_types, primary_key) do |name| + @attributes_builder ||= ActiveModel::AttributeSet::Builder.new(attribute_types, primary_key) do |name| unless columns_hash.key?(name) _default_attributes[name].dup end @@ -346,7 +346,7 @@ module ActiveRecord end def yaml_encoder # :nodoc: - @yaml_encoder ||= AttributeSet::YAMLEncoder.new(attribute_types) + @yaml_encoder ||= ActiveModel::AttributeSet::YAMLEncoder.new(attribute_types) end # Returns the type of the attribute with the given name, after applying @@ -376,7 +376,7 @@ module ActiveRecord end def _default_attributes # :nodoc: - @default_attributes ||= AttributeSet.new({}) + @default_attributes ||= ActiveModel::AttributeSet.new({}) end # Returns an array of column names as strings. diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 723272b4b2..3bca2982e0 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -97,16 +97,27 @@ db_namespace = namespace :db do task up: [:environment, :load_config] do raise "VERSION is required" if !ENV["VERSION"] || ENV["VERSION"].empty? - version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil - ActiveRecord::Migrator.run(:up, ActiveRecord::Tasks::DatabaseTasks.migrations_paths, version) + ActiveRecord::Tasks::DatabaseTasks.check_target_version + + ActiveRecord::Migrator.run( + :up, + ActiveRecord::Tasks::DatabaseTasks.migrations_paths, + ActiveRecord::Tasks::DatabaseTasks.target_version + ) db_namespace["_dump"].invoke end # desc 'Runs the "down" for a given migration VERSION.' task down: [:environment, :load_config] do raise "VERSION is required - To go down one migration, use db:rollback" if !ENV["VERSION"] || ENV["VERSION"].empty? - version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil - ActiveRecord::Migrator.run(:down, ActiveRecord::Tasks::DatabaseTasks.migrations_paths, version) + + ActiveRecord::Tasks::DatabaseTasks.check_target_version + + ActiveRecord::Migrator.run( + :down, + ActiveRecord::Tasks::DatabaseTasks.migrations_paths, + ActiveRecord::Tasks::DatabaseTasks.target_version + ) db_namespace["_dump"].invoke end diff --git a/activerecord/lib/active_record/railties/jdbcmysql_error.rb b/activerecord/lib/active_record/railties/jdbcmysql_error.rb deleted file mode 100644 index 72c75ddd52..0000000000 --- a/activerecord/lib/active_record/railties/jdbcmysql_error.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -#FIXME Remove if ArJdbcMysql will give. -module ArJdbcMySQL #:nodoc: - class Error < StandardError #:nodoc: - attr_accessor :error_number, :sql_state - - def initialize(msg) - super - @error_number = nil - @sql_state = nil - end - - # Mysql gem compatibility - alias_method :errno, :error_number - alias_method :error, :message - end -end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 997cfe4b5e..7615fb6ee9 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -359,6 +359,11 @@ module ActiveRecord def update_all(updates) raise ArgumentError, "Empty list of attributes to change" if updates.blank? + if eager_loading? + relation = apply_join_dependency + return relation.update_all(updates) + end + stmt = Arel::UpdateManager.new stmt.set Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates)) @@ -423,6 +428,11 @@ module ActiveRecord raise ActiveRecordError.new("delete_all doesn't support #{invalid_methods.join(', ')}") end + if eager_loading? + relation = apply_join_dependency + return relation.delete_all + end + stmt = Arel::DeleteManager.new stmt.from(table) diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 116bddce85..11256ab3d9 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -130,7 +130,7 @@ module ActiveRecord # end def calculate(operation, column_name) if has_include?(column_name) - relation = apply_join_dependency(construct_join_dependency) + relation = apply_join_dependency relation.distinct! if operation.to_s.downcase == "count" relation.calculate(operation, column_name) @@ -180,7 +180,7 @@ module ActiveRecord end if has_include?(column_names.first) - relation = apply_join_dependency(construct_join_dependency) + relation = apply_join_dependency relation.pluck(*column_names) else relation = spawn diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 707245bab2..18566b5662 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -394,7 +394,7 @@ module ActiveRecord ) end - def apply_join_dependency(join_dependency) + def apply_join_dependency(join_dependency = construct_join_dependency) relation = except(:includes, :eager_load, :preload).joins!(join_dependency) if using_limitable_reflections?(join_dependency.reflections) diff --git a/activerecord/lib/active_record/relation/query_attribute.rb b/activerecord/lib/active_record/relation/query_attribute.rb index fad08e2613..3532f28858 100644 --- a/activerecord/lib/active_record/relation/query_attribute.rb +++ b/activerecord/lib/active_record/relation/query_attribute.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true -require "active_record/attribute" +require "active_model/attribute" module ActiveRecord class Relation - class QueryAttribute < Attribute # :nodoc: + class QueryAttribute < ActiveModel::Attribute # :nodoc: def type_cast(value) value end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 897ff5c8af..9c5be4ad9b 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -930,7 +930,7 @@ module ActiveRecord arel.where(where_clause.ast) unless where_clause.empty? arel.having(having_clause.ast) unless having_clause.empty? if limit_value - limit_attribute = Attribute.with_cast_value( + limit_attribute = ActiveModel::Attribute.with_cast_value( "LIMIT".freeze, connection.sanitize_limit(limit_value), Type.default_value, @@ -938,7 +938,7 @@ module ActiveRecord arel.take(Arel::Nodes::BindParam.new(limit_attribute)) end if offset_value - offset_attribute = Attribute.with_cast_value( + offset_attribute = ActiveModel::Attribute.with_cast_value( "OFFSET".freeze, offset_value.to_i, Type.default_value, diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index 6fa096c1fe..310af72c41 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -24,8 +24,14 @@ module ActiveRecord # You can define a scope that applies to all finders using # {default_scope}[rdoc-ref:Scoping::Default::ClassMethods#default_scope]. def all + current_scope = self.current_scope + if current_scope - current_scope.clone + if self == current_scope.klass + current_scope.clone + else + relation.merge!(current_scope) + end else default_scoped end diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index ff388ff1f6..4657e51e6d 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -164,13 +164,12 @@ module ActiveRecord end def migrate - raise "Empty VERSION provided" if ENV["VERSION"] && ENV["VERSION"].empty? + check_target_version verbose = ENV["VERBOSE"] ? ENV["VERBOSE"] != "false" : true - version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil scope = ENV["SCOPE"] verbose_was, Migration.verbose = Migration.verbose, verbose - Migrator.migrate(migrations_paths, version) do |migration| + Migrator.migrate(migrations_paths, target_version) do |migration| scope.blank? || scope == migration.scope end ActiveRecord::Base.clear_cache! @@ -178,6 +177,16 @@ module ActiveRecord Migration.verbose = verbose_was end + def check_target_version + if target_version && !(Migration::MigrationFilenameRegexp.match?(ENV["VERSION"]) || /\A\d+\z/.match?(ENV["VERSION"])) + raise "Invalid format of target version: `VERSION=#{ENV['VERSION']}`" + end + end + + def target_version + ENV["VERSION"].to_i if ENV["VERSION"] && !ENV["VERSION"].empty? + end + def charset_current(environment = env) charset ActiveRecord::Base.configurations[environment] end diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index 2f91884f59..e697fa6def 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -3,8 +3,6 @@ module ActiveRecord module Tasks # :nodoc: class MySQLDatabaseTasks # :nodoc: - ACCESS_DENIED_ERROR = 1045 - delegate :connection, :establish_connection, to: ActiveRecord::Base def initialize(configuration) @@ -21,20 +19,6 @@ module ActiveRecord else raise end - rescue error_class => error - if error.respond_to?(:errno) && error.errno == ACCESS_DENIED_ERROR - $stdout.print error.message - establish_connection root_configuration_without_database - connection.create_database configuration["database"], creation_options - if configuration["username"] != "root" - connection.execute grant_statement.gsub(/\s+/, " ").strip - end - establish_connection configuration - else - $stderr.puts error.inspect - $stderr.puts "Couldn't create database for #{configuration.inspect}, #{creation_options.inspect}" - $stderr.puts "(If you set the charset manually, make sure you have a matching collation)" if configuration["encoding"] - end end def drop @@ -99,37 +83,6 @@ module ActiveRecord end end - def error_class - if configuration["adapter"].include?("jdbc") - require "active_record/railties/jdbcmysql_error" - ArJdbcMySQL::Error - elsif defined?(Mysql2) - Mysql2::Error - else - StandardError - end - end - - def grant_statement - <<-SQL -GRANT ALL PRIVILEGES ON `#{configuration['database']}`.* - TO '#{configuration['username']}'@'localhost' -IDENTIFIED BY '#{configuration['password']}' WITH GRANT OPTION; - SQL - end - - def root_configuration_without_database - configuration_without_database.merge( - "username" => "root", - "password" => root_password - ) - end - - def root_password - $stdout.print "Please provide the root password for your MySQL installation\n>" - $stdin.gets.strip - end - def prepare_command_options args = { "host" => "--host", diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index e69cfe5e52..829e12fbc8 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -37,7 +37,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase def test_eager_association_loading_with_hmt_does_not_table_name_collide_when_joining_associations authors = Author.joins(:posts).eager_load(:comments).where(posts: { tags_count: 1 }).to_a - assert_equal 1, assert_no_queries { authors.size } + assert_equal 3, assert_no_queries { authors.size } assert_equal 10, assert_no_queries { authors[0].comments.size } end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 6bd11a5d81..4ca11af791 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -831,7 +831,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_find_scoped_grouped_having - assert_equal 1, authors(:david).popular_grouped_posts.length + assert_equal 2, authors(:david).popular_grouped_posts.length assert_equal 0, authors(:mary).popular_grouped_posts.length end diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb index 4cf5a9ffc4..65d30d011b 100644 --- a/activerecord/test/cases/associations/nested_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -579,6 +579,28 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase assert !c.post_taggings.empty? end + def test_polymorphic_has_many_through_when_through_association_has_not_loaded + cake_designer = CakeDesigner.create!(chef: Chef.new) + drink_designer = DrinkDesigner.create!(chef: Chef.new) + department = Department.create!(chefs: [cake_designer.chef, drink_designer.chef]) + Hotel.create!(departments: [department]) + hotel = Hotel.includes(:cake_designers, :drink_designers).take + + assert_equal [cake_designer], hotel.cake_designers + assert_equal [drink_designer], hotel.drink_designers + end + + def test_polymorphic_has_many_through_when_through_association_has_already_loaded + cake_designer = CakeDesigner.create!(chef: Chef.new) + drink_designer = DrinkDesigner.create!(chef: Chef.new) + department = Department.create!(chefs: [cake_designer.chef, drink_designer.chef]) + Hotel.create!(departments: [department]) + hotel = Hotel.includes(:chefs, :cake_designers, :drink_designers).take + + assert_equal [cake_designer], hotel.cake_designers + assert_equal [drink_designer], hotel.drink_designers + end + def test_polymorphic_has_many_through_joined_different_table_twice cake_designer = CakeDesigner.create!(chef: Chef.new) drink_designer = DrinkDesigner.create!(chef: Chef.new) diff --git a/activerecord/test/cases/collection_cache_key_test.rb b/activerecord/test/cases/collection_cache_key_test.rb index c137693211..19d6464a22 100644 --- a/activerecord/test/cases/collection_cache_key_test.rb +++ b/activerecord/test/cases/collection_cache_key_test.rb @@ -73,6 +73,16 @@ module ActiveRecord assert_equal last_developer_timestamp.to_s(ActiveRecord::Base.cache_timestamp_format), $3 end + test "cache_key for relation with includes" do + comments = Comment.includes(:post).where("posts.type": "Post") + assert_match(/\Acomments\/query-(\h+)-(\d+)-(\d+)\z/, comments.cache_key) + end + + test "cache_key for loaded relation with includes" do + comments = Comment.includes(:post).where("posts.type": "Post").load + assert_match(/\Acomments\/query-(\h+)-(\d+)-(\d+)\z/, comments.cache_key) + end + test "it triggers at most one query" do developers = Developer.where(name: "David") diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 6791d50940..e857180bd1 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -611,14 +611,12 @@ unless in_memory_db? end end - if current_adapter?(:PostgreSQLAdapter, :OracleAdapter) - def test_no_locks_no_wait - first, second = duel { Person.find 1 } - assert first.end > second.end - end - - private + def test_no_locks_no_wait + first, second = duel { Person.find 1 } + assert first.end > second.end + end + private def duel(zzz = 5) t0, t1, t2, t3 = nil, nil, nil, nil @@ -646,6 +644,5 @@ unless in_memory_db? assert t3 > t2 [t0.to_f..t1.to_f, t2.to_f..t3.to_f] end - end end end diff --git a/activerecord/test/cases/migrator_test.rb b/activerecord/test/cases/migrator_test.rb index ee10be119c..1047ba1367 100644 --- a/activerecord/test/cases/migrator_test.rb +++ b/activerecord/test/cases/migrator_test.rb @@ -66,6 +66,26 @@ class MigratorTest < ActiveRecord::TestCase list = [ActiveRecord::Migration.new("Foo", 1), ActiveRecord::Migration.new("Bar", 2)] ActiveRecord::Migrator.new(:up, list, 3).run end + + assert_raises(ActiveRecord::UnknownMigrationVersionError) do + list = [ActiveRecord::Migration.new("Foo", 1), ActiveRecord::Migration.new("Bar", 2)] + ActiveRecord::Migrator.new(:up, list, -1).run + end + + assert_raises(ActiveRecord::UnknownMigrationVersionError) do + list = [ActiveRecord::Migration.new("Foo", 1), ActiveRecord::Migration.new("Bar", 2)] + ActiveRecord::Migrator.new(:up, list, 0).run + end + + assert_raises(ActiveRecord::UnknownMigrationVersionError) do + list = [ActiveRecord::Migration.new("Foo", 1), ActiveRecord::Migration.new("Bar", 2)] + ActiveRecord::Migrator.new(:up, list, 3).migrate + end + + assert_raises(ActiveRecord::UnknownMigrationVersionError) do + list = [ActiveRecord::Migration.new("Foo", 1), ActiveRecord::Migration.new("Bar", 2)] + ActiveRecord::Migrator.new(:up, list, -1).migrate + end end def test_finds_migrations diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 6cbe18cc8c..f088c064f5 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -94,27 +94,31 @@ class PersistenceTest < ActiveRecord::TestCase end def test_delete_all_with_joins_and_where_part_is_hash - where_args = { toys: { name: "Bone" } } - count = Pet.joins(:toys).where(where_args).count + pets = Pet.joins(:toys).where(toys: { name: "Bone" }) - assert_equal count, 1 - assert_equal count, Pet.joins(:toys).where(where_args).delete_all + assert_equal true, pets.exists? + assert_equal pets.count, pets.delete_all + end + + def test_delete_all_with_joins_and_where_part_is_not_hash + pets = Pet.joins(:toys).where("toys.name = ?", "Bone") + + assert_equal true, pets.exists? + assert_equal pets.count, pets.delete_all end def test_delete_all_with_left_joins - where_args = { toys: { name: "Bone" } } - count = Pet.left_joins(:toys).where(where_args).count + pets = Pet.left_joins(:toys).where(toys: { name: "Bone" }) - assert_equal count, 1 - assert_equal count, Pet.left_joins(:toys).where(where_args).delete_all + assert_equal true, pets.exists? + assert_equal pets.count, pets.delete_all end - def test_delete_all_with_joins_and_where_part_is_not_hash - where_args = ["toys.name = ?", "Bone"] - count = Pet.joins(:toys).where(where_args).count + def test_delete_all_with_includes + pets = Pet.includes(:toys).where(toys: { name: "Bone" }) - assert_equal count, 1 - assert_equal count, Pet.joins(:toys).where(where_args).delete_all + assert_equal true, pets.exists? + assert_equal pets.count, pets.delete_all end def test_increment_attribute @@ -496,17 +500,24 @@ class PersistenceTest < ActiveRecord::TestCase end def test_update_all_with_joins - where_args = { toys: { name: "Bone" } } - count = Pet.left_joins(:toys).where(where_args).count + pets = Pet.joins(:toys).where(toys: { name: "Bone" }) - assert_equal count, Pet.joins(:toys).where(where_args).update_all(name: "Bob") + assert_equal true, pets.exists? + assert_equal pets.count, pets.update_all(name: "Bob") end def test_update_all_with_left_joins - where_args = { toys: { name: "Bone" } } - count = Pet.left_joins(:toys).where(where_args).count + pets = Pet.left_joins(:toys).where(toys: { name: "Bone" }) + + assert_equal true, pets.exists? + assert_equal pets.count, pets.update_all(name: "Bob") + end + + def test_update_all_with_includes + pets = Pet.includes(:toys).where(toys: { name: "Bone" }) - assert_equal count, Pet.left_joins(:toys).where(where_args).update_all(name: "Bob") + assert_equal true, pets.exists? + assert_equal pets.count, pets.update_all(name: "Bob") end def test_update_all_with_non_standard_table_name diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 72433d1e8e..844be0d0bf 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -927,13 +927,13 @@ class RelationTest < ActiveRecord::TestCase assert_equal 11, posts.count(:all) assert_equal 11, posts.count(:id) - assert_equal 1, posts.where("comments_count > 1").count - assert_equal 9, posts.where(comments_count: 0).count + assert_equal 3, posts.where("comments_count > 1").count + assert_equal 6, posts.where(comments_count: 0).count end def test_count_with_block posts = Post.all - assert_equal 10, posts.count { |p| p.comments_count.even? } + assert_equal 8, posts.count { |p| p.comments_count.even? } end def test_count_on_association_relation @@ -950,10 +950,10 @@ class RelationTest < ActiveRecord::TestCase def test_count_with_distinct posts = Post.all - assert_equal 3, posts.distinct(true).count(:comments_count) + assert_equal 4, posts.distinct(true).count(:comments_count) assert_equal 11, posts.distinct(false).count(:comments_count) - assert_equal 3, posts.distinct(true).select(:comments_count).count + assert_equal 4, posts.distinct(true).select(:comments_count).count assert_equal 11, posts.distinct(false).select(:comments_count).count end @@ -992,7 +992,7 @@ class RelationTest < ActiveRecord::TestCase best_posts = posts.where(comments_count: 0) best_posts.load # force load - assert_no_queries { assert_equal 9, best_posts.size } + assert_no_queries { assert_equal 6, best_posts.size } end def test_size_with_limit @@ -1003,7 +1003,7 @@ class RelationTest < ActiveRecord::TestCase best_posts = posts.where(comments_count: 0) best_posts.load # force load - assert_no_queries { assert_equal 9, best_posts.size } + assert_no_queries { assert_equal 6, best_posts.size } end def test_size_with_zero_limit @@ -1026,7 +1026,7 @@ class RelationTest < ActiveRecord::TestCase def test_count_complex_chained_relations posts = Post.select("comments_count").where("id is not null").group("author_id").where("comments_count > 0") - expected = { 1 => 2 } + expected = { 1 => 4, 2 => 1 } assert_equal expected, posts.count end @@ -1781,7 +1781,7 @@ class RelationTest < ActiveRecord::TestCase end test "arel_attribute respects a custom table" do - assert_equal [posts(:welcome)], custom_post_relation.ranked_by_comments.limit_by(1).to_a + assert_equal [posts(:sti_comments)], custom_post_relation.ranked_by_comments.limit_by(1).to_a end test "alias_tracker respects a custom table" do diff --git a/activerecord/test/cases/scoping/relation_scoping_test.rb b/activerecord/test/cases/scoping/relation_scoping_test.rb index f3b84d88c2..116f8e83aa 100644 --- a/activerecord/test/cases/scoping/relation_scoping_test.rb +++ b/activerecord/test/cases/scoping/relation_scoping_test.rb @@ -240,6 +240,20 @@ class RelationScopingTest < ActiveRecord::TestCase assert_nil SpecialComment.current_scope end + def test_scoping_respects_current_class + Comment.unscoped do + assert_equal "a comment...", Comment.all.what_are_you + assert_equal "a special comment...", SpecialComment.all.what_are_you + end + end + + def test_scoping_respects_sti_constraint + Comment.unscoped do + assert_equal comments(:greetings), Comment.find(1) + assert_raises(ActiveRecord::RecordNotFound) { SpecialComment.find(1) } + end + end + def test_circular_joins_with_scoping_does_not_crash posts = Post.joins(comments: :post).scoping do Post.first(10) diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index 1495d2ab89..5a094ead42 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -357,8 +357,15 @@ module ActiveRecord ActiveRecord::Migration.expects(:verbose=).with(ActiveRecord::Migration.verbose) ActiveRecord::Tasks::DatabaseTasks.migrate + ENV["VERBOSE"] = "" + ENV["VERSION"] = "" + ActiveRecord::Migrator.expects(:migrate).with("custom/path", nil) + ActiveRecord::Migration.expects(:verbose=).with(true) + ActiveRecord::Migration.expects(:verbose=).with(ActiveRecord::Migration.verbose) + ActiveRecord::Tasks::DatabaseTasks.migrate + ENV["VERBOSE"] = "yes" - ENV["VERSION"] = "unknown" + ENV["VERSION"] = "0" ActiveRecord::Migrator.expects(:migrate).with("custom/path", 0) ActiveRecord::Migration.expects(:verbose=).with(true) ActiveRecord::Migration.expects(:verbose=).with(ActiveRecord::Migration.verbose) @@ -367,15 +374,47 @@ module ActiveRecord ENV["VERBOSE"], ENV["VERSION"] = verbose, version end - def test_migrate_raise_error_on_empty_version + def test_migrate_raise_error_on_invalid_version_format version = ENV["VERSION"] - ENV["VERSION"] = "" + + ENV["VERSION"] = "unknown" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "0.1.11" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1.1.11" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "0 " + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1." + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1_" e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } - assert_equal "Empty VERSION provided", e.message + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1_name" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) ensure ENV["VERSION"] = version end + def test_migrate_raise_error_on_failed_check_target_version + ActiveRecord::Tasks::DatabaseTasks.stubs(:check_target_version).raises("foo") + + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_equal "foo", e.message + end + def test_migrate_clears_schema_cache_afterward ActiveRecord::Base.expects(:clear_cache!) ActiveRecord::Tasks::DatabaseTasks.migrate @@ -444,6 +483,108 @@ module ActiveRecord end end + class DatabaseTaskTargetVersionTest < ActiveRecord::TestCase + def test_target_version_returns_nil_if_version_does_not_exist + version = ENV.delete("VERSION") + assert_nil ActiveRecord::Tasks::DatabaseTasks.target_version + ensure + ENV["VERSION"] = version + end + + def test_target_version_returns_nil_if_version_is_empty + version = ENV["VERSION"] + + ENV["VERSION"] = "" + assert_nil ActiveRecord::Tasks::DatabaseTasks.target_version + ensure + ENV["VERSION"] = version + end + + def test_target_version_returns_converted_to_integer_env_version_if_version_exists + version = ENV["VERSION"] + + ENV["VERSION"] = "0" + assert_equal ENV["VERSION"].to_i, ActiveRecord::Tasks::DatabaseTasks.target_version + + ENV["VERSION"] = "42" + assert_equal ENV["VERSION"].to_i, ActiveRecord::Tasks::DatabaseTasks.target_version + + ENV["VERSION"] = "042" + assert_equal ENV["VERSION"].to_i, ActiveRecord::Tasks::DatabaseTasks.target_version + ensure + ENV["VERSION"] = version + end + end + + class DatabaseTaskCheckTargetVersionTest < ActiveRecord::TestCase + def test_check_target_version_does_not_raise_error_on_empty_version + version = ENV["VERSION"] + ENV["VERSION"] = "" + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + ensure + ENV["VERSION"] = version + end + + def test_check_target_version_does_not_raise_error_if_version_is_not_setted + version = ENV.delete("VERSION") + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + ensure + ENV["VERSION"] = version + end + + def test_check_target_version_raises_error_on_invalid_version_format + version = ENV["VERSION"] + + ENV["VERSION"] = "unknown" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "0.1.11" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1.1.11" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "0 " + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1." + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1_" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1_name" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + ensure + ENV["VERSION"] = version + end + + def test_check_target_version_does_not_raise_error_on_valid_version_format + version = ENV["VERSION"] + + ENV["VERSION"] = "0" + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + + ENV["VERSION"] = "1" + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + + ENV["VERSION"] = "001" + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + + ENV["VERSION"] = "001_name.rb" + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + ensure + ENV["VERSION"] = version + end + end + class DatabaseTasksStructureDumpTest < ActiveRecord::TestCase include DatabaseTasksSetupper diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index 98fe24baa0..047153e7cc 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -75,7 +75,7 @@ if current_adapter?(:Mysql2Adapter) end end - class MysqlDBCreateAsRootTest < ActiveRecord::TestCase + class MysqlDBCreateWithInvalidPermissionsTest < ActiveRecord::TestCase def setup @connection = stub("Connection", create_database: true) @error = Mysql2::Error.new("Invalid permissions") @@ -86,13 +86,8 @@ if current_adapter?(:Mysql2Adapter) "password" => "wossname" } - $stdin.stubs(:gets).returns("secret\n") - $stdout.stubs(:print).returns(nil) - @error.stubs(:errno).returns(1045) ActiveRecord::Base.stubs(:connection).returns(@connection) - ActiveRecord::Base.stubs(:establish_connection). - raises(@error). - then.returns(true) + ActiveRecord::Base.stubs(:establish_connection).raises(@error) $stdout, @original_stdout = StringIO.new, $stdout $stderr, @original_stderr = StringIO.new, $stderr @@ -102,75 +97,11 @@ if current_adapter?(:Mysql2Adapter) $stdout, $stderr = @original_stdout, @original_stderr end - def test_root_password_is_requested - assert_permissions_granted_for("pat") - $stdin.expects(:gets).returns("secret\n") - - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - def test_connection_established_as_root - assert_permissions_granted_for("pat") - ActiveRecord::Base.expects(:establish_connection).with( - "adapter" => "mysql2", - "database" => nil, - "username" => "root", - "password" => "secret" - ) - - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - def test_database_created_by_root - assert_permissions_granted_for("pat") - @connection.expects(:create_database). - with("my-app-db", {}) - - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - def test_grant_privileges_for_normal_user - assert_permissions_granted_for("pat") - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - def test_do_not_grant_privileges_for_root_user - @configuration["username"] = "root" - @configuration["password"] = "" - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - def test_connection_established_as_normal_user - assert_permissions_granted_for("pat") - ActiveRecord::Base.expects(:establish_connection).returns do - ActiveRecord::Base.expects(:establish_connection).with( - "adapter" => "mysql2", - "database" => "my-app-db", - "username" => "pat", - "password" => "secret" - ) - - raise @error + def test_raises_error + assert_raises(Mysql2::Error) do + ActiveRecord::Tasks::DatabaseTasks.create @configuration end - - ActiveRecord::Tasks::DatabaseTasks.create @configuration end - - def test_sends_output_to_stderr_when_other_errors - @error.stubs(:errno).returns(42) - - $stderr.expects(:puts).at_least_once.returns(nil) - - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - private - - def assert_permissions_granted_for(db_user) - db_name = @configuration["database"] - db_password = @configuration["password"] - @connection.expects(:execute).with("GRANT ALL PRIVILEGES ON `#{db_name}`.* TO '#{db_user}'@'localhost' IDENTIFIED BY '#{db_password}' WITH GRANT OPTION;") - end end class MySQLDBDropTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/transaction_isolation_test.rb b/activerecord/test/cases/transaction_isolation_test.rb index b1ebccdcc3..eaafd13360 100644 --- a/activerecord/test/cases/transaction_isolation_test.rb +++ b/activerecord/test/cases/transaction_isolation_test.rb @@ -15,9 +15,7 @@ unless ActiveRecord::Base.connection.supports_transaction_isolation? end end end -end - -if ActiveRecord::Base.connection.supports_transaction_isolation? +else class TransactionIsolationTest < ActiveRecord::TestCase self.use_transactional_tests = false diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 7fd125ab74..5c8ae4d3cb 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -954,27 +954,25 @@ class TransactionsWithTransactionalFixturesTest < ActiveRecord::TestCase end end if Topic.connection.supports_savepoints? -if current_adapter?(:PostgreSQLAdapter) +if ActiveRecord::Base.connection.supports_transaction_isolation? class ConcurrentTransactionTest < TransactionTest # This will cause transactions to overlap and fail unless they are performed on # separate database connections. - unless in_memory_db? - def test_transaction_per_thread - threads = 3.times.map do - Thread.new do - Topic.transaction do - topic = Topic.find(1) - topic.approved = !topic.approved? - assert topic.save! - topic.approved = !topic.approved? - assert topic.save! - end - Topic.connection.close + def test_transaction_per_thread + threads = 3.times.map do + Thread.new do + Topic.transaction do + topic = Topic.find(1) + topic.approved = !topic.approved? + assert topic.save! + topic.approved = !topic.approved? + assert topic.save! end + Topic.connection.close end - - threads.each(&:join) end + + threads.each(&:join) end # Test for dirty reads among simultaneous transactions. diff --git a/activerecord/test/fixtures/other_posts.yml b/activerecord/test/fixtures/other_posts.yml index 39ff763547..3e11a33802 100644 --- a/activerecord/test/fixtures/other_posts.yml +++ b/activerecord/test/fixtures/other_posts.yml @@ -5,3 +5,4 @@ second_welcome: author_id: 1 title: Welcome to the another weblog body: It's really nice today + comments_count: 1 diff --git a/activerecord/test/fixtures/posts.yml b/activerecord/test/fixtures/posts.yml index 86d46f753a..8d7e1e0ae7 100644 --- a/activerecord/test/fixtures/posts.yml +++ b/activerecord/test/fixtures/posts.yml @@ -28,6 +28,7 @@ sti_comments: author_id: 1 title: sti comments body: hello + comments_count: 5 type: Post sti_post_and_comments: @@ -35,6 +36,7 @@ sti_post_and_comments: author_id: 1 title: sti me body: hello + comments_count: 2 type: StiPost sti_habtm: @@ -50,6 +52,8 @@ eager_other: title: eager loading with OR'd conditions body: hello type: Post + comments_count: 1 + tags_count: 3 misc_by_bob: id: 8 @@ -57,6 +61,7 @@ misc_by_bob: title: misc post by bob body: hello type: Post + tags_count: 1 misc_by_mary: id: 9 @@ -64,6 +69,7 @@ misc_by_mary: title: misc post by mary body: hello type: Post + tags_count: 1 other_by_bob: id: 10 @@ -71,6 +77,7 @@ other_by_bob: title: other post by bob body: hello type: Post + tags_count: 1 other_by_mary: id: 11 @@ -78,3 +85,4 @@ other_by_mary: title: other post by mary body: hello type: Post + tags_count: 1 diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index 740aa593ac..5ab433f2d9 100644 --- a/activerecord/test/models/comment.rb +++ b/activerecord/test/models/comment.rb @@ -60,6 +60,10 @@ end class SpecialComment < Comment default_scope { where(deleted_at: nil) } + + def self.what_are_you + "a special comment..." + end end class SubSpecialComment < SpecialComment diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 8f872c38ba..a4505a4892 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -191,6 +191,7 @@ ActiveRecord::Schema.define do t.string :resource_id t.string :resource_type t.integer :developer_id + t.datetime :updated_at t.datetime :deleted_at t.integer :comments end diff --git a/activestorage/app/jobs/active_storage/analyze_job.rb b/activestorage/app/jobs/active_storage/analyze_job.rb index a11a73d030..2a952f9f74 100644 --- a/activestorage/app/jobs/active_storage/analyze_job.rb +++ b/activestorage/app/jobs/active_storage/analyze_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # Provides asynchronous analysis of ActiveStorage::Blob records via ActiveStorage::Blob#analyze_later. -class ActiveStorage::AnalyzeJob < ActiveJob::Base +class ActiveStorage::AnalyzeJob < ActiveStorage::BaseJob def perform(blob) blob.analyze end diff --git a/activestorage/app/jobs/active_storage/base_job.rb b/activestorage/app/jobs/active_storage/base_job.rb new file mode 100644 index 0000000000..6caab42a2d --- /dev/null +++ b/activestorage/app/jobs/active_storage/base_job.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class ActiveStorage::BaseJob < ActiveJob::Base + queue_as { ActiveStorage.queue } +end diff --git a/activestorage/app/jobs/active_storage/purge_job.rb b/activestorage/app/jobs/active_storage/purge_job.rb index 188840f702..98874d2250 100644 --- a/activestorage/app/jobs/active_storage/purge_job.rb +++ b/activestorage/app/jobs/active_storage/purge_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # Provides asynchronous purging of ActiveStorage::Blob records via ActiveStorage::Blob#purge_later. -class ActiveStorage::PurgeJob < ActiveJob::Base +class ActiveStorage::PurgeJob < ActiveStorage::BaseJob # FIXME: Limit this to a custom ActiveStorage error retry_on StandardError diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index cfdb2a8acd..d1ff6b7032 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -38,6 +38,7 @@ module ActiveStorage mattr_accessor :logger mattr_accessor :verifier + mattr_accessor :queue mattr_accessor :previewers, default: [] mattr_accessor :analyzers, default: [] end diff --git a/activestorage/lib/active_storage/downloading.rb b/activestorage/lib/active_storage/downloading.rb index ceb7cce0c7..3dac6b116a 100644 --- a/activestorage/lib/active_storage/downloading.rb +++ b/activestorage/lib/active_storage/downloading.rb @@ -3,9 +3,9 @@ module ActiveStorage module Downloading private - # Opens a new tempfile and copies blob data into it. Yields the tempfile. + # Opens a new tempfile in #tempdir and copies blob data into it. Yields the tempfile. def download_blob_to_tempfile # :doc: - Tempfile.open("ActiveStorage") do |file| + Tempfile.open("ActiveStorage", tempdir) do |file| download_blob_to file yield file end @@ -17,5 +17,10 @@ module ActiveStorage blob.download { |chunk| file.write(chunk) } file.rewind end + + # Returns the directory in which tempfiles should be opened. Defaults to +Dir.tmpdir+. + def tempdir # :doc: + Dir.tmpdir + end end end diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index a01a14cd83..6cf6635c4f 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -19,9 +19,12 @@ module ActiveStorage config.eager_load_namespaces << ActiveStorage - initializer "active_storage.logger" do + initializer "active_storage.configs" do config.after_initialize do |app| - ActiveStorage.logger = app.config.active_storage.logger || Rails.logger + ActiveStorage.logger = app.config.active_storage.logger || Rails.logger + ActiveStorage.queue = app.config.active_storage.queue + ActiveStorage.previewers = app.config.active_storage.previewers || [] + ActiveStorage.analyzers = app.config.active_storage.analyzers || [] end end @@ -65,17 +68,5 @@ module ActiveStorage end end end - - initializer "active_storage.previewers" do - config.after_initialize do |app| - ActiveStorage.previewers = app.config.active_storage.previewers || [] - end - end - - initializer "active_storage.analyzers" do - config.after_initialize do |app| - ActiveStorage.analyzers = app.config.active_storage.analyzers || [] - end - end end end diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb index 460f6d5678..ed75bae3b5 100644 --- a/activestorage/lib/active_storage/previewer.rb +++ b/activestorage/lib/active_storage/previewer.rb @@ -40,8 +40,10 @@ module ActiveStorage # end # end # end + # + # The output tempfile is opened in the directory returned by ActiveStorage::Downloading#tempdir. def draw(*argv) # :doc: - Tempfile.open("ActiveStorage") do |file| + Tempfile.open("ActiveStorage", tempdir) do |file| capture(*argv, to: file) yield file end diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index 27dd192ce6..f3877ad9c9 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -28,7 +28,7 @@ module ActiveStorage end end - def download(key) + def download(key, &block) if block_given? instrument :streaming_download, key do stream(key, &block) @@ -108,15 +108,15 @@ module ActiveStorage end # Reads the object for the given key in chunks, yielding each to the block. - def stream(key, options = {}, &block) + def stream(key) blob = blob_for(key) chunk_size = 5.megabytes offset = 0 while offset < blob.properties[:content_length] - _, io = blobs.get_blob(container, key, start_range: offset, end_range: offset + chunk_size - 1) - yield io + _, chunk = blobs.get_blob(container, key, start_range: offset, end_range: offset + chunk_size - 1) + yield chunk.force_encoding(Encoding::BINARY) offset += chunk_size end end diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 3e93cdd072..6957119780 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -26,7 +26,7 @@ module ActiveStorage end end - def download(key) + def download(key, &block) if block_given? instrument :streaming_download, key do stream(key, &block) @@ -85,14 +85,14 @@ module ActiveStorage end # Reads the object for the given key in chunks, yielding each to the block. - def stream(key, options = {}, &block) + def stream(key) object = object_for(key) chunk_size = 5.megabytes offset = 0 while offset < object.content_length - yield object.read(options.merge(range: "bytes=#{offset}-#{offset + chunk_size - 1}")) + yield object.get(range: "bytes=#{offset}-#{offset + chunk_size - 1}").body.read.force_encoding(Encoding::BINARY) offset += chunk_size end end diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb index a9e1cb6ce9..ade91ab89a 100644 --- a/activestorage/test/service/shared_service_tests.rb +++ b/activestorage/test/service/shared_service_tests.rb @@ -50,6 +50,16 @@ module ActiveStorage::Service::SharedServiceTests assert_equal FIXTURE_DATA, @service.download(FIXTURE_KEY) end + test "downloading in chunks" do + chunks = [] + + @service.download(FIXTURE_KEY) do |chunk| + chunks << chunk + end + + assert_equal [ FIXTURE_DATA ], chunks + end + test "existing" do assert @service.exist?(FIXTURE_KEY) assert_not @service.exist?(FIXTURE_KEY + "nonsense") diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 8e8e9b9440..780c887b49 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,108 @@ +* Allow `Range#include?` on TWZ ranges + + In #11474 we prevented TWZ ranges being iterated over which matched + Ruby's handling of Time ranges and as a consequence `include?` + stopped working with both Time ranges and TWZ ranges. However in + ruby/ruby@b061634 support was added for `include?` to use `cover?` + for 'linear' objects. Since we have no way of making Ruby consider + TWZ instances as 'linear' we have to override `Range#include?`. + + Fixes #30799. + + *Andrew White* + +* Fix acronym support in `humanize` + + Acronym inflections are stored with lowercase keys in the hash but + the match wasn't being lowercased before being looked up in the hash. + This shouldn't have any performance impact because before it would + fail to find the acronym and perform the `downcase` operation anyway. + + Fixes #31052. + + *Andrew White* + +* Add same method signature for `Time#prev_year` and `Time#next_year` + in accordance with `Date#prev_year`, `Date#next_year`. + + Allows pass argument for `Time#prev_year` and `Time#next_year`. + + Before: + ``` + Time.new(2017, 9, 16, 17, 0).prev_year # => 2016-09-16 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).prev_year(1) + # => ArgumentError: wrong number of arguments (given 1, expected 0) + + Time.new(2017, 9, 16, 17, 0).next_year # => 2018-09-16 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).next_year(1) + # => ArgumentError: wrong number of arguments (given 1, expected 0) + ``` + + After: + ``` + Time.new(2017, 9, 16, 17, 0).prev_year # => 2016-09-16 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).prev_year(1) # => 2016-09-16 17:00:00 +0300 + + Time.new(2017, 9, 16, 17, 0).next_year # => 2018-09-16 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).next_year(1) # => 2018-09-16 17:00:00 +0300 + ``` + + *bogdanvlviv* + +* Add same method signature for `Time#prev_month` and `Time#next_month` + in accordance with `Date#prev_month`, `Date#next_month`. + + Allows pass argument for `Time#prev_month` and `Time#next_month`. + + Before: + ``` + Time.new(2017, 9, 16, 17, 0).prev_month # => 2017-08-16 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).prev_month(1) + # => ArgumentError: wrong number of arguments (given 1, expected 0) + + Time.new(2017, 9, 16, 17, 0).next_month # => 2017-10-16 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).next_month(1) + # => ArgumentError: wrong number of arguments (given 1, expected 0) + ``` + + After: + ``` + Time.new(2017, 9, 16, 17, 0).prev_month # => 2017-08-16 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).prev_month(1) # => 2017-08-16 17:00:00 +0300 + + Time.new(2017, 9, 16, 17, 0).next_month # => 2017-10-16 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).next_month(1) # => 2017-10-16 17:00:00 +0300 + ``` + + *bogdanvlviv* + +* Add same method signature for `Time#prev_day` and `Time#next_day` + in accordance with `Date#prev_day`, `Date#next_day`. + + Allows pass argument for `Time#prev_day` and `Time#next_day`. + + Before: + ``` + Time.new(2017, 9, 16, 17, 0).prev_day # => 2017-09-15 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).prev_day(1) + # => ArgumentError: wrong number of arguments (given 1, expected 0) + + Time.new(2017, 9, 16, 17, 0).next_day # => 2017-09-17 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).next_day(1) + # => ArgumentError: wrong number of arguments (given 1, expected 0) + ``` + + After: + ``` + Time.new(2017, 9, 16, 17, 0).prev_day # => 2017-09-15 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).prev_day(1) # => 2017-09-15 17:00:00 +0300 + + Time.new(2017, 9, 16, 17, 0).next_day # => 2017-09-17 17:00:00 +0300 + Time.new(2017, 9, 16, 17, 0).next_day(1) # => 2017-09-17 17:00:00 +0300 + ``` + + *bogdanvlviv* + * `IO#to_json` now returns the `to_s` representation, rather than attempting to convert to an array. This fixes a bug where `IO#to_json` would raise an `IOError` when called on an unreadable object. diff --git a/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb b/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb index 265c6949e1..061b79e098 100644 --- a/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb @@ -20,9 +20,9 @@ module DateAndTime advance(days: -1) end - # Returns a new date/time representing the previous day. - def prev_day - advance(days: -1) + # Returns a new date/time the specified number of days ago. + def prev_day(days = 1) + advance(days: -days) end # Returns a new date/time representing tomorrow. @@ -30,9 +30,9 @@ module DateAndTime advance(days: 1) end - # Returns a new date/time representing the next day. - def next_day - advance(days: 1) + # Returns a new date/time the specified number of days in the future. + def next_day(days = 1) + advance(days: days) end # Returns true if the date/time is today. @@ -188,9 +188,9 @@ module DateAndTime end end - # Short-hand for months_since(1). - def next_month - months_since(1) + # Returns a new date/time the specified number of months in the future. + def next_month(months = 1) + advance(months: months) end # Short-hand for months_since(3) @@ -198,9 +198,9 @@ module DateAndTime months_since(3) end - # Short-hand for years_since(1). - def next_year - years_since(1) + # Returns a new date/time the specified number of years in the future. + def next_year(years = 1) + advance(years: years) end # Returns a new date/time representing the given day in the previous week. @@ -223,11 +223,15 @@ module DateAndTime end alias_method :last_weekday, :prev_weekday + # Returns a new date/time the specified number of months ago. + def prev_month(months = 1) + advance(months: -months) + end + # Short-hand for months_ago(1). - def prev_month + def last_month months_ago(1) end - alias_method :last_month, :prev_month # Short-hand for months_ago(3). def prev_quarter @@ -235,11 +239,15 @@ module DateAndTime end alias_method :last_quarter, :prev_quarter + # Returns a new date/time the specified number of years ago. + def prev_year(years = 1) + advance(years: -years) + end + # Short-hand for years_ago(1). - def prev_year + def last_year years_ago(1) end - alias_method :last_year, :prev_year # Returns the number of days to the start of the week on the given day. # Week is assumed to start on +start_day+, default is diff --git a/activesupport/lib/active_support/core_ext/range.rb b/activesupport/lib/active_support/core_ext/range.rb index 51ae0ddd21..4074e91d17 100644 --- a/activesupport/lib/active_support/core_ext/range.rb +++ b/activesupport/lib/active_support/core_ext/range.rb @@ -2,5 +2,6 @@ require "active_support/core_ext/range/conversions" require "active_support/core_ext/range/include_range" +require "active_support/core_ext/range/include_time_with_zone" require "active_support/core_ext/range/overlaps" require "active_support/core_ext/range/each" diff --git a/activesupport/lib/active_support/core_ext/range/each.rb b/activesupport/lib/active_support/core_ext/range/each.rb index cdff6393d7..2f22cd0e92 100644 --- a/activesupport/lib/active_support/core_ext/range/each.rb +++ b/activesupport/lib/active_support/core_ext/range/each.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/time_with_zone" + module ActiveSupport module EachTimeWithZone #:nodoc: def each(&block) @@ -15,7 +17,7 @@ module ActiveSupport private def ensure_iteration_allowed - raise TypeError, "can't iterate from #{first.class}" if first.is_a?(Time) + raise TypeError, "can't iterate from #{first.class}" if first.is_a?(TimeWithZone) end end end diff --git a/activesupport/lib/active_support/core_ext/range/include_time_with_zone.rb b/activesupport/lib/active_support/core_ext/range/include_time_with_zone.rb new file mode 100644 index 0000000000..5f80acf68e --- /dev/null +++ b/activesupport/lib/active_support/core_ext/range/include_time_with_zone.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require "active_support/time_with_zone" + +module ActiveSupport + module IncludeTimeWithZone #:nodoc: + # Extends the default Range#include? to support ActiveSupport::TimeWithZone. + # + # (1.hour.ago..1.hour.from_now).include?(Time.current) # => true + # + def include?(value) + if first.is_a?(TimeWithZone) + cover?(value) + elsif last.is_a?(TimeWithZone) + cover?(value) + else + super + end + end + end +end + +Range.prepend(ActiveSupport::IncludeTimeWithZone) diff --git a/activesupport/lib/active_support/inflector/methods.rb b/activesupport/lib/active_support/inflector/methods.rb index 72de42b1c3..3357b5eb32 100644 --- a/activesupport/lib/active_support/inflector/methods.rb +++ b/activesupport/lib/active_support/inflector/methods.rb @@ -138,7 +138,7 @@ module ActiveSupport result.tr!("_".freeze, " ".freeze) result.gsub!(/([a-z\d]*)/i) do |match| - "#{inflections.acronyms[match] || match.downcase}" + "#{inflections.acronyms[match.downcase] || match.downcase}" end if capitalize diff --git a/activesupport/lib/active_support/inflector/transliterate.rb b/activesupport/lib/active_support/inflector/transliterate.rb index 9801f1d118..9fb3a2e0af 100644 --- a/activesupport/lib/active_support/inflector/transliterate.rb +++ b/activesupport/lib/active_support/inflector/transliterate.rb @@ -71,17 +71,23 @@ module ActiveSupport # a 'pretty' URL. # # parameterize("Donald E. Knuth") # => "donald-e-knuth" - # parameterize("^trés|Jolie-- ") # => "tres-jolie" + # parameterize("^très|Jolie-- ") # => "tres-jolie" # # To use a custom separator, override the `separator` argument. # # parameterize("Donald E. Knuth", separator: '_') # => "donald_e_knuth" - # parameterize("^trés|Jolie-- ", separator: '_') # => "tres_jolie" + # parameterize("^très|Jolie__ ", separator: '_') # => "tres_jolie" # # To preserve the case of the characters in a string, use the `preserve_case` argument. # # parameterize("Donald E. Knuth", preserve_case: true) # => "Donald-E-Knuth" - # parameterize("^trés|Jolie-- ", preserve_case: true) # => "tres-Jolie" + # parameterize("^très|Jolie-- ", preserve_case: true) # => "tres-Jolie" + # + # It preserves dashes and underscores unless they are used as separators: + # + # parameterize("^très|Jolie__ ") # => "tres-jolie__" + # parameterize("^très|Jolie-- ", separator: "_") # => "tres_jolie--" + # parameterize("^très_Jolie-- ", separator: ".") # => "tres_jolie--" # def parameterize(string, separator: "-", preserve_case: false) # Replace accented chars with their ASCII equivalents. diff --git a/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb b/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb index b7ad76bb62..eb528a0583 100644 --- a/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb +++ b/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb @@ -37,14 +37,6 @@ module ActiveSupport private - def calculate_rounded_number(multiplier) - (number / BigDecimal.new(multiplier.to_f.to_s)).round * multiplier - end - - def digit_count(number) - number.zero? ? 1 : (Math.log10(absolute_number(number)) + 1).floor - end - def strip_insignificant_zeros options[:strip_insignificant_zeros] end diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb index e2bc51ff7a..b24aa36ede 100644 --- a/activesupport/lib/active_support/testing/assertions.rb +++ b/activesupport/lib/active_support/testing/assertions.rb @@ -159,7 +159,7 @@ module ActiveSupport if to == UNTRACKED error = "#{expression.inspect} didn't change" error = "#{message}.\n#{error}" if message - assert_not_equal before, after, error + assert before != after, error else error = "#{expression.inspect} didn't change to #{to}" error = "#{message}.\n#{error}" if message @@ -190,7 +190,7 @@ module ActiveSupport error = "#{expression.inspect} did change to #{after}" error = "#{message}.\n#{error}" if message - assert_equal before, after, error + assert before == after, error retval end diff --git a/activesupport/test/core_ext/date_and_time_behavior.rb b/activesupport/test/core_ext/date_and_time_behavior.rb index 256353c309..42da6f6cd0 100644 --- a/activesupport/test/core_ext/date_and_time_behavior.rb +++ b/activesupport/test/core_ext/date_and_time_behavior.rb @@ -9,6 +9,11 @@ module DateAndTimeBehavior end def test_prev_day + assert_equal date_time_init(2005, 2, 24, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_day(-2) + assert_equal date_time_init(2005, 2, 23, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_day(-1) + assert_equal date_time_init(2005, 2, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_day(0) + assert_equal date_time_init(2005, 2, 21, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_day(1) + assert_equal date_time_init(2005, 2, 20, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_day(2) assert_equal date_time_init(2005, 2, 21, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_day assert_equal date_time_init(2005, 2, 28, 10, 10, 10), date_time_init(2005, 3, 2, 10, 10, 10).prev_day.prev_day end @@ -19,6 +24,11 @@ module DateAndTimeBehavior end def test_next_day + assert_equal date_time_init(2005, 2, 20, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_day(-2) + assert_equal date_time_init(2005, 2, 21, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_day(-1) + assert_equal date_time_init(2005, 2, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_day(0) + assert_equal date_time_init(2005, 2, 23, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_day(1) + assert_equal date_time_init(2005, 2, 24, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_day(2) assert_equal date_time_init(2005, 2, 23, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_day assert_equal date_time_init(2005, 3, 2, 10, 10, 10), date_time_init(2005, 2, 28, 10, 10, 10).next_day.next_day end @@ -151,6 +161,16 @@ module DateAndTimeBehavior assert_equal date_time_init(2015, 1, 5, 15, 15, 10), date_time_init(2015, 1, 3, 15, 15, 10).next_weekday end + def test_next_month + assert_equal date_time_init(2004, 12, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_month(-2) + assert_equal date_time_init(2005, 1, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_month(-1) + assert_equal date_time_init(2005, 2, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_month(0) + assert_equal date_time_init(2005, 3, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_month(1) + assert_equal date_time_init(2005, 4, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_month(2) + assert_equal date_time_init(2005, 3, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_month + assert_equal date_time_init(2005, 4, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).next_month.next_month + end + def test_next_month_on_31st assert_equal date_time_init(2005, 9, 30, 15, 15, 10), date_time_init(2005, 8, 31, 15, 15, 10).next_month end @@ -160,7 +180,13 @@ module DateAndTimeBehavior end def test_next_year + assert_equal date_time_init(2003, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).next_year(-2) + assert_equal date_time_init(2004, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).next_year(-1) + assert_equal date_time_init(2005, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).next_year(0) + assert_equal date_time_init(2006, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).next_year(1) + assert_equal date_time_init(2007, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).next_year(2) assert_equal date_time_init(2006, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).next_year + assert_equal date_time_init(2007, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).next_year.next_year end def test_prev_week @@ -203,6 +229,16 @@ module DateAndTimeBehavior assert_equal date_time_init(2015, 1, 2, 15, 15, 10), date_time_init(2015, 1, 4, 15, 15, 10).prev_weekday end + def test_prev_month + assert_equal date_time_init(2005, 4, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_month(-2) + assert_equal date_time_init(2005, 3, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_month(-1) + assert_equal date_time_init(2005, 2, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_month(0) + assert_equal date_time_init(2005, 1, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_month(1) + assert_equal date_time_init(2004, 12, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_month(2) + assert_equal date_time_init(2005, 1, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_month + assert_equal date_time_init(2004, 12, 22, 10, 10, 10), date_time_init(2005, 2, 22, 10, 10, 10).prev_month.prev_month + end + def test_prev_month_on_31st assert_equal date_time_init(2004, 2, 29, 10, 10, 10), date_time_init(2004, 3, 31, 10, 10, 10).prev_month end @@ -212,7 +248,21 @@ module DateAndTimeBehavior end def test_prev_year + assert_equal date_time_init(2007, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).prev_year(-2) + assert_equal date_time_init(2006, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).prev_year(-1) + assert_equal date_time_init(2005, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).prev_year(0) + assert_equal date_time_init(2004, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).prev_year(1) + assert_equal date_time_init(2003, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).prev_year(2) assert_equal date_time_init(2004, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).prev_year + assert_equal date_time_init(2003, 6, 5, 10, 10, 10), date_time_init(2005, 6, 5, 10, 10, 10).prev_year.prev_year + end + + def test_last_month_on_31st + assert_equal date_time_init(2004, 2, 29, 0, 0, 0), date_time_init(2004, 3, 31, 0, 0, 0).last_month + end + + def test_last_year + assert_equal date_time_init(2004, 6, 5, 10, 0, 0), date_time_init(2005, 6, 5, 10, 0, 0).last_year end def test_days_to_week_start diff --git a/activesupport/test/core_ext/date_ext_test.rb b/activesupport/test/core_ext/date_ext_test.rb index b8672eac4b..0c6f3f595a 100644 --- a/activesupport/test/core_ext/date_ext_test.rb +++ b/activesupport/test/core_ext/date_ext_test.rb @@ -120,10 +120,6 @@ class DateExtCalculationsTest < ActiveSupport::TestCase assert_equal Date.new(1582, 10, 4), Date.new(1583, 10, 14).prev_year end - def test_last_year - assert_equal Date.new(2004, 6, 5), Date.new(2005, 6, 5).last_year - end - def test_last_year_in_leap_years assert_equal Date.new(1999, 2, 28), Date.new(2000, 2, 29).last_year end @@ -185,10 +181,6 @@ class DateExtCalculationsTest < ActiveSupport::TestCase assert_equal Date.new(1582, 10, 18), Date.new(1582, 10, 4).next_week end - def test_last_month_on_31st - assert_equal Date.new(2004, 2, 29), Date.new(2004, 3, 31).last_month - end - def test_last_quarter_on_31st assert_equal Date.new(2004, 2, 29), Date.new(2004, 5, 31).last_quarter end diff --git a/activesupport/test/core_ext/date_time_ext_test.rb b/activesupport/test/core_ext/date_time_ext_test.rb index a3c2018a31..d942cddb2a 100644 --- a/activesupport/test/core_ext/date_time_ext_test.rb +++ b/activesupport/test/core_ext/date_time_ext_test.rb @@ -162,10 +162,6 @@ class DateTimeExtCalculationsTest < ActiveSupport::TestCase assert_equal DateTime.civil(2005, 4, 30, 23, 59, Rational(59999999999, 1000000000)), DateTime.civil(2005, 4, 20, 10, 10, 10).end_of_month end - def test_last_year - assert_equal DateTime.civil(2004, 6, 5, 10), DateTime.civil(2005, 6, 5, 10, 0, 0).last_year - end - def test_ago assert_equal DateTime.civil(2005, 2, 22, 10, 10, 9), DateTime.civil(2005, 2, 22, 10, 10, 10).ago(1) assert_equal DateTime.civil(2005, 2, 22, 9, 10, 10), DateTime.civil(2005, 2, 22, 10, 10, 10).ago(3600) @@ -248,10 +244,6 @@ class DateTimeExtCalculationsTest < ActiveSupport::TestCase assert_equal DateTime.civil(2016, 2, 29), DateTime.civil(2016, 3, 7).last_week end - def test_last_month_on_31st - assert_equal DateTime.civil(2004, 2, 29), DateTime.civil(2004, 3, 31).last_month - end - def test_last_quarter_on_31st assert_equal DateTime.civil(2004, 2, 29), DateTime.civil(2004, 5, 31).last_quarter end diff --git a/activesupport/test/core_ext/range_ext_test.rb b/activesupport/test/core_ext/range_ext_test.rb index 0467123e55..049fac8fd4 100644 --- a/activesupport/test/core_ext/range_ext_test.rb +++ b/activesupport/test/core_ext/range_ext_test.rb @@ -121,9 +121,12 @@ class RangeTest < ActiveSupport::TestCase def test_include_on_time_with_zone twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone["Eastern Time (US & Canada)"] , Time.utc(2006, 11, 28, 10, 30)) - assert_raises TypeError do - ((twz - 1.hour)..twz).include?(twz) - end + assert ((twz - 1.hour)..twz).include?(twz) + end + + def test_case_equals_on_time_with_zone + twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone["Eastern Time (US & Canada)"] , Time.utc(2006, 11, 28, 10, 30)) + assert ((twz - 1.hour)..twz) === twz end def test_date_time_with_each diff --git a/activesupport/test/core_ext/time_ext_test.rb b/activesupport/test/core_ext/time_ext_test.rb index dc2f4c5ac7..8cb17df01b 100644 --- a/activesupport/test/core_ext/time_ext_test.rb +++ b/activesupport/test/core_ext/time_ext_test.rb @@ -178,10 +178,6 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase assert_equal Time.local(2005, 2, 4, 19, 30, 59, Rational(999999999, 1000)), Time.local(2005, 2, 4, 19, 30, 10).end_of_minute end - def test_last_year - assert_equal Time.local(2004, 6, 5, 10), Time.local(2005, 6, 5, 10, 0, 0).last_year - end - def test_ago assert_equal Time.local(2005, 2, 22, 10, 10, 9), Time.local(2005, 2, 22, 10, 10, 10).ago(1) assert_equal Time.local(2005, 2, 22, 9, 10, 10), Time.local(2005, 2, 22, 10, 10, 10).ago(3600) @@ -664,10 +660,6 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase end end - def test_last_month_on_31st - assert_equal Time.local(2004, 2, 29), Time.local(2004, 3, 31).last_month - end - def test_xmlschema_is_available assert_nothing_raised { Time.now.xmlschema } end diff --git a/activesupport/test/inflector_test.rb b/activesupport/test/inflector_test.rb index eeec0ab1a5..6cc1039d8e 100644 --- a/activesupport/test/inflector_test.rb +++ b/activesupport/test/inflector_test.rb @@ -356,6 +356,19 @@ class InflectorTest < ActiveSupport::TestCase assert_equal("Col rpted bugs", ActiveSupport::Inflector.humanize("COL_rpted_bugs")) end + def test_humanize_with_acronyms + ActiveSupport::Inflector.inflections do |inflect| + inflect.acronym "LAX" + inflect.acronym "SFO" + end + assert_equal("LAX roundtrip to SFO", ActiveSupport::Inflector.humanize("LAX ROUNDTRIP TO SFO")) + assert_equal("LAX roundtrip to SFO", ActiveSupport::Inflector.humanize("LAX ROUNDTRIP TO SFO", capitalize: false)) + assert_equal("LAX roundtrip to SFO", ActiveSupport::Inflector.humanize("lax roundtrip to sfo")) + assert_equal("LAX roundtrip to SFO", ActiveSupport::Inflector.humanize("lax roundtrip to sfo", capitalize: false)) + assert_equal("LAX roundtrip to SFO", ActiveSupport::Inflector.humanize("Lax Roundtrip To Sfo")) + assert_equal("LAX roundtrip to SFO", ActiveSupport::Inflector.humanize("Lax Roundtrip To Sfo", capitalize: false)) + end + def test_constantize run_constantize_tests_on do |string| ActiveSupport::Inflector.constantize(string) diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb index 9bc9183668..84e4953fe2 100644 --- a/activesupport/test/test_case_test.rb +++ b/activesupport/test/test_case_test.rb @@ -179,6 +179,7 @@ class AssertDifferenceTest < ActiveSupport::TestCase end def test_assert_changes_works_with_any_object + # Silences: instance variable @new_object not initialized. retval = silence_warnings do assert_changes :@new_object, from: nil, to: 42 do @new_object = 42 @@ -201,7 +202,7 @@ class AssertDifferenceTest < ActiveSupport::TestCase def test_assert_changes_with_to_and_case_operator token = nil - assert_changes -> { token }, to: /\w{32}/ do + assert_changes -> { token }, to: /\w{32}/ do token = SecureRandom.hex end end @@ -236,7 +237,7 @@ class AssertDifferenceTest < ActiveSupport::TestCase end end - assert_equal "@object.num should not change.\n\"@object.num\" did change to 1.\nExpected: 0\n Actual: 1", error.message + assert_equal "@object.num should not change.\n\"@object.num\" did change to 1", error.message end end diff --git a/guides/bug_report_templates/action_controller_master.rb b/guides/bug_report_templates/action_controller_master.rb index cf76de80d2..932d329943 100644 --- a/guides/bug_report_templates/action_controller_master.rb +++ b/guides/bug_report_templates/action_controller_master.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +gem "bundler", "< 1.16" + begin require "bundler/inline" rescue LoadError => e diff --git a/guides/bug_report_templates/active_job_master.rb b/guides/bug_report_templates/active_job_master.rb index ce480cbb52..36d9137b71 100644 --- a/guides/bug_report_templates/active_job_master.rb +++ b/guides/bug_report_templates/active_job_master.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +gem "bundler", "< 1.16" + begin require "bundler/inline" rescue LoadError => e diff --git a/guides/bug_report_templates/active_record_master.rb b/guides/bug_report_templates/active_record_master.rb index 78411e2d57..b66deb36f3 100644 --- a/guides/bug_report_templates/active_record_master.rb +++ b/guides/bug_report_templates/active_record_master.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +gem "bundler", "< 1.16" + begin require "bundler/inline" rescue LoadError => e diff --git a/guides/bug_report_templates/active_record_migrations_master.rb b/guides/bug_report_templates/active_record_migrations_master.rb index fce8d1d848..737ce66d7b 100644 --- a/guides/bug_report_templates/active_record_migrations_master.rb +++ b/guides/bug_report_templates/active_record_migrations_master.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +gem "bundler", "< 1.16" + begin require "bundler/inline" rescue LoadError => e diff --git a/guides/bug_report_templates/benchmark.rb b/guides/bug_report_templates/benchmark.rb index fb51273e3e..f5c88086a9 100644 --- a/guides/bug_report_templates/benchmark.rb +++ b/guides/bug_report_templates/benchmark.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +gem "bundler", "< 1.16" + begin require "bundler/inline" rescue LoadError => e diff --git a/guides/bug_report_templates/generic_master.rb b/guides/bug_report_templates/generic_master.rb index 384c8b1833..240571ba9a 100644 --- a/guides/bug_report_templates/generic_master.rb +++ b/guides/bug_report_templates/generic_master.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +gem "bundler", "< 1.16" + begin require "bundler/inline" rescue LoadError => e diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index 54d3dec1c2..66d2fbd939 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -2970,6 +2970,32 @@ Extensions to `Date` NOTE: All the following methods are defined in `active_support/core_ext/date/calculations.rb`. +```ruby +yesterday +tomorrow +beginning_of_week (at_beginning_of_week) +end_of_week (at_end_of_week) +monday +sunday +weeks_ago +prev_week (last_week) +next_week +months_ago +months_since +beginning_of_month (at_beginning_of_month) +end_of_month (at_end_of_month) +last_month +beginning_of_quarter (at_beginning_of_quarter) +end_of_quarter (at_end_of_quarter) +beginning_of_year (at_beginning_of_year) +end_of_year (at_end_of_year) +years_ago +years_since +last_year +on_weekday? +on_weekend? +``` + INFO: The following calculation methods have edge cases in October 1582, since days 5..14 just do not exist. This guide does not document their behavior around those days for brevity, but it is enough to say that they do what you would expect. That is, `Date.new(1582, 10, 4).tomorrow` returns `Date.new(1582, 10, 15)` and so on. Please check `test/core_ext/date_ext_test.rb` in the Active Support test suite for expected behavior. #### `Date.current` @@ -2980,68 +3006,6 @@ When making Date comparisons using methods which honor the user time zone, make #### Named dates -##### `prev_year`, `next_year` - -In Ruby 1.9 `prev_year` and `next_year` return a date with the same day/month in the last or next year: - -```ruby -d = Date.new(2010, 5, 8) # => Sat, 08 May 2010 -d.prev_year # => Fri, 08 May 2009 -d.next_year # => Sun, 08 May 2011 -``` - -If date is the 29th of February of a leap year, you obtain the 28th: - -```ruby -d = Date.new(2000, 2, 29) # => Tue, 29 Feb 2000 -d.prev_year # => Sun, 28 Feb 1999 -d.next_year # => Wed, 28 Feb 2001 -``` - -`prev_year` is aliased to `last_year`. - -##### `prev_month`, `next_month` - -In Ruby 1.9 `prev_month` and `next_month` return the date with the same day in the last or next month: - -```ruby -d = Date.new(2010, 5, 8) # => Sat, 08 May 2010 -d.prev_month # => Thu, 08 Apr 2010 -d.next_month # => Tue, 08 Jun 2010 -``` - -If such a day does not exist, the last day of the corresponding month is returned: - -```ruby -Date.new(2000, 5, 31).prev_month # => Sun, 30 Apr 2000 -Date.new(2000, 3, 31).prev_month # => Tue, 29 Feb 2000 -Date.new(2000, 5, 31).next_month # => Fri, 30 Jun 2000 -Date.new(2000, 1, 31).next_month # => Tue, 29 Feb 2000 -``` - -`prev_month` is aliased to `last_month`. - -##### `prev_quarter`, `next_quarter` - -Same as `prev_month` and `next_month`. It returns the date with the same day in the previous or next quarter: - -```ruby -t = Time.local(2010, 5, 8) # => Sat, 08 May 2010 -t.prev_quarter # => Mon, 08 Feb 2010 -t.next_quarter # => Sun, 08 Aug 2010 -``` - -If such a day does not exist, the last day of the corresponding month is returned: - -```ruby -Time.local(2000, 7, 31).prev_quarter # => Sun, 30 Apr 2000 -Time.local(2000, 5, 31).prev_quarter # => Tue, 29 Feb 2000 -Time.local(2000, 10, 31).prev_quarter # => Mon, 30 Oct 2000 -Time.local(2000, 11, 31).next_quarter # => Wed, 28 Feb 2001 -``` - -`prev_quarter` is aliased to `last_quarter`. - ##### `beginning_of_week`, `end_of_week` The methods `beginning_of_week` and `end_of_week` return the dates for the @@ -3159,6 +3123,8 @@ Date.new(2012, 2, 29).years_ago(3) # => Sat, 28 Feb 2009 Date.new(2012, 2, 29).years_since(3) # => Sat, 28 Feb 2015 ``` +`last_year` is short-hand for `#years_ago(1)`. + ##### `months_ago`, `months_since` The methods `months_ago` and `months_since` work analogously for months: @@ -3175,6 +3141,8 @@ Date.new(2010, 4, 30).months_ago(2) # => Sun, 28 Feb 2010 Date.new(2009, 12, 31).months_since(2) # => Sun, 28 Feb 2010 ``` +`last_month` is short-hand for `#months_ago(1)`. + ##### `weeks_ago` The method `weeks_ago` works analogously for weeks: @@ -3337,35 +3305,7 @@ WARNING: `DateTime` is not aware of DST rules and so some of these methods have NOTE: All the following methods are defined in `active_support/core_ext/date_time/calculations.rb`. -The class `DateTime` is a subclass of `Date` so by loading `active_support/core_ext/date/calculations.rb` you inherit these methods and their aliases, except that they will always return datetimes: - -```ruby -yesterday -tomorrow -beginning_of_week (at_beginning_of_week) -end_of_week (at_end_of_week) -monday -sunday -weeks_ago -prev_week (last_week) -next_week -months_ago -months_since -beginning_of_month (at_beginning_of_month) -end_of_month (at_end_of_month) -prev_month (last_month) -next_month -beginning_of_quarter (at_beginning_of_quarter) -end_of_quarter (at_end_of_quarter) -beginning_of_year (at_beginning_of_year) -end_of_year (at_end_of_year) -years_ago -years_since -prev_year (last_year) -next_year -on_weekday? -on_weekend? -``` +The class `DateTime` is a subclass of `Date` so by loading `active_support/core_ext/date/calculations.rb` you inherit these methods and their aliases, except that they will always return datetimes. The following methods are reimplemented so you do **not** need to load `active_support/core_ext/date/calculations.rb` for these ones: @@ -3513,8 +3453,6 @@ Extensions to `Time` NOTE: All the following methods are defined in `active_support/core_ext/time/calculations.rb`. -Active Support adds to `Time` many of the methods available for `DateTime`: - ```ruby past? today? @@ -3526,6 +3464,8 @@ change advance ago since (in) +prev_day +next_day beginning_of_day (midnight, at_midnight, at_beginning_of_day) end_of_day beginning_of_hour (at_beginning_of_hour) @@ -3541,15 +3481,17 @@ months_ago months_since beginning_of_month (at_beginning_of_month) end_of_month (at_end_of_month) -prev_month (last_month) +prev_month next_month +last_month beginning_of_quarter (at_beginning_of_quarter) end_of_quarter (at_end_of_quarter) beginning_of_year (at_beginning_of_year) end_of_year (at_end_of_year) years_ago years_since -prev_year (last_year) +prev_year +last_year next_year on_weekday? on_weekend? @@ -3607,6 +3549,74 @@ now.all_year # => Fri, 01 Jan 2010 00:00:00 UTC +00:00..Fri, 31 Dec 2010 23:59:59 UTC +00:00 ``` +#### `prev_day`, `next_day` + +In Ruby 1.9 `prev_day` and `next_day` return the date in the last or next day: + +```ruby +d = Date.new(2010, 5, 8) # => Sat, 08 May 2010 +d.prev_day # => Fri, 07 May 2010 +d.next_day # => Sun, 09 May 2010 +``` + +#### `prev_month`, `next_month` + +In Ruby 1.9 `prev_month` and `next_month` return the date with the same day in the last or next month: + +```ruby +d = Date.new(2010, 5, 8) # => Sat, 08 May 2010 +d.prev_month # => Thu, 08 Apr 2010 +d.next_month # => Tue, 08 Jun 2010 +``` + +If such a day does not exist, the last day of the corresponding month is returned: + +```ruby +Date.new(2000, 5, 31).prev_month # => Sun, 30 Apr 2000 +Date.new(2000, 3, 31).prev_month # => Tue, 29 Feb 2000 +Date.new(2000, 5, 31).next_month # => Fri, 30 Jun 2000 +Date.new(2000, 1, 31).next_month # => Tue, 29 Feb 2000 +``` + +#### `prev_year`, `next_year` + +In Ruby 1.9 `prev_year` and `next_year` return a date with the same day/month in the last or next year: + +```ruby +d = Date.new(2010, 5, 8) # => Sat, 08 May 2010 +d.prev_year # => Fri, 08 May 2009 +d.next_year # => Sun, 08 May 2011 +``` + +If date is the 29th of February of a leap year, you obtain the 28th: + +```ruby +d = Date.new(2000, 2, 29) # => Tue, 29 Feb 2000 +d.prev_year # => Sun, 28 Feb 1999 +d.next_year # => Wed, 28 Feb 2001 +``` + +#### `prev_quarter`, `next_quarter` + +`prev_quarter` and `next_quarter` return the date with the same day in the previous or next quarter: + +```ruby +t = Time.local(2010, 5, 8) # => 2010-05-08 00:00:00 +0300 +t.prev_quarter # => 2010-02-08 00:00:00 +0200 +t.next_quarter # => 2010-08-08 00:00:00 +0300 +``` + +If such a day does not exist, the last day of the corresponding month is returned: + +```ruby +Time.local(2000, 7, 31).prev_quarter # => 2000-04-30 00:00:00 +0300 +Time.local(2000, 5, 31).prev_quarter # => 2000-02-29 00:00:00 +0200 +Time.local(2000, 10, 31).prev_quarter # => 2000-07-31 00:00:00 +0300 +Time.local(2000, 11, 31).next_quarter # => 2001-03-01 00:00:00 +0200 +``` + +`prev_quarter` is aliased to `last_quarter`. + ### Time Constructors Active Support defines `Time.current` to be `Time.zone.now` if there's a user time zone defined, with fallback to `Time.now`: diff --git a/guides/source/autoloading_and_reloading_constants.md b/guides/source/autoloading_and_reloading_constants.md index ede0324a51..dea87a18f8 100644 --- a/guides/source/autoloading_and_reloading_constants.md +++ b/guides/source/autoloading_and_reloading_constants.md @@ -330,11 +330,17 @@ its resolution next. Let's define *parent* to be that qualifying class or module object, that is, `Billing` in the example above. The algorithm for qualified constants goes like this: -1. The constant is looked up in the parent and its ancestors. +1. The constant is looked up in the parent and its ancestors. In Ruby >= 2.5, +`Object` is skipped if present among the ancestors. `Kernel` and `BasicObject` +are still checked though. 2. If the lookup fails, `const_missing` is invoked in the parent. The default implementation of `const_missing` raises `NameError`, but it can be overridden. +INFO. In Ruby < 2.5 `String::Hash` evaluates to `Hash` and the interpreter +issues a warning: "toplevel constant Hash referenced by String::Hash". Starting +with 2.5, `String::Hash` raises `NameError` because `Object` is skipped. + As you see, this algorithm is simpler than the one for relative constants. In particular, the nesting plays no role here, and modules are not special-cased, if neither they nor their ancestors have the constants, `Object` is **not** @@ -1178,6 +1184,8 @@ end #### Qualified References +WARNING. This gotcha is only possible in Ruby < 2.5. + Given ```ruby diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 20603dedaf..e69a1231b1 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,9 @@ +* `rails new` and `rails plugin new` get `Active Storage` by default. + Add ability to skip `Active Storage` with `--skip-active-storage` + and do so automatically when `--skip-active-record` is used. + + *bogdanvlviv* + * Gemfile for new apps: upgrade redis-rb from ~> 3.0 to 4.0. *Jeremy Daer* diff --git a/railties/lib/rails/all.rb b/railties/lib/rails/all.rb index e55b2e2433..f5dccd2381 100644 --- a/railties/lib/rails/all.rb +++ b/railties/lib/rails/all.rb @@ -4,12 +4,12 @@ require "rails" %w( active_record/railtie + active_storage/engine action_controller/railtie action_view/railtie action_mailer/railtie active_job/railtie action_cable/engine - active_storage/engine rails/test_unit/railtie sprockets/railtie ).each do |railtie| diff --git a/railties/lib/rails/app_updater.rb b/railties/lib/rails/app_updater.rb index c2436a69f9..a076d082d5 100644 --- a/railties/lib/rails/app_updater.rb +++ b/railties/lib/rails/app_updater.rb @@ -21,11 +21,12 @@ module Rails private def generator_options options = { api: !!Rails.application.config.api_only, update: true } - options[:skip_active_record] = !defined?(ActiveRecord::Railtie) - options[:skip_action_mailer] = !defined?(ActionMailer::Railtie) - options[:skip_action_cable] = !defined?(ActionCable::Engine) - options[:skip_sprockets] = !defined?(Sprockets::Railtie) - options[:skip_puma] = !defined?(Puma) + options[:skip_active_record] = !defined?(ActiveRecord::Railtie) + options[:skip_active_storage] = !defined?(ActiveStorage::Engine) || !defined?(ActiveRecord::Railtie) + options[:skip_action_mailer] = !defined?(ActionMailer::Railtie) + options[:skip_action_cable] = !defined?(ActionCable::Engine) + options[:skip_sprockets] = !defined?(Sprockets::Railtie) + options[:skip_puma] = !defined?(Puma) options end end diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index 2d265818f7..5592e8d78e 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -218,6 +218,7 @@ module Rails rails.delete("app") rails.delete("plugin") rails.delete("encrypted_secrets") + rails.delete("credentials") hidden_namespaces.each { |n| groups.delete(n.to_s) } diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index 9800e5750a..3362bf629a 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -221,6 +221,7 @@ module Rails # rake("db:migrate") # rake("db:migrate", env: "production") # rake("gems:install", sudo: true) + # rake("gems:install", capture: true) def rake(command, options = {}) execute_command :rake, command, options end @@ -230,6 +231,7 @@ module Rails # rails_command("db:migrate") # rails_command("db:migrate", env: "production") # rails_command("gems:install", sudo: true) + # rails_command("gems:install", capture: true) def rails_command(command, options = {}) execute_command :rails, command, options end @@ -292,7 +294,11 @@ module Rails log executor, command env = options[:env] || ENV["RAILS_ENV"] || "development" sudo = options[:sudo] && !Gem.win_platform? ? "sudo " : "" - in_root { run("#{sudo}#{extify(executor)} #{command} RAILS_ENV=#{env}", verbose: false) } + config = { verbose: false } + + config.merge!(capture: options[:capture]) if options[:capture] + + in_root { run("#{sudo}#{extify(executor)} #{command} RAILS_ENV=#{env}", config) } end # Add an extension to the given name based on the platform. diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index bc00adabae..bdeddff645 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -26,75 +26,78 @@ module Rails end def self.add_shared_options_for(name) - class_option :template, type: :string, aliases: "-m", - desc: "Path to some #{name} template (can be a filesystem path or URL)" + class_option :template, type: :string, aliases: "-m", + desc: "Path to some #{name} template (can be a filesystem path or URL)" - class_option :database, type: :string, aliases: "-d", default: "sqlite3", - desc: "Preconfigure for selected database (options: #{DATABASES.join('/')})" + class_option :database, type: :string, aliases: "-d", default: "sqlite3", + desc: "Preconfigure for selected database (options: #{DATABASES.join('/')})" - class_option :skip_yarn, type: :boolean, default: false, - desc: "Don't use Yarn for managing JavaScript dependencies" + class_option :skip_yarn, type: :boolean, default: false, + desc: "Don't use Yarn for managing JavaScript dependencies" - class_option :skip_gemfile, type: :boolean, default: false, - desc: "Don't create a Gemfile" + class_option :skip_gemfile, type: :boolean, default: false, + desc: "Don't create a Gemfile" - class_option :skip_git, type: :boolean, aliases: "-G", default: false, - desc: "Skip .gitignore file" + class_option :skip_git, type: :boolean, aliases: "-G", default: false, + desc: "Skip .gitignore file" - class_option :skip_keeps, type: :boolean, default: false, - desc: "Skip source control .keep files" + class_option :skip_keeps, type: :boolean, default: false, + desc: "Skip source control .keep files" - class_option :skip_action_mailer, type: :boolean, aliases: "-M", - default: false, - desc: "Skip Action Mailer files" + class_option :skip_action_mailer, type: :boolean, aliases: "-M", + default: false, + desc: "Skip Action Mailer files" - class_option :skip_active_record, type: :boolean, aliases: "-O", default: false, - desc: "Skip Active Record files" + class_option :skip_active_record, type: :boolean, aliases: "-O", default: false, + desc: "Skip Active Record files" - class_option :skip_puma, type: :boolean, aliases: "-P", default: false, - desc: "Skip Puma related files" + class_option :skip_active_storage, type: :boolean, default: false, + desc: "Skip Active Storage files" - class_option :skip_action_cable, type: :boolean, aliases: "-C", default: false, - desc: "Skip Action Cable files" + class_option :skip_puma, type: :boolean, aliases: "-P", default: false, + desc: "Skip Puma related files" - class_option :skip_sprockets, type: :boolean, aliases: "-S", default: false, - desc: "Skip Sprockets files" + class_option :skip_action_cable, type: :boolean, aliases: "-C", default: false, + desc: "Skip Action Cable files" - class_option :skip_spring, type: :boolean, default: false, - desc: "Don't install Spring application preloader" + class_option :skip_sprockets, type: :boolean, aliases: "-S", default: false, + desc: "Skip Sprockets files" - class_option :skip_listen, type: :boolean, default: false, - desc: "Don't generate configuration that depends on the listen gem" + class_option :skip_spring, type: :boolean, default: false, + desc: "Don't install Spring application preloader" - class_option :skip_coffee, type: :boolean, default: false, - desc: "Don't use CoffeeScript" + class_option :skip_listen, type: :boolean, default: false, + desc: "Don't generate configuration that depends on the listen gem" - class_option :skip_javascript, type: :boolean, aliases: "-J", default: false, - desc: "Skip JavaScript files" + class_option :skip_coffee, type: :boolean, default: false, + desc: "Don't use CoffeeScript" - class_option :skip_turbolinks, type: :boolean, default: false, - desc: "Skip turbolinks gem" + class_option :skip_javascript, type: :boolean, aliases: "-J", default: false, + desc: "Skip JavaScript files" - class_option :skip_test, type: :boolean, aliases: "-T", default: false, - desc: "Skip test files" + class_option :skip_turbolinks, type: :boolean, default: false, + desc: "Skip turbolinks gem" - class_option :skip_system_test, type: :boolean, default: false, - desc: "Skip system test files" + class_option :skip_test, type: :boolean, aliases: "-T", default: false, + desc: "Skip test files" - class_option :dev, type: :boolean, default: false, - desc: "Setup the #{name} with Gemfile pointing to your Rails checkout" + class_option :skip_system_test, type: :boolean, default: false, + desc: "Skip system test files" - class_option :edge, type: :boolean, default: false, - desc: "Setup the #{name} with Gemfile pointing to Rails repository" + class_option :dev, type: :boolean, default: false, + desc: "Setup the #{name} with Gemfile pointing to your Rails checkout" - class_option :rc, type: :string, default: nil, - desc: "Path to file containing extra configuration options for rails command" + class_option :edge, type: :boolean, default: false, + desc: "Setup the #{name} with Gemfile pointing to Rails repository" - class_option :no_rc, type: :boolean, default: false, - desc: "Skip loading of extra configuration options from .railsrc file" + class_option :rc, type: :string, default: nil, + desc: "Path to file containing extra configuration options for rails command" - class_option :help, type: :boolean, aliases: "-h", group: :rails, - desc: "Show this help message and quit" + class_option :no_rc, type: :boolean, default: false, + desc: "Skip loading of extra configuration options from .railsrc file" + + class_option :help, type: :boolean, aliases: "-h", group: :rails, + desc: "Show this help message and quit" end def initialize(*args) @@ -193,11 +196,29 @@ module Rails end def include_all_railties? # :doc: - options.values_at(:skip_active_record, :skip_action_mailer, :skip_test, :skip_sprockets, :skip_action_cable).none? + [ + options.values_at( + :skip_active_record, + :skip_action_mailer, + :skip_test, + :skip_sprockets, + :skip_action_cable + ), + skip_active_storage? + ].flatten.none? end def comment_if(value) # :doc: - options[value] ? "# " : "" + question = "#{value}?" + + comment = + if respond_to?(question, true) + send(question) + else + options[value] + end + + comment ? "# " : "" end def keeps? # :doc: @@ -208,6 +229,10 @@ module Rails !options[:skip_active_record] && options[:database] == "sqlite3" end + def skip_active_storage? # :doc: + options[:skip_active_storage] || options[:skip_active_record] + end + class GemfileEntry < Struct.new(:name, :version, :comment, :options, :commented_out) def initialize(name, version, comment, options = {}, commented_out = false) super @@ -434,6 +459,16 @@ module Rails end end + def run_active_storage + unless skip_active_storage? + if bundle_install? + rails_command "active_storage:install", capture: options[:quiet] + else + log("Active Storage installation was skipped. Please run `bin/rails active_storage:install` to install Active Storage files.") + end + end + end + def empty_directory_with_keep_file(destination, config = {}) empty_directory(destination, config) keep_file(destination) diff --git a/railties/lib/rails/generators/named_base.rb b/railties/lib/rails/generators/named_base.rb index 44f5ab45d3..99165168fd 100644 --- a/railties/lib/rails/generators/named_base.rb +++ b/railties/lib/rails/generators/named_base.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/module/introspection" require "rails/generators/base" require "rails/generators/generated_attribute" @@ -158,26 +157,26 @@ module Rails def model_resource_name(prefix: "") # :doc: resource_name = "#{prefix}#{singular_table_name}" - if controller_class_path.empty? - resource_name - else + if options[:model_name] "[#{controller_class_path.map { |name| ":" + name }.join(", ")}, #{resource_name}]" + else + resource_name end end def singular_route_name # :doc: - if controller_class_path.empty? - singular_table_name - else + if options[:model_name] "#{controller_class_path.join('_')}_#{singular_table_name}" + else + singular_table_name end end def plural_route_name # :doc: - if controller_class_path.empty? - plural_table_name - else + if options[:model_name] "#{controller_class_path.join('_')}_#{plural_table_name}" + else + plural_table_name end end @@ -222,7 +221,7 @@ module Rails # def self.check_class_collision(options = {}) # :doc: define_method :check_class_collision do - name = if respond_to?(:controller_class_name) # for ScaffoldBase + name = if respond_to?(:controller_class_name) # for ResourceHelpers controller_class_name else class_name diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 46954f64b5..1c32fad3ea 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -114,7 +114,7 @@ module Rails template "cable.yml" unless options[:skip_action_cable] template "puma.rb" unless options[:skip_puma] template "spring.rb" if spring_install? - template "storage.yml" + template "storage.yml" unless skip_active_storage? directory "environments" directory "initializers" @@ -139,7 +139,7 @@ module Rails template "config/cable.yml" end - if !active_storage_config_exist + if !skip_active_storage? && !active_storage_config_exist template "config/storage.yml" end @@ -355,6 +355,10 @@ module Rails build(:system_test) if depends_on_system_test? end + def create_storage_files + build(:storage) unless skip_active_storage? + end + def create_tmp_files build(:tmp) end @@ -457,6 +461,7 @@ module Rails public_task :apply_rails_template, :run_bundle public_task :run_webpack, :generate_spring_binstubs + public_task :run_active_storage def run_after_bundle_callbacks @after_bundle_callbacks.each(&:call) diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index 95e8be96b6..ee9db83ca2 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -20,9 +20,10 @@ ruby <%= "'#{RUBY_VERSION}'" -%> # Use ActiveModel has_secure_password # gem 'bcrypt', '~> 3.1.7' - +<% unless skip_active_storage? -%> # Use ActiveStorage variant # gem 'mini_magick', '~> 4.8' +<% end -%> # Use Capistrano for deployment # gem 'capistrano-rails', group: :development diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt index 62fd04f113..5183bcd256 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt +++ b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt @@ -12,7 +12,9 @@ // <% unless options[:skip_javascript] -%> //= require rails-ujs +<% unless skip_active_storage? -%> //= require activestorage +<% end -%> <% unless options[:skip_turbolinks] -%> //= require turbolinks <% end -%> diff --git a/railties/lib/rails/generators/rails/app/templates/config/application.rb b/railties/lib/rails/generators/rails/app/templates/config/application.rb index dde09edb94..9e03e86771 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/application.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/application.rb @@ -8,10 +8,10 @@ require "rails" require "active_model/railtie" require "active_job/railtie" <%= comment_if :skip_active_record %>require "active_record/railtie" +<%= comment_if :skip_active_storage %>require "active_storage/engine" require "action_controller/railtie" <%= comment_if :skip_action_mailer %>require "action_mailer/railtie" require "action_view/railtie" -require "active_storage/engine" <%= comment_if :skip_action_cable %>require "action_cable/engine" <%= comment_if :skip_sprockets %>require "sprockets/railtie" <%= comment_if :skip_test %>require "rails/test_unit/railtie" diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt index 98689cc30d..2746aedd6f 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt @@ -27,8 +27,10 @@ Rails.application.configure do config.cache_store = :null_store end + <%- unless skip_active_storage? -%> # Store uploaded files on the local file system (see config/storage.yml for options) config.active_storage.service = :local + <%- end -%> <%- unless options.skip_action_mailer? -%> # Don't care if the mailer can't send. diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt index 2e0b555f6f..4c0f36db98 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt @@ -44,9 +44,11 @@ Rails.application.configure do # config.action_dispatch.x_sendfile_header = 'X-Sendfile' # for Apache # config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for NGINX + <%- unless skip_active_storage? -%> # Store uploaded files on the local file system (see config/storage.yml for options) config.active_storage.service = :local + <%- end -%> <%- unless options[:skip_action_cable] -%> # Mount Action Cable outside main process or domain # config.action_cable.mount_path = nil diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt index a53978bc6e..a19f490d91 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt @@ -28,8 +28,11 @@ Rails.application.configure do # Disable request forgery protection in test environment. config.action_controller.allow_forgery_protection = false + <%- unless skip_active_storage? -%> # Store uploaded files on the local file system in a temporary directory config.active_storage.service = :test + + <%- end -%> <%- unless options.skip_action_mailer? -%> config.action_mailer.perform_caching = false diff --git a/railties/lib/rails/generators/rails/app/templates/gitignore b/railties/lib/rails/generators/rails/app/templates/gitignore index 83a7b211aa..bd6b34346a 100644 --- a/railties/lib/rails/generators/rails/app/templates/gitignore +++ b/railties/lib/rails/generators/rails/app/templates/gitignore @@ -21,8 +21,10 @@ !/tmp/.keep <% end -%> +<% unless skip_active_storage? -%> # Ignore uploaded files in development /storage/* +<% end -%> <% unless options.skip_yarn? -%> /node_modules diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index d9ca4712d0..bc2dcf008e 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -87,7 +87,7 @@ task default: :test end PASSTHROUGH_OPTIONS = [ - :skip_active_record, :skip_action_mailer, :skip_javascript, :skip_action_cable, :skip_sprockets, :database, + :skip_active_record, :skip_active_storage, :skip_action_mailer, :skip_javascript, :skip_action_cable, :skip_sprockets, :database, :javascript, :skip_yarn, :api, :quiet, :pretend, :skip ] diff --git a/railties/lib/rails/generators/rails/plugin/templates/gitignore b/railties/lib/rails/generators/rails/plugin/templates/gitignore index e15863d860..7a68da5c4b 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/gitignore +++ b/railties/lib/rails/generators/rails/plugin/templates/gitignore @@ -11,5 +11,8 @@ pkg/ <%= dummy_path %>/node_modules/ <%= dummy_path %>/yarn-error.log <% end -%> +<% unless skip_active_storage? -%> +<%= dummy_path %>/storage/ +<% end -%> <%= dummy_path %>/tmp/ <% end -%> diff --git a/railties/lib/rails/generators/rails/plugin/templates/rails/application.rb b/railties/lib/rails/generators/rails/plugin/templates/rails/application.rb index 47b56ae3df..06ffe2f1ed 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/rails/application.rb +++ b/railties/lib/rails/generators/rails/plugin/templates/rails/application.rb @@ -8,10 +8,10 @@ require "rails" require "active_model/railtie" require "active_job/railtie" <%= comment_if :skip_active_record %>require "active_record/railtie" +<%= comment_if :skip_active_storage %>require "active_storage/engine" require "action_controller/railtie" <%= comment_if :skip_action_mailer %>require "action_mailer/railtie" require "action_view/railtie" -require "active_storage/engine" <%= comment_if :skip_action_cable %>require "action_cable/engine" <%= comment_if :skip_sprockets %>require "sprockets/railtie" <%= comment_if :skip_test %>require "rails/test_unit/railtie" diff --git a/railties/lib/rails/generators/rails/plugin/templates/rails/javascripts.js b/railties/lib/rails/generators/rails/plugin/templates/rails/javascripts.js index e54c6461cc..f3d80c87f5 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/rails/javascripts.js +++ b/railties/lib/rails/generators/rails/plugin/templates/rails/javascripts.js @@ -10,4 +10,7 @@ // Read Sprockets README (https://github.com/rails/sprockets#sprockets-directives) for details // about supported directives. // +<% unless skip_active_storage? -%> +//= require activestorage +<% end -%> //= require_tree . diff --git a/railties/test/application/generators_test.rb b/railties/test/application/generators_test.rb index 47c815d221..e5e557d204 100644 --- a/railties/test/application/generators_test.rb +++ b/railties/test/application/generators_test.rb @@ -188,10 +188,11 @@ module ApplicationTests Rails::Command.send(:remove_const, "APP_PATH") end - test "help does not show hidden namespaces" do + test "help does not show hidden namespaces and hidden commands" do FileUtils.cd(rails_root) do output = rails("generate", "--help") assert_no_match "active_record:migration", output + assert_no_match "credentials", output output = rails("destroy", "--help") assert_no_match "active_record:migration", output diff --git a/railties/test/application/integration_test_case_test.rb b/railties/test/application/integration_test_case_test.rb index 9edc907fce..c08761092b 100644 --- a/railties/test/application/integration_test_case_test.rb +++ b/railties/test/application/integration_test_case_test.rb @@ -1,10 +1,11 @@ # frozen_string_literal: true require "isolation/abstract_unit" +require "env_helpers" module ApplicationTests class IntegrationTestCaseTest < ActiveSupport::TestCase - include ActiveSupport::Testing::Isolation + include ActiveSupport::Testing::Isolation, EnvHelpers setup do build_app @@ -39,13 +40,14 @@ module ApplicationTests end RUBY + with_rails_env("test") { rails("db:migrate") } output = rails("test") assert_match(/0 failures, 0 errors/, output) end end class IntegrationTestDefaultApp < ActiveSupport::TestCase - include ActiveSupport::Testing::Isolation + include ActiveSupport::Testing::Isolation, EnvHelpers setup do build_app @@ -66,6 +68,7 @@ module ApplicationTests end RUBY + with_rails_env("test") { rails("db:migrate") } output = rails("test") assert_match(/0 failures, 0 errors/, output) end diff --git a/railties/test/application/middleware/exceptions_test.rb b/railties/test/application/middleware/exceptions_test.rb index 75afeec905..2d659ade8d 100644 --- a/railties/test/application/middleware/exceptions_test.rb +++ b/railties/test/application/middleware/exceptions_test.rb @@ -102,7 +102,7 @@ module ApplicationTests end end - test "routing to an nonexistent controller when action_dispatch.show_exceptions and consider_all_requests_local are set shows diagnostics" do + test "routing to a nonexistent controller when action_dispatch.show_exceptions and consider_all_requests_local are set shows diagnostics" do app_file "config/routes.rb", <<-RUBY Rails.application.routes.draw do resources :articles diff --git a/railties/test/application/rake/migrations_test.rb b/railties/test/application/rake/migrations_test.rb index 33f2b7038c..788f9160d6 100644 --- a/railties/test/application/rake/migrations_test.rb +++ b/railties/test/application/rake/migrations_test.rb @@ -37,9 +37,64 @@ module ApplicationTests assert_match(/AMigration: reverted/, output) end + test "migrate with specified VERSION in different formats" do + app_file "db/migrate/01_one_migration.rb", <<-MIGRATION + class OneMigration < ActiveRecord::Migration::Current + end + MIGRATION + + app_file "db/migrate/02_two_migration.rb", <<-MIGRATION + class TwoMigration < ActiveRecord::Migration::Current + end + MIGRATION + + app_file "db/migrate/03_three_migration.rb", <<-MIGRATION + class ThreeMigration < ActiveRecord::Migration::Current + end + MIGRATION + + rails "db:migrate" + + output = rails("db:migrate:status") + assert_match(/up\s+001\s+One migration/, output) + assert_match(/up\s+002\s+Two migration/, output) + assert_match(/up\s+003\s+Three migration/, output) + + rails "db:migrate", "VERSION=01_one_migration.rb" + output = rails("db:migrate:status") + assert_match(/up\s+001\s+One migration/, output) + assert_match(/down\s+002\s+Two migration/, output) + assert_match(/down\s+003\s+Three migration/, output) + + rails "db:migrate", "VERSION=3" + output = rails("db:migrate:status") + assert_match(/up\s+001\s+One migration/, output) + assert_match(/up\s+002\s+Two migration/, output) + assert_match(/up\s+003\s+Three migration/, output) + + rails "db:migrate", "VERSION=001" + output = rails("db:migrate:status") + assert_match(/up\s+001\s+One migration/, output) + assert_match(/down\s+002\s+Two migration/, output) + assert_match(/down\s+003\s+Three migration/, output) + end + test "migration with empty version" do - output = rails("db:migrate", "VERSION=", allow_failure: true) - assert_match(/Empty VERSION provided/, output) + app_file "db/migrate/01_one_migration.rb", <<-MIGRATION + class OneMigration < ActiveRecord::Migration::Current + end + MIGRATION + + app_file "db/migrate/02_two_migration.rb", <<-MIGRATION + class TwoMigration < ActiveRecord::Migration::Current + end + MIGRATION + + rails("db:migrate", "VERSION=") + + output = rails("db:migrate:status") + assert_match(/up\s+001\s+One migration/, output) + assert_match(/up\s+002\s+Two migration/, output) output = rails("db:migrate:redo", "VERSION=", allow_failure: true) assert_match(/Empty VERSION provided/, output) @@ -55,6 +110,34 @@ module ApplicationTests output = rails("db:migrate:down", allow_failure: true) assert_match(/VERSION is required - To go down one migration, use db:rollback/, output) + + output = rails("db:migrate:status") + assert_match(/up\s+001\s+One migration/, output) + assert_match(/up\s+002\s+Two migration/, output) + end + + test "migration with 0 version" do + app_file "db/migrate/01_one_migration.rb", <<-MIGRATION + class OneMigration < ActiveRecord::Migration::Current + end + MIGRATION + + app_file "db/migrate/02_two_migration.rb", <<-MIGRATION + class TwoMigration < ActiveRecord::Migration::Current + end + MIGRATION + + rails "db:migrate" + + output = rails("db:migrate:status") + assert_match(/up\s+001\s+One migration/, output) + assert_match(/up\s+002\s+Two migration/, output) + + rails "db:migrate", "VERSION=0" + + output = rails("db:migrate:status") + assert_match(/down\s+001\s+One migration/, output) + assert_match(/down\s+002\s+Two migration/, output) end test "model and migration generator with change syntax" do @@ -192,6 +275,75 @@ module ApplicationTests end end + test "raise error on any move when target migration does not exist" do + app_file "db/migrate/01_one_migration.rb", <<-MIGRATION + class OneMigration < ActiveRecord::Migration::Current + end + MIGRATION + + app_file "db/migrate/02_two_migration.rb", <<-MIGRATION + class TwoMigration < ActiveRecord::Migration::Current + end + MIGRATION + + rails "db:migrate" + + output = rails("db:migrate:status") + assert_match(/up\s+001\s+One migration/, output) + assert_match(/up\s+002\s+Two migration/, output) + + output = rails("db:migrate", "VERSION=3", allow_failure: true) + assert_match(/rails aborted!/, output) + assert_match(/ActiveRecord::UnknownMigrationVersionError:/, output) + assert_match(/No migration with version number 3/, output) + + output = rails("db:migrate:status") + assert_match(/up\s+001\s+One migration/, output) + assert_match(/up\s+002\s+Two migration/, output) + end + + test "raise error on any move when VERSION has invalid format" do + output = rails("db:migrate", "VERSION=unknown", allow_failure: true) + assert_match(/rails aborted!/, output) + assert_match(/Invalid format of target version/, output) + + output = rails("db:migrate", "VERSION=0.1.11", allow_failure: true) + assert_match(/rails aborted!/, output) + assert_match(/Invalid format of target version/, output) + + output = rails("db:migrate", "VERSION=1.1.11", allow_failure: true) + assert_match(/rails aborted!/, output) + assert_match(/Invalid format of target version/, output) + + output = rails("db:migrate", "VERSION='0 '", allow_failure: true) + assert_match(/rails aborted!/, output) + assert_match(/Invalid format of target version/, output) + + output = rails("db:migrate", "VERSION=1.", allow_failure: true) + assert_match(/rails aborted!/, output) + assert_match(/Invalid format of target version/, output) + + output = rails("db:migrate", "VERSION=1_", allow_failure: true) + assert_match(/rails aborted!/, output) + assert_match(/Invalid format of target version/, output) + + output = rails("db:migrate", "VERSION=1_name", allow_failure: true) + assert_match(/rails aborted!/, output) + assert_match(/Invalid format of target version/, output) + + output = rails("db:migrate:redo", "VERSION=unknown", allow_failure: true) + assert_match(/rails aborted!/, output) + assert_match(/Invalid format of target version/, output) + + output = rails("db:migrate:up", "VERSION=unknown", allow_failure: true) + assert_match(/rails aborted!/, output) + assert_match(/Invalid format of target version/, output) + + output = rails("db:migrate:down", "VERSION=unknown", allow_failure: true) + assert_match(/rails aborted!/, output) + assert_match(/Invalid format of target version/, output) + end + test "migration status after rollback and redo without timestamps" do add_to_config("config.active_record.timestamped_migrations = false") diff --git a/railties/test/generators/actions_test.rb b/railties/test/generators/actions_test.rb index 3ecfb4edd9..f421207025 100644 --- a/railties/test/generators/actions_test.rb +++ b/railties/test/generators/actions_test.rb @@ -308,6 +308,14 @@ class ActionsTest < Rails::Generators::TestCase end end + test "rake command with capture option should run rake command with capture" do + assert_called_with(generator, :run, ["rake log:clear RAILS_ENV=development", verbose: false, capture: true]) do + with_rails_env nil do + action :rake, "log:clear", capture: true + end + end + end + test "rails command should run rails_command with default env" do assert_called_with(generator, :run, ["rails log:clear RAILS_ENV=development", verbose: false]) do with_rails_env nil do @@ -346,6 +354,14 @@ class ActionsTest < Rails::Generators::TestCase end end + test "rails command with capture option should run rails_command with capture" do + assert_called_with(generator, :run, ["rails log:clear RAILS_ENV=development", verbose: false, capture: true]) do + with_rails_env nil do + action :rails_command, "log:clear", capture: true + end + end + end + def test_capify_should_run_the_capify_command content = capture(:stderr) do assert_called_with(generator, :run, ["capify .", verbose: false]) do diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 160ad1a928..78962ee30b 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -75,6 +75,7 @@ DEFAULT_APP_FILES = %w( log package.json public + storage test/application_system_test_case.rb test/test_helper.rb test/fixtures @@ -89,6 +90,7 @@ DEFAULT_APP_FILES = %w( tmp tmp/cache tmp/cache/assets + tmp/storage ) class AppGeneratorTest < Rails::Generators::TestCase @@ -296,6 +298,80 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_active_storage_mini_magick_gem + run_generator + assert_file "Gemfile", /^# gem 'mini_magick'/ + end + + def test_active_storage_install + command_check = -> command, _ do + @binstub_called ||= 0 + case command + when "active_storage:install" + @binstub_called += 1 + assert_equal 1, @binstub_called, "active_storage:install expected to be called once, but was called #{@install_called} times." + end + end + + generator.stub :rails_command, command_check do + quietly { generator.invoke_all } + end + end + + def test_app_update_does_not_generate_active_storage_contents_when_skip_active_storage_is_given + app_root = File.join(destination_root, "myapp") + run_generator [app_root, "--skip-active-storage"] + + FileUtils.cd(app_root) do + quietly { system("bin/rails app:update") } + end + + assert_file "#{app_root}/config/environments/development.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_file "#{app_root}/config/environments/production.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_file "#{app_root}/config/environments/test.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_no_file "#{app_root}/config/storage.yml" + + assert_file "#{app_root}/Gemfile" do |content| + assert_no_match(/gem 'mini_magick'/, content) + end + end + + def test_app_update_does_not_generate_active_storage_contents_when_skip_active_record_is_given + app_root = File.join(destination_root, "myapp") + run_generator [app_root, "--skip-active-record"] + + FileUtils.cd(app_root) do + quietly { system("bin/rails app:update") } + end + + assert_file "#{app_root}/config/environments/development.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_file "#{app_root}/config/environments/production.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_file "#{app_root}/config/environments/test.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_no_file "#{app_root}/config/storage.yml" + + assert_file "#{app_root}/Gemfile" do |content| + assert_no_match(/gem 'mini_magick'/, content) + end + end + def test_application_names_are_not_singularized run_generator [File.join(destination_root, "hats")] assert_file "hats/config/environment.rb", /Rails\.application\.initialize!/ @@ -742,7 +818,7 @@ class AppGeneratorTest < Rails::Generators::TestCase template end - sequence = ["git init", "install", "exec spring binstub --all", "echo ran after_bundle"] + sequence = ["git init", "install", "exec spring binstub --all", "active_storage:install", "echo ran after_bundle"] @sequence_step ||= 0 ensure_bundler_first = -> command, options = nil do assert_equal sequence[@sequence_step], command, "commands should be called in sequence #{sequence}" @@ -752,12 +828,14 @@ class AppGeneratorTest < Rails::Generators::TestCase generator([destination_root], template: path).stub(:open, check_open, template) do generator.stub(:bundle_command, ensure_bundler_first) do generator.stub(:run, ensure_bundler_first) do - quietly { generator.invoke_all } + generator.stub(:rails_command, ensure_bundler_first) do + quietly { generator.invoke_all } + end end end end - assert_equal 4, @sequence_step + assert_equal 5, @sequence_step end def test_system_tests_directory_generated diff --git a/railties/test/generators/named_base_test.rb b/railties/test/generators/named_base_test.rb index 967754c813..4e61b660d7 100644 --- a/railties/test/generators/named_base_test.rb +++ b/railties/test/generators/named_base_test.rb @@ -33,6 +33,17 @@ class NamedBaseTest < Rails::Generators::TestCase assert_name g, "foos", :plural_name assert_name g, "admin.foo", :i18n_scope assert_name g, "admin_foos", :table_name + assert_name g, "admin/foos", :controller_name + assert_name g, %w(admin), :controller_class_path + assert_name g, "Admin::Foos", :controller_class_name + assert_name g, "admin/foos", :controller_file_path + assert_name g, "foos", :controller_file_name + assert_name g, "admin.foos", :controller_i18n_scope + assert_name g, "admin_foo", :singular_route_name + assert_name g, "admin_foos", :plural_route_name + assert_name g, "@admin_foo", :redirect_resource_name + assert_name g, "admin_foo", :model_resource_name + assert_name g, "admin_foos", :index_helper end def test_named_generator_attributes_as_ruby @@ -47,6 +58,17 @@ class NamedBaseTest < Rails::Generators::TestCase assert_name g, "foos", :plural_name assert_name g, "admin.foo", :i18n_scope assert_name g, "admin_foos", :table_name + assert_name g, "Admin::Foos", :controller_name + assert_name g, %w(admin), :controller_class_path + assert_name g, "Admin::Foos", :controller_class_name + assert_name g, "admin/foos", :controller_file_path + assert_name g, "foos", :controller_file_name + assert_name g, "admin.foos", :controller_i18n_scope + assert_name g, "admin_foo", :singular_route_name + assert_name g, "admin_foos", :plural_route_name + assert_name g, "@admin_foo", :redirect_resource_name + assert_name g, "admin_foo", :model_resource_name + assert_name g, "admin_foos", :index_helper end def test_named_generator_attributes_without_pluralized diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 38130ceb68..49256883d6 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -240,8 +240,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase run_generator [destination_root, "--full", "--skip_active_record"] FileUtils.cd destination_root quietly { system "bundle install" } - # FIXME: Active Storage will provoke a test error without ActiveRecord (fix by allowing to skip active storage) - assert_match(/1 runs, 0 assertions, 0 failures, 1 errors/, `bundle exec rake test 2>&1`) + assert_match(/1 runs, 1 assertions, 0 failures, 0 errors/, `bundle exec rake test 2>&1`) end def test_ensure_that_migration_tasks_work_with_mountable_option diff --git a/railties/test/generators/scaffold_controller_generator_test.rb b/railties/test/generators/scaffold_controller_generator_test.rb index 513b037043..fd5aa817b4 100644 --- a/railties/test/generators/scaffold_controller_generator_test.rb +++ b/railties/test/generators/scaffold_controller_generator_test.rb @@ -207,6 +207,7 @@ class ScaffoldControllerGeneratorTest < Rails::Generators::TestCase Dir.chdir(engine_path) do quietly { `bin/rails g controller dashboard foo` } + quietly { `bin/rails db:migrate RAILS_ENV=test` } assert_match(/2 runs, 2 assertions, 0 failures, 0 errors/, `bin/rails test 2>&1`) end end @@ -218,6 +219,7 @@ class ScaffoldControllerGeneratorTest < Rails::Generators::TestCase Dir.chdir(engine_path) do quietly { `bin/rails g controller dashboard foo` } + quietly { `bin/rails db:migrate RAILS_ENV=test` } assert_match(/2 runs, 2 assertions, 0 failures, 0 errors/, `bin/rails test 2>&1`) end end diff --git a/railties/test/generators/scaffold_generator_test.rb b/railties/test/generators/scaffold_generator_test.rb index 03322c1c59..b6294c3b94 100644 --- a/railties/test/generators/scaffold_generator_test.rb +++ b/railties/test/generators/scaffold_generator_test.rb @@ -282,7 +282,14 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase /class Admin::RolesTest < ApplicationSystemTestCase/ # Views - %w(index edit new show _form).each do |view| + assert_file "app/views/admin/roles/index.html.erb" do |content| + assert_match("'Show', admin_role", content) + assert_match("'Edit', edit_admin_role_path(admin_role)", content) + assert_match("'Destroy', admin_role", content) + assert_match("'New Admin Role', new_admin_role_path", content) + end + + %w(edit new show _form).each do |view| assert_file "app/views/admin/roles/#{view}.html.erb" end assert_no_file "app/views/layouts/admin/roles.html.erb" diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index 654d16de65..6b746f3fed 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -129,13 +129,26 @@ module SharedGeneratorTests end def test_default_frameworks_are_required_when_others_are_removed - run_generator [destination_root, "--skip-active-record", "--skip-action-mailer", "--skip-action-cable", "--skip-sprockets"] - assert_file "#{application_path}/config/application.rb", /require\s+["']rails["']/ - assert_file "#{application_path}/config/application.rb", /require\s+["']active_model\/railtie["']/ - assert_file "#{application_path}/config/application.rb", /require\s+["']active_job\/railtie["']/ - assert_file "#{application_path}/config/application.rb", /require\s+["']action_controller\/railtie["']/ - assert_file "#{application_path}/config/application.rb", /require\s+["']action_view\/railtie["']/ - assert_file "#{application_path}/config/application.rb", /require\s+["']active_storage\/engine["']/ + run_generator [ + destination_root, + "--skip-active-record", + "--skip-active-storage", + "--skip-action-mailer", + "--skip-action-cable", + "--skip-sprockets" + ] + + assert_file "#{application_path}/config/application.rb", /^require\s+["']rails["']/ + assert_file "#{application_path}/config/application.rb", /^require\s+["']active_model\/railtie["']/ + assert_file "#{application_path}/config/application.rb", /^require\s+["']active_job\/railtie["']/ + assert_file "#{application_path}/config/application.rb", /^# require\s+["']active_record\/railtie["']/ + assert_file "#{application_path}/config/application.rb", /^# require\s+["']active_storage\/engine["']/ + assert_file "#{application_path}/config/application.rb", /^require\s+["']action_controller\/railtie["']/ + assert_file "#{application_path}/config/application.rb", /^# require\s+["']action_mailer\/railtie["']/ + assert_file "#{application_path}/config/application.rb", /^require\s+["']action_view\/railtie["']/ + assert_file "#{application_path}/config/application.rb", /^# require\s+["']action_cable\/engine["']/ + assert_file "#{application_path}/config/application.rb", /^# require\s+["']sprockets\/railtie["']/ + assert_file "#{application_path}/config/application.rb", /^require\s+["']rails\/test_unit\/railtie["']/ end def test_generator_without_skips @@ -189,6 +202,96 @@ module SharedGeneratorTests end end + def test_generator_for_active_storage + run_generator + + assert_file "#{application_path}/app/assets/javascripts/application.js" do |content| + assert_match(/^\/\/= require activestorage/, content) + end + + assert_file "#{application_path}/config/environments/development.rb" do |content| + assert_match(/config\.active_storage/, content) + end + + assert_file "#{application_path}/config/environments/production.rb" do |content| + assert_match(/config\.active_storage/, content) + end + + assert_file "#{application_path}/config/environments/test.rb" do |content| + assert_match(/config\.active_storage/, content) + end + + assert_file "#{application_path}/config/storage.yml" + assert_directory "#{application_path}/storage" + assert_directory "#{application_path}/tmp/storage" + + assert_file ".gitignore" do |content| + assert_match(/\/storage\//, content) + end + end + + def test_generator_if_skip_active_storage_is_given + run_generator [destination_root, "--skip-active-storage"] + + assert_file "#{application_path}/config/application.rb", /#\s+require\s+["']active_storage\/engine["']/ + + assert_file "#{application_path}/app/assets/javascripts/application.js" do |content| + assert_no_match(/^\/\/= require activestorage/, content) + end + + assert_file "#{application_path}/config/environments/development.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_file "#{application_path}/config/environments/production.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_file "#{application_path}/config/environments/test.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_no_file "#{application_path}/config/storage.yml" + assert_no_directory "#{application_path}/db/migrate" + assert_no_directory "#{application_path}/storage" + assert_no_directory "#{application_path}/tmp/storage" + + assert_file ".gitignore" do |content| + assert_no_match(/\/storage\//, content) + end + end + + def test_generator_does_not_generate_active_storage_contents_if_skip_active_record_is_given + run_generator [destination_root, "--skip-active-record"] + + assert_file "#{application_path}/config/application.rb", /#\s+require\s+["']active_storage\/engine["']/ + + assert_file "#{application_path}/app/assets/javascripts/application.js" do |content| + assert_no_match(/^\/\/= require activestorage/, content) + end + + assert_file "#{application_path}/config/environments/development.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_file "#{application_path}/config/environments/production.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_file "#{application_path}/config/environments/test.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end + + assert_no_file "#{application_path}/config/storage.yml" + assert_no_directory "#{application_path}/db/migrate" + assert_no_directory "#{application_path}/storage" + assert_no_directory "#{application_path}/tmp/storage" + + assert_file ".gitignore" do |content| + assert_no_match(/\/storage\//, content) + end + end + def test_generator_if_skip_action_mailer_is_given run_generator [destination_root, "--skip-action-mailer"] assert_file "#{application_path}/config/application.rb", /#\s+require\s+["']action_mailer\/railtie["']/ diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 29daaacdb2..7522237a38 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -153,6 +153,7 @@ module TestHelpers def teardown_app ENV["RAILS_ENV"] = @prev_rails_env if @prev_rails_env + FileUtils.rm_rf(tmp_path) end # Make a very basic app, without creating the whole directory structure. diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index e6964b4b18..fc710feb63 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -32,6 +32,11 @@ module RailtiesTest require "#{app_path}/config/environment" end + def migrations + migration_root = File.expand_path(ActiveRecord::Migrator.migrations_paths.first, app_path) + ActiveRecord::Migrator.migrations(migration_root) + end + test "serving sprocket's assets" do @plugin.write "app/assets/javascripts/engine.js.erb", "<%= :alert %>();" add_to_env_config "development", "config.assets.digest = false" @@ -82,31 +87,32 @@ module RailtiesTest end RUBY - add_to_config "ActiveRecord::Base.timestamped_migrations = false" - boot_rails Dir.chdir(app_path) do + # Install Active Storage migration file first so as not to affect test. + `bundle exec rake active_storage:install` output = `bundle exec rake bukkits:install:migrations` - assert File.exist?("#{app_path}/db/migrate/2_create_users.bukkits.rb") - assert File.exist?("#{app_path}/db/migrate/3_add_last_name_to_users.bukkits.rb") - assert_match(/Copied migration 2_create_users\.bukkits\.rb from bukkits/, output) - assert_match(/Copied migration 3_add_last_name_to_users\.bukkits\.rb from bukkits/, output) - assert_match(/NOTE: Migration 3_create_sessions\.rb from bukkits has been skipped/, output) - assert_equal 3, Dir["#{app_path}/db/migrate/*.rb"].length + ["CreateUsers", "AddLastNameToUsers", "CreateSessions"].each do |migration_name| + assert migrations.detect { |migration| migration.name == migration_name } + end + assert_match(/Copied migration \d+_create_users\.bukkits\.rb from bukkits/, output) + assert_match(/Copied migration \d+_add_last_name_to_users\.bukkits\.rb from bukkits/, output) + assert_match(/NOTE: Migration \d+_create_sessions\.rb from bukkits has been skipped/, output) - output = `bundle exec rake railties:install:migrations`.split("\n") + migrations_count = Dir["#{app_path}/db/migrate/*.rb"].length - assert_no_match(/2_create_users/, output.join("\n")) + assert_equal migrations.length, migrations_count - bukkits_migration_order = output.index(output.detect { |o| /NOTE: Migration 3_create_sessions\.rb from bukkits has been skipped/ =~ o }) - assert_not_nil bukkits_migration_order, "Expected migration to be skipped" - - migrations_count = Dir["#{app_path}/db/migrate/*.rb"].length - `bundle exec rake railties:install:migrations` + output = `bundle exec rake railties:install:migrations`.split("\n") assert_equal migrations_count, Dir["#{app_path}/db/migrate/*.rb"].length + + assert_no_match(/\d+_create_users/, output.join("\n")) + + bukkits_migration_order = output.index(output.detect { |o| /NOTE: Migration \d+_create_sessions\.rb from bukkits has been skipped/ =~ o }) + assert_not_nil bukkits_migration_order, "Expected migration to be skipped" end end @@ -171,10 +177,12 @@ module RailtiesTest boot_rails Dir.chdir(app_path) do + # Install Active Storage migration file first so as not to affect test. + `bundle exec rake active_storage:install` output = `bundle exec rake railties:install:migrations`.split("\n") - assert_match(/Copied migration \d+_create_users\.core_engine\.rb from core_engine/, output.second) - assert_match(/Copied migration \d+_create_keys\.api_engine\.rb from api_engine/, output.last) + assert_match(/Copied migration \d+_create_users\.core_engine\.rb from core_engine/, output.first) + assert_match(/Copied migration \d+_create_keys\.api_engine\.rb from api_engine/, output.second) end end @@ -203,9 +211,12 @@ module RailtiesTest Dir.chdir(@plugin.path) do output = `bundle exec rake app:bukkits:install:migrations` - assert File.exist?("#{app_path}/db/migrate/0_add_first_name_to_users.bukkits.rb") - assert_match(/Copied migration 0_add_first_name_to_users\.bukkits\.rb from bukkits/, output) - assert_equal 1, Dir["#{app_path}/db/migrate/*.rb"].length + + migration_with_engine_path = migrations.detect { |migration| migration.name == "AddFirstNameToUsers" } + assert migration_with_engine_path + assert_match(/\/db\/migrate\/\d+_add_first_name_to_users\.bukkits\.rb/, migration_with_engine_path.filename) + assert_match(/Copied migration \d+_add_first_name_to_users\.bukkits\.rb from bukkits/, output) + assert_equal migrations.length, Dir["#{app_path}/db/migrate/*.rb"].length end end |